From 88cb42886341f726505dae9e9a07401e5ffa3a50 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Wed, 11 Sep 2024 14:29:18 -0700 Subject: [PATCH] params: additional ChainConfig hooks --- libevm/hookstest/stub.go | 2 ++ params/config.go | 6 +++--- params/config.libevm_test.go | 40 +++++++++++++++++++++++++---------- params/example.libevm_test.go | 5 +++++ params/hooks.libevm.go | 20 +++++++++++++++++- 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/libevm/hookstest/stub.go b/libevm/hookstest/stub.go index 2915d487453f..45b4dcc7a529 100644 --- a/libevm/hookstest/stub.go +++ b/libevm/hookstest/stub.go @@ -27,6 +27,8 @@ 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 + + params.NOOPHooks } // Register is a convenience wrapper for registering s as both the diff --git a/params/config.go b/params/config.go index 2e5850c440dc..56d179ba827a 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().CheckCompatible(newcfg, headNumber, headTimestamp) } // BaseFeeChangeDenominator bounds the amount the base fee can change between blocks. diff --git a/params/config.libevm_test.go b/params/config.libevm_test.go index 68ef8bca12bc..129cc76ec5bc 100644 --- a/params/config.libevm_test.go +++ b/params/config.libevm_test.go @@ -121,7 +121,7 @@ func TestRegisterExtras(t *testing.T) { } } -func TestZeroExtrasAndPointers(t *testing.T) { +func TestModificationOfZeroExtras(t *testing.T) { type ( ccExtra struct { X int @@ -137,39 +137,57 @@ func TestZeroExtrasAndPointers(t *testing.T) { t.Cleanup(TestOnlyClearRegisteredExtras) getter := RegisterExtras(Extras[ccExtra, rulesExtra]{}) - var ( - config ChainConfig - rules Rules - ) + config := new(ChainConfig) + rules := new(Rules) // These assertion helpers are defined before any modifications so that the // closure is demonstrably over the original zero values. assertChainConfigExtra := func(t *testing.T, want ccExtra, msg string) { t.Helper() - assert.Equalf(t, want, getter.FromChainConfig(&config), "%T: "+msg, &config) + assert.Equalf(t, want, getter.FromChainConfig(config), "%T: "+msg, &config) } assertRulesExtra := func(t *testing.T, want rulesExtra, msg string) { t.Helper() - assert.Equalf(t, want, getter.FromRules(&rules), "%T: "+msg, &rules) + assert.Equalf(t, want, getter.FromRules(rules), "%T: "+msg, &rules) } assertChainConfigExtra(t, ccExtra{}, "zero value") assertRulesExtra(t, rulesExtra{}, "zero value") const answer = 42 - getter.PointerFromChainConfig(&config).X = answer + getter.PointerFromChainConfig(config).X = answer assertChainConfigExtra(t, ccExtra{X: answer}, "after setting via pointer field") const pi = 314159 - getter.PointerFromRules(&rules).X = pi + getter.PointerFromRules(rules).X = pi assertRulesExtra(t, rulesExtra{X: pi}, "after setting via pointer field") ccReplace := ccExtra{X: 142857} - *getter.PointerFromChainConfig(&config) = ccReplace + *getter.PointerFromChainConfig(config) = ccReplace assertChainConfigExtra(t, ccReplace, "after replacement of entire extra via `*pointer = x`") rulesReplace := rulesExtra{X: 18101986} - *getter.PointerFromRules(&rules) = rulesReplace + *getter.PointerFromRules(rules) = rulesReplace assertRulesExtra(t, rulesReplace, "after replacement of entire extra via `*pointer = x`") + + if t.Failed() { + // The test of shallow copying is now guaranteed to fail. + return + } + t.Run("shallow copy", func(t *testing.T) { + ccCopy := *config + rCopy := *rules + + assert.Equal(t, getter.FromChainConfig(&ccCopy), ccReplace, "ChainConfig extras copied") + assert.Equal(t, getter.FromRules(&rCopy), rulesReplace, "Rules extras copied") + + const seqUp = 123456789 + getter.PointerFromChainConfig(&ccCopy).X = seqUp + assertChainConfigExtra(t, ccExtra{X: seqUp}, "original changed because copy only shallow") + + const seqDown = 987654321 + getter.PointerFromRules(&rCopy).X = seqDown + assertRulesExtra(t, rulesExtra{X: seqDown}, "original changed because copy only shallow") + }) } func TestExtrasPanic(t *testing.T) { diff --git a/params/example.libevm_test.go b/params/example.libevm_test.go index e2a7340d5610..ba676f892570 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 [ChainHooks] interface, resulting in + // default Ethereum behaviour. + params.NOOPHooks } // RulesExtra can be any struct. It too mirrors a common pattern in diff --git a/params/hooks.libevm.go b/params/hooks.libevm.go index c44cd3f1f055..2b0b74fe86d0 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 { + CheckCompatible(newcfg *ChainConfig, headNumber *big.Int, headTimestamp uint64) *ConfigCompatError + CheckConfigForkOrder() error + 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) CheckCompatible(_ *ChainConfig, _ *big.Int, _ uint64) *ConfigCompatError { + return nil +} + +func (NOOPHooks) CheckConfigForkOrder() error { + 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