From 6e09eb3ec13771a70a499553b7a1d2eb7993885a Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 11:26:22 -0400 Subject: [PATCH 1/5] feat: `CheckConfig{Compatible,ForkOrder}` + `Description` hooks --- libevm/hookstest/stub.go | 26 ++++++++++++++++++++++++++ params/config.go | 6 +++--- params/example.libevm_test.go | 8 +++++--- params/hooks.libevm.go | 20 +++++++++++++++++++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 719a16664f9f..ea337d131e25 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -25,6 +25,9 @@ func Register[C params.ChainConfigHooks, R params.RulesHooks](tb testing.TB, ext // [params.RulesHooks]. Each of the fields, if non-nil, back their respective // hook methods, which otherwise fall back to the default behaviour. type Stub struct { + CheckConfigForkOrderFn func() error + CheckConfigCompatibleFn func(*params.ChainConfig, *big.Int, uint64) *params.ConfigCompatError + DescriptionValue string PrecompileOverrides map[common.Address]libevm.PrecompiledContract CanExecuteTransactionFn func(common.Address, *common.Address, libevm.StateReader) error CanCreateContractFn func(*libevm.AddressContext, libevm.StateReader) error @@ -52,6 +55,29 @@ func (s Stub) PrecompileOverride(a common.Address) (libevm.PrecompiledContract, return p, ok } +// CheckConfigForkOrder proxies arguments to the s.CheckConfigForkOrderFn +// function if non-nil, otherwise it acts as a noop. +func (s Stub) CheckConfigForkOrder() error { + if f := s.CheckConfigForkOrderFn; f != nil { + return f() + } + return nil +} + +// CheckConfigCompatible proxies arguments to the s.CheckConfigCompatibleFn +// function if non-nil, otherwise it acts as a noop. +func (s Stub) CheckConfigCompatible(newcfg *params.ChainConfig, headNumber *big.Int, headTimestamp uint64) *params.ConfigCompatError { + if f := s.CheckConfigCompatibleFn; f != nil { + return f(newcfg, headNumber, headTimestamp) + } + return nil +} + +// Description returns s.DescriptionValue. +func (s Stub) Description() string { + return s.DescriptionValue +} + // CanExecuteTransaction proxies arguments to the s.CanExecuteTransactionFn // function if non-nil, otherwise it acts as a noop. func (s Stub) CanExecuteTransaction(from common.Address, to *common.Address, sr libevm.StateReader) error { diff --git a/params/config.go b/params/config.go index 2e5850c440dc..2762a504c44b 100644 --- a/params/config.go +++ b/params/config.go @@ -478,7 +478,7 @@ func (c *ChainConfig) Description() string { if c.VerkleTime != nil { banner += fmt.Sprintf(" - Verkle: @%-10v\n", *c.VerkleTime) } - return banner + return banner + c.Hooks().Description() } // IsHomestead returns whether num is either equal to the homestead block or greater. @@ -671,7 +671,7 @@ func (c *ChainConfig) CheckConfigForkOrder() error { lastFork = cur } } - return nil + return c.Hooks().CheckConfigForkOrder() } func (c *ChainConfig) checkCompatible(newcfg *ChainConfig, headNumber *big.Int, headTimestamp uint64) *ConfigCompatError { @@ -742,7 +742,7 @@ func (c *ChainConfig) checkCompatible(newcfg *ChainConfig, headNumber *big.Int, if isForkTimestampIncompatible(c.VerkleTime, newcfg.VerkleTime, headTimestamp) { return newTimestampCompatError("Verkle fork timestamp", c.VerkleTime, newcfg.VerkleTime) } - return nil + return c.Hooks().CheckConfigCompatible(newcfg, headNumber, headTimestamp) } // BaseFeeChangeDenominator bounds the amount the base fee can change between blocks. diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index dd264e43c789..8ee4f6c0ff75 100644 --- a/params/example.libevm_test.go +++ b/params/example.libevm_test.go @@ -51,6 +51,11 @@ func constructRulesExtra(c *params.ChainConfig, r *params.Rules, cEx ChainConfig // the standard [params.ChainConfig] struct. type ChainConfigExtra struct { MyForkTime *uint64 `json:"myForkTime"` + + // (Optional) If not all hooks are desirable then embedding a [NOOPHooks] + // allows the type to satisfy the [ChainConfigHooks] interface, resulting in + // default Ethereum behaviour. + params.NOOPHooks } // RulesExtra can be any struct. It too mirrors a common pattern in @@ -59,9 +64,6 @@ type RulesExtra struct { IsMyFork bool timestamp uint64 - // (Optional) If not all hooks are desirable then embedding a [NOOPHooks] - // allows the type to satisfy the [RulesHooks] interface, resulting in - // default Ethereum behaviour. params.NOOPHooks } diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index 74fd88ccef53..13523e83169c 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -1,13 +1,19 @@ package params import ( + "math/big" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/libevm" ) // ChainConfigHooks are required for all types registered as [Extras] for // [ChainConfig] payloads. -type ChainConfigHooks interface{} +type ChainConfigHooks interface { + CheckConfigForkOrder() error + CheckConfigCompatible(newcfg *ChainConfig, headNumber *big.Int, headTimestamp uint64) *ConfigCompatError + Description() string +} // TODO(arr4n): given the choice of whether a hook should be defined on a // ChainConfig or on the Rules, what are the guiding principles? A ChainConfig @@ -66,6 +72,18 @@ var _ interface { RulesHooks } = NOOPHooks{} +func (NOOPHooks) CheckConfigForkOrder() error { + return nil +} + +func (NOOPHooks) CheckConfigCompatible(*ChainConfig, *big.Int, uint64) *ConfigCompatError { + return nil +} + +func (NOOPHooks) Description() string { + return "" +} + // CanExecuteTransaction allows all (otherwise valid) transactions. func (NOOPHooks) CanExecuteTransaction(_ common.Address, _ *common.Address, _ libevm.StateReader) error { return nil From 0fc930a94f7da39054b1fd01bd398b5d459af55f Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 12:12:55 -0400 Subject: [PATCH 2/5] doc: comments on `NOOPHooks` methods --- params/hooks.libevm.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index 13523e83169c..c97e080c3a0c 100644 --- a/params/hooks.libevm.go +++ b/params/hooks.libevm.go @@ -72,14 +72,17 @@ var _ interface { RulesHooks } = NOOPHooks{} +// CheckConfigForkOrder verifies all (otherwise valid) fork orders. func (NOOPHooks) CheckConfigForkOrder() error { return nil } +// CheckConfigCompatible verifies all (otherwise valid) new configs. func (NOOPHooks) CheckConfigCompatible(*ChainConfig, *big.Int, uint64) *ConfigCompatError { return nil } +// Description returns the empty string. func (NOOPHooks) Description() string { return "" } From 6122f033d35856ee04bd64e4d73d7bfbb3e7622e Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 13:52:37 -0400 Subject: [PATCH 3/5] test: all new hooks --- libevm/hookstest/stub.go | 12 +++---- params/hooks.libevm_test.go | 62 +++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 params/hooks.libevm_test.go diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 0788fc1f77b3..7a5a32a097af 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -14,11 +14,11 @@ import ( // Register clears any registered [params.Extras] and then registers `extras` // for the lifetime of the current test, clearing them via tb's // [testing.TB.Cleanup]. -func Register[C params.ChainConfigHooks, R params.RulesHooks](tb testing.TB, extras params.Extras[C, R]) { +func Register[C params.ChainConfigHooks, R params.RulesHooks](tb testing.TB, extras params.Extras[C, R]) params.ExtraPayloads[C, R] { tb.Helper() params.TestOnlyClearRegisteredExtras() tb.Cleanup(params.TestOnlyClearRegisteredExtras) - params.RegisterExtras(extras) + return params.RegisterExtras(extras) } // A Stub is a test double for [params.ChainConfigHooks] and @@ -27,7 +27,7 @@ func Register[C params.ChainConfigHooks, R params.RulesHooks](tb testing.TB, ext type Stub struct { CheckConfigForkOrderFn func() error CheckConfigCompatibleFn func(*params.ChainConfig, *big.Int, uint64) *params.ConfigCompatError - DescriptionValue string + DescriptionSuffix string PrecompileOverrides map[common.Address]libevm.PrecompiledContract CanExecuteTransactionFn func(common.Address, *common.Address, libevm.StateReader) error CanCreateContractFn func(*libevm.AddressContext, uint64, libevm.StateReader) (uint64, error) @@ -35,9 +35,9 @@ type Stub struct { // Register is a convenience wrapper for registering s as both the // [params.ChainConfigHooks] and [params.RulesHooks] via [Register]. -func (s *Stub) Register(tb testing.TB) { +func (s *Stub) Register(tb testing.TB) params.ExtraPayloads[*Stub, *Stub] { tb.Helper() - Register(tb, params.Extras[*Stub, *Stub]{ + return Register(tb, params.Extras[*Stub, *Stub]{ NewRules: func(_ *params.ChainConfig, _ *params.Rules, _ *Stub, blockNum *big.Int, isMerge bool, timestamp uint64) *Stub { return s }, @@ -75,7 +75,7 @@ func (s Stub) CheckConfigCompatible(newcfg *params.ChainConfig, headNumber *big. // Description returns s.DescriptionValue. func (s Stub) Description() string { - return s.DescriptionValue + return s.DescriptionSuffix } // CanExecuteTransaction proxies arguments to the s.CanExecuteTransactionFn diff --git a/params/hooks.libevm_test.go b/params/hooks.libevm_test.go new file mode 100644 index 000000000000..e22c099b211f --- /dev/null +++ b/params/hooks.libevm_test.go @@ -0,0 +1,62 @@ +package params_test + +import ( + "errors" + "fmt" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/libevm/ethtest" + "github.com/ethereum/go-ethereum/libevm/hookstest" + "github.com/ethereum/go-ethereum/params" + "github.com/stretchr/testify/require" +) + +func TestChainConfigHooks_Description(t *testing.T) { + const suffix = "Arran was here" + c := new(params.ChainConfig) + want := c.Description() + suffix + + hooks := &hookstest.Stub{ + DescriptionSuffix: "Arran was here", + } + hooks.Register(t).SetOnChainConfig(c, hooks) + require.Equal(t, want, c.Description(), "ChainConfigHooks.Description() is appended to non-extras equivalent") +} + +func TestChainConfigHooks_CheckConfigForkOrder(t *testing.T) { + err := errors.New("uh oh") + + c := new(params.ChainConfig) + require.NoError(t, c.CheckConfigForkOrder(), "CheckConfigForkOrder() with no hooks") + + hooks := &hookstest.Stub{ + CheckConfigForkOrderFn: func() error { return err }, + } + hooks.Register(t).SetOnChainConfig(c, hooks) + require.Equal(t, err, c.CheckConfigForkOrder(), "CheckConfigForkOrder() with error-producing hook") +} + +func TestChainConfigHooks_CheckConfigCompatible(t *testing.T) { + rng := ethtest.NewPseudoRand(1234567890) + newcfg := ¶ms.ChainConfig{ + ChainID: rng.BigUint64(), + } + headNumber := rng.Uint64() + headTimestamp := rng.Uint64() + + c := new(params.ChainConfig) + require.Nil(t, c.CheckCompatible(newcfg, headNumber, headTimestamp), "CheckCompatible() with no hooks") + + makeCompatErr := func(newcfg *params.ChainConfig, headNumber *big.Int, headTimestamp uint64) *params.ConfigCompatError { + return ¶ms.ConfigCompatError{ + What: fmt.Sprintf("ChainID: %v Head #: %v Head Time: %d", newcfg.ChainID, headNumber, headTimestamp), + } + } + hooks := &hookstest.Stub{ + CheckConfigCompatibleFn: makeCompatErr, + } + hooks.Register(t).SetOnChainConfig(c, hooks) + want := makeCompatErr(newcfg, new(big.Int).SetUint64(headNumber), headTimestamp) + require.Equal(t, want, c.CheckCompatible(newcfg, headNumber, headTimestamp), "CheckCompatible() with error-producing hook") +} From 9be23e3b73064fe4897256ed48f255e7c8c74c08 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 17 Sep 2024 13:57:40 -0400 Subject: [PATCH 4/5] chore: `gci` --- params/hooks.libevm_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/params/hooks.libevm_test.go b/params/hooks.libevm_test.go index e22c099b211f..4b9fbf4e9d13 100644 --- a/params/hooks.libevm_test.go +++ b/params/hooks.libevm_test.go @@ -6,10 +6,11 @@ import ( "math/big" "testing" + "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum/libevm/ethtest" "github.com/ethereum/go-ethereum/libevm/hookstest" "github.com/ethereum/go-ethereum/params" - "github.com/stretchr/testify/require" ) func TestChainConfigHooks_Description(t *testing.T) { From de2d3029394161e1c8d4295c13c1333318cac0f4 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Date: Tue, 17 Sep 2024 15:48:52 -0400 Subject: [PATCH 5/5] doc: fix `hookstest.Stub.Description` comment Co-authored-by: Darioush Jalali Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> --- libevm/hookstest/stub.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 7a5a32a097af..270c113f5713 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -73,7 +73,7 @@ func (s Stub) CheckConfigCompatible(newcfg *params.ChainConfig, headNumber *big. return nil } -// Description returns s.DescriptionValue. +// Description returns s.DescriptionSuffix. func (s Stub) Description() string { return s.DescriptionSuffix }