From b20c71a9fefd38ac66ba5c07cb03304064707772 Mon Sep 17 00:00:00 2001 From: Pablo Date: Thu, 4 Jun 2026 12:04:09 -0600 Subject: [PATCH 1/6] fix: replace sender to timelock address during error decoding Signed-off-by: Pablo --- sdk/evm/execution_error.go | 52 ++++++- sdk/evm/execution_error_test.go | 239 ++++++++++++++++++++++++++++++ sdk/evm/timelock_executor_test.go | 40 +++++ 3 files changed, 328 insertions(+), 3 deletions(-) diff --git a/sdk/evm/execution_error.go b/sdk/evm/execution_error.go index b7c15891b..5540e26ef 100644 --- a/sdk/evm/execution_error.go +++ b/sdk/evm/execution_error.go @@ -1,6 +1,7 @@ package evm import ( + "bytes" "context" "encoding/json" "errors" @@ -31,6 +32,12 @@ var ( // customErrorPattern matches "custom error 0x<8-hex-chars>: " // Captures: group 1 = selector (8 hex chars), group 2 = data (hex chars with optional spaces) customErrorPattern = regexp.MustCompile(`custom error 0x([0-9a-fA-F]{8}):\s*([0-9a-fA-F\s]+)`) + + // callProxyRuntimePrefix and callProxyRuntimeSuffix fingerprint the deployed + // runtime of bindings.CallProxy (immutable target embedded between them). + // If CallProxy is recompiled, update these and TestCallProxyBytecodeFingerprintMatchesBinding. + callProxyRuntimePrefix = common.FromHex("0x60806040527f") + callProxyRuntimeSuffix = common.FromHex("0x366000803760008036600034855af13d6000803e80156043573d6000f35b503d6000fd") ) const ( @@ -295,7 +302,8 @@ func BuildExecutionError( if strings.Contains(errStr, "RBACTimelock: underlying transaction reverted") || strings.Contains(execErr.RevertReasonDecoded, "RBACTimelock: underlying transaction reverted") || (isCallReverted && timelockAddr != (common.Address{}) && len(timelockCallData) > 0) { - rawUnderlyingReason, decodedUnderlyingReason := getUnderlyingRevertReason(ctx, timelockAddr, timelockCallData, opts, client) + underlyingCallSender := resolveUnderlyingCallSender(ctx, timelockAddr, client) + rawUnderlyingReason, decodedUnderlyingReason := getUnderlyingRevertReason(ctx, underlyingCallSender, timelockCallData, opts, client) execErr.UnderlyingReasonRaw = rawUnderlyingReason execErr.UnderlyingReasonDecoded = decodedUnderlyingReason } @@ -303,6 +311,44 @@ func BuildExecutionError( return execErr } +func resolveUnderlyingCallSender(ctx context.Context, executionAddr common.Address, client ContractDeployBackend) common.Address { + target, ok := callProxyTargetFromChain(ctx, executionAddr, client) + if ok { + return target + } + + return executionAddr +} + +func callProxyTargetFromChain(ctx context.Context, address common.Address, client ContractDeployBackend) (common.Address, bool) { + if address == (common.Address{}) || client == nil { + return common.Address{}, false + } + + code, err := client.CodeAt(ctx, address, nil) + if err != nil { + return common.Address{}, false + } + + return callProxyTargetFromRuntime(code) +} + +func callProxyTargetFromRuntime(code []byte) (common.Address, bool) { + targetOffset := len(callProxyRuntimePrefix) + suffixOffset := targetOffset + common.HashLength + if len(code) < suffixOffset+len(callProxyRuntimeSuffix) { + return common.Address{}, false + } + if !bytes.Equal(code[:targetOffset], callProxyRuntimePrefix) { + return common.Address{}, false + } + if !bytes.Equal(code[suffixOffset:suffixOffset+len(callProxyRuntimeSuffix)], callProxyRuntimeSuffix) { + return common.Address{}, false + } + + return common.BytesToAddress(code[targetOffset:suffixOffset]), true +} + // extractHexEncodedRevertData extracts hex-encoded revert data (0x...) from an error string. // Returns the extracted bytes if found, nil otherwise. func extractHexEncodedRevertData(errStr string) []byte { @@ -626,8 +672,8 @@ func getUnderlyingRevertReason( return "", "" } - // Simulate the underlying transaction using CallContract (best-effort) - // Use timelock address as From since that's what msg.sender will be when RBACTimelock executes + // Simulate the underlying transaction using CallContract (best-effort). + // timelockAddr is the underlying-call sender (RBACTimelock), including after CallProxy resolution. _, err := client.CallContract(ctx, ethereum.CallMsg{ From: timelockAddr, To: &underlyingCall.Target, diff --git a/sdk/evm/execution_error_test.go b/sdk/evm/execution_error_test.go index f1418a33c..65b9517b1 100644 --- a/sdk/evm/execution_error_test.go +++ b/sdk/evm/execution_error_test.go @@ -1,6 +1,7 @@ package evm import ( + "bytes" "context" "encoding/json" "errors" @@ -8,9 +9,11 @@ import ( "strings" "testing" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" + gethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1420,6 +1423,242 @@ func TestGetUnderlyingRevertReason(t *testing.T) { } } +const ( + onlyCallableByOwnerSelector = "0x2b5c74de" + realUnderlyingErrorSelector = "0x4b5786e7" +) + +func TestCallProxyBytecodeFingerprintMatchesBinding(t *testing.T) { + t.Parallel() + + runtime := callProxyRuntimeFromBinding(t) + require.GreaterOrEqual(t, len(runtime), len(callProxyRuntimePrefix)+len(callProxyRuntimeSuffix)) + assert.Equal(t, callProxyRuntimePrefix, runtime[:len(callProxyRuntimePrefix)]) + suffixStart := len(callProxyRuntimePrefix) + common.HashLength + assert.Equal(t, callProxyRuntimeSuffix, runtime[suffixStart:suffixStart+len(callProxyRuntimeSuffix)]) + + target := common.HexToAddress("0x1111111111111111111111111111111111111111") + patched := callProxyRuntime(target) + got, ok := callProxyTargetFromRuntime(patched) + require.True(t, ok) + assert.Equal(t, target, got) +} + +func TestCallProxyTargetFromRuntime(t *testing.T) { + t.Parallel() + + target := common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0") + + tests := []struct { + name string + code []byte + wantAddr common.Address + wantOK bool + }{ + { + name: "valid call proxy runtime", + code: callProxyRuntime(target), + wantAddr: target, + wantOK: true, + }, + { + name: "too short", + code: []byte{0x60}, + wantAddr: common.Address{}, + wantOK: false, + }, + { + name: "wrong prefix", + code: append([]byte{0xff}, callProxyRuntime(target)[1:]...), + wantAddr: common.Address{}, + wantOK: false, + }, + { + name: "wrong suffix", + code: append(callProxyRuntime(target)[:len(callProxyRuntime(target))-1], 0xff), + wantAddr: common.Address{}, + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, ok := callProxyTargetFromRuntime(tt.code) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.wantAddr, got) + }) + } +} + +func TestResolveUnderlyingCallSender(t *testing.T) { + t.Parallel() + + executionAddr := common.HexToAddress("0x0B665A6d3ec78BF99e17Ca81c071bb9Dfdf081Fc") + realTimelock := common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0") + + tests := []struct { + name string + executionAddr common.Address + setupMock func(*mocks.ContractDeployBackend) + wantSender common.Address + }{ + { + name: "call proxy resolves to embedded timelock", + executionAddr: executionAddr, + setupMock: func(m *mocks.ContractDeployBackend) { + m.On("CodeAt", mock.Anything, executionAddr, mock.Anything). + Return(callProxyRuntime(realTimelock), nil).Once() + }, + wantSender: realTimelock, + }, + { + name: "non-proxy bytecode falls back to execution address", + executionAddr: executionAddr, + setupMock: func(m *mocks.ContractDeployBackend) { + m.On("CodeAt", mock.Anything, executionAddr, mock.Anything). + Return([]byte("not a call proxy"), nil).Once() + }, + wantSender: executionAddr, + }, + { + name: "code lookup error falls back to execution address", + executionAddr: executionAddr, + setupMock: func(m *mocks.ContractDeployBackend) { + m.On("CodeAt", mock.Anything, executionAddr, mock.Anything). + Return(nil, errors.New("rpc error")).Once() + }, + wantSender: executionAddr, + }, + { + name: "nil client falls back to execution address", + executionAddr: executionAddr, + setupMock: nil, + wantSender: executionAddr, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var client ContractDeployBackend + if tt.setupMock != nil { + mockClient := mocks.NewContractDeployBackend(t) + tt.setupMock(mockClient) + client = mockClient + } + + got := resolveUnderlyingCallSender(context.Background(), tt.executionAddr, client) + assert.Equal(t, tt.wantSender, got) + }) + } +} + +func TestBuildExecutionError_callProxyUnderlyingReplay(t *testing.T) { + t.Parallel() + + timelockABI, err := bindings.RBACTimelockMetaData.GetAbi() + require.NoError(t, err, errMsgFailedTimelockABI) + + testCall := bindings.RBACTimelockCall{ + Target: common.HexToAddress("0x1234567890123456789012345678901234567890"), + Value: big.NewInt(0), + Data: []byte{1, 2, 3}, + } + callData := packBypasserExecuteBatch(t, timelockABI, []bindings.RBACTimelockCall{testCall}) + + callProxyAddr := common.HexToAddress("0x0B665A6d3ec78BF99e17Ca81c071bb9Dfdf081Fc") + realTimelock := common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0") + opts := &bind.TransactOpts{GasLimit: 1_000_000} + + mockClient := mocks.NewContractDeployBackend(t) + mockClient.On("CodeAt", mock.Anything, callProxyAddr, mock.Anything). + Return(callProxyRuntime(realTimelock), nil).Once() + mockClient.On("CallContract", mock.Anything, mock.MatchedBy(func(msg ethereum.CallMsg) bool { + return msg.From == realTimelock && + msg.To != nil && + *msg.To == testCall.Target + }), mock.Anything). + Return(nil, errors.New("execution reverted: "+realUnderlyingErrorSelector)).Once() + + execErr := BuildExecutionError( + context.Background(), + errors.New("execution reverted: CallReverted"), + gethtypes.NewTx(&gethtypes.LegacyTx{}), + opts, + callProxyAddr, + mockClient, + callProxyAddr, + callData, + ) + + require.NotNil(t, execErr) + assert.Equal(t, realUnderlyingErrorSelector, execErr.UnderlyingReasonRaw) + assert.NotEqual(t, onlyCallableByOwnerSelector, execErr.UnderlyingReasonRaw) +} + +func TestBuildExecutionError_callProxyPreservesRealOwnershipFailure(t *testing.T) { + t.Parallel() + + timelockABI, err := bindings.RBACTimelockMetaData.GetAbi() + require.NoError(t, err, errMsgFailedTimelockABI) + + testCall := bindings.RBACTimelockCall{ + Target: common.HexToAddress("0x1234567890123456789012345678901234567890"), + Value: big.NewInt(0), + Data: []byte{1, 2, 3}, + } + callData := packBypasserExecuteBatch(t, timelockABI, []bindings.RBACTimelockCall{testCall}) + + callProxyAddr := common.HexToAddress("0x0B665A6d3ec78BF99e17Ca81c071bb9Dfdf081Fc") + realTimelock := common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0") + opts := &bind.TransactOpts{GasLimit: 1_000_000} + + mockClient := mocks.NewContractDeployBackend(t) + mockClient.On("CodeAt", mock.Anything, callProxyAddr, mock.Anything). + Return(callProxyRuntime(realTimelock), nil).Once() + mockClient.On("CallContract", mock.Anything, mock.MatchedBy(func(msg ethereum.CallMsg) bool { + return msg.From == realTimelock + }), mock.Anything). + Return(nil, errors.New("execution reverted: "+onlyCallableByOwnerSelector)).Once() + + execErr := BuildExecutionError( + context.Background(), + errors.New("execution reverted: CallReverted"), + gethtypes.NewTx(&gethtypes.LegacyTx{}), + opts, + callProxyAddr, + mockClient, + callProxyAddr, + callData, + ) + + require.NotNil(t, execErr) + assert.Equal(t, onlyCallableByOwnerSelector, execErr.UnderlyingReasonRaw) +} + +func callProxyRuntime(target common.Address) []byte { + code := make([]byte, 0, len(callProxyRuntimePrefix)+common.HashLength+len(callProxyRuntimeSuffix)) + code = append(code, callProxyRuntimePrefix...) + code = append(code, common.LeftPadBytes(target.Bytes(), common.HashLength)...) + code = append(code, callProxyRuntimeSuffix...) + + return code +} + +func callProxyRuntimeFromBinding(t *testing.T) []byte { + t.Helper() + + bin := common.FromHex(bindings.CallProxyBin) + marker := []byte{0xf3, 0xfe} + idx := bytes.Index(bin, marker) + require.Greater(t, idx, 0, "call proxy init/runtime separator not found in CallProxyBin") + + return bin[idx+len(marker):] +} + // Helper functions for packing timelock method calls // encodeErrorString ABI-encodes a string for Error(string) format. diff --git a/sdk/evm/timelock_executor_test.go b/sdk/evm/timelock_executor_test.go index 9f16abe77..fde69e8d4 100644 --- a/sdk/evm/timelock_executor_test.go +++ b/sdk/evm/timelock_executor_test.go @@ -7,6 +7,7 @@ import ( "math/big" "testing" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" evmTypes "github.com/ethereum/go-ethereum/core/types" @@ -74,6 +75,7 @@ func TestTimelockExecutor_Execute(t *testing.T) { mockSetup func(m *evm_mocks.ContractDeployBackend) wantTxHash string wantErr error + wantErrContains []string }{ { name: "success", @@ -97,6 +99,40 @@ func TestTimelockExecutor_Execute(t *testing.T) { wantTxHash: "0xc381f411283719726be93f957b9e3ca7d8041725c22fefab8dcf132770adf7a9", wantErr: nil, }, + { + name: "failure through call proxy replays underlying call from timelock", + auth: mockAuth, + timelockAddress: "0x0B665A6d3ec78BF99e17Ca81c071bb9Dfdf081Fc", + bop: types.BatchOperation{ + ChainSelector: chaintest.Chain1Selector, + Transactions: []types.Transaction{ + { + To: "0x1234567890123456789012345678901234567890", + Data: []byte{1, 2, 3}, + AdditionalFields: json.RawMessage(`{"value": 0}`)}, + }, + }, + mockSetup: func(m *evm_mocks.ContractDeployBackend) { + m.EXPECT().SendTransaction(mock.Anything, mock.Anything). + Return(errors.New("execution reverted: CallReverted")) + m.EXPECT().CodeAt( + mock.Anything, + common.HexToAddress("0x0B665A6d3ec78BF99e17Ca81c071bb9Dfdf081Fc"), + mock.Anything, + ).Return(callProxyRuntime(common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0")), nil) + m.EXPECT().CallContract(mock.Anything, mock.MatchedBy(func(msg ethereum.CallMsg) bool { + return msg.From == common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0") && + msg.To != nil && + *msg.To == common.HexToAddress("0x1234567890123456789012345678901234567890") + }), mock.Anything).Return(nil, errors.New("execution reverted: 0x12345678")) + sharedMockSetup(m) + }, + wantTxHash: "", + wantErrContains: []string{ + "execution reverted: CallReverted", + "underlying reason: 0x12345678", + }, + }, { name: "failure in tx execution", auth: mockAuth, @@ -136,6 +172,10 @@ func TestTimelockExecutor_Execute(t *testing.T) { assert.Equal(t, test.wantTxHash, tx.Hash) if test.wantErr != nil { assert.EqualError(t, err, test.wantErr.Error()) + } else if len(test.wantErrContains) > 0 { + for _, want := range test.wantErrContains { + assert.ErrorContains(t, err, want) + } } else { assert.NoError(t, err) } From c0470b49b901f9eff44ffdb3191b91f51f837970 Mon Sep 17 00:00:00 2001 From: Pablo Date: Thu, 4 Jun 2026 13:21:19 -0600 Subject: [PATCH 2/6] fix: copilot comments Signed-off-by: Pablo --- sdk/evm/execution_error.go | 24 ++++++++---------------- sdk/evm/execution_error_test.go | 2 +- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/sdk/evm/execution_error.go b/sdk/evm/execution_error.go index 5540e26ef..1df155491 100644 --- a/sdk/evm/execution_error.go +++ b/sdk/evm/execution_error.go @@ -312,27 +312,19 @@ func BuildExecutionError( } func resolveUnderlyingCallSender(ctx context.Context, executionAddr common.Address, client ContractDeployBackend) common.Address { - target, ok := callProxyTargetFromChain(ctx, executionAddr, client) - if ok { - return target + if executionAddr != (common.Address{}) && client != nil { + if code, err := client.CodeAt(ctx, executionAddr, nil); err == nil { + if targetAddress, ok := callProxyTargetFromRuntime(code); ok { + return targetAddress + } + } } return executionAddr } -func callProxyTargetFromChain(ctx context.Context, address common.Address, client ContractDeployBackend) (common.Address, bool) { - if address == (common.Address{}) || client == nil { - return common.Address{}, false - } - - code, err := client.CodeAt(ctx, address, nil) - if err != nil { - return common.Address{}, false - } - - return callProxyTargetFromRuntime(code) -} - +// callProxyTargetFromRuntime checks whether code matches bindings.CallProxy runtime +// (fixed prefix + suffix with a 32-byte immutable target between them) and returns that target. func callProxyTargetFromRuntime(code []byte) (common.Address, bool) { targetOffset := len(callProxyRuntimePrefix) suffixOffset := targetOffset + common.HashLength diff --git a/sdk/evm/execution_error_test.go b/sdk/evm/execution_error_test.go index 65b9517b1..da88dc857 100644 --- a/sdk/evm/execution_error_test.go +++ b/sdk/evm/execution_error_test.go @@ -1432,7 +1432,7 @@ func TestCallProxyBytecodeFingerprintMatchesBinding(t *testing.T) { t.Parallel() runtime := callProxyRuntimeFromBinding(t) - require.GreaterOrEqual(t, len(runtime), len(callProxyRuntimePrefix)+len(callProxyRuntimeSuffix)) + require.GreaterOrEqual(t, len(runtime), len(callProxyRuntimePrefix)+common.HashLength+len(callProxyRuntimeSuffix)) assert.Equal(t, callProxyRuntimePrefix, runtime[:len(callProxyRuntimePrefix)]) suffixStart := len(callProxyRuntimePrefix) + common.HashLength assert.Equal(t, callProxyRuntimeSuffix, runtime[suffixStart:suffixStart+len(callProxyRuntimeSuffix)]) From e53ac22cdd2f4f2021e94d424c5afce38bffc3a2 Mon Sep 17 00:00:00 2001 From: Pablo Date: Thu, 4 Jun 2026 13:27:15 -0600 Subject: [PATCH 3/6] fix: linting Signed-off-by: Pablo --- sdk/evm/execution_error_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/evm/execution_error_test.go b/sdk/evm/execution_error_test.go index da88dc857..a254343f0 100644 --- a/sdk/evm/execution_error_test.go +++ b/sdk/evm/execution_error_test.go @@ -1654,7 +1654,7 @@ func callProxyRuntimeFromBinding(t *testing.T) []byte { bin := common.FromHex(bindings.CallProxyBin) marker := []byte{0xf3, 0xfe} idx := bytes.Index(bin, marker) - require.Greater(t, idx, 0, "call proxy init/runtime separator not found in CallProxyBin") + require.Positive(t, idx, "call proxy init/runtime separator not found in CallProxyBin") return bin[idx+len(marker):] } From 0a06c2e5dd35cfdf03f2e2af8d763294a3b68912 Mon Sep 17 00:00:00 2001 From: Pablo Date: Fri, 5 Jun 2026 08:10:18 -0600 Subject: [PATCH 4/6] fix: linting Signed-off-by: Pablo --- sdk/evm/execution_error.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/evm/execution_error.go b/sdk/evm/execution_error.go index 1df155491..aa234fbaf 100644 --- a/sdk/evm/execution_error.go +++ b/sdk/evm/execution_error.go @@ -486,14 +486,14 @@ func parseBytesFromString(s string) []byte { if len(parts) == 0 { return nil } - bytes := make([]byte, 0, len(parts)) + parsed := make([]byte, 0, len(parts)) for _, part := range parts { if val, err := strconv.ParseUint(part, 10, 8); err == nil { - bytes = append(bytes, byte(val)) + parsed = append(parsed, byte(val)) } } - return bytes + return parsed } // decodeRevertReasonFromCustomError decodes the revert reason using the error selector to find From 238fc595e3842e9be6ee50a3dd0b50da3217e7bf Mon Sep 17 00:00:00 2001 From: Pablo Estrada <139084212+ecPablo@users.noreply.github.com> Date: Fri, 5 Jun 2026 08:42:52 -0600 Subject: [PATCH 5/6] fix: copilot comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- sdk/evm/execution_error_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/evm/execution_error_test.go b/sdk/evm/execution_error_test.go index a254343f0..40f66f1ee 100644 --- a/sdk/evm/execution_error_test.go +++ b/sdk/evm/execution_error_test.go @@ -1654,8 +1654,7 @@ func callProxyRuntimeFromBinding(t *testing.T) []byte { bin := common.FromHex(bindings.CallProxyBin) marker := []byte{0xf3, 0xfe} idx := bytes.Index(bin, marker) - require.Positive(t, idx, "call proxy init/runtime separator not found in CallProxyBin") - + require.NotEqual(t, -1, idx, "call proxy init/runtime separator not found in CallProxyBin") return bin[idx+len(marker):] } From 8b49daff30edfbb58fceded8d46035d342229c92 Mon Sep 17 00:00:00 2001 From: Pablo Date: Fri, 5 Jun 2026 08:56:18 -0600 Subject: [PATCH 6/6] fix: linting Signed-off-by: Pablo --- sdk/evm/execution_error_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/evm/execution_error_test.go b/sdk/evm/execution_error_test.go index 40f66f1ee..66e3489d7 100644 --- a/sdk/evm/execution_error_test.go +++ b/sdk/evm/execution_error_test.go @@ -1655,6 +1655,7 @@ func callProxyRuntimeFromBinding(t *testing.T) []byte { marker := []byte{0xf3, 0xfe} idx := bytes.Index(bin, marker) require.NotEqual(t, -1, idx, "call proxy init/runtime separator not found in CallProxyBin") + return bin[idx+len(marker):] }