Skip to content

Commit bd29b7d

Browse files
karlfloerschopsuperchainclaudegeoknee
authored
interop: add strong consistency guarantees (ethereum-optimism#19505)
* interop: add consistency guarantees with minimal diff Add 5 targeted consistency fixes to the supernode interop activity, restructured around an observe/decide/execute pattern with a pure Decide() function for testability. Fixes: - Crash-safe invalidations via WAL pattern in VerifiedDB - L1 fork consistency check via byNumberConsistencyChecker - Accepted snapshot re-verification (L2 drift + L1 canonicality) - Deny-list decision provenance via DecisionTimestamp + pruning - Queued resets to eliminate race between Reset() and main loop Additional hardening: - Stale logsDB detection in loadLogs (same-height hash mismatch) - Idempotent startup recovery (WAL replay, deny-list prune, logsDB trim) - Deterministic chain iteration (sorted by ChainID) - VerifiedDB internal locking (sync.RWMutex) - RewindEngine atomic guard against concurrent resets - ErrL1AtSafeHeadNotFound mapped to ethereum.NotFound - WAL preserved on invalidation failure for retry - VerifiedBlockAtL1 scan bounded by activationTimestamp Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * interop: unify recovery and transition apply path Squashed follow-up commits: - interop: unify recovery and transition apply path - interop: remove dead wrappers and stale comments - interop: make rewind transition building explicit - interop: fail closed on invalid rewind WAL - interop: report all rewind apply errors - chain-container: drop unused ClearDenied hook - chain-container: remove dead denylist clear helper - chain-container: restrict rewinds to interop callers - Revert "chain-container: restrict rewinds to interop callers" - chain-container: document reorg footgun - supernode: make interop reset callback a silent no-op - interop: clarify block-time warning Signed-off-by: Karl Floersch <karl@oplabs.co> * Update op-supernode/supernode/chain_container/chain_container.go Co-authored-by: George Knee <georgeknee@googlemail.com> * interop: address review nits and add follow-up issue ref Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Signed-off-by: Karl Floersch <karl@oplabs.co> Co-authored-by: opsuperchain <opsuperchain@slop.bot> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: George Knee <georgeknee@googlemail.com>
1 parent 02330cf commit bd29b7d

21 files changed

Lines changed: 1982 additions & 646 deletions

op-supernode/supernode/activity/interop/algo.go

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -73,39 +73,49 @@ func (i *Interop) verifyInteropMessages(ts uint64, blocksAtTimestamp blockPerCha
7373
}
7474

7575
for chainID, expectedBlock := range blocksAtTimestamp {
76-
db, ok := i.logsDBs[chainID]
77-
if !ok {
78-
// Skip chains that we don't have a logsDB for
79-
// This can happen if blocksAtTimestamp includes chains not registered with the interop activity
80-
continue
81-
}
76+
var (
77+
blockRef eth.BlockRef
78+
execMsgs map[uint32]*types.ExecutingMessage
79+
err error
80+
)
81+
if frontierBlock, ok := i.frontierView.block(chainID); ok {
82+
blockRef = frontierBlock.ref
83+
execMsgs = frontierBlock.execMsgs
84+
} else {
85+
db, ok := i.logsDBs[chainID]
86+
if !ok {
87+
// Skip chains that we don't have a logsDB for
88+
// This can happen if blocksAtTimestamp includes chains not registered with the interop activity
89+
continue
90+
}
8291

83-
// Get the block from the logsDB
84-
blockRef, _, execMsgs, err := db.OpenBlock(expectedBlock.Number)
85-
if err != nil {
86-
// OpenBlock fails for the first block in the DB because it tries to find the parent.
87-
// Handle this by checking if this is the first sealed block and using FirstSealedBlock instead.
88-
if errors.Is(err, types.ErrSkipped) {
89-
firstBlock, firstErr := db.FirstSealedBlock()
90-
if firstErr != nil {
91-
return Result{}, fmt.Errorf("chain %s: failed to open block %d and failed to get first block: %w", chainID, expectedBlock.Number, err)
92-
}
93-
if firstBlock.Number == expectedBlock.Number {
94-
// This is the first block in the logsDB. Use FirstSealedBlock info.
95-
// The first block has no executing messages (since we can't verify them without prior data).
96-
if firstBlock.Hash != expectedBlock.Hash {
97-
i.log.Warn("first block hash mismatch",
98-
"chain", chainID,
99-
"expected", expectedBlock.Hash,
100-
"got", firstBlock.Hash,
101-
)
102-
result.InvalidHeads[chainID] = expectedBlock
92+
// Get the block from the logsDB
93+
blockRef, _, execMsgs, err = db.OpenBlock(expectedBlock.Number)
94+
if err != nil {
95+
// OpenBlock fails for the first block in the DB because it tries to find the parent.
96+
// Handle this by checking if this is the first sealed block and using FirstSealedBlock instead.
97+
if errors.Is(err, types.ErrSkipped) {
98+
firstBlock, firstErr := db.FirstSealedBlock()
99+
if firstErr != nil {
100+
return Result{}, fmt.Errorf("chain %s: failed to open block %d and failed to get first block: %w", chainID, expectedBlock.Number, err)
101+
}
102+
if firstBlock.Number == expectedBlock.Number {
103+
// This is the first block in the logsDB. Use FirstSealedBlock info.
104+
// The first block has no executing messages (since we can't verify them without prior data).
105+
if firstBlock.Hash != expectedBlock.Hash {
106+
i.log.Warn("first block hash mismatch",
107+
"chain", chainID,
108+
"expected", expectedBlock.Hash,
109+
"got", firstBlock.Hash,
110+
)
111+
result.InvalidHeads[chainID] = expectedBlock
112+
}
113+
result.L2Heads[chainID] = expectedBlock
114+
continue
103115
}
104-
result.L2Heads[chainID] = expectedBlock
105-
continue
106116
}
117+
return Result{}, fmt.Errorf("chain %s: failed to open block %d: %w", chainID, expectedBlock.Number, err)
107118
}
108-
return Result{}, fmt.Errorf("chain %s: failed to open block %d: %w", chainID, expectedBlock.Number, err)
109119
}
110120

111121
// Verify the block hash matches what we expect
@@ -177,6 +187,14 @@ func (i *Interop) verifyExecutingMessage(executingChain eth.ChainID, executingTi
177187
Checksum: execMsg.Checksum,
178188
}
179189

190+
// Same-timestamp dependencies may live in the current frontier view rather
191+
// than accepted-history logsDB.
192+
if execMsg.Timestamp == executingTimestamp {
193+
if _, ok := i.frontierView.contains(execMsg.ChainID, query); ok {
194+
return nil
195+
}
196+
}
197+
180198
// Check if the initiating message exists in the source chain's logsDB
181199
_, err := sourceDB.Contains(query)
182200
return err

op-supernode/supernode/activity/interop/algo_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,9 +929,12 @@ func (m *algoMockChain) RewindEngine(ctx context.Context, timestamp uint64, inva
929929
return nil
930930
}
931931
func (m *algoMockChain) BlockTime() uint64 { return 1 }
932-
func (m *algoMockChain) InvalidateBlock(ctx context.Context, height uint64, payloadHash common.Hash) (bool, error) {
932+
func (m *algoMockChain) InvalidateBlock(ctx context.Context, height uint64, payloadHash common.Hash, decisionTimestamp uint64) (bool, error) {
933933
return false, nil
934934
}
935+
func (m *algoMockChain) PruneDeniedAtOrAfterTimestamp(timestamp uint64) (map[uint64][]common.Hash, error) {
936+
return nil, nil
937+
}
935938
func (m *algoMockChain) IsDenied(height uint64, payloadHash common.Hash) (bool, error) {
936939
return false, nil
937940
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package interop
2+
3+
import (
4+
"context"
5+
6+
"github.com/ethereum-optimism/optimism/op-service/eth"
7+
)
8+
9+
// l1ByNumberSource provides L1 block lookups by number for consistency checking.
10+
type l1ByNumberSource interface {
11+
L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1BlockRef, error)
12+
}
13+
14+
// byNumberConsistencyChecker verifies that a set of L1 block IDs all belong to
15+
// the same L1 fork by comparing each against the canonical chain.
16+
type byNumberConsistencyChecker struct {
17+
l1 l1ByNumberSource
18+
}
19+
20+
func newByNumberConsistencyChecker(l1 l1ByNumberSource) *byNumberConsistencyChecker {
21+
if l1 == nil {
22+
return nil
23+
}
24+
return &byNumberConsistencyChecker{l1: l1}
25+
}
26+
27+
// SameL1Chain returns true if all non-zero heads belong to the same canonical L1 chain.
28+
func (c *byNumberConsistencyChecker) SameL1Chain(ctx context.Context, heads []eth.BlockID) (bool, error) {
29+
for _, head := range heads {
30+
if head == (eth.BlockID{}) {
31+
continue
32+
}
33+
canonical, err := c.l1.L1BlockRefByNumber(ctx, head.Number)
34+
if err != nil {
35+
return false, err
36+
}
37+
if canonical.Hash != head.Hash {
38+
return false, nil
39+
}
40+
}
41+
return true, nil
42+
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package interop
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/ethereum-optimism/optimism/op-service/eth"
9+
"github.com/ethereum/go-ethereum/common"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
type mockL1Source struct {
14+
blocks map[uint64]eth.L1BlockRef
15+
}
16+
17+
func (m *mockL1Source) L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1BlockRef, error) {
18+
ref, ok := m.blocks[num]
19+
if !ok {
20+
return eth.L1BlockRef{}, fmt.Errorf("block %d not found", num)
21+
}
22+
return ref, nil
23+
}
24+
25+
func TestByNumberConsistencyChecker_SameL1Chain(t *testing.T) {
26+
t.Parallel()
27+
28+
hashA := common.HexToHash("0xaaaa")
29+
hashB := common.HexToHash("0xbbbb")
30+
31+
source := &mockL1Source{
32+
blocks: map[uint64]eth.L1BlockRef{
33+
100: {Hash: hashA, Number: 100},
34+
200: {Hash: hashB, Number: 200},
35+
},
36+
}
37+
checker := newByNumberConsistencyChecker(source)
38+
39+
t.Run("all match canonical", func(t *testing.T) {
40+
same, err := checker.SameL1Chain(context.Background(), []eth.BlockID{
41+
{Hash: hashA, Number: 100},
42+
{Hash: hashB, Number: 200},
43+
})
44+
require.NoError(t, err)
45+
require.True(t, same)
46+
})
47+
48+
t.Run("mismatch detected", func(t *testing.T) {
49+
same, err := checker.SameL1Chain(context.Background(), []eth.BlockID{
50+
{Hash: hashA, Number: 100},
51+
{Hash: common.HexToHash("0xdead"), Number: 200}, // wrong hash
52+
})
53+
require.NoError(t, err)
54+
require.False(t, same)
55+
})
56+
57+
t.Run("zero block IDs skipped", func(t *testing.T) {
58+
same, err := checker.SameL1Chain(context.Background(), []eth.BlockID{
59+
{},
60+
{Hash: hashA, Number: 100},
61+
{},
62+
})
63+
require.NoError(t, err)
64+
require.True(t, same)
65+
})
66+
67+
t.Run("empty heads list", func(t *testing.T) {
68+
same, err := checker.SameL1Chain(context.Background(), []eth.BlockID{})
69+
require.NoError(t, err)
70+
require.True(t, same)
71+
})
72+
73+
t.Run("nil checker returns nil", func(t *testing.T) {
74+
nilChecker := newByNumberConsistencyChecker(nil)
75+
require.Nil(t, nilChecker)
76+
})
77+
}

op-supernode/supernode/activity/interop/cycle.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,13 @@ func (i *Interop) verifyCycleMessages(ts uint64, blocksAtTimestamp map[eth.Chain
177177
// collect all EMs for the given blocks per chain
178178
chainEMs := make(map[eth.ChainID]map[uint32]*types.ExecutingMessage)
179179
for chainID, blockID := range blocksAtTimestamp {
180+
if frontierBlock, ok := i.frontierView.block(chainID); ok {
181+
if frontierBlock.ref.Time == ts {
182+
chainEMs[chainID] = frontierBlock.execMsgs
183+
}
184+
continue
185+
}
186+
180187
db, ok := i.logsDBs[chainID]
181188
if !ok {
182189
// Chain not in logsDBs - skip it for cycle verification
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
package interop
2+
3+
import (
4+
"testing"
5+
6+
"github.com/ethereum-optimism/optimism/op-service/eth"
7+
"github.com/ethereum/go-ethereum/common"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestCheckPreconditions(t *testing.T) {
12+
t.Parallel()
13+
14+
tests := []struct {
15+
name string
16+
obs RoundObservation
17+
want *Decision
18+
}{
19+
{
20+
name: "pause when paused",
21+
obs: RoundObservation{
22+
Paused: true,
23+
ChainsReady: true,
24+
L1Consistent: true,
25+
},
26+
want: ptrDecision(DecisionWait),
27+
},
28+
{
29+
name: "wait when chains not ready",
30+
obs: RoundObservation{
31+
ChainsReady: false,
32+
},
33+
want: ptrDecision(DecisionWait),
34+
},
35+
{
36+
name: "rewind when L1 inconsistent",
37+
obs: RoundObservation{
38+
ChainsReady: true,
39+
L1Consistent: false,
40+
},
41+
want: ptrDecision(DecisionRewind),
42+
},
43+
{
44+
name: "proceed when preconditions are satisfied",
45+
obs: RoundObservation{
46+
ChainsReady: true,
47+
L1Consistent: true,
48+
},
49+
want: nil,
50+
},
51+
}
52+
53+
for _, tt := range tests {
54+
t.Run(tt.name, func(t *testing.T) {
55+
t.Parallel()
56+
got := checkPreconditions(tt.obs)
57+
if tt.want == nil {
58+
require.Nil(t, got)
59+
return
60+
}
61+
require.NotNil(t, got)
62+
require.Equal(t, *tt.want, got.Decision)
63+
})
64+
}
65+
}
66+
67+
func TestDecideVerifiedResult(t *testing.T) {
68+
t.Parallel()
69+
70+
ts := uint64(1000)
71+
validResult := Result{
72+
Timestamp: ts + 1,
73+
L1Inclusion: eth.BlockID{Hash: common.HexToHash("0xl1"), Number: 50},
74+
L2Heads: map[eth.ChainID]eth.BlockID{
75+
eth.ChainIDFromUInt64(1): {Hash: common.HexToHash("0xa"), Number: 100},
76+
},
77+
}
78+
invalidResult := Result{
79+
Timestamp: ts + 1,
80+
L1Inclusion: eth.BlockID{Hash: common.HexToHash("0xl1"), Number: 50},
81+
L2Heads: map[eth.ChainID]eth.BlockID{
82+
eth.ChainIDFromUInt64(1): {Hash: common.HexToHash("0xa"), Number: 100},
83+
},
84+
InvalidHeads: map[eth.ChainID]eth.BlockID{
85+
eth.ChainIDFromUInt64(2): {Hash: common.HexToHash("0xbad"), Number: 200},
86+
},
87+
}
88+
89+
tests := []struct {
90+
name string
91+
verified Result
92+
want Decision
93+
}{
94+
{
95+
name: "wait when verification result is empty",
96+
verified: Result{},
97+
want: DecisionWait,
98+
},
99+
{
100+
name: "invalidate on invalid verification",
101+
verified: invalidResult,
102+
want: DecisionInvalidate,
103+
},
104+
{
105+
name: "advance on valid verification",
106+
verified: validResult,
107+
want: DecisionAdvance,
108+
},
109+
}
110+
111+
for _, tt := range tests {
112+
t.Run(tt.name, func(t *testing.T) {
113+
t.Parallel()
114+
got := decideVerifiedResult(RoundObservation{}, tt.verified)
115+
require.Equal(t, tt.want, got.Decision, "unexpected decision")
116+
117+
if tt.want == DecisionInvalidate {
118+
require.NotEmpty(t, got.Result.InvalidHeads, "invalidate should carry invalid heads")
119+
}
120+
if tt.want == DecisionAdvance {
121+
require.False(t, got.Result.IsEmpty(), "advance should carry result")
122+
require.True(t, got.Result.IsValid(), "advance result should be valid")
123+
}
124+
})
125+
}
126+
}
127+
128+
func ptrDecision(d Decision) *Decision {
129+
return &d
130+
}

0 commit comments

Comments
 (0)