From 6170c14f70a58768416250b45f6632143429b437 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 15 Nov 2024 14:25:20 +0000 Subject: [PATCH 01/10] refactor: `PrecompileEnvironment.UseGas()` in lieu of return value --- core/vm/contracts.libevm.go | 25 +++++++++++++++++----- core/vm/contracts.libevm_test.go | 29 +++++++++++++------------ core/vm/environment.libevm.go | 36 +++++++++++++++++++++++++------- 3 files changed, 65 insertions(+), 25 deletions(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index c235d0a78094..d95f1314d327 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -92,7 +92,8 @@ func (t CallType) OpCode() OpCode { // regular types. func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { if p, ok := p.(statefulPrecompile); ok { - return p(args.env(), input, suppliedGas) + // `suppliedGas` is already held by the args. + return p.run(args.env(), input) } // Gas consumption for regular precompiles was already handled by the native // RunPrecompiledContract(), which called this method. @@ -102,7 +103,11 @@ func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas ui // PrecompiledStatefulContract is the stateful equivalent of a // [PrecompiledContract]. -type PrecompiledStatefulContract func(env PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) +// +// Instead of receiving and returning gas limits, stateful precompiles use the +// respective methods on [PrecompileEnvironment]. If a call to UseGas() returns +// false, a stateful precompile SHOULD return [ErrOutOfGas]. +type PrecompiledStatefulContract func(env PrecompileEnvironment, input []byte) (ret []byte, err error) // NewStatefulPrecompile constructs a new PrecompiledContract that can be used // via an [EVM] instance but MUST NOT be called directly; a direct call to Run() @@ -118,6 +123,11 @@ func NewStatefulPrecompile(run PrecompiledStatefulContract) PrecompiledContract // [PrecompiledStatefulContract] to hide implementation details. type statefulPrecompile PrecompiledStatefulContract +func (p statefulPrecompile) run(env *environment, input []byte) ([]byte, uint64, error) { + ret, err := p(env, input) + return ret, env.self.Gas, err +} + // RequiredGas always returns zero as this gas is consumed by native geth code // before the contract is run. func (statefulPrecompile) RequiredGas([]byte) uint64 { return 0 } @@ -135,13 +145,18 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) { type PrecompileEnvironment interface { ChainConfig() *params.ChainConfig Rules() params.Rules - ReadOnly() bool // StateDB will be non-nil i.f.f !ReadOnly(). StateDB() StateDB // ReadOnlyState will always be non-nil. ReadOnlyState() libevm.StateReader - Addresses() *libevm.AddressContext + IncomingCallType() CallType + Addresses() *libevm.AddressContext + ReadOnly() bool + // Equivalent to respective methods on [Contract]. + Gas() uint64 + UseGas(uint64) bool + Value() *uint256.Int BlockHeader() (types.Header, error) BlockNumber() *big.Int @@ -150,7 +165,7 @@ type PrecompileEnvironment interface { // Call is equivalent to [EVM.Call] except that the `caller` argument is // removed and automatically determined according to the type of call that // invoked the precompile. - Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, gasRemaining uint64, _ error) + Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, _ error) } func (args *evmCallArgs) env() *environment { diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index 8c451ff66c89..d821f1b8128c 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -146,13 +146,16 @@ func TestNewStatefulPrecompile(t *testing.T) { const gasLimit = 1e6 gasCost := rng.Uint64n(gasLimit) - run := func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) ([]byte, uint64, error) { + run := func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { if got, want := env.StateDB() != nil, !env.ReadOnly(); got != want { - return nil, 0, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) + return nil, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) } hdr, err := env.BlockHeader() if err != nil { - return nil, 0, err + return nil, err + } + if !env.UseGas(gasCost) { + return nil, vm.ErrOutOfGas } out := &statefulPrecompileOutput{ @@ -166,7 +169,7 @@ func TestNewStatefulPrecompile(t *testing.T) { Input: input, IncomingCallType: env.IncomingCallType(), } - return out.Bytes(), suppliedGas - gasCost, nil + return out.Bytes(), nil } hooks := &hookstest.Stub{ PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{ @@ -318,11 +321,11 @@ func TestInheritReadOnly(t *testing.T) { hooks := &hookstest.Stub{ PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{ precompile: vm.NewStatefulPrecompile( - func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) ([]byte, uint64, error) { + func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { if env.ReadOnly() { - return []byte{ifReadOnly}, suppliedGas, nil + return []byte{ifReadOnly}, nil } - return []byte{ifNotReadOnly}, suppliedGas, nil + return []byte{ifNotReadOnly}, nil }, ), }, @@ -535,21 +538,21 @@ func TestPrecompileMakeCall(t *testing.T) { 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) { + sut: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte) (ret []byte, 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, precompileCallData, suppliedGas, uint256.NewInt(0), opts...) + return env.Call(dest, precompileCallData, env.Gas(), uint256.NewInt(0), opts...) }), - dest: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { + dest: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte) (ret []byte, err error) { out := &statefulPrecompileOutput{ Addresses: env.Addresses(), ReadOnly: env.ReadOnly(), Input: input, // expected to be callData } - return out.Bytes(), suppliedGas, nil + return out.Bytes(), nil }), }, } @@ -696,8 +699,8 @@ func TestPrecompileCallWithTracer(t *testing.T) { hooks := &hookstest.Stub{ PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{ - precompile: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { - return env.Call(contract, nil, suppliedGas, uint256.NewInt(0)) + precompile: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte) (ret []byte, err error) { + return env.Call(contract, nil, env.Gas(), uint256.NewInt(0)) }), }, } diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index 1f87213e2ce6..fbf4e0348fb8 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -23,6 +23,7 @@ import ( "github.com/holiman/uint256" "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/common/math" "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/libevm" "github.com/ava-labs/libevm/params" @@ -36,6 +37,10 @@ type environment struct { callType CallType } +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 e.self.Value() } + func (e *environment) ChainConfig() *params.ChainConfig { return e.evm.chainConfig } func (e *environment) Rules() params.Rules { return e.evm.chainRules } func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB } @@ -43,6 +48,15 @@ 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) refundGas(add uint64) error { + gas, overflow := math.SafeAdd(e.self.Gas, add) + if overflow { + return ErrGasUintOverflow + } + e.self.Gas = gas + return nil +} + func (e *environment) ReadOnly() bool { // A switch statement provides clearer code coverage for difficult-to-test // cases. @@ -86,11 +100,11 @@ func (e *environment) BlockHeader() (types.Header, error) { return *hdr, nil } -func (e *environment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) { +func (e *environment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, error) { 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) (retData []byte, retGas uint64, retErr error) { +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. @@ -118,13 +132,17 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by } case nil: default: - return nil, gas, fmt.Errorf("unsupported option %T", o) + return nil, fmt.Errorf("unsupported option %T", o) } } if in.readOnly && value != nil && !value.IsZero() { - return nil, gas, ErrWriteProtection + return nil, ErrWriteProtection } + if !e.UseGas(gas) { + return nil, ErrOutOfGas + } + if t := e.evm.Config.Tracer; t != nil { var bigVal *big.Int if value != nil { @@ -134,13 +152,17 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by startGas := gas defer func() { - t.CaptureEnd(retData, startGas-retGas, retErr) + t.CaptureEnd(retData, startGas-e.Gas(), retErr) }() } switch typ { case Call: - return e.evm.Call(caller, addr, input, gas, value) + ret, returnGas, err := e.evm.Call(caller, addr, input, gas, value) + if err := e.refundGas(returnGas); err != nil { + return nil, err + } + return ret, err 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 @@ -149,6 +171,6 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by // compatibility. fallthrough default: - return nil, gas, fmt.Errorf("unimplemented precompile call type %v", typ) + return nil, fmt.Errorf("unimplemented precompile call type %v", typ) } } From bbc3b1b017146b1d2e6aba58a5f6734e3dacad90 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 15 Nov 2024 14:48:13 +0000 Subject: [PATCH 02/10] feat: `legacy` package for backwards-compatible type support --- core/vm/contracts.libevm_test.go | 18 ++++++++------- libevm/legacy/legacy.go | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 libevm/legacy/legacy.go diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index d821f1b8128c..e4d134ed17ea 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -39,6 +39,7 @@ import ( "github.com/ava-labs/libevm/libevm" "github.com/ava-labs/libevm/libevm/ethtest" "github.com/ava-labs/libevm/libevm/hookstest" + "github.com/ava-labs/libevm/libevm/legacy" "github.com/ava-labs/libevm/params" ) @@ -146,16 +147,13 @@ func TestNewStatefulPrecompile(t *testing.T) { const gasLimit = 1e6 gasCost := rng.Uint64n(gasLimit) - run := func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { + run := func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) ([]byte, uint64, error) { if got, want := env.StateDB() != nil, !env.ReadOnly(); got != want { - return nil, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) + return nil, suppliedGas, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) } hdr, err := env.BlockHeader() if err != nil { - return nil, err - } - if !env.UseGas(gasCost) { - return nil, vm.ErrOutOfGas + return nil, suppliedGas, err } out := &statefulPrecompileOutput{ @@ -169,11 +167,15 @@ func TestNewStatefulPrecompile(t *testing.T) { Input: input, IncomingCallType: env.IncomingCallType(), } - return out.Bytes(), nil + return out.Bytes(), suppliedGas - gasCost, nil } hooks := &hookstest.Stub{ PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{ - precompile: vm.NewStatefulPrecompile(run), + precompile: vm.NewStatefulPrecompile( + // In production, the new function signature should be used, but + // this just exercises the converter. + legacy.PrecompiledStatefulContract(run).Upgrade(), + ), }, } hooks.Register(t) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go new file mode 100644 index 000000000000..36f8787d134e --- /dev/null +++ b/libevm/legacy/legacy.go @@ -0,0 +1,39 @@ +// Copyright 2024 the libevm authors. +// +// The libevm additions to go-ethereum are free software: you can redistribute +// them and/or modify them under the terms of the GNU Lesser General Public License +// as published by the Free Software Foundation, either version 3 of the License, +// or (at your option) any later version. +// +// The libevm additions are distributed in the hope that they will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser +// General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see +// . + +// Package legacy provides converters between legacy types and their refactored +// equivalents. +package legacy + +import "github.com/ava-labs/libevm/core/vm" + +// PrecompiledStatefulContract is the legacy signature of +// [vm.PrecompiledStatefulContract], which explicitly accepts and returns gas +// values. Instances SHOULD NOT use the [vm.PrecompileEnvironment] +// gas-management methods as this may result in unexpected behaviour. +type PrecompiledStatefulContract func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) + +// Upgrade converts the legacy precompile signature into the now-required form. +func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { + return func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { + gas := env.Gas() + ret, remainingGas, err := c(env, input, env.Gas()) + if used := gas - remainingGas; used < gas { + env.UseGas(used) + } + return ret, err + } +} From 405740fe3a756f128be500c9d3f5b1e417fc50d0 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 15 Nov 2024 14:51:52 +0000 Subject: [PATCH 03/10] chore: revert inconsequential change to test code --- core/vm/contracts.libevm.go | 2 +- core/vm/contracts.libevm_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index d95f1314d327..dadd7e3f118d 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -92,7 +92,7 @@ func (t CallType) OpCode() OpCode { // regular types. func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { if p, ok := p.(statefulPrecompile); ok { - // `suppliedGas` is already held by the args. + // `suppliedGas` is already held by the args, and captured by `env()`. return p.run(args.env(), input) } // Gas consumption for regular precompiles was already handled by the native diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index e4d134ed17ea..21b8b88d02ff 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -149,11 +149,11 @@ func TestNewStatefulPrecompile(t *testing.T) { run := func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) ([]byte, uint64, error) { if got, want := env.StateDB() != nil, !env.ReadOnly(); got != want { - return nil, suppliedGas, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) + return nil, 0, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) } hdr, err := env.BlockHeader() if err != nil { - return nil, suppliedGas, err + return nil, 0, err } out := &statefulPrecompileOutput{ From f6518140136741f1e58e9c0bab3511eee7336590 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Sat, 16 Nov 2024 13:22:43 +0000 Subject: [PATCH 04/10] test: `vm.PrecompileEnvironment.Value()` --- core/vm/contracts.libevm.go | 6 ++--- core/vm/contracts.libevm_test.go | 42 ++++++++++++++++++++------------ 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index dadd7e3f118d..5ff9bda40713 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -104,9 +104,9 @@ func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas ui // PrecompiledStatefulContract is the stateful equivalent of a // [PrecompiledContract]. // -// Instead of receiving and returning gas limits, stateful precompiles use the -// respective methods on [PrecompileEnvironment]. If a call to UseGas() returns -// false, a stateful precompile SHOULD return [ErrOutOfGas]. +// Instead of receiving and returning gas arguments, stateful precompiles use +// the respective methods on [PrecompileEnvironment]. If a call to UseGas() +// returns false, a stateful precompile SHOULD return [ErrOutOfGas]. type PrecompiledStatefulContract func(env PrecompileEnvironment, input []byte) (ret []byte, err error) // NewStatefulPrecompile constructs a new PrecompiledContract that can be used diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index 21b8b88d02ff..1bb98fac2fa0 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -107,6 +107,7 @@ type statefulPrecompileOutput struct { ChainID *big.Int Addresses *libevm.AddressContext StateValue common.Hash + ValueReceived *uint256.Int ReadOnly bool BlockNumber, Difficulty *big.Int BlockTime uint64 @@ -160,6 +161,7 @@ func TestNewStatefulPrecompile(t *testing.T) { ChainID: env.ChainConfig().ChainID, Addresses: env.Addresses(), StateValue: env.ReadOnlyState().GetState(precompile, slot), + ValueReceived: env.Value(), ReadOnly: env.ReadOnly(), BlockNumber: env.BlockNumber(), BlockTime: env.BlockTime(), @@ -186,7 +188,8 @@ func TestNewStatefulPrecompile(t *testing.T) { Difficulty: rng.BigUint64(), } input := rng.Bytes(8) - value := rng.Hash() + stateValue := rng.Hash() + transferValue := rng.Uint256() chainID := rng.BigUint64() caller := common.HexToAddress("CA11E12") // caller of the precompile @@ -202,13 +205,15 @@ func TestNewStatefulPrecompile(t *testing.T) { ¶ms.ChainConfig{ChainID: chainID}, ), ) - state.SetState(precompile, slot, value) + state.SetState(precompile, slot, stateValue) + state.SetBalance(caller, new(uint256.Int).Not(uint256.NewInt(0))) evm.Origin = eoa tests := []struct { - name string - call func() ([]byte, uint64, error) - wantAddresses *libevm.AddressContext + name string + call func() ([]byte, uint64, error) + wantAddresses *libevm.AddressContext + wantTransferValue *uint256.Int // Note that this only covers evm.readOnly being true because of the // precompile's call. See TestInheritReadOnly for alternate case. wantReadOnly bool @@ -217,28 +222,30 @@ func TestNewStatefulPrecompile(t *testing.T) { { name: "EVM.Call()", call: func() ([]byte, uint64, error) { - return evm.Call(callerContract, precompile, input, gasLimit, uint256.NewInt(0)) + return evm.Call(callerContract, precompile, input, gasLimit, transferValue) }, wantAddresses: &libevm.AddressContext{ Origin: eoa, Caller: caller, Self: precompile, }, - wantReadOnly: false, - wantCallType: vm.Call, + wantReadOnly: false, + wantTransferValue: transferValue, + wantCallType: vm.Call, }, { name: "EVM.CallCode()", call: func() ([]byte, uint64, error) { - return evm.CallCode(callerContract, precompile, input, gasLimit, uint256.NewInt(0)) + return evm.CallCode(callerContract, precompile, input, gasLimit, transferValue) }, wantAddresses: &libevm.AddressContext{ Origin: eoa, Caller: caller, Self: caller, }, - wantReadOnly: false, - wantCallType: vm.CallCode, + wantReadOnly: false, + wantTransferValue: transferValue, + wantCallType: vm.CallCode, }, { name: "EVM.DelegateCall()", @@ -250,8 +257,9 @@ func TestNewStatefulPrecompile(t *testing.T) { Caller: eoa, // inherited from caller Self: caller, }, - wantReadOnly: false, - wantCallType: vm.DelegateCall, + wantReadOnly: false, + wantTransferValue: uint256.NewInt(0), + wantCallType: vm.DelegateCall, }, { name: "EVM.StaticCall()", @@ -263,8 +271,9 @@ func TestNewStatefulPrecompile(t *testing.T) { Caller: caller, Self: precompile, }, - wantReadOnly: true, - wantCallType: vm.StaticCall, + wantReadOnly: true, + wantTransferValue: uint256.NewInt(0), + wantCallType: vm.StaticCall, }, } @@ -273,7 +282,8 @@ func TestNewStatefulPrecompile(t *testing.T) { wantOutput := statefulPrecompileOutput{ ChainID: chainID, Addresses: tt.wantAddresses, - StateValue: value, + StateValue: stateValue, + ValueReceived: tt.wantTransferValue, ReadOnly: tt.wantReadOnly, BlockNumber: header.Number, BlockTime: header.Time, From abd110e24d10a6a4960e3f4f727486d0f6852d4e Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 18 Nov 2024 14:49:16 +0000 Subject: [PATCH 05/10] refactor: remove echoed gas argument from `evmCallArgs.run()` --- core/vm/contracts.go | 4 +++- core/vm/contracts.libevm.go | 36 ++++++++++++++++-------------------- core/vm/libevm_test.go | 2 +- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/core/vm/contracts.go b/core/vm/contracts.go index dc07de850251..a869debebba6 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -177,7 +177,9 @@ func (args *evmCallArgs) RunPrecompiledContract(p PrecompiledContract, input []b return nil, 0, ErrOutOfGas } suppliedGas -= gasCost - return args.run(p, input, suppliedGas) + args.gasRemaining = suppliedGas + output, err := args.run(p, input) + return output, args.gasRemaining, err } // ECRECOVER implemented as a native contract. diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index 5ff9bda40713..b3033a5c154b 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -45,11 +45,11 @@ type evmCallArgs struct { callType CallType // args:start - caller ContractRef - addr common.Address - input []byte - gas uint64 - value *uint256.Int + caller ContractRef + addr common.Address + input []byte + gasRemaining uint64 + value *uint256.Int // args:end } @@ -89,16 +89,17 @@ func (t CallType) OpCode() OpCode { } // 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) { - if p, ok := p.(statefulPrecompile); ok { - // `suppliedGas` is already held by the args, and captured by `env()`. - return p.run(args.env(), input) +// regular types, updating `gasRemaining` in the stateful case. +func (args *evmCallArgs) run(p PrecompiledContract, input []byte) (ret []byte, err error) { + switch p := p.(type) { + default: + return p.Run(input) + case statefulPrecompile: + env := args.env() + ret, err := p(env, input) + args.gasRemaining = env.Gas() + return ret, err } - // Gas consumption for regular precompiles was already handled by the native - // RunPrecompiledContract(), which called this method. - ret, err = p.Run(input) - return ret, suppliedGas, err } // PrecompiledStatefulContract is the stateful equivalent of a @@ -123,11 +124,6 @@ func NewStatefulPrecompile(run PrecompiledStatefulContract) PrecompiledContract // [PrecompiledStatefulContract] to hide implementation details. type statefulPrecompile PrecompiledStatefulContract -func (p statefulPrecompile) run(env *environment, input []byte) ([]byte, uint64, error) { - ret, err := p(env, input) - return ret, env.self.Gas, err -} - // RequiredGas always returns zero as this gas is consumed by native geth code // before the contract is run. func (statefulPrecompile) RequiredGas([]byte) uint64 { return 0 } @@ -189,7 +185,7 @@ func (args *evmCallArgs) env() *environment { // 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) + contract := NewContract(args.caller, AccountRef(self), value, args.gasRemaining) if args.callType == DelegateCall { contract = contract.AsDelegate() } diff --git a/core/vm/libevm_test.go b/core/vm/libevm_test.go index 4e365d4e425d..76ccfee9a49f 100644 --- a/core/vm/libevm_test.go +++ b/core/vm/libevm_test.go @@ -18,5 +18,5 @@ package vm // The original RunPrecompiledContract was migrated to being a method on // [evmCallArgs]. We need to replace it for use by regular geth tests. func RunPrecompiledContract(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { - return (*evmCallArgs)(nil).RunPrecompiledContract(p, input, suppliedGas) + return new(evmCallArgs).RunPrecompiledContract(p, input, suppliedGas) } From d2697b1a2bf5321c87ca089e5f7c9f16d438e287 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 18 Nov 2024 14:59:24 +0000 Subject: [PATCH 06/10] refactor: `PrecompileEnvironment.Value()` returns a copy --- core/vm/environment.libevm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index fbf4e0348fb8..c7cb135e2be1 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -39,7 +39,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 e.self.Value() } +func (e *environment) Value() *uint256.Int { return new(uint256.Int).Set(e.self.Value()) } func (e *environment) ChainConfig() *params.ChainConfig { return e.evm.chainConfig } func (e *environment) Rules() params.Rules { return e.evm.chainRules } From aae731a67bfd114aee6e63272e85165ed8815be4 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:00:00 +0000 Subject: [PATCH 07/10] refactor: avoid double call to `env.Gas()` Co-authored-by: Quentin McGaw Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> --- libevm/legacy/legacy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index 36f8787d134e..f9ff73090f0a 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -30,7 +30,7 @@ type PrecompiledStatefulContract func(env vm.PrecompileEnvironment, input []byte func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { return func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { gas := env.Gas() - ret, remainingGas, err := c(env, input, env.Gas()) + ret, remainingGas, err := c(env, input, gas) if used := gas - remainingGas; used < gas { env.UseGas(used) } From 2982ff2ae273dd1f25a1d42f76111968ce620452 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:01:07 +0000 Subject: [PATCH 08/10] doc: named return value for `UseGas()` interface method Co-authored-by: Quentin McGaw Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> --- core/vm/contracts.libevm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index b3033a5c154b..f5c8fea0a5cc 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -151,7 +151,7 @@ type PrecompileEnvironment interface { ReadOnly() bool // Equivalent to respective methods on [Contract]. Gas() uint64 - UseGas(uint64) bool + UseGas(uint64) (hasEnoughGas bool) Value() *uint256.Int BlockHeader() (types.Header, error) From dd098d58d654e8809bca9351453ff4ef2ca0fda7 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Dec 2024 17:47:35 +0000 Subject: [PATCH 09/10] doc: clarify `evmCallArgs.run()` comment --- core/vm/contracts.libevm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index f5c8fea0a5cc..66e3c91783a0 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -89,7 +89,7 @@ func (t CallType) OpCode() OpCode { } // run runs the [PrecompiledContract], differentiating between stateful and -// regular types, updating `gasRemaining` in the stateful case. +// 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: From 1cf2df049e698337af5f24a3d63ffe1fffdeef98 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Dec 2024 17:49:29 +0000 Subject: [PATCH 10/10] refactor: rename ambiguous `err` variable --- core/vm/environment.libevm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index c7cb135e2be1..5b79442e0adb 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -158,11 +158,11 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by switch typ { case Call: - ret, returnGas, err := e.evm.Call(caller, addr, input, gas, value) + ret, returnGas, callErr := e.evm.Call(caller, addr, input, gas, value) if err := e.refundGas(returnGas); err != nil { return nil, err } - return ret, err + return ret, callErr 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