From 83002eaac93a38f5c82526b4cc2fba33d045a481 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 13 Sep 2024 21:16:43 +0100 Subject: [PATCH 01/16] feat: draft `vm.JumpTable` override --- core/vm/interpreter.go | 3 ++ core/vm/jump_table.libevm.go | 32 +++++++++++++++ core/vm/jump_table.libevm_test.go | 66 +++++++++++++++++++++++++++++++ core/vm/jump_table_export.go | 7 +++- libevm/hookstest/stub.go | 5 +++ params/hooks.libevm.go | 5 +++ 6 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 core/vm/jump_table.libevm.go create mode 100644 core/vm/jump_table.libevm_test.go diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 1968289f4eaa..0d03c7f75467 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -94,6 +94,9 @@ func NewEVMInterpreter(evm *EVM) *EVMInterpreter { extraEips = append(extraEips, eip) } } + if evm.chainRules.Hooks().OverrideJumpTable() { + table = libevmHooks.OverrideJumpTable(evm.chainRules, table) + } evm.Config.ExtraEips = extraEips return &EVMInterpreter{evm: evm, table: table} } diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go new file mode 100644 index 000000000000..a2223283a7b1 --- /dev/null +++ b/core/vm/jump_table.libevm.go @@ -0,0 +1,32 @@ +package vm + +import "github.com/ethereum/go-ethereum/params" + +func NewOperation( + execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error), + constantGas uint64, + dynamicGas func(e *EVM, c *Contract, s *Stack, m *Memory, u uint64) (uint64, error), + minStack, maxStack int, + memorySize func(s *Stack) (size uint64, overflow bool), +) *operation { + return &operation{ + execute, + constantGas, + dynamicGas, + minStack, maxStack, + memorySize, + } +} + +type Hooks interface { + OverrideJumpTable(params.Rules, *JumpTable) *JumpTable +} + +var libevmHooks Hooks + +func RegisterHooks(h Hooks) { + if libevmHooks != nil { + panic("already registered") + } + libevmHooks = h +} diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go new file mode 100644 index 000000000000..2b31e11a29df --- /dev/null +++ b/core/vm/jump_table.libevm_test.go @@ -0,0 +1,66 @@ +package vm_test + +import ( + "fmt" + "testing" + + "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/libevm/ethtest" + "github.com/ethereum/go-ethereum/libevm/hookstest" + "github.com/ethereum/go-ethereum/params" + "github.com/holiman/uint256" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type vmHooksStub struct { + replacement *vm.JumpTable +} + +func (s *vmHooksStub) OverrideJumpTable(_ params.Rules, jt *vm.JumpTable) *vm.JumpTable { + for op, instr := range s.replacement { + if instr != nil { + fmt.Println(op, instr) + jt[op] = instr + } + } + return jt +} + +func TestOverrideJumpTable(t *testing.T) { + hooks := &hookstest.Stub{ + OverrideJumpTableFlag: true, + } + hooks.Register(t) + + var called bool + const opcode = 1 + + vmHooks := &vmHooksStub{ + replacement: &vm.JumpTable{ + opcode: vm.NewOperation( + func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) { + called = true + return nil, nil + }, + 10, nil, + 0, 0, + func(s *vm.Stack) (size uint64, overflow bool) { + return 0, false + }, + ), + }, + } + vm.RegisterHooks(vmHooks) + + state, evm := ethtest.NewZeroEVM(t) + + rng := ethtest.NewPseudoRand(142857) + contract := rng.Address() + state.CreateAccount(contract) + state.SetCode(contract, []byte{opcode}) + + _, _, err := evm.Call(vm.AccountRef(rng.Address()), contract, []byte{}, 1e6, uint256.NewInt(0)) + require.NoError(t, err) + assert.True(t, called) +} diff --git a/core/vm/jump_table_export.go b/core/vm/jump_table_export.go index b74109da0ad1..50a7b7b67389 100644 --- a/core/vm/jump_table_export.go +++ b/core/vm/jump_table_export.go @@ -24,7 +24,12 @@ import ( // LookupInstructionSet returns the instruction set for the fork configured by // the rules. -func LookupInstructionSet(rules params.Rules) (JumpTable, error) { +func LookupInstructionSet(rules params.Rules) (jt JumpTable, err error) { + defer func() { + if err == nil && rules.Hooks().OverrideJumpTable() { + jt = *libevmHooks.OverrideJumpTable(rules, &jt) + } + }() switch { case rules.IsVerkle: return newCancunInstructionSet(), errors.New("verkle-fork not defined yet") diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 719a16664f9f..e6e5ff661fe1 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -28,6 +28,7 @@ type Stub struct { PrecompileOverrides map[common.Address]libevm.PrecompiledContract CanExecuteTransactionFn func(common.Address, *common.Address, libevm.StateReader) error CanCreateContractFn func(*libevm.AddressContext, libevm.StateReader) error + OverrideJumpTableFlag bool } // Register is a convenience wrapper for registering s as both the @@ -70,6 +71,10 @@ func (s Stub) CanCreateContract(cc *libevm.AddressContext, sr libevm.StateReader return nil } +func (s Stub) OverrideJumpTable() bool { + return s.OverrideJumpTableFlag +} + var _ interface { params.ChainConfigHooks params.RulesHooks diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index 74fd88ccef53..ec8ca6c2cbbe 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -27,6 +27,7 @@ type RulesHooks interface { // [PrecompiledContract] is non-nil. If it returns `false` then the default // precompile behaviour is honoured. PrecompileOverride(common.Address) (_ libevm.PrecompiledContract, override bool) + OverrideJumpTable() bool } // RulesAllowlistHooks are a subset of [RulesHooks] that gate actions, signalled @@ -81,3 +82,7 @@ func (NOOPHooks) CanCreateContract(*libevm.AddressContext, libevm.StateReader) e func (NOOPHooks) PrecompileOverride(common.Address) (libevm.PrecompiledContract, bool) { return nil, false } + +func (NOOPHooks) OverrideJumpTable() bool { + return false +} From 482edd9306dc2969713af312c00629893aee2161 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 16:17:03 -0400 Subject: [PATCH 02/16] doc: comments on all exported identifiers --- core/vm/jump_table.libevm.go | 7 +++++++ params/hooks.libevm.go | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index a2223283a7b1..1a27b3b294b7 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -2,6 +2,7 @@ package vm import "github.com/ethereum/go-ethereum/params" +// NewOperation constructs a new operation for inclusion in a [JumpTable]. func NewOperation( execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error), constantGas uint64, @@ -18,12 +19,18 @@ func NewOperation( } } +// Hooks are arbitrary configuration functions to modify default VM behaviour. type Hooks interface { + // OverrideJumpTable will only be called if + // [params.RulesHooks.OverrideJumpTable] returns true. This allows for + // recursive calling into [LookupInstructionSet]. OverrideJumpTable(params.Rules, *JumpTable) *JumpTable } var libevmHooks Hooks +// RegisterHooks registers the Hooks. It is expected to be called in an `init()` +// function and MUST NOT be called more than once. func RegisterHooks(h Hooks) { if libevmHooks != nil { panic("already registered") diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index fd9f9df5fb6d..3cb9d95503f1 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -27,6 +27,9 @@ type RulesHooks interface { // [PrecompiledContract] is non-nil. If it returns `false` then the default // precompile behaviour is honoured. PrecompileOverride(common.Address) (_ libevm.PrecompiledContract, override bool) + // OverrideJumpTable signals that [vm.Hooks.OverrideJumpTable] MUST be used. + // Toggling the behaviour via [Rules] allows for recursive calling into + // functions that would otherwise infinitely call the override hook. OverrideJumpTable() bool } @@ -86,6 +89,8 @@ func (NOOPHooks) PrecompileOverride(common.Address) (libevm.PrecompiledContract, return nil, false } +// OverrideJumpTable returns false, leaving all JumpTables unmodified even if +// [vm.Hooks] have been registered. func (NOOPHooks) OverrideJumpTable() bool { return false } From ce10b13af22cd15725bc169236c4d12a5659b919 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 16:28:14 -0400 Subject: [PATCH 03/16] test: improved strategy and coverage --- core/vm/interpreter.go | 4 +-- core/vm/jump_table.libevm.go | 13 +++++++ core/vm/jump_table.libevm_test.go | 60 ++++++++++++++++++++++++------- core/vm/jump_table_export.go | 4 +-- libevm/hookstest/stub.go | 9 +++-- 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 0d03c7f75467..06df4270df5c 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -94,10 +94,8 @@ func NewEVMInterpreter(evm *EVM) *EVMInterpreter { extraEips = append(extraEips, eip) } } - if evm.chainRules.Hooks().OverrideJumpTable() { - table = libevmHooks.OverrideJumpTable(evm.chainRules, table) - } evm.Config.ExtraEips = extraEips + table = overrideJumpTable(evm.chainRules, table) return &EVMInterpreter{evm: evm, table: table} } diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 1a27b3b294b7..793fb3395883 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -2,6 +2,19 @@ package vm import "github.com/ethereum/go-ethereum/params" +// overrideJumpTable returns `libevmHooks.OverrideJumpTable(r,jt)“ i.f.f. the +// Rules' hooks indicate that it must, otherwise it echoes `jt` unchanged. +func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { + if !r.Hooks().OverrideJumpTable() { + return jt + } + // We don't check that libevmHooks is non-nil because the user has clearly + // signified that they want an override. A nil-pointer dereference will + // occur in tests whereas graceful degradation would frustrate the user with + // a hard-to-find bug. + return libevmHooks.OverrideJumpTable(r, jt) +} + // NewOperation constructs a new operation for inclusion in a [JumpTable]. func NewOperation( execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error), diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index 2b31e11a29df..6fa472970a10 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -2,6 +2,7 @@ package vm_test import ( "fmt" + "math/big" "testing" "github.com/ethereum/go-ethereum/core/vm" @@ -15,9 +16,14 @@ import ( type vmHooksStub struct { replacement *vm.JumpTable + overridden bool } +var _ vm.Hooks = (*vmHooksStub)(nil) + +// OverrideJumpTable overrides all non-nil operations from s.replacement . func (s *vmHooksStub) OverrideJumpTable(_ params.Rules, jt *vm.JumpTable) *vm.JumpTable { + s.overridden = true for op, instr := range s.replacement { if instr != nil { fmt.Println(op, instr) @@ -28,22 +34,30 @@ func (s *vmHooksStub) OverrideJumpTable(_ params.Rules, jt *vm.JumpTable) *vm.Ju } func TestOverrideJumpTable(t *testing.T) { + override := new(bool) hooks := &hookstest.Stub{ - OverrideJumpTableFlag: true, + OverrideJumpTableFn: func() bool { + return *override + }, } hooks.Register(t) - var called bool - const opcode = 1 + const ( + opcode = 1 + gasLimit uint64 = 1e6 + ) + rng := ethtest.NewPseudoRand(142857) + gasCost := 1 + rng.Uint64n(gasLimit) + executed := false vmHooks := &vmHooksStub{ replacement: &vm.JumpTable{ opcode: vm.NewOperation( func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) { - called = true + executed = true return nil, nil }, - 10, nil, + gasCost, nil, 0, 0, func(s *vm.Stack) (size uint64, overflow bool) { return 0, false @@ -53,14 +67,34 @@ func TestOverrideJumpTable(t *testing.T) { } vm.RegisterHooks(vmHooks) - state, evm := ethtest.NewZeroEVM(t) + t.Run("LookupInstructionSet", func(t *testing.T) { + _, evm := ethtest.NewZeroEVM(t) + rules := evm.ChainConfig().Rules(big.NewInt(0), false, 0) - rng := ethtest.NewPseudoRand(142857) - contract := rng.Address() - state.CreateAccount(contract) - state.SetCode(contract, []byte{opcode}) + for _, b := range []bool{false, true} { + vmHooks.overridden = false + + *override = b + _, err := vm.LookupInstructionSet(rules) + require.NoError(t, err) + require.Equal(t, b, vmHooks.overridden, "vm.Hooks.OverrideJumpTable() called i.f.f. params.RulesHooks.OverrideJumpTable() returns true") + } + }) + + t.Run("EVMInterpreter", func(t *testing.T) { + // We don't need to test the non-override case in EVMInterpreter because + // that uses code shared with LookupInstructionSet. Here we only care + // that the op gets executed as expected. + *override = true + state, evm := ethtest.NewZeroEVM(t) + + contract := rng.Address() + state.CreateAccount(contract) + state.SetCode(contract, []byte{opcode}) - _, _, err := evm.Call(vm.AccountRef(rng.Address()), contract, []byte{}, 1e6, uint256.NewInt(0)) - require.NoError(t, err) - assert.True(t, called) + _, gasRemaining, err := evm.Call(vm.AccountRef(rng.Address()), contract, []byte{}, gasLimit, uint256.NewInt(0)) + require.NoError(t, err, "evm.Call([contract with overridden opcode])") + assert.True(t, executed, "executionFunc was called") + assert.Equal(t, gasLimit-gasCost, gasRemaining, "gas remaining") + }) } diff --git a/core/vm/jump_table_export.go b/core/vm/jump_table_export.go index 50a7b7b67389..7aa791cfc9c6 100644 --- a/core/vm/jump_table_export.go +++ b/core/vm/jump_table_export.go @@ -26,8 +26,8 @@ import ( // the rules. func LookupInstructionSet(rules params.Rules) (jt JumpTable, err error) { defer func() { - if err == nil && rules.Hooks().OverrideJumpTable() { - jt = *libevmHooks.OverrideJumpTable(rules, &jt) + if err == nil { // NOTE `err ==` NOT != + jt = *overrideJumpTable(rules, &jt) } }() switch { diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 59ddf5755620..7de3d280d5d8 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -28,7 +28,7 @@ type Stub struct { PrecompileOverrides map[common.Address]libevm.PrecompiledContract CanExecuteTransactionFn func(common.Address, *common.Address, libevm.StateReader) error CanCreateContractFn func(*libevm.AddressContext, uint64, libevm.StateReader) (uint64, error) - OverrideJumpTableFlag bool + OverrideJumpTableFn func() bool } // Register is a convenience wrapper for registering s as both the @@ -71,8 +71,13 @@ func (s Stub) CanCreateContract(cc *libevm.AddressContext, gas uint64, sr libevm return gas, nil } +// OverrideJumpTable proxies arguments to the s.OverrideJumpTableFn function if +// non-nil, otherwise it acts as a noop. func (s Stub) OverrideJumpTable() bool { - return s.OverrideJumpTableFlag + if f := s.OverrideJumpTableFn; f != nil { + return f() + } + return false } var _ interface { From 55ef2a4b6891f76155b80f38405f55bf455a49d2 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 16:34:30 -0400 Subject: [PATCH 04/16] chore: `gci` --- core/vm/jump_table.libevm_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index 6fa472970a10..42e92a950844 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -5,13 +5,14 @@ import ( "math/big" "testing" + "github.com/holiman/uint256" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/libevm/ethtest" "github.com/ethereum/go-ethereum/libevm/hookstest" "github.com/ethereum/go-ethereum/params" - "github.com/holiman/uint256" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) type vmHooksStub struct { From 6a5de7537fc085bbb6965c94f2dc62bc04e19e69 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 08:33:37 -0400 Subject: [PATCH 05/16] refactor: replace `NewOperation` with `OperationBuilder` --- core/vm/jump_table.libevm.go | 49 ++++++++++++++++++++++--------- core/vm/jump_table.libevm_test.go | 22 +++++++++----- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 793fb3395883..22082ff05d0e 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -15,21 +15,42 @@ func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { return libevmHooks.OverrideJumpTable(r, jt) } -// NewOperation constructs a new operation for inclusion in a [JumpTable]. -func NewOperation( - execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error), - constantGas uint64, - dynamicGas func(e *EVM, c *Contract, s *Stack, m *Memory, u uint64) (uint64, error), - minStack, maxStack int, - memorySize func(s *Stack) (size uint64, overflow bool), -) *operation { - return &operation{ - execute, - constantGas, - dynamicGas, - minStack, maxStack, - memorySize, +// An OperationBuilder is a factory for a new operations to include in a +// [JumpTable]. All of its fields are required. +type OperationBuilder[G interface { + uint64 | func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error) +}] struct { + Execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error) + Gas G + MinStack, MaxStack int + MemorySize func(s *Stack) (size uint64, overflow bool) +} + +type ( + // OperationBuilderConstantGas is the constant-gas version of an + // OperationBuilder. + OperationBuilderConstantGas = OperationBuilder[uint64] + // OperationBuilderDynamicGas is the dynamic-gas version of an + // OperationBuilder. + OperationBuilderDynamicGas = OperationBuilder[func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error)] +) + +// Build constructs the operation. +func (b OperationBuilder[G]) Build() *operation { + o := &operation{ + execute: b.Execute, + minStack: b.MinStack, + maxStack: b.MaxStack, + memorySize: b.MemorySize, + } + + switch g := any(b.Gas).(type) { + case uint64: + o.constantGas = g + case gasFunc: + o.dynamicGas = g } + return o } // Hooks are arbitrary configuration functions to modify default VM behaviour. diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index 42e92a950844..602fc0545d79 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -3,6 +3,7 @@ package vm_test import ( "fmt" "math/big" + "reflect" "testing" "github.com/holiman/uint256" @@ -53,24 +54,22 @@ func TestOverrideJumpTable(t *testing.T) { vmHooks := &vmHooksStub{ replacement: &vm.JumpTable{ - opcode: vm.NewOperation( - func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) { + opcode: vm.OperationBuilderConstantGas{ + Execute: func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) { executed = true return nil, nil }, - gasCost, nil, - 0, 0, - func(s *vm.Stack) (size uint64, overflow bool) { + Gas: gasCost, + MemorySize: func(s *vm.Stack) (size uint64, overflow bool) { return 0, false }, - ), + }.Build(), }, } vm.RegisterHooks(vmHooks) t.Run("LookupInstructionSet", func(t *testing.T) { - _, evm := ethtest.NewZeroEVM(t) - rules := evm.ChainConfig().Rules(big.NewInt(0), false, 0) + rules := new(params.ChainConfig).Rules(big.NewInt(0), false, 0) for _, b := range []bool{false, true} { vmHooks.overridden = false @@ -99,3 +98,10 @@ func TestOverrideJumpTable(t *testing.T) { assert.Equal(t, gasLimit-gasCost, gasRemaining, "gas remaining") }) } + +func TestOperationFieldCount(t *testing.T) { + // The libevm OperationBuilder assumes that the 6 struct fields are the only + // ones. + op := vm.OperationBuilderConstantGas{}.Build() + require.Equalf(t, 6, reflect.TypeOf(*op).NumField(), "number of fields in %T struct", *op) +} From 7d054b9896b201fd0658b4e3087fba591a55bb1f Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 09:46:01 -0400 Subject: [PATCH 06/16] refactor: allow constant and dynamic gas simultaneously --- core/vm/jump_table.libevm.go | 37 +++++++++---------------------- core/vm/jump_table.libevm_test.go | 6 ++--- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 22082ff05d0e..571587bccfa8 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -16,39 +16,24 @@ func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { } // An OperationBuilder is a factory for a new operations to include in a -// [JumpTable]. All of its fields are required. -type OperationBuilder[G interface { - uint64 | func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error) -}] struct { +// [JumpTable]. +type OperationBuilder struct { Execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error) - Gas G + ConstantGas uint64 + DynamicGas func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error) MinStack, MaxStack int MemorySize func(s *Stack) (size uint64, overflow bool) } -type ( - // OperationBuilderConstantGas is the constant-gas version of an - // OperationBuilder. - OperationBuilderConstantGas = OperationBuilder[uint64] - // OperationBuilderDynamicGas is the dynamic-gas version of an - // OperationBuilder. - OperationBuilderDynamicGas = OperationBuilder[func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error)] -) - // Build constructs the operation. -func (b OperationBuilder[G]) Build() *operation { +func (b OperationBuilder) Build() *operation { o := &operation{ - execute: b.Execute, - minStack: b.MinStack, - maxStack: b.MaxStack, - memorySize: b.MemorySize, - } - - switch g := any(b.Gas).(type) { - case uint64: - o.constantGas = g - case gasFunc: - o.dynamicGas = g + execute: b.Execute, + constantGas: b.ConstantGas, + dynamicGas: b.DynamicGas, + minStack: b.MinStack, + maxStack: b.MaxStack, + memorySize: b.MemorySize, } return o } diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index 602fc0545d79..878c4a030d2e 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -54,12 +54,12 @@ func TestOverrideJumpTable(t *testing.T) { vmHooks := &vmHooksStub{ replacement: &vm.JumpTable{ - opcode: vm.OperationBuilderConstantGas{ + opcode: vm.OperationBuilder{ Execute: func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) { executed = true return nil, nil }, - Gas: gasCost, + ConstantGas: gasCost, MemorySize: func(s *vm.Stack) (size uint64, overflow bool) { return 0, false }, @@ -102,6 +102,6 @@ func TestOverrideJumpTable(t *testing.T) { func TestOperationFieldCount(t *testing.T) { // The libevm OperationBuilder assumes that the 6 struct fields are the only // ones. - op := vm.OperationBuilderConstantGas{}.Build() + op := vm.OperationBuilder{}.Build() require.Equalf(t, 6, reflect.TypeOf(*op).NumField(), "number of fields in %T struct", *op) } From 58ded5b2e4f07df2b676be10717d684644af4c95 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 09:56:07 -0400 Subject: [PATCH 07/16] feat: `vm.OperationEnvironment` for custom instructions --- core/vm/jump_table.libevm.go | 25 +++++++++++++++++++++++-- core/vm/jump_table.libevm_test.go | 22 ++++++++++++++++------ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 571587bccfa8..24d94b8cceeb 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -18,7 +18,7 @@ func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { // An OperationBuilder is a factory for a new operations to include in a // [JumpTable]. type OperationBuilder struct { - Execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error) + Execute OperationFunc ConstantGas uint64 DynamicGas func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error) MinStack, MaxStack int @@ -28,7 +28,7 @@ type OperationBuilder struct { // Build constructs the operation. func (b OperationBuilder) Build() *operation { o := &operation{ - execute: b.Execute, + execute: b.Execute.internal(), constantGas: b.ConstantGas, dynamicGas: b.DynamicGas, minStack: b.MinStack, @@ -38,6 +38,27 @@ func (b OperationBuilder) Build() *operation { return o } +// An OperationFunc is the execution function of a custom instruction. +type OperationFunc func(_ *OperationEnvironment, pc *uint64, _ *EVMInterpreter, _ *ScopeContext) ([]byte, error) + +// An OperationEnvironment provides information about the context in which a +// custom instruction is being executed. +type OperationEnvironment struct { + StateDB StateDB +} + +// internal converts an exported [OperationFunc] into an un-exported +// [executionFunc] as required to build an [operation]. +func (fn OperationFunc) internal() executionFunc { + return func(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { + return fn( + &OperationEnvironment{ + StateDB: interpreter.evm.StateDB, + }, pc, interpreter, scope, + ) + } +} + // Hooks are arbitrary configuration functions to modify default VM behaviour. type Hooks interface { // OverrideJumpTable will only be called if diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index 878c4a030d2e..d2147fbe2b04 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/libevm/ethtest" "github.com/ethereum/go-ethereum/libevm/hookstest" @@ -35,6 +36,16 @@ func (s *vmHooksStub) OverrideJumpTable(_ params.Rules, jt *vm.JumpTable) *vm.Ju return jt } +// An opRecorder is an instruction that records its inputs. +type opRecorder struct { + stateVal common.Hash +} + +func (op *opRecorder) execute(env *vm.OperationEnvironment, pc *uint64, interpreter *vm.EVMInterpreter, scope *vm.ScopeContext) ([]byte, error) { + op.stateVal = env.StateDB.GetState(scope.Contract.Address(), common.Hash{}) + return nil, nil +} + func TestOverrideJumpTable(t *testing.T) { override := new(bool) hooks := &hookstest.Stub{ @@ -50,15 +61,12 @@ func TestOverrideJumpTable(t *testing.T) { ) rng := ethtest.NewPseudoRand(142857) gasCost := 1 + rng.Uint64n(gasLimit) - executed := false + spy := &opRecorder{} vmHooks := &vmHooksStub{ replacement: &vm.JumpTable{ opcode: vm.OperationBuilder{ - Execute: func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) { - executed = true - return nil, nil - }, + Execute: spy.execute, ConstantGas: gasCost, MemorySize: func(s *vm.Stack) (size uint64, overflow bool) { return 0, false @@ -91,11 +99,13 @@ func TestOverrideJumpTable(t *testing.T) { contract := rng.Address() state.CreateAccount(contract) state.SetCode(contract, []byte{opcode}) + value := rng.Hash() + state.SetState(contract, common.Hash{}, value) _, gasRemaining, err := evm.Call(vm.AccountRef(rng.Address()), contract, []byte{}, gasLimit, uint256.NewInt(0)) require.NoError(t, err, "evm.Call([contract with overridden opcode])") - assert.True(t, executed, "executionFunc was called") assert.Equal(t, gasLimit-gasCost, gasRemaining, "gas remaining") + assert.Equal(t, spy.stateVal, value, "StateDB propagated") }) } From a0d8ef9691abf27333101403ffff8ec046faad16 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 10:08:30 -0400 Subject: [PATCH 08/16] refactor: move hooks logic to its own file --- core/vm/hooks.libevm.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 core/vm/hooks.libevm.go diff --git a/core/vm/hooks.libevm.go b/core/vm/hooks.libevm.go new file mode 100644 index 000000000000..de6268a4b358 --- /dev/null +++ b/core/vm/hooks.libevm.go @@ -0,0 +1,35 @@ +package vm + +import "github.com/ethereum/go-ethereum/params" + +// RegisterHooks registers the Hooks. It is expected to be called in an `init()` +// function and MUST NOT be called more than once. +func RegisterHooks(h Hooks) { + if libevmHooks != nil { + panic("already registered") + } + libevmHooks = h +} + +var libevmHooks Hooks + +// Hooks are arbitrary configuration functions to modify default VM behaviour. +type Hooks interface { + // OverrideJumpTable will only be called if + // [params.RulesHooks.OverrideJumpTable] returns true. This allows for + // recursive calling into [LookupInstructionSet]. + OverrideJumpTable(params.Rules, *JumpTable) *JumpTable +} + +// overrideJumpTable returns `libevmHooks.OverrideJumpTable(r,jt)“ i.f.f. the +// Rules' hooks indicate that it must, otherwise it echoes `jt` unchanged. +func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { + if !r.Hooks().OverrideJumpTable() { + return jt + } + // We don't check that libevmHooks is non-nil because the user has clearly + // signified that they want an override. A nil-pointer dereference will + // occur in tests whereas graceful degradation would frustrate the user with + // a hard-to-find bug. + return libevmHooks.OverrideJumpTable(r, jt) +} From 263a9cbda40782cda9cb2e4797d160a6c3e7dfbf Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 10:09:33 -0400 Subject: [PATCH 09/16] chore: include changes forgotten in last commit --- core/vm/jump_table.libevm.go | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 24d94b8cceeb..78a63423107e 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -1,20 +1,5 @@ package vm -import "github.com/ethereum/go-ethereum/params" - -// overrideJumpTable returns `libevmHooks.OverrideJumpTable(r,jt)“ i.f.f. the -// Rules' hooks indicate that it must, otherwise it echoes `jt` unchanged. -func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { - if !r.Hooks().OverrideJumpTable() { - return jt - } - // We don't check that libevmHooks is non-nil because the user has clearly - // signified that they want an override. A nil-pointer dereference will - // occur in tests whereas graceful degradation would frustrate the user with - // a hard-to-find bug. - return libevmHooks.OverrideJumpTable(r, jt) -} - // An OperationBuilder is a factory for a new operations to include in a // [JumpTable]. type OperationBuilder struct { @@ -58,22 +43,3 @@ func (fn OperationFunc) internal() executionFunc { ) } } - -// Hooks are arbitrary configuration functions to modify default VM behaviour. -type Hooks interface { - // OverrideJumpTable will only be called if - // [params.RulesHooks.OverrideJumpTable] returns true. This allows for - // recursive calling into [LookupInstructionSet]. - OverrideJumpTable(params.Rules, *JumpTable) *JumpTable -} - -var libevmHooks Hooks - -// RegisterHooks registers the Hooks. It is expected to be called in an `init()` -// function and MUST NOT be called more than once. -func RegisterHooks(h Hooks) { - if libevmHooks != nil { - panic("already registered") - } - libevmHooks = h -} From 792eb29d0a48cd3d7c627140516895e73a6bc9b0 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 10:15:50 -0400 Subject: [PATCH 10/16] feat: `OperationEnvironment` read-only state access --- core/vm/jump_table.libevm.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 78a63423107e..91ab6370fd29 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -1,5 +1,7 @@ package vm +import "github.com/ethereum/go-ethereum/libevm" + // An OperationBuilder is a factory for a new operations to include in a // [JumpTable]. type OperationBuilder struct { @@ -29,17 +31,24 @@ type OperationFunc func(_ *OperationEnvironment, pc *uint64, _ *EVMInterpreter, // An OperationEnvironment provides information about the context in which a // custom instruction is being executed. type OperationEnvironment struct { + ReadOnly bool + // StateDB will be non-nil i.f.f !ReadOnly. StateDB StateDB + // ReadOnlyState will always be non-nil. + ReadOnlyState libevm.StateReader } // internal converts an exported [OperationFunc] into an un-exported // [executionFunc] as required to build an [operation]. func (fn OperationFunc) internal() executionFunc { return func(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { - return fn( - &OperationEnvironment{ - StateDB: interpreter.evm.StateDB, - }, pc, interpreter, scope, - ) + env := &OperationEnvironment{ + ReadOnly: interpreter.readOnly, + ReadOnlyState: interpreter.evm.StateDB, + } + if !env.ReadOnly { + env.StateDB = interpreter.evm.StateDB + } + return fn(env, pc, interpreter, scope) } } From 0473ed8f5af44251ed6e588f5357db35bdae2b1e Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 19 Sep 2024 16:00:38 -0400 Subject: [PATCH 11/16] chore: remove unnecessary `RulesHooks.OverrideJumpTable() bool` #yagni --- core/vm/hooks.libevm.go | 6 +--- core/vm/jump_table.libevm_test.go | 49 +++++++------------------------ libevm/hookstest/stub.go | 10 ------- params/hooks.libevm.go | 10 ------- 4 files changed, 11 insertions(+), 64 deletions(-) diff --git a/core/vm/hooks.libevm.go b/core/vm/hooks.libevm.go index de6268a4b358..30315fc798ad 100644 --- a/core/vm/hooks.libevm.go +++ b/core/vm/hooks.libevm.go @@ -24,12 +24,8 @@ type Hooks interface { // overrideJumpTable returns `libevmHooks.OverrideJumpTable(r,jt)“ i.f.f. the // Rules' hooks indicate that it must, otherwise it echoes `jt` unchanged. func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { - if !r.Hooks().OverrideJumpTable() { + if libevmHooks == nil { return jt } - // We don't check that libevmHooks is non-nil because the user has clearly - // signified that they want an override. A nil-pointer dereference will - // occur in tests whereas graceful degradation would frustrate the user with - // a hard-to-find bug. return libevmHooks.OverrideJumpTable(r, jt) } diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index d2147fbe2b04..7a82b5edc3b7 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -2,7 +2,6 @@ package vm_test import ( "fmt" - "math/big" "reflect" "testing" @@ -13,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/libevm/ethtest" - "github.com/ethereum/go-ethereum/libevm/hookstest" "github.com/ethereum/go-ethereum/params" ) @@ -47,14 +45,6 @@ func (op *opRecorder) execute(env *vm.OperationEnvironment, pc *uint64, interpre } func TestOverrideJumpTable(t *testing.T) { - override := new(bool) - hooks := &hookstest.Stub{ - OverrideJumpTableFn: func() bool { - return *override - }, - } - hooks.Register(t) - const ( opcode = 1 gasLimit uint64 = 1e6 @@ -76,37 +66,18 @@ func TestOverrideJumpTable(t *testing.T) { } vm.RegisterHooks(vmHooks) - t.Run("LookupInstructionSet", func(t *testing.T) { - rules := new(params.ChainConfig).Rules(big.NewInt(0), false, 0) - - for _, b := range []bool{false, true} { - vmHooks.overridden = false - - *override = b - _, err := vm.LookupInstructionSet(rules) - require.NoError(t, err) - require.Equal(t, b, vmHooks.overridden, "vm.Hooks.OverrideJumpTable() called i.f.f. params.RulesHooks.OverrideJumpTable() returns true") - } - }) - - t.Run("EVMInterpreter", func(t *testing.T) { - // We don't need to test the non-override case in EVMInterpreter because - // that uses code shared with LookupInstructionSet. Here we only care - // that the op gets executed as expected. - *override = true - state, evm := ethtest.NewZeroEVM(t) + state, evm := ethtest.NewZeroEVM(t) - contract := rng.Address() - state.CreateAccount(contract) - state.SetCode(contract, []byte{opcode}) - value := rng.Hash() - state.SetState(contract, common.Hash{}, value) + contract := rng.Address() + state.CreateAccount(contract) + state.SetCode(contract, []byte{opcode}) + value := rng.Hash() + state.SetState(contract, common.Hash{}, value) - _, gasRemaining, err := evm.Call(vm.AccountRef(rng.Address()), contract, []byte{}, gasLimit, uint256.NewInt(0)) - require.NoError(t, err, "evm.Call([contract with overridden opcode])") - assert.Equal(t, gasLimit-gasCost, gasRemaining, "gas remaining") - assert.Equal(t, spy.stateVal, value, "StateDB propagated") - }) + _, gasRemaining, err := evm.Call(vm.AccountRef(rng.Address()), contract, []byte{}, gasLimit, uint256.NewInt(0)) + require.NoError(t, err, "evm.Call([contract with overridden opcode])") + assert.Equal(t, gasLimit-gasCost, gasRemaining, "gas remaining") + assert.Equal(t, spy.stateVal, value, "StateDB propagated") } func TestOperationFieldCount(t *testing.T) { diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 7de3d280d5d8..5c090387ea21 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -28,7 +28,6 @@ type Stub struct { PrecompileOverrides map[common.Address]libevm.PrecompiledContract CanExecuteTransactionFn func(common.Address, *common.Address, libevm.StateReader) error CanCreateContractFn func(*libevm.AddressContext, uint64, libevm.StateReader) (uint64, error) - OverrideJumpTableFn func() bool } // Register is a convenience wrapper for registering s as both the @@ -71,15 +70,6 @@ func (s Stub) CanCreateContract(cc *libevm.AddressContext, gas uint64, sr libevm return gas, nil } -// OverrideJumpTable proxies arguments to the s.OverrideJumpTableFn function if -// non-nil, otherwise it acts as a noop. -func (s Stub) OverrideJumpTable() bool { - if f := s.OverrideJumpTableFn; f != nil { - return f() - } - return false -} - var _ interface { params.ChainConfigHooks params.RulesHooks diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index 3cb9d95503f1..75fabbd3cce1 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -27,10 +27,6 @@ type RulesHooks interface { // [PrecompiledContract] is non-nil. If it returns `false` then the default // precompile behaviour is honoured. PrecompileOverride(common.Address) (_ libevm.PrecompiledContract, override bool) - // OverrideJumpTable signals that [vm.Hooks.OverrideJumpTable] MUST be used. - // Toggling the behaviour via [Rules] allows for recursive calling into - // functions that would otherwise infinitely call the override hook. - OverrideJumpTable() bool } // RulesAllowlistHooks are a subset of [RulesHooks] that gate actions, signalled @@ -88,9 +84,3 @@ func (NOOPHooks) CanCreateContract(_ *libevm.AddressContext, gas uint64, _ libev func (NOOPHooks) PrecompileOverride(common.Address) (libevm.PrecompiledContract, bool) { return nil, false } - -// OverrideJumpTable returns false, leaving all JumpTables unmodified even if -// [vm.Hooks] have been registered. -func (NOOPHooks) OverrideJumpTable() bool { - return false -} From 59cf9d7c8dd3267f615d738cb223c877e57226ef Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 19 Sep 2024 16:03:15 -0400 Subject: [PATCH 12/16] doc: update due to removals in last commit --- core/vm/hooks.libevm.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/vm/hooks.libevm.go b/core/vm/hooks.libevm.go index 30315fc798ad..f97fd5c7e451 100644 --- a/core/vm/hooks.libevm.go +++ b/core/vm/hooks.libevm.go @@ -15,14 +15,11 @@ var libevmHooks Hooks // Hooks are arbitrary configuration functions to modify default VM behaviour. type Hooks interface { - // OverrideJumpTable will only be called if - // [params.RulesHooks.OverrideJumpTable] returns true. This allows for - // recursive calling into [LookupInstructionSet]. OverrideJumpTable(params.Rules, *JumpTable) *JumpTable } -// overrideJumpTable returns `libevmHooks.OverrideJumpTable(r,jt)“ i.f.f. the -// Rules' hooks indicate that it must, otherwise it echoes `jt` unchanged. +// overrideJumpTable returns `libevmHooks.OverrideJumpTable(r,jt)` i.f.f. Hooks +// have been registered. func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable { if libevmHooks == nil { return jt From 7e62c287d26eeb52423cc2fe5b3b8165a5edfec9 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 14:57:11 -0400 Subject: [PATCH 13/16] refactor: combine `vm.{Precompile,Operation}Environment` --- core/vm/contracts.libevm.go | 91 +++++-------------------------- core/vm/contracts.libevm_test.go | 4 +- core/vm/environment.libevm.go | 74 +++++++++++++++++++++++++ core/vm/jump_table.libevm.go | 30 ++++------ core/vm/jump_table.libevm_test.go | 4 +- 5 files changed, 103 insertions(+), 100 deletions(-) create mode 100644 core/vm/environment.libevm.go diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index e615ed545a69..f77be9c63d6a 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -2,14 +2,11 @@ package vm import ( "fmt" - "math/big" "github.com/holiman/uint256" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/libevm" - "github.com/ethereum/go-ethereum/params" ) // evmCallArgs mirrors the parameters of the [EVM] methods Call(), CallCode(), @@ -54,7 +51,7 @@ const ( // 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, input, suppliedGas) + return p(args.env(), input, suppliedGas) } // Gas consumption for regular precompiles was already handled by the native // RunPrecompiledContract(), which called this method. @@ -64,7 +61,7 @@ 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) +type PrecompiledStatefulContract func(env Environment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, 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() @@ -91,41 +88,19 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) { panic(fmt.Sprintf("BUG: call to %T.Run(); MUST call %T itself", p, p)) } -// A PrecompileEnvironment provides information about the context in which a -// precompiled contract is being run. -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 - - BlockHeader() (types.Header, error) - BlockNumber() *big.Int - BlockTime() uint64 +func (args *evmCallArgs) env() *environment { + return &environment{ + evm: args.evm, + readonly: args.readOnly, + addrs: libevm.AddressContext{ + Origin: args.evm.Origin, + Caller: args.caller.Address(), + Self: args.addr, + }, + } } -// -// ****** SECURITY ****** -// -// If you are updating PrecompileEnvironment to provide the ability to call back -// into another contract, you MUST revisit the evmCallArgs.forceReadOnly flag. -// -// It is possible that forceReadOnly is true but evm.interpreter.readOnly is -// false. This is safe for now, but may not be if recursive calling *from* a -// precompile is enabled. -// -// ****** SECURITY ****** - -var _ PrecompileEnvironment = (*evmCallArgs)(nil) - -func (args *evmCallArgs) ChainConfig() *params.ChainConfig { return args.evm.chainConfig } -func (args *evmCallArgs) Rules() params.Rules { return args.evm.chainRules } - -func (args *evmCallArgs) ReadOnly() bool { +func (args *evmCallArgs) readOnly() bool { if args.readWrite == inheritReadOnly { if args.evm.interpreter.readOnly { //nolint:gosimple // Clearer code coverage for difficult-to-test branch return true @@ -138,46 +113,6 @@ func (args *evmCallArgs) ReadOnly() bool { return true } -func (args *evmCallArgs) StateDB() StateDB { - if args.ReadOnly() { - return nil - } - return args.evm.StateDB -} - -func (args *evmCallArgs) ReadOnlyState() libevm.StateReader { - // Even though we're actually returning a full state database, the user - // would have to actively circumvent the returned interface to use it. At - // that point they're off-piste and it's not our problem. - return args.evm.StateDB -} - -func (args *evmCallArgs) Addresses() *libevm.AddressContext { - return &libevm.AddressContext{ - Origin: args.evm.TxContext.Origin, - Caller: args.caller.Address(), - Self: args.addr, - } -} - -func (args *evmCallArgs) BlockHeader() (types.Header, error) { - hdr := args.evm.Context.Header - if hdr == nil { - // Although [core.NewEVMBlockContext] sets the field and is in the - // typical hot path (e.g. miner), there are other ways to create a - // [vm.BlockContext] (e.g. directly in tests) that may result in no - // available header. - return types.Header{}, fmt.Errorf("nil %T in current %T", hdr, args.evm.Context) - } - return *hdr, nil -} - -func (args *evmCallArgs) BlockNumber() *big.Int { - return new(big.Int).Set(args.evm.Context.BlockNumber) -} - -func (args *evmCallArgs) BlockTime() uint64 { return args.evm.Context.Time } - 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/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index b709da7445f4..79ad192f368c 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -117,7 +117,7 @@ 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.Environment, input []byte, suppliedGas uint64) ([]byte, uint64, 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) } @@ -261,7 +261,7 @@ 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.Environment, input []byte, suppliedGas uint64) ([]byte, uint64, error) { if env.ReadOnly() { return []byte{ifReadOnly}, suppliedGas, nil } diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go new file mode 100644 index 000000000000..dc39db81a499 --- /dev/null +++ b/core/vm/environment.libevm.go @@ -0,0 +1,74 @@ +package vm + +import ( + "fmt" + "math/big" + + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/libevm" + "github.com/ethereum/go-ethereum/params" +) + +// An Environment provides information about the context in which a precompiled +// contract or an instruction is being executed. +type Environment 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 + + BlockHeader() (types.Header, error) + BlockNumber() *big.Int + BlockTime() uint64 +} + +// +// ****** SECURITY ****** +// +// If you are updating PrecompileEnvironment to provide the ability to call back +// into another contract, you MUST revisit the evmCallArgs.forceReadOnly flag. +// +// It is possible that forceReadOnly is true but evm.interpreter.readOnly is +// false. This is safe for now, but may not be if recursive calling *from* a +// precompile is enabled. +// +// ****** SECURITY ****** + +var _ Environment = (*environment)(nil) + +type environment struct { + evm *EVM + readonly func() bool + addrs libevm.AddressContext +} + +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.readonly() } +func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB } +func (e *environment) Addresses() *libevm.AddressContext { return &e.addrs } +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) StateDB() StateDB { + if e.ReadOnly() { + return nil + } + return e.evm.StateDB +} + +func (e *environment) BlockHeader() (types.Header, error) { + hdr := e.evm.Context.Header + if hdr == nil { + // Although [core.NewEVMBlockContext] sets the field and is in the + // typical hot path (e.g. miner), there are other ways to create a + // [vm.BlockContext] (e.g. directly in tests) that may result in no + // available header. + return types.Header{}, fmt.Errorf("nil %T in current %T", hdr, e.evm.Context) + } + return *hdr, nil +} diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 91ab6370fd29..7e5b25aefe3d 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -1,6 +1,8 @@ package vm -import "github.com/ethereum/go-ethereum/libevm" +import ( + "github.com/ethereum/go-ethereum/libevm" +) // An OperationBuilder is a factory for a new operations to include in a // [JumpTable]. @@ -26,28 +28,20 @@ func (b OperationBuilder) Build() *operation { } // An OperationFunc is the execution function of a custom instruction. -type OperationFunc func(_ *OperationEnvironment, pc *uint64, _ *EVMInterpreter, _ *ScopeContext) ([]byte, error) - -// An OperationEnvironment provides information about the context in which a -// custom instruction is being executed. -type OperationEnvironment struct { - ReadOnly bool - // StateDB will be non-nil i.f.f !ReadOnly. - StateDB StateDB - // ReadOnlyState will always be non-nil. - ReadOnlyState libevm.StateReader -} +type OperationFunc func(_ Environment, pc *uint64, _ *EVMInterpreter, _ *ScopeContext) ([]byte, error) // internal converts an exported [OperationFunc] into an un-exported // [executionFunc] as required to build an [operation]. func (fn OperationFunc) internal() executionFunc { return func(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { - env := &OperationEnvironment{ - ReadOnly: interpreter.readOnly, - ReadOnlyState: interpreter.evm.StateDB, - } - if !env.ReadOnly { - env.StateDB = interpreter.evm.StateDB + env := &environment{ + evm: interpreter.evm, + readonly: func() bool { return interpreter.readOnly }, + addrs: libevm.AddressContext{ + Origin: interpreter.evm.Origin, + Caller: scope.Contract.CallerAddress, + Self: scope.Contract.Address(), + }, } return fn(env, pc, interpreter, scope) } diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index 2e702d2640e1..444fbcfe010a 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -41,8 +41,8 @@ type opRecorder struct { stateVal common.Hash } -func (op *opRecorder) execute(env *vm.OperationEnvironment, pc *uint64, interpreter *vm.EVMInterpreter, scope *vm.ScopeContext) ([]byte, error) { - op.stateVal = env.StateDB.GetState(scope.Contract.Address(), common.Hash{}) +func (op *opRecorder) execute(env vm.Environment, pc *uint64, interpreter *vm.EVMInterpreter, scope *vm.ScopeContext) ([]byte, error) { + op.stateVal = env.StateDB().GetState(scope.Contract.Address(), common.Hash{}) return nil, nil } From 966d1480c7efa07ef1dd027006928ccfe76a809a Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 15:02:48 -0400 Subject: [PATCH 14/16] chore: fix remnant references to `PrecompileEnvironment` --- core/vm/contracts.libevm_test.go | 2 +- core/vm/environment.libevm.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index 79ad192f368c..0b64bc1ed6f3 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -119,7 +119,7 @@ func TestNewStatefulPrecompile(t *testing.T) { run := func(env vm.Environment, input []byte, suppliedGas uint64) ([]byte, uint64, 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, 0, fmt.Errorf("Environment().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 { diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index dc39db81a499..f3bbc3bcbde5 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -29,8 +29,8 @@ type Environment interface { // // ****** SECURITY ****** // -// If you are updating PrecompileEnvironment to provide the ability to call back -// into another contract, you MUST revisit the evmCallArgs.forceReadOnly flag. +// If you are updating Environment to provide the ability to call back into +// another contract, you MUST revisit the evmCallArgs.forceReadOnly flag. // // It is possible that forceReadOnly is true but evm.interpreter.readOnly is // false. This is safe for now, but may not be if recursive calling *from* a From 524167ebb2c7865639fe8fccc511c924dd1df67d Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Sep 2024 15:05:50 -0400 Subject: [PATCH 15/16] refactor: `*environment.readOnly bool` instead of `func() bool` --- core/vm/contracts.libevm.go | 2 +- core/vm/environment.libevm.go | 4 ++-- core/vm/jump_table.libevm.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index f77be9c63d6a..5678e344b3f0 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -91,7 +91,7 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) { func (args *evmCallArgs) env() *environment { return &environment{ evm: args.evm, - readonly: args.readOnly, + readOnly: args.readOnly(), addrs: libevm.AddressContext{ Origin: args.evm.Origin, Caller: args.caller.Address(), diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index f3bbc3bcbde5..8b7cf358f68f 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -42,13 +42,13 @@ var _ Environment = (*environment)(nil) type environment struct { evm *EVM - readonly func() bool + readOnly bool addrs libevm.AddressContext } 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.readonly() } +func (e *environment) ReadOnly() bool { return e.readOnly } func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB } func (e *environment) Addresses() *libevm.AddressContext { return &e.addrs } func (e *environment) BlockNumber() *big.Int { return new(big.Int).Set(e.evm.Context.BlockNumber) } diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 7e5b25aefe3d..3ecb0e713175 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -36,7 +36,7 @@ func (fn OperationFunc) internal() executionFunc { return func(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { env := &environment{ evm: interpreter.evm, - readonly: func() bool { return interpreter.readOnly }, + readOnly: interpreter.readOnly, addrs: libevm.AddressContext{ Origin: interpreter.evm.Origin, Caller: scope.Contract.CallerAddress, From d80b492d6978537a63a8848655f61e25984afa2c Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 25 Sep 2024 15:37:57 +0100 Subject: [PATCH 16/16] chore: copyright headers --- core/vm/environment.libevm.go | 16 ++++++++++++++++ core/vm/jump_table.libevm.go | 16 ++++++++++++++++ core/vm/jump_table.libevm_test.go | 16 ++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index 8b7cf358f68f..0d9ce9f1f85a 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -1,3 +1,19 @@ +// 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 vm import ( diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go index 3ecb0e713175..e22137fd9a5b 100644 --- a/core/vm/jump_table.libevm.go +++ b/core/vm/jump_table.libevm.go @@ -1,3 +1,19 @@ +// 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 vm import ( diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go index f6be1bbc730a..7556c20ebbd8 100644 --- a/core/vm/jump_table.libevm_test.go +++ b/core/vm/jump_table.libevm_test.go @@ -1,3 +1,19 @@ +// 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 vm_test import (