fix: replace sender to timelock address during error decoding (#775)

Timelock execution failures through a CallProxy were decoded with the
wrong underlying revert. CI reported `OnlyCallableByOwner()` on
WorkflowRegistry, but the real failure was
`NodeDoesNotSupportCapability(...)`.

#### Root cause
When `mcms.WithCallProxy` is used, the EVM executor sends the tx to the
`CallProxy,` not the `RBACTimelock`. On failure, `BuildExecutionError`
replayed the inner timelock call with `From` set to that execution
address (the proxy). That synthetic path is not what happens on chain:
CallProxy forwards to the timelock, and the target sees `msg.sender` as
the timelock. Replaying from the proxy made Ownable targets revert with
`OnlyCallableByOwner()` even when the real execution had already passed
ownership and failed for a different reason.

#### Fix
EVM-only change in `sdk/evm/execution_error.go`: before underlying-call
replay, resolve the real sender. If the execution address is a generated
CallProxy (runtime bytecode fingerprint from bindings.CallProxy),
extract the embedded timelock target and use it as From for eth_call
replay. Otherwise behavior is unchanged.

### AI Summary

This pull request enhances the handling of execution errors for EVM
timelock transactions, especially when transactions are routed through a
CallProxy contract. The main improvement is the ability to correctly
identify and replay the underlying call from the true sender (the
embedded timelock) even when a proxy is involved, providing more
accurate revert reasons. The changes also add robust testing for
CallProxy bytecode fingerprinting and sender resolution logic.

**Enhancements to CallProxy handling and error replay:**

* Added logic to detect when an address is a CallProxy contract by
matching its runtime bytecode against known prefix/suffix fingerprints,
and to extract the embedded target (timelock) address from the bytecode.
This allows the system to replay the underlying transaction as the
correct sender, improving error diagnosis.
(`sdk/evm/execution_error.go`)
[[1]](diffhunk://#diff-9877f8386f0be068752fac0fe1b707306afd2dca253b8b10b494f145c3b08131R35-R40)
[[2]](diffhunk://#diff-9877f8386f0be068752fac0fe1b707306afd2dca253b8b10b494f145c3b08131L298-R351)
* Introduced the `resolveUnderlyingCallSender` function, which
determines the true sender for replaying failed calls, handling both
proxy and non-proxy cases. (`sdk/evm/execution_error.go`)

**Testing and validation improvements:**

* Added comprehensive unit tests for CallProxy bytecode detection,
target extraction, and sender resolution, including edge cases for
incorrect bytecode and error fallbacks.
(`sdk/evm/execution_error_test.go`)
* Added integration-style tests to ensure that when a timelock operation
fails through a CallProxy, the error replay uses the correct sender and
the underlying revert reason is surfaced accurately.
(`sdk/evm/timelock_executor_test.go`)
[[1]](diffhunk://#diff-2f3502ced6798d1ac2675c1ae7d2e1beae121201c834d810ca2b00dbf6ab6accR102-R135)
[[2]](diffhunk://#diff-2f3502ced6798d1ac2675c1ae7d2e1beae121201c834d810ca2b00dbf6ab6accR78)
[[3]](diffhunk://#diff-2f3502ced6798d1ac2675c1ae7d2e1beae121201c834d810ca2b00dbf6ab6accR175-R178)

**Other changes:**

* Updated imports to support new logic and tests.
(`sdk/evm/execution_error.go`, `sdk/evm/execution_error_test.go`,
`sdk/evm/timelock_executor_test.go`)
[[1]](diffhunk://#diff-9877f8386f0be068752fac0fe1b707306afd2dca253b8b10b494f145c3b08131R4)
[[2]](diffhunk://#diff-809122f496465d2c75e2526e781484db6146c7e09f05750872260c7a19a8a0e0R4-R16)
[[3]](diffhunk://#diff-2f3502ced6798d1ac2675c1ae7d2e1beae121201c834d810ca2b00dbf6ab6accR10)

These changes significantly improve the reliability and transparency of
error handling for timelock operations, especially in the presence of
proxy contracts.

---------

Signed-off-by: Pablo <pablo.estrada@smartcontract.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: ecPablo

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>: <hex-data>"
 	// 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 (
@@ -296,14 +303,45 @@ 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
 	}
 
 	return execErr
 }
 
+func resolveUnderlyingCallSender(ctx context.Context, executionAddr common.Address, client ContractDeployBackend) common.Address {
+	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
+}
+
+// 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
+	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 {
@@ -449,14 +487,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
@@ -627,8 +665,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,

sdk/evm/execution_error_test.go

@@ -1,6 +1,7 @@
 package evm
 
 import (
+	"bytes"
 	"context"
 	"encoding/binary"
 	"encoding/json"
@@ -11,9 +12,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"
@@ -1423,6 +1426,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)+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)])
+
+	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.NotEqual(t, -1, idx, "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.