-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat(op-reth): implement block builder failsafe and interop filter integration #20001
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
e6a4445
a77a564
2cb23dc
10e4de4
77c3a34
0f258de
1b72e98
102787d
097cd00
e22750a
b686ae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| package filter | ||
|
|
||
| import ( | ||
| "context" | ||
| "math/rand" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/ethereum/go-ethereum/core/types" | ||
| "github.com/ethereum/go-ethereum/crypto" | ||
|
|
||
| "github.com/ethereum-optimism/optimism/op-acceptance-tests/tests/interop" | ||
| "github.com/ethereum-optimism/optimism/op-core/predeploys" | ||
| "github.com/ethereum-optimism/optimism/op-devstack/devtest" | ||
| "github.com/ethereum-optimism/optimism/op-devstack/dsl" | ||
| "github.com/ethereum-optimism/optimism/op-devstack/presets" | ||
| "github.com/ethereum-optimism/optimism/op-service/eth" | ||
| "github.com/ethereum-optimism/optimism/op-service/retry" | ||
| "github.com/ethereum-optimism/optimism/op-service/txplan" | ||
| suptypes "github.com/ethereum-optimism/optimism/op-supervisor/supervisor/types" | ||
| ) | ||
|
|
||
| func setupInteropFilterTest(t devtest.T) *presets.TwoL2SupernodeInterop { | ||
| return presets.NewTwoL2SupernodeInterop(t, 0, presets.WithInteropFilter()) | ||
| } | ||
|
|
||
| // waitForFailsafeState confirms the interop filter's state matches expected, | ||
| // then waits for two L2 blocks to ensure op-reth's 1s polling task has had | ||
| // time to pick up the change. This replaces time.Sleep-based waits. | ||
| func waitForFailsafeState(t devtest.T, sys *presets.TwoL2SupernodeInterop, expected bool) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function doesn't seem to wait for a given state, it seems to expect that state up-front, then do some waiting. |
||
| t.Require().Equal(expected, sys.InteropFilter.FailsafeEnabled(), | ||
| "interop filter failsafe state should already be %v after SetFailsafeEnabled", expected) | ||
| // Op-reth polls admin_getFailsafeEnabled every 1s. With 2s L2 block times, | ||
| // waiting for 2 blocks guarantees at least 2 poll cycles have elapsed. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why does it matter that 2 poll cycles elapse? |
||
| sys.L2B.WaitForBlock() | ||
| sys.L2B.WaitForBlock() | ||
| } | ||
|
|
||
| // TestInteropFilter_IngressAcceptsValid verifies that a valid interop transaction | ||
| // with correct cross-chain references passes through the interop filter. | ||
| func TestInteropFilter_IngressAcceptsValid(gt *testing.T) { | ||
| t := devtest.ParallelT(gt) | ||
| sys := setupInteropFilterTest(t) | ||
|
|
||
| alice := sys.FunderA.NewFundedEOA(eth.OneHundredthEther) | ||
| bob := sys.FunderB.NewFundedEOA(eth.OneHundredthEther) | ||
|
|
||
| eventLoggerAddress := alice.DeployEventLogger() | ||
|
|
||
| sys.L2B.CatchUpTo(sys.L2A) | ||
|
|
||
| // Send init message on chain A | ||
| rng := rand.New(rand.NewSource(time.Now().UnixNano())) | ||
| initMsg := alice.SendInitMessage(interop.RandomInitTrigger(rng, eventLoggerAddress, 2, 10)) | ||
|
|
||
| // Wait for at least one block between init and exec | ||
| sys.L2B.WaitForBlock() | ||
|
|
||
| // Send exec message on chain B — the interop filter validates the access list | ||
| execMsg := bob.SendExecMessage(initMsg) | ||
|
Comment on lines
+57
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the interop filter check is only implied, right? this function I'm guessing the DSL already has something for sending an init/exec message pair and waiting their inclusions. |
||
|
|
||
| // Verify cross-safe safety passes for both messages | ||
| dsl.CheckAll(t, | ||
| sys.L2ACL.ReachedRefFn(suptypes.CrossSafe, initMsg.BlockID(), 500), | ||
| sys.L2BCL.ReachedRefFn(suptypes.CrossSafe, execMsg.BlockID(), 500), | ||
| ) | ||
| } | ||
|
|
||
| // TestInteropFilter_IngressRejectsInvalid verifies that a transaction with fabricated | ||
| // CrossL2Inbox access list entries is rejected by the interop filter. | ||
| func TestInteropFilter_IngressRejectsInvalid(gt *testing.T) { | ||
| t := devtest.ParallelT(gt) | ||
| sys := setupInteropFilterTest(t) | ||
| require := t.Require() | ||
|
|
||
| bob := sys.FunderB.NewFundedEOA(eth.OneHundredthEther) | ||
|
|
||
| // Construct a fabricated access list entry with a random storage key | ||
| // that the filter won't recognize as a valid cross-chain message | ||
| fakeStorageKey := crypto.Keccak256Hash([]byte("fabricated-inbox-entry")) | ||
| accessList := types.AccessList{{ | ||
| Address: predeploys.CrossL2InboxAddr, | ||
| StorageKeys: []common.Hash{fakeStorageKey}, | ||
| }} | ||
|
|
||
| // Send a transaction with the fabricated access list. | ||
| // The interop filter should reject this because the inbox entry doesn't | ||
| // correspond to any real cross-chain message. | ||
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
| defer cancel() | ||
|
|
||
| bobAddr := bob.Address() | ||
| elClient := sys.L2ELB.EthClient() | ||
| tx := txplan.NewPlannedTx( | ||
| bob.Plan(), | ||
| // Override retry submission with single-attempt submitter so the | ||
| // filter's rejection propagates immediately instead of retrying | ||
| // until the context expires. | ||
| txplan.WithTransactionSubmitter(elClient), | ||
| txplan.WithTo(&bobAddr), | ||
| txplan.WithValue(eth.GWei(1)), | ||
| txplan.WithAccessList(accessList), | ||
| txplan.WithGasLimit(100_000), | ||
| ) | ||
|
|
||
| // The transaction should be explicitly rejected by the interop filter. | ||
| _, err := tx.Submitted.Eval(ctx) | ||
| require.Error(err, "transaction with fabricated access list should not be included") | ||
| require.Contains(err.Error(), "failed to parse access entry", | ||
| "expected interop filter rejection, got: %v", err) | ||
| } | ||
|
|
||
| // TestInteropFilter_FailsafeLifecycle verifies the full failsafe lifecycle: | ||
| // interop txs succeed normally, are blocked when failsafe is enabled, | ||
| // and recover after failsafe is disabled. | ||
| func TestInteropFilter_FailsafeLifecycle(gt *testing.T) { | ||
| t := devtest.ParallelT(gt) | ||
| sys := setupInteropFilterTest(t) | ||
| require := t.Require() | ||
|
|
||
| alice := sys.FunderA.NewFundedEOA(eth.OneHundredthEther) | ||
| bob := sys.FunderB.NewFundedEOA(eth.OneHundredthEther) | ||
|
|
||
| eventLoggerAddress := alice.DeployEventLogger() | ||
| sys.L2B.CatchUpTo(sys.L2A) | ||
|
|
||
| // Step 1: Send a valid interop tx — should succeed before failsafe | ||
| rng := rand.New(rand.NewSource(time.Now().UnixNano())) | ||
| initMsg := alice.SendInitMessage(interop.RandomInitTrigger(rng, eventLoggerAddress, 2, 10)) | ||
| sys.L2B.WaitForBlock() | ||
| execMsg := bob.SendExecMessage(initMsg) | ||
| require.Equal(types.ReceiptStatusSuccessful, execMsg.Receipt.Status, | ||
| "interop tx should succeed before failsafe") | ||
|
|
||
| // Step 2: Enable failsafe and wait for propagation to op-reth | ||
| require.NotNil(sys.InteropFilter, "interop filter must be configured") | ||
| sys.InteropFilter.SetFailsafeEnabled(true) | ||
| waitForFailsafeState(t, sys, true) | ||
|
|
||
| // Step 3: Send another init message and try exec — should fail | ||
| initMsg2 := alice.SendInitMessage(interop.RandomInitTrigger(rng, eventLoggerAddress, 1, 5)) | ||
| sys.L2B.WaitForBlock() | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) | ||
| defer cancel() | ||
|
|
||
| // During failsafe, even valid access lists should be rejected | ||
| result, err := initMsg2.Tx.Result.Eval(ctx) | ||
| require.NoError(err, "init message result must be available") | ||
| require.Greater(len(result.Entries), 0, "init message must have entries") | ||
|
|
||
| msg := result.Entries[0] | ||
| accessList := types.AccessList{{ | ||
| Address: predeploys.CrossL2InboxAddr, | ||
| StorageKeys: suptypes.EncodeAccessList([]suptypes.Access{msg.Access()}), | ||
| }} | ||
|
|
||
| bobAddr := bob.Address() | ||
| elClient := sys.L2ELB.EthClient() | ||
| tx := txplan.NewPlannedTx( | ||
| bob.Plan(), | ||
| txplan.WithTransactionSubmitter(elClient), | ||
| txplan.WithTo(&bobAddr), | ||
| txplan.WithValue(eth.GWei(1)), | ||
| txplan.WithAccessList(accessList), | ||
| txplan.WithGasLimit(100_000), | ||
| ) | ||
|
|
||
| _, err = tx.Submitted.Eval(ctx) | ||
| require.Error(err, "interop tx should be rejected during failsafe") | ||
| require.Contains(err.Error(), "interop failsafe is active", | ||
| "expected failsafe rejection, got: %v", err) | ||
|
|
||
| // Step 4: Disable failsafe and wait for propagation | ||
| sys.InteropFilter.SetFailsafeEnabled(false) | ||
| waitForFailsafeState(t, sys, false) | ||
|
|
||
| // Step 5: Verify interop txs recover — retry to tolerate propagation lag | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm a little concerned about how flakey this might be in practice . could the DSL support some sort of |
||
| err = retry.Do0(context.Background(), 5, &retry.FixedStrategy{Dur: 2 * time.Second}, func() error { | ||
| initMsg3 := alice.SendInitMessage(interop.RandomInitTrigger(rng, eventLoggerAddress, 1, 3)) | ||
| sys.L2B.WaitForBlock() | ||
| _ = bob.SendExecMessage(initMsg3) | ||
| return nil | ||
| }) | ||
| require.NoError(err, "interop flow should recover after failsafe disabled") | ||
| } | ||
|
|
||
| // TestInteropFilter_NonInteropUnaffected verifies that regular (non-interop) | ||
| // transactions are accepted on both chains regardless of failsafe state. | ||
| func TestInteropFilter_NonInteropUnaffected(gt *testing.T) { | ||
| t := devtest.ParallelT(gt) | ||
| sys := setupInteropFilterTest(t) | ||
| require := t.Require() | ||
|
|
||
| aliceA := sys.FunderA.NewFundedEOA(eth.OneHundredthEther) | ||
| bobA := sys.FunderA.NewFundedEOA(eth.OneHundredthEther) | ||
| aliceB := sys.FunderB.NewFundedEOA(eth.OneHundredthEther) | ||
| bobB := sys.FunderB.NewFundedEOA(eth.OneHundredthEther) | ||
|
|
||
| // Enable failsafe and wait for propagation | ||
| require.NotNil(sys.InteropFilter, "interop filter must be configured") | ||
| sys.InteropFilter.SetFailsafeEnabled(true) | ||
| waitForFailsafeState(t, sys, true) | ||
|
|
||
| // Send regular (non-interop) transfers on both chains — should succeed even during failsafe | ||
| txA := aliceA.Transfer(bobA.Address(), eth.GWei(1000)) | ||
| receiptA, err := txA.Included.Eval(context.Background()) | ||
| require.NoError(err, "regular transfer on chain A should succeed during failsafe") | ||
| require.Equal(types.ReceiptStatusSuccessful, receiptA.Status, "regular transfer on chain A should succeed") | ||
|
|
||
| txB := aliceB.Transfer(bobB.Address(), eth.GWei(1000)) | ||
| receiptB, err := txB.Included.Eval(context.Background()) | ||
| require.NoError(err, "regular transfer on chain B should succeed during failsafe") | ||
| require.Equal(types.ReceiptStatusSuccessful, receiptB.Status, "regular transfer on chain B should succeed") | ||
|
|
||
| // Disable failsafe | ||
| sys.InteropFilter.SetFailsafeEnabled(false) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a feeling some of these tests might already exist through other presets/packages, and we could hopefully leverage that.
like
TestInteropFilter_IngressAcceptsValidis equal to all other "Valid Message" tests, just with the filter turned on.