Skip to content

Commit ec9fee9

Browse files
authored
op-{program,supervisor}: Short-circuit inclusion check of expired msgs (#15553)
* op-{program,supervisor}: Short-circuit inclusion check of expired msgs * assert no-calls rather than return an error
1 parent 931e991 commit ec9fee9

File tree

4 files changed

+139
-9
lines changed

4 files changed

+139
-9
lines changed

op-program/client/interop/consolidate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func singleRoundConsolidation(
151151
if err != nil {
152152
return fmt.Errorf("failed to get dependency set: %w", err)
153153
}
154-
deps, err := newConsolidateCheckDeps(depset, bootInfo, consolidateState.TransitionState, superRoot.Chains, l2PreimageOracle, consolidateState)
154+
deps, err := newConsolidateCheckDeps(depset, bootInfo.Configs, consolidateState.TransitionState, superRoot.Chains, l2PreimageOracle, consolidateState)
155155
if err != nil {
156156
return fmt.Errorf("failed to create consolidate check deps: %w", err)
157157
}
@@ -265,7 +265,7 @@ type consolidateCheckDeps struct {
265265

266266
func newConsolidateCheckDeps(
267267
depset depset.DependencySet,
268-
bootInfo *boot.BootInfoInterop,
268+
configSource boot.ConfigSource,
269269
transitionState *types.TransitionState,
270270
chains []eth.ChainIDAndOutput,
271271
oracle l2.Oracle,
@@ -281,7 +281,7 @@ func newConsolidateCheckDeps(
281281
blockByHash := func(hash common.Hash) *ethtypes.Block {
282282
return oracle.BlockByHash(hash, chain.ChainID)
283283
}
284-
l2ChainConfig, err := bootInfo.Configs.ChainConfig(chain.ChainID)
284+
l2ChainConfig, err := configSource.ChainConfig(chain.ChainID)
285285
if err != nil {
286286
return nil, fmt.Errorf("no chain config available for chain ID %v: %w", chain.ChainID, err)
287287
}

op-program/client/interop/interop_test.go

Lines changed: 119 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/ethereum-optimism/optimism/op-service/eth"
2020
"github.com/ethereum-optimism/optimism/op-service/testlog"
2121
"github.com/ethereum-optimism/optimism/op-service/testutils"
22+
"github.com/ethereum-optimism/optimism/op-supervisor/supervisor/backend/cross"
2223
"github.com/ethereum-optimism/optimism/op-supervisor/supervisor/backend/depset"
2324
supervisortypes "github.com/ethereum-optimism/optimism/op-supervisor/supervisor/types"
2425
"github.com/ethereum/go-ethereum/common"
@@ -33,7 +34,22 @@ import (
3334
"github.com/stretchr/testify/require"
3435
)
3536

36-
func setupTwoChains() (*staticConfigSource, *eth.SuperV1, *stubTasks) {
37+
type chainSetupOpts struct {
38+
expiryWindow uint64
39+
}
40+
41+
func WithExpiryWindow(window uint64) func(*chainSetupOpts) {
42+
return func(opts *chainSetupOpts) {
43+
opts.expiryWindow = window
44+
}
45+
}
46+
47+
func setupTwoChains(opts ...func(*chainSetupOpts)) (*staticConfigSource, *eth.SuperV1, *stubTasks) {
48+
chainSetupOpts := &chainSetupOpts{}
49+
for _, opt := range opts {
50+
opt(chainSetupOpts)
51+
}
52+
3753
rollupCfg1 := chaincfg.OPSepolia()
3854
chainCfg1 := chainconfig.OPSepoliaChainConfig()
3955

@@ -49,14 +65,23 @@ func setupTwoChains() (*staticConfigSource, *eth.SuperV1, *stubTasks) {
4965
{ChainID: eth.ChainIDFromBig(rollupCfg2.L2ChainID), Output: eth.OutputRoot(&eth.OutputV0{BlockHash: common.Hash{0x22}})},
5066
},
5167
}
52-
depset, _ := depset.NewStaticConfigDependencySet(map[eth.ChainID]*depset.StaticConfigDependency{
53-
eth.ChainIDFromBig(rollupCfg1.L2ChainID): {ChainIndex: chainA, ActivationTime: 0, HistoryMinTime: 0},
54-
eth.ChainIDFromBig(rollupCfg2.L2ChainID): {ChainIndex: chainB, ActivationTime: 0, HistoryMinTime: 0},
55-
})
68+
69+
var ds *depset.StaticConfigDependencySet
70+
if chainSetupOpts.expiryWindow > 0 {
71+
ds, _ = depset.NewStaticConfigDependencySetWithMessageExpiryOverride(map[eth.ChainID]*depset.StaticConfigDependency{
72+
eth.ChainIDFromBig(rollupCfg1.L2ChainID): {ChainIndex: chainA, ActivationTime: 0, HistoryMinTime: 0},
73+
eth.ChainIDFromBig(rollupCfg2.L2ChainID): {ChainIndex: chainB, ActivationTime: 0, HistoryMinTime: 0},
74+
}, chainSetupOpts.expiryWindow)
75+
} else {
76+
ds, _ = depset.NewStaticConfigDependencySet(map[eth.ChainID]*depset.StaticConfigDependency{
77+
eth.ChainIDFromBig(rollupCfg1.L2ChainID): {ChainIndex: chainA, ActivationTime: 0, HistoryMinTime: 0},
78+
eth.ChainIDFromBig(rollupCfg2.L2ChainID): {ChainIndex: chainB, ActivationTime: 0, HistoryMinTime: 0},
79+
})
80+
}
5681
configSource := &staticConfigSource{
5782
rollupCfgs: []*rollup.Config{rollupCfg1, &rollupCfg2},
5883
chainConfigs: []*params.ChainConfig{chainCfg1, &chainCfg2},
59-
depset: depset,
84+
depset: ds,
6085
}
6186
tasksStub := &stubTasks{
6287
l2SafeHead: eth.L2BlockRef{Number: 918429823450218}, // Past the claimed block
@@ -577,6 +602,94 @@ func TestPanicIfAgreedPrestateIsAfterGameTimestamp(t *testing.T) {
577602
})
578603
}
579604

605+
func TestHazardSet_ExpiredMessageShortCircuitsInclusionCheck(t *testing.T) {
606+
// This test is also covered by safe_update_test.go in op-supervisor.
607+
// However, since this short-circuit behavior is critical for fault proofs, we doubly assert the desired behavior here to prevent a regression.
608+
609+
runTest := func(t *testing.T, expiryWindow uint64, expectInclusionCheck bool) {
610+
logger := testlog.Logger(t, log.LevelError)
611+
configSource, agreedSuperRoot, tasksStub := setupTwoChains(WithExpiryWindow(expiryWindow))
612+
defer tasksStub.AssertExpectations(t)
613+
rng := rand.New(rand.NewSource(123))
614+
615+
configA := configSource.rollupCfgs[0]
616+
configB := configSource.rollupCfgs[1]
617+
618+
initLog := &gethTypes.Log{Address: initiatingMessageOrigin, Topics: []common.Hash{initiatingMessageTopic}}
619+
block1A, _ := createBlock(rng, configA, 1, gethTypes.Receipts{{Logs: []*gethTypes.Log{initLog}}})
620+
621+
exec := interoptypes.Message{
622+
Identifier: interoptypes.Identifier{
623+
Origin: initiatingMessageOrigin,
624+
BlockNumber: 1,
625+
Timestamp: block1A.Time(),
626+
ChainID: uint256.Int(eth.ChainIDFromBig(configB.L2ChainID)),
627+
},
628+
PayloadHash: initPayloadHash,
629+
}
630+
logA := convertExecutingMessageToLog(t, exec)
631+
block2A, block2AReceipts := createBlock(rng, configA, 2, gethTypes.Receipts{{Logs: []*gethTypes.Log{logA}}})
632+
block2B, block2BReceipts := createBlock(rng, configB, 2, nil)
633+
634+
pendingOutputs := [2]*eth.OutputV0{0: createOutput(block2A.Hash()), 1: createOutput(block2B.Hash())}
635+
transitionState := &types.TransitionState{
636+
SuperRoot: agreedSuperRoot.Marshal(),
637+
PendingProgress: []types.OptimisticBlock{
638+
{BlockHash: block2A.Hash(), OutputRoot: eth.OutputRoot(pendingOutputs[0])},
639+
{BlockHash: block2B.Hash(), OutputRoot: eth.OutputRoot(pendingOutputs[1])},
640+
},
641+
Step: ConsolidateStep,
642+
}
643+
l2PreimageOracle, _ := test.NewStubOracle(t)
644+
l2PreimageOracle.Blocks[block2A.Hash()] = block2A
645+
l2PreimageOracle.Blocks[block2B.Hash()] = block2B
646+
l2PreimageOracle.Receipts[block2A.Hash()] = block2AReceipts
647+
l2PreimageOracle.Receipts[block2B.Hash()] = block2BReceipts
648+
649+
consolidateState := newConsolidateState(transitionState)
650+
consolidateDeps, err := newConsolidateCheckDeps(configSource.depset, configSource, transitionState, agreedSuperRoot.Chains, l2PreimageOracle, consolidateState)
651+
require.NoError(t, err)
652+
653+
mockConsolidateDeps := &mockConsolidateDeps{consolidateCheckDeps: consolidateDeps}
654+
mockConsolidateDeps.
655+
On("Contains", mock.Anything, mock.Anything).Return(supervisortypes.BlockSeal{}, supervisortypes.ErrConflict).
656+
Maybe()
657+
658+
deps := &cross.UnsafeHazardDeps{UnsafeStartDeps: mockConsolidateDeps}
659+
candidate := supervisortypes.BlockSeal{
660+
Hash: block2A.Hash(),
661+
Number: block2A.NumberU64(),
662+
Timestamp: block2A.Time(),
663+
}
664+
_, err = cross.NewHazardSet(deps, logger, eth.ChainIDFromBig(configA.L2ChainID), candidate)
665+
require.ErrorIs(t, err, supervisortypes.ErrConflict)
666+
667+
if expectInclusionCheck {
668+
mockConsolidateDeps.AssertCalled(t, "Contains", mock.Anything, mock.Anything)
669+
} else {
670+
mockConsolidateDeps.AssertNotCalled(t, "Contains", mock.Anything, mock.Anything)
671+
}
672+
mockConsolidateDeps.AssertExpectations(t)
673+
}
674+
675+
t.Run("expired message short-circuits inclusion check", func(t *testing.T) {
676+
runTest(t, 1, false)
677+
})
678+
t.Run("message not expired does not short-circuit inclusion check", func(t *testing.T) {
679+
runTest(t, 2, true)
680+
})
681+
}
682+
683+
type mockConsolidateDeps struct {
684+
mock.Mock
685+
*consolidateCheckDeps
686+
}
687+
688+
func (m *mockConsolidateDeps) Contains(chainID eth.ChainID, query supervisortypes.ContainsQuery) (supervisortypes.BlockSeal, error) {
689+
out := m.Mock.Called(chainID, query)
690+
return out.Get(0).(supervisortypes.BlockSeal), out.Error(1)
691+
}
692+
580693
func verifyResult(t *testing.T, logger log.Logger, tasks *stubTasks, configSource *staticConfigSource, l2PreimageOracle *test.StubBlockOracle, agreedPrestate common.Hash, gameTimestamp uint64, expectedClaim common.Hash) {
581694
bootInfo := &boot.BootInfoInterop{
582695
AgreedPrestate: agreedPrestate,

op-supervisor/supervisor/backend/cross/hazard_set.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ func (h *HazardSet) checkMessageWithCurrentTimestamp(msg *types.ExecutingMessage
103103
return ok, nil
104104
}
105105

106+
// checkMessageForExpiry checks that a message has not expired.
107+
func (h *HazardSet) checkMessageForExpiry(deps HazardDeps, msg *types.ExecutingMessage, initChainID eth.ChainID, candidateTimestamp uint64) error {
108+
expiresAt := msg.Timestamp + deps.DependencySet().MessageExpiryWindow()
109+
if expiresAt < candidateTimestamp {
110+
return fmt.Errorf("timestamp of message %s (chain %s) has expired: %d < %d: %w", msg, initChainID, expiresAt, candidateTimestamp, types.ErrConflict)
111+
}
112+
return nil
113+
}
114+
106115
// build adds a block to the hazard set and recursively adds any blocks that it depends on.
107116
// Warning for future: If we have sub-second distinct blocks (different block number),
108117
// we need to increase precision on the above timestamp invariant.
@@ -147,6 +156,10 @@ func (h *HazardSet) build(deps HazardDeps, logger log.Logger, chainID eth.ChainI
147156
if err := h.checkChainCanInitiate(depSet, srcChainID, candidate, msg); err != nil {
148157
return err
149158
}
159+
// performance: short-circuit inclusion checks if the message has expired
160+
if err := h.checkMessageForExpiry(deps, msg, srcChainID, candidate.Timestamp); err != nil {
161+
return err
162+
}
150163
q := types.ChecksumArgs{
151164
BlockNumber: msg.BlockNum,
152165
LogIndex: msg.LogIdx,

op-supervisor/supervisor/backend/cross/safe_update_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,10 @@ func TestScopedCrossSafeUpdate(t *testing.T) {
459459
csd.openBlockFn = func(chainID eth.ChainID, blockNum uint64) (ref eth.BlockRef, logCount uint32, execMsgs map[uint32]*types.ExecutingMessage, err error) {
460460
return opened, 0, execs, nil
461461
}
462+
csd.checkFn = func(chainID eth.ChainID, blockNum uint64, logIdx uint32, checksum types.MessageChecksum) (types.BlockSeal, error) {
463+
require.Fail(t, "unexpected checkFn call. expected short-circuit for expired message")
464+
return types.BlockSeal{}, errors.New("unexpected checkFn call. expected short-circuit")
465+
}
462466
// when OpenBlock and CandidateCrossSafe return different blocks,
463467
// an ErrConflict is returned
464468
pair, err := scopedCrossSafeUpdate(logger, chainID, csd)

0 commit comments

Comments
 (0)