Skip to content

interop: address strong-consistency review follow-ups (#19561) #20194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wwared wants to merge 7 commits into from
Open

interop: address strong-consistency review follow-ups (#19561) #20194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c04b920
interop: replace nil-check with no-op l1Checker in tests (#19561)
wwared Apr 17, 2026
77f797c
interop: serialize Decision as a string in the WAL (#19561)
wwared Apr 17, 2026
27ec9ff
interop: compare deserialized VerifiedResult on idempotent replay (#1…
wwared Apr 17, 2026
e10cdc9
interop: add rewind-replay-after-failure test (#19561)
wwared Apr 20, 2026
b7e6030
interop: add progressAndRecord integration tests (#19561)
wwared Apr 20, 2026
318ad04
interop: pass frontierView through call chain (#19561)
wwared Apr 20, 2026
fd5deda
interop: hide reorg ops behind InteropChain interface (#19561)
wwared Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions op-supernode/supernode/activity/interop/algo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (i *Interop) l1Inclusion(ts uint64, blocksAtTimestamp blockPerChain) (eth.B
// - Verify the initiating message exists in the source chain's logsDB
// - Verify the initiating message timestamp <= executing message timestamp
// - Verify the initiating message hasn't expired (within message expiry window)
func (i *Interop) verifyInteropMessages(ts uint64, blocksAtTimestamp blockPerChain) (Result, error) {
func (i *Interop) verifyInteropMessages(ts uint64, blocksAtTimestamp blockPerChain, view *frontierVerificationView) (Result, error) {
result := Result{
Timestamp: ts,
L2Heads: make(blockPerChain),
Expand All @@ -78,7 +78,7 @@ func (i *Interop) verifyInteropMessages(ts uint64, blocksAtTimestamp blockPerCha
execMsgs map[uint32]*types.ExecutingMessage
err error
)
if frontierBlock, ok := i.frontierView.block(chainID); ok {
if frontierBlock, ok := view.block(chainID); ok {
blockRef = frontierBlock.ref
execMsgs = frontierBlock.execMsgs
} else {
Expand Down Expand Up @@ -141,7 +141,7 @@ func (i *Interop) verifyInteropMessages(ts uint64, blocksAtTimestamp blockPerCha
// Verify each executing message
blockValid := true
for logIdx, execMsg := range execMsgs {
err := i.verifyExecutingMessage(chainID, blockRef.Time, logIdx, execMsg)
err := i.verifyExecutingMessage(chainID, blockRef.Time, logIdx, execMsg, view)
if err != nil {
i.log.Warn("invalid executing message",
"chain", chainID,
Expand Down Expand Up @@ -172,7 +172,7 @@ func (i *Interop) verifyInteropMessages(ts uint64, blocksAtTimestamp blockPerCha
// 1. The initiating message exists in the source chain's database
// 2. The initiating message's timestamp is not greater than the executing block's timestamp
// 3. The initiating message hasn't expired (timestamp + messageExpiryWindow >= executing timestamp)
func (i *Interop) verifyExecutingMessage(executingChain eth.ChainID, executingTimestamp uint64, logIdx uint32, execMsg *types.ExecutingMessage) error {
func (i *Interop) verifyExecutingMessage(executingChain eth.ChainID, executingTimestamp uint64, logIdx uint32, execMsg *types.ExecutingMessage, view *frontierVerificationView) error {
// Get the source chain's logsDB
sourceDB, ok := i.logsDBs[execMsg.ChainID]
if !ok {
Expand Down Expand Up @@ -202,7 +202,7 @@ func (i *Interop) verifyExecutingMessage(executingChain eth.ChainID, executingTi
// Same-timestamp dependencies may live in the current frontier view rather
// than accepted-history logsDB.
if execMsg.Timestamp == executingTimestamp {
if _, ok := i.frontierView.contains(execMsg.ChainID, query); ok {
if _, ok := view.contains(execMsg.ChainID, query); ok {
return nil
}
}
Expand Down
38 changes: 19 additions & 19 deletions op-supernode/supernode/activity/interop/algo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type verifyInteropTestCase struct {
func runVerifyInteropTest(t *testing.T, tc verifyInteropTestCase) {
t.Parallel()
interop, timestamp, blocks := tc.setup()
result, err := interop.verifyInteropMessages(timestamp, blocks)
result, err := interop.verifyInteropMessages(timestamp, blocks, nil)
Copy link
Copy Markdown
Contributor

@ajsutton ajsutton Apr 21, 2026

Choose a reason for hiding this comment

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

How does this not crash with a nil pointer?

Copy link
Copy Markdown
Contributor Author

@wwared wwared Apr 23, 2026

Choose a reason for hiding this comment

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

It's a bit tricky, but this is actually a typed nil pointer (of type *frontierVerificationView), so it calls into this function which exits early because it never dereferences v:

optimism/op-supernode/supernode/activity/interop/verification_view.go

Lines 93 to 95 in f6746ef

func (v *frontierVerificationView) block(chainID eth.ChainID) (frontierBlockView, bool) {
if v == nil {
return frontierBlockView{}, false

The same happens here:

optimism/op-supernode/supernode/activity/interop/verification_view.go

Lines 101 to 105 in f6746ef

func (v *frontierVerificationView) contains(chainID eth.ChainID, query suptypes.ContainsQuery) (suptypes.BlockSeal, bool) {
block, ok := v.block(chainID)
if !ok {
return suptypes.BlockSeal{}, false
}


if tc.expectError {
require.Error(t, err)
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestL1Inclusion(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{},
chains: map[eth.ChainID]cc.ChainContainer{chainID: &algoMockChain{id: chainID, optimisticL1: l1Block}},
chains: map[eth.ChainID]cc.InteropChain{chainID: &algoMockChain{id: chainID, optimisticL1: l1Block}},
}
return interop, 1000, map[eth.ChainID]eth.BlockID{chainID: expectedBlock}
},
Expand All @@ -109,7 +109,7 @@ func TestL1Inclusion(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
chain1ID: &algoMockChain{id: chain1ID, optimisticL1: eth.BlockID{Number: 60, Hash: common.HexToHash("0xL1_1")}},
chain2ID: &algoMockChain{id: chain2ID, optimisticL1: eth.BlockID{Number: 45, Hash: common.HexToHash("0xL1_2")}},
chain3ID: &algoMockChain{id: chain3ID, optimisticL1: eth.BlockID{Number: 50, Hash: common.HexToHash("0xL1_3")}},
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestL1Inclusion(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
chain1ID: &algoMockChain{id: chain1ID, optimisticL1: l1Block1},
// chain2ID NOT in chains map
},
Expand All @@ -160,7 +160,7 @@ func TestL1Inclusion(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
chainID: &algoMockChain{id: chainID, optimisticAtErr: errors.New("optimistic at error")},
},
}
Expand All @@ -178,7 +178,7 @@ func TestL1Inclusion(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{},
chains: map[eth.ChainID]cc.ChainContainer{},
chains: map[eth.ChainID]cc.InteropChain{},
}
return interop, 1000, map[eth.ChainID]eth.BlockID{}
},
Expand All @@ -197,7 +197,7 @@ func TestL1Inclusion(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{},
chains: map[eth.ChainID]cc.ChainContainer{chainID: &algoMockChain{id: chainID, optimisticL1: l1Block}},
chains: map[eth.ChainID]cc.InteropChain{chainID: &algoMockChain{id: chainID, optimisticL1: l1Block}},
}
return interop, 0, map[eth.ChainID]eth.BlockID{
chainID: {Number: 0, Hash: common.HexToHash("0x123")},
Expand Down Expand Up @@ -254,7 +254,7 @@ func TestVerifyInteropMessages(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{chainID: mockDB},
chains: map[eth.ChainID]cc.ChainContainer{chainID: newMockChainWithL1(chainID, l1Block)},
chains: map[eth.ChainID]cc.InteropChain{chainID: newMockChainWithL1(chainID, l1Block)},
}

return interop, 1000, map[eth.ChainID]eth.BlockID{chainID: expectedBlock}
Expand Down Expand Up @@ -307,7 +307,7 @@ func TestVerifyInteropMessages(t *testing.T) {
sourceChainID: sourceDB,
destChainID: destDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
sourceChainID: newMockChainWithL1(sourceChainID, l1Block),
destChainID: newMockChainWithL1(destChainID, l1Block),
},
Expand Down Expand Up @@ -367,7 +367,7 @@ func TestVerifyInteropMessages(t *testing.T) {
sourceChainID: sourceDB,
destChainID: destDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
sourceChainID: newMockChainWithL1(sourceChainID, l1Block),
destChainID: newMockChainWithL1(destChainID, l1Block),
},
Expand Down Expand Up @@ -429,7 +429,7 @@ func TestVerifyInteropMessages(t *testing.T) {
sourceChainID: sourceDB,
destChainID: destDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
sourceChainID: newMockChainWithL1(sourceChainID, l1Block),
destChainID: newMockChainWithL1(destChainID, l1Block),
},
Expand Down Expand Up @@ -464,7 +464,7 @@ func TestVerifyInteropMessages(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{registeredChain: mockDB},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
registeredChain: newMockChainWithL1(registeredChain, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
},
}
Expand Down Expand Up @@ -502,7 +502,7 @@ func TestVerifyInteropMessages(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{chainID: mockDB},
chains: map[eth.ChainID]cc.ChainContainer{chainID: newMockChainWithL1(chainID, l1Block, expectedBlock)},
chains: map[eth.ChainID]cc.InteropChain{chainID: newMockChainWithL1(chainID, l1Block, expectedBlock)},
}

return interop, 1000, map[eth.ChainID]eth.BlockID{chainID: expectedBlock}
Expand Down Expand Up @@ -550,7 +550,7 @@ func TestVerifyInteropMessages(t *testing.T) {
sourceChainID: sourceDB,
destChainID: destDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
sourceChainID: newMockChainWithL1(sourceChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
destChainID: newMockChainWithL1(destChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}, destBlock),
},
Expand Down Expand Up @@ -602,7 +602,7 @@ func TestVerifyInteropMessages(t *testing.T) {
sourceChainID: sourceDB,
destChainID: destDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
sourceChainID: newMockChainWithL1(sourceChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
destChainID: newMockChainWithL1(destChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}, destBlock),
},
Expand Down Expand Up @@ -647,7 +647,7 @@ func TestVerifyInteropMessages(t *testing.T) {
destChainID: destDB,
// Note: unknownSourceChain NOT in logsDBs
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
unknownSourceChain: newMockChainWithL1(unknownSourceChain, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
destChainID: newMockChainWithL1(destChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}, destBlock),
},
Expand Down Expand Up @@ -701,7 +701,7 @@ func TestVerifyInteropMessages(t *testing.T) {
sourceChainID: sourceDB,
destChainID: destDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
sourceChainID: newMockChainWithL1(sourceChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
destChainID: newMockChainWithL1(destChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}, destBlock),
},
Expand Down Expand Up @@ -760,7 +760,7 @@ func TestVerifyInteropMessages(t *testing.T) {
validChainID: validDB,
invalidChainID: invalidDB,
},
chains: map[eth.ChainID]cc.ChainContainer{
chains: map[eth.ChainID]cc.InteropChain{
invalidChainID: newMockChainWithL1(invalidChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}, invalidBlock),
sourceChainID: newMockChainWithL1(sourceChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
validChainID: newMockChainWithL1(validChainID, eth.BlockID{Number: 40, Hash: common.HexToHash("0xL1")}),
Expand Down Expand Up @@ -800,7 +800,7 @@ func TestVerifyInteropMessages(t *testing.T) {
messageExpiryWindow: defaultMessageExpiryWindow,
log: gethlog.New(),
logsDBs: map[eth.ChainID]LogsDB{chainID: mockDB},
chains: map[eth.ChainID]cc.ChainContainer{chainID: newMockChainWithL1(chainID, l1Block)},
chains: map[eth.ChainID]cc.InteropChain{chainID: newMockChainWithL1(chainID, l1Block)},
}

return interop, 1000, map[eth.ChainID]eth.BlockID{chainID: block}
Expand Down
18 changes: 12 additions & 6 deletions op-supernode/supernode/activity/interop/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,27 @@ type l1ByNumberSource interface {
L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1BlockRef, error)
}

// byNumberConsistencyChecker verifies that a set of L1 block IDs all belong to
// the same L1 fork by comparing each against the canonical chain.
type byNumberConsistencyChecker struct {
// l1ConsistencyChecker verifies that a set of L1 block IDs all belong to
// the same L1 fork.
type l1ConsistencyChecker interface {
SameL1Chain(ctx context.Context, heads []eth.BlockID) (bool, error)
}

// l1ByNumberChecker is the production l1ConsistencyChecker: it checks each
// head against the canonical L1 chain fetched from an l1ByNumberSource.
type l1ByNumberChecker struct {
l1 l1ByNumberSource
}

func newByNumberConsistencyChecker(l1 l1ByNumberSource) *byNumberConsistencyChecker {
func newL1ConsistencyChecker(l1 l1ByNumberSource) l1ConsistencyChecker {
if l1 == nil {
return nil
}
return &byNumberConsistencyChecker{l1: l1}
return &l1ByNumberChecker{l1: l1}
}

// SameL1Chain returns true if all non-zero heads belong to the same canonical L1 chain.
func (c *byNumberConsistencyChecker) SameL1Chain(ctx context.Context, heads []eth.BlockID) (bool, error) {
func (c *l1ByNumberChecker) SameL1Chain(ctx context.Context, heads []eth.BlockID) (bool, error) {
for _, head := range heads {
if head == (eth.BlockID{}) {
continue
Expand Down
22 changes: 19 additions & 3 deletions op-supernode/supernode/activity/interop/checker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,23 @@ func (m *mockL1Source) L1BlockRefByNumber(ctx context.Context, num uint64) (eth.
return ref, nil
}

func TestByNumberConsistencyChecker_SameL1Chain(t *testing.T) {
// noopL1Checker treats every set of heads as canonical. Intended for tests that
// do not exercise L1 consistency — production must always use the real checker.
type noopL1Checker struct{}

func (noopL1Checker) SameL1Chain(context.Context, []eth.BlockID) (bool, error) {
return true, nil
}

// inconsistentL1Checker always reports that heads disagree on the canonical L1
// chain. Tests use it to drive observeRound into a rewind decision.
type inconsistentL1Checker struct{}

func (inconsistentL1Checker) SameL1Chain(context.Context, []eth.BlockID) (bool, error) {
return false, nil
}

func TestL1ConsistencyChecker_SameL1Chain(t *testing.T) {
t.Parallel()

hashA := common.HexToHash("0xaaaa")
Expand All @@ -34,7 +50,7 @@ func TestByNumberConsistencyChecker_SameL1Chain(t *testing.T) {
200: {Hash: hashB, Number: 200},
},
}
checker := newByNumberConsistencyChecker(source)
checker := newL1ConsistencyChecker(source)

t.Run("all match canonical", func(t *testing.T) {
same, err := checker.SameL1Chain(context.Background(), []eth.BlockID{
Expand Down Expand Up @@ -71,7 +87,7 @@ func TestByNumberConsistencyChecker_SameL1Chain(t *testing.T) {
})

t.Run("nil checker returns nil", func(t *testing.T) {
nilChecker := newByNumberConsistencyChecker(nil)
nilChecker := newL1ConsistencyChecker(nil)
require.Nil(t, nilChecker)
})
}
4 changes: 2 additions & 2 deletions op-supernode/supernode/activity/interop/cycle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func buildCycleGraph(ts uint64, chainEMs map[eth.ChainID]map[uint32]*types.Execu
// using Kahn's topological sort algorithm.
//
// Returns a Result with InvalidHeads populated for chains participating in cycles.
func (i *Interop) verifyCycleMessages(ts uint64, blocksAtTimestamp map[eth.ChainID]eth.BlockID) (Result, error) {
func (i *Interop) verifyCycleMessages(ts uint64, blocksAtTimestamp map[eth.ChainID]eth.BlockID, view *frontierVerificationView) (Result, error) {
result := Result{
Timestamp: ts,
L2Heads: blocksAtTimestamp,
Expand All @@ -178,7 +178,7 @@ func (i *Interop) verifyCycleMessages(ts uint64, blocksAtTimestamp map[eth.Chain
// collect all EMs for the given blocks per chain
chainEMs := make(map[eth.ChainID]map[uint32]*types.ExecutingMessage)
for chainID, blockID := range blocksAtTimestamp {
if frontierBlock, ok := i.frontierView.block(chainID); ok {
if frontierBlock, ok := view.block(chainID); ok {
if frontierBlock.ref.Time == ts {
chainEMs[chainID] = frontierBlock.execMsgs
}
Expand Down
Loading
Loading