From 8d288b76b6775bb13a61b6a55c46280d7784a462 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 09:15:36 +0100 Subject: [PATCH 01/23] fix(libevm/legacy): disallow remaining gas higher than input gas in `PrecompiledStatefulContract` --- libevm/legacy/legacy.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index f9ff73090f0a..792cb773ec04 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -18,7 +18,12 @@ // equivalents. package legacy -import "github.com/ava-labs/libevm/core/vm" +import ( + "errors" + "fmt" + + "github.com/ava-labs/libevm/core/vm" +) // PrecompiledStatefulContract is the legacy signature of // [vm.PrecompiledStatefulContract], which explicitly accepts and returns gas @@ -26,11 +31,18 @@ import "github.com/ava-labs/libevm/core/vm" // 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) +var ( + ErrGasRemainingExceedsGasSupplied = errors.New("remaining gas exceeds supplied gas") +) + // 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, gas) + if remainingGas > gas { + return nil, fmt.Errorf("%w: %d > %d", ErrGasRemainingExceedsGasSupplied, remainingGas, gas) + } if used := gas - remainingGas; used < gas { env.UseGas(used) } From 6a8fb2e85bbf1c2f8fed77d1108784e8ee181bfd Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 09:16:17 +0100 Subject: [PATCH 02/23] fix(libevm/legacy): allow to use all the gas in PrecompiledStatefulContract --- libevm/legacy/legacy.go | 11 ++-- libevm/legacy/legacy_test.go | 100 +++++++++++++++++++++++++++++++++++ libevm/legacy/stubs_test.go | 67 +++++++++++++++++++++++ 3 files changed, 170 insertions(+), 8 deletions(-) create mode 100644 libevm/legacy/legacy_test.go create mode 100644 libevm/legacy/stubs_test.go diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index 792cb773ec04..f44f4a8c4d98 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -1,4 +1,4 @@ -// Copyright 2024 the libevm authors. +// Copyright 2024-2025 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 @@ -19,7 +19,6 @@ package legacy import ( - "errors" "fmt" "github.com/ava-labs/libevm/core/vm" @@ -31,19 +30,15 @@ import ( // 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) -var ( - ErrGasRemainingExceedsGasSupplied = errors.New("remaining gas exceeds supplied gas") -) - // 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, gas) if remainingGas > gas { - return nil, fmt.Errorf("%w: %d > %d", ErrGasRemainingExceedsGasSupplied, remainingGas, gas) + return nil, fmt.Errorf("remaining gas %d exceeds supplied gas %d", remainingGas, gas) } - if used := gas - remainingGas; used < gas { + if used := gas - remainingGas; used <= gas { env.UseGas(used) } return ret, err diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go new file mode 100644 index 000000000000..582ddf3276dd --- /dev/null +++ b/libevm/legacy/legacy_test.go @@ -0,0 +1,100 @@ +// Copyright 2025 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 + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/libevm/core/vm" +) + +func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + envGas uint64 + input []byte + cRet []byte + cRemainingGas uint64 + cErr error + wantRet []byte + wantErr string + wantGasUsed uint64 + }{ + "call_error": { + envGas: 10, + input: []byte{1}, + cRet: []byte{2}, + cRemainingGas: 6, + cErr: errors.New("test error"), + wantRet: []byte{2}, + wantErr: "test error", + wantGasUsed: 4, + }, + "remaining_gas_exceeds_supplied_gas": { + envGas: 10, + input: []byte{1}, + cRet: []byte{2}, + cRemainingGas: 11, + wantErr: "remaining gas 11 exceeds supplied gas 10", + }, + "zero_remaining_gas": { + envGas: 10, + input: []byte{1}, + cRet: []byte{2}, + wantRet: []byte{2}, + wantGasUsed: 10, + }, + "used_one_gas": { + envGas: 10, + input: []byte{1}, + cRet: []byte{2}, + cRemainingGas: 9, + wantRet: []byte{2}, + wantGasUsed: 1, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + env := &stubPrecompileEnvironment{ + gasToReturn: testCase.envGas, + } + c := PrecompiledStatefulContract(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { + return testCase.cRet, testCase.cRemainingGas, testCase.cErr + }) + + upgraded := c.Upgrade() + + ret, err := upgraded(env, testCase.input) + if testCase.wantErr == "" { + require.NoError(t, err) + } else { + require.EqualError(t, err, testCase.wantErr) + } + assert.Equal(t, testCase.wantRet, ret) + assert.Equal(t, testCase.wantGasUsed, env.gasUsed) + }) + } +} diff --git a/libevm/legacy/stubs_test.go b/libevm/legacy/stubs_test.go new file mode 100644 index 000000000000..d9b77d9eb254 --- /dev/null +++ b/libevm/legacy/stubs_test.go @@ -0,0 +1,67 @@ +// Copyright 2025 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 ( + "math/big" + + "github.com/holiman/uint256" + + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/libevm/core/vm" + "github.com/ava-labs/libevm/libevm" + "github.com/ava-labs/libevm/params" +) + +var _ vm.PrecompileEnvironment = (*stubPrecompileEnvironment)(nil) + +// stubPrecompileEnvironment implements [vm.PrecompileEnvironment] for testing. +type stubPrecompileEnvironment struct { + gasToReturn uint64 + gasUsed uint64 +} + +// Gas returns the gas supplied to the precompile. +func (s *stubPrecompileEnvironment) Gas() uint64 { + return s.gasToReturn +} + +// UseGas records the gas used by the precompile. +func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { + s.gasUsed += gas + return true +} + +func (s *stubPrecompileEnvironment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...vm.CallOption) (ret []byte, _ error) { + return nil, nil +} + +func (s *stubPrecompileEnvironment) ChainConfig() *params.ChainConfig { return nil } +func (s *stubPrecompileEnvironment) Rules() params.Rules { return params.Rules{} } +func (s *stubPrecompileEnvironment) StateDB() vm.StateDB { return nil } +func (s *stubPrecompileEnvironment) ReadOnlyState() libevm.StateReader { return nil } +func (s *stubPrecompileEnvironment) IncomingCallType() vm.CallType { return vm.Call } +func (s *stubPrecompileEnvironment) Addresses() *libevm.AddressContext { return nil } +func (s *stubPrecompileEnvironment) ReadOnly() bool { return false } +func (s *stubPrecompileEnvironment) Value() *uint256.Int { return nil } +func (s *stubPrecompileEnvironment) BlockHeader() (h types.Header, err error) { return h, nil } +func (s *stubPrecompileEnvironment) BlockNumber() *big.Int { return nil } +func (s *stubPrecompileEnvironment) BlockTime() uint64 { return 0 } From 40ea8897d962e19728797edd029c4bf6d3984cb6 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Tue, 4 Feb 2025 13:03:50 +0100 Subject: [PATCH 03/23] Add context to assertions Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- libevm/legacy/legacy_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 582ddf3276dd..8883a39c6154 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -93,8 +93,8 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { } else { require.EqualError(t, err, testCase.wantErr) } - assert.Equal(t, testCase.wantRet, ret) - assert.Equal(t, testCase.wantGasUsed, env.gasUsed) + assert.Equal(t, testCase.wantRet, ret, "bytes returned by upgraded contract") + assert.Equalf(t, testCase.wantGasUsed, env.gasUsed, "sum of %T.UseGas() calls", env) }) } } From fabf9d941b1c22d7c5f4b011a38af5b4077e8166 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Tue, 4 Feb 2025 13:05:52 +0100 Subject: [PATCH 04/23] Remove bad copied comment from stubs_test.go Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- libevm/legacy/stubs_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/libevm/legacy/stubs_test.go b/libevm/legacy/stubs_test.go index d9b77d9eb254..0d44bb6e4ea2 100644 --- a/libevm/legacy/stubs_test.go +++ b/libevm/legacy/stubs_test.go @@ -14,9 +14,6 @@ // along with the go-ethereum library. If not, see // . -// Package legacy provides converters between legacy types and their refactored -// equivalents. - package legacy import ( From 7948b4ab13bcead358868ed02ed90597a63a0bb5 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Tue, 4 Feb 2025 13:07:26 +0100 Subject: [PATCH 05/23] Simplify check for used gas Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- 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 f44f4a8c4d98..d40c631a338f 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -38,7 +38,7 @@ func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { if remainingGas > gas { return nil, fmt.Errorf("remaining gas %d exceeds supplied gas %d", remainingGas, gas) } - if used := gas - remainingGas; used <= gas { + if used := gas - remainingGas; used > 0 { env.UseGas(used) } return ret, err From 58f2cf57afd634d5ee480d69707b9bbf88288089 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:12:35 +0100 Subject: [PATCH 06/23] Embed `vm.PrecompileEnvironment` in `stubPrecompileEnvironment` --- libevm/legacy/stubs_test.go | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/libevm/legacy/stubs_test.go b/libevm/legacy/stubs_test.go index 0d44bb6e4ea2..579e32c354cc 100644 --- a/libevm/legacy/stubs_test.go +++ b/libevm/legacy/stubs_test.go @@ -17,21 +17,14 @@ package legacy import ( - "math/big" - - "github.com/holiman/uint256" - - "github.com/ava-labs/libevm/common" - "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/core/vm" - "github.com/ava-labs/libevm/libevm" - "github.com/ava-labs/libevm/params" ) var _ vm.PrecompileEnvironment = (*stubPrecompileEnvironment)(nil) // stubPrecompileEnvironment implements [vm.PrecompileEnvironment] for testing. type stubPrecompileEnvironment struct { + vm.PrecompileEnvironment gasToReturn uint64 gasUsed uint64 } @@ -46,19 +39,3 @@ func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { s.gasUsed += gas return true } - -func (s *stubPrecompileEnvironment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...vm.CallOption) (ret []byte, _ error) { - return nil, nil -} - -func (s *stubPrecompileEnvironment) ChainConfig() *params.ChainConfig { return nil } -func (s *stubPrecompileEnvironment) Rules() params.Rules { return params.Rules{} } -func (s *stubPrecompileEnvironment) StateDB() vm.StateDB { return nil } -func (s *stubPrecompileEnvironment) ReadOnlyState() libevm.StateReader { return nil } -func (s *stubPrecompileEnvironment) IncomingCallType() vm.CallType { return vm.Call } -func (s *stubPrecompileEnvironment) Addresses() *libevm.AddressContext { return nil } -func (s *stubPrecompileEnvironment) ReadOnly() bool { return false } -func (s *stubPrecompileEnvironment) Value() *uint256.Int { return nil } -func (s *stubPrecompileEnvironment) BlockHeader() (h types.Header, err error) { return h, nil } -func (s *stubPrecompileEnvironment) BlockNumber() *big.Int { return nil } -func (s *stubPrecompileEnvironment) BlockTime() uint64 { return 0 } From 5c2be655dc00fc969aed7875b0c25833a670b37c Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:13:19 +0100 Subject: [PATCH 07/23] Move `stubPrecompileEnvironment` to `legacy_test.go` --- libevm/legacy/legacy_test.go | 18 ++++++++++++++++ libevm/legacy/stubs_test.go | 41 ------------------------------------ 2 files changed, 18 insertions(+), 41 deletions(-) delete mode 100644 libevm/legacy/stubs_test.go diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 8883a39c6154..775fde69bf50 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -26,6 +26,24 @@ import ( "github.com/ava-labs/libevm/core/vm" ) +// stubPrecompileEnvironment implements [vm.PrecompileEnvironment] for testing. +type stubPrecompileEnvironment struct { + vm.PrecompileEnvironment + gasToReturn uint64 + gasUsed uint64 +} + +// Gas returns the gas supplied to the precompile. +func (s *stubPrecompileEnvironment) Gas() uint64 { + return s.gasToReturn +} + +// UseGas records the gas used by the precompile. +func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { + s.gasUsed += gas + return true +} + func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { t.Parallel() diff --git a/libevm/legacy/stubs_test.go b/libevm/legacy/stubs_test.go deleted file mode 100644 index 579e32c354cc..000000000000 --- a/libevm/legacy/stubs_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2025 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 - -import ( - "github.com/ava-labs/libevm/core/vm" -) - -var _ vm.PrecompileEnvironment = (*stubPrecompileEnvironment)(nil) - -// stubPrecompileEnvironment implements [vm.PrecompileEnvironment] for testing. -type stubPrecompileEnvironment struct { - vm.PrecompileEnvironment - gasToReturn uint64 - gasUsed uint64 -} - -// Gas returns the gas supplied to the precompile. -func (s *stubPrecompileEnvironment) Gas() uint64 { - return s.gasToReturn -} - -// UseGas records the gas used by the precompile. -func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { - s.gasUsed += gas - return true -} From 1cf655182b715b275d1fefa506cb12ed213ac8dc Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:17:08 +0100 Subject: [PATCH 08/23] Rename `testCase(s)` -> `test(s)` --- libevm/legacy/legacy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 775fde69bf50..28b403a5e6d9 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -47,7 +47,7 @@ func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { t.Parallel() - testCases := map[string]struct { + tests := map[string]struct { envGas uint64 input []byte cRet []byte @@ -91,8 +91,8 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { }, } - for name, testCase := range testCases { - testCase := testCase + for name, test := range tests { + testCase := test t.Run(name, func(t *testing.T) { t.Parallel() From 1ba366bfff05fc8f00f03cf1417da7b1c8eaeb98 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:18:16 +0100 Subject: [PATCH 09/23] Remove `input` field from tests and set it to a constant --- libevm/legacy/legacy_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 28b403a5e6d9..8987e6567147 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -49,7 +49,6 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { tests := map[string]struct { envGas uint64 - input []byte cRet []byte cRemainingGas uint64 cErr error @@ -59,7 +58,6 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { }{ "call_error": { envGas: 10, - input: []byte{1}, cRet: []byte{2}, cRemainingGas: 6, cErr: errors.New("test error"), @@ -69,21 +67,18 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { }, "remaining_gas_exceeds_supplied_gas": { envGas: 10, - input: []byte{1}, cRet: []byte{2}, cRemainingGas: 11, wantErr: "remaining gas 11 exceeds supplied gas 10", }, "zero_remaining_gas": { envGas: 10, - input: []byte{1}, cRet: []byte{2}, wantRet: []byte{2}, wantGasUsed: 10, }, "used_one_gas": { envGas: 10, - input: []byte{1}, cRet: []byte{2}, cRemainingGas: 9, wantRet: []byte{2}, @@ -105,7 +100,9 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { upgraded := c.Upgrade() - ret, err := upgraded(env, testCase.input) + input := []byte("unused") + + ret, err := upgraded(env, input) if testCase.wantErr == "" { require.NoError(t, err) } else { From da2f151e434fd99a2af6828a140ec15f45189fd4 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:18:50 +0100 Subject: [PATCH 10/23] Rename test field `cRet` -> `precompileRet` --- libevm/legacy/legacy_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 8987e6567147..264a506760ec 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -49,7 +49,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { tests := map[string]struct { envGas uint64 - cRet []byte + precompileRet []byte cRemainingGas uint64 cErr error wantRet []byte @@ -58,7 +58,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { }{ "call_error": { envGas: 10, - cRet: []byte{2}, + precompileRet: []byte{2}, cRemainingGas: 6, cErr: errors.New("test error"), wantRet: []byte{2}, @@ -67,19 +67,19 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { }, "remaining_gas_exceeds_supplied_gas": { envGas: 10, - cRet: []byte{2}, + precompileRet: []byte{2}, cRemainingGas: 11, wantErr: "remaining gas 11 exceeds supplied gas 10", }, "zero_remaining_gas": { - envGas: 10, - cRet: []byte{2}, - wantRet: []byte{2}, - wantGasUsed: 10, + envGas: 10, + precompileRet: []byte{2}, + wantRet: []byte{2}, + wantGasUsed: 10, }, "used_one_gas": { envGas: 10, - cRet: []byte{2}, + precompileRet: []byte{2}, cRemainingGas: 9, wantRet: []byte{2}, wantGasUsed: 1, @@ -95,7 +95,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { gasToReturn: testCase.envGas, } c := PrecompiledStatefulContract(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { - return testCase.cRet, testCase.cRemainingGas, testCase.cErr + return testCase.precompileRet, testCase.cRemainingGas, testCase.cErr }) upgraded := c.Upgrade() From c34844f59ef08dc1488f91cbf48d8dd32fe3bb5a Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:19:11 +0100 Subject: [PATCH 11/23] Rename test field `cRemainingGas` -> `remainingGas` --- libevm/legacy/legacy_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 264a506760ec..3e9cea693942 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -50,7 +50,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { tests := map[string]struct { envGas uint64 precompileRet []byte - cRemainingGas uint64 + remainingGas uint64 cErr error wantRet []byte wantErr string @@ -59,7 +59,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { "call_error": { envGas: 10, precompileRet: []byte{2}, - cRemainingGas: 6, + remainingGas: 6, cErr: errors.New("test error"), wantRet: []byte{2}, wantErr: "test error", @@ -68,7 +68,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { "remaining_gas_exceeds_supplied_gas": { envGas: 10, precompileRet: []byte{2}, - cRemainingGas: 11, + remainingGas: 11, wantErr: "remaining gas 11 exceeds supplied gas 10", }, "zero_remaining_gas": { @@ -80,7 +80,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { "used_one_gas": { envGas: 10, precompileRet: []byte{2}, - cRemainingGas: 9, + remainingGas: 9, wantRet: []byte{2}, wantGasUsed: 1, }, @@ -95,7 +95,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { gasToReturn: testCase.envGas, } c := PrecompiledStatefulContract(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { - return testCase.precompileRet, testCase.cRemainingGas, testCase.cErr + return testCase.precompileRet, testCase.remainingGas, testCase.cErr }) upgraded := c.Upgrade() From ef76203cd3fd8ca26334d0d1b42c63c9a5b51c61 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:19:30 +0100 Subject: [PATCH 12/23] Rename test field `cErr` -> `precompileErr` --- libevm/legacy/legacy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 3e9cea693942..5c1d283b3d39 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -51,7 +51,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { envGas uint64 precompileRet []byte remainingGas uint64 - cErr error + precompileErr error wantRet []byte wantErr string wantGasUsed uint64 @@ -60,7 +60,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { envGas: 10, precompileRet: []byte{2}, remainingGas: 6, - cErr: errors.New("test error"), + precompileErr: errors.New("test error"), wantRet: []byte{2}, wantErr: "test error", wantGasUsed: 4, @@ -95,7 +95,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { gasToReturn: testCase.envGas, } c := PrecompiledStatefulContract(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { - return testCase.precompileRet, testCase.remainingGas, testCase.cErr + return testCase.precompileRet, testCase.remainingGas, testCase.precompileErr }) upgraded := c.Upgrade() From a5bcae2fa6b4135a46fd41e38dbcb80c28e31f7c Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:20:26 +0100 Subject: [PATCH 13/23] Move env declaration in subtest closer to where it's used --- libevm/legacy/legacy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 5c1d283b3d39..f19f4d7ee31b 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -91,15 +91,15 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - env := &stubPrecompileEnvironment{ - gasToReturn: testCase.envGas, - } c := PrecompiledStatefulContract(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) { return testCase.precompileRet, testCase.remainingGas, testCase.precompileErr }) upgraded := c.Upgrade() + env := &stubPrecompileEnvironment{ + gasToReturn: testCase.envGas, + } input := []byte("unused") ret, err := upgraded(env, input) From 5ca19fbbd5272ba2aa09a099d76b68b6990d3419 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 13:43:00 +0100 Subject: [PATCH 14/23] Use unexported sentinel error and require.ErrorIs --- libevm/legacy/legacy.go | 7 ++++++- libevm/legacy/legacy_test.go | 16 +++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index d40c631a338f..9e372b1e64d2 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -19,11 +19,16 @@ package legacy import ( + "errors" "fmt" "github.com/ava-labs/libevm/core/vm" ) +var ( + errRemainingGasExceedsSuppliedGas = errors.New("remaining gas exceeds supplied gas") +) + // PrecompiledStatefulContract is the legacy signature of // [vm.PrecompiledStatefulContract], which explicitly accepts and returns gas // values. Instances SHOULD NOT use the [vm.PrecompileEnvironment] @@ -36,7 +41,7 @@ func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { gas := env.Gas() ret, remainingGas, err := c(env, input, gas) if remainingGas > gas { - return nil, fmt.Errorf("remaining gas %d exceeds supplied gas %d", remainingGas, gas) + return nil, fmt.Errorf("%w: %d > %d", errRemainingGasExceedsSuppliedGas, remainingGas, gas) } if used := gas - remainingGas; used > 0 { env.UseGas(used) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index f19f4d7ee31b..fe9d031ad574 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -47,29 +47,31 @@ func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { t.Parallel() + errTest := errors.New("test error") + tests := map[string]struct { envGas uint64 precompileRet []byte remainingGas uint64 precompileErr error wantRet []byte - wantErr string + wantErr error wantGasUsed uint64 }{ "call_error": { envGas: 10, precompileRet: []byte{2}, remainingGas: 6, - precompileErr: errors.New("test error"), + precompileErr: errTest, wantRet: []byte{2}, - wantErr: "test error", + wantErr: errTest, wantGasUsed: 4, }, "remaining_gas_exceeds_supplied_gas": { envGas: 10, precompileRet: []byte{2}, remainingGas: 11, - wantErr: "remaining gas 11 exceeds supplied gas 10", + wantErr: errRemainingGasExceedsSuppliedGas, }, "zero_remaining_gas": { envGas: 10, @@ -103,11 +105,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { input := []byte("unused") ret, err := upgraded(env, input) - if testCase.wantErr == "" { - require.NoError(t, err) - } else { - require.EqualError(t, err, testCase.wantErr) - } + require.ErrorIs(t, err, testCase.wantErr) assert.Equal(t, testCase.wantRet, ret, "bytes returned by upgraded contract") assert.Equalf(t, testCase.wantGasUsed, env.gasUsed, "sum of %T.UseGas() calls", env) }) From 45ee2988051b997db228450b9ab8ce79757d5639 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 4 Feb 2025 16:56:40 +0100 Subject: [PATCH 15/23] Check env.UseGas return value --- libevm/legacy/legacy.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index 9e372b1e64d2..aeb5f7806d7b 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -44,7 +44,9 @@ func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { return nil, fmt.Errorf("%w: %d > %d", errRemainingGasExceedsSuppliedGas, remainingGas, gas) } if used := gas - remainingGas; used > 0 { - env.UseGas(used) + if hasEnoughGas := env.UseGas(used); !hasEnoughGas { + return nil, vm.ErrOutOfGas + } } return ret, err } From c0126a8899a5f5b7395a03a4a081371704a77aa2 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 5 Feb 2025 13:31:00 +0100 Subject: [PATCH 16/23] Always return `ret` --- libevm/legacy/legacy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index aeb5f7806d7b..51d83b14cb4f 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -41,11 +41,11 @@ func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { gas := env.Gas() ret, remainingGas, err := c(env, input, gas) if remainingGas > gas { - return nil, fmt.Errorf("%w: %d > %d", errRemainingGasExceedsSuppliedGas, remainingGas, gas) + return ret, fmt.Errorf("%w: %d > %d", errRemainingGasExceedsSuppliedGas, remainingGas, gas) } if used := gas - remainingGas; used > 0 { if hasEnoughGas := env.UseGas(used); !hasEnoughGas { - return nil, vm.ErrOutOfGas + return ret, vm.ErrOutOfGas } } return ret, err From 6de69da7135267052aa5005b6dcb17dac7a0a227 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 5 Feb 2025 13:32:17 +0100 Subject: [PATCH 17/23] Reduce nesting --- libevm/legacy/legacy.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libevm/legacy/legacy.go b/libevm/legacy/legacy.go index 51d83b14cb4f..d904f7fb6f7c 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -43,10 +43,8 @@ func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { if remainingGas > gas { return ret, fmt.Errorf("%w: %d > %d", errRemainingGasExceedsSuppliedGas, remainingGas, gas) } - if used := gas - remainingGas; used > 0 { - if hasEnoughGas := env.UseGas(used); !hasEnoughGas { - return ret, vm.ErrOutOfGas - } + if used := gas - remainingGas; !env.UseGas(used) { + return ret, vm.ErrOutOfGas } return ret, err } From 3f1a4b5924f97402a7e04f57ead79b523a63a2d1 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 5 Feb 2025 13:33:21 +0100 Subject: [PATCH 18/23] Add remainingGas: 0 in test case `zero_remaining_gas` --- libevm/legacy/legacy_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index fe9d031ad574..7c1252d3d64a 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -74,6 +74,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { wantErr: errRemainingGasExceedsSuppliedGas, }, "zero_remaining_gas": { + remainingGas: 0, envGas: 10, precompileRet: []byte{2}, wantRet: []byte{2}, From 4910f93304f071a1bad3403601de6808ea7bc12a Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 5 Feb 2025 13:34:20 +0100 Subject: [PATCH 19/23] Remove `wantRet` test case field --- libevm/legacy/legacy_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 7c1252d3d64a..af4c52308fbc 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -54,7 +54,6 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { precompileRet []byte remainingGas uint64 precompileErr error - wantRet []byte wantErr error wantGasUsed uint64 }{ @@ -63,7 +62,6 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { precompileRet: []byte{2}, remainingGas: 6, precompileErr: errTest, - wantRet: []byte{2}, wantErr: errTest, wantGasUsed: 4, }, @@ -77,14 +75,12 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { remainingGas: 0, envGas: 10, precompileRet: []byte{2}, - wantRet: []byte{2}, wantGasUsed: 10, }, "used_one_gas": { envGas: 10, precompileRet: []byte{2}, remainingGas: 9, - wantRet: []byte{2}, wantGasUsed: 1, }, } @@ -107,7 +103,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { ret, err := upgraded(env, input) require.ErrorIs(t, err, testCase.wantErr) - assert.Equal(t, testCase.wantRet, ret, "bytes returned by upgraded contract") + assert.Equal(t, testCase.precompileRet, ret, "bytes returned by upgraded contract") assert.Equalf(t, testCase.wantGasUsed, env.gasUsed, "sum of %T.UseGas() calls", env) }) } From 58902bca85ebf137ebada2ce3bed8a4ea04ce5de Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Thu, 6 Feb 2025 13:48:54 +0100 Subject: [PATCH 20/23] Simplify if check using env.UseGas Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- 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 d904f7fb6f7c..c78505a823ae 100644 --- a/libevm/legacy/legacy.go +++ b/libevm/legacy/legacy.go @@ -43,7 +43,7 @@ func (c PrecompiledStatefulContract) Upgrade() vm.PrecompiledStatefulContract { if remainingGas > gas { return ret, fmt.Errorf("%w: %d > %d", errRemainingGasExceedsSuppliedGas, remainingGas, gas) } - if used := gas - remainingGas; !env.UseGas(used) { + if !env.UseGas(gas - remainingGas) { return ret, vm.ErrOutOfGas } return ret, err From b32c8c31931ae04a26e0b094ef08f6cbbdfbee5a Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 6 Feb 2025 13:58:52 +0100 Subject: [PATCH 21/23] Rework gas logic since it was horrendous --- libevm/legacy/legacy_test.go | 37 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index af4c52308fbc..cc154e836f95 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -29,18 +29,18 @@ import ( // stubPrecompileEnvironment implements [vm.PrecompileEnvironment] for testing. type stubPrecompileEnvironment struct { vm.PrecompileEnvironment - gasToReturn uint64 - gasUsed uint64 + gas uint64 } -// Gas returns the gas supplied to the precompile. func (s *stubPrecompileEnvironment) Gas() uint64 { - return s.gasToReturn + return s.gas } -// UseGas records the gas used by the precompile. -func (s *stubPrecompileEnvironment) UseGas(gas uint64) bool { - s.gasUsed += gas +func (s *stubPrecompileEnvironment) UseGas(gas uint64) (hasEnoughGas bool) { + if s.gas < gas { + return false + } + s.gas -= gas return true } @@ -50,38 +50,39 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { errTest := errors.New("test error") tests := map[string]struct { - envGas uint64 + gas uint64 precompileRet []byte remainingGas uint64 precompileErr error wantErr error - wantGasUsed uint64 + wantGas uint64 }{ "call_error": { - envGas: 10, + gas: 10, precompileRet: []byte{2}, remainingGas: 6, precompileErr: errTest, wantErr: errTest, - wantGasUsed: 4, + wantGas: 6, }, "remaining_gas_exceeds_supplied_gas": { - envGas: 10, + gas: 10, precompileRet: []byte{2}, remainingGas: 11, wantErr: errRemainingGasExceedsSuppliedGas, + wantGas: 10, }, "zero_remaining_gas": { remainingGas: 0, - envGas: 10, + gas: 10, precompileRet: []byte{2}, - wantGasUsed: 10, + wantGas: 0, }, "used_one_gas": { - envGas: 10, + gas: 10, precompileRet: []byte{2}, remainingGas: 9, - wantGasUsed: 1, + wantGas: 9, }, } @@ -97,14 +98,14 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { upgraded := c.Upgrade() env := &stubPrecompileEnvironment{ - gasToReturn: testCase.envGas, + gas: testCase.gas, } input := []byte("unused") ret, err := upgraded(env, input) require.ErrorIs(t, err, testCase.wantErr) assert.Equal(t, testCase.precompileRet, ret, "bytes returned by upgraded contract") - assert.Equalf(t, testCase.wantGasUsed, env.gasUsed, "sum of %T.UseGas() calls", env) + assert.Equalf(t, testCase.wantGas, env.gas, "remaining gas in %T", env) }) } } From 2ddb863890e324f8bb83da14809e5a5196b4f51d Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 6 Feb 2025 14:41:31 +0100 Subject: [PATCH 22/23] Rename `gas` -> `suppliedGas` --- libevm/legacy/legacy_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index cc154e836f95..81317d25acce 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -50,7 +50,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { errTest := errors.New("test error") tests := map[string]struct { - gas uint64 + suppliedGas uint64 precompileRet []byte remainingGas uint64 precompileErr error @@ -58,7 +58,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { wantGas uint64 }{ "call_error": { - gas: 10, + suppliedGas: 10, precompileRet: []byte{2}, remainingGas: 6, precompileErr: errTest, @@ -66,7 +66,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { wantGas: 6, }, "remaining_gas_exceeds_supplied_gas": { - gas: 10, + suppliedGas: 10, precompileRet: []byte{2}, remainingGas: 11, wantErr: errRemainingGasExceedsSuppliedGas, @@ -74,12 +74,12 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { }, "zero_remaining_gas": { remainingGas: 0, - gas: 10, + suppliedGas: 10, precompileRet: []byte{2}, wantGas: 0, }, "used_one_gas": { - gas: 10, + suppliedGas: 10, precompileRet: []byte{2}, remainingGas: 9, wantGas: 9, @@ -98,7 +98,7 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { upgraded := c.Upgrade() env := &stubPrecompileEnvironment{ - gas: testCase.gas, + gas: testCase.suppliedGas, } input := []byte("unused") From 0d1892bbc744acace077a2e78712775f2944f91c Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 6 Feb 2025 14:42:42 +0100 Subject: [PATCH 23/23] Fix the horrendous order of fields in zero_remaining_gas test case --- libevm/legacy/legacy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libevm/legacy/legacy_test.go b/libevm/legacy/legacy_test.go index 81317d25acce..afd3df03da04 100644 --- a/libevm/legacy/legacy_test.go +++ b/libevm/legacy/legacy_test.go @@ -73,9 +73,9 @@ func TestPrecompiledStatefulContract_Upgrade(t *testing.T) { wantGas: 10, }, "zero_remaining_gas": { - remainingGas: 0, suppliedGas: 10, precompileRet: []byte{2}, + remainingGas: 0, wantGas: 0, }, "used_one_gas": {