From 13f74078fa3e278ec5d58676fdff74e04047a65c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 13:56:34 +0200 Subject: [PATCH 1/8] Add a new RPCError struct, together with codes and interface implementation to the execution package --- execution/rpcerror.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 execution/rpcerror.go diff --git a/execution/rpcerror.go b/execution/rpcerror.go new file mode 100644 index 00000000000..a74d7f5e288 --- /dev/null +++ b/execution/rpcerror.go @@ -0,0 +1,43 @@ +// Copyright 2026, Offchain Labs, Inc. +// For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md + +package execution + +import "errors" + +// Application-level JSON-RPC error codes for Consensus/Execution communication. +// These are below -32768, outside the JSON-RPC 2.0 spec reserved range +// (-32768 to -32000), so they cannot collide with standard or go-ethereum codes. +const ( + ErrCodeRetrySequencer = -33000 + ErrCodeInsertLockTaken = -33001 + ErrCodeResultNotFound = -33002 +) + +// RPCError is an error that carries a JSON-RPC error code. +// +// On the server side it implements the rpc.Error interface, which causes +// go-ethereum's framework to include the code in the JSON-RPC error response +// instead of using the default -32000. +// +// On the client side the Is method enables code-based comparison via +// errors.Is, so callers do not need to inspect error message strings. +type RPCError struct { + code int + msg string +} + +func (e *RPCError) Error() string { return e.msg } +func (e *RPCError) ErrorCode() int { return e.code } + +// Is reports whether target is an RPCError with the same code. +// This makes errors.Is(receivedErr, sentinel) return true whenever the +// received error carries the same code as the sentinel, regardless of whether +// it is the exact same pointer or a jsonError reconstructed from the wire. +func (e *RPCError) Is(target error) bool { + var t *RPCError + if errors.As(target, &t) { + return t.code == e.code + } + return false +} From c9e97d6492039b3027a8e0b3b71fa14c8c4bc446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 14:02:59 +0200 Subject: [PATCH 2/8] Use errors.As instead of direct cast in geth errorMessage --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index bf231100a4e..4f62d14421d 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit bf231100a4e59852dac45a39da22f26eea975069 +Subproject commit 4f62d14421d1574a97d3ef08104ab855e49c2ad5 From 0fe43f6d0fa6f745f4a0fbfb3e032d141739bee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 14:15:32 +0200 Subject: [PATCH 3/8] Bring ResultNotFound to execution package --- arbnode/consensus_execution_syncer.go | 3 +-- execution/gethexec/executionengine.go | 3 +-- execution/interface.go | 6 +++--- execution/rpcclient/client.go | 5 ++--- execution/rpcclient/client_test.go | 11 +++++------ 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/arbnode/consensus_execution_syncer.go b/arbnode/consensus_execution_syncer.go index dee4b7569a5..c53c656124d 100644 --- a/arbnode/consensus_execution_syncer.go +++ b/arbnode/consensus_execution_syncer.go @@ -14,7 +14,6 @@ import ( "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/execution" - "github.com/offchainlabs/nitro/execution/gethexec" "github.com/offchainlabs/nitro/staker" "github.com/offchainlabs/nitro/util" "github.com/offchainlabs/nitro/util/headerreader" @@ -108,7 +107,7 @@ func (c *ConsensusExecutionSyncer) getFinalityData( } msgIdx := msgCount - 1 msgResult, err := c.txStreamer.ResultAtMessageIndex(msgIdx) - if errors.Is(err, gethexec.ResultNotFound) { + if errors.Is(err, execution.ResultNotFound) { log.Debug("Message result not found, node out of sync", "msgIdx", msgIdx, "err", err) return nil, nil } else if err != nil { diff --git a/execution/gethexec/executionengine.go b/execution/gethexec/executionengine.go index e7e31bee2cb..09fe85d19e7 100644 --- a/execution/gethexec/executionengine.go +++ b/execution/gethexec/executionengine.go @@ -71,7 +71,6 @@ var ( var ( ExecutionEngineBlockCreationStopped = errors.New("block creation stopped in execution engine") - ResultNotFound = errors.New("result not found") BlockNumBeforeGenesis = errors.New("block number is before genesis") ) @@ -1016,7 +1015,7 @@ func (s *ExecutionEngine) appendBlock(block *types.Block, statedb *state.StateDB func (s *ExecutionEngine) resultFromHeader(header *types.Header) (*execution.MessageResult, error) { if header == nil { - return nil, ResultNotFound + return nil, execution.ResultNotFound } info := types.DeserializeHeaderExtraInformation(header) return &execution.MessageResult{ diff --git a/execution/interface.go b/execution/interface.go index c084875f4da..cca95dafad5 100644 --- a/execution/interface.go +++ b/execution/interface.go @@ -4,7 +4,6 @@ package execution import ( "context" - "errors" "time" "github.com/ethereum/go-ethereum/common" @@ -42,8 +41,9 @@ type ConsensusSyncData struct { UpdatedAt time.Time } -var ErrRetrySequencer = errors.New("please retry transaction") -var ErrSequencerInsertLockTaken = errors.New("insert lock taken") +var ErrRetrySequencer = &RPCError{code: ErrCodeRetrySequencer, msg: "please retry transaction"} +var ErrSequencerInsertLockTaken = &RPCError{code: ErrCodeInsertLockTaken, msg: "insert lock taken"} +var ResultNotFound = &RPCError{code: ErrCodeResultNotFound, msg: "result not found"} // always needed type ExecutionClient interface { diff --git a/execution/rpcclient/client.go b/execution/rpcclient/client.go index 2692d5d9968..c8b5fc53ca4 100644 --- a/execution/rpcclient/client.go +++ b/execution/rpcclient/client.go @@ -13,7 +13,6 @@ import ( "github.com/offchainlabs/nitro/arbos/arbostypes" "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/execution" - "github.com/offchainlabs/nitro/execution/gethexec" "github.com/offchainlabs/nitro/util/containers" "github.com/offchainlabs/nitro/util/rpcclient" "github.com/offchainlabs/nitro/util/stopwaiter" @@ -48,8 +47,8 @@ func convertError(err error) error { errStr := err.Error() if strings.Contains(errStr, execution.ErrRetrySequencer.Error()) { return execution.ErrRetrySequencer - } else if strings.Contains(errStr, gethexec.ResultNotFound.Error()) { - return gethexec.ResultNotFound + } else if strings.Contains(errStr, execution.ResultNotFound.Error()) { + return execution.ResultNotFound } return err } diff --git a/execution/rpcclient/client_test.go b/execution/rpcclient/client_test.go index b4a4e1582dd..f1c3e2f8e6b 100644 --- a/execution/rpcclient/client_test.go +++ b/execution/rpcclient/client_test.go @@ -15,7 +15,6 @@ import ( "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/execution" - "github.com/offchainlabs/nitro/execution/gethexec" utilrpc "github.com/offchainlabs/nitro/util/rpcclient" "github.com/offchainlabs/nitro/util/testhelpers" ) @@ -66,13 +65,13 @@ func TestClientErrorHandling(t *testing.T) { }{ { name: "ResultNotFound mapped to sentinel", - serverErr: gethexec.ResultNotFound, - expectedErr: gethexec.ResultNotFound, + serverErr: execution.ResultNotFound, + expectedErr: execution.ResultNotFound, }, { name: "ResultNotFound wrapped in longer message mapped to sentinel", - serverErr: fmt.Errorf("execution context: %w", gethexec.ResultNotFound), - expectedErr: gethexec.ResultNotFound, + serverErr: fmt.Errorf("execution context: %w", execution.ResultNotFound), + expectedErr: execution.ResultNotFound, }, { name: "ErrRetrySequencer mapped to sentinel", @@ -111,7 +110,7 @@ func TestClientErrorHandling(t *testing.T) { t.Fatal("expected an error from server, got nil") } switch { - case errors.Is(tc.expectedErr, gethexec.ResultNotFound), errors.Is(tc.expectedErr, execution.ErrRetrySequencer): + case errors.Is(tc.expectedErr, execution.ResultNotFound), errors.Is(tc.expectedErr, execution.ErrRetrySequencer): if !errors.Is(err, tc.expectedErr) { t.Errorf("expected sentinel error %v, got %v", tc.expectedErr, err) } From e583faf180bf0050603c3459a60dd5a3abad6619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 14:29:46 +0200 Subject: [PATCH 4/8] Move errors to the rpcerror file --- execution/interface.go | 4 ---- execution/rpcerror.go | 7 +++++++ execution_consensus/init.go | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/execution/interface.go b/execution/interface.go index cca95dafad5..fc75dd183dc 100644 --- a/execution/interface.go +++ b/execution/interface.go @@ -41,10 +41,6 @@ type ConsensusSyncData struct { UpdatedAt time.Time } -var ErrRetrySequencer = &RPCError{code: ErrCodeRetrySequencer, msg: "please retry transaction"} -var ErrSequencerInsertLockTaken = &RPCError{code: ErrCodeInsertLockTaken, msg: "insert lock taken"} -var ResultNotFound = &RPCError{code: ErrCodeResultNotFound, msg: "result not found"} - // always needed type ExecutionClient interface { ArbOSVersionGetter diff --git a/execution/rpcerror.go b/execution/rpcerror.go index a74d7f5e288..f4aeb3837ca 100644 --- a/execution/rpcerror.go +++ b/execution/rpcerror.go @@ -30,6 +30,13 @@ type RPCError struct { func (e *RPCError) Error() string { return e.msg } func (e *RPCError) ErrorCode() int { return e.code } +// Sentinel errors returned across the Consensus/Execution RPC boundary. +var ( + ErrRetrySequencer = &RPCError{code: ErrCodeRetrySequencer, msg: "please retry transaction"} + ErrSequencerInsertLockTaken = &RPCError{code: ErrCodeInsertLockTaken, msg: "insert lock taken"} + ResultNotFound = &RPCError{code: ErrCodeResultNotFound, msg: "result not found"} +) + // Is reports whether target is an RPCError with the same code. // This makes errors.Is(receivedErr, sentinel) return true whenever the // received error carries the same code as the sentinel, regardless of whether diff --git a/execution_consensus/init.go b/execution_consensus/init.go index 08f8170aa1e..03db931a194 100644 --- a/execution_consensus/init.go +++ b/execution_consensus/init.go @@ -1,5 +1,6 @@ // Copyright 2025-2026, Offchain Labs, Inc. // For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md + package execution_consensus import ( From be641f88ba33387113504996ee11431d023a5bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 14:40:49 +0200 Subject: [PATCH 5/8] Get rid of the convertError --- consensus/consensusrpcclient/client.go | 22 +++++----------------- execution/rpcclient/client.go | 16 +--------------- execution/rpcclient/client_test.go | 24 ++++++------------------ 3 files changed, 12 insertions(+), 50 deletions(-) diff --git a/consensus/consensusrpcclient/client.go b/consensus/consensusrpcclient/client.go index ec10f97c1da..ef03734a5c9 100644 --- a/consensus/consensusrpcclient/client.go +++ b/consensus/consensusrpcclient/client.go @@ -4,7 +4,6 @@ package consensusrpcclient import ( "context" - "strings" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/node" @@ -40,22 +39,11 @@ func (c *ConsensusRPCClient) StopAndWait() { c.StopWaiter.StopAndWait() } -func convertError(err error) error { - if err == nil { - return nil - } - errStr := err.Error() - if strings.Contains(errStr, execution.ErrSequencerInsertLockTaken.Error()) { - return execution.ErrSequencerInsertLockTaken - } - return err -} - func (c *ConsensusRPCClient) GetL1Confirmations(msgIdx arbutil.MessageIndex) containers.PromiseInterface[uint64] { return stopwaiter.LaunchPromiseThread(c, func(ctx context.Context) (uint64, error) { var res uint64 err := c.client.CallContext(ctx, &res, consensus.RPCNamespace+"_getL1Confirmations", msgIdx) - return res, convertError(err) + return res, err }) } @@ -63,7 +51,7 @@ func (c *ConsensusRPCClient) FindBatchContainingMessage(msgIdx arbutil.MessageIn return stopwaiter.LaunchPromiseThread(c, func(ctx context.Context) (uint64, error) { var res uint64 err := c.client.CallContext(ctx, &res, consensus.RPCNamespace+"_findBatchContainingMessage", msgIdx) - return res, convertError(err) + return res, err }) } @@ -71,7 +59,7 @@ func (c *ConsensusRPCClient) BlockMetadataAtMessageIndex(msgIdx arbutil.MessageI return stopwaiter.LaunchPromiseThread(c, func(ctx context.Context) (common.BlockMetadata, error) { var res common.BlockMetadata err := c.client.CallContext(ctx, &res, consensus.RPCNamespace+"_blockMetadataAtMessageIndex", msgIdx) - return res, convertError(err) + return res, err }) } @@ -79,7 +67,7 @@ func (c *ConsensusRPCClient) WriteMessageFromSequencer(msgIdx arbutil.MessageInd return stopwaiter.LaunchPromiseThread(c, func(ctx context.Context) (struct{}, error) { var res struct{} err := c.client.CallContext(ctx, &res, consensus.RPCNamespace+"_writeMessageFromSequencer", msgIdx, msgWithMeta, msgResult, blockMetadata) - return res, convertError(err) + return res, err }) } @@ -87,6 +75,6 @@ func (c *ConsensusRPCClient) ExpectChosenSequencer() containers.PromiseInterface return stopwaiter.LaunchPromiseThread(c, func(ctx context.Context) (struct{}, error) { var res struct{} err := c.client.CallContext(ctx, &res, consensus.RPCNamespace+"_expectChosenSequencer") - return res, convertError(err) + return res, err }) } diff --git a/execution/rpcclient/client.go b/execution/rpcclient/client.go index c8b5fc53ca4..de27f56970e 100644 --- a/execution/rpcclient/client.go +++ b/execution/rpcclient/client.go @@ -5,7 +5,6 @@ package rpcclient import ( "context" - "strings" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/node" @@ -40,24 +39,11 @@ func (c *Client) StopAndWait() { c.StopWaiter.StopAndWait() } -func convertError(err error) error { - if err == nil { - return nil - } - errStr := err.Error() - if strings.Contains(errStr, execution.ErrRetrySequencer.Error()) { - return execution.ErrRetrySequencer - } else if strings.Contains(errStr, execution.ResultNotFound.Error()) { - return execution.ResultNotFound - } - return err -} - func sendRequest[T any](c *Client, method string, args ...any) containers.PromiseInterface[T] { return stopwaiter.LaunchPromiseThread(c, func(ctx context.Context) (T, error) { var res T err := c.client.CallContext(ctx, &res, execution.RPCNamespace+method, args...) - return res, convertError(err) + return res, err }) } diff --git a/execution/rpcclient/client_test.go b/execution/rpcclient/client_test.go index f1c3e2f8e6b..08939488a75 100644 --- a/execution/rpcclient/client_test.go +++ b/execution/rpcclient/client_test.go @@ -64,30 +64,25 @@ func TestClientErrorHandling(t *testing.T) { expectedErr error }{ { - name: "ResultNotFound mapped to sentinel", + name: "ResultNotFound", serverErr: execution.ResultNotFound, expectedErr: execution.ResultNotFound, }, { - name: "ResultNotFound wrapped in longer message mapped to sentinel", + name: "ResultNotFound wrapped", serverErr: fmt.Errorf("execution context: %w", execution.ResultNotFound), expectedErr: execution.ResultNotFound, }, { - name: "ErrRetrySequencer mapped to sentinel", + name: "ErrRetrySequencer", serverErr: execution.ErrRetrySequencer, expectedErr: execution.ErrRetrySequencer, }, { - name: "ErrRetrySequencer wrapped in longer message mapped to sentinel", + name: "ErrRetrySequencer wrapped", serverErr: fmt.Errorf("rpc context: %w", execution.ErrRetrySequencer), expectedErr: execution.ErrRetrySequencer, }, - { - name: "generic error message is preserved", - serverErr: errors.New("unexpected failure"), - expectedErr: errors.New("unexpected failure"), - }, } for _, tc := range tests { @@ -109,15 +104,8 @@ func TestClientErrorHandling(t *testing.T) { if err == nil { t.Fatal("expected an error from server, got nil") } - switch { - case errors.Is(tc.expectedErr, execution.ResultNotFound), errors.Is(tc.expectedErr, execution.ErrRetrySequencer): - if !errors.Is(err, tc.expectedErr) { - t.Errorf("expected sentinel error %v, got %v", tc.expectedErr, err) - } - default: - if err.Error() != tc.expectedErr.Error() { - t.Errorf("expected error message %q, got %q", tc.expectedErr.Error(), err.Error()) - } + if !errors.Is(err, tc.expectedErr) { + t.Errorf("expected %v, got %v", tc.expectedErr, err) } }) } From 25767261803c8194ab096539efc374dd1adabeb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 14:52:16 +0200 Subject: [PATCH 6/8] Negative test --- execution/rpcclient/client_test.go | 46 ++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/execution/rpcclient/client_test.go b/execution/rpcclient/client_test.go index 08939488a75..3ab544c2666 100644 --- a/execution/rpcclient/client_test.go +++ b/execution/rpcclient/client_test.go @@ -53,6 +53,12 @@ func createMockExecutionNode(t *testing.T, errToReturn error) *node.Node { return stack } +var allSentinels = []error{ + execution.ResultNotFound, + execution.ErrRetrySequencer, + execution.ErrSequencerInsertLockTaken, +} + func TestClientErrorHandling(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), time.Minute) @@ -83,6 +89,16 @@ func TestClientErrorHandling(t *testing.T) { serverErr: fmt.Errorf("rpc context: %w", execution.ErrRetrySequencer), expectedErr: execution.ErrRetrySequencer, }, + { + name: "ErrSequencerInsertLockTaken", + serverErr: execution.ErrSequencerInsertLockTaken, + expectedErr: execution.ErrSequencerInsertLockTaken, + }, + { + name: "ErrSequencerInsertLockTaken wrapped", + serverErr: fmt.Errorf("consensus context: %w", execution.ErrSequencerInsertLockTaken), + expectedErr: execution.ErrSequencerInsertLockTaken, + }, } for _, tc := range tests { @@ -110,3 +126,33 @@ func TestClientErrorHandling(t *testing.T) { }) } } + +// TestClientErrorNoFalsePositives verifies that a plain server error (which +// arrives with the default JSON-RPC code -32000) does not match any sentinel. +func TestClientErrorNoFalsePositives(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + stack := createMockExecutionNode(t, errors.New("some unrelated failure")) + + config := &utilrpc.ClientConfig{ + URL: "self", + Timeout: 5 * time.Second, + } + testhelpers.RequireImpl(t, config.Validate()) + + client := NewClient(func() *utilrpc.ClientConfig { return config }, stack) + testhelpers.RequireImpl(t, client.Start(ctx)) + defer client.StopAndWait() + + _, err := client.HeadMessageIndex().Await(ctx) + if err == nil { + t.Fatal("expected an error from server, got nil") + } + for _, sentinel := range allSentinels { + if errors.Is(err, sentinel) { + t.Errorf("plain error should not match sentinel %v", sentinel) + } + } +} From 2e3ab53d40d22982cd320dfebe8d81bb4f2d388a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 15:18:03 +0200 Subject: [PATCH 7/8] changelog --- changelog/pmikolajczyk-nit-4738.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog/pmikolajczyk-nit-4738.md diff --git a/changelog/pmikolajczyk-nit-4738.md b/changelog/pmikolajczyk-nit-4738.md new file mode 100644 index 00000000000..e6c56fe1f7e --- /dev/null +++ b/changelog/pmikolajczyk-nit-4738.md @@ -0,0 +1,2 @@ +### Internal +- Replace string matching with JSON-RPC error codes in Consensus/Execution RPC clients From 3ed26470915aede98bcf5a2a5acd75588a8bf6d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 10 Apr 2026 16:00:39 +0200 Subject: [PATCH 8/8] minor updates, test the other client --- arbnode/consensus_execution_syncer.go | 2 +- consensus/consensusrpcclient/client_test.go | 169 ++++++++++++++++++++ execution/gethexec/executionengine.go | 2 +- execution/rpcclient/client_test.go | 10 +- execution/rpcerror.go | 2 +- 5 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 consensus/consensusrpcclient/client_test.go diff --git a/arbnode/consensus_execution_syncer.go b/arbnode/consensus_execution_syncer.go index c53c656124d..b899aea8ef0 100644 --- a/arbnode/consensus_execution_syncer.go +++ b/arbnode/consensus_execution_syncer.go @@ -107,7 +107,7 @@ func (c *ConsensusExecutionSyncer) getFinalityData( } msgIdx := msgCount - 1 msgResult, err := c.txStreamer.ResultAtMessageIndex(msgIdx) - if errors.Is(err, execution.ResultNotFound) { + if errors.Is(err, execution.ErrResultNotFound) { log.Debug("Message result not found, node out of sync", "msgIdx", msgIdx, "err", err) return nil, nil } else if err != nil { diff --git a/consensus/consensusrpcclient/client_test.go b/consensus/consensusrpcclient/client_test.go new file mode 100644 index 00000000000..ccda11a3c53 --- /dev/null +++ b/consensus/consensusrpcclient/client_test.go @@ -0,0 +1,169 @@ +// Copyright 2026, Offchain Labs, Inc. +// For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md + +package consensusrpcclient + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/node" + "github.com/ethereum/go-ethereum/rpc" + + "github.com/offchainlabs/nitro/arbos/arbostypes" + "github.com/offchainlabs/nitro/arbutil" + "github.com/offchainlabs/nitro/consensus" + "github.com/offchainlabs/nitro/consensus/consensusrpcserver" + "github.com/offchainlabs/nitro/execution" + "github.com/offchainlabs/nitro/util/containers" + utilrpc "github.com/offchainlabs/nitro/util/rpcclient" + "github.com/offchainlabs/nitro/util/testhelpers" +) + +// mockConsensusService implements consensus.FullConsensusClient for testing. +type mockConsensusService struct { + err error +} + +func (m *mockConsensusService) GetL1Confirmations(_ arbutil.MessageIndex) containers.PromiseInterface[uint64] { + return containers.NewReadyPromise[uint64](0, m.err) +} + +func (m *mockConsensusService) FindBatchContainingMessage(_ arbutil.MessageIndex) containers.PromiseInterface[uint64] { + return containers.NewReadyPromise[uint64](0, m.err) +} + +func (m *mockConsensusService) BlockMetadataAtMessageIndex(_ arbutil.MessageIndex) containers.PromiseInterface[common.BlockMetadata] { + return containers.NewReadyPromise[common.BlockMetadata](nil, m.err) +} + +func (m *mockConsensusService) WriteMessageFromSequencer(_ arbutil.MessageIndex, _ arbostypes.MessageWithMetadata, _ execution.MessageResult, _ common.BlockMetadata) containers.PromiseInterface[struct{}] { + return containers.NewReadyPromise[struct{}](struct{}{}, m.err) +} + +func (m *mockConsensusService) ExpectChosenSequencer() containers.PromiseInterface[struct{}] { + return containers.NewReadyPromise[struct{}](struct{}{}, m.err) +} + +func createMockConsensusNode(t *testing.T, errToReturn error) *node.Node { + t.Helper() + stackConf := node.DefaultConfig + stackConf.HTTPPort = 0 + stackConf.DataDir = "" + stackConf.WSHost = "127.0.0.1" + stackConf.WSPort = 0 + stackConf.WSModules = []string{consensus.RPCNamespace} + stackConf.P2P.NoDiscovery = true + stackConf.P2P.ListenAddr = "" + + stack, err := node.New(&stackConf) + testhelpers.RequireImpl(t, err) + + stack.RegisterAPIs([]rpc.API{{ + Namespace: consensus.RPCNamespace, + Service: consensusrpcserver.NewConsensusRPCServer(&mockConsensusService{err: errToReturn}), + Public: true, + }}) + + testhelpers.RequireImpl(t, stack.Start()) + t.Cleanup(func() { _ = stack.Close() }) + return stack +} + +func TestConsensusClientErrorHandling(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + tests := []struct { + name string + serverErr error + expectedErr error + }{ + { + name: "ErrSequencerInsertLockTaken", + serverErr: execution.ErrSequencerInsertLockTaken, + expectedErr: execution.ErrSequencerInsertLockTaken, + }, + { + name: "ErrSequencerInsertLockTaken wrapped", + serverErr: fmt.Errorf("consensus context: %w", execution.ErrSequencerInsertLockTaken), + expectedErr: execution.ErrSequencerInsertLockTaken, + }, + { + name: "ErrRetrySequencer", + serverErr: execution.ErrRetrySequencer, + expectedErr: execution.ErrRetrySequencer, + }, + { + name: "ErrRetrySequencer wrapped", + serverErr: fmt.Errorf("consensus context: %w", execution.ErrRetrySequencer), + expectedErr: execution.ErrRetrySequencer, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stack := createMockConsensusNode(t, tc.serverErr) + + config := &utilrpc.ClientConfig{ + URL: "self", + Timeout: 5 * time.Second, + } + testhelpers.RequireImpl(t, config.Validate()) + + client := NewConsensusRPCClient(func() *utilrpc.ClientConfig { return config }, stack) + testhelpers.RequireImpl(t, client.Start(ctx)) + defer client.StopAndWait() + + _, err := client.ExpectChosenSequencer().Await(ctx) + + if err == nil { + t.Fatal("expected an error from server, got nil") + } + if !errors.Is(err, tc.expectedErr) { + t.Errorf("expected %v, got %v", tc.expectedErr, err) + } + }) + } +} + +// TestConsensusClientErrorNoFalsePositives verifies that a plain server error +// (arriving with the default JSON-RPC code -32000) does not match any sentinel. +func TestConsensusClientErrorNoFalsePositives(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + stack := createMockConsensusNode(t, errors.New("some unrelated failure")) + + config := &utilrpc.ClientConfig{ + URL: "self", + Timeout: 5 * time.Second, + } + testhelpers.RequireImpl(t, config.Validate()) + + client := NewConsensusRPCClient(func() *utilrpc.ClientConfig { return config }, stack) + testhelpers.RequireImpl(t, client.Start(ctx)) + defer client.StopAndWait() + + _, err := client.ExpectChosenSequencer().Await(ctx) + if err == nil { + t.Fatal("expected an error from server, got nil") + } + + allSentinels := []error{ + execution.ErrResultNotFound, + execution.ErrRetrySequencer, + execution.ErrSequencerInsertLockTaken, + } + for _, sentinel := range allSentinels { + if errors.Is(err, sentinel) { + t.Errorf("plain error should not match sentinel %v", sentinel) + } + } +} diff --git a/execution/gethexec/executionengine.go b/execution/gethexec/executionengine.go index 09fe85d19e7..e442d068f7e 100644 --- a/execution/gethexec/executionengine.go +++ b/execution/gethexec/executionengine.go @@ -1015,7 +1015,7 @@ func (s *ExecutionEngine) appendBlock(block *types.Block, statedb *state.StateDB func (s *ExecutionEngine) resultFromHeader(header *types.Header) (*execution.MessageResult, error) { if header == nil { - return nil, execution.ResultNotFound + return nil, execution.ErrResultNotFound } info := types.DeserializeHeaderExtraInformation(header) return &execution.MessageResult{ diff --git a/execution/rpcclient/client_test.go b/execution/rpcclient/client_test.go index 3ab544c2666..4a0ed2ef9bf 100644 --- a/execution/rpcclient/client_test.go +++ b/execution/rpcclient/client_test.go @@ -54,7 +54,7 @@ func createMockExecutionNode(t *testing.T, errToReturn error) *node.Node { } var allSentinels = []error{ - execution.ResultNotFound, + execution.ErrResultNotFound, execution.ErrRetrySequencer, execution.ErrSequencerInsertLockTaken, } @@ -71,13 +71,13 @@ func TestClientErrorHandling(t *testing.T) { }{ { name: "ResultNotFound", - serverErr: execution.ResultNotFound, - expectedErr: execution.ResultNotFound, + serverErr: execution.ErrResultNotFound, + expectedErr: execution.ErrResultNotFound, }, { name: "ResultNotFound wrapped", - serverErr: fmt.Errorf("execution context: %w", execution.ResultNotFound), - expectedErr: execution.ResultNotFound, + serverErr: fmt.Errorf("execution context: %w", execution.ErrResultNotFound), + expectedErr: execution.ErrResultNotFound, }, { name: "ErrRetrySequencer", diff --git a/execution/rpcerror.go b/execution/rpcerror.go index f4aeb3837ca..6e7618540d4 100644 --- a/execution/rpcerror.go +++ b/execution/rpcerror.go @@ -34,7 +34,7 @@ func (e *RPCError) ErrorCode() int { return e.code } var ( ErrRetrySequencer = &RPCError{code: ErrCodeRetrySequencer, msg: "please retry transaction"} ErrSequencerInsertLockTaken = &RPCError{code: ErrCodeInsertLockTaken, msg: "insert lock taken"} - ResultNotFound = &RPCError{code: ErrCodeResultNotFound, msg: "result not found"} + ErrResultNotFound = &RPCError{code: ErrCodeResultNotFound, msg: "result not found"} ) // Is reports whether target is an RPCError with the same code.