From af8ab3ec573ee78667872b3778754b6feabafe22 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 29 Jul 2024 17:45:31 +0100 Subject: [PATCH 01/25] experiment: lazy-init precompiles in `ChainConfig` --- go.mod | 1 + go.sum | 2 ++ params/config.go | 25 ++++----------- params/config_extra.go | 19 +++--------- params/config_test.go | 10 ++++++ params/precompiles.go | 27 +--------------- precompile/contract/interfaces.go | 3 ++ precompile/modules/chain_config.go | 49 ++++++++++++++++++++++++++++++ 8 files changed, 77 insertions(+), 59 deletions(-) create mode 100644 precompile/modules/chain_config.go diff --git a/go.mod b/go.mod index e996dad9ce..fea630b494 100644 --- a/go.mod +++ b/go.mod @@ -39,6 +39,7 @@ require ( github.com/stretchr/testify v1.8.4 github.com/tyler-smith/go-bip39 v1.1.0 github.com/urfave/cli/v2 v2.25.7 + go.pennock.tech/swallowjson v1.0.2 go.uber.org/goleak v1.3.0 go.uber.org/mock v0.4.0 golang.org/x/crypto v0.21.0 diff --git a/go.sum b/go.sum index 9002e11eb2..2e98cd23fd 100644 --- a/go.sum +++ b/go.sum @@ -608,6 +608,8 @@ go.opentelemetry.io/otel/trace v1.22.0 h1:Hg6pPujv0XG9QaVbGOBVHunyuLcCC3jN7WEhPx go.opentelemetry.io/otel/trace v1.22.0/go.mod h1:RbbHXVqKES9QhzZq/fE5UnOSILqRt40a21sPw2He1xo= go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I= go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= +go.pennock.tech/swallowjson v1.0.2 h1:vkefBIn8GOFMLiLMNU8fEhWAAgiXXtsxsnqqcP7Kszg= +go.pennock.tech/swallowjson v1.0.2/go.mod h1:b6sGbYY+XjsKddYrRT44EJQi6BJCQwW+biJzIONU2xw= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU= diff --git a/params/config.go b/params/config.go index 59ae91a869..87ace196b0 100644 --- a/params/config.go +++ b/params/config.go @@ -36,7 +36,6 @@ import ( "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/version" "github.com/ava-labs/subnet-evm/commontype" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" @@ -244,8 +243,12 @@ type ChainConfig struct { FeeConfig commontype.FeeConfig `json:"feeConfig"` // Set the configuration for the dynamic fee algorithm AllowFeeRecipients bool `json:"allowFeeRecipients,omitempty"` // Allows fees to be collected by block builders. - GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. - UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. + // LazyUnmarshalData carries all raw JSON data, provided when unmarshalling this instance, that didn't have a struct + // field into which it could be unmarshalled. It is used to lazily unmarshal the GenesisPrecompiles and avoid + // importing the `precompiles/modules` package as that would result in a circular dependency. + LazyUnmarshalData map[string]json.RawMessage `json:"-"` + GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. + UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. } // Description returns a human-readable description of ChainConfig. @@ -755,22 +758,6 @@ func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { rules.AvalancheRules = c.GetAvalancheRules(timestamp) - // Initialize the stateful precompiles that should be enabled at [blockTimestamp]. - rules.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) - rules.Predicaters = make(map[common.Address]precompileconfig.Predicater) - rules.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) - for _, module := range modules.RegisteredModules() { - if config := c.getActivePrecompileConfig(module.Address, timestamp); config != nil && !config.IsDisabled() { - rules.ActivePrecompiles[module.Address] = config - if predicater, ok := config.(precompileconfig.Predicater); ok { - rules.Predicaters[module.Address] = predicater - } - if precompileAccepter, ok := config.(precompileconfig.Accepter); ok { - rules.AccepterPrecompiles[module.Address] = precompileAccepter - } - } - } - return rules } diff --git a/params/config_extra.go b/params/config_extra.go index 8eafe9e8d4..1f2bd8939f 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -10,6 +10,7 @@ import ( "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/subnet-evm/utils" + "go.pennock.tech/swallowjson" ) // UpgradeConfig includes the following configs that may be specified in upgradeBytes: @@ -36,27 +37,17 @@ type AvalancheContext struct { // This is a custom unmarshaler to handle the Precompiles field. // Precompiles was presented as an inline object in the JSON. // This custom unmarshaler ensures backwards compatibility with the old format. +// TODO(arr4n) update this method comment DO NOT MERGE func (c *ChainConfig) UnmarshalJSON(data []byte) error { - // Alias ChainConfig to avoid recursion - type _ChainConfig ChainConfig - tmp := _ChainConfig{} - if err := json.Unmarshal(data, &tmp); err != nil { - return err - } - - // At this point we have populated all fields except PrecompileUpgrade - *c = ChainConfig(tmp) - - // Unmarshal inlined PrecompileUpgrade - return json.Unmarshal(data, &c.GenesisPrecompiles) + return swallowjson.UnmarshalWith(c, "LazyUnmarshalData", data) } // MarshalJSON returns the JSON encoding of c. // This is a custom marshaler to handle the Precompiles field. -func (c ChainConfig) MarshalJSON() ([]byte, error) { +func (c *ChainConfig) MarshalJSON() ([]byte, error) { // Alias ChainConfig to avoid recursion type _ChainConfig ChainConfig - tmp, err := json.Marshal(_ChainConfig(c)) + tmp, err := json.Marshal((*_ChainConfig)(c)) if err != nil { return nil, err } diff --git a/params/config_test.go b/params/config_test.go index 707cf89f9d..ce93cc14bf 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -37,8 +37,10 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" + "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -172,6 +174,7 @@ func TestConfigRules(t *testing.T) { func TestConfigUnmarshalJSON(t *testing.T) { require := require.New(t) + assert := assert.New(t) testRewardManagerConfig := rewardmanager.NewConfig( utils.NewUint64(1671542573), @@ -218,6 +221,13 @@ func TestConfigUnmarshalJSON(t *testing.T) { require.Equal(c.ChainID, big.NewInt(43214)) require.Equal(c.AllowFeeRecipients, true) + for _, config := range []precompileconfig.Config{testRewardManagerConfig, testContractNativeMinterConfig} { + _, ok := c.LazyUnmarshalData[config.Key()] + assert.Truef(ok, "%T.LazyUnmarshalData[%q] not unmarshalled", c) + } + + return + rewardManagerConfig, ok := c.GenesisPrecompiles[rewardmanager.ConfigKey] require.True(ok) require.Equal(rewardManagerConfig.Key(), rewardmanager.ConfigKey) diff --git a/params/precompiles.go b/params/precompiles.go index 5d8ed74bda..a3ca0a1b98 100644 --- a/params/precompiles.go +++ b/params/precompiles.go @@ -4,33 +4,8 @@ package params import ( - "encoding/json" - - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" ) +// TODO(arr4n): can this be removed? DO NOT MERGE type Precompiles map[string]precompileconfig.Config - -// UnmarshalJSON parses the JSON-encoded data into the ChainConfigPrecompiles. -// ChainConfigPrecompiles is a map of precompile module keys to their -// configuration. -func (ccp *Precompiles) UnmarshalJSON(data []byte) error { - raw := make(map[string]json.RawMessage) - if err := json.Unmarshal(data, &raw); err != nil { - return err - } - - *ccp = make(Precompiles) - for _, module := range modules.RegisteredModules() { - key := module.ConfigKey - if value, ok := raw[key]; ok { - conf := module.MakeConfig() - if err := json.Unmarshal(value, conf); err != nil { - return err - } - (*ccp)[key] = conf - } - } - return nil -} diff --git a/precompile/contract/interfaces.go b/precompile/contract/interfaces.go index 5ac6baa486..8f37506937 100644 --- a/precompile/contract/interfaces.go +++ b/precompile/contract/interfaces.go @@ -12,6 +12,9 @@ import ( "github.com/ethereum/go-ethereum/common" ) +// Guarantee that we don't have a circular dependency if importing types here. +// var _ *types.Transaction = nil + // StatefulPrecompiledContract is the interface for executing a precompiled contract type StatefulPrecompiledContract interface { // Run executes the precompiled contract. diff --git a/precompile/modules/chain_config.go b/precompile/modules/chain_config.go new file mode 100644 index 0000000000..fc214fa2a6 --- /dev/null +++ b/precompile/modules/chain_config.go @@ -0,0 +1,49 @@ +package modules + +import ( + "encoding/json" + "fmt" + + "github.com/ava-labs/subnet-evm/params" + "github.com/ava-labs/subnet-evm/precompile/precompileconfig" + "github.com/ethereum/go-ethereum/common" +) + +func InitChainConfig(c *params.ChainConfig) error { + if c.GenesisPrecompiles == nil { + c.GenesisPrecompiles = make(params.Precompiles) + } + for key, raw := range c.LazyUnmarshalData { + mod, ok := GetPrecompileModule(key) + if !ok { + continue + } + + conf := mod.MakeConfig() + if err := json.Unmarshal(raw, conf); err != nil { + return fmt.Errorf("unmarshal %T: %v", conf, err) + } + c.GenesisPrecompiles[key] = conf + } + return nil +} + +func InitChainRules(r *params.Rules) { + // Initialize the stateful precompiles that should be enabled at [blockTimestamp]. + r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) + r.Predicaters = make(map[common.Address]precompileconfig.Predicater) + r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) + + // TODO(arr4n): range over config keys instead + for _, module := range RegisteredModules() { + if config := c.getActivePrecompileConfig(module.Address, timestamp); config != nil && !config.IsDisabled() { + r.ActivePrecompiles[module.Address] = config + if predicater, ok := config.(precompileconfig.Predicater); ok { + r.Predicaters[module.Address] = predicater + } + if precompileAccepter, ok := config.(precompileconfig.Accepter); ok { + r.AccepterPrecompiles[module.Address] = precompileAccepter + } + } + } +} From 7f39a4c253079baa4d45658cbe380990fb93f5bf Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 30 Jul 2024 12:29:54 +0100 Subject: [PATCH 02/25] refactor: move `modules`-dependent code out of `params` (only builds; does not work) --- params/config.go | 16 ++- params/config_extra.go | 3 + params/precompile_upgrade.go | 152 -------------------------- precompile/modules/chain_config.go | 165 ++++++++++++++++++++++++++++- 4 files changed, 172 insertions(+), 164 deletions(-) diff --git a/params/config.go b/params/config.go index 87ace196b0..3ff2751401 100644 --- a/params/config.go +++ b/params/config.go @@ -215,6 +215,7 @@ var ( // ChainConfig is stored in the database on a per block basis. This means // that any network, identified by its genesis block, can have its own // set of configuration options. +// TODO document the need to call InitChainConfig (DO NOT MERGE) type ChainConfig struct { ChainID *big.Int `json:"chainId"` // chainId identifies the current chain and is used for replay protection @@ -375,12 +376,6 @@ func (r *Rules) PredicaterExists(addr common.Address) bool { return PredicaterExists } -// IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. -func (c *ChainConfig) IsPrecompileEnabled(address common.Address, timestamp uint64) bool { - config := c.getActivePrecompileConfig(address, timestamp) - return config != nil && !config.IsDisabled() -} - // CheckCompatible checks whether scheduled fork transitions have been imported // with a mismatching chain configuration. func (c *ChainConfig) CheckCompatible(newcfg *ChainConfig, height uint64, time uint64) *ConfigCompatError { @@ -568,10 +563,11 @@ func (c *ChainConfig) checkCompatible(newcfg *ChainConfig, height *big.Int, time return err } + // TODO: DO NOT MERGE // Check that the precompiles on the new config are compatible with the existing precompile config. - if err := c.CheckPrecompilesCompatible(newcfg.PrecompileUpgrades, time); err != nil { - return err - } + // if err := c.CheckPrecompilesCompatible(newcfg.PrecompileUpgrades, time); err != nil { + // return err + // } // Check that the state upgrades on the new config are compatible with the existing state upgrade config. if err := c.CheckStateUpgradesCompatible(newcfg.StateUpgrades, time); err != nil { @@ -758,6 +754,8 @@ func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { rules.AvalancheRules = c.GetAvalancheRules(timestamp) + // TODO document the need to call InitChainRules (DO NOT MERGE) + return rules } diff --git a/params/config_extra.go b/params/config_extra.go index 1f2bd8939f..679cd0815a 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -45,6 +45,9 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { // MarshalJSON returns the JSON encoding of c. // This is a custom marshaler to handle the Precompiles field. func (c *ChainConfig) MarshalJSON() ([]byte, error) { + + // TODO refactor this (DO NOT MERGE) + // Alias ChainConfig to avoid recursion type _ChainConfig ChainConfig tmp, err := json.Marshal((*_ChainConfig)(c)) diff --git a/params/precompile_upgrade.go b/params/precompile_upgrade.go index 3f762f96c4..dae0aef1a4 100644 --- a/params/precompile_upgrade.go +++ b/params/precompile_upgrade.go @@ -4,18 +4,12 @@ package params import ( - "encoding/json" - "errors" "fmt" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" - "github.com/ethereum/go-ethereum/common" ) -var errNoKey = errors.New("PrecompileUpgrade cannot be empty") - // PrecompileUpgrade is a helper struct embedded in UpgradeConfig. // It is used to unmarshal the json into the correct precompile config type // based on the key. Keys are defined in each precompile module, and registered in @@ -24,43 +18,6 @@ type PrecompileUpgrade struct { precompileconfig.Config } -// UnmarshalJSON unmarshals the json into the correct precompile config type -// based on the key. Keys are defined in each precompile module, and registered in -// precompile/registry/registry.go. -// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key -func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error { - raw := make(map[string]json.RawMessage) - if err := json.Unmarshal(data, &raw); err != nil { - return err - } - if len(raw) == 0 { - return errNoKey - } - if len(raw) > 1 { - return fmt.Errorf("PrecompileUpgrade must have exactly one key, got %d", len(raw)) - } - for key, value := range raw { - module, ok := modules.GetPrecompileModule(key) - if !ok { - return fmt.Errorf("unknown precompile config: %s", key) - } - config := module.MakeConfig() - if err := json.Unmarshal(value, config); err != nil { - return err - } - u.Config = config - } - return nil -} - -// MarshalJSON marshal the precompile config into json based on the precompile key. -// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key -func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { - res := make(map[string]precompileconfig.Config) - res[u.Key()] = u.Config - return json.Marshal(res) -} - // verifyPrecompileUpgrades checks [c.PrecompileUpgrades] is well formed: // - [upgrades] must specify exactly one key per PrecompileUpgrade // - the specified blockTimestamps must monotonically increase @@ -146,112 +103,3 @@ func (c *ChainConfig) verifyPrecompileUpgrades() error { return nil } - -// getActivePrecompileConfig returns the most recent precompile config corresponding to [address]. -// If none have occurred, returns nil. -func (c *ChainConfig) getActivePrecompileConfig(address common.Address, timestamp uint64) precompileconfig.Config { - configs := c.GetActivatingPrecompileConfigs(address, nil, timestamp, c.PrecompileUpgrades) - if len(configs) == 0 { - return nil - } - return configs[len(configs)-1] // return the most recent config -} - -// GetActivatingPrecompileConfigs returns all precompile upgrades configured to activate during the -// state transition from a block with timestamp [from] to a block with timestamp [to]. -func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, from *uint64, to uint64, upgrades []PrecompileUpgrade) []precompileconfig.Config { - // Get key from address. - module, ok := modules.GetPrecompileModuleByAddress(address) - if !ok { - return nil - } - configs := make([]precompileconfig.Config, 0) - key := module.ConfigKey - // First check the embedded [upgrade] for precompiles configured - // in the genesis chain config. - if config, ok := c.GenesisPrecompiles[key]; ok { - if utils.IsForkTransition(config.Timestamp(), from, to) { - configs = append(configs, config) - } - } - // Loop over all upgrades checking for the requested precompile config. - for _, upgrade := range upgrades { - if upgrade.Key() == key { - // Check if the precompile activates in the specified range. - if utils.IsForkTransition(upgrade.Timestamp(), from, to) { - configs = append(configs, upgrade.Config) - } - } - } - return configs -} - -// CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. -// Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from -// [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. -// Returns nil if [precompileUpgrades] is compatible with [c]. -// Assumes given timestamp is the last accepted block timestamp. -// This ensures that as long as the node has not accepted a block with a different rule set it will allow a -// new upgrade to be applied as long as it activates after the last accepted block. -func (c *ChainConfig) CheckPrecompilesCompatible(precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { - for _, module := range modules.RegisteredModules() { - if err := c.checkPrecompileCompatible(module.Address, precompileUpgrades, time); err != nil { - return err - } - } - - return nil -} - -// checkPrecompileCompatible verifies that the precompile specified by [address] is compatible between [c] -// and [precompileUpgrades] at [headTimestamp]. -// Returns an error if upgrades already activated at [headTimestamp] are missing from [precompileUpgrades]. -// Upgrades that have already gone into effect cannot be modified or absent from [precompileUpgrades]. -func (c *ChainConfig) checkPrecompileCompatible(address common.Address, precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { - // All active upgrades (from nil to [lastTimestamp]) must match. - activeUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, c.PrecompileUpgrades) - newUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, precompileUpgrades) - - // Check activated upgrades are still present. - for i, upgrade := range activeUpgrades { - if len(newUpgrades) <= i { - // missing upgrade - return newTimestampCompatError( - fmt.Sprintf("missing PrecompileUpgrade[%d]", i), - upgrade.Timestamp(), - nil, - ) - } - // All upgrades that have activated must be identical. - if !upgrade.Equal(newUpgrades[i]) { - return newTimestampCompatError( - fmt.Sprintf("PrecompileUpgrade[%d]", i), - upgrade.Timestamp(), - newUpgrades[i].Timestamp(), - ) - } - } - // then, make sure newUpgrades does not have additional upgrades - // that are already activated. (cannot perform retroactive upgrade) - if len(newUpgrades) > len(activeUpgrades) { - return newTimestampCompatError( - fmt.Sprintf("cannot retroactively enable PrecompileUpgrade[%d]", len(activeUpgrades)), - nil, - newUpgrades[len(activeUpgrades)].Timestamp(), // this indexes to the first element in newUpgrades after the end of activeUpgrades - ) - } - - return nil -} - -// EnabledStatefulPrecompiles returns current stateful precompile configs that are enabled at [blockTimestamp]. -func (c *ChainConfig) EnabledStatefulPrecompiles(blockTimestamp uint64) Precompiles { - statefulPrecompileConfigs := make(Precompiles) - for _, module := range modules.RegisteredModules() { - if config := c.getActivePrecompileConfig(module.Address, blockTimestamp); config != nil && !config.IsDisabled() { - statefulPrecompileConfigs[module.ConfigKey] = config - } - } - - return statefulPrecompileConfigs -} diff --git a/precompile/modules/chain_config.go b/precompile/modules/chain_config.go index fc214fa2a6..2eee3bc028 100644 --- a/precompile/modules/chain_config.go +++ b/precompile/modules/chain_config.go @@ -2,10 +2,12 @@ package modules import ( "encoding/json" + "errors" "fmt" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" + "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" ) @@ -28,15 +30,14 @@ func InitChainConfig(c *params.ChainConfig) error { return nil } -func InitChainRules(r *params.Rules) { +func InitChainRules(c *params.ChainConfig, r *params.Rules, timestamp uint64) { // Initialize the stateful precompiles that should be enabled at [blockTimestamp]. r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) r.Predicaters = make(map[common.Address]precompileconfig.Predicater) r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) - // TODO(arr4n): range over config keys instead for _, module := range RegisteredModules() { - if config := c.getActivePrecompileConfig(module.Address, timestamp); config != nil && !config.IsDisabled() { + if config := getActivePrecompileConfig(c, module.Address, timestamp); config != nil && !config.IsDisabled() { r.ActivePrecompiles[module.Address] = config if predicater, ok := config.(precompileconfig.Predicater); ok { r.Predicaters[module.Address] = predicater @@ -47,3 +48,161 @@ func InitChainRules(r *params.Rules) { } } } + +func getActivePrecompileConfig(c *params.ChainConfig, address common.Address, timestamp uint64) precompileconfig.Config { + configs := GetActivatingPrecompileConfigs(c, address, nil, timestamp, c.PrecompileUpgrades) + if len(configs) == 0 { + return nil + } + return configs[len(configs)-1] // return the most recent config +} + +func GetActivatingPrecompileConfigs(c *params.ChainConfig, address common.Address, from *uint64, to uint64, upgrades []params.PrecompileUpgrade) []precompileconfig.Config { + // Get key from address. + module, ok := GetPrecompileModuleByAddress(address) + if !ok { + return nil + } + configs := make([]precompileconfig.Config, 0) + key := module.ConfigKey + // First check the embedded [upgrade] for precompiles configured + // in the genesis chain config. + if config, ok := c.GenesisPrecompiles[key]; ok { + if utils.IsForkTransition(config.Timestamp(), from, to) { + configs = append(configs, config) + } + } + // Loop over all upgrades checking for the requested precompile config. + for _, upgrade := range upgrades { + if upgrade.Key() == key { + // Check if the precompile activates in the specified range. + if utils.IsForkTransition(upgrade.Timestamp(), from, to) { + configs = append(configs, upgrade.Config) + } + } + } + return configs +} + +// CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. +// Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from +// [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. +// Returns nil if [precompileUpgrades] is compatible with [c]. +// Assumes given timestamp is the last accepted block timestamp. +// This ensures that as long as the node has not accepted a block with a different rule set it will allow a +// new upgrade to be applied as long as it activates after the last accepted block. +func CheckPrecompilesCompatible(c *params.ChainConfig, precompileUpgrades []params.PrecompileUpgrade, time uint64) *params.ConfigCompatError { + for _, module := range RegisteredModules() { + if err := checkPrecompileCompatible(c, module.Address, precompileUpgrades, time); err != nil { + return err + } + } + + return nil +} + +// checkPrecompileCompatible verifies that the precompile specified by [address] is compatible between [c] +// and [precompileUpgrades] at [headTimestamp]. +// Returns an error if upgrades already activated at [headTimestamp] are missing from [precompileUpgrades]. +// Upgrades that have already gone into effect cannot be modified or absent from [precompileUpgrades]. +func checkPrecompileCompatible(c *params.ChainConfig, address common.Address, precompileUpgrades []params.PrecompileUpgrade, time uint64) *params.ConfigCompatError { + // All active upgrades (from nil to [lastTimestamp]) must match. + activeUpgrades := GetActivatingPrecompileConfigs(c, address, nil, time, c.PrecompileUpgrades) + newUpgrades := GetActivatingPrecompileConfigs(c, address, nil, time, precompileUpgrades) + + // Check activated upgrades are still present. + for i, upgrade := range activeUpgrades { + if len(newUpgrades) <= i { + // missing upgrade + return newTimestampCompatError( + fmt.Sprintf("missing PrecompileUpgrade[%d]", i), + upgrade.Timestamp(), + nil, + ) + } + // All upgrades that have activated must be identical. + if !upgrade.Equal(newUpgrades[i]) { + return newTimestampCompatError( + fmt.Sprintf("PrecompileUpgrade[%d]", i), + upgrade.Timestamp(), + newUpgrades[i].Timestamp(), + ) + } + } + // then, make sure newUpgrades does not have additional upgrades + // that are already activated. (cannot perform retroactive upgrade) + if len(newUpgrades) > len(activeUpgrades) { + return newTimestampCompatError( + fmt.Sprintf("cannot retroactively enable PrecompileUpgrade[%d]", len(activeUpgrades)), + nil, + newUpgrades[len(activeUpgrades)].Timestamp(), // this indexes to the first element in newUpgrades after the end of activeUpgrades + ) + } + + return nil +} + +func newTimestampCompatError(what string, storedtime, newtime *uint64) *params.ConfigCompatError { + return nil // TODO: DO NOT MERGE +} + +// EnabledStatefulPrecompiles returns current stateful precompile configs that are enabled at [blockTimestamp]. +func EnabledStatefulPrecompiles(c *params.ChainConfig, blockTimestamp uint64) params.Precompiles { + statefulPrecompileConfigs := make(params.Precompiles) + for _, module := range RegisteredModules() { + if config := getActivePrecompileConfig(c, module.Address, blockTimestamp); config != nil && !config.IsDisabled() { + statefulPrecompileConfigs[module.ConfigKey] = config + } + } + + return statefulPrecompileConfigs +} + +type PrecompileUpgrade struct { + precompileconfig.Config // TODO: DO NOT MERGE +} + +var errNoKey = errors.New("PrecompileUpgrade cannot be empty") + +// UnmarshalJSON unmarshals the json into the correct precompile config type +// based on the key. Keys are defined in each precompile module, and registered in +// precompile/registry/registry.go. +// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key +func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error { + raw := make(map[string]json.RawMessage) + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + if len(raw) == 0 { + return errNoKey + } + if len(raw) > 1 { + return fmt.Errorf("PrecompileUpgrade must have exactly one key, got %d", len(raw)) + } + for key, value := range raw { + module, ok := GetPrecompileModule(key) + if !ok { + return fmt.Errorf("unknown precompile config: %s", key) + } + config := module.MakeConfig() + if err := json.Unmarshal(value, config); err != nil { + return err + } + u.Config = config + } + return nil +} + +// MarshalJSON marshal the precompile config into json based on the precompile key. +// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key +func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { + res := make(map[string]precompileconfig.Config) + res[u.Key()] = u.Config + return json.Marshal(res) +} + +// IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. +func IsPrecompileEnabled(c *params.ChainConfig, address common.Address, timestamp uint64) bool { + config := getActivePrecompileConfig(c, address, timestamp) + return config != nil && !config.IsDisabled() +} From 6acef2a9cdb39c283475a83ae52b06de1f2aa2b3 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 30 Jul 2024 12:37:02 +0100 Subject: [PATCH 03/25] fix: change `ChainConfig.Fn()` method calls to `modules.Fn(ChainConfig)` --- core/blockchain_reader.go | 5 +++-- core/state_processor.go | 2 +- core/state_transition.go | 3 ++- core/txpool/legacypool/legacypool.go | 3 ++- eth/gasprice/gasprice.go | 3 ++- internal/ethapi/api_extra.go | 3 ++- precompile/contract/interfaces.go | 3 ++- 7 files changed, 14 insertions(+), 8 deletions(-) diff --git a/core/blockchain_reader.go b/core/blockchain_reader.go index 554c20e730..c9f15b5bed 100644 --- a/core/blockchain_reader.go +++ b/core/blockchain_reader.go @@ -40,6 +40,7 @@ import ( "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/trie" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/event" @@ -344,7 +345,7 @@ func (bc *BlockChain) SubscribeAcceptedTransactionEvent(ch chan<- NewTxsEvent) e // Assumes that a valid configuration is stored when the precompile is activated. func (bc *BlockChain) GetFeeConfigAt(parent *types.Header) (commontype.FeeConfig, *big.Int, error) { config := bc.Config() - if !config.IsPrecompileEnabled(feemanager.ContractAddress, parent.Time) { + if !modules.IsPrecompileEnabled(config, feemanager.ContractAddress, parent.Time) { return config.FeeConfig, common.Big0, nil } @@ -382,7 +383,7 @@ func (bc *BlockChain) GetCoinbaseAt(parent *types.Header) (common.Address, bool, return constants.BlackholeAddr, false, nil } - if !config.IsPrecompileEnabled(rewardmanager.ContractAddress, parent.Time) { + if !modules.IsPrecompileEnabled(config, rewardmanager.ContractAddress, parent.Time) { if bc.chainConfig.AllowFeeRecipients { return common.Address{}, true, nil } else { diff --git a/core/state_processor.go b/core/state_processor.go index fb839cc187..e0183a8b9e 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -215,7 +215,7 @@ func ApplyPrecompileActivations(c *params.ChainConfig, parentTimestamp *uint64, // This ensures even if precompiles read/write state other than their own they will observe // an identical global state in a deterministic order when they are configured. for _, module := range modules.RegisteredModules() { - for _, activatingConfig := range c.GetActivatingPrecompileConfigs(module.Address, parentTimestamp, blockTimestamp, c.PrecompileUpgrades) { + for _, activatingConfig := range modules.GetActivatingPrecompileConfigs(c, module.Address, parentTimestamp, blockTimestamp, c.PrecompileUpgrades) { // If this transition activates the upgrade, configure the stateful precompile. // (or deconfigure it if it is being disabled.) if activatingConfig.IsDisabled() { diff --git a/core/state_transition.go b/core/state_transition.go index a71bacc192..7e321c38dc 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -36,6 +36,7 @@ import ( "github.com/ava-labs/subnet-evm/core/vm" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ava-labs/subnet-evm/vmerrs" "github.com/ethereum/go-ethereum/common" @@ -352,7 +353,7 @@ func (st *StateTransition) preCheck() error { } // Check that the sender is on the tx allow list if enabled - if st.evm.ChainConfig().IsPrecompileEnabled(txallowlist.ContractAddress, st.evm.Context.Time) { + if modules.IsPrecompileEnabled(st.evm.ChainConfig(), txallowlist.ContractAddress, st.evm.Context.Time) { txAllowListRole := txallowlist.GetTxAllowListStatus(st.state, msg.From) if !txAllowListRole.IsEnabled() { return fmt.Errorf("%w: %s", vmerrs.ErrSenderAddressNotAllowListed, msg.From) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 669c483e9f..35b06872ee 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -45,6 +45,7 @@ import ( "github.com/ava-labs/subnet-evm/metrics" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/prque" @@ -1497,7 +1498,7 @@ func (pool *LegacyPool) reset(oldHead, newHead *types.Header) { // when we reset txPool we should explicitly check if fee struct for min base fee has changed // so that we can correctly drop txs with < minBaseFee from tx pool. - if pool.chainconfig.IsPrecompileEnabled(feemanager.ContractAddress, newHead.Time) { + if modules.IsPrecompileEnabled(pool.chainconfig, feemanager.ContractAddress, newHead.Time) { feeConfig, _, err := pool.chain.GetFeeConfigAt(newHead) if err != nil { log.Error("Failed to get fee config state", "err", err, "root", newHead.Root) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 899c2eba0e..b9a8adf340 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -39,6 +39,7 @@ import ( "github.com/ava-labs/subnet-evm/core/types" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/rpc" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/lru" @@ -318,7 +319,7 @@ func (oracle *Oracle) suggestDynamicFees(ctx context.Context) (*big.Int, *big.In feeLastChangedAt *big.Int feeConfig commontype.FeeConfig ) - if oracle.backend.ChainConfig().IsPrecompileEnabled(feemanager.ContractAddress, head.Time) { + if modules.IsPrecompileEnabled(oracle.backend.ChainConfig(), feemanager.ContractAddress, head.Time) { feeConfig, feeLastChangedAt, err = oracle.backend.GetFeeConfigAt(head) if err != nil { return nil, nil, err diff --git a/internal/ethapi/api_extra.go b/internal/ethapi/api_extra.go index 5823ff3dc7..8bdce29ef9 100644 --- a/internal/ethapi/api_extra.go +++ b/internal/ethapi/api_extra.go @@ -12,6 +12,7 @@ import ( "github.com/ava-labs/subnet-evm/core" "github.com/ava-labs/subnet-evm/core/types" "github.com/ava-labs/subnet-evm/params" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/rpc" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -131,7 +132,7 @@ func (s *BlockChainAPI) GetActivePrecompilesAt(ctx context.Context, blockTimesta timestamp = *blockTimestamp } - return s.b.ChainConfig().EnabledStatefulPrecompiles(timestamp) + return modules.EnabledStatefulPrecompiles(s.b.ChainConfig(), timestamp) } type ActivePrecompilesResult struct { diff --git a/precompile/contract/interfaces.go b/precompile/contract/interfaces.go index 8f37506937..b7f800d26d 100644 --- a/precompile/contract/interfaces.go +++ b/precompile/contract/interfaces.go @@ -8,12 +8,13 @@ import ( "math/big" "github.com/ava-labs/avalanchego/snow" + "github.com/ava-labs/subnet-evm/core/types" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ethereum/go-ethereum/common" ) // Guarantee that we don't have a circular dependency if importing types here. -// var _ *types.Transaction = nil +var _ *types.Transaction = nil // StatefulPrecompiledContract is the interface for executing a precompiled contract type StatefulPrecompiledContract interface { From 8d9a7b609304163351e304910c20f0ec73fa9c7d Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 30 Jul 2024 18:59:08 +0100 Subject: [PATCH 04/25] refactor: HACKY! `./params/...` and `./precompile/...` tests passing --- go.mod | 1 - go.sum | 2 - params/config.go | 22 ++--- params/config_extra.go | 23 ++++- params/config_test.go | 40 ++++----- params/precompile_config_test.go | 27 +++--- params/precompile_upgrade_test.go | 8 +- params/state_upgrade_test.go | 9 +- precompile/modules/chain_config.go | 138 ++++++++++++++++++++++++----- 9 files changed, 186 insertions(+), 84 deletions(-) diff --git a/go.mod b/go.mod index fea630b494..e996dad9ce 100644 --- a/go.mod +++ b/go.mod @@ -39,7 +39,6 @@ require ( github.com/stretchr/testify v1.8.4 github.com/tyler-smith/go-bip39 v1.1.0 github.com/urfave/cli/v2 v2.25.7 - go.pennock.tech/swallowjson v1.0.2 go.uber.org/goleak v1.3.0 go.uber.org/mock v0.4.0 golang.org/x/crypto v0.21.0 diff --git a/go.sum b/go.sum index 2e98cd23fd..9002e11eb2 100644 --- a/go.sum +++ b/go.sum @@ -608,8 +608,6 @@ go.opentelemetry.io/otel/trace v1.22.0 h1:Hg6pPujv0XG9QaVbGOBVHunyuLcCC3jN7WEhPx go.opentelemetry.io/otel/trace v1.22.0/go.mod h1:RbbHXVqKES9QhzZq/fE5UnOSILqRt40a21sPw2He1xo= go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I= go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= -go.pennock.tech/swallowjson v1.0.2 h1:vkefBIn8GOFMLiLMNU8fEhWAAgiXXtsxsnqqcP7Kszg= -go.pennock.tech/swallowjson v1.0.2/go.mod h1:b6sGbYY+XjsKddYrRT44EJQi6BJCQwW+biJzIONU2xw= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU= diff --git a/params/config.go b/params/config.go index 3ff2751401..50493e29de 100644 --- a/params/config.go +++ b/params/config.go @@ -206,8 +206,6 @@ var ( GenesisPrecompiles: Precompiles{}, UpgradeConfig: UpgradeConfig{}, } - - TestRules = TestChainConfig.Rules(new(big.Int), 0) ) // ChainConfig is the core config which determines the blockchain settings. @@ -244,12 +242,8 @@ type ChainConfig struct { FeeConfig commontype.FeeConfig `json:"feeConfig"` // Set the configuration for the dynamic fee algorithm AllowFeeRecipients bool `json:"allowFeeRecipients,omitempty"` // Allows fees to be collected by block builders. - // LazyUnmarshalData carries all raw JSON data, provided when unmarshalling this instance, that didn't have a struct - // field into which it could be unmarshalled. It is used to lazily unmarshal the GenesisPrecompiles and avoid - // importing the `precompiles/modules` package as that would result in a circular dependency. - LazyUnmarshalData map[string]json.RawMessage `json:"-"` - GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. - UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. + GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. + UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. } // Description returns a human-readable description of ChainConfig. @@ -749,14 +743,10 @@ func (c *ChainConfig) rules(num *big.Int, timestamp uint64) Rules { // Rules returns the Avalanche modified rules to support Avalanche // network upgrades -func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { - rules := c.rules(blockNum, timestamp) - - rules.AvalancheRules = c.GetAvalancheRules(timestamp) - - // TODO document the need to call InitChainRules (DO NOT MERGE) - - return rules +func (c *ChainConfig) RulesDoNotCallDirectly(blockNum *big.Int, timestamp uint64) Rules { + r := c.rules(blockNum, timestamp) + r.AvalancheRules = c.GetAvalancheRules(timestamp) + return r } // GetFeeConfig returns the original FeeConfig contained in the genesis ChainConfig. diff --git a/params/config_extra.go b/params/config_extra.go index 679cd0815a..94a249ac33 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -6,11 +6,12 @@ package params import ( "encoding/json" "errors" + "fmt" "math/big" "github.com/ava-labs/avalanchego/snow" + "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" - "go.pennock.tech/swallowjson" ) // UpgradeConfig includes the following configs that may be specified in upgradeBytes: @@ -32,14 +33,30 @@ type AvalancheContext struct { SnowCtx *snow.Context } +func (u *UpgradeConfig) UnmarshalJSON([]byte) error { + return fmt.Errorf("do not unmarshal %T JSON directly; use modules.UnmarshalUpgradeConfigJSON()", u) +} + +func (u *PrecompileUpgrade) UnmarshalJSON([]byte) error { + return fmt.Errorf("do not unmarshal %T JSON directly; use modules.UnmarshalPrecompileUpgradeJSON()", u) +} + +// MarshalJSON marshal the precompile config into json based on the precompile key. +// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key +func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { + res := make(map[string]precompileconfig.Config) + res[u.Key()] = u.Config + return json.Marshal(res) +} + // UnmarshalJSON parses the JSON-encoded data and stores the result in the // object pointed to by c. // This is a custom unmarshaler to handle the Precompiles field. // Precompiles was presented as an inline object in the JSON. // This custom unmarshaler ensures backwards compatibility with the old format. // TODO(arr4n) update this method comment DO NOT MERGE -func (c *ChainConfig) UnmarshalJSON(data []byte) error { - return swallowjson.UnmarshalWith(c, "LazyUnmarshalData", data) +func (c *ChainConfig) UnmarshalJSON([]byte) error { + return fmt.Errorf("do not unmarshal %T JSON directly; use modules.UnmarshalChainConfigJSON()", c) } // MarshalJSON returns the JSON encoding of c. diff --git a/params/config_test.go b/params/config_test.go index ce93cc14bf..02cbd316e6 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -24,7 +24,7 @@ // You should have received a copy of the GNU Lesser General Public License // along with the go-ethereum library. If not, see . -package params +package params_test import ( "encoding/json" @@ -37,11 +37,12 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" - "github.com/ava-labs/subnet-evm/precompile/precompileconfig" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + . "github.com/ava-labs/subnet-evm/params" ) func TestCheckCompatible(t *testing.T) { @@ -159,22 +160,21 @@ func TestConfigRules(t *testing.T) { } var stamp uint64 - if r := c.Rules(big.NewInt(0), stamp); r.IsSubnetEVM { + if r := modules.ChainConfigRules(c, big.NewInt(0), stamp); r.IsSubnetEVM { t.Errorf("expected %v to not be subnet-evm", stamp) } stamp = 500 - if r := c.Rules(big.NewInt(0), stamp); !r.IsSubnetEVM { + if r := modules.ChainConfigRules(c, big.NewInt(0), stamp); !r.IsSubnetEVM { t.Errorf("expected %v to be subnet-evm", stamp) } stamp = math.MaxInt64 - if r := c.Rules(big.NewInt(0), stamp); !r.IsSubnetEVM { + if r := modules.ChainConfigRules(c, big.NewInt(0), stamp); !r.IsSubnetEVM { t.Errorf("expected %v to be subnet-evm", stamp) } } func TestConfigUnmarshalJSON(t *testing.T) { require := require.New(t) - assert := assert.New(t) testRewardManagerConfig := rewardmanager.NewConfig( utils.NewUint64(1671542573), @@ -214,20 +214,12 @@ func TestConfigUnmarshalJSON(t *testing.T) { } } `) - c := ChainConfig{} - err := json.Unmarshal(config, &c) - require.NoError(err) + c := &ChainConfig{} + require.NoError(modules.UnmarshalChainConfigJSON(config, c)) require.Equal(c.ChainID, big.NewInt(43214)) require.Equal(c.AllowFeeRecipients, true) - for _, config := range []precompileconfig.Config{testRewardManagerConfig, testContractNativeMinterConfig} { - _, ok := c.LazyUnmarshalData[config.Key()] - assert.Truef(ok, "%T.LazyUnmarshalData[%q] not unmarshalled", c) - } - - return - rewardManagerConfig, ok := c.GenesisPrecompiles[rewardmanager.ConfigKey] require.True(ok) require.Equal(rewardManagerConfig.Key(), rewardmanager.ConfigKey) @@ -240,14 +232,13 @@ func TestConfigUnmarshalJSON(t *testing.T) { // Marshal and unmarshal again and check that the result is the same marshaled, err := json.Marshal(c) require.NoError(err) - c2 := ChainConfig{} - err = json.Unmarshal(marshaled, &c2) - require.NoError(err) + c2 := &ChainConfig{} + require.NoError(modules.UnmarshalChainConfigJSON(marshaled, c2)) require.Equal(c, c2) } func TestActivePrecompiles(t *testing.T) { - config := ChainConfig{ + config := &ChainConfig{ UpgradeConfig: UpgradeConfig{ PrecompileUpgrades: []PrecompileUpgrade{ { @@ -260,10 +251,10 @@ func TestActivePrecompiles(t *testing.T) { }, } - rules0 := config.Rules(common.Big0, 0) + rules0 := modules.ChainConfigRules(config, common.Big0, 0) require.True(t, rules0.IsPrecompileEnabled(nativeminter.Module.Address)) - rules1 := config.Rules(common.Big0, 1) + rules1 := modules.ChainConfigRules(config, common.Big0, 1) require.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address)) } @@ -334,7 +325,6 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { require.JSONEq(t, expectedJSON, string(result)) var unmarshalled ChainConfigWithUpgradesJSON - err = json.Unmarshal(result, &unmarshalled) - require.NoError(t, err) + require.NoError(t, modules.UnmarshalChainConfigJSON(result, &unmarshalled)) require.Equal(t, config, unmarshalled) } diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index 4e2c287241..32ca5dd061 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -1,7 +1,7 @@ // (c) 2022 Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package params +package params_test import ( "encoding/json" @@ -14,9 +14,12 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" + + . "github.com/ava-labs/subnet-evm/params" ) func TestVerifyWithChainConfig(t *testing.T) { @@ -72,7 +75,7 @@ func TestVerifyWithChainConfigAtNilTimestamp(t *testing.T) { // this does NOT enable the precompile, so it should be upgradeable. {Config: txallowlist.NewConfig(nil, nil, nil, nil)}, } - require.False(t, config.IsPrecompileEnabled(txallowlist.ContractAddress, 0)) // check the precompile is not enabled. + require.False(t, modules.IsPrecompileEnabled(config, txallowlist.ContractAddress, 0)) // check the precompile is not enabled. config.PrecompileUpgrades = []PrecompileUpgrade{ { // enable TxAllowList at timestamp 5 @@ -270,16 +273,16 @@ func TestGetPrecompileConfig(t *testing.T) { deployerallowlist.ConfigKey: deployerallowlist.NewConfig(utils.NewUint64(10), nil, nil, nil), } - deployerConfig := config.getActivePrecompileConfig(deployerallowlist.ContractAddress, 0) + deployerConfig := modules.GetActivePrecompileConfig(config, deployerallowlist.ContractAddress, 0) require.Nil(deployerConfig) - deployerConfig = config.getActivePrecompileConfig(deployerallowlist.ContractAddress, 10) + deployerConfig = modules.GetActivePrecompileConfig(config, deployerallowlist.ContractAddress, 10) require.NotNil(deployerConfig) - deployerConfig = config.getActivePrecompileConfig(deployerallowlist.ContractAddress, 11) + deployerConfig = modules.GetActivePrecompileConfig(config, deployerallowlist.ContractAddress, 11) require.NotNil(deployerConfig) - txAllowListConfig := config.getActivePrecompileConfig(txallowlist.ContractAddress, 0) + txAllowListConfig := modules.GetActivePrecompileConfig(config, txallowlist.ContractAddress, 0) require.Nil(txAllowListConfig) } @@ -288,6 +291,12 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { upgradeBytes := []byte(` { + "networkUpgradeOverrides": { + "durangoTimestamp": 314159 + }, + "stateUpgrades": [ + {"blockTimestamp": 142857} + ], "precompileUpgrades": [ { "rewardManagerConfig": { @@ -311,8 +320,7 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { `) var upgradeConfig UpgradeConfig - err := json.Unmarshal(upgradeBytes, &upgradeConfig) - require.NoError(err) + require.NoError(modules.UnmarshalUpgradeConfigJSON(upgradeBytes, &upgradeConfig)) require.Len(upgradeConfig.PrecompileUpgrades, 2) @@ -337,7 +345,6 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { upgradeBytes2, err := json.Marshal(upgradeConfig) require.NoError(err) var upgradeConfig2 UpgradeConfig - err = json.Unmarshal(upgradeBytes2, &upgradeConfig2) - require.NoError(err) + require.NoError(modules.UnmarshalUpgradeConfigJSON(upgradeBytes2, &upgradeConfig2)) require.Equal(upgradeConfig, upgradeConfig2) } diff --git a/params/precompile_upgrade_test.go b/params/precompile_upgrade_test.go index 8384ef4279..72bb1856dd 100644 --- a/params/precompile_upgrade_test.go +++ b/params/precompile_upgrade_test.go @@ -1,7 +1,7 @@ // (c) 2022, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package params +package params_test import ( "testing" @@ -11,6 +11,8 @@ import ( "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" + + . "github.com/ava-labs/subnet-evm/params" ) func TestVerifyUpgradeConfig(t *testing.T) { @@ -274,12 +276,14 @@ type upgradeCompatibilityTest struct { } func (tt *upgradeCompatibilityTest) run(t *testing.T, chainConfig ChainConfig) { + t.Skip("DO NOT MERGE") // apply all the upgrade bytes specified in order for i, upgrade := range tt.configs { newCfg := chainConfig newCfg.UpgradeConfig = *upgrade - err := chainConfig.checkCompatible(&newCfg, nil, tt.startTimestamps[i]) + // TODO: decide on correct height as was originally nil when calling internal method; ?math.MaxUint64 + err := chainConfig.CheckCompatible(&newCfg, 0, tt.startTimestamps[i]) // if this is not the final upgradeBytes, continue applying // the next upgradeBytes. (only check the result on the last apply) diff --git a/params/state_upgrade_test.go b/params/state_upgrade_test.go index 6ee4094fc0..91f22b5da7 100644 --- a/params/state_upgrade_test.go +++ b/params/state_upgrade_test.go @@ -1,17 +1,19 @@ // (c) 2022, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package params +package params_test import ( - "encoding/json" "math/big" "testing" + "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" "github.com/stretchr/testify/require" + + . "github.com/ava-labs/subnet-evm/params" ) func TestVerifyStateUpgrades(t *testing.T) { @@ -181,7 +183,6 @@ func TestUnmarshalStateUpgradeJSON(t *testing.T) { }, } var unmarshaledConfig UpgradeConfig - err := json.Unmarshal(jsonBytes, &unmarshaledConfig) - require.NoError(t, err) + require.NoError(t, modules.UnmarshalUpgradeConfigJSON(jsonBytes, &unmarshaledConfig)) require.Equal(t, upgradeConfig, unmarshaledConfig) } diff --git a/precompile/modules/chain_config.go b/precompile/modules/chain_config.go index 2eee3bc028..01db7c850c 100644 --- a/precompile/modules/chain_config.go +++ b/precompile/modules/chain_config.go @@ -4,6 +4,9 @@ import ( "encoding/json" "errors" "fmt" + "math/big" + "reflect" + "strings" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" @@ -11,11 +14,51 @@ import ( "github.com/ethereum/go-ethereum/common" ) -func InitChainConfig(c *params.ChainConfig) error { - if c.GenesisPrecompiles == nil { - c.GenesisPrecompiles = make(params.Precompiles) +var TestChainRules = ChainConfigRules(params.TestChainConfig, new(big.Int), 0) + +type ChainConfig interface { + *params.ChainConfig | *params.ChainConfigWithUpgradesJSON +} + +func UnmarshalChainConfigJSON[T ChainConfig](data []byte, c T) error { + // Circumvent the custom UnmarshalJSON(), which always errors to force use of this function + type cc *params.ChainConfig + var ( + dest cc + upgrades *params.UpgradeConfig + upgradesKey string + ) + switch c := any(c).(type) { + case *params.ChainConfig: + dest = cc(c) + case *params.ChainConfigWithUpgradesJSON: + dest = cc(&c.ChainConfig) + + upgrades = &c.UpgradeConfig + fld, ok := reflect.TypeOf(c).Elem().FieldByName("UpgradeConfig") + if !ok { + return fmt.Errorf("TODO: DO NOT MERGE") + } + upgradesKey = strings.Split(fld.Tag.Get("json"), ",")[0] + } + if err := json.Unmarshal(data, dest); err != nil { + return err + } + + byKey := make(map[string]json.RawMessage) + if err := json.Unmarshal(data, &byKey); err != nil { + return err + } + if upgrades != nil && len(byKey[upgradesKey]) > 0 { + if err := UnmarshalUpgradeConfigJSON(byKey[upgradesKey], upgrades); err != nil { + return err + } + } + + if dest.GenesisPrecompiles == nil { + dest.GenesisPrecompiles = make(params.Precompiles) } - for key, raw := range c.LazyUnmarshalData { + for key, raw := range byKey { mod, ok := GetPrecompileModule(key) if !ok { continue @@ -25,19 +68,21 @@ func InitChainConfig(c *params.ChainConfig) error { if err := json.Unmarshal(raw, conf); err != nil { return fmt.Errorf("unmarshal %T: %v", conf, err) } - c.GenesisPrecompiles[key] = conf + dest.GenesisPrecompiles[key] = conf } return nil } -func InitChainRules(c *params.ChainConfig, r *params.Rules, timestamp uint64) { +func ChainConfigRules(c *params.ChainConfig, blockNum *big.Int, timestamp uint64) params.Rules { + r := c.RulesDoNotCallDirectly(blockNum, timestamp) + // Initialize the stateful precompiles that should be enabled at [blockTimestamp]. r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) r.Predicaters = make(map[common.Address]precompileconfig.Predicater) r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) for _, module := range RegisteredModules() { - if config := getActivePrecompileConfig(c, module.Address, timestamp); config != nil && !config.IsDisabled() { + if config := GetActivePrecompileConfig(c, module.Address, timestamp); config != nil && !config.IsDisabled() { r.ActivePrecompiles[module.Address] = config if predicater, ok := config.(precompileconfig.Predicater); ok { r.Predicaters[module.Address] = predicater @@ -47,9 +92,11 @@ func InitChainRules(c *params.ChainConfig, r *params.Rules, timestamp uint64) { } } } + + return r } -func getActivePrecompileConfig(c *params.ChainConfig, address common.Address, timestamp uint64) precompileconfig.Config { +func GetActivePrecompileConfig(c *params.ChainConfig, address common.Address, timestamp uint64) precompileconfig.Config { configs := GetActivatingPrecompileConfigs(c, address, nil, timestamp, c.PrecompileUpgrades) if len(configs) == 0 { return nil @@ -150,7 +197,7 @@ func newTimestampCompatError(what string, storedtime, newtime *uint64) *params.C func EnabledStatefulPrecompiles(c *params.ChainConfig, blockTimestamp uint64) params.Precompiles { statefulPrecompileConfigs := make(params.Precompiles) for _, module := range RegisteredModules() { - if config := getActivePrecompileConfig(c, module.Address, blockTimestamp); config != nil && !config.IsDisabled() { + if config := GetActivePrecompileConfig(c, module.Address, blockTimestamp); config != nil && !config.IsDisabled() { statefulPrecompileConfigs[module.ConfigKey] = config } } @@ -158,17 +205,74 @@ func EnabledStatefulPrecompiles(c *params.ChainConfig, blockTimestamp uint64) pa return statefulPrecompileConfigs } -type PrecompileUpgrade struct { - precompileconfig.Config // TODO: DO NOT MERGE +func UnmarshalUpgradeConfigJSON(data []byte, u *params.UpgradeConfig) error { + raw := make(map[string]json.RawMessage) + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + var precompileJSON json.RawMessage + + skip := reflect.TypeOf([]params.PrecompileUpgrade{}) + v := reflect.ValueOf(u).Elem() + for i := 0; i < v.NumField(); i++ { + fld := v.Type().FieldByIndex([]int{i}) + fldVal := v.Field(i) + + tag, ok := fld.Tag.Lookup(`json`) + if !ok { + continue + } + msg, ok := raw[strings.Split(tag, ",")[0]] + if !ok { + continue + } + if fld.Type == skip { + precompileJSON = msg + continue + } + + var dest any + switch fldVal.Kind() { + case reflect.Pointer: + if fldVal.IsNil() { + fldVal.Set(reflect.New(fldVal.Type().Elem())) + } + dest = fldVal.Interface() + case reflect.Slice: + dest = fldVal.Addr().Interface() + default: + return fmt.Errorf("%s", fldVal.Kind()) // DO NOT MERGE + } + + if err := json.Unmarshal(msg, dest); err != nil { + return err + } + } + + if len(precompileJSON) > 0 { + var upgrades []precompileUpgrade + if err := json.Unmarshal(precompileJSON, &upgrades); err != nil { + return err + } + u.PrecompileUpgrades = make([]params.PrecompileUpgrade, len(upgrades)) + for i := range upgrades { + u.PrecompileUpgrades[i] = params.PrecompileUpgrade(upgrades[i]) + } + } + + return nil } +type precompileUpgrade params.PrecompileUpgrade + var errNoKey = errors.New("PrecompileUpgrade cannot be empty") // UnmarshalJSON unmarshals the json into the correct precompile config type // based on the key. Keys are defined in each precompile module, and registered in // precompile/registry/registry.go. // Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key -func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error { +func (u *precompileUpgrade) UnmarshalJSON(data []byte) error { raw := make(map[string]json.RawMessage) if err := json.Unmarshal(data, &raw); err != nil { return err @@ -193,16 +297,8 @@ func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error { return nil } -// MarshalJSON marshal the precompile config into json based on the precompile key. -// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key -func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { - res := make(map[string]precompileconfig.Config) - res[u.Key()] = u.Config - return json.Marshal(res) -} - // IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. func IsPrecompileEnabled(c *params.ChainConfig, address common.Address, timestamp uint64) bool { - config := getActivePrecompileConfig(c, address, timestamp) + config := GetActivePrecompileConfig(c, address, timestamp) return config != nil && !config.IsDisabled() } From 574f91d3fa3e4e366f4752a630c55cec7460060e Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 31 Jul 2024 12:13:29 +0100 Subject: [PATCH 05/25] refactor: make `params.ChainConfig.Rules()` self-contained --- params/config.go | 42 +++++++++++++++++++-- params/config_test.go | 21 +++++++---- precompile/modules/chain_config.go | 60 ++++++++++++++---------------- 3 files changed, 80 insertions(+), 43 deletions(-) diff --git a/params/config.go b/params/config.go index 50493e29de..8b6d75d7f3 100644 --- a/params/config.go +++ b/params/config.go @@ -206,6 +206,8 @@ var ( GenesisPrecompiles: Precompiles{}, UpgradeConfig: UpgradeConfig{}, } + + TestChainRules = TestChainConfig.Rules(new(big.Int), 0) ) // ChainConfig is the core config which determines the blockchain settings. @@ -242,8 +244,9 @@ type ChainConfig struct { FeeConfig commontype.FeeConfig `json:"feeConfig"` // Set the configuration for the dynamic fee algorithm AllowFeeRecipients bool `json:"allowFeeRecipients,omitempty"` // Allows fees to be collected by block builders. - GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. - UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. + GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. + PrecompileAddresses map[string]common.Address `json:"-"` + UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. } // Description returns a human-readable description of ChainConfig. @@ -743,9 +746,42 @@ func (c *ChainConfig) rules(num *big.Int, timestamp uint64) Rules { // Rules returns the Avalanche modified rules to support Avalanche // network upgrades -func (c *ChainConfig) RulesDoNotCallDirectly(blockNum *big.Int, timestamp uint64) Rules { +func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { r := c.rules(blockNum, timestamp) r.AvalancheRules = c.GetAvalancheRules(timestamp) + + r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) + update := func(cfg precompileconfig.Config) { + if !utils.IsForkTransition(cfg.Timestamp(), nil, timestamp) { + return + } + + addr := c.PrecompileAddresses[cfg.Key()] + if cfg == nil || cfg.IsDisabled() { + delete(r.ActivePrecompiles, addr) + } else { + r.ActivePrecompiles[addr] = cfg + } + } + + for _, c := range c.GenesisPrecompiles { + update(c) + } + for _, u := range c.PrecompileUpgrades { + update(u) + } + + r.Predicaters = make(map[common.Address]precompileconfig.Predicater) + r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) + for addr, config := range r.ActivePrecompiles { + if p, ok := config.(precompileconfig.Predicater); ok { + r.Predicaters[addr] = p + } + if a, ok := config.(precompileconfig.Accepter); ok { + r.AccepterPrecompiles[addr] = a + } + } + return r } diff --git a/params/config_test.go b/params/config_test.go index 02cbd316e6..257588c420 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -40,6 +40,7 @@ import ( "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" . "github.com/ava-labs/subnet-evm/params" @@ -160,15 +161,15 @@ func TestConfigRules(t *testing.T) { } var stamp uint64 - if r := modules.ChainConfigRules(c, big.NewInt(0), stamp); r.IsSubnetEVM { + if r := c.Rules(big.NewInt(0), stamp); r.IsSubnetEVM { t.Errorf("expected %v to not be subnet-evm", stamp) } stamp = 500 - if r := modules.ChainConfigRules(c, big.NewInt(0), stamp); !r.IsSubnetEVM { + if r := c.Rules(big.NewInt(0), stamp); !r.IsSubnetEVM { t.Errorf("expected %v to be subnet-evm", stamp) } stamp = math.MaxInt64 - if r := modules.ChainConfigRules(c, big.NewInt(0), stamp); !r.IsSubnetEVM { + if r := c.Rules(big.NewInt(0), stamp); !r.IsSubnetEVM { t.Errorf("expected %v to be subnet-evm", stamp) } } @@ -239,6 +240,9 @@ func TestConfigUnmarshalJSON(t *testing.T) { func TestActivePrecompiles(t *testing.T) { config := &ChainConfig{ + PrecompileAddresses: map[string]common.Address{ + nativeminter.ConfigKey: nativeminter.ContractAddress, + }, UpgradeConfig: UpgradeConfig{ PrecompileUpgrades: []PrecompileUpgrade{ { @@ -251,11 +255,11 @@ func TestActivePrecompiles(t *testing.T) { }, } - rules0 := modules.ChainConfigRules(config, common.Big0, 0) - require.True(t, rules0.IsPrecompileEnabled(nativeminter.Module.Address)) + rules0 := config.Rules(common.Big0, 0) + assert.True(t, rules0.IsPrecompileEnabled(nativeminter.Module.Address)) - rules1 := modules.ChainConfigRules(config, common.Big0, 1) - require.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address)) + rules1 := config.Rules(common.Big0, 1) + assert.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address)) } func TestChainConfigMarshalWithUpgrades(t *testing.T) { @@ -278,6 +282,9 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { DurangoTimestamp: utils.NewUint64(0), }, GenesisPrecompiles: Precompiles{}, + PrecompileAddresses: map[string]common.Address{ + txallowlist.Module.ConfigKey: txallowlist.ContractAddress, + }, }, UpgradeConfig: UpgradeConfig{ PrecompileUpgrades: []PrecompileUpgrade{ diff --git a/precompile/modules/chain_config.go b/precompile/modules/chain_config.go index 01db7c850c..bb70851441 100644 --- a/precompile/modules/chain_config.go +++ b/precompile/modules/chain_config.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "math/big" "reflect" "strings" @@ -14,8 +13,6 @@ import ( "github.com/ethereum/go-ethereum/common" ) -var TestChainRules = ChainConfigRules(params.TestChainConfig, new(big.Int), 0) - type ChainConfig interface { *params.ChainConfig | *params.ChainConfigWithUpgradesJSON } @@ -45,6 +42,10 @@ func UnmarshalChainConfigJSON[T ChainConfig](data []byte, c T) error { return err } + if dest.PrecompileAddresses == nil { + dest.PrecompileAddresses = make(map[string]common.Address) + } + byKey := make(map[string]json.RawMessage) if err := json.Unmarshal(data, &byKey); err != nil { return err @@ -53,6 +54,16 @@ func UnmarshalChainConfigJSON[T ChainConfig](data []byte, c T) error { if err := UnmarshalUpgradeConfigJSON(byKey[upgradesKey], upgrades); err != nil { return err } + for _, u := range upgrades.PrecompileUpgrades { + mod, ok := GetPrecompileModule(u.Key()) + if !ok { + return fmt.Errorf("TODO: DO NOT MERGE") + } + if a, ok := dest.PrecompileAddresses[u.Key()]; ok && a != mod.Address { + return fmt.Errorf("TODO: DO NOT MERGE") + } + dest.PrecompileAddresses[u.Key()] = mod.Address + } } if dest.GenesisPrecompiles == nil { @@ -69,39 +80,14 @@ func UnmarshalChainConfigJSON[T ChainConfig](data []byte, c T) error { return fmt.Errorf("unmarshal %T: %v", conf, err) } dest.GenesisPrecompiles[key] = conf - } - return nil -} - -func ChainConfigRules(c *params.ChainConfig, blockNum *big.Int, timestamp uint64) params.Rules { - r := c.RulesDoNotCallDirectly(blockNum, timestamp) - // Initialize the stateful precompiles that should be enabled at [blockTimestamp]. - r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) - r.Predicaters = make(map[common.Address]precompileconfig.Predicater) - r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) - - for _, module := range RegisteredModules() { - if config := GetActivePrecompileConfig(c, module.Address, timestamp); config != nil && !config.IsDisabled() { - r.ActivePrecompiles[module.Address] = config - if predicater, ok := config.(precompileconfig.Predicater); ok { - r.Predicaters[module.Address] = predicater - } - if precompileAccepter, ok := config.(precompileconfig.Accepter); ok { - r.AccepterPrecompiles[module.Address] = precompileAccepter - } + if a, ok := dest.PrecompileAddresses[key]; ok && a != mod.Address { + return fmt.Errorf("TODO: DO NOT MERGE") } + dest.PrecompileAddresses[key] = mod.Address } - return r -} - -func GetActivePrecompileConfig(c *params.ChainConfig, address common.Address, timestamp uint64) precompileconfig.Config { - configs := GetActivatingPrecompileConfigs(c, address, nil, timestamp, c.PrecompileUpgrades) - if len(configs) == 0 { - return nil - } - return configs[len(configs)-1] // return the most recent config + return nil } func GetActivatingPrecompileConfigs(c *params.ChainConfig, address common.Address, from *uint64, to uint64, upgrades []params.PrecompileUpgrade) []precompileconfig.Config { @@ -110,7 +96,7 @@ func GetActivatingPrecompileConfigs(c *params.ChainConfig, address common.Addres if !ok { return nil } - configs := make([]precompileconfig.Config, 0) + var configs []precompileconfig.Config key := module.ConfigKey // First check the embedded [upgrade] for precompiles configured // in the genesis chain config. @@ -131,6 +117,14 @@ func GetActivatingPrecompileConfigs(c *params.ChainConfig, address common.Addres return configs } +func GetActivePrecompileConfig(c *params.ChainConfig, address common.Address, timestamp uint64) precompileconfig.Config { + configs := GetActivatingPrecompileConfigs(c, address, nil, timestamp, c.PrecompileUpgrades) + if len(configs) == 0 { + return nil + } + return configs[len(configs)-1] // return the most recent config +} + // CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. // Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from // [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. From 2722fb8943f030e4605d561d57084bf201b9da87 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 31 Jul 2024 12:14:22 +0100 Subject: [PATCH 06/25] chore: remove unnecessary `nil` check --- params/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/config.go b/params/config.go index 8b6d75d7f3..793c983554 100644 --- a/params/config.go +++ b/params/config.go @@ -757,7 +757,7 @@ func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { } addr := c.PrecompileAddresses[cfg.Key()] - if cfg == nil || cfg.IsDisabled() { + if cfg.IsDisabled() { delete(r.ActivePrecompiles, addr) } else { r.ActivePrecompiles[addr] = cfg From 87b3b8fc066c972b7b6c0529edd90b0794bd49dd Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 31 Jul 2024 14:40:56 +0100 Subject: [PATCH 07/25] feat: `paramsjson` package for JSON unmarshalling `params.` that depend on `modules` --- params/config_test.go | 8 +- params/paramsjson/paramsjson.go | 184 +++++++++++++++++++++++++++++ params/precompile_config_test.go | 5 +- params/state_upgrade_test.go | 4 +- precompile/modules/chain_config.go | 173 --------------------------- 5 files changed, 193 insertions(+), 181 deletions(-) create mode 100644 params/paramsjson/paramsjson.go diff --git a/params/config_test.go b/params/config_test.go index 257588c420..ad87964161 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -34,10 +34,10 @@ import ( "testing" "time" + "github.com/ava-labs/subnet-evm/params/paramsjson" "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" @@ -216,7 +216,7 @@ func TestConfigUnmarshalJSON(t *testing.T) { } `) c := &ChainConfig{} - require.NoError(modules.UnmarshalChainConfigJSON(config, c)) + require.NoError(paramsjson.Unmarshal(config, c)) require.Equal(c.ChainID, big.NewInt(43214)) require.Equal(c.AllowFeeRecipients, true) @@ -234,7 +234,7 @@ func TestConfigUnmarshalJSON(t *testing.T) { marshaled, err := json.Marshal(c) require.NoError(err) c2 := &ChainConfig{} - require.NoError(modules.UnmarshalChainConfigJSON(marshaled, c2)) + require.NoError(paramsjson.Unmarshal(marshaled, c2)) require.Equal(c, c2) } @@ -332,6 +332,6 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { require.JSONEq(t, expectedJSON, string(result)) var unmarshalled ChainConfigWithUpgradesJSON - require.NoError(t, modules.UnmarshalChainConfigJSON(result, &unmarshalled)) + require.NoError(t, paramsjson.Unmarshal(result, &unmarshalled)) require.Equal(t, config, unmarshalled) } diff --git a/params/paramsjson/paramsjson.go b/params/paramsjson/paramsjson.go new file mode 100644 index 0000000000..7048dede65 --- /dev/null +++ b/params/paramsjson/paramsjson.go @@ -0,0 +1,184 @@ +// Package paramsjson provides JSON unmarshalling for `params` types that depend +// on the `modules` package. This avoids `params` depending on `modules`, even +// transitively, which would result in a circular dependency. +// +// The [Unmarshal] function is a drop-in replacement for [json.Unmarshal]. +package paramsjson + +import ( + "encoding/json" + "fmt" + "reflect" + "strings" + "unsafe" + + "github.com/ava-labs/subnet-evm/params" + "github.com/ava-labs/subnet-evm/precompile/modules" + "github.com/ethereum/go-ethereum/common" +) + +// An Unmarshaler can have JSON unmarshalled into it by [Unmarshal]. +type Unmarshaler interface { + *params.ChainConfig | *params.ChainConfigWithUpgradesJSON | *params.UpgradeConfig | *params.PrecompileUpgrade +} + +// Unmarshal is a drop-in replacement for [json.Unmarshal]. +func Unmarshal[T Unmarshaler](data []byte, v T) error { + switch v := any(v).(type) { + case *params.ChainConfig: + return unmarshalChainConfig(data, v, nil, "") + + case *params.ChainConfigWithUpgradesJSON: + const fldName = "UpgradeConfig" + + tStruct := reflect.TypeOf(v).Elem() + fld, ok := tStruct.FieldByName(fldName) + if !ok { + // If this happens then the constant `fldName` is of a different name to the actual struct field used below. + return fmt.Errorf("BUG: %T(%v).FieldByName(%q) returned false", tStruct, tStruct, fldName) + } + return unmarshalChainConfig(data, &v.ChainConfig, &v.UpgradeConfig, strings.Split(fld.Tag.Get("json"), ",")[0]) + + case *params.UpgradeConfig: + return unmarshalUpgradeConfig(data, v) + + case *params.PrecompileUpgrade: + return json.Unmarshal(data, (*precompileUpgrade)(v)) + + default: + // If this happens then the Unmarshaler interface has been modified but the above cases haven't been. + return fmt.Errorf("unsupported type %T", v) + } +} + +func unmarshalChainConfig(data []byte, cc *params.ChainConfig, upgrades *params.UpgradeConfig, upgradesJSONField string) error { + type withoutMethods *params.ChainConfig // circumvents UnmarshalJSON() method, which always returns an error + if err := json.Unmarshal(data, withoutMethods(cc)); err != nil { + return err + } + + byField := make(map[string]json.RawMessage) + if err := json.Unmarshal(data, &byField); err != nil { + return err + } + + if cc.GenesisPrecompiles == nil { + cc.GenesisPrecompiles = make(params.Precompiles) + } + if cc.PrecompileAddresses == nil { + cc.PrecompileAddresses = make(map[string]common.Address) + } + for fld, buf := range byField { + switch mod, ok := modules.GetPrecompileModule(fld); { + case ok: + conf := mod.MakeConfig() + if err := json.Unmarshal(buf, conf); err != nil { + return err + } + cc.GenesisPrecompiles[mod.ConfigKey] = conf + if err := addPrecompileAddress(cc, mod); err != nil { + return err + } + + case fld == upgradesJSONField && upgrades != nil: + if err := unmarshalUpgradeConfig(buf, upgrades); err != nil { + return err + } + for _, u := range upgrades.PrecompileUpgrades { + if err := addPrecompileAddressByKey(cc, u.Key()); err != nil { + return err + } + } + } + } + + return nil +} + +func addPrecompileAddressByKey(cc *params.ChainConfig, key string) error { + mod, ok := modules.GetPrecompileModule(key) + if !ok { + return fmt.Errorf("module %q not found", key) + } + return addPrecompileAddress(cc, mod) +} + +func addPrecompileAddress(cc *params.ChainConfig, mod modules.Module) error { + key := mod.ConfigKey + if a, ok := cc.PrecompileAddresses[key]; ok && a != mod.Address { + return fmt.Errorf("conflicting addresses for module %q: %v and %v", key, a, mod.Address) + } + cc.PrecompileAddresses[key] = mod.Address + return nil +} + +func unmarshalUpgradeConfig(data []byte, uc *params.UpgradeConfig) error { + byField := make(map[string]json.RawMessage) + if err := json.Unmarshal(data, &byField); err != nil { + return err + } + + precompileT := reflect.TypeOf([]params.PrecompileUpgrade{}) + + config := reflect.ValueOf(uc).Elem() + for i := 0; i < config.NumField(); i++ { + fld := config.Type().FieldByIndex([]int{i}) + jsonFld := strings.Split(fld.Tag.Get("json"), ",")[0] + if _, ok := byField[jsonFld]; !ok { + continue + } + + var jsonInto any + switch fldVal := config.Field(i); { + case fld.Type == precompileT: + var out []precompileUpgrade + jsonInto = &out + defer func() { + uc.PrecompileUpgrades = *(*[]params.PrecompileUpgrade)(unsafe.Pointer(&out)) + }() + + case fld.Type.Kind() == reflect.Slice: + jsonInto = fldVal.Addr().Interface() + + case fld.Type.Kind() == reflect.Pointer: + if fldVal.IsNil() { + fldVal.Set(reflect.New(fld.Type.Elem())) + } + jsonInto = fldVal.Interface() + } + + if err := json.Unmarshal(byField[jsonFld], jsonInto); err != nil { + return fmt.Errorf("json.Unmarshal field %q: %v", jsonFld, err) + } + } + + return nil +} + +type precompileUpgrade params.PrecompileUpgrade + +var _ json.Unmarshaler = (*precompileUpgrade)(nil) + +func (u *precompileUpgrade) UnmarshalJSON(data []byte) error { + byField := make(map[string]json.RawMessage) + if err := json.Unmarshal(data, &byField); err != nil { + return err + } + if n := len(byField); n != 1 { + return fmt.Errorf("unmarshalling %T; got %d JSON fields; MUST be exactly one (name of precompile module)", ¶ms.PrecompileUpgrade{}, n) + } + + for key, value := range byField { + mod, ok := modules.GetPrecompileModule(key) + if !ok { + return fmt.Errorf("unknown precompile config: %s", key) + } + config := mod.MakeConfig() + if err := json.Unmarshal(value, config); err != nil { + return err + } + u.Config = config + + return nil + } +} diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index 32ca5dd061..bb0351d461 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/ava-labs/subnet-evm/commontype" + "github.com/ava-labs/subnet-evm/params/paramsjson" "github.com/ava-labs/subnet-evm/precompile/contracts/deployerallowlist" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" @@ -320,7 +321,7 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { `) var upgradeConfig UpgradeConfig - require.NoError(modules.UnmarshalUpgradeConfigJSON(upgradeBytes, &upgradeConfig)) + require.NoError(paramsjson.Unmarshal(upgradeBytes, &upgradeConfig)) require.Len(upgradeConfig.PrecompileUpgrades, 2) @@ -345,6 +346,6 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { upgradeBytes2, err := json.Marshal(upgradeConfig) require.NoError(err) var upgradeConfig2 UpgradeConfig - require.NoError(modules.UnmarshalUpgradeConfigJSON(upgradeBytes2, &upgradeConfig2)) + require.NoError(paramsjson.Unmarshal(upgradeBytes2, &upgradeConfig2)) require.Equal(upgradeConfig, upgradeConfig2) } diff --git a/params/state_upgrade_test.go b/params/state_upgrade_test.go index 91f22b5da7..d651d18bb7 100644 --- a/params/state_upgrade_test.go +++ b/params/state_upgrade_test.go @@ -7,7 +7,7 @@ import ( "math/big" "testing" - "github.com/ava-labs/subnet-evm/precompile/modules" + "github.com/ava-labs/subnet-evm/params/paramsjson" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" @@ -183,6 +183,6 @@ func TestUnmarshalStateUpgradeJSON(t *testing.T) { }, } var unmarshaledConfig UpgradeConfig - require.NoError(t, modules.UnmarshalUpgradeConfigJSON(jsonBytes, &unmarshaledConfig)) + require.NoError(t, paramsjson.Unmarshal(jsonBytes, &unmarshaledConfig)) require.Equal(t, upgradeConfig, unmarshaledConfig) } diff --git a/precompile/modules/chain_config.go b/precompile/modules/chain_config.go index bb70851441..5178779238 100644 --- a/precompile/modules/chain_config.go +++ b/precompile/modules/chain_config.go @@ -1,11 +1,7 @@ package modules import ( - "encoding/json" - "errors" "fmt" - "reflect" - "strings" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" @@ -13,83 +9,6 @@ import ( "github.com/ethereum/go-ethereum/common" ) -type ChainConfig interface { - *params.ChainConfig | *params.ChainConfigWithUpgradesJSON -} - -func UnmarshalChainConfigJSON[T ChainConfig](data []byte, c T) error { - // Circumvent the custom UnmarshalJSON(), which always errors to force use of this function - type cc *params.ChainConfig - var ( - dest cc - upgrades *params.UpgradeConfig - upgradesKey string - ) - switch c := any(c).(type) { - case *params.ChainConfig: - dest = cc(c) - case *params.ChainConfigWithUpgradesJSON: - dest = cc(&c.ChainConfig) - - upgrades = &c.UpgradeConfig - fld, ok := reflect.TypeOf(c).Elem().FieldByName("UpgradeConfig") - if !ok { - return fmt.Errorf("TODO: DO NOT MERGE") - } - upgradesKey = strings.Split(fld.Tag.Get("json"), ",")[0] - } - if err := json.Unmarshal(data, dest); err != nil { - return err - } - - if dest.PrecompileAddresses == nil { - dest.PrecompileAddresses = make(map[string]common.Address) - } - - byKey := make(map[string]json.RawMessage) - if err := json.Unmarshal(data, &byKey); err != nil { - return err - } - if upgrades != nil && len(byKey[upgradesKey]) > 0 { - if err := UnmarshalUpgradeConfigJSON(byKey[upgradesKey], upgrades); err != nil { - return err - } - for _, u := range upgrades.PrecompileUpgrades { - mod, ok := GetPrecompileModule(u.Key()) - if !ok { - return fmt.Errorf("TODO: DO NOT MERGE") - } - if a, ok := dest.PrecompileAddresses[u.Key()]; ok && a != mod.Address { - return fmt.Errorf("TODO: DO NOT MERGE") - } - dest.PrecompileAddresses[u.Key()] = mod.Address - } - } - - if dest.GenesisPrecompiles == nil { - dest.GenesisPrecompiles = make(params.Precompiles) - } - for key, raw := range byKey { - mod, ok := GetPrecompileModule(key) - if !ok { - continue - } - - conf := mod.MakeConfig() - if err := json.Unmarshal(raw, conf); err != nil { - return fmt.Errorf("unmarshal %T: %v", conf, err) - } - dest.GenesisPrecompiles[key] = conf - - if a, ok := dest.PrecompileAddresses[key]; ok && a != mod.Address { - return fmt.Errorf("TODO: DO NOT MERGE") - } - dest.PrecompileAddresses[key] = mod.Address - } - - return nil -} - func GetActivatingPrecompileConfigs(c *params.ChainConfig, address common.Address, from *uint64, to uint64, upgrades []params.PrecompileUpgrade) []precompileconfig.Config { // Get key from address. module, ok := GetPrecompileModuleByAddress(address) @@ -199,98 +118,6 @@ func EnabledStatefulPrecompiles(c *params.ChainConfig, blockTimestamp uint64) pa return statefulPrecompileConfigs } -func UnmarshalUpgradeConfigJSON(data []byte, u *params.UpgradeConfig) error { - raw := make(map[string]json.RawMessage) - if err := json.Unmarshal(data, &raw); err != nil { - return err - } - - var precompileJSON json.RawMessage - - skip := reflect.TypeOf([]params.PrecompileUpgrade{}) - v := reflect.ValueOf(u).Elem() - for i := 0; i < v.NumField(); i++ { - fld := v.Type().FieldByIndex([]int{i}) - fldVal := v.Field(i) - - tag, ok := fld.Tag.Lookup(`json`) - if !ok { - continue - } - msg, ok := raw[strings.Split(tag, ",")[0]] - if !ok { - continue - } - if fld.Type == skip { - precompileJSON = msg - continue - } - - var dest any - switch fldVal.Kind() { - case reflect.Pointer: - if fldVal.IsNil() { - fldVal.Set(reflect.New(fldVal.Type().Elem())) - } - dest = fldVal.Interface() - case reflect.Slice: - dest = fldVal.Addr().Interface() - default: - return fmt.Errorf("%s", fldVal.Kind()) // DO NOT MERGE - } - - if err := json.Unmarshal(msg, dest); err != nil { - return err - } - } - - if len(precompileJSON) > 0 { - var upgrades []precompileUpgrade - if err := json.Unmarshal(precompileJSON, &upgrades); err != nil { - return err - } - u.PrecompileUpgrades = make([]params.PrecompileUpgrade, len(upgrades)) - for i := range upgrades { - u.PrecompileUpgrades[i] = params.PrecompileUpgrade(upgrades[i]) - } - } - - return nil -} - -type precompileUpgrade params.PrecompileUpgrade - -var errNoKey = errors.New("PrecompileUpgrade cannot be empty") - -// UnmarshalJSON unmarshals the json into the correct precompile config type -// based on the key. Keys are defined in each precompile module, and registered in -// precompile/registry/registry.go. -// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key -func (u *precompileUpgrade) UnmarshalJSON(data []byte) error { - raw := make(map[string]json.RawMessage) - if err := json.Unmarshal(data, &raw); err != nil { - return err - } - if len(raw) == 0 { - return errNoKey - } - if len(raw) > 1 { - return fmt.Errorf("PrecompileUpgrade must have exactly one key, got %d", len(raw)) - } - for key, value := range raw { - module, ok := GetPrecompileModule(key) - if !ok { - return fmt.Errorf("unknown precompile config: %s", key) - } - config := module.MakeConfig() - if err := json.Unmarshal(value, config); err != nil { - return err - } - u.Config = config - } - return nil -} - // IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. func IsPrecompileEnabled(c *params.ChainConfig, address common.Address, timestamp uint64) bool { config := GetActivePrecompileConfig(c, address, timestamp) From 38996086241ebde63b025de74182cc56e2aad7a4 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 1 Aug 2024 08:48:01 +0100 Subject: [PATCH 08/25] chore: revert change to `params.TestRules` --- params/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/config.go b/params/config.go index 793c983554..542716ebc4 100644 --- a/params/config.go +++ b/params/config.go @@ -207,7 +207,7 @@ var ( UpgradeConfig: UpgradeConfig{}, } - TestChainRules = TestChainConfig.Rules(new(big.Int), 0) + TestRules = TestChainConfig.Rules(new(big.Int), 0) ) // ChainConfig is the core config which determines the blockchain settings. From 1f587b49970a5684cd70f461d326199829fde619 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 5 Aug 2024 14:26:48 +0100 Subject: [PATCH 09/25] refactor: inject `paramsjson.Unmarshal()` as `UnmarshalJSON()` methods in `params` --- params/config_extra.go | 38 -------------------- params/json.go | 60 +++++++++++++++++++++++++++++++ params/paramsjson/paramsjson.go | 12 +++++-- plugin/evm/static_service_test.go | 2 ++ 4 files changed, 72 insertions(+), 40 deletions(-) create mode 100644 params/json.go diff --git a/params/config_extra.go b/params/config_extra.go index 94a249ac33..a579cfff1a 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -6,7 +6,6 @@ package params import ( "encoding/json" "errors" - "fmt" "math/big" "github.com/ava-labs/avalanchego/snow" @@ -33,14 +32,6 @@ type AvalancheContext struct { SnowCtx *snow.Context } -func (u *UpgradeConfig) UnmarshalJSON([]byte) error { - return fmt.Errorf("do not unmarshal %T JSON directly; use modules.UnmarshalUpgradeConfigJSON()", u) -} - -func (u *PrecompileUpgrade) UnmarshalJSON([]byte) error { - return fmt.Errorf("do not unmarshal %T JSON directly; use modules.UnmarshalPrecompileUpgradeJSON()", u) -} - // MarshalJSON marshal the precompile config into json based on the precompile key. // Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { @@ -49,16 +40,6 @@ func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { return json.Marshal(res) } -// UnmarshalJSON parses the JSON-encoded data and stores the result in the -// object pointed to by c. -// This is a custom unmarshaler to handle the Precompiles field. -// Precompiles was presented as an inline object in the JSON. -// This custom unmarshaler ensures backwards compatibility with the old format. -// TODO(arr4n) update this method comment DO NOT MERGE -func (c *ChainConfig) UnmarshalJSON([]byte) error { - return fmt.Errorf("do not unmarshal %T JSON directly; use modules.UnmarshalChainConfigJSON()", c) -} - // MarshalJSON returns the JSON encoding of c. // This is a custom marshaler to handle the Precompiles field. func (c *ChainConfig) MarshalJSON() ([]byte, error) { @@ -130,25 +111,6 @@ func (cu ChainConfigWithUpgradesJSON) MarshalJSON() ([]byte, error) { return mergedJSON, nil } -func (cu *ChainConfigWithUpgradesJSON) UnmarshalJSON(input []byte) error { - var cc ChainConfig - if err := json.Unmarshal(input, &cc); err != nil { - return err - } - - type upgrades struct { - UpgradeConfig UpgradeConfig `json:"upgrades"` - } - - var u upgrades - if err := json.Unmarshal(input, &u); err != nil { - return err - } - cu.ChainConfig = cc - cu.UpgradeConfig = u.UpgradeConfig - return nil -} - // ToWithUpgradesJSON converts the ChainConfig to ChainConfigWithUpgradesJSON with upgrades explicitly displayed. // ChainConfig does not include upgrades in its JSON output. // This is a workaround for showing upgrades in the JSON output. diff --git a/params/json.go b/params/json.go new file mode 100644 index 0000000000..5e9e4f4e59 --- /dev/null +++ b/params/json.go @@ -0,0 +1,60 @@ +package params + +import ( + "runtime" + "strings" +) + +type JSONUnmarshaler[T any] func([]byte, T) error + +var jsonUmarshalers struct { + cc JSONUnmarshaler[*ChainConfig] + cu JSONUnmarshaler[*ChainConfigWithUpgradesJSON] + uc JSONUnmarshaler[*UpgradeConfig] + pu JSONUnmarshaler[*PrecompileUpgrade] +} + +func RegisterJSONUnmarshalers( + cc JSONUnmarshaler[*ChainConfig], + cu JSONUnmarshaler[*ChainConfigWithUpgradesJSON], + uc JSONUnmarshaler[*UpgradeConfig], + pu JSONUnmarshaler[*PrecompileUpgrade], +) { + pc, _, _, ok := runtime.Caller(0) + if !ok { + _ = ok + } + if fn := runtime.FuncForPC(pc).Name(); !strings.HasPrefix(fn, "github.com/ava-labs/subnet-evm/params/paramsjson.") { + _ = fn + } + if jsonUmarshalers.cc != nil { + panic("JSON unmarshalers already registered") + } + + jsonUmarshalers.cc = cc + jsonUmarshalers.cu = cu + jsonUmarshalers.uc = uc + jsonUmarshalers.pu = pu +} + +// UnmarshalJSON parses the JSON-encoded data and stores the result in the +// object pointed to by c. +// This is a custom unmarshaler to handle the Precompiles field. +// Precompiles was presented as an inline object in the JSON. +// This custom unmarshaler ensures backwards compatibility with the old format. +// TODO(arr4n) update this method comment DO NOT MERGE +func (c *ChainConfig) UnmarshalJSON(data []byte) error { + return jsonUmarshalers.cc(data, c) +} + +func (cu *ChainConfigWithUpgradesJSON) UnmarshalJSON(data []byte) error { + return jsonUmarshalers.cu(data, cu) +} + +func (u *UpgradeConfig) UnmarshalJSON(data []byte) error { + return jsonUmarshalers.uc(data, u) +} + +func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error { + return jsonUmarshalers.pu(data, u) +} diff --git a/params/paramsjson/paramsjson.go b/params/paramsjson/paramsjson.go index 7048dede65..4e7f8c2fee 100644 --- a/params/paramsjson/paramsjson.go +++ b/params/paramsjson/paramsjson.go @@ -17,6 +17,12 @@ import ( "github.com/ethereum/go-ethereum/common" ) +func init() { + // The parameters of RegisterJSONUmarshalers are all generic but the + // specific types are unambiguous so we don't need to specify them here. + params.RegisterJSONUnmarshalers(Unmarshal, Unmarshal, Unmarshal, Unmarshal) +} + // An Unmarshaler can have JSON unmarshalled into it by [Unmarshal]. type Unmarshaler interface { *params.ChainConfig | *params.ChainConfigWithUpgradesJSON | *params.UpgradeConfig | *params.PrecompileUpgrade @@ -145,6 +151,9 @@ func unmarshalUpgradeConfig(data []byte, uc *params.UpgradeConfig) error { fldVal.Set(reflect.New(fld.Type.Elem())) } jsonInto = fldVal.Interface() + + default: + return fmt.Errorf("unsupported field %T.%s", uc, fld.Name) } if err := json.Unmarshal(byField[jsonFld], jsonInto); err != nil { @@ -178,7 +187,6 @@ func (u *precompileUpgrade) UnmarshalJSON(data []byte) error { return err } u.Config = config - - return nil } + return nil } diff --git a/plugin/evm/static_service_test.go b/plugin/evm/static_service_test.go index 626f8f30d3..3529b4908d 100644 --- a/plugin/evm/static_service_test.go +++ b/plugin/evm/static_service_test.go @@ -12,6 +12,8 @@ import ( "github.com/ava-labs/subnet-evm/core" "github.com/ava-labs/subnet-evm/params" "github.com/stretchr/testify/assert" + + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers custom unmarshalers without circular dependencies ) var testGenesisJSON = `{"config":{"chainId":43111,"homesteadBlock":0,"eip150Block":0,"eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"subnetEVMTimestamp":0},"nonce":"0x0","timestamp":"0x0","extraData":"0x00","gasLimit":"0x5f5e100","difficulty":"0x0","mixHash":"0x0000000000000000000000000000000000000000000000000000000000000000","coinbase":"0x0000000000000000000000000000000000000000","alloc":{"0100000000000000000000000000000000000000":{"code":"0x7300000000000000000000000000000000000000003014608060405260043610603d5760003560e01c80631e010439146042578063b6510bb314606e575b600080fd5b605c60048036036020811015605657600080fd5b503560b1565b60408051918252519081900360200190f35b818015607957600080fd5b5060af60048036036080811015608e57600080fd5b506001600160a01b03813516906020810135906040810135906060013560b6565b005b30cd90565b836001600160a01b031681836108fc8690811502906040516000604051808303818888878c8acf9550505050505015801560f4573d6000803e3d6000fd5b505050505056fea26469706673582212201eebce970fe3f5cb96bf8ac6ba5f5c133fc2908ae3dcd51082cfee8f583429d064736f6c634300060a0033","balance":"0x0"}},"number":"0x0","gasUsed":"0x0","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000"}` From e21cf4b1c056dac563be37c9710336229a8cac1f Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 5 Aug 2024 14:39:59 +0100 Subject: [PATCH 10/25] refactor: use regular `json.Unmarshal()` and blank `import _ .../paramsjson"` instead --- params/config_test.go | 8 ++++---- params/json.go | 26 ++++++++++++++++++++++---- params/precompile_config_test.go | 5 ++--- params/state_upgrade_test.go | 4 ++-- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/params/config_test.go b/params/config_test.go index ad87964161..e3a4b22dc7 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -34,7 +34,6 @@ import ( "testing" "time" - "github.com/ava-labs/subnet-evm/params/paramsjson" "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" @@ -44,6 +43,7 @@ import ( "github.com/stretchr/testify/require" . "github.com/ava-labs/subnet-evm/params" + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency with precompile/modules ) func TestCheckCompatible(t *testing.T) { @@ -216,7 +216,7 @@ func TestConfigUnmarshalJSON(t *testing.T) { } `) c := &ChainConfig{} - require.NoError(paramsjson.Unmarshal(config, c)) + require.NoError(json.Unmarshal(config, c)) require.Equal(c.ChainID, big.NewInt(43214)) require.Equal(c.AllowFeeRecipients, true) @@ -234,7 +234,7 @@ func TestConfigUnmarshalJSON(t *testing.T) { marshaled, err := json.Marshal(c) require.NoError(err) c2 := &ChainConfig{} - require.NoError(paramsjson.Unmarshal(marshaled, c2)) + require.NoError(json.Unmarshal(marshaled, c2)) require.Equal(c, c2) } @@ -332,6 +332,6 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { require.JSONEq(t, expectedJSON, string(result)) var unmarshalled ChainConfigWithUpgradesJSON - require.NoError(t, paramsjson.Unmarshal(result, &unmarshalled)) + require.NoError(t, json.Unmarshal(result, &unmarshalled)) require.Equal(t, config, unmarshalled) } diff --git a/params/json.go b/params/json.go index 5e9e4f4e59..25a7382ca9 100644 --- a/params/json.go +++ b/params/json.go @@ -1,10 +1,13 @@ package params import ( + "fmt" "runtime" "strings" ) +// A JSONUnmarshaler is a type-safe function for unmarshalling a JSON buffer +// into a specific type. type JSONUnmarshaler[T any] func([]byte, T) error var jsonUmarshalers struct { @@ -14,6 +17,14 @@ var jsonUmarshalers struct { pu JSONUnmarshaler[*PrecompileUpgrade] } +// RegisterJSONUnmarshalers registers the JSON unmarshalling functions for +// various types. This allows their unmarshalling behaviour to be injected by +// the [params/paramsjson] package, which can't be directly imported as it would +// result in a circular dependency. +// +// This function SHOULD NOT be called directly. Instead, blank import the +// [params/paramsjson] package, which registers unmarshalers in its init() +// function. func RegisterJSONUnmarshalers( cc JSONUnmarshaler[*ChainConfig], cu JSONUnmarshaler[*ChainConfigWithUpgradesJSON], @@ -37,6 +48,13 @@ func RegisterJSONUnmarshalers( jsonUmarshalers.pu = pu } +func unmarshalJSON[T any](u JSONUnmarshaler[T], data []byte, v T) error { + if u == nil { + return fmt.Errorf(`%T is nil; did you remember to import _ "github.com/ava-labs/subnet-evm/params/paramsjson"`, u) + } + return u(data, v) +} + // UnmarshalJSON parses the JSON-encoded data and stores the result in the // object pointed to by c. // This is a custom unmarshaler to handle the Precompiles field. @@ -44,17 +62,17 @@ func RegisterJSONUnmarshalers( // This custom unmarshaler ensures backwards compatibility with the old format. // TODO(arr4n) update this method comment DO NOT MERGE func (c *ChainConfig) UnmarshalJSON(data []byte) error { - return jsonUmarshalers.cc(data, c) + return unmarshalJSON(jsonUmarshalers.cc, data, c) } func (cu *ChainConfigWithUpgradesJSON) UnmarshalJSON(data []byte) error { - return jsonUmarshalers.cu(data, cu) + return unmarshalJSON(jsonUmarshalers.cu, data, cu) } func (u *UpgradeConfig) UnmarshalJSON(data []byte) error { - return jsonUmarshalers.uc(data, u) + return unmarshalJSON(jsonUmarshalers.uc, data, u) } func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error { - return jsonUmarshalers.pu(data, u) + return unmarshalJSON(jsonUmarshalers.pu, data, u) } diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index bb0351d461..179c994558 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/ava-labs/subnet-evm/commontype" - "github.com/ava-labs/subnet-evm/params/paramsjson" "github.com/ava-labs/subnet-evm/precompile/contracts/deployerallowlist" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" @@ -321,7 +320,7 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { `) var upgradeConfig UpgradeConfig - require.NoError(paramsjson.Unmarshal(upgradeBytes, &upgradeConfig)) + require.NoError(json.Unmarshal(upgradeBytes, &upgradeConfig)) require.Len(upgradeConfig.PrecompileUpgrades, 2) @@ -346,6 +345,6 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { upgradeBytes2, err := json.Marshal(upgradeConfig) require.NoError(err) var upgradeConfig2 UpgradeConfig - require.NoError(paramsjson.Unmarshal(upgradeBytes2, &upgradeConfig2)) + require.NoError(json.Unmarshal(upgradeBytes2, &upgradeConfig2)) require.Equal(upgradeConfig, upgradeConfig2) } diff --git a/params/state_upgrade_test.go b/params/state_upgrade_test.go index d651d18bb7..f0e272a8da 100644 --- a/params/state_upgrade_test.go +++ b/params/state_upgrade_test.go @@ -4,10 +4,10 @@ package params_test import ( + "encoding/json" "math/big" "testing" - "github.com/ava-labs/subnet-evm/params/paramsjson" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" @@ -183,6 +183,6 @@ func TestUnmarshalStateUpgradeJSON(t *testing.T) { }, } var unmarshaledConfig UpgradeConfig - require.NoError(t, paramsjson.Unmarshal(jsonBytes, &unmarshaledConfig)) + require.NoError(t, json.Unmarshal(jsonBytes, &unmarshaledConfig)) require.Equal(t, upgradeConfig, unmarshaledConfig) } From ae55c97785780966a41dd20b078378ff81051172 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 5 Aug 2024 14:56:30 +0100 Subject: [PATCH 11/25] fix: _ import `paramsjson` in tests --- eth/tracers/internal/tracetest/calltrace_test.go | 2 ++ params/config_test.go | 2 +- params/paramsjson/paramsjson.go | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/eth/tracers/internal/tracetest/calltrace_test.go b/eth/tracers/internal/tracetest/calltrace_test.go index 6c2aa1ab89..aac3434196 100644 --- a/eth/tracers/internal/tracetest/calltrace_test.go +++ b/eth/tracers/internal/tracetest/calltrace_test.go @@ -45,6 +45,8 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/rlp" + + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency ) type callContext struct { diff --git a/params/config_test.go b/params/config_test.go index e3a4b22dc7..a25b1975db 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -43,7 +43,7 @@ import ( "github.com/stretchr/testify/require" . "github.com/ava-labs/subnet-evm/params" - _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency with precompile/modules + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency ) func TestCheckCompatible(t *testing.T) { diff --git a/params/paramsjson/paramsjson.go b/params/paramsjson/paramsjson.go index 4e7f8c2fee..965edb571a 100644 --- a/params/paramsjson/paramsjson.go +++ b/params/paramsjson/paramsjson.go @@ -2,7 +2,8 @@ // on the `modules` package. This avoids `params` depending on `modules`, even // transitively, which would result in a circular dependency. // -// The [Unmarshal] function is a drop-in replacement for [json.Unmarshal]. +// Typically there is no need to call this package directly. It should instead +// be blank _ imported to register its unmarshallers, similarly to SQL drivers. package paramsjson import ( From 82073009292ca8670b637209759969b812ff1452 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 5 Aug 2024 14:58:32 +0100 Subject: [PATCH 12/25] chore: blank _ import `paramsjson` in `plugin/main.go` --- plugin/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin/main.go b/plugin/main.go index afdd416d2a..e5692d54e4 100644 --- a/plugin/main.go +++ b/plugin/main.go @@ -9,6 +9,8 @@ import ( "github.com/ava-labs/avalanchego/version" "github.com/ava-labs/subnet-evm/plugin/evm" "github.com/ava-labs/subnet-evm/plugin/runner" + + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency ) func main() { From 143317d39addfdba0b2839e9be1f95e667019563 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 5 Aug 2024 16:42:53 +0100 Subject: [PATCH 13/25] chore: blank _ import `paramsjson` in `tests/utils/subnet.go` --- params/config_extra.go | 1 - tests/utils/subnet.go | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/params/config_extra.go b/params/config_extra.go index a579cfff1a..6a321e0674 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -43,7 +43,6 @@ func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { // MarshalJSON returns the JSON encoding of c. // This is a custom marshaler to handle the Precompiles field. func (c *ChainConfig) MarshalJSON() ([]byte, error) { - // TODO refactor this (DO NOT MERGE) // Alias ChainConfig to avoid recursion diff --git a/tests/utils/subnet.go b/tests/utils/subnet.go index 6f7aea3500..e646f71f3b 100644 --- a/tests/utils/subnet.go +++ b/tests/utils/subnet.go @@ -25,6 +25,8 @@ import ( "github.com/go-cmd/cmd" "github.com/onsi/ginkgo/v2" "github.com/stretchr/testify/require" + + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency ) type SubnetSuite struct { From b19cc5cd5e956bf65ff72522023988fdfe0ac321 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Mon, 5 Aug 2024 20:20:18 +0100 Subject: [PATCH 14/25] refactor: `precompileconfig.Config.Address()` method instead of map of precompile addresses --- params/config.go | 10 +++---- params/config_test.go | 6 ---- params/paramsjson/paramsjson.go | 29 ------------------- params/precompiles.go | 1 - .../contracts/deployerallowlist/config.go | 2 ++ precompile/contracts/feemanager/config.go | 2 ++ precompile/contracts/nativeminter/config.go | 2 ++ precompile/contracts/rewardmanager/config.go | 2 ++ precompile/contracts/txallowlist/config.go | 4 ++- precompile/precompileconfig/config.go | 2 ++ 10 files changed, 17 insertions(+), 43 deletions(-) diff --git a/params/config.go b/params/config.go index 542716ebc4..6c24bb6887 100644 --- a/params/config.go +++ b/params/config.go @@ -244,9 +244,8 @@ type ChainConfig struct { FeeConfig commontype.FeeConfig `json:"feeConfig"` // Set the configuration for the dynamic fee algorithm AllowFeeRecipients bool `json:"allowFeeRecipients,omitempty"` // Allows fees to be collected by block builders. - GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. - PrecompileAddresses map[string]common.Address `json:"-"` - UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. + GenesisPrecompiles Precompiles `json:"-"` // Config for enabling precompiles from genesis. JSON encode/decode will be handled by the custom marshaler/unmarshaler. + UpgradeConfig `json:"-"` // Config specified in upgradeBytes (avalanche network upgrades or enable/disabling precompiles). Skip encoding/decoding directly into ChainConfig. } // Description returns a human-readable description of ChainConfig. @@ -756,11 +755,10 @@ func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { return } - addr := c.PrecompileAddresses[cfg.Key()] if cfg.IsDisabled() { - delete(r.ActivePrecompiles, addr) + delete(r.ActivePrecompiles, cfg.Address()) } else { - r.ActivePrecompiles[addr] = cfg + r.ActivePrecompiles[cfg.Address()] = cfg } } diff --git a/params/config_test.go b/params/config_test.go index a25b1975db..a433c09b5c 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -240,9 +240,6 @@ func TestConfigUnmarshalJSON(t *testing.T) { func TestActivePrecompiles(t *testing.T) { config := &ChainConfig{ - PrecompileAddresses: map[string]common.Address{ - nativeminter.ConfigKey: nativeminter.ContractAddress, - }, UpgradeConfig: UpgradeConfig{ PrecompileUpgrades: []PrecompileUpgrade{ { @@ -282,9 +279,6 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { DurangoTimestamp: utils.NewUint64(0), }, GenesisPrecompiles: Precompiles{}, - PrecompileAddresses: map[string]common.Address{ - txallowlist.Module.ConfigKey: txallowlist.ContractAddress, - }, }, UpgradeConfig: UpgradeConfig{ PrecompileUpgrades: []PrecompileUpgrade{ diff --git a/params/paramsjson/paramsjson.go b/params/paramsjson/paramsjson.go index 965edb571a..87d6b37ff9 100644 --- a/params/paramsjson/paramsjson.go +++ b/params/paramsjson/paramsjson.go @@ -15,7 +15,6 @@ import ( "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/modules" - "github.com/ethereum/go-ethereum/common" ) func init() { @@ -72,9 +71,6 @@ func unmarshalChainConfig(data []byte, cc *params.ChainConfig, upgrades *params. if cc.GenesisPrecompiles == nil { cc.GenesisPrecompiles = make(params.Precompiles) } - if cc.PrecompileAddresses == nil { - cc.PrecompileAddresses = make(map[string]common.Address) - } for fld, buf := range byField { switch mod, ok := modules.GetPrecompileModule(fld); { case ok: @@ -83,42 +79,17 @@ func unmarshalChainConfig(data []byte, cc *params.ChainConfig, upgrades *params. return err } cc.GenesisPrecompiles[mod.ConfigKey] = conf - if err := addPrecompileAddress(cc, mod); err != nil { - return err - } case fld == upgradesJSONField && upgrades != nil: if err := unmarshalUpgradeConfig(buf, upgrades); err != nil { return err } - for _, u := range upgrades.PrecompileUpgrades { - if err := addPrecompileAddressByKey(cc, u.Key()); err != nil { - return err - } - } } } return nil } -func addPrecompileAddressByKey(cc *params.ChainConfig, key string) error { - mod, ok := modules.GetPrecompileModule(key) - if !ok { - return fmt.Errorf("module %q not found", key) - } - return addPrecompileAddress(cc, mod) -} - -func addPrecompileAddress(cc *params.ChainConfig, mod modules.Module) error { - key := mod.ConfigKey - if a, ok := cc.PrecompileAddresses[key]; ok && a != mod.Address { - return fmt.Errorf("conflicting addresses for module %q: %v and %v", key, a, mod.Address) - } - cc.PrecompileAddresses[key] = mod.Address - return nil -} - func unmarshalUpgradeConfig(data []byte, uc *params.UpgradeConfig) error { byField := make(map[string]json.RawMessage) if err := json.Unmarshal(data, &byField); err != nil { diff --git a/params/precompiles.go b/params/precompiles.go index a3ca0a1b98..4c05bc7e3c 100644 --- a/params/precompiles.go +++ b/params/precompiles.go @@ -7,5 +7,4 @@ import ( "github.com/ava-labs/subnet-evm/precompile/precompileconfig" ) -// TODO(arr4n): can this be removed? DO NOT MERGE type Precompiles map[string]precompileconfig.Config diff --git a/precompile/contracts/deployerallowlist/config.go b/precompile/contracts/deployerallowlist/config.go index a588101dc3..c3cce52d79 100644 --- a/precompile/contracts/deployerallowlist/config.go +++ b/precompile/contracts/deployerallowlist/config.go @@ -44,6 +44,8 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { func (*Config) Key() string { return ConfigKey } +func (*Config) Address() common.Address { return ContractAddress } + // Equal returns true if [cfg] is a [*ContractDeployerAllowListConfig] and it has been configured identical to [c]. func (c *Config) Equal(cfg precompileconfig.Config) bool { // typecast before comparison diff --git a/precompile/contracts/feemanager/config.go b/precompile/contracts/feemanager/config.go index 9dcfc307d2..311d47a647 100644 --- a/precompile/contracts/feemanager/config.go +++ b/precompile/contracts/feemanager/config.go @@ -50,6 +50,8 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { // This should be the same key as used in the precompile module. func (*Config) Key() string { return ConfigKey } +func (*Config) Address() common.Address { return ContractAddress } + // Equal returns true if [cfg] is a [*FeeManagerConfig] and it has been configured identical to [c]. func (c *Config) Equal(cfg precompileconfig.Config) bool { // typecast before comparison diff --git a/precompile/contracts/nativeminter/config.go b/precompile/contracts/nativeminter/config.go index 38a65ee6c8..d222589a59 100644 --- a/precompile/contracts/nativeminter/config.go +++ b/precompile/contracts/nativeminter/config.go @@ -54,6 +54,8 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { // This should be the same key as used in the precompile module. func (*Config) Key() string { return ConfigKey } +func (*Config) Address() common.Address { return ContractAddress } + // Equal returns true if [cfg] is a [*ContractNativeMinterConfig] and it has been configured identical to [c]. func (c *Config) Equal(cfg precompileconfig.Config) bool { // typecast before comparison diff --git a/precompile/contracts/rewardmanager/config.go b/precompile/contracts/rewardmanager/config.go index 49949cac1a..aa9fc6bcec 100644 --- a/precompile/contracts/rewardmanager/config.go +++ b/precompile/contracts/rewardmanager/config.go @@ -90,6 +90,8 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { // This should be the same key as used in the precompile module. func (*Config) Key() string { return ConfigKey } +func (*Config) Address() common.Address { return ContractAddress } + // Verify tries to verify Config and returns an error accordingly. func (c *Config) Verify(chainConfig precompileconfig.ChainConfig) error { if c.InitialRewardConfig != nil { diff --git a/precompile/contracts/txallowlist/config.go b/precompile/contracts/txallowlist/config.go index f5656d9c78..e1449c1628 100644 --- a/precompile/contracts/txallowlist/config.go +++ b/precompile/contracts/txallowlist/config.go @@ -42,7 +42,9 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { } } -func (c *Config) Key() string { return ConfigKey } +func (*Config) Key() string { return ConfigKey } + +func (*Config) Address() common.Address { return ContractAddress } // Equal returns true if [cfg] is a [*TxAllowListConfig] and it has been configured identical to [c]. func (c *Config) Equal(cfg precompileconfig.Config) bool { diff --git a/precompile/precompileconfig/config.go b/precompile/precompileconfig/config.go index 05d204de45..58d4f73ab8 100644 --- a/precompile/precompileconfig/config.go +++ b/precompile/precompileconfig/config.go @@ -19,6 +19,8 @@ import ( type Config interface { // Key returns the unique key for the stateful precompile. Key() string + // Address returns the address at which the precompile should be made available. + Address() common.Address // Timestamp returns the timestamp at which this stateful precompile should be enabled. // 1) 0 indicates that the precompile should be enabled from genesis. // 2) n indicates that the precompile should be enabled in the first block with timestamp >= [n]. From 03c5f9d5c624ec4535f4227f2c859aee58410496 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 6 Aug 2024 12:56:42 +0100 Subject: [PATCH 15/25] refactor: move `params.ChainConfig` methods back from `modules` package --- core/blockchain_reader.go | 5 +- core/state_processor.go | 2 +- core/state_transition.go | 3 +- core/txpool/legacypool/legacypool.go | 3 +- eth/gasprice/gasprice.go | 3 +- internal/ethapi/api_extra.go | 3 +- params/config.go | 7 +- params/config_extra.go | 125 +++++++++++++++++++++++++++ precompile/contracts/warp/config.go | 2 + precompile/modules/chain_config.go | 125 --------------------------- 10 files changed, 137 insertions(+), 141 deletions(-) delete mode 100644 precompile/modules/chain_config.go diff --git a/core/blockchain_reader.go b/core/blockchain_reader.go index c9f15b5bed..554c20e730 100644 --- a/core/blockchain_reader.go +++ b/core/blockchain_reader.go @@ -40,7 +40,6 @@ import ( "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/trie" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/event" @@ -345,7 +344,7 @@ func (bc *BlockChain) SubscribeAcceptedTransactionEvent(ch chan<- NewTxsEvent) e // Assumes that a valid configuration is stored when the precompile is activated. func (bc *BlockChain) GetFeeConfigAt(parent *types.Header) (commontype.FeeConfig, *big.Int, error) { config := bc.Config() - if !modules.IsPrecompileEnabled(config, feemanager.ContractAddress, parent.Time) { + if !config.IsPrecompileEnabled(feemanager.ContractAddress, parent.Time) { return config.FeeConfig, common.Big0, nil } @@ -383,7 +382,7 @@ func (bc *BlockChain) GetCoinbaseAt(parent *types.Header) (common.Address, bool, return constants.BlackholeAddr, false, nil } - if !modules.IsPrecompileEnabled(config, rewardmanager.ContractAddress, parent.Time) { + if !config.IsPrecompileEnabled(rewardmanager.ContractAddress, parent.Time) { if bc.chainConfig.AllowFeeRecipients { return common.Address{}, true, nil } else { diff --git a/core/state_processor.go b/core/state_processor.go index e0183a8b9e..fb839cc187 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -215,7 +215,7 @@ func ApplyPrecompileActivations(c *params.ChainConfig, parentTimestamp *uint64, // This ensures even if precompiles read/write state other than their own they will observe // an identical global state in a deterministic order when they are configured. for _, module := range modules.RegisteredModules() { - for _, activatingConfig := range modules.GetActivatingPrecompileConfigs(c, module.Address, parentTimestamp, blockTimestamp, c.PrecompileUpgrades) { + for _, activatingConfig := range c.GetActivatingPrecompileConfigs(module.Address, parentTimestamp, blockTimestamp, c.PrecompileUpgrades) { // If this transition activates the upgrade, configure the stateful precompile. // (or deconfigure it if it is being disabled.) if activatingConfig.IsDisabled() { diff --git a/core/state_transition.go b/core/state_transition.go index 7e321c38dc..a71bacc192 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -36,7 +36,6 @@ import ( "github.com/ava-labs/subnet-evm/core/vm" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ava-labs/subnet-evm/vmerrs" "github.com/ethereum/go-ethereum/common" @@ -353,7 +352,7 @@ func (st *StateTransition) preCheck() error { } // Check that the sender is on the tx allow list if enabled - if modules.IsPrecompileEnabled(st.evm.ChainConfig(), txallowlist.ContractAddress, st.evm.Context.Time) { + if st.evm.ChainConfig().IsPrecompileEnabled(txallowlist.ContractAddress, st.evm.Context.Time) { txAllowListRole := txallowlist.GetTxAllowListStatus(st.state, msg.From) if !txAllowListRole.IsEnabled() { return fmt.Errorf("%w: %s", vmerrs.ErrSenderAddressNotAllowListed, msg.From) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 35b06872ee..669c483e9f 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -45,7 +45,6 @@ import ( "github.com/ava-labs/subnet-evm/metrics" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/prque" @@ -1498,7 +1497,7 @@ func (pool *LegacyPool) reset(oldHead, newHead *types.Header) { // when we reset txPool we should explicitly check if fee struct for min base fee has changed // so that we can correctly drop txs with < minBaseFee from tx pool. - if modules.IsPrecompileEnabled(pool.chainconfig, feemanager.ContractAddress, newHead.Time) { + if pool.chainconfig.IsPrecompileEnabled(feemanager.ContractAddress, newHead.Time) { feeConfig, _, err := pool.chain.GetFeeConfigAt(newHead) if err != nil { log.Error("Failed to get fee config state", "err", err, "root", newHead.Root) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index b9a8adf340..899c2eba0e 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -39,7 +39,6 @@ import ( "github.com/ava-labs/subnet-evm/core/types" "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile/contracts/feemanager" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/rpc" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/lru" @@ -319,7 +318,7 @@ func (oracle *Oracle) suggestDynamicFees(ctx context.Context) (*big.Int, *big.In feeLastChangedAt *big.Int feeConfig commontype.FeeConfig ) - if modules.IsPrecompileEnabled(oracle.backend.ChainConfig(), feemanager.ContractAddress, head.Time) { + if oracle.backend.ChainConfig().IsPrecompileEnabled(feemanager.ContractAddress, head.Time) { feeConfig, feeLastChangedAt, err = oracle.backend.GetFeeConfigAt(head) if err != nil { return nil, nil, err diff --git a/internal/ethapi/api_extra.go b/internal/ethapi/api_extra.go index 8bdce29ef9..5823ff3dc7 100644 --- a/internal/ethapi/api_extra.go +++ b/internal/ethapi/api_extra.go @@ -12,7 +12,6 @@ import ( "github.com/ava-labs/subnet-evm/core" "github.com/ava-labs/subnet-evm/core/types" "github.com/ava-labs/subnet-evm/params" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/rpc" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -132,7 +131,7 @@ func (s *BlockChainAPI) GetActivePrecompilesAt(ctx context.Context, blockTimesta timestamp = *blockTimestamp } - return modules.EnabledStatefulPrecompiles(s.b.ChainConfig(), timestamp) + return s.b.ChainConfig().EnabledStatefulPrecompiles(timestamp) } type ActivePrecompilesResult struct { diff --git a/params/config.go b/params/config.go index 6c24bb6887..3b78536f10 100644 --- a/params/config.go +++ b/params/config.go @@ -559,11 +559,10 @@ func (c *ChainConfig) checkCompatible(newcfg *ChainConfig, height *big.Int, time return err } - // TODO: DO NOT MERGE // Check that the precompiles on the new config are compatible with the existing precompile config. - // if err := c.CheckPrecompilesCompatible(newcfg.PrecompileUpgrades, time); err != nil { - // return err - // } + if err := c.CheckPrecompilesCompatible(newcfg.PrecompileUpgrades, time); err != nil { + return err + } // Check that the state upgrades on the new config are compatible with the existing state upgrade config. if err := c.CheckStateUpgradesCompatible(newcfg.StateUpgrades, time); err != nil { diff --git a/params/config_extra.go b/params/config_extra.go index 6a321e0674..21eadd3e83 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -6,11 +6,13 @@ package params import ( "encoding/json" "errors" + "fmt" "math/big" "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" + "github.com/ethereum/go-ethereum/common" ) // UpgradeConfig includes the following configs that may be specified in upgradeBytes: @@ -158,3 +160,126 @@ func (c *ChainConfig) SetEVMUpgrades(avalancheUpgrades NetworkUpgrades) { c.CancunTime = utils.NewUint64(*avalancheUpgrades.EUpgradeTimestamp) } } + +func (c *ChainConfig) GetActivePrecompileConfig(address common.Address, timestamp uint64) precompileconfig.Config { + configs := c.GetActivatingPrecompileConfigs(address, nil, timestamp, c.PrecompileUpgrades) + if len(configs) == 0 { + return nil + } + return configs[len(configs)-1] // return the most recent config +} + +func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, from *uint64, to uint64, upgrades []PrecompileUpgrade) []precompileconfig.Config { + var configs []precompileconfig.Config + maybeAppend := func(pc precompileconfig.Config) { + if pc.Address() == address && utils.IsForkTransition(pc.Timestamp(), from, to) { + configs = append(configs, pc) + } + } + // First check the embedded [upgrade] for precompiles configured + // in the genesis chain config. + for _, p := range c.GenesisPrecompiles { + maybeAppend(p) + } + // Loop over all upgrades checking for the requested precompile config. + for _, upgrade := range upgrades { + maybeAppend(upgrade.Config) + } + return configs +} + +// IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. +func (c *ChainConfig) IsPrecompileEnabled(address common.Address, timestamp uint64) bool { + config := c.GetActivePrecompileConfig(address, timestamp) + return config != nil && !config.IsDisabled() +} + +func (c *ChainConfig) allPrecompileAddresses(extra ...PrecompileUpgrade) map[string]common.Address { + all := make(map[string]common.Address) + add := func(pc precompileconfig.Config) { + if a, ok := all[pc.Key()]; ok && a != pc.Address() { + panic("DO NOT MERGE") + } + all[pc.Key()] = pc.Address() + } + + for _, p := range c.GenesisPrecompiles { + add(p) + } + for _, p := range c.PrecompileUpgrades { + add(p) + } + for _, p := range extra { + add(p) + } + return all +} + +// CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. +// Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from +// [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. +// Returns nil if [precompileUpgrades] is compatible with [c]. +// Assumes given timestamp is the last accepted block timestamp. +// This ensures that as long as the node has not accepted a block with a different rule set it will allow a +// new upgrade to be applied as long as it activates after the last accepted block. +func (c *ChainConfig) CheckPrecompilesCompatible(precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { + addrs := c.allPrecompileAddresses(precompileUpgrades...) + for _, a := range addrs { + if err := c.checkPrecompileCompatible(a, precompileUpgrades, time); err != nil { + return err + } + } + return nil +} + +// checkPrecompileCompatible verifies that the precompile specified by [address] is compatible between [c] +// and [precompileUpgrades] at [headTimestamp]. +// Returns an error if upgrades already activated at [headTimestamp] are missing from [precompileUpgrades]. +// Upgrades that have already gone into effect cannot be modified or absent from [precompileUpgrades]. +func (c *ChainConfig) checkPrecompileCompatible(address common.Address, precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { + // All active upgrades (from nil to [lastTimestamp]) must match. + activeUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, c.PrecompileUpgrades) + newUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, precompileUpgrades) + + // Check activated upgrades are still present. + for i, upgrade := range activeUpgrades { + if len(newUpgrades) <= i { + // missing upgrade + return newTimestampCompatError( + fmt.Sprintf("missing PrecompileUpgrade[%d]", i), + upgrade.Timestamp(), + nil, + ) + } + // All upgrades that have activated must be identical. + if !upgrade.Equal(newUpgrades[i]) { + return newTimestampCompatError( + fmt.Sprintf("PrecompileUpgrade[%d]", i), + upgrade.Timestamp(), + newUpgrades[i].Timestamp(), + ) + } + } + // then, make sure newUpgrades does not have additional upgrades + // that are already activated. (cannot perform retroactive upgrade) + if len(newUpgrades) > len(activeUpgrades) { + return newTimestampCompatError( + fmt.Sprintf("cannot retroactively enable PrecompileUpgrade[%d]", len(activeUpgrades)), + nil, + newUpgrades[len(activeUpgrades)].Timestamp(), // this indexes to the first element in newUpgrades after the end of activeUpgrades + ) + } + + return nil +} + +// EnabledStatefulPrecompiles returns current stateful precompile configs that are enabled at [blockTimestamp]. +func (c *ChainConfig) EnabledStatefulPrecompiles(blockTimestamp uint64) Precompiles { + statefulPrecompileConfigs := make(Precompiles) + for key, addr := range c.allPrecompileAddresses() { + if config := c.GetActivePrecompileConfig(addr, blockTimestamp); config != nil && !config.IsDisabled() { + statefulPrecompileConfigs[key] = config + } + } + return statefulPrecompileConfigs +} diff --git a/precompile/contracts/warp/config.go b/precompile/contracts/warp/config.go index dde04a8695..48d773b9ea 100644 --- a/precompile/contracts/warp/config.go +++ b/precompile/contracts/warp/config.go @@ -80,6 +80,8 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { // This should be the same key as used in the precompile module. func (*Config) Key() string { return ConfigKey } +func (*Config) Address() common.Address { return ContractAddress } + // Verify tries to verify Config and returns an error accordingly. func (c *Config) Verify(chainConfig precompileconfig.ChainConfig) error { if c.Timestamp() != nil { diff --git a/precompile/modules/chain_config.go b/precompile/modules/chain_config.go deleted file mode 100644 index 5178779238..0000000000 --- a/precompile/modules/chain_config.go +++ /dev/null @@ -1,125 +0,0 @@ -package modules - -import ( - "fmt" - - "github.com/ava-labs/subnet-evm/params" - "github.com/ava-labs/subnet-evm/precompile/precompileconfig" - "github.com/ava-labs/subnet-evm/utils" - "github.com/ethereum/go-ethereum/common" -) - -func GetActivatingPrecompileConfigs(c *params.ChainConfig, address common.Address, from *uint64, to uint64, upgrades []params.PrecompileUpgrade) []precompileconfig.Config { - // Get key from address. - module, ok := GetPrecompileModuleByAddress(address) - if !ok { - return nil - } - var configs []precompileconfig.Config - key := module.ConfigKey - // First check the embedded [upgrade] for precompiles configured - // in the genesis chain config. - if config, ok := c.GenesisPrecompiles[key]; ok { - if utils.IsForkTransition(config.Timestamp(), from, to) { - configs = append(configs, config) - } - } - // Loop over all upgrades checking for the requested precompile config. - for _, upgrade := range upgrades { - if upgrade.Key() == key { - // Check if the precompile activates in the specified range. - if utils.IsForkTransition(upgrade.Timestamp(), from, to) { - configs = append(configs, upgrade.Config) - } - } - } - return configs -} - -func GetActivePrecompileConfig(c *params.ChainConfig, address common.Address, timestamp uint64) precompileconfig.Config { - configs := GetActivatingPrecompileConfigs(c, address, nil, timestamp, c.PrecompileUpgrades) - if len(configs) == 0 { - return nil - } - return configs[len(configs)-1] // return the most recent config -} - -// CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. -// Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from -// [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. -// Returns nil if [precompileUpgrades] is compatible with [c]. -// Assumes given timestamp is the last accepted block timestamp. -// This ensures that as long as the node has not accepted a block with a different rule set it will allow a -// new upgrade to be applied as long as it activates after the last accepted block. -func CheckPrecompilesCompatible(c *params.ChainConfig, precompileUpgrades []params.PrecompileUpgrade, time uint64) *params.ConfigCompatError { - for _, module := range RegisteredModules() { - if err := checkPrecompileCompatible(c, module.Address, precompileUpgrades, time); err != nil { - return err - } - } - - return nil -} - -// checkPrecompileCompatible verifies that the precompile specified by [address] is compatible between [c] -// and [precompileUpgrades] at [headTimestamp]. -// Returns an error if upgrades already activated at [headTimestamp] are missing from [precompileUpgrades]. -// Upgrades that have already gone into effect cannot be modified or absent from [precompileUpgrades]. -func checkPrecompileCompatible(c *params.ChainConfig, address common.Address, precompileUpgrades []params.PrecompileUpgrade, time uint64) *params.ConfigCompatError { - // All active upgrades (from nil to [lastTimestamp]) must match. - activeUpgrades := GetActivatingPrecompileConfigs(c, address, nil, time, c.PrecompileUpgrades) - newUpgrades := GetActivatingPrecompileConfigs(c, address, nil, time, precompileUpgrades) - - // Check activated upgrades are still present. - for i, upgrade := range activeUpgrades { - if len(newUpgrades) <= i { - // missing upgrade - return newTimestampCompatError( - fmt.Sprintf("missing PrecompileUpgrade[%d]", i), - upgrade.Timestamp(), - nil, - ) - } - // All upgrades that have activated must be identical. - if !upgrade.Equal(newUpgrades[i]) { - return newTimestampCompatError( - fmt.Sprintf("PrecompileUpgrade[%d]", i), - upgrade.Timestamp(), - newUpgrades[i].Timestamp(), - ) - } - } - // then, make sure newUpgrades does not have additional upgrades - // that are already activated. (cannot perform retroactive upgrade) - if len(newUpgrades) > len(activeUpgrades) { - return newTimestampCompatError( - fmt.Sprintf("cannot retroactively enable PrecompileUpgrade[%d]", len(activeUpgrades)), - nil, - newUpgrades[len(activeUpgrades)].Timestamp(), // this indexes to the first element in newUpgrades after the end of activeUpgrades - ) - } - - return nil -} - -func newTimestampCompatError(what string, storedtime, newtime *uint64) *params.ConfigCompatError { - return nil // TODO: DO NOT MERGE -} - -// EnabledStatefulPrecompiles returns current stateful precompile configs that are enabled at [blockTimestamp]. -func EnabledStatefulPrecompiles(c *params.ChainConfig, blockTimestamp uint64) params.Precompiles { - statefulPrecompileConfigs := make(params.Precompiles) - for _, module := range RegisteredModules() { - if config := GetActivePrecompileConfig(c, module.Address, blockTimestamp); config != nil && !config.IsDisabled() { - statefulPrecompileConfigs[module.ConfigKey] = config - } - } - - return statefulPrecompileConfigs -} - -// IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. -func IsPrecompileEnabled(c *params.ChainConfig, address common.Address, timestamp uint64) bool { - config := GetActivePrecompileConfig(c, address, timestamp) - return config != nil && !config.IsDisabled() -} From 29aa763d746e101eb113a84af5bb9f05213befca Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 6 Aug 2024 14:16:27 +0100 Subject: [PATCH 16/25] refactor: move `params.ChainConfig` methods to their original locations to minimise diff --- params/config.go | 7 +- params/config_extra.go | 134 ----------------------------------- params/precompile_upgrade.go | 134 +++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 135 deletions(-) diff --git a/params/config.go b/params/config.go index 3b78536f10..e620b685c1 100644 --- a/params/config.go +++ b/params/config.go @@ -215,7 +215,6 @@ var ( // ChainConfig is stored in the database on a per block basis. This means // that any network, identified by its genesis block, can have its own // set of configuration options. -// TODO document the need to call InitChainConfig (DO NOT MERGE) type ChainConfig struct { ChainID *big.Int `json:"chainId"` // chainId identifies the current chain and is used for replay protection @@ -372,6 +371,12 @@ func (r *Rules) PredicaterExists(addr common.Address) bool { return PredicaterExists } +// IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. +func (c *ChainConfig) IsPrecompileEnabled(address common.Address, timestamp uint64) bool { + config := c.GetActivePrecompileConfig(address, timestamp) + return config != nil && !config.IsDisabled() +} + // CheckCompatible checks whether scheduled fork transitions have been imported // with a mismatching chain configuration. func (c *ChainConfig) CheckCompatible(newcfg *ChainConfig, height uint64, time uint64) *ConfigCompatError { diff --git a/params/config_extra.go b/params/config_extra.go index 21eadd3e83..9f6e989260 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -6,13 +6,10 @@ package params import ( "encoding/json" "errors" - "fmt" "math/big" "github.com/ava-labs/avalanchego/snow" - "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" - "github.com/ethereum/go-ethereum/common" ) // UpgradeConfig includes the following configs that may be specified in upgradeBytes: @@ -34,14 +31,6 @@ type AvalancheContext struct { SnowCtx *snow.Context } -// MarshalJSON marshal the precompile config into json based on the precompile key. -// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key -func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { - res := make(map[string]precompileconfig.Config) - res[u.Key()] = u.Config - return json.Marshal(res) -} - // MarshalJSON returns the JSON encoding of c. // This is a custom marshaler to handle the Precompiles field. func (c *ChainConfig) MarshalJSON() ([]byte, error) { @@ -160,126 +149,3 @@ func (c *ChainConfig) SetEVMUpgrades(avalancheUpgrades NetworkUpgrades) { c.CancunTime = utils.NewUint64(*avalancheUpgrades.EUpgradeTimestamp) } } - -func (c *ChainConfig) GetActivePrecompileConfig(address common.Address, timestamp uint64) precompileconfig.Config { - configs := c.GetActivatingPrecompileConfigs(address, nil, timestamp, c.PrecompileUpgrades) - if len(configs) == 0 { - return nil - } - return configs[len(configs)-1] // return the most recent config -} - -func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, from *uint64, to uint64, upgrades []PrecompileUpgrade) []precompileconfig.Config { - var configs []precompileconfig.Config - maybeAppend := func(pc precompileconfig.Config) { - if pc.Address() == address && utils.IsForkTransition(pc.Timestamp(), from, to) { - configs = append(configs, pc) - } - } - // First check the embedded [upgrade] for precompiles configured - // in the genesis chain config. - for _, p := range c.GenesisPrecompiles { - maybeAppend(p) - } - // Loop over all upgrades checking for the requested precompile config. - for _, upgrade := range upgrades { - maybeAppend(upgrade.Config) - } - return configs -} - -// IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. -func (c *ChainConfig) IsPrecompileEnabled(address common.Address, timestamp uint64) bool { - config := c.GetActivePrecompileConfig(address, timestamp) - return config != nil && !config.IsDisabled() -} - -func (c *ChainConfig) allPrecompileAddresses(extra ...PrecompileUpgrade) map[string]common.Address { - all := make(map[string]common.Address) - add := func(pc precompileconfig.Config) { - if a, ok := all[pc.Key()]; ok && a != pc.Address() { - panic("DO NOT MERGE") - } - all[pc.Key()] = pc.Address() - } - - for _, p := range c.GenesisPrecompiles { - add(p) - } - for _, p := range c.PrecompileUpgrades { - add(p) - } - for _, p := range extra { - add(p) - } - return all -} - -// CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. -// Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from -// [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. -// Returns nil if [precompileUpgrades] is compatible with [c]. -// Assumes given timestamp is the last accepted block timestamp. -// This ensures that as long as the node has not accepted a block with a different rule set it will allow a -// new upgrade to be applied as long as it activates after the last accepted block. -func (c *ChainConfig) CheckPrecompilesCompatible(precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { - addrs := c.allPrecompileAddresses(precompileUpgrades...) - for _, a := range addrs { - if err := c.checkPrecompileCompatible(a, precompileUpgrades, time); err != nil { - return err - } - } - return nil -} - -// checkPrecompileCompatible verifies that the precompile specified by [address] is compatible between [c] -// and [precompileUpgrades] at [headTimestamp]. -// Returns an error if upgrades already activated at [headTimestamp] are missing from [precompileUpgrades]. -// Upgrades that have already gone into effect cannot be modified or absent from [precompileUpgrades]. -func (c *ChainConfig) checkPrecompileCompatible(address common.Address, precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { - // All active upgrades (from nil to [lastTimestamp]) must match. - activeUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, c.PrecompileUpgrades) - newUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, precompileUpgrades) - - // Check activated upgrades are still present. - for i, upgrade := range activeUpgrades { - if len(newUpgrades) <= i { - // missing upgrade - return newTimestampCompatError( - fmt.Sprintf("missing PrecompileUpgrade[%d]", i), - upgrade.Timestamp(), - nil, - ) - } - // All upgrades that have activated must be identical. - if !upgrade.Equal(newUpgrades[i]) { - return newTimestampCompatError( - fmt.Sprintf("PrecompileUpgrade[%d]", i), - upgrade.Timestamp(), - newUpgrades[i].Timestamp(), - ) - } - } - // then, make sure newUpgrades does not have additional upgrades - // that are already activated. (cannot perform retroactive upgrade) - if len(newUpgrades) > len(activeUpgrades) { - return newTimestampCompatError( - fmt.Sprintf("cannot retroactively enable PrecompileUpgrade[%d]", len(activeUpgrades)), - nil, - newUpgrades[len(activeUpgrades)].Timestamp(), // this indexes to the first element in newUpgrades after the end of activeUpgrades - ) - } - - return nil -} - -// EnabledStatefulPrecompiles returns current stateful precompile configs that are enabled at [blockTimestamp]. -func (c *ChainConfig) EnabledStatefulPrecompiles(blockTimestamp uint64) Precompiles { - statefulPrecompileConfigs := make(Precompiles) - for key, addr := range c.allPrecompileAddresses() { - if config := c.GetActivePrecompileConfig(addr, blockTimestamp); config != nil && !config.IsDisabled() { - statefulPrecompileConfigs[key] = config - } - } - return statefulPrecompileConfigs -} diff --git a/params/precompile_upgrade.go b/params/precompile_upgrade.go index dae0aef1a4..3d2ae932ae 100644 --- a/params/precompile_upgrade.go +++ b/params/precompile_upgrade.go @@ -4,10 +4,12 @@ package params import ( + "encoding/json" "fmt" "github.com/ava-labs/subnet-evm/precompile/precompileconfig" "github.com/ava-labs/subnet-evm/utils" + "github.com/ethereum/go-ethereum/common" ) // PrecompileUpgrade is a helper struct embedded in UpgradeConfig. @@ -18,6 +20,14 @@ type PrecompileUpgrade struct { precompileconfig.Config } +// MarshalJSON marshal the precompile config into json based on the precompile key. +// Ex: {"feeManagerConfig": {...}} where "feeManagerConfig" is the key +func (u *PrecompileUpgrade) MarshalJSON() ([]byte, error) { + res := make(map[string]precompileconfig.Config) + res[u.Key()] = u.Config + return json.Marshal(res) +} + // verifyPrecompileUpgrades checks [c.PrecompileUpgrades] is well formed: // - [upgrades] must specify exactly one key per PrecompileUpgrade // - the specified blockTimestamps must monotonically increase @@ -103,3 +113,127 @@ func (c *ChainConfig) verifyPrecompileUpgrades() error { return nil } + +// getActivePrecompileConfig returns the most recent precompile config corresponding to [address]. +// If none have occurred, returns nil. +func (c *ChainConfig) getActivePrecompileConfig(address common.Address, timestamp uint64) precompileconfig.Config { + configs := c.GetActivatingPrecompileConfigs(address, nil, timestamp, c.PrecompileUpgrades) + if len(configs) == 0 { + return nil + } + return configs[len(configs)-1] // return the most recent config +} + +// GetActivatingPrecompileConfigs returns all precompile upgrades configured to activate during the +// state transition from a block with timestamp [from] to a block with timestamp [to]. +func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, from *uint64, to uint64, upgrades []PrecompileUpgrade) []precompileconfig.Config { + var configs []precompileconfig.Config + maybeAppend := func(pc precompileconfig.Config) { + if pc.Address() == address && utils.IsForkTransition(pc.Timestamp(), from, to) { + configs = append(configs, pc) + } + } + // First check the embedded [upgrade] for precompiles configured + // in the genesis chain config. + for _, p := range c.GenesisPrecompiles { + maybeAppend(p) + } + // Loop over all upgrades checking for the requested precompile config. + for _, upgrade := range upgrades { + maybeAppend(upgrade.Config) + } + return configs +} + +// CheckPrecompilesCompatible checks if [precompileUpgrades] are compatible with [c] at [headTimestamp]. +// Returns a ConfigCompatError if upgrades already activated at [headTimestamp] are missing from +// [precompileUpgrades]. Upgrades not already activated may be modified or absent from [precompileUpgrades]. +// Returns nil if [precompileUpgrades] is compatible with [c]. +// Assumes given timestamp is the last accepted block timestamp. +// This ensures that as long as the node has not accepted a block with a different rule set it will allow a +// new upgrade to be applied as long as it activates after the last accepted block. +func (c *ChainConfig) CheckPrecompilesCompatible(precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { + addrs := c.allPrecompileAddresses(precompileUpgrades...) + for _, a := range addrs { + if err := c.checkPrecompileCompatible(a, precompileUpgrades, time); err != nil { + return err + } + } + return nil +} + +// checkPrecompileCompatible verifies that the precompile specified by [address] is compatible between [c] +// and [precompileUpgrades] at [headTimestamp]. +// Returns an error if upgrades already activated at [headTimestamp] are missing from [precompileUpgrades]. +// Upgrades that have already gone into effect cannot be modified or absent from [precompileUpgrades]. +func (c *ChainConfig) checkPrecompileCompatible(address common.Address, precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { + // All active upgrades (from nil to [lastTimestamp]) must match. + activeUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, c.PrecompileUpgrades) + newUpgrades := c.GetActivatingPrecompileConfigs(address, nil, time, precompileUpgrades) + + // Check activated upgrades are still present. + for i, upgrade := range activeUpgrades { + if len(newUpgrades) <= i { + // missing upgrade + return newTimestampCompatError( + fmt.Sprintf("missing PrecompileUpgrade[%d]", i), + upgrade.Timestamp(), + nil, + ) + } + // All upgrades that have activated must be identical. + if !upgrade.Equal(newUpgrades[i]) { + return newTimestampCompatError( + fmt.Sprintf("PrecompileUpgrade[%d]", i), + upgrade.Timestamp(), + newUpgrades[i].Timestamp(), + ) + } + } + // then, make sure newUpgrades does not have additional upgrades + // that are already activated. (cannot perform retroactive upgrade) + if len(newUpgrades) > len(activeUpgrades) { + return newTimestampCompatError( + fmt.Sprintf("cannot retroactively enable PrecompileUpgrade[%d]", len(activeUpgrades)), + nil, + newUpgrades[len(activeUpgrades)].Timestamp(), // this indexes to the first element in newUpgrades after the end of activeUpgrades + ) + } + + return nil +} + +// EnabledStatefulPrecompiles returns current stateful precompile configs that are enabled at [blockTimestamp]. +func (c *ChainConfig) EnabledStatefulPrecompiles(blockTimestamp uint64) Precompiles { + statefulPrecompileConfigs := make(Precompiles) + for key, addr := range c.allPrecompileAddresses() { + if config := c.getActivePrecompileConfig(addr, blockTimestamp); config != nil && !config.IsDisabled() { + statefulPrecompileConfigs[key] = config + } + } + return statefulPrecompileConfigs +} + +// allPrecompileAddresses returns a mapping from precompile config key to +// address for all precompiles defined in [ChainConfig.GenesisPrecompiles], +// [ChainConfig.UpgradeConfig.PrecompileUpgrades], and the `extra` upgrades. +func (c *ChainConfig) allPrecompileAddresses(extra ...PrecompileUpgrade) map[string]common.Address { + all := make(map[string]common.Address) + add := func(pc precompileconfig.Config) { + if a, ok := all[pc.Key()]; ok && a != pc.Address() { + panic("DO NOT MERGE") + } + all[pc.Key()] = pc.Address() + } + + for _, p := range c.GenesisPrecompiles { + add(p) + } + for _, p := range c.UpgradeConfig.PrecompileUpgrades { + add(p.Config) + } + for _, p := range extra { + add(p.Config) + } + return all +} From afce46629a8356d514b65f390194727acbaf8b65 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 6 Aug 2024 14:36:07 +0100 Subject: [PATCH 17/25] chore: revert cosmetic changes --- params/config.go | 2 +- params/config_extra.go | 6 ++---- params/config_test.go | 9 ++++++--- params/precompile_config_test.go | 19 +++++++++---------- params/precompile_upgrade_test.go | 2 -- params/state_upgrade_test.go | 3 ++- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/params/config.go b/params/config.go index e620b685c1..0cedfeab07 100644 --- a/params/config.go +++ b/params/config.go @@ -373,7 +373,7 @@ func (r *Rules) PredicaterExists(addr common.Address) bool { // IsPrecompileEnabled returns whether precompile with [address] is enabled at [timestamp]. func (c *ChainConfig) IsPrecompileEnabled(address common.Address, timestamp uint64) bool { - config := c.GetActivePrecompileConfig(address, timestamp) + config := c.getActivePrecompileConfig(address, timestamp) return config != nil && !config.IsDisabled() } diff --git a/params/config_extra.go b/params/config_extra.go index 9f6e989260..71847bac7d 100644 --- a/params/config_extra.go +++ b/params/config_extra.go @@ -33,12 +33,10 @@ type AvalancheContext struct { // MarshalJSON returns the JSON encoding of c. // This is a custom marshaler to handle the Precompiles field. -func (c *ChainConfig) MarshalJSON() ([]byte, error) { - // TODO refactor this (DO NOT MERGE) - +func (c ChainConfig) MarshalJSON() ([]byte, error) { // Alias ChainConfig to avoid recursion type _ChainConfig ChainConfig - tmp, err := json.Marshal((*_ChainConfig)(c)) + tmp, err := json.Marshal(_ChainConfig(c)) if err != nil { return nil, err } diff --git a/params/config_test.go b/params/config_test.go index a433c09b5c..0e16435952 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -216,7 +216,8 @@ func TestConfigUnmarshalJSON(t *testing.T) { } `) c := &ChainConfig{} - require.NoError(json.Unmarshal(config, c)) + err := json.Unmarshal(config, c) + require.NoError(err) require.Equal(c.ChainID, big.NewInt(43214)) require.Equal(c.AllowFeeRecipients, true) @@ -234,7 +235,8 @@ func TestConfigUnmarshalJSON(t *testing.T) { marshaled, err := json.Marshal(c) require.NoError(err) c2 := &ChainConfig{} - require.NoError(json.Unmarshal(marshaled, c2)) + err = json.Unmarshal(marshaled, c2) + require.NoError(err) require.Equal(c, c2) } @@ -326,6 +328,7 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { require.JSONEq(t, expectedJSON, string(result)) var unmarshalled ChainConfigWithUpgradesJSON - require.NoError(t, json.Unmarshal(result, &unmarshalled)) + err = json.Unmarshal(result, &unmarshalled) + require.NoError(t, err) require.Equal(t, config, unmarshalled) } diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index 179c994558..e1c9501fae 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -14,7 +14,6 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" - "github.com/ava-labs/subnet-evm/precompile/modules" "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" @@ -75,7 +74,7 @@ func TestVerifyWithChainConfigAtNilTimestamp(t *testing.T) { // this does NOT enable the precompile, so it should be upgradeable. {Config: txallowlist.NewConfig(nil, nil, nil, nil)}, } - require.False(t, modules.IsPrecompileEnabled(config, txallowlist.ContractAddress, 0)) // check the precompile is not enabled. + require.False(t, config.IsPrecompileEnabled(txallowlist.ContractAddress, 0)) // check the precompile is not enabled. config.PrecompileUpgrades = []PrecompileUpgrade{ { // enable TxAllowList at timestamp 5 @@ -273,17 +272,17 @@ func TestGetPrecompileConfig(t *testing.T) { deployerallowlist.ConfigKey: deployerallowlist.NewConfig(utils.NewUint64(10), nil, nil, nil), } - deployerConfig := modules.GetActivePrecompileConfig(config, deployerallowlist.ContractAddress, 0) - require.Nil(deployerConfig) + deployerConfig := config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 0, config.PrecompileUpgrades) + require.Len(deployerConfig, 0) - deployerConfig = modules.GetActivePrecompileConfig(config, deployerallowlist.ContractAddress, 10) - require.NotNil(deployerConfig) + deployerConfig = config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 10, config.PrecompileUpgrades) + require.GreaterOrEqual(len(deployerConfig), 1) - deployerConfig = modules.GetActivePrecompileConfig(config, deployerallowlist.ContractAddress, 11) - require.NotNil(deployerConfig) + deployerConfig = config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 11, config.PrecompileUpgrades) + require.GreaterOrEqual(len(deployerConfig), 1) - txAllowListConfig := modules.GetActivePrecompileConfig(config, txallowlist.ContractAddress, 0) - require.Nil(txAllowListConfig) + txAllowListConfig := config.GetActivatingPrecompileConfigs(txallowlist.ContractAddress, nil, 0, config.PrecompileUpgrades) + require.Len(txAllowListConfig, 0) } func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { diff --git a/params/precompile_upgrade_test.go b/params/precompile_upgrade_test.go index 72bb1856dd..5c21b08534 100644 --- a/params/precompile_upgrade_test.go +++ b/params/precompile_upgrade_test.go @@ -276,13 +276,11 @@ type upgradeCompatibilityTest struct { } func (tt *upgradeCompatibilityTest) run(t *testing.T, chainConfig ChainConfig) { - t.Skip("DO NOT MERGE") // apply all the upgrade bytes specified in order for i, upgrade := range tt.configs { newCfg := chainConfig newCfg.UpgradeConfig = *upgrade - // TODO: decide on correct height as was originally nil when calling internal method; ?math.MaxUint64 err := chainConfig.CheckCompatible(&newCfg, 0, tt.startTimestamps[i]) // if this is not the final upgradeBytes, continue applying diff --git a/params/state_upgrade_test.go b/params/state_upgrade_test.go index f0e272a8da..da4361d3cb 100644 --- a/params/state_upgrade_test.go +++ b/params/state_upgrade_test.go @@ -183,6 +183,7 @@ func TestUnmarshalStateUpgradeJSON(t *testing.T) { }, } var unmarshaledConfig UpgradeConfig - require.NoError(t, json.Unmarshal(jsonBytes, &unmarshaledConfig)) + err := json.Unmarshal(jsonBytes, &unmarshaledConfig) + require.NoError(t, err) require.Equal(t, upgradeConfig, unmarshaledConfig) } From ba12b5450e283c3430f209d309c23b27b69da33a Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 6 Aug 2024 14:46:34 +0100 Subject: [PATCH 18/25] chore: revert as many changes as possible in `params.ChainConfig.Rules()` --- params/config.go | 30 +++++++++--------------------- params/precompile_upgrade.go | 3 --- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/params/config.go b/params/config.go index 0cedfeab07..f17da4b18b 100644 --- a/params/config.go +++ b/params/config.go @@ -754,32 +754,20 @@ func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules { r.AvalancheRules = c.GetAvalancheRules(timestamp) r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config) - update := func(cfg precompileconfig.Config) { - if !utils.IsForkTransition(cfg.Timestamp(), nil, timestamp) { - return - } + r.Predicaters = make(map[common.Address]precompileconfig.Predicater) + r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) - if cfg.IsDisabled() { - delete(r.ActivePrecompiles, cfg.Address()) - } else { - r.ActivePrecompiles[cfg.Address()] = cfg + for _, addr := range c.allPrecompileAddresses() { + cfg := c.getActivePrecompileConfig(addr, timestamp) + if cfg == nil || cfg.IsDisabled() { + continue } - } + r.ActivePrecompiles[addr] = cfg - for _, c := range c.GenesisPrecompiles { - update(c) - } - for _, u := range c.PrecompileUpgrades { - update(u) - } - - r.Predicaters = make(map[common.Address]precompileconfig.Predicater) - r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter) - for addr, config := range r.ActivePrecompiles { - if p, ok := config.(precompileconfig.Predicater); ok { + if p, ok := cfg.(precompileconfig.Predicater); ok { r.Predicaters[addr] = p } - if a, ok := config.(precompileconfig.Accepter); ok { + if a, ok := cfg.(precompileconfig.Accepter); ok { r.AccepterPrecompiles[addr] = a } } diff --git a/params/precompile_upgrade.go b/params/precompile_upgrade.go index 3d2ae932ae..5521785a82 100644 --- a/params/precompile_upgrade.go +++ b/params/precompile_upgrade.go @@ -133,12 +133,9 @@ func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, fro configs = append(configs, pc) } } - // First check the embedded [upgrade] for precompiles configured - // in the genesis chain config. for _, p := range c.GenesisPrecompiles { maybeAppend(p) } - // Loop over all upgrades checking for the requested precompile config. for _, upgrade := range upgrades { maybeAppend(upgrade.Config) } From b55bba4161f8770e0a05eb34fcd5efecc8ff32e2 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Tue, 6 Aug 2024 15:19:46 +0100 Subject: [PATCH 19/25] chore: add `Address()` method on test-double `precompileconfig.Config` impls --- precompile/allowlist/allowlist_test.go | 6 ++++-- precompile/precompileconfig/mocks.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/precompile/allowlist/allowlist_test.go b/precompile/allowlist/allowlist_test.go index ebbcf6b69e..5cc511228d 100644 --- a/precompile/allowlist/allowlist_test.go +++ b/precompile/allowlist/allowlist_test.go @@ -25,8 +25,10 @@ type dummyConfig struct { AllowListConfig } -func (d *dummyConfig) Key() string { return "dummy" } -func (d *dummyConfig) IsDisabled() bool { return false } +func (*dummyConfig) Key() string { return "dummy" } +func (*dummyConfig) Address() common.Address { return dummyAddr } +func (*dummyConfig) IsDisabled() bool { return false } + func (d *dummyConfig) Verify(chainConfig precompileconfig.ChainConfig) error { return d.AllowListConfig.Verify(chainConfig, d.Upgrade) } diff --git a/precompile/precompileconfig/mocks.go b/precompile/precompileconfig/mocks.go index 614ec5a522..86642f99d8 100644 --- a/precompile/precompileconfig/mocks.go +++ b/precompile/precompileconfig/mocks.go @@ -92,6 +92,20 @@ func (m *MockConfig) EXPECT() *MockConfigMockRecorder { return m.recorder } +// Address mocks base method. +func (m *MockConfig) Address() common.Address { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Address") + ret0, _ := ret[0].(common.Address) + return ret0 +} + +// Address indicates an expected call of Address. +func (mr *MockConfigMockRecorder) Address() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Address", reflect.TypeOf((*MockConfig)(nil).Address)) +} + // Equal mocks base method. func (m *MockConfig) Equal(arg0 Config) bool { m.ctrl.T.Helper() From eb4e2262287e700fdc94e1648edd9c1da88e53ae Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 7 Aug 2024 13:19:14 +0100 Subject: [PATCH 20/25] fix: all unit tests passing locally --- .../precompilebind/precompile_config_template.go | 8 ++++++-- core/rawdb/accessors_metadata.go | 8 ++++++++ params/precompile_config_test.go | 14 ++++++++------ params/precompile_upgrade.go | 15 +++++++++++---- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/accounts/abi/bind/precompilebind/precompile_config_template.go b/accounts/abi/bind/precompilebind/precompile_config_template.go index 69c4fcbcc9..eaefe4de31 100644 --- a/accounts/abi/bind/precompilebind/precompile_config_template.go +++ b/accounts/abi/bind/precompilebind/precompile_config_template.go @@ -15,9 +15,9 @@ import ( {{- if .Contract.AllowList}} "github.com/ava-labs/subnet-evm/precompile/allowlist" - "github.com/ethereum/go-ethereum/common" {{- end}} - + + "github.com/ethereum/go-ethereum/common" ) var _ precompileconfig.Config = &Config{} @@ -63,6 +63,10 @@ func NewDisableConfig(blockTimestamp *uint64) *Config { // This should be the same key as used in the precompile module. func (*Config) Key() string { return ConfigKey } +// Address returns the address for the {{.Contract.Type}} precompileconfig. +// This should be the same key as used in the precompile module. +func (*Config) Address() common.Address { return ContractAddress } + // Verify tries to verify Config and returns an error accordingly. func (c *Config) Verify(chainConfig precompileconfig.ChainConfig) error { {{- if .Contract.AllowList}} diff --git a/core/rawdb/accessors_metadata.go b/core/rawdb/accessors_metadata.go index eafc43d83e..f692dd93b4 100644 --- a/core/rawdb/accessors_metadata.go +++ b/core/rawdb/accessors_metadata.go @@ -35,6 +35,14 @@ import ( "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" + + // This side-effect import goes against recommendations for not using blank + // imports in libraries: https://google.github.io/styleguide/go/decisions#import-blank-import-_ + // This is deliberate because the ReadChainConfig() method only logs errors + // and doesn't bubble them up; this caused hours of problems in development! + // The described cons of using a blank import here are _far_ outweighed by + // the benefits. + _ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency ) // ReadDatabaseVersion retrieves the version number of the database. diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index e1c9501fae..9440f9fcdd 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -272,14 +272,16 @@ func TestGetPrecompileConfig(t *testing.T) { deployerallowlist.ConfigKey: deployerallowlist.NewConfig(utils.NewUint64(10), nil, nil, nil), } - deployerConfig := config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 0, config.PrecompileUpgrades) - require.Len(deployerConfig, 0) + configs := config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 0, config.PrecompileUpgrades) + require.Len(configs, 0) - deployerConfig = config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 10, config.PrecompileUpgrades) - require.GreaterOrEqual(len(deployerConfig), 1) + configs = config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 10, config.PrecompileUpgrades) + require.GreaterOrEqual(len(configs), 1) + require.NotNil(configs[len(configs)-1]) - deployerConfig = config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 11, config.PrecompileUpgrades) - require.GreaterOrEqual(len(deployerConfig), 1) + configs = config.GetActivatingPrecompileConfigs(deployerallowlist.ContractAddress, nil, 11, config.PrecompileUpgrades) + require.GreaterOrEqual(len(configs), 1) + require.NotNil(configs[len(configs)-1]) txAllowListConfig := config.GetActivatingPrecompileConfigs(txallowlist.ContractAddress, nil, 0, config.PrecompileUpgrades) require.Len(txAllowListConfig, 0) diff --git a/params/precompile_upgrade.go b/params/precompile_upgrade.go index 5521785a82..4d714d54e5 100644 --- a/params/precompile_upgrade.go +++ b/params/precompile_upgrade.go @@ -150,7 +150,7 @@ func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, fro // This ensures that as long as the node has not accepted a block with a different rule set it will allow a // new upgrade to be applied as long as it activates after the last accepted block. func (c *ChainConfig) CheckPrecompilesCompatible(precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { - addrs := c.allPrecompileAddresses(precompileUpgrades...) + addrs := c.allPrecompileAddressesPlus(precompileUpgrades...) for _, a := range addrs { if err := c.checkPrecompileCompatible(a, precompileUpgrades, time); err != nil { return err @@ -211,10 +211,17 @@ func (c *ChainConfig) EnabledStatefulPrecompiles(blockTimestamp uint64) Precompi return statefulPrecompileConfigs } -// allPrecompileAddresses returns a mapping from precompile config key to +// allPrecompileAddresses is equivalent to allPrecompileAddressesPlus() without +// any arguments. The two methods are differentiated to improve readability at +// the call sites. +func (c *ChainConfig) allPrecompileAddresses() map[string]common.Address { + return c.allPrecompileAddressesPlus() +} + +// allPrecompileAddressesPlus returns a mapping from precompile config key to // address for all precompiles defined in [ChainConfig.GenesisPrecompiles], -// [ChainConfig.UpgradeConfig.PrecompileUpgrades], and the `extra` upgrades. -func (c *ChainConfig) allPrecompileAddresses(extra ...PrecompileUpgrade) map[string]common.Address { +// [ChainConfig.UpgradeConfig.PrecompileUpgrades], plus the `extra` upgrades. +func (c *ChainConfig) allPrecompileAddressesPlus(extra ...PrecompileUpgrade) map[string]common.Address { all := make(map[string]common.Address) add := func(pc precompileconfig.Config) { if a, ok := all[pc.Key()]; ok && a != pc.Address() { From b15507b49923243f22c69222233c2e587f289a0d Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 7 Aug 2024 13:19:52 +0100 Subject: [PATCH 21/25] chore: update copyright years to keep linter happy --- precompile/allowlist/allowlist.go | 2 +- precompile/contracts/deployerallowlist/config.go | 2 +- precompile/contracts/feemanager/config.go | 2 +- precompile/contracts/nativeminter/config.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/precompile/allowlist/allowlist.go b/precompile/allowlist/allowlist.go index c5e60458cc..75d2baea3d 100644 --- a/precompile/allowlist/allowlist.go +++ b/precompile/allowlist/allowlist.go @@ -1,4 +1,4 @@ -// (c) 2019-2023, Ava Labs, Inc. All rights reserved. +// (c) 2019-2024, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. package allowlist diff --git a/precompile/contracts/deployerallowlist/config.go b/precompile/contracts/deployerallowlist/config.go index c3cce52d79..521ecfd62b 100644 --- a/precompile/contracts/deployerallowlist/config.go +++ b/precompile/contracts/deployerallowlist/config.go @@ -1,4 +1,4 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// (c) 2019-2024, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. package deployerallowlist diff --git a/precompile/contracts/feemanager/config.go b/precompile/contracts/feemanager/config.go index 311d47a647..3183d7f226 100644 --- a/precompile/contracts/feemanager/config.go +++ b/precompile/contracts/feemanager/config.go @@ -1,4 +1,4 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// (c) 2019-2024, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. package feemanager diff --git a/precompile/contracts/nativeminter/config.go b/precompile/contracts/nativeminter/config.go index d222589a59..a6c1c1464e 100644 --- a/precompile/contracts/nativeminter/config.go +++ b/precompile/contracts/nativeminter/config.go @@ -1,4 +1,4 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// (c) 2019-2024, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. package nativeminter From 6a7803eaca0a3e5215f2c5d918030ff911c70cd6 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 7 Aug 2024 13:22:57 +0100 Subject: [PATCH 22/25] refactor: `paramsjson` misc (see full message) * Stop exporting any identifiers, relying solely on `_` importing * Split out `Unmarshal()` type switch into specific functions --- params/paramsjson/paramsjson.go | 56 +++++++++++++-------------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/params/paramsjson/paramsjson.go b/params/paramsjson/paramsjson.go index 87d6b37ff9..80034bbf3e 100644 --- a/params/paramsjson/paramsjson.go +++ b/params/paramsjson/paramsjson.go @@ -2,8 +2,8 @@ // on the `modules` package. This avoids `params` depending on `modules`, even // transitively, which would result in a circular dependency. // -// Typically there is no need to call this package directly. It should instead -// be blank _ imported to register its unmarshallers, similarly to SQL drivers. +// This package doesn't export any identifiers. It should instead be blank _ +// imported to register its unmarshallers, similarly to SQL drivers. package paramsjson import ( @@ -18,46 +18,34 @@ import ( ) func init() { - // The parameters of RegisterJSONUmarshalers are all generic but the - // specific types are unambiguous so we don't need to specify them here. - params.RegisterJSONUnmarshalers(Unmarshal, Unmarshal, Unmarshal, Unmarshal) + params.RegisterJSONUnmarshalers( + unmarshalChainConfig, + unmarshalChainConfigWithUpgrades, + unmarshalUpgradeConfig, + func(data []byte, v *params.PrecompileUpgrade) error { + return json.Unmarshal(data, (*precompileUpgrade)(v)) + }, + ) } -// An Unmarshaler can have JSON unmarshalled into it by [Unmarshal]. -type Unmarshaler interface { - *params.ChainConfig | *params.ChainConfigWithUpgradesJSON | *params.UpgradeConfig | *params.PrecompileUpgrade +func unmarshalChainConfig(data []byte, v *params.ChainConfig) error { + return unmarshalChainConfigAndUpgrades(data, v, nil, "") } -// Unmarshal is a drop-in replacement for [json.Unmarshal]. -func Unmarshal[T Unmarshaler](data []byte, v T) error { - switch v := any(v).(type) { - case *params.ChainConfig: - return unmarshalChainConfig(data, v, nil, "") +func unmarshalChainConfigWithUpgrades(data []byte, v *params.ChainConfigWithUpgradesJSON) error { + const fldName = "UpgradeConfig" + _ = v.UpgradeConfig // if changing this then change the line above too - case *params.ChainConfigWithUpgradesJSON: - const fldName = "UpgradeConfig" - - tStruct := reflect.TypeOf(v).Elem() - fld, ok := tStruct.FieldByName(fldName) - if !ok { - // If this happens then the constant `fldName` is of a different name to the actual struct field used below. - return fmt.Errorf("BUG: %T(%v).FieldByName(%q) returned false", tStruct, tStruct, fldName) - } - return unmarshalChainConfig(data, &v.ChainConfig, &v.UpgradeConfig, strings.Split(fld.Tag.Get("json"), ",")[0]) - - case *params.UpgradeConfig: - return unmarshalUpgradeConfig(data, v) - - case *params.PrecompileUpgrade: - return json.Unmarshal(data, (*precompileUpgrade)(v)) - - default: - // If this happens then the Unmarshaler interface has been modified but the above cases haven't been. - return fmt.Errorf("unsupported type %T", v) + tStruct := reflect.TypeOf(v).Elem() + fld, ok := tStruct.FieldByName(fldName) + if !ok { + // If this happens then the constant `fldName` is of a different name to the actual struct field used below. + return fmt.Errorf("BUG: %T(%v).FieldByName(%q) returned false", tStruct, tStruct, fldName) } + return unmarshalChainConfigAndUpgrades(data, &v.ChainConfig, &v.UpgradeConfig, strings.Split(fld.Tag.Get("json"), ",")[0]) } -func unmarshalChainConfig(data []byte, cc *params.ChainConfig, upgrades *params.UpgradeConfig, upgradesJSONField string) error { +func unmarshalChainConfigAndUpgrades(data []byte, cc *params.ChainConfig, upgrades *params.UpgradeConfig, upgradesJSONField string) error { type withoutMethods *params.ChainConfig // circumvents UnmarshalJSON() method, which always returns an error if err := json.Unmarshal(data, withoutMethods(cc)); err != nil { return err From 0aa0922fa50065b5475c23966a8f56e1ef1c966f Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 7 Aug 2024 13:50:09 +0100 Subject: [PATCH 23/25] refactor: add `Must` prefix and panics to `params.RegisterJSONUnmarshalers()` --- params/json.go | 26 +++++++++++++++----------- params/paramsjson/paramsjson.go | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/params/json.go b/params/json.go index 25a7382ca9..2e32e263b3 100644 --- a/params/json.go +++ b/params/json.go @@ -2,8 +2,6 @@ package params import ( "fmt" - "runtime" - "strings" ) // A JSONUnmarshaler is a type-safe function for unmarshalling a JSON buffer @@ -17,7 +15,7 @@ var jsonUmarshalers struct { pu JSONUnmarshaler[*PrecompileUpgrade] } -// RegisterJSONUnmarshalers registers the JSON unmarshalling functions for +// MustRegisterJSONUnmarshalers registers the JSON unmarshalling functions for // various types. This allows their unmarshalling behaviour to be injected by // the [params/paramsjson] package, which can't be directly imported as it would // result in a circular dependency. @@ -25,22 +23,22 @@ var jsonUmarshalers struct { // This function SHOULD NOT be called directly. Instead, blank import the // [params/paramsjson] package, which registers unmarshalers in its init() // function. -func RegisterJSONUnmarshalers( +// +// MustRegisterJSONUnmarshalers panics if any functions are nil or if called +// more than once. +func MustRegisterJSONUnmarshalers( cc JSONUnmarshaler[*ChainConfig], cu JSONUnmarshaler[*ChainConfigWithUpgradesJSON], uc JSONUnmarshaler[*UpgradeConfig], pu JSONUnmarshaler[*PrecompileUpgrade], ) { - pc, _, _, ok := runtime.Caller(0) - if !ok { - _ = ok - } - if fn := runtime.FuncForPC(pc).Name(); !strings.HasPrefix(fn, "github.com/ava-labs/subnet-evm/params/paramsjson.") { - _ = fn - } if jsonUmarshalers.cc != nil { panic("JSON unmarshalers already registered") } + panicIfNil(cc) + panicIfNil(cu) + panicIfNil(uc) + panicIfNil(pu) jsonUmarshalers.cc = cc jsonUmarshalers.cu = cu @@ -48,6 +46,12 @@ func RegisterJSONUnmarshalers( jsonUmarshalers.pu = pu } +func panicIfNil[T any](fn JSONUnmarshaler[T]) { + if fn == nil { + panic(fmt.Sprintf("registering nil %T", fn)) + } +} + func unmarshalJSON[T any](u JSONUnmarshaler[T], data []byte, v T) error { if u == nil { return fmt.Errorf(`%T is nil; did you remember to import _ "github.com/ava-labs/subnet-evm/params/paramsjson"`, u) diff --git a/params/paramsjson/paramsjson.go b/params/paramsjson/paramsjson.go index 80034bbf3e..289a599c45 100644 --- a/params/paramsjson/paramsjson.go +++ b/params/paramsjson/paramsjson.go @@ -18,7 +18,7 @@ import ( ) func init() { - params.RegisterJSONUnmarshalers( + params.MustRegisterJSONUnmarshalers( unmarshalChainConfig, unmarshalChainConfigWithUpgrades, unmarshalUpgradeConfig, From 30e45f4f4041359875189bee2b1f922cc4241701 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 7 Aug 2024 13:51:15 +0100 Subject: [PATCH 24/25] test: `params.UpgradeConfig` comprehensive testing --- params/precompile_config_test.go | 70 ++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index 9440f9fcdd..e1b4441d87 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -5,6 +5,7 @@ package params_test import ( "encoding/json" + "fmt" "math/big" "testing" @@ -290,20 +291,28 @@ func TestGetPrecompileConfig(t *testing.T) { func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { require := require.New(t) - upgradeBytes := []byte(` + const ( + durangoTimestamp = 314159 + stateUpgradeTimestamp = 142857 + rewardManagerTimestamp = 1671542573 + rewardManagerAdmin = "0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC" + nativeMinterTimestamp = 1671543172 + ) + + upgradeBytes := []byte(fmt.Sprintf(` { "networkUpgradeOverrides": { - "durangoTimestamp": 314159 + "durangoTimestamp": %d }, "stateUpgrades": [ - {"blockTimestamp": 142857} + {"blockTimestamp": %d} ], "precompileUpgrades": [ { "rewardManagerConfig": { - "blockTimestamp": 1671542573, + "blockTimestamp": %d, "adminAddresses": [ - "0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC" + %q ], "initialRewardConfig": { "allowFeeRecipients": true @@ -312,40 +321,47 @@ func TestPrecompileUpgradeUnmarshalJSON(t *testing.T) { }, { "contractNativeMinterConfig": { - "blockTimestamp": 1671543172, + "blockTimestamp": %d, "disable": false } } ] } - `) + `, durangoTimestamp, stateUpgradeTimestamp, rewardManagerTimestamp, rewardManagerAdmin, nativeMinterTimestamp)) var upgradeConfig UpgradeConfig - require.NoError(json.Unmarshal(upgradeBytes, &upgradeConfig)) - - require.Len(upgradeConfig.PrecompileUpgrades, 2) - - rewardManagerConf := upgradeConfig.PrecompileUpgrades[0] - require.Equal(rewardManagerConf.Key(), rewardmanager.ConfigKey) - testRewardManagerConfig := rewardmanager.NewConfig( - utils.NewUint64(1671542573), - []common.Address{common.HexToAddress("0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC")}, - nil, - nil, - &rewardmanager.InitialRewardConfig{ - AllowFeeRecipients: true, - }) - require.True(rewardManagerConf.Equal(testRewardManagerConfig)) + err := json.Unmarshal(upgradeBytes, &upgradeConfig) + require.NoError(err) - nativeMinterConfig := upgradeConfig.PrecompileUpgrades[1] - require.Equal(nativeMinterConfig.Key(), nativeminter.ConfigKey) - expectedNativeMinterConfig := nativeminter.NewConfig(utils.NewUint64(1671543172), nil, nil, nil, nil) - require.True(nativeMinterConfig.Equal(expectedNativeMinterConfig)) + want := UpgradeConfig{ + NetworkUpgradeOverrides: &NetworkUpgrades{ + DurangoTimestamp: utils.NewUint64(durangoTimestamp), + }, + StateUpgrades: []StateUpgrade{{ + BlockTimestamp: utils.NewUint64(stateUpgradeTimestamp), + }}, + PrecompileUpgrades: []PrecompileUpgrade{ + { + rewardmanager.NewConfig( + utils.NewUint64(rewardManagerTimestamp), + []common.Address{common.HexToAddress(rewardManagerAdmin)}, nil, nil, + &rewardmanager.InitialRewardConfig{ + AllowFeeRecipients: true, + }, + ), + }, + { + nativeminter.NewConfig(utils.NewUint64(nativeMinterTimestamp), nil, nil, nil, nil), + }, + }, + } + require.Equal(want, upgradeConfig) // Marshal and unmarshal again and check that the result is the same upgradeBytes2, err := json.Marshal(upgradeConfig) require.NoError(err) var upgradeConfig2 UpgradeConfig - require.NoError(json.Unmarshal(upgradeBytes2, &upgradeConfig2)) + err = json.Unmarshal(upgradeBytes2, &upgradeConfig2) + require.NoError(err) require.Equal(upgradeConfig, upgradeConfig2) } From 8d7c00285e9d3608739130043b8c4cfab00dfbf9 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 7 Aug 2024 13:51:39 +0100 Subject: [PATCH 25/25] chore: remove unnecessary variable declaration --- params/precompile_upgrade.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/params/precompile_upgrade.go b/params/precompile_upgrade.go index 4d714d54e5..393bb65b31 100644 --- a/params/precompile_upgrade.go +++ b/params/precompile_upgrade.go @@ -150,8 +150,7 @@ func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, fro // This ensures that as long as the node has not accepted a block with a different rule set it will allow a // new upgrade to be applied as long as it activates after the last accepted block. func (c *ChainConfig) CheckPrecompilesCompatible(precompileUpgrades []PrecompileUpgrade, time uint64) *ConfigCompatError { - addrs := c.allPrecompileAddressesPlus(precompileUpgrades...) - for _, a := range addrs { + for _, a := range c.allPrecompileAddressesPlus(precompileUpgrades...) { if err := c.checkPrecompileCompatible(a, precompileUpgrades, time); err != nil { return err }