feat: CheckConfig{Compatible,ForkOrder} + Description hooks (#29)

* feat: `CheckConfig{Compatible,ForkOrder}` + `Description` hooks

* doc: comments on `NOOPHooks` methods

* test: all new hooks

* chore: `gci`

* doc: fix `hookstest.Stub.Description` comment

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>

---------

Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

libevm/hookstest/stub.go

@@ -14,17 +14,20 @@ 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
 // [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
+	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)
@@ -32,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
 		},
@@ -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.DescriptionSuffix.
+func (s Stub) Description() string {
+	return s.DescriptionSuffix
+}
+
 // 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 {

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.

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
 }
 

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
@@ -68,6 +74,21 @@ 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 ""
+}
+
 // CanExecuteTransaction allows all (otherwise valid) transactions.
 func (NOOPHooks) CanExecuteTransaction(_ common.Address, _ *common.Address, _ libevm.StateReader) error {
 	return nil

params/hooks.libevm_test.go

@@ -0,0 +1,63 @@
+package params_test
+
+import (
+	"errors"
+	"fmt"
+	"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"
+)
+
+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 := &params.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 &params.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")
+}