Skip to content

Commit b136257

Browse files
karlfloerschclaude
andcommitted
fix: make CheckAccessList sender parameter optional for backward compat
op-geth (v1.101701.0-rc.3) calls supervisor_checkAccessList with 3 args but the sender allow-listing PR changed the RPC to require 4. This parameter count mismatch caused ALL interop transactions to be filtered, breaking the TestInteropBlockBuilding/with_mempool_filtering e2e test. Change sender from common.Address to *common.Address, which makes it optional in go-ethereum's RPC framework. Old callers (op-geth) omit the sender and get nil; new callers (op-reth) pass it explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c4287fb commit b136257

File tree

12 files changed

+29
-24
lines changed

12 files changed

+29
-24
lines changed

op-acceptance-tests/tests/interop/upgrade/pre_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestPreNoInbox(gt *testing.T) {
105105
accessEntries := []stypes.Access{execTrigger.Msg.Access()}
106106
accessList := stypes.EncodeAccessList(accessEntries)
107107

108-
err = sys.Supervisor.Escape().QueryAPI().CheckAccessList(ctx, accessList, stypes.CrossSafe, ed, common.Address{})
108+
err = sys.Supervisor.Escape().QueryAPI().CheckAccessList(ctx, accessList, stypes.CrossSafe, ed, nil)
109109
require.ErrorContains(err, "conflicting data")
110110
}
111111

op-e2e/interop/interop_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,13 +237,13 @@ func TestInterop_EmitLogs(t *testing.T) {
237237
timestamp := uint64(time.Now().Unix())
238238
ed := types.ExecutingDescriptor{Timestamp: timestamp, ChainID: eth.ChainIDFromBig(s2.ChainID(chainB))}
239239
ctx = context.Background()
240-
err = supervisor.CheckAccessList(ctx, accessList, types.CrossSafe, ed, common.Address{})
240+
err = supervisor.CheckAccessList(ctx, accessList, types.CrossSafe, ed, nil)
241241
require.NoError(t, err, "logsA must all be cross-safe")
242242

243243
// a log should be invalid if the timestamp is incorrect
244244
accessEntries[0].Timestamp = 333
245245
accessList = types.EncodeAccessList(accessEntries)
246-
err = supervisor.CheckAccessList(ctx, accessList, types.CrossSafe, ed, common.Address{})
246+
err = supervisor.CheckAccessList(ctx, accessList, types.CrossSafe, ed, nil)
247247
require.ErrorContains(t, err, "conflict")
248248
}
249249
config := SuperSystemConfig{

op-interop-filter/filter/backend.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func supportedSafetyLevel(level types.SafetyLevel) bool {
139139

140140
// CheckAccessList validates the given access list entries.
141141
func (b *Backend) CheckAccessList(ctx context.Context, inboxEntries []common.Hash,
142-
minSafety types.SafetyLevel, execDescriptor types.ExecutingDescriptor, sender common.Address) error {
142+
minSafety types.SafetyLevel, execDescriptor types.ExecutingDescriptor, sender *common.Address) error {
143143

144144
if b.passthrough {
145145
b.metrics.RecordCheckAccessList(true)
@@ -167,8 +167,12 @@ func (b *Backend) CheckAccessList(ctx context.Context, inboxEntries []common.Has
167167
return fmt.Errorf("executing chain %s: %w", execDescriptor.ChainID, types.ErrUnknownChain)
168168
}
169169

170-
if !b.senderPolicy.Allows(sender) {
171-
b.log.Debug("Rejecting interop tx from unauthorized sender", "sender", sender)
170+
senderAddr := common.Address{}
171+
if sender != nil {
172+
senderAddr = *sender
173+
}
174+
if !b.senderPolicy.Allows(senderAddr) {
175+
b.log.Debug("Rejecting interop tx from unauthorized sender", "sender", senderAddr)
172176
b.metrics.RecordCheckAccessList(false)
173177
return fmt.Errorf("%w: %s", types.ErrUnauthorizedSender, sender)
174178
}

op-interop-filter/filter/backend_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -204,29 +204,29 @@ func TestBackend_CheckAccessList(t *testing.T) {
204204
// Failsafe enabled returns error
205205
backend, _ := newTestBackendWithMockChain(testChainA)
206206
backend.SetFailsafeEnabled(true)
207-
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 100, 0), testAllowedSender)
207+
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 100, 0), &testAllowedSender)
208208
require.ErrorIs(t, err, types.ErrFailsafeEnabled)
209209

210210
// Not ready returns error
211211
backend = newTestBackend() // No chains = not ready
212-
err = backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 100, 0), testAllowedSender)
212+
err = backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 100, 0), &testAllowedSender)
213213
require.ErrorIs(t, err, types.ErrUninitialized)
214214

215215
// Unsupported safety level returns error
216216
backend, mock := newTestBackendWithMockChain(testChainA)
217217
mock.SetReady(true)
218-
err = backend.CheckAccessList(context.Background(), nil, types.Finalized, makeExecDescriptor(testChainA, 100, 0), testAllowedSender)
218+
err = backend.CheckAccessList(context.Background(), nil, types.Finalized, makeExecDescriptor(testChainA, 100, 0), &testAllowedSender)
219219
require.Error(t, err)
220220
require.Contains(t, err.Error(), "unsupported safety level")
221221

222222
// Unknown executing chain returns error
223223
mock.SetLatestTimestamp(200)
224224
unknownChainID := uint64(999)
225-
err = backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(unknownChainID, 150, 0), testAllowedSender)
225+
err = backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(unknownChainID, 150, 0), &testAllowedSender)
226226
require.ErrorIs(t, err, types.ErrUnknownChain)
227227

228228
// LocalUnsafe with empty access list passes
229-
err = backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), testAllowedSender)
229+
err = backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), &testAllowedSender)
230230
require.NoError(t, err)
231231
}
232232

@@ -236,7 +236,7 @@ func TestBackend_CheckAccessList_RejectsUnauthorizedSender(t *testing.T) {
236236
mock.SetLatestTimestamp(200)
237237
backend.senderPolicy = mustParseTestSenderPolicy(t, common.HexToAddress("0x9999999999999999999999999999999999999999").Hex())
238238

239-
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), testAllowedSender)
239+
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), &testAllowedSender)
240240
require.ErrorIs(t, err, types.ErrUnauthorizedSender)
241241
}
242242

@@ -246,7 +246,7 @@ func TestBackend_CheckAccessList_AllowsConfiguredSender(t *testing.T) {
246246
mock.SetLatestTimestamp(200)
247247
backend.senderPolicy = mustParseTestSenderPolicy(t, testAllowedSender.Hex())
248248

249-
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), testAllowedSender)
249+
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), &testAllowedSender)
250250
require.NoError(t, err)
251251
}
252252

@@ -256,6 +256,7 @@ func TestBackend_CheckAccessList_AllowsWildcardSenderPolicy(t *testing.T) {
256256
mock.SetLatestTimestamp(200)
257257
backend.senderPolicy = mustParseTestSenderPolicy(t, "*")
258258

259-
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), common.HexToAddress("0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
259+
anySender := common.HexToAddress("0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
260+
err := backend.CheckAccessList(context.Background(), nil, types.LocalUnsafe, makeExecDescriptor(testChainA, 150, 0), &anySender)
260261
require.NoError(t, err)
261262
}

op-interop-filter/filter/frontend.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ type QueryFrontend struct {
1818

1919
// CheckAccessList validates interop executing messages
2020
func (f *QueryFrontend) CheckAccessList(ctx context.Context, inboxEntries []common.Hash,
21-
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender common.Address) error {
21+
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender *common.Address) error {
2222

2323
err := f.backend.CheckAccessList(ctx, inboxEntries, minSafety, executingDescriptor, sender)
2424
if err != nil {

op-service/apis/supervisor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ type SupervisorAdminAPI interface {
2626

2727
type SupervisorQueryAPI interface {
2828
CheckAccessList(ctx context.Context, inboxEntries []common.Hash,
29-
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender common.Address) error
29+
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender *common.Address) error
3030
CrossDerivedToSource(ctx context.Context, chainID eth.ChainID, derived eth.BlockID) (derivedFrom eth.BlockRef, err error)
3131
LocalUnsafe(ctx context.Context, chainID eth.ChainID) (eth.BlockID, error)
3232
LocalSafe(ctx context.Context, chainID eth.ChainID) (result types.DerivedIDPair, err error)

op-service/sources/supervisor_client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (cl *SupervisorClient) GetFailsafeEnabled(ctx context.Context) (bool, error
7474
}
7575

7676
func (cl *SupervisorClient) CheckAccessList(ctx context.Context, inboxEntries []common.Hash,
77-
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender common.Address) error {
77+
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender *common.Address) error {
7878
return cl.client.CallContext(ctx, nil, "supervisor_checkAccessList", inboxEntries, minSafety, executingDescriptor, sender)
7979
}
8080

op-supervisor/supervisor/backend/backend.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ func (su *SupervisorBackend) checkSafety(chainID eth.ChainID, blockID eth.BlockI
567567
}
568568

569569
func (su *SupervisorBackend) CheckAccessList(ctx context.Context, inboxEntries []common.Hash,
570-
minSafety types.SafetyLevel, execDescr types.ExecutingDescriptor, sender common.Address) error {
570+
minSafety types.SafetyLevel, execDescr types.ExecutingDescriptor, sender *common.Address) error {
571571
_ = sender
572572
// Check if failsafe is enabled
573573
if su.isFailsafeEnabled() {

op-supervisor/supervisor/backend/backend_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ func TestFailsafeEnabled(t *testing.T) {
631631
require.False(t, enabled, "failsafe should be disabled by default")
632632

633633
// Test that CheckAccessList works normally in initial state
634-
err = b.CheckAccessList(context.Background(), []common.Hash{}, types.LocalUnsafe, types.ExecutingDescriptor{}, common.Address{})
634+
err = b.CheckAccessList(context.Background(), []common.Hash{}, types.LocalUnsafe, types.ExecutingDescriptor{}, nil)
635635
require.NoError(t, err, "CheckAccessList should work normally when failsafe is disabled")
636636

637637
// Test setting failsafe to true
@@ -642,7 +642,7 @@ func TestFailsafeEnabled(t *testing.T) {
642642
require.True(t, enabled, "failsafe should be enabled after setting to true")
643643

644644
// Test that CheckAccessList returns ErrFailsafeEnabled when failsafe is enabled
645-
err = b.CheckAccessList(context.Background(), []common.Hash{}, types.LocalUnsafe, types.ExecutingDescriptor{}, common.Address{})
645+
err = b.CheckAccessList(context.Background(), []common.Hash{}, types.LocalUnsafe, types.ExecutingDescriptor{}, nil)
646646
require.ErrorIs(t, err, types.ErrFailsafeEnabled, "CheckAccessList should return ErrFailsafeEnabled when failsafe is enabled")
647647

648648
// Test setting failsafe to false
@@ -653,7 +653,7 @@ func TestFailsafeEnabled(t *testing.T) {
653653
require.False(t, enabled, "failsafe should be disabled after setting to false")
654654

655655
// Test that CheckAccessList works normally when failsafe is disabled
656-
err = b.CheckAccessList(context.Background(), []common.Hash{}, types.LocalUnsafe, types.ExecutingDescriptor{}, common.Address{})
656+
err = b.CheckAccessList(context.Background(), []common.Hash{}, types.LocalUnsafe, types.ExecutingDescriptor{}, nil)
657657
require.NoError(t, err, "CheckAccessList should work normally when failsafe is disabled")
658658
}
659659

op-supervisor/supervisor/backend/mock.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (m *MockBackend) AddL2RPC(ctx context.Context, rpc string, jwtSecret eth.By
4747
}
4848

4949
func (m *MockBackend) CheckAccessList(ctx context.Context, inboxEntries []common.Hash,
50-
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender common.Address) error {
50+
minSafety types.SafetyLevel, executingDescriptor types.ExecutingDescriptor, sender *common.Address) error {
5151
_ = sender
5252
return nil
5353
}

0 commit comments

Comments
 (0)