diff --git a/core/vm/contracts.go b/core/vm/contracts.go index ff1f3a0ca79..7e005f54a25 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -168,15 +168,15 @@ func activePrecompiles(rules params.Rules) []common.Address { // - the returned bytes, // - the _remaining_ gas, // - any error that occurred -func (args *evmCallArgs) RunPrecompiledContract(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { +func (env *environment) RunPrecompiledContract(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { gasCost := p.RequiredGas(input) if suppliedGas < gasCost { return nil, 0, ErrOutOfGas } suppliedGas -= gasCost - args.gasRemaining = suppliedGas - output, err := args.run(p, input) - return output, args.gasRemaining, err + env.gasRemaining = suppliedGas + output, err := env.run(p, input) + return output, env.GasRemaining(), err } // ECRECOVER implemented as a native contract. diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index d56dbd1c396..a5c9d3c0d82 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -57,31 +57,6 @@ func ActivePrecompiles(rules params.Rules) []common.Address { return active } -// evmCallArgs mirrors the parameters of the [EVM] methods Call(), CallCode(), -// DelegateCall() and StaticCall(). Its fields are identical to those of the -// parameters, prepended with the receiver name and call type. As -// {Delegate,Static}Call don't accept a value, they MAY set the respective field -// to nil as it will be ignored. -// -// Instantiation can be achieved by merely copying the parameter names, in -// order, which is trivially achieved with AST manipulation: -// -// func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte, gas uint64) ... { -// ... -// args := &evmCallArgs{evm, staticCall, caller, addr, input, gas, nil /*value*/} -type evmCallArgs struct { - evm *EVM - callType CallType - - // args:start - caller ContractRef - addr common.Address - input []byte - gasRemaining uint64 - value *uint256.Int - // args:end -} - // A CallType refers to a *CALL* [OpCode] / respective method on [EVM]. type CallType OpCode @@ -101,6 +76,12 @@ func (t CallType) isValid() bool { } } +// readOnly returns whether the CallType induces a read-only state if not +// already in one. +func (t CallType) readOnly() bool { + return t == StaticCall +} + // String returns a human-readable representation of the CallType. func (t CallType) String() string { if t.isValid() { @@ -119,16 +100,28 @@ func (t CallType) OpCode() OpCode { // run runs the [PrecompiledContract], differentiating between stateful and // regular types, updating `args.gasRemaining` in the stateful case. -func (args *evmCallArgs) run(p PrecompiledContract, input []byte) (ret []byte, err error) { - switch p := p.(type) { - default: +func (env *environment) run(p PrecompiledContract, input []byte) (ret []byte, err error) { + sp, ok := p.(statefulPrecompile) + if !ok { return p.Run(input) - case statefulPrecompile: - env := args.env() - ret, err := p(env, input) - args.gasRemaining = env.Gas() - return ret, err } + + // 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. + in := env.evm.interpreter + + in.evm.depth++ + defer func() { in.evm.depth-- }() + + if env.readOnlyArg && !in.readOnly { + in.readOnly = true + defer func() { in.readOnly = false }() + } + + ret, err = sp(env, input) + env.gasRemaining = env.Gas() + return ret, err } // PrecompiledStatefulContract is the stateful equivalent of a @@ -196,41 +189,6 @@ type PrecompileEnvironment interface { Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, _ error) } -func (args *evmCallArgs) env() *environment { - var ( - self common.Address - value = args.value - ) - switch args.callType { - case StaticCall: - value = new(uint256.Int) - fallthrough - case Call: - self = args.addr - - case DelegateCall: - value = nil - fallthrough - 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.gasRemaining) - if args.callType == DelegateCall { - contract = contract.AsDelegate() - } - - return &environment{ - evm: args.evm, - self: contract, - callType: args.callType, - rawCaller: args.caller.Address(), - rawSelf: args.addr, - } -} - var ( // These lock in the assumptions made when implementing [evmCallArgs]. If // these break then the struct fields SHOULD be changed to match these diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index edd6c4314b6..2bf5a553b47 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -33,9 +33,11 @@ import ( var _ PrecompileEnvironment = (*environment)(nil) type environment struct { - evm *EVM - self *Contract - callType CallType + evm *EVM + self *Contract + callType CallType + readOnlyArg bool // readOnlyArg is different than interpreter.readOnly + gasRemaining uint64 rawSelf, rawCaller common.Address } @@ -43,6 +45,7 @@ type environment struct { func (e *environment) Gas() uint64 { return e.self.Gas } func (e *environment) UseGas(gas uint64) bool { return e.self.UseGas(gas) } func (e *environment) Value() *uint256.Int { return new(uint256.Int).Set(e.self.Value()) } +func (e *environment) GasRemaining() uint64 { return e.gasRemaining } func (e *environment) ChainConfig() *params.ChainConfig { return e.evm.chainConfig } func (e *environment) Rules() params.Rules { return e.evm.chainRules } @@ -63,19 +66,7 @@ func (e *environment) refundGas(add uint64) error { } 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 - } + return e.evm.interpreter.readOnly } func (e *environment) Addresses() *libevm.AddressContext { @@ -116,19 +107,6 @@ func (e *environment) Call(addr common.Address, input []byte, gas uint64, value } func (e *environment) callContract(typ CallType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) (retData []byte, retErr 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. - in := e.evm.interpreter - - in.evm.depth++ - defer func() { in.evm.depth-- }() - - if e.ReadOnly() && !in.readOnly { // i.e. the precompile was StaticCall()ed - in.readOnly = true - defer func() { in.readOnly = false }() - } - var caller ContractRef = e.self if options.As[callConfig](opts...).unsafeCallerAddressProxying { // Note that, in addition to being unsafe, this breaks an EVM @@ -141,7 +119,7 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by } } - if in.readOnly && value != nil && !value.IsZero() { + if e.ReadOnly() && value != nil && !value.IsZero() { return nil, ErrWriteProtection } if !e.UseGas(gas) { diff --git a/core/vm/evm.go b/core/vm/evm.go index b9fd682b9a7..8f0673253ec 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -235,8 +235,17 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas } if isPrecompile { - args := &evmCallArgs{evm, Call, caller, addr, input, gas, value} - ret, gas, err = args.RunPrecompiledContract(p, input, gas) + addrCopy := addr + env := &environment{ + evm: evm, + callType: Call, + rawSelf: addrCopy, + rawCaller: caller.Address(), + gasRemaining: gas, + self: NewContract(caller, AccountRef(addrCopy), value, gas), + readOnlyArg: false, + } + ret, gas, err = env.RunPrecompiledContract(p, input, gas) } else { // Initialise a new contract and set the code that is to be used by the EVM. // The contract is a scoped environment for this execution context only. @@ -287,7 +296,7 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte, if !evm.Context.CanTransfer(evm.StateDB, caller.Address(), value) { return nil, gas, ErrInsufficientBalance } - var snapshot = evm.StateDB.Snapshot() + snapshot := evm.StateDB.Snapshot() // Invoke tracer hooks that signal entering/exiting a call frame if evm.Config.Tracer != nil { @@ -299,8 +308,17 @@ 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} - ret, gas, err = args.RunPrecompiledContract(p, input, gas) + addrCopy := addr + env := &environment{ + evm: evm, + callType: Call, + rawSelf: addrCopy, + rawCaller: caller.Address(), + gasRemaining: gas, + self: NewContract(caller, AccountRef(addrCopy), value, gas), + readOnlyArg: false, + } + ret, gas, err = env.RunPrecompiledContract(p, input, gas) } else { addrCopy := addr // Initialise a new contract and set the code that is to be used by the EVM. @@ -329,7 +347,7 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by if evm.depth > int(params.CallCreateDepth) { return nil, gas, ErrDepth } - var snapshot = evm.StateDB.Snapshot() + snapshot := evm.StateDB.Snapshot() // Invoke tracer hooks that signal entering/exiting a call frame if evm.Config.Tracer != nil { @@ -345,8 +363,16 @@ 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} - ret, gas, err = args.RunPrecompiledContract(p, input, gas) + env := &environment{ + evm: evm, + callType: DelegateCall, + rawSelf: addr, + rawCaller: caller.Address(), + gasRemaining: gas, + self: NewContract(caller, AccountRef(caller.Address()), nil, gas).AsDelegate(), + readOnlyArg: false, + } + ret, gas, err = env.RunPrecompiledContract(p, input, gas) } else { addrCopy := addr // Initialise a new contract and make initialise the delegate values @@ -378,7 +404,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte // after all empty accounts were deleted, so this is not required. However, if we omit this, // then certain tests start failing; stRevertTest/RevertPrecompiledTouchExactOOG.json. // We could change this, but for now it's left for legacy reasons - var snapshot = evm.StateDB.Snapshot() + snapshot := evm.StateDB.Snapshot() // We do an AddBalance of zero here, just in order to trigger a touch. // This doesn't matter on Mainnet, where all empties are gone at the time of Byzantium, @@ -395,8 +421,17 @@ 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} - ret, gas, err = args.RunPrecompiledContract(p, input, gas) + addrCopy := addr + env := &environment{ + evm: evm, + callType: StaticCall, + rawSelf: addrCopy, + rawCaller: caller.Address(), + gasRemaining: gas, + self: NewContract(caller, AccountRef(addrCopy), new(uint256.Int), gas), + readOnlyArg: true, + } + ret, gas, err = env.RunPrecompiledContract(p, input, gas) } else { // At this point, we use a copy of address. If we don't, the go compiler will // leak the 'contract' to the outer scope, and make allocation for 'contract'