diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 542e16247d3b..ec48e30d725d 100644 --- a/.github/workflows/go.yml +++ b/.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}"); diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index 78b634ad4bf6..0c49bccce6b1 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -42,7 +42,7 @@ import ( // args := &evmCallArgs{evm, staticCall, caller, addr, input, gas, nil /*value*/} type evmCallArgs struct { evm *EVM - callType callType + callType CallType // args:start caller ContractRef @@ -53,15 +53,32 @@ type evmCallArgs struct { // args:end } -type callType uint8 +// A CallType refers to a *CALL* [OpCode] / respective method on [EVM]. +type CallType uint8 const ( - call callType = iota + 1 - callCode - delegateCall - staticCall + UnknownCallType CallType = iota + Call + CallCode + DelegateCall + StaticCall ) +// String returns a human-readable representation of the CallType. +func (t CallType) String() string { + switch t { + case Call: + return "Call" + case CallCode: + return "CallCode" + case DelegateCall: + return "DelegateCall" + case StaticCall: + return "StaticCall" + } + return fmt.Sprintf("Unknown %T(%d)", t, t) +} + // run runs the [PrecompiledContract], differentiating between stateful and // regular types. func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { @@ -115,6 +132,7 @@ type PrecompileEnvironment interface { // ReadOnlyState will always be non-nil. ReadOnlyState() libevm.StateReader Addresses() *libevm.AddressContext + IncomingCallType() CallType BlockHeader() (types.Header, error) BlockNumber() *big.Int @@ -132,46 +150,30 @@ func (args *evmCallArgs) env() *environment { value = args.value ) switch args.callType { - case staticCall: + case StaticCall: value = new(uint256.Int) fallthrough - case call: + case Call: self = args.addr - case delegateCall: + case DelegateCall: value = nil fallthrough - case callCode: + case CallCode: self = args.caller.Address() } // This is equivalent to the `contract` variables created by evm.*Call*() // methods, for non precompiles, to pass to [EVMInterpreter.Run]. contract := NewContract(args.caller, AccountRef(self), value, args.gas) - if args.callType == delegateCall { + if args.callType == DelegateCall { contract = contract.AsDelegate() } 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, } } diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index f1cecca30cf8..3fedb55db4d6 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -16,6 +16,7 @@ package vm_test import ( + "bytes" "fmt" "math/big" "reflect" @@ -106,6 +107,7 @@ type statefulPrecompileOutput struct { BlockNumber, Difficulty *big.Int BlockTime uint64 Input []byte + IncomingCallType vm.CallType } func (o statefulPrecompileOutput) String() string { @@ -121,6 +123,8 @@ func (o statefulPrecompileOutput) String() string { verb = "%#x" case *libevm.AddressContext: verb = "%+v" + case vm.CallType: + verb = "%d (%[2]q)" } lines = append(lines, fmt.Sprintf("%s: "+verb, name, fld)) } @@ -149,14 +153,15 @@ func TestNewStatefulPrecompile(t *testing.T) { } out := &statefulPrecompileOutput{ - ChainID: env.ChainConfig().ChainID, - Addresses: env.Addresses(), - StateValue: env.ReadOnlyState().GetState(precompile, slot), - ReadOnly: env.ReadOnly(), - BlockNumber: env.BlockNumber(), - BlockTime: env.BlockTime(), - Difficulty: hdr.Difficulty, - Input: input, + ChainID: env.ChainConfig().ChainID, + Addresses: env.Addresses(), + StateValue: env.ReadOnlyState().GetState(precompile, slot), + ReadOnly: env.ReadOnly(), + BlockNumber: env.BlockNumber(), + BlockTime: env.BlockTime(), + Difficulty: hdr.Difficulty, + Input: input, + IncomingCallType: env.IncomingCallType(), } return out.Bytes(), suppliedGas - gasCost, nil } @@ -199,6 +204,7 @@ func TestNewStatefulPrecompile(t *testing.T) { // Note that this only covers evm.readOnly being true because of the // precompile's call. See TestInheritReadOnly for alternate case. wantReadOnly bool + wantCallType vm.CallType }{ { name: "EVM.Call()", @@ -211,6 +217,7 @@ func TestNewStatefulPrecompile(t *testing.T) { Self: precompile, }, wantReadOnly: false, + wantCallType: vm.Call, }, { name: "EVM.CallCode()", @@ -223,6 +230,7 @@ func TestNewStatefulPrecompile(t *testing.T) { Self: caller, }, wantReadOnly: false, + wantCallType: vm.CallCode, }, { name: "EVM.DelegateCall()", @@ -235,6 +243,7 @@ func TestNewStatefulPrecompile(t *testing.T) { Self: caller, }, wantReadOnly: false, + wantCallType: vm.DelegateCall, }, { name: "EVM.StaticCall()", @@ -247,20 +256,22 @@ func TestNewStatefulPrecompile(t *testing.T) { Self: precompile, }, wantReadOnly: true, + wantCallType: vm.StaticCall, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { wantOutput := statefulPrecompileOutput{ - ChainID: chainID, - Addresses: tt.wantAddresses, - StateValue: value, - ReadOnly: tt.wantReadOnly, - BlockNumber: header.Number, - BlockTime: header.Time, - Difficulty: header.Difficulty, - Input: input, + ChainID: chainID, + Addresses: tt.wantAddresses, + StateValue: value, + ReadOnly: tt.wantReadOnly, + BlockNumber: header.Number, + BlockTime: header.Time, + Difficulty: header.Difficulty, + Input: input, + IncomingCallType: tt.wantCallType, } wantGasLeft := gasLimit - gasCost @@ -375,10 +386,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 +524,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 +559,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 +573,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 +596,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 +619,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,7 +642,7 @@ 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 @@ -592,19 +650,56 @@ func TestPrecompileMakeCall(t *testing.T) { 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)) }) } } + +//nolint:testableexamples // Including output would only make the example more complicated and hide the true intent +func ExamplePrecompileEnvironment() { + // To determine the actual caller of a precompile, as against the effective + // caller (under EVM rules, as exposed by `Addresses().Caller`): + actualCaller := func(env vm.PrecompileEnvironment) common.Address { + if env.IncomingCallType() == vm.DelegateCall { + // DelegateCall acts as if it were its own caller. + return env.Addresses().Self + } + // CallCode could return either `Self` or `Caller` as it acts as its + // caller but doesn't inherit the caller's caller as DelegateCall does. + // Having it handled here is arbitrary from a behavioural perspective + // and is done only to simplify the code. + // + // Call and StaticCall don't affect self/caller semantics in any way. + return env.Addresses().Caller + } + + // actualCaller would typically be a top-level function. It's only a + // variable to include it in this example function. + _ = actualCaller +} diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index 75a10c0d8570..312627d8649e 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -31,18 +31,34 @@ 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) IncomingCallType() CallType { return e.callType } 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, @@ -71,10 +87,10 @@ func (e *environment) BlockHeader() (types.Header, error) { } func (e *environment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) { - return e.callContract(call, addr, input, gas, value, opts...) + return e.callContract(Call, addr, input, gas, value, opts...) } -func (e *environment) callContract(typ callType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) { +func (e *environment) callContract(typ CallType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) { // Depth and read-only setting are handled by [EVMInterpreter.Run], which // isn't used for precompiles, so we need to do it ourselves to maintain the // expected invariants. @@ -83,7 +99,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 +111,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) @@ -102,12 +123,12 @@ func (e *environment) callContract(typ callType, addr common.Address, input []by } switch typ { - case call: + case Call: if in.readOnly && !value.IsZero() { return nil, gas, ErrWriteProtection } return e.evm.Call(caller, addr, input, gas, value) - case callCode, delegateCall, staticCall: + case CallCode, DelegateCall, StaticCall: // TODO(arr4n): these cases should be very similar to CALL, hence the // early abstraction, to signal to future maintainers. If implementing // them, there's likely no need to honour the diff --git a/core/vm/evm.go b/core/vm/evm.go index c6c735627fd4..813a33414abc 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -230,7 +230,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas } if isPrecompile { - args := &evmCallArgs{evm, call, caller, addr, input, gas, value} + args := &evmCallArgs{evm, Call, caller, addr, input, gas, value} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { // Initialise a new contract and set the code that is to be used by the EVM. @@ -294,7 +294,7 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte, // It is allowed to call precompiles, even via delegatecall if p, isPrecompile := evm.precompile(addr); isPrecompile { - args := &evmCallArgs{evm, callCode, caller, addr, input, gas, value} + args := &evmCallArgs{evm, CallCode, caller, addr, input, gas, value} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { addrCopy := addr @@ -340,7 +340,7 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by // It is allowed to call precompiles, even via delegatecall if p, isPrecompile := evm.precompile(addr); isPrecompile { - args := &evmCallArgs{evm, delegateCall, caller, addr, input, gas, nil} + args := &evmCallArgs{evm, DelegateCall, caller, addr, input, gas, nil} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { addrCopy := addr @@ -390,7 +390,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte } if p, isPrecompile := evm.precompile(addr); isPrecompile { - args := &evmCallArgs{evm, staticCall, caller, addr, input, gas, nil} + args := &evmCallArgs{evm, StaticCall, caller, addr, input, gas, nil} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { // At this point, we use a copy of address. If we don't, the go compiler will