fix: vm.WithUNSAFECallerAddressProxying under DELEGATECALL

.github/workflows/go.yml

@@ -18,6 +18,6 @@ jobs:
           go-version: 1.21.4
       - name: Run tests
         run: | # Upstream flakes are race conditions exacerbated by concurrent tests
-          FLAKY_REGEX='go-ethereum/(eth|eth/tracers/logger|accounts/keystore|eth/downloader|miner|ethclient|ethclient/gethclient|eth/catalyst)$';
+          FLAKY_REGEX='go-ethereum/(eth|eth/tracers/js|eth/tracers/logger|accounts/keystore|eth/downloader|miner|ethclient|ethclient/gethclient|eth/catalyst)$';
           go list ./... | grep -P "${FLAKY_REGEX}" | xargs -n 1 go test -short;
           go test -short $(go list ./... | grep -Pv "${FLAKY_REGEX}");

core/vm/contracts.libevm.go

@@ -153,25 +153,9 @@ func (args *evmCallArgs) env() *environment {
 	}
 
 	return &environment{
-		evm:           args.evm,
-		self:          contract,
-		forceReadOnly: args.readOnly(),
-	}
-}
-
-func (args *evmCallArgs) readOnly() bool {
-	// A switch statement provides clearer code coverage for difficult-to-test
-	// cases.
-	switch {
-	case args.callType == staticCall:
-		// evm.interpreter.readOnly is only set to true via a call to
-		// EVMInterpreter.Run() so, if a precompile is called directly with
-		// StaticCall(), then readOnly might not be set yet.
-		return true
-	case args.evm.interpreter.readOnly:
-		return true
-	default:
-		return false
+		evm:      args.evm,
+		self:     contract,
+		callType: args.callType,
 	}
 }
 

core/vm/contracts.libevm_test.go

@@ -16,6 +16,7 @@
 package vm_test
 
 import (
+	"bytes"
 	"fmt"
 	"math/big"
 	"reflect"
@@ -375,10 +376,12 @@ func makeReturnProxy(t *testing.T, dest common.Address, call vm.OpCode) []vm.OpC
 	t.Helper()
 	const p0 = vm.PUSH0
 	contract := []vm.OpCode{
-		vm.PUSH1, 1, // retSize (bytes)
-		p0, // retOffset
-		p0, // argSize
-		p0, // argOffset
+		vm.CALLDATASIZE, p0, p0, vm.CALLDATACOPY,
+
+		p0,              // retSize
+		p0,              // retOffset
+		vm.CALLDATASIZE, // argSize
+		p0,              // argOffset
 	}
 
 	// See CALL signature: https://www.evm.codes/#f1?fork=cancun
@@ -511,13 +514,21 @@ func TestPrecompileMakeCall(t *testing.T) {
 	dest := common.HexToAddress("DE57")
 
 	rng := ethtest.NewPseudoRand(142857)
-	callData := rng.Bytes(8)
+	precompileCallData := rng.Bytes(8)
+
+	// If the SUT precompile receives this as its calldata then it will use the
+	// vm.WithUNSAFECallerAddressProxying() option.
+	unsafeCallerProxyOptSentinel := []byte("override-caller sentinel")
 
 	hooks := &hookstest.Stub{
 		PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
 			sut: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
+				var opts []vm.CallOption
+				if bytes.Equal(input, unsafeCallerProxyOptSentinel) {
+					opts = append(opts, vm.WithUNSAFECallerAddressProxying())
+				}
 				// We are ultimately testing env.Call(), hence why this is the SUT.
-				return env.Call(dest, callData, suppliedGas, uint256.NewInt(0))
+				return env.Call(dest, precompileCallData, suppliedGas, uint256.NewInt(0), opts...)
 			}),
 			dest: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
 				out := &statefulPrecompileOutput{
@@ -538,6 +549,7 @@ func TestPrecompileMakeCall(t *testing.T) {
 
 	tests := []struct {
 		incomingCallType vm.OpCode
+		eoaTxCallData    []byte
 		// Unlike TestNewStatefulPrecompile, which tests the AddressContext of
 		// the precompile itself, these test the AddressContext of a contract
 		// called by the precompile.
@@ -551,7 +563,19 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: sut,
 					Self:   dest,
 				},
-				Input: callData,
+				Input: precompileCallData,
+			},
+		},
+		{
+			incomingCallType: vm.CALL,
+			eoaTxCallData:    unsafeCallerProxyOptSentinel,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: caller, // overridden by CallOption
+					Self:   dest,
+				},
+				Input: precompileCallData,
 			},
 		},
 		{
@@ -562,7 +586,19 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // SUT runs as its own caller because of CALLCODE
 					Self:   dest,
 				},
-				Input: callData,
+				Input: precompileCallData,
+			},
+		},
+		{
+			incomingCallType: vm.CALLCODE,
+			eoaTxCallData:    unsafeCallerProxyOptSentinel,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: caller, // CallOption is a NOOP
+					Self:   dest,
+				},
+				Input: precompileCallData,
 			},
 		},
 		{
@@ -573,7 +609,19 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // as with CALLCODE
 					Self:   dest,
 				},
-				Input: callData,
+				Input: precompileCallData,
+			},
+		},
+		{
+			incomingCallType: vm.DELEGATECALL,
+			eoaTxCallData:    unsafeCallerProxyOptSentinel,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: caller, // CallOption is a NOOP
+					Self:   dest,
+				},
+				Input: precompileCallData,
 			},
 		},
 		{
@@ -584,25 +632,39 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: sut,
 					Self:   dest,
 				},
-				Input: callData,
+				Input: precompileCallData,
 				// This demonstrates that even though the precompile makes a
 				// (non-static) CALL, the read-only state is inherited. Yes,
 				// this is _another_ way to get a read-only state, different to
 				// the other tests.
 				ReadOnly: true,
 			},
 		},
+		{
+			incomingCallType: vm.STATICCALL,
+			eoaTxCallData:    unsafeCallerProxyOptSentinel,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: caller, // overridden by CallOption
+					Self:   dest,
+				},
+				Input:    precompileCallData,
+				ReadOnly: true,
+			},
+		},
 	}
 
 	for _, tt := range tests {
-		t.Run(fmt.Sprintf("via %s", tt.incomingCallType), func(t *testing.T) {
+		t.Run(tt.incomingCallType.String(), func(t *testing.T) {
+			t.Logf("calldata = %q", tt.eoaTxCallData)
 			state, evm := ethtest.NewZeroEVM(t)
 			evm.Origin = eoa
 			state.CreateAccount(caller)
 			proxy := makeReturnProxy(t, sut, tt.incomingCallType)
 			state.SetCode(caller, convertBytes[vm.OpCode, byte](proxy))
 
-			got, _, err := evm.Call(vm.AccountRef(eoa), caller, nil, 1e6, uint256.NewInt(0))
+			got, _, err := evm.Call(vm.AccountRef(eoa), caller, tt.eoaTxCallData, 1e6, uint256.NewInt(0))
 			require.NoError(t, err)
 			require.Equal(t, tt.want.String(), string(got))
 		})

core/vm/environment.libevm.go

@@ -31,18 +31,33 @@ import (
 var _ PrecompileEnvironment = (*environment)(nil)
 
 type environment struct {
-	evm           *EVM
-	self          *Contract
-	forceReadOnly bool
+	evm      *EVM
+	self     *Contract
+	callType callType
 }
 
 func (e *environment) ChainConfig() *params.ChainConfig  { return e.evm.chainConfig }
 func (e *environment) Rules() params.Rules               { return e.evm.chainRules }
-func (e *environment) ReadOnly() bool                    { return e.forceReadOnly || e.evm.interpreter.readOnly }
 func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB }
 func (e *environment) BlockNumber() *big.Int             { return new(big.Int).Set(e.evm.Context.BlockNumber) }
 func (e *environment) BlockTime() uint64                 { return e.evm.Context.Time }
 
+func (e *environment) ReadOnly() bool {
+	// A switch statement provides clearer code coverage for difficult-to-test
+	// cases.
+	switch {
+	case e.callType == staticCall:
+		// evm.interpreter.readOnly is only set to true via a call to
+		// EVMInterpreter.Run() so, if a precompile is called directly with
+		// StaticCall(), then readOnly might not be set yet.
+		return true
+	case e.evm.interpreter.readOnly:
+		return true
+	default:
+		return false
+	}
+}
+
 func (e *environment) Addresses() *libevm.AddressContext {
 	return &libevm.AddressContext{
 		Origin: e.evm.Origin,
@@ -83,7 +98,7 @@ func (e *environment) callContract(typ callType, addr common.Address, input []by
 	in.evm.depth++
 	defer func() { in.evm.depth-- }()
 
-	if e.forceReadOnly && !in.readOnly { // i.e. the precompile was StaticCall()ed
+	if e.ReadOnly() && !in.readOnly { // i.e. the precompile was StaticCall()ed
 		in.readOnly = true
 		defer func() { in.readOnly = false }()
 	}
@@ -95,6 +110,11 @@ func (e *environment) callContract(typ callType, addr common.Address, input []by
 			// Note that, in addition to being unsafe, this breaks an EVM
 			// assumption that the caller ContractRef is always a *Contract.
 			caller = AccountRef(e.self.CallerAddress)
+			if e.callType == delegateCall {
+				// self was created with AsDelegate(), which means that
+				// CallerAddress was inherited.
+				caller = AccountRef(e.self.Address())
+			}
 		case nil:
 		default:
 			return nil, gas, fmt.Errorf("unsupported option %T", o)