From fc28b9c0b3dfd89ea697efede4fafa6d34b79ec9 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 2 Sep 2024 14:58:33 +0100 Subject: [PATCH 1/5] feat: precompile override via `params.Extras` hooks --- core/vm/evm.go | 3 + core/vm/precompiles.libevm_test.go | 113 +++++++++++++++++++++++++++++ libevm/interfaces_test.go | 16 ++++ libevm/libevm.go | 9 +++ params/config.libevm.go | 51 ++++++++++++- params/config.libevm_test.go | 41 +++-------- params/example.libevm_test.go | 9 +++ params/hooks.libevm.go | 50 +++++++++++++ 8 files changed, 259 insertions(+), 33 deletions(-) create mode 100644 core/vm/precompiles.libevm_test.go create mode 100644 libevm/interfaces_test.go create mode 100644 libevm/libevm.go create mode 100644 params/hooks.libevm.go diff --git a/core/vm/evm.go b/core/vm/evm.go index 16cc8549080a..35a0aa227cbc 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -38,6 +38,9 @@ type ( ) func (evm *EVM) precompile(addr common.Address) (PrecompiledContract, bool) { + if p, override := evm.chainRules.PrecompileOverride(addr); override { + return p, p != nil + } var precompiles map[common.Address]PrecompiledContract switch { case evm.chainRules.IsCancun: diff --git a/core/vm/precompiles.libevm_test.go b/core/vm/precompiles.libevm_test.go new file mode 100644 index 000000000000..c3d2018658fe --- /dev/null +++ b/core/vm/precompiles.libevm_test.go @@ -0,0 +1,113 @@ +package vm + +import ( + "fmt" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/libevm" + "github.com/ethereum/go-ethereum/params" + "github.com/holiman/uint256" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/exp/rand" +) + +// precompileOverrides is a [params.RulesHooks] that overrides precompiles from +// a map of predefined addresses. +type precompileOverrides struct { + contracts map[common.Address]PrecompiledContract + params.NoopHooks // all other hooks +} + +func (o precompileOverrides) PrecompileOverride(_ params.Rules, a common.Address) (libevm.PrecompiledContract, bool) { + c, ok := o.contracts[a] + return c, ok +} + +// A precompileStub is a [PrecompiledContract] that always returns the same +// values. +type precompileStub struct { + requiredGas uint64 + returnData []byte +} + +func (s *precompileStub) RequiredGas([]byte) uint64 { return s.requiredGas } +func (s *precompileStub) Run([]byte) ([]byte, error) { return s.returnData, nil } + +func TestPrecompileOverride(t *testing.T) { + type test struct { + name string + addr common.Address + requiredGas uint64 + stubData []byte + } + + const gasLimit = uint64(1e7) + + tests := []test{ + { + name: "arbitrary values", + addr: common.Address{'p', 'r', 'e', 'c', 'o', 'm', 'p', 'i', 'l', 'e'}, + requiredGas: 314159, + stubData: []byte("the return data"), + }, + } + + rng := rand.New(rand.NewSource(42)) + for _, addr := range PrecompiledAddressesCancun { + tests = append(tests, test{ + name: fmt.Sprintf("existing precompile %v", addr), + addr: addr, + requiredGas: rng.Uint64n(gasLimit), + stubData: addr[:], + }) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + precompile := &precompileStub{ + requiredGas: tt.requiredGas, + returnData: tt.stubData, + } + + params.TestOnlyClearRegisteredExtras() + params.RegisterExtras(params.Extras[params.NoopHooks, precompileOverrides]{ + NewRules: func(_ *params.ChainConfig, _ *params.Rules, _ *params.NoopHooks, blockNum *big.Int, isMerge bool, timestamp uint64) *precompileOverrides { + return &precompileOverrides{ + contracts: map[common.Address]PrecompiledContract{ + tt.addr: precompile, + }, + } + }, + }) + + t.Run(fmt.Sprintf("%T.Call([overridden precompile address = %v])", &EVM{}, tt.addr), func(t *testing.T) { + gotData, gotGasLeft, err := newEVM(t).Call(AccountRef{}, tt.addr, nil, gasLimit, uint256.NewInt(0)) + require.NoError(t, err) + assert.Equal(t, tt.stubData, gotData, "contract's return data") + assert.Equal(t, gasLimit-tt.requiredGas, gotGasLeft, "gas left") + }) + }) + } +} + +func newEVM(t *testing.T) *EVM { + t.Helper() + + sdb, err := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + require.NoError(t, err, "state.New()") + + return NewEVM( + BlockContext{ + Transfer: func(_ StateDB, _, _ common.Address, _ *uint256.Int) {}, + }, + TxContext{}, + sdb, + ¶ms.ChainConfig{}, + Config{}, + ) +} diff --git a/libevm/interfaces_test.go b/libevm/interfaces_test.go new file mode 100644 index 000000000000..13691be27eb4 --- /dev/null +++ b/libevm/interfaces_test.go @@ -0,0 +1,16 @@ +package libevm_test + +import ( + "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/libevm" +) + +// These two interfaces MUST be identical. If this breaks then the libevm copy +// MUST be updated. +var ( + // Each assignment demonstrates that the methods of the LHS interface are a + // (non-strict) subset of the RHS interface's; both being possible + // proves that they are identical. + _ vm.PrecompiledContract = (libevm.PrecompiledContract)(nil) + _ libevm.PrecompiledContract = (vm.PrecompiledContract)(nil) +) diff --git a/libevm/libevm.go b/libevm/libevm.go new file mode 100644 index 000000000000..b48eb1c5fae2 --- /dev/null +++ b/libevm/libevm.go @@ -0,0 +1,9 @@ +package libevm + +// PrecompiledContract is an exact copy of [vm.PrecompiledContract], mirrored +// here for instances where importing [vm] would result in a circular +// dependency. +type PrecompiledContract interface { + RequiredGas(input []byte) uint64 + Run(input []byte) ([]byte, error) +} diff --git a/params/config.libevm.go b/params/config.libevm.go index 66e995f9a6a8..423d2d662e7f 100644 --- a/params/config.libevm.go +++ b/params/config.libevm.go @@ -5,13 +5,15 @@ import ( "fmt" "math/big" "reflect" + "runtime" + "strings" "github.com/ethereum/go-ethereum/libevm/pseudo" ) // Extras are arbitrary payloads to be added as extra fields in [ChainConfig] // and [Rules] structs. See [RegisterExtras]. -type Extras[C any, R any] struct { +type Extras[C ChainConfigHooks, R RulesHooks] struct { // NewRules, if non-nil is called at the end of [ChainConfig.Rules] with the // newly created [Rules] and other context from the method call. Its // returned value will be the extra payload of the [Rules]. If NewRules is @@ -38,18 +40,35 @@ type Extras[C any, R any] struct { // The payloads can be accessed via the [ExtraPayloadGetter.FromChainConfig] and // [ExtraPayloadGetter.FromRules] methods of the getter returned by // RegisterExtras. -func RegisterExtras[C any, R any](e Extras[C, R]) ExtraPayloadGetter[C, R] { +func RegisterExtras[C ChainConfigHooks, R RulesHooks](e Extras[C, R]) ExtraPayloadGetter[C, R] { if registeredExtras != nil { panic("re-registration of Extras") } mustBeStruct[C]() mustBeStruct[R]() + + getter := e.getter() registeredExtras = &extraConstructors{ chainConfig: pseudo.NewConstructor[C](), rules: pseudo.NewConstructor[R](), newForRules: e.newForRules, + getter: getter, + } + return getter +} + +// TestOnlyClearRegisteredExtras clears the [Extras] previously passed to +// [RegisterExtras]. It panics if called from a non-test file. +// +// In tests, it SHOULD be called before every call to [RegisterExtras] and then +// defer-called afterwards. This is a workaround for the single-call limitation +// on [RegisterExtras]. +func TestOnlyClearRegisteredExtras() { + _, file, _, ok := runtime.Caller(1 /* 0 would be here, not our caller */) + if !ok || !strings.HasSuffix(file, "_test.go") { + panic("call from non-test file") } - return e.getter() + registeredExtras = nil } // registeredExtras holds non-generic constructors for the [Extras] types @@ -59,6 +78,10 @@ var registeredExtras *extraConstructors type extraConstructors struct { chainConfig, rules pseudo.Constructor newForRules func(_ *ChainConfig, _ *Rules, blockNum *big.Int, isMerge bool, timestamp uint64) *pseudo.Type + getter interface { // use hooksFrom() methods for access + hooksFromChainConfig(*ChainConfig) ChainConfigHooks + hooksFromRules(*Rules) RulesHooks + } } func (e *Extras[C, R]) newForRules(c *ChainConfig, r *Rules, blockNum *big.Int, isMerge bool, timestamp uint64) *pseudo.Type { @@ -88,7 +111,7 @@ func notStructMessage[T any]() string { // An ExtraPayloadGettter provides strongly typed access to the extra payloads // carried by [ChainConfig] and [Rules] structs. The only valid way to construct // a getter is by a call to [RegisterExtras]. -type ExtraPayloadGetter[C any, R any] struct { +type ExtraPayloadGetter[C ChainConfigHooks, R RulesHooks] struct { _ struct{} // make godoc show unexported fields so nobody tries to make their own getter ;) } @@ -97,11 +120,31 @@ func (ExtraPayloadGetter[C, R]) FromChainConfig(c *ChainConfig) *C { return pseudo.MustNewValue[*C](c.extraPayload()).Get() } +// hooksFromChainConfig is equivalent to FromChainConfig(), but returns an +// interface instead of the concrete type implementing it; this allows it to be +// used in non-generic code. If the concrete-type value is nil (typically +// because no [Extras] were registered) a [noopHooks] is returned so it can be +// used without nil checks. +func (e ExtraPayloadGetter[C, R]) hooksFromChainConfig(c *ChainConfig) ChainConfigHooks { + if h := e.FromChainConfig(c); h != nil { + return *h + } + return NoopHooks{} +} + // FromRules returns the Rules' extra payload. func (ExtraPayloadGetter[C, R]) FromRules(r *Rules) *R { return pseudo.MustNewValue[*R](r.extraPayload()).Get() } +// hooksFromRules is the [RulesHooks] equivalent of hooksFromChainConfig(). +func (e ExtraPayloadGetter[C, R]) hooksFromRules(r *Rules) RulesHooks { + if h := e.FromRules(r); h != nil { + return *h + } + return NoopHooks{} +} + // UnmarshalJSON implements the [json.Unmarshaler] interface. func (c *ChainConfig) UnmarshalJSON(data []byte) error { type raw ChainConfig // doesn't inherit methods so avoids recursing back here (infinitely) diff --git a/params/config.libevm_test.go b/params/config.libevm_test.go index 7f74e748a704..83854c304424 100644 --- a/params/config.libevm_test.go +++ b/params/config.libevm_test.go @@ -10,13 +10,6 @@ import ( "github.com/stretchr/testify/require" ) -// testOnlyClearRegisteredExtras SHOULD be called before every call to -// [RegisterExtras] and then defer-called afterwards. This is a workaround for -// the single-call limitation on [RegisterExtras]. -func testOnlyClearRegisteredExtras() { - registeredExtras = nil -} - type rawJSON struct { json.RawMessage } @@ -30,15 +23,19 @@ func TestRegisterExtras(t *testing.T) { type ( ccExtraA struct { A string `json:"a"` + ChainConfigHooks } rulesExtraA struct { A string + RulesHooks } ccExtraB struct { B string `json:"b"` + ChainConfigHooks } rulesExtraB struct { B string + RulesHooks } ) @@ -79,20 +76,20 @@ func TestRegisterExtras(t *testing.T) { { name: "custom JSON handling honoured", register: func() { - RegisterExtras(Extras[rawJSON, struct{}]{}) + RegisterExtras(Extras[rawJSON, struct{ RulesHooks }]{}) }, ccExtra: pseudo.From(&rawJSON{ RawMessage: []byte(`"hello, world"`), }).Type, - wantRulesExtra: (*struct{})(nil), + wantRulesExtra: (*struct{ RulesHooks })(nil), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - testOnlyClearRegisteredExtras() + TestOnlyClearRegisteredExtras() tt.register() - defer testOnlyClearRegisteredExtras() + defer TestOnlyClearRegisteredExtras() in := &ChainConfig{ ChainID: big.NewInt(142857), @@ -116,22 +113,8 @@ func TestRegisterExtras(t *testing.T) { } func TestExtrasPanic(t *testing.T) { - testOnlyClearRegisteredExtras() - defer testOnlyClearRegisteredExtras() - - assertPanics( - t, func() { - RegisterExtras(Extras[int, struct{}]{}) - }, - notStructMessage[int](), - ) - - assertPanics( - t, func() { - RegisterExtras(Extras[struct{}, bool]{}) - }, - notStructMessage[bool](), - ) + TestOnlyClearRegisteredExtras() + defer TestOnlyClearRegisteredExtras() assertPanics( t, func() { @@ -147,11 +130,11 @@ func TestExtrasPanic(t *testing.T) { "before RegisterExtras", ) - RegisterExtras(Extras[struct{}, struct{}]{}) + RegisterExtras(Extras[struct{ ChainConfigHooks }, struct{ RulesHooks }]{}) assertPanics( t, func() { - RegisterExtras(Extras[struct{}, struct{}]{}) + RegisterExtras(Extras[struct{ ChainConfigHooks }, struct{ RulesHooks }]{}) }, "re-registration", ) diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index c6c99219fa7d..4db9fdef8a51 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -15,12 +15,16 @@ import ( "log" "math/big" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/libevm" "github.com/ethereum/go-ethereum/params" ) // In practice this would be a regular init() function but nuances around the // testing of this package require it to be called in the Example(). func initFn() { + params.TestOnlyClearRegisteredExtras() // not necessary outside of the example // This registration makes *all* [params.ChainConfig] and [params.Rules] // instances respect the payload types. They do not need to be modified to // know about `extraparams`. @@ -52,6 +56,11 @@ type RulesExtra struct { IsMyFork bool } +func (r RulesExtra) PrecompileOverride(params.Rules, common.Address) (libevm.PrecompiledContract, bool) { + var override vm.PrecompiledContract + return override, true +} + // FromChainConfig returns the extra payload carried by the ChainConfig. func FromChainConfig(c *params.ChainConfig) *ChainConfigExtra { return getter.FromChainConfig(c) diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go new file mode 100644 index 000000000000..56dbc964da98 --- /dev/null +++ b/params/hooks.libevm.go @@ -0,0 +1,50 @@ +package params + +import ( + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/libevm" +) + +// ChainConfigHooks are... +type ChainConfigHooks interface{} + +// RulesHooks are... +type RulesHooks interface { + PrecompileOverride(Rules, common.Address) (_ libevm.PrecompiledContract, override bool) +} + +func (e *extraConstructors) hooksFromChainConfig(c *ChainConfig) ChainConfigHooks { + if e == nil { + return NoopHooks{} + } + return e.getter.hooksFromChainConfig(c) +} + +func (e *extraConstructors) hooksFromRules(r *Rules) RulesHooks { + if e == nil { + return NoopHooks{} + } + return e.getter.hooksFromRules(r) +} + +// PrecompileOverride... +func (r Rules) PrecompileOverride(addr common.Address) (libevm.PrecompiledContract, bool) { + return registeredExtras.hooksFromRules(&r).PrecompileOverride(r, addr) +} + +// NoopHooks implement both [ChainConfigHooks] and [RulesHooks] such that every +// hook is a no-op. This allows it to be returned instead of nil, which would +// require every usage site to perform a nil check. It can also be embedded in +// structs that only wish to implement a sub-set of hooks. +type NoopHooks struct{} + +var _ interface { + ChainConfigHooks + RulesHooks +} = NoopHooks{} + +// PrecompileOverride always returns `nil, false`; the `false` indicates that +// the `nil` should be ignored. +func (NoopHooks) PrecompileOverride(Rules, common.Address) (libevm.PrecompiledContract, bool) { + return nil, false +} From fb916fe4a941678e5abb192d896c6c650ba979c9 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 2 Sep 2024 17:19:17 +0100 Subject: [PATCH 2/5] doc: flesh out `PrecompileOverride()` in example --- params/example.libevm_test.go | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index 4db9fdef8a51..e779da4ddfa3 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -56,11 +56,6 @@ type RulesExtra struct { IsMyFork bool } -func (r RulesExtra) PrecompileOverride(params.Rules, common.Address) (libevm.PrecompiledContract, bool) { - var override vm.PrecompiledContract - return override, true -} - // FromChainConfig returns the extra payload carried by the ChainConfig. func FromChainConfig(c *params.ChainConfig) *ChainConfigExtra { return getter.FromChainConfig(c) @@ -71,6 +66,33 @@ func FromRules(r *params.Rules) *RulesExtra { return getter.FromRules(r) } +// myForkPrecompiledContracts is analogous to the vm.PrecompiledContracts +// maps. Note [RulesExtra.PrecompileOverride] treatment of nil values here. +var myForkPrecompiledContracts = map[common.Address]vm.PrecompiledContract{ + //... + common.BytesToAddress([]byte{0x2}): nil, // i.e disabled + //... +} + +// PrecompileOverride implements the required [params.RuleHooks] method. +func (rex RulesExtra) PrecompileOverride(_ params.Rules, addr common.Address) (_ libevm.PrecompiledContract, override bool) { + if !rex.IsMyFork { + return nil, false + } + p, ok := myForkPrecompiledContracts[addr] + // The returned boolean indicates whether or not [vm.EVMInterpreter] MUST + // override the address, not what it returns as its own `isPrecompile` + // boolean. + // + // Therefore returning `nil, true` here indicates that the precompile will + // be disabled. Returning `false` here indicates that the default precompile + // behaviour will be exhibited. + // + // The same pattern can alternatively be implemented with an explicit + // `disabledPrecompiles` set to make the behaviour clearer. + return p, ok +} + // This example demonstrates how the rest of this file would be used from a // *different* package. func ExampleExtraPayloadGetter() { From 7d1d2069a2170a4b9cb20991221dd11bef4b6ed8 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 2 Sep 2024 18:20:11 +0100 Subject: [PATCH 3/5] doc: complete commentary and improve readability --- core/vm/precompiles.libevm_test.go | 6 ++-- libevm/libevm.go | 4 +-- params/config.libevm.go | 14 +++++--- params/hooks.libevm.go | 58 +++++++++++++++++++----------- 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/core/vm/precompiles.libevm_test.go b/core/vm/precompiles.libevm_test.go index c3d2018658fe..7f6a5405ca39 100644 --- a/core/vm/precompiles.libevm_test.go +++ b/core/vm/precompiles.libevm_test.go @@ -20,7 +20,7 @@ import ( // a map of predefined addresses. type precompileOverrides struct { contracts map[common.Address]PrecompiledContract - params.NoopHooks // all other hooks + params.NOOPHooks // all other hooks } func (o precompileOverrides) PrecompileOverride(_ params.Rules, a common.Address) (libevm.PrecompiledContract, bool) { @@ -75,8 +75,8 @@ func TestPrecompileOverride(t *testing.T) { } params.TestOnlyClearRegisteredExtras() - params.RegisterExtras(params.Extras[params.NoopHooks, precompileOverrides]{ - NewRules: func(_ *params.ChainConfig, _ *params.Rules, _ *params.NoopHooks, blockNum *big.Int, isMerge bool, timestamp uint64) *precompileOverrides { + params.RegisterExtras(params.Extras[params.NOOPHooks, precompileOverrides]{ + NewRules: func(_ *params.ChainConfig, _ *params.Rules, _ *params.NOOPHooks, blockNum *big.Int, isMerge bool, timestamp uint64) *precompileOverrides { return &precompileOverrides{ contracts: map[common.Address]PrecompiledContract{ tt.addr: precompile, diff --git a/libevm/libevm.go b/libevm/libevm.go index b48eb1c5fae2..edb54cc3c00c 100644 --- a/libevm/libevm.go +++ b/libevm/libevm.go @@ -1,7 +1,7 @@ package libevm -// PrecompiledContract is an exact copy of [vm.PrecompiledContract], mirrored -// here for instances where importing [vm] would result in a circular +// PrecompiledContract is an exact copy of vm.PrecompiledContract, mirrored here +// for instances where importing that package would result in a circular // dependency. type PrecompiledContract interface { RequiredGas(input []byte) uint64 diff --git a/params/config.libevm.go b/params/config.libevm.go index 423d2d662e7f..9ddc00359e45 100644 --- a/params/config.libevm.go +++ b/params/config.libevm.go @@ -39,7 +39,9 @@ type Extras[C ChainConfigHooks, R RulesHooks] struct { // // The payloads can be accessed via the [ExtraPayloadGetter.FromChainConfig] and // [ExtraPayloadGetter.FromRules] methods of the getter returned by -// RegisterExtras. +// RegisterExtras. Where stated in the interface definitions, they will also be +// used as hooks to alter Ethereum behaviour; if this isn't desired then they +// can embed [NOOPHooks] to satisfy either interface. func RegisterExtras[C ChainConfigHooks, R RulesHooks](e Extras[C, R]) ExtraPayloadGetter[C, R] { if registeredExtras != nil { panic("re-registration of Extras") @@ -60,7 +62,7 @@ func RegisterExtras[C ChainConfigHooks, R RulesHooks](e Extras[C, R]) ExtraPaylo // TestOnlyClearRegisteredExtras clears the [Extras] previously passed to // [RegisterExtras]. It panics if called from a non-test file. // -// In tests, it SHOULD be called before every call to [RegisterExtras] and then +// In tests it SHOULD be called before every call to [RegisterExtras] and then // defer-called afterwards. This is a workaround for the single-call limitation // on [RegisterExtras]. func TestOnlyClearRegisteredExtras() { @@ -78,7 +80,9 @@ var registeredExtras *extraConstructors type extraConstructors struct { chainConfig, rules pseudo.Constructor newForRules func(_ *ChainConfig, _ *Rules, blockNum *big.Int, isMerge bool, timestamp uint64) *pseudo.Type - getter interface { // use hooksFrom() methods for access + // use top-level hooksFrom() functions instead of these as they handle + // instances where no [Extras] were registered. + getter interface { hooksFromChainConfig(*ChainConfig) ChainConfigHooks hooksFromRules(*Rules) RulesHooks } @@ -129,7 +133,7 @@ func (e ExtraPayloadGetter[C, R]) hooksFromChainConfig(c *ChainConfig) ChainConf if h := e.FromChainConfig(c); h != nil { return *h } - return NoopHooks{} + return NOOPHooks{} } // FromRules returns the Rules' extra payload. @@ -142,7 +146,7 @@ func (e ExtraPayloadGetter[C, R]) hooksFromRules(r *Rules) RulesHooks { if h := e.FromRules(r); h != nil { return *h } - return NoopHooks{} + return NOOPHooks{} } // UnmarshalJSON implements the [json.Unmarshaler] interface. diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index 56dbc964da98..4133f14815d5 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -5,46 +5,62 @@ import ( "github.com/ethereum/go-ethereum/libevm" ) -// ChainConfigHooks are... +// ChainConfigHooks are required for all types registered as [Extras] for +// [ChainConfig] payloads. type ChainConfigHooks interface{} -// RulesHooks are... +// RulesHooks are required for all types registered as [Extras] for [Rules] +// payloads. type RulesHooks interface { + // PrecompileOverride signals whether or not the EVM interpreter MUST + // override its treatment of the address when deciding if it is a + // precompiled contract. If PrecompileOverride returns `true` then the + // interpreter will treat the address as a precompile i.f.f the + // [PrecompiledContract] is non-nil. If it returns `false` then the default + // precompile behaviour is honoured. PrecompileOverride(Rules, common.Address) (_ libevm.PrecompiledContract, override bool) } -func (e *extraConstructors) hooksFromChainConfig(c *ChainConfig) ChainConfigHooks { - if e == nil { - return NoopHooks{} +// hooksFromChainConfig returns the registered hooks, or [NOOPHooks] if none +// were registered. +func hooksFromChainConfig(c *ChainConfig) ChainConfigHooks { + if e := registeredExtras; e != nil { + return e.getter.hooksFromChainConfig(c) } - return e.getter.hooksFromChainConfig(c) + return NOOPHooks{} } -func (e *extraConstructors) hooksFromRules(r *Rules) RulesHooks { - if e == nil { - return NoopHooks{} +// hooksFromRules returns the registered hooks, or [NOOPHooks] if none were +// registered. +func hooksFromRules(r *Rules) RulesHooks { + if e := registeredExtras; e != nil { + return e.getter.hooksFromRules(r) } - return e.getter.hooksFromRules(r) + return NOOPHooks{} } -// PrecompileOverride... +// PrecompileOverride is a wrapper for the equivalent method of the registered +// [RulesHooks] carried by the Rules. If no [Extras] were registered, a +// [NOOPHooks] is used instead, such that the default precompile behaviour is +// exhibited. func (r Rules) PrecompileOverride(addr common.Address) (libevm.PrecompiledContract, bool) { - return registeredExtras.hooksFromRules(&r).PrecompileOverride(r, addr) + return hooksFromRules(&r).PrecompileOverride(r, addr) } -// NoopHooks implement both [ChainConfigHooks] and [RulesHooks] such that every -// hook is a no-op. This allows it to be returned instead of nil, which would -// require every usage site to perform a nil check. It can also be embedded in -// structs that only wish to implement a sub-set of hooks. -type NoopHooks struct{} +// NOOPHooks implements both [ChainConfigHooks] and [RulesHooks] such that every +// hook is a no-op. This allows it to be returned instead of a nil interface, +// which would otherwise require every usage site to perform a nil check. It can +// also be embedded in structs that only wish to implement a sub-set of hooks. +// Use of a NOOPHooks is equivalent to default Ethereum behaviour. +type NOOPHooks struct{} var _ interface { ChainConfigHooks RulesHooks -} = NoopHooks{} +} = NOOPHooks{} -// PrecompileOverride always returns `nil, false`; the `false` indicates that -// the `nil` should be ignored. -func (NoopHooks) PrecompileOverride(Rules, common.Address) (libevm.PrecompiledContract, bool) { +// PrecompileOverride instructs the EVM interpreter to use the default +// precompile behaviour. +func (NOOPHooks) PrecompileOverride(Rules, common.Address) (libevm.PrecompiledContract, bool) { return nil, false } From 9dee6b288edb0dfc7acb1ef405387cf6758768d4 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 3 Sep 2024 21:16:50 +0100 Subject: [PATCH 4/5] refactor: `ChainConfig.Hooks()` + `Rules` equivalent --- core/vm/evm.go | 2 +- core/vm/precompiles.libevm_test.go | 2 +- params/config.libevm.go | 6 ++++-- params/config.libevm_test.go | 8 ++++++++ params/example.libevm_test.go | 4 ++-- params/hooks.libevm.go | 24 ++++++++---------------- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/core/vm/evm.go b/core/vm/evm.go index 35a0aa227cbc..6cabfccd94c8 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -38,7 +38,7 @@ type ( ) func (evm *EVM) precompile(addr common.Address) (PrecompiledContract, bool) { - if p, override := evm.chainRules.PrecompileOverride(addr); override { + if p, override := evm.chainRules.Hooks().PrecompileOverride(addr); override { return p, p != nil } var precompiles map[common.Address]PrecompiledContract diff --git a/core/vm/precompiles.libevm_test.go b/core/vm/precompiles.libevm_test.go index 7f6a5405ca39..b0b276162828 100644 --- a/core/vm/precompiles.libevm_test.go +++ b/core/vm/precompiles.libevm_test.go @@ -23,7 +23,7 @@ type precompileOverrides struct { params.NOOPHooks // all other hooks } -func (o precompileOverrides) PrecompileOverride(_ params.Rules, a common.Address) (libevm.PrecompiledContract, bool) { +func (o precompileOverrides) PrecompileOverride(a common.Address) (libevm.PrecompiledContract, bool) { c, ok := o.contracts[a] return c, ok } diff --git a/params/config.libevm.go b/params/config.libevm.go index 9ddc00359e45..d9312b528fe8 100644 --- a/params/config.libevm.go +++ b/params/config.libevm.go @@ -156,8 +156,10 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { *raw Extra *pseudo.Type `json:"extra"` }{ - raw: (*raw)(c), // embedded to achieve regular JSON unmarshalling - Extra: registeredExtras.chainConfig.NilPointer(), // `c.extra` is otherwise unexported + raw: (*raw)(c), // embedded to achieve regular JSON unmarshalling + } + if e := registeredExtras; e != nil { + cc.Extra = e.chainConfig.NilPointer() // `c.extra` is otherwise unexported } if err := json.Unmarshal(data, cc); err != nil { diff --git a/params/config.libevm_test.go b/params/config.libevm_test.go index 83854c304424..9b17cb428795 100644 --- a/params/config.libevm_test.go +++ b/params/config.libevm_test.go @@ -12,6 +12,7 @@ import ( type rawJSON struct { json.RawMessage + NOOPHooks } var _ interface { @@ -130,6 +131,13 @@ func TestExtrasPanic(t *testing.T) { "before RegisterExtras", ) + assertPanics( + t, func() { + mustBeStruct[int]() + }, + notStructMessage[int](), + ) + RegisterExtras(Extras[struct{ ChainConfigHooks }, struct{ RulesHooks }]{}) assertPanics( diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index e779da4ddfa3..0d91f70531a7 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -75,8 +75,8 @@ var myForkPrecompiledContracts = map[common.Address]vm.PrecompiledContract{ } // PrecompileOverride implements the required [params.RuleHooks] method. -func (rex RulesExtra) PrecompileOverride(_ params.Rules, addr common.Address) (_ libevm.PrecompiledContract, override bool) { - if !rex.IsMyFork { +func (r RulesExtra) PrecompileOverride(addr common.Address) (_ libevm.PrecompiledContract, override bool) { + if !r.IsMyFork { return nil, false } p, ok := myForkPrecompiledContracts[addr] diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index 4133f14815d5..54b58a798f65 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -18,35 +18,27 @@ type RulesHooks interface { // interpreter will treat the address as a precompile i.f.f the // [PrecompiledContract] is non-nil. If it returns `false` then the default // precompile behaviour is honoured. - PrecompileOverride(Rules, common.Address) (_ libevm.PrecompiledContract, override bool) + PrecompileOverride(common.Address) (_ libevm.PrecompiledContract, override bool) } -// hooksFromChainConfig returns the registered hooks, or [NOOPHooks] if none -// were registered. -func hooksFromChainConfig(c *ChainConfig) ChainConfigHooks { +// Hooks returns the hooks registered with [RegisterExtras], or [NOOPHooks] if +// none were registered. +func (c *ChainConfig) Hooks() ChainConfigHooks { if e := registeredExtras; e != nil { return e.getter.hooksFromChainConfig(c) } return NOOPHooks{} } -// hooksFromRules returns the registered hooks, or [NOOPHooks] if none were -// registered. -func hooksFromRules(r *Rules) RulesHooks { +// Hooks returns the hooks registered with [RegisterExtras], or [NOOPHooks] if +// none were registered. +func (r *Rules) Hooks() RulesHooks { if e := registeredExtras; e != nil { return e.getter.hooksFromRules(r) } return NOOPHooks{} } -// PrecompileOverride is a wrapper for the equivalent method of the registered -// [RulesHooks] carried by the Rules. If no [Extras] were registered, a -// [NOOPHooks] is used instead, such that the default precompile behaviour is -// exhibited. -func (r Rules) PrecompileOverride(addr common.Address) (libevm.PrecompiledContract, bool) { - return hooksFromRules(&r).PrecompileOverride(r, addr) -} - // NOOPHooks implements both [ChainConfigHooks] and [RulesHooks] such that every // hook is a no-op. This allows it to be returned instead of a nil interface, // which would otherwise require every usage site to perform a nil check. It can @@ -61,6 +53,6 @@ var _ interface { // PrecompileOverride instructs the EVM interpreter to use the default // precompile behaviour. -func (NOOPHooks) PrecompileOverride(Rules, common.Address) (libevm.PrecompiledContract, bool) { +func (NOOPHooks) PrecompileOverride(common.Address) (libevm.PrecompiledContract, bool) { return nil, false } From 639aa1cadf528ad9c46975b0a8f91268d6aad372 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 3 Sep 2024 21:18:43 +0100 Subject: [PATCH 5/5] chore: rename precompiles test file in keeping with geth equivalent --- core/vm/{precompiles.libevm_test.go => contracts.libevm_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename core/vm/{precompiles.libevm_test.go => contracts.libevm_test.go} (100%) diff --git a/core/vm/precompiles.libevm_test.go b/core/vm/contracts.libevm_test.go similarity index 100% rename from core/vm/precompiles.libevm_test.go rename to core/vm/contracts.libevm_test.go