Skip to content

fix(op-supernode): guard chain container vn against torn interface read#20211

Draft
nonsense wants to merge 1 commit intodevelopfrom
fix/supernode-vn-race
Draft

fix(op-supernode): guard chain container vn against torn interface read#20211
nonsense wants to merge 1 commit intodevelopfrom
fix/supernode-vn-race

Conversation

@nonsense
Copy link
Copy Markdown
Contributor

Fixes panic at: https://app.circleci.com/pipelines/github/ethereum-optimism/optimism/123123/workflows/65b0b312-f33d-4a35-b114-b5941ee8719b/jobs/4873005


Supernode.Start fires the Interop activity and chain-container goroutines in parallel. Interop's checkChainsReady immediately calls c.OptimisticAt -> c.SyncStatus, which reads c.vn while the chain container is concurrently assigning it in its restart loop. A torn two-word interface read can yield (type=*simpleVirtualNode, data=nil); the c.vn == nil guard returns false, the method dispatches on a nil receiver, and v.mu.Lock() panics at offset 0x58 (the mu field inside simpleVirtualNode).

Protect c.vn with a sync.RWMutex and route all reads/writes through getVN/setVN helpers so every method takes one consistent snapshot. Also add nil-receiver guards on the *simpleVirtualNode methods as a defense against typed-nil interfaces slipping through in tests or future refactors.

Supernode.Start fires the Interop activity and chain-container goroutines
in parallel. Interop's checkChainsReady immediately calls c.OptimisticAt ->
c.SyncStatus, which reads c.vn while the chain container is concurrently
assigning it in its restart loop. A torn two-word interface read can yield
(type=*simpleVirtualNode, data=nil); the `c.vn == nil` guard returns false,
the method dispatches on a nil receiver, and v.mu.Lock() panics at offset
0x58 (the mu field inside simpleVirtualNode).

Protect c.vn with a sync.RWMutex and route all reads/writes through
getVN/setVN helpers so every method takes one consistent snapshot. Also
add nil-receiver guards on the *simpleVirtualNode methods as a defense
against typed-nil interfaces slipping through in tests or future refactors.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - this will fix #19573

Comment on lines +228 to +230
if v == nil {
return eth.BlockID{}, ErrVirtualNodeNotRunning
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking if self is nil is a weird pattern - we probably should just put these checks in the calling code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants