From 5359f70d11957ca19ebbc76fca8d27cd29a560e0 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 24 Aug 2022 17:30:25 +0300 Subject: [PATCH 01/31] add initial precompile configs and handle them --- precompile/allow_list.go | 11 ++++++++++- precompile/contract_native_minter.go | 7 +++++++ precompile/fee_config_manager.go | 6 ++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index 66eab623bf..ce70595bd6 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -43,12 +43,17 @@ var ( // AllowListConfig specifies the initial set of allow list admins. type AllowListConfig struct { - AllowListAdmins []common.Address `json:"adminAddresses"` + AllowListAdmins []common.Address `json:"adminAddresses"` + EnabledAddresses []common.Address `json:"enabledAddresses,omitempty"` // initial enabled addresses } // Configure initializes the address space of [precompileAddr] by initializing the role of each of // the addresses in [AllowListAdmins]. func (c *AllowListConfig) Configure(state StateDB, precompileAddr common.Address) { + // First set enabled roles so these can be upgraded to admin addresses below. + for _, enabledAddr := range c.EnabledAddresses { + setAllowListRole(state, precompileAddr, enabledAddr, AllowListEnabled) + } for _, adminAddr := range c.AllowListAdmins { setAllowListRole(state, precompileAddr, adminAddr, AllowListAdmin) } @@ -125,6 +130,10 @@ func setAllowListRole(stateDB StateDB, precompileAddr, address common.Address, r // Generate the state key for [address] addressKey := address.Hash() // Assign [role] to the address + // This stores the [role] in the contract storage with address [precompileAddr] + // and [addressKey] hash. It means that any reusage of the [addressKey] for different value + // conflicts with the same slot [role] is stored. + // Precompile implementations must use a different key than [addressKey] stateDB.SetState(precompileAddr, addressKey, common.Hash(role)) } diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index d26458b6f7..2fdda08481 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -35,6 +35,8 @@ var ( type ContractNativeMinterConfig struct { AllowListConfig UpgradeableConfig + // TODO: use hex amounts? + InitialMint map[common.Address]*big.Int `json:"initialMint,omitempty"` // initial mint config to be immediately minted } // NewContractNativeMinterConfig returns a config for a network upgrade at [blockTimestamp] that enables @@ -64,6 +66,11 @@ func (c *ContractNativeMinterConfig) Address() common.Address { // Configure configures [state] with the desired admins based on [c]. func (c *ContractNativeMinterConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) { + if len(c.InitialMint) != 0 { + for to, amount := range c.InitialMint { + state.AddBalance(to, amount) + } + } c.AllowListConfig.Configure(state, ContractNativeMinterAddress) } diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index ae28e5b38b..e1f08aeaac 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -56,6 +56,7 @@ var ( type FeeConfigManagerConfig struct { AllowListConfig // Config for the fee config manager allow list UpgradeableConfig + InitialFeeConfig *commontype.FeeConfig `json:"initialFeeConfig,omitempty"` // initial fee config to be immediately activated } // NewFeeManagerConfig returns a config for a network upgrade at [blockTimestamp] that enables @@ -96,6 +97,11 @@ func (c *FeeConfigManagerConfig) Equal(s StatefulPrecompileConfig) bool { // Configure configures [state] with the desired admins based on [c]. func (c *FeeConfigManagerConfig) Configure(chainConfig ChainConfig, state StateDB, blockContext BlockContext) { // Store the initial fee config into the state when the fee config manager activates. + if c.InitialFeeConfig != nil { + if err := StoreFeeConfig(state, chainConfig.GetFeeConfig(), blockContext); err != nil { + panic(fmt.Sprintf("invalid feeConfig provided: %s", err)) + } + } if err := StoreFeeConfig(state, chainConfig.GetFeeConfig(), blockContext); err != nil { panic(fmt.Sprintf("fee config should have been verified in genesis: %s", err)) } From 8a488c5fcb7a71af56fa40d205d76ca27b531b63 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 25 Aug 2022 21:57:15 +0300 Subject: [PATCH 02/31] add verify to precompile configs --- params/config.go | 2 +- params/precompile_config.go | 9 ++++++++- precompile/allow_list.go | 22 +++++++++++++++++++--- precompile/contract_native_minter.go | 9 ++++++--- precompile/fee_config_manager.go | 16 ++++++++++++---- precompile/stateful_precompile_config.go | 3 +++ 6 files changed, 49 insertions(+), 12 deletions(-) diff --git a/params/config.go b/params/config.go index ea798ed6c1..302227dadb 100644 --- a/params/config.go +++ b/params/config.go @@ -287,7 +287,7 @@ func (c *ChainConfig) Verify() error { } // Verify the precompile upgrades are internally consistent given the existing chainConfig. - if err := c.VerifyPrecompileUpgrades(); err != nil { + if err := c.verifyPrecompileUpgrades(); err != nil { return err } diff --git a/params/precompile_config.go b/params/precompile_config.go index 79561d3e52..986e04b7a6 100644 --- a/params/precompile_config.go +++ b/params/precompile_config.go @@ -66,7 +66,7 @@ func (p *PrecompileUpgrade) getByKey(key precompileKey) (precompile.StatefulPrec // - the specified blockTimestamps must be compatible with those // specified in the chainConfig by genesis. // - check a precompile is disabled before it is re-enabled -func (c *ChainConfig) VerifyPrecompileUpgrades() error { +func (c *ChainConfig) verifyPrecompileUpgrades() error { var lastBlockTimestamp *big.Int for i, upgrade := range c.PrecompileUpgrades { hasKey := false // used to verify if there is only one key per Upgrade @@ -103,6 +103,9 @@ func (c *ChainConfig) VerifyPrecompileUpgrades() error { ) // check the genesis chain config for any enabled upgrade if config, ok := c.PrecompileUpgrade.getByKey(key); ok { + if err := config.Verify(); err != nil { + return err + } disabled = false lastUpgraded = config.Timestamp() } else { @@ -123,6 +126,10 @@ func (c *ChainConfig) VerifyPrecompileUpgrades() error { return fmt.Errorf("PrecompileUpgrades[%d] config timestamp (%v) <= previous timestamp (%v)", i, config.Timestamp(), lastUpgraded) } + if err := config.Verify(); err != nil { + return err + } + disabled = config.IsDisabled() lastUpgraded = config.Timestamp() } diff --git a/precompile/allow_list.go b/precompile/allow_list.go index ce70595bd6..db16922356 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -12,9 +12,6 @@ import ( "github.com/ethereum/go-ethereum/common" ) -// Enum constants for valid AllowListRole -type AllowListRole common.Hash - const ( SetAdminFuncKey = "setAdmin" SetEnabledFuncKey = "setEnabled" @@ -75,6 +72,25 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { return true } +// Verify returns an error if there is an overlapping address between admins and enableds +func (c *AllowListConfig) Verify() error { + enabledMap := make(map[common.Address]struct{}) + for _, enabledAddr := range c.EnabledAddresses { + if _, ok := enabledMap[enabledAddr]; !ok { + enabledMap[enabledAddr] = struct{}{} + } + } + for _, adminAddr := range c.AllowListAdmins { + if _, ok := enabledMap[adminAddr]; ok { + return fmt.Errorf("cannot set address %s as both admin and enabled", adminAddr) + } + } + return nil +} + +// Enum constants for valid AllowListRole +type AllowListRole common.Hash + // Valid returns true iff [s] represents a valid role. func (s AllowListRole) Valid() bool { switch s { diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index 2fdda08481..d0e29ae4fe 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -10,6 +10,7 @@ import ( "github.com/ava-labs/subnet-evm/vmerrs" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/math" ) const ( @@ -35,8 +36,7 @@ var ( type ContractNativeMinterConfig struct { AllowListConfig UpgradeableConfig - // TODO: use hex amounts? - InitialMint map[common.Address]*big.Int `json:"initialMint,omitempty"` // initial mint config to be immediately minted + InitialMint map[common.Address]*math.HexOrDecimal256 `json:"initialMint,omitempty"` // initial mint config to be immediately minted } // NewContractNativeMinterConfig returns a config for a network upgrade at [blockTimestamp] that enables @@ -68,7 +68,10 @@ func (c *ContractNativeMinterConfig) Address() common.Address { func (c *ContractNativeMinterConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) { if len(c.InitialMint) != 0 { for to, amount := range c.InitialMint { - state.AddBalance(to, amount) + if amount != nil { + bigIntAmount := (*big.Int)(amount) + state.AddBalance(to, bigIntAmount) + } } } c.AllowListConfig.Configure(state, ContractNativeMinterAddress) diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index e1f08aeaac..c33079601b 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -98,12 +98,13 @@ func (c *FeeConfigManagerConfig) Equal(s StatefulPrecompileConfig) bool { func (c *FeeConfigManagerConfig) Configure(chainConfig ChainConfig, state StateDB, blockContext BlockContext) { // Store the initial fee config into the state when the fee config manager activates. if c.InitialFeeConfig != nil { - if err := StoreFeeConfig(state, chainConfig.GetFeeConfig(), blockContext); err != nil { + if err := StoreFeeConfig(state, *c.InitialFeeConfig, blockContext); err != nil { panic(fmt.Sprintf("invalid feeConfig provided: %s", err)) } - } - if err := StoreFeeConfig(state, chainConfig.GetFeeConfig(), blockContext); err != nil { - panic(fmt.Sprintf("fee config should have been verified in genesis: %s", err)) + } else { + if err := StoreFeeConfig(state, chainConfig.GetFeeConfig(), blockContext); err != nil { + panic(fmt.Sprintf("fee config should have been verified in genesis: %s", err)) + } } c.AllowListConfig.Configure(state, FeeConfigManagerAddress) } @@ -113,6 +114,13 @@ func (c *FeeConfigManagerConfig) Contract() StatefulPrecompiledContract { return FeeConfigManagerPrecompile } +func (c *FeeConfigManagerConfig) Verify() error { + if err := c.AllowListConfig.Verify(); err != nil { + return err + } + return c.InitialFeeConfig.Verify() +} + // GetFeeConfigManagerStatus returns the role of [address] for the fee config manager list. func GetFeeConfigManagerStatus(stateDB StateDB, address common.Address) AllowListRole { return getAllowListStatus(stateDB, FeeConfigManagerAddress, address) diff --git a/precompile/stateful_precompile_config.go b/precompile/stateful_precompile_config.go index 70ec2d0a56..e82493fb60 100644 --- a/precompile/stateful_precompile_config.go +++ b/precompile/stateful_precompile_config.go @@ -35,6 +35,9 @@ type StatefulPrecompileConfig interface { // Contract returns a thread-safe singleton that can be used as the StatefulPrecompiledContract when // this config is enabled. Contract() StatefulPrecompiledContract + // Verify is called before configuring stateful precompile config. It is important to capture any invalid configuration + // before activation. + Verify() error } // Configure sets the nonce and code to non-empty values then calls Configure on [precompileConfig] to make the necessary From 538f5987926fcbc293e5cabe61cfbc6b6393c364 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 25 Aug 2022 21:57:36 +0300 Subject: [PATCH 03/31] add unit tests for initial configs and verify --- accounts/abi/bind/precompile_template.go | 14 +++ core/stateful_precompile_test.go | 53 +++++++++ params/precompile_config_test.go | 142 ++++++++++++++++++++--- 3 files changed, 190 insertions(+), 19 deletions(-) diff --git a/accounts/abi/bind/precompile_template.go b/accounts/abi/bind/precompile_template.go index 261942fcfc..b7203f7bc8 100644 --- a/accounts/abi/bind/precompile_template.go +++ b/accounts/abi/bind/precompile_template.go @@ -192,6 +192,20 @@ func (c *{{.Contract.Type}}Config) Contract() StatefulPrecompiledContract { return {{.Contract.Type}}Precompile } +// Verify tries to verify {{.Contract.Type}}Config and returns an error accordingly. +func (c *{{.Contract.Type}}Config) Verify() error { + {{if .Contract.AllowList}} + // Verify AllowList first + if err := c.AllowListConfig.Verify(); err != nil{ + return err + } + {{end}} + // CUSTOM CODE STARTS HERE + // Add your own custom verify code for {{.Contract.Type}}Config here + // and return an error accordingly + return nil +} + {{if .Contract.AllowList}} // Get{{.Contract.Type}}AllowListStatus returns the role of [address] for the {{.Contract.Type}} list. func Get{{.Contract.Type}}AllowListStatus(stateDB StateDB, address common.Address) AllowListRole { diff --git a/core/stateful_precompile_test.go b/core/stateful_precompile_test.go index d53017f647..8887a33fc9 100644 --- a/core/stateful_precompile_test.go +++ b/core/stateful_precompile_test.go @@ -11,6 +11,7 @@ import ( "github.com/ava-labs/subnet-evm/commontype" "github.com/ava-labs/subnet-evm/core/rawdb" "github.com/ava-labs/subnet-evm/core/state" + "github.com/ava-labs/subnet-evm/params" "github.com/ava-labs/subnet-evm/precompile" "github.com/ava-labs/subnet-evm/vmerrs" "github.com/ethereum/go-ethereum/common" @@ -535,6 +536,7 @@ func TestContractNativeMinterRun(t *testing.T) { input func() []byte suppliedGas uint64 readOnly bool + config *precompile.ContractNativeMinterConfig expectedRes []byte expectedErr string @@ -581,6 +583,27 @@ func TestContractNativeMinterRun(t *testing.T) { assert.Equal(t, common.Big1, state.GetBalance(allowAddr), "expected minted funds") }, }, + "initial mint funds": { + caller: allowAddr, + precompileAddr: precompile.ContractNativeMinterAddress, + config: &precompile.ContractNativeMinterConfig{ + InitialMint: map[common.Address]*math.HexOrDecimal256{ + allowAddr: math.NewHexOrDecimal256(2), + }, + }, + input: func() []byte { + return precompile.PackReadAllowList(noRoleAddr) + }, + suppliedGas: precompile.ReadAllowListGasCost, + readOnly: false, + expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), + assertState: func(t *testing.T, state *state.StateDB) { + res := precompile.GetContractNativeMinterStatus(state, allowAddr) + assert.Equal(t, precompile.AllowListEnabled, res) + + assert.Equal(t, common.Big2, state.GetBalance(allowAddr), "expected minted funds") + }, + }, "mint funds from admin address": { caller: adminAddr, precompileAddr: precompile.ContractNativeMinterAddress, @@ -756,6 +779,9 @@ func TestContractNativeMinterRun(t *testing.T) { precompile.SetContractNativeMinterStatus(state, allowAddr, precompile.AllowListEnabled) precompile.SetContractNativeMinterStatus(state, noRoleAddr, precompile.AllowListNoRole) blockContext := &mockBlockContext{blockNumber: common.Big0} + if test.config != nil { + test.config.Configure(params.TestChainConfig, state, blockContext) + } ret, remainingGas, err := precompile.ContractNativeMinterPrecompile.Run(&mockAccessibleState{state: state, blockContext: blockContext}, test.caller, test.precompileAddr, test.input(), test.suppliedGas, test.readOnly) if len(test.expectedErr) != 0 { if err == nil { @@ -788,6 +814,7 @@ func TestFeeConfigManagerRun(t *testing.T) { input func() []byte suppliedGas uint64 readOnly bool + config *precompile.FeeConfigManagerConfig expectedRes []byte expectedErr string @@ -908,6 +935,29 @@ func TestFeeConfigManagerRun(t *testing.T) { assert.EqualValues(t, big.NewInt(6), lastChangedAt) }, }, + "get initial fee config": { + caller: noRoleAddr, + precompileAddr: precompile.FeeConfigManagerAddress, + input: func() []byte { + return precompile.PackGetFeeConfigInput() + }, + suppliedGas: precompile.GetFeeConfigGasCost, + config: &precompile.FeeConfigManagerConfig{ + InitialFeeConfig: &testFeeConfig, + }, + readOnly: true, + expectedRes: func() []byte { + res, err := precompile.PackFeeConfig(testFeeConfig) + assert.NoError(t, err) + return res + }(), + assertState: func(t *testing.T, state *state.StateDB) { + feeConfig := precompile.GetStoredFeeConfig(state) + lastChangedAt := precompile.GetFeeConfigLastChangedAt(state) + assert.Equal(t, testFeeConfig, feeConfig) + assert.EqualValues(t, testBlockNumber, lastChangedAt) + }, + }, "get last changed at from non-enabled address": { caller: noRoleAddr, precompileAddr: precompile.FeeConfigManagerAddress, @@ -1038,6 +1088,9 @@ func TestFeeConfigManagerRun(t *testing.T) { } blockContext := &mockBlockContext{blockNumber: testBlockNumber} + if test.config != nil { + test.config.Configure(params.TestChainConfig, state, blockContext) + } ret, remainingGas, err := precompile.FeeConfigManagerPrecompile.Run(&mockAccessibleState{state: state, blockContext: blockContext}, test.caller, test.precompileAddr, test.input(), test.suppliedGas, test.readOnly) if len(test.expectedErr) != 0 { if err == nil { diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index 6c0fb521fc..cf29b3c044 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -7,19 +7,21 @@ import ( "math/big" "testing" + "github.com/ava-labs/subnet-evm/commontype" "github.com/ava-labs/subnet-evm/precompile" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidateWithChainConfig(t *testing.T) { admins := []common.Address{{1}} - config := &ChainConfig{ - PrecompileUpgrade: PrecompileUpgrade{ - TxAllowListConfig: &precompile.TxAllowListConfig{ - UpgradeableConfig: precompile.UpgradeableConfig{ - BlockTimestamp: big.NewInt(2), - }, + baseConfig := *SubnetEVMDefaultChainConfig + config := &baseConfig + config.PrecompileUpgrade = PrecompileUpgrade{ + TxAllowListConfig: &precompile.TxAllowListConfig{ + UpgradeableConfig: precompile.UpgradeableConfig{ + BlockTimestamp: big.NewInt(2), }, }, } @@ -35,7 +37,7 @@ func TestValidateWithChainConfig(t *testing.T) { } // check this config is valid - err := config.VerifyPrecompileUpgrades() + err := config.Verify() assert.NoError(t, err) // same precompile cannot be configured twice for the same timestamp @@ -46,7 +48,7 @@ func TestValidateWithChainConfig(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(5)), }, ) - err = badConfig.VerifyPrecompileUpgrades() + err = badConfig.Verify() assert.ErrorContains(t, err, "config timestamp (5) <= previous timestamp (5)") // cannot enable a precompile without disabling it first. @@ -57,30 +59,132 @@ func TestValidateWithChainConfig(t *testing.T) { TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(5), admins), }, ) - err = badConfig.VerifyPrecompileUpgrades() + err = badConfig.Verify() assert.ErrorContains(t, err, "disable should be [true]") } -func TestValidate(t *testing.T) { +func TestVerifyPrecompileUpgrades(t *testing.T) { admins := []common.Address{{1}} - config := &ChainConfig{} - config.PrecompileUpgrades = []PrecompileUpgrade{ + tests := []struct { + name string + upgrades []PrecompileUpgrade + expectedError string + }{ { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins), + name: "enable and disable tx allow list", + upgrades: []PrecompileUpgrade{ + { + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins), + }, + { + TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(2)), + }, + }, + expectedError: "", }, { - TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(2)), + name: "invalid allow list config in tx allowlist", + upgrades: []PrecompileUpgrade{ + { + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins), + }, + { + TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(2)), + }, + { + TxAllowListConfig: &precompile.TxAllowListConfig{ + AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, + UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + }, + }, + }, + expectedError: "cannot set address", + }, + { + name: "invalid initial fee manager config", + upgrades: []PrecompileUpgrade{ + { + FeeManagerConfig: &precompile.FeeConfigManagerConfig{ + AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins}, + UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + InitialFeeConfig: &commontype.FeeConfig{ + GasLimit: big.NewInt(-1), + }, + }, + }, + }, + expectedError: "gasLimit = -1 cannot be less than or equal to 0", }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + baseConfig := *SubnetEVMDefaultChainConfig + config := &baseConfig + config.PrecompileUpgrades = tt.upgrades + + err := config.Verify() + if tt.expectedError == "" { + require.NoError(err) + } else { + require.ErrorContains(err, tt.expectedError) + } + }) + } +} - // check this config is valid - err := config.VerifyPrecompileUpgrades() - assert.NoError(t, err) +func TestVerifyPrecompiles(t *testing.T) { + admins := []common.Address{{1}} + tests := []struct { + name string + upgrade PrecompileUpgrade + expectedError string + }{ + { + name: "invalid allow list config in tx allowlist", + upgrade: PrecompileUpgrade{ + TxAllowListConfig: &precompile.TxAllowListConfig{ + AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, + UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + }, + }, + expectedError: "cannot set address", + }, + { + name: "invalid initial fee manager config", + upgrade: PrecompileUpgrade{ + FeeManagerConfig: &precompile.FeeConfigManagerConfig{ + AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins}, + UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + InitialFeeConfig: &commontype.FeeConfig{ + GasLimit: big.NewInt(-1), + }, + }, + }, + expectedError: "gasLimit = -1 cannot be less than or equal to 0", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + baseConfig := *SubnetEVMDefaultChainConfig + config := &baseConfig + config.PrecompileUpgrade = tt.upgrade + + err := config.Verify() + if tt.expectedError == "" { + require.NoError(err) + } else { + require.ErrorContains(err, tt.expectedError) + } + }) + } } func TestValidateRequiresSortedTimestamps(t *testing.T) { admins := []common.Address{{1}} - config := &ChainConfig{} + baseConfig := *SubnetEVMDefaultChainConfig + config := &baseConfig config.PrecompileUpgrades = []PrecompileUpgrade{ { TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), admins), @@ -91,7 +195,7 @@ func TestValidateRequiresSortedTimestamps(t *testing.T) { } // block timestamps must be monotonically increasing, so this config is invalid - err := config.VerifyPrecompileUpgrades() + err := config.Verify() assert.ErrorContains(t, err, "config timestamp (1) < previous timestamp (2)") } From 27e207fd69271b59e507556f9019777c69db9a6f Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 26 Aug 2022 12:22:31 +0300 Subject: [PATCH 04/31] add a nil check for initial fee config verification --- precompile/fee_config_manager.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index c33079601b..066774c412 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -118,7 +118,10 @@ func (c *FeeConfigManagerConfig) Verify() error { if err := c.AllowListConfig.Verify(); err != nil { return err } - return c.InitialFeeConfig.Verify() + if c.InitialFeeConfig != nil { + return c.InitialFeeConfig.Verify() + } + return nil } // GetFeeConfigManagerStatus returns the role of [address] for the fee config manager list. From 97d34722a7afe1efd2033e3018501e859f6bd5e1 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 26 Aug 2022 14:52:56 +0300 Subject: [PATCH 05/31] conditionally verify allowlist --- precompile/allow_list.go | 19 +++++++++++-------- precompile/contract_native_minter.go | 11 +++++------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index db16922356..b58328e06e 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -74,15 +74,18 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { // Verify returns an error if there is an overlapping address between admins and enableds func (c *AllowListConfig) Verify() error { - enabledMap := make(map[common.Address]struct{}) - for _, enabledAddr := range c.EnabledAddresses { - if _, ok := enabledMap[enabledAddr]; !ok { - enabledMap[enabledAddr] = struct{}{} + // check if both lists are empty + if len(c.EnabledAddresses) != 0 && len(c.AllowListAdmins) != 0 { + enabledMap := make(map[common.Address]struct{}) + for _, enabledAddr := range c.EnabledAddresses { + if _, ok := enabledMap[enabledAddr]; !ok { + enabledMap[enabledAddr] = struct{}{} + } } - } - for _, adminAddr := range c.AllowListAdmins { - if _, ok := enabledMap[adminAddr]; ok { - return fmt.Errorf("cannot set address %s as both admin and enabled", adminAddr) + for _, adminAddr := range c.AllowListAdmins { + if _, ok := enabledMap[adminAddr]; ok { + return fmt.Errorf("cannot set address %s as both admin and enabled", adminAddr) + } } } return nil diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index d0e29ae4fe..32d16544c7 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -66,14 +66,13 @@ func (c *ContractNativeMinterConfig) Address() common.Address { // Configure configures [state] with the desired admins based on [c]. func (c *ContractNativeMinterConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) { - if len(c.InitialMint) != 0 { - for to, amount := range c.InitialMint { - if amount != nil { - bigIntAmount := (*big.Int)(amount) - state.AddBalance(to, bigIntAmount) - } + for to, amount := range c.InitialMint { + if amount != nil { + bigIntAmount := (*big.Int)(amount) + state.AddBalance(to, bigIntAmount) } } + c.AllowListConfig.Configure(state, ContractNativeMinterAddress) } From e70a80869f85f2a300688ec94402e35a0a74786a Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Mon, 29 Aug 2022 23:35:04 +0300 Subject: [PATCH 06/31] Update precompile/stateful_precompile_config.go Co-authored-by: aaronbuchwald --- precompile/stateful_precompile_config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/precompile/stateful_precompile_config.go b/precompile/stateful_precompile_config.go index e82493fb60..4e4490cae7 100644 --- a/precompile/stateful_precompile_config.go +++ b/precompile/stateful_precompile_config.go @@ -35,8 +35,7 @@ type StatefulPrecompileConfig interface { // Contract returns a thread-safe singleton that can be used as the StatefulPrecompiledContract when // this config is enabled. Contract() StatefulPrecompiledContract - // Verify is called before configuring stateful precompile config. It is important to capture any invalid configuration - // before activation. + // Verify is called on startup and an error is treated as fatal. Configure can assume the Config has passed verification. Verify() error } From 38cc210f035f88f2fbb0dfb6996dadb292fe155b Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Aug 2022 15:20:07 +0300 Subject: [PATCH 07/31] add config test to precompile package --- precompile/config_test.go | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 precompile/config_test.go diff --git a/precompile/config_test.go b/precompile/config_test.go new file mode 100644 index 0000000000..2eea178f30 --- /dev/null +++ b/precompile/config_test.go @@ -0,0 +1,78 @@ +// (c) 2022 Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package precompile + +import ( + "math/big" + "testing" + + "github.com/ava-labs/subnet-evm/commontype" + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/require" +) + +func TestVerifyPrecompileUpgrades(t *testing.T) { + admins := []common.Address{{1}} + tests := []struct { + name string + config StatefulPrecompileConfig + expectedError string + }{ + { + name: "invalid allow list config in tx allowlist", + config: &TxAllowListConfig{ + AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, + UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + }, + expectedError: "cannot set address", + }, + { + name: "invalid allow list config in deployer allowlist", + config: &ContractDeployerAllowListConfig{ + AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, + UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + }, + expectedError: "cannot set address", + }, + { + name: "invalid allow list config in native minter allowlist", + config: &ContractNativeMinterConfig{ + AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, + UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + }, + expectedError: "cannot set address", + }, + { + name: "invalid allow list config in fee manager allowlist", + config: &FeeConfigManagerConfig{ + AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, + UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + }, + expectedError: "cannot set address", + }, + { + name: "invalid initial fee manager config", + config: &FeeConfigManagerConfig{ + AllowListConfig: AllowListConfig{AllowListAdmins: admins}, + UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + InitialFeeConfig: &commontype.FeeConfig{ + GasLimit: big.NewInt(0), + }, + }, + expectedError: "gasLimit = 0 cannot be less than or equal to 0", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + err := tt.config.Verify() + if tt.expectedError == "" { + require.NoError(err) + } else { + require.ErrorContains(err, tt.expectedError) + } + }) + } +} From 55d59e4070d940df45732ee8b8479d10110c92c0 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Aug 2022 15:23:17 +0300 Subject: [PATCH 08/31] add 0 test case for gas limit --- params/precompile_config_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index cf29b3c044..b7c6a082ee 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -115,6 +115,21 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { }, expectedError: "gasLimit = -1 cannot be less than or equal to 0", }, + { + name: "invalid initial fee manager config gas limit 0", + upgrades: []PrecompileUpgrade{ + { + FeeManagerConfig: &precompile.FeeConfigManagerConfig{ + AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins}, + UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, + InitialFeeConfig: &commontype.FeeConfig{ + GasLimit: big.NewInt(0), + }, + }, + }, + }, + expectedError: "gasLimit = 0 cannot be less than or equal to 0", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 258ae40b6b714e6a0b0739bc30f320d762399742 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Aug 2022 15:38:44 +0300 Subject: [PATCH 09/31] add comments for allowlist storage keys --- accounts/abi/bind/precompile_template.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/accounts/abi/bind/precompile_template.go b/accounts/abi/bind/precompile_template.go index b7203f7bc8..9a10f64bf7 100644 --- a/accounts/abi/bind/precompile_template.go +++ b/accounts/abi/bind/precompile_template.go @@ -214,6 +214,10 @@ func Get{{.Contract.Type}}AllowListStatus(stateDB StateDB, address common.Addres // Set{{.Contract.Type}}AllowListStatus sets the permissions of [address] to [role] for the // {{.Contract.Type}} list. Assumes [role] has already been verified as valid. +// This stores the [role] in the contract storage with address [{{.Contract.Type}}Address] +// and [address] hash. It means that any reusage of the [address] key for different value +// conflicts with the same slot [role] is stored. +// Precompile implementations must use a different key than [address] for their storage. func Set{{.Contract.Type}}AllowListStatus(stateDB StateDB, address common.Address, role AllowListRole) { setAllowListRole(stateDB, {{.Contract.Type}}Address, address, role) } From c60ec84b2bfa449f013b95556f0e897d570f496f Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 31 Aug 2022 16:26:24 +0300 Subject: [PATCH 10/31] move allow list role to different file --- precompile/allow_list.go | 65 +++++++---------------------------- precompile/allow_list_role.go | 49 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 53 deletions(-) create mode 100644 precompile/allow_list_role.go diff --git a/precompile/allow_list.go b/precompile/allow_list.go index b58328e06e..46501af351 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -75,63 +75,22 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { // Verify returns an error if there is an overlapping address between admins and enableds func (c *AllowListConfig) Verify() error { // check if both lists are empty - if len(c.EnabledAddresses) != 0 && len(c.AllowListAdmins) != 0 { - enabledMap := make(map[common.Address]struct{}) - for _, enabledAddr := range c.EnabledAddresses { - if _, ok := enabledMap[enabledAddr]; !ok { - enabledMap[enabledAddr] = struct{}{} - } - } - for _, adminAddr := range c.AllowListAdmins { - if _, ok := enabledMap[adminAddr]; ok { - return fmt.Errorf("cannot set address %s as both admin and enabled", adminAddr) - } - } - } - return nil -} - -// Enum constants for valid AllowListRole -type AllowListRole common.Hash - -// Valid returns true iff [s] represents a valid role. -func (s AllowListRole) Valid() bool { - switch s { - case AllowListNoRole, AllowListEnabled, AllowListAdmin: - return true - default: - return false + if len(c.EnabledAddresses) == 0 || len(c.AllowListAdmins) == 0 { + return nil } -} - -// IsNoRole returns true if [s] indicates no specific role. -func (s AllowListRole) IsNoRole() bool { - switch s { - case AllowListNoRole: - return true - default: - return false + enabledMap := make(map[common.Address]struct{}) + for _, enabledAddr := range c.EnabledAddresses { + if _, ok := enabledMap[enabledAddr]; !ok { + enabledMap[enabledAddr] = struct{}{} + } } -} - -// IsAdmin returns true if [s] indicates the permission to modify the allow list. -func (s AllowListRole) IsAdmin() bool { - switch s { - case AllowListAdmin: - return true - default: - return false + for _, adminAddr := range c.AllowListAdmins { + if _, ok := enabledMap[adminAddr]; ok { + return fmt.Errorf("cannot set address %s as both admin and enabled", adminAddr) + } } -} -// IsEnabled returns true if [s] indicates that it has permission to access the resource. -func (s AllowListRole) IsEnabled() bool { - switch s { - case AllowListAdmin, AllowListEnabled: - return true - default: - return false - } + return nil } // getAllowListStatus returns the allow list role of [address] for the precompile diff --git a/precompile/allow_list_role.go b/precompile/allow_list_role.go new file mode 100644 index 0000000000..0c815d0819 --- /dev/null +++ b/precompile/allow_list_role.go @@ -0,0 +1,49 @@ +// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package precompile + +import "github.com/ethereum/go-ethereum/common" + +// Enum constants for valid AllowListRole +type AllowListRole common.Hash + +// Valid returns true iff [s] represents a valid role. +func (s AllowListRole) Valid() bool { + switch s { + case AllowListNoRole, AllowListEnabled, AllowListAdmin: + return true + default: + return false + } +} + +// IsNoRole returns true if [s] indicates no specific role. +func (s AllowListRole) IsNoRole() bool { + switch s { + case AllowListNoRole: + return true + default: + return false + } +} + +// IsAdmin returns true if [s] indicates the permission to modify the allow list. +func (s AllowListRole) IsAdmin() bool { + switch s { + case AllowListAdmin: + return true + default: + return false + } +} + +// IsEnabled returns true if [s] indicates that it has permission to access the resource. +func (s AllowListRole) IsEnabled() bool { + switch s { + case AllowListAdmin, AllowListEnabled: + return true + default: + return false + } +} From bf00bb567e1e3e6868c9fe4ae0d0cf6c66c38172 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 31 Aug 2022 16:28:54 +0300 Subject: [PATCH 11/31] Update precompile/allow_list.go Co-authored-by: aaronbuchwald --- precompile/allow_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index b58328e06e..6717915266 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -41,7 +41,7 @@ var ( // AllowListConfig specifies the initial set of allow list admins. type AllowListConfig struct { AllowListAdmins []common.Address `json:"adminAddresses"` - EnabledAddresses []common.Address `json:"enabledAddresses,omitempty"` // initial enabled addresses + EnabledAddresses []common.Address `json:"enabledAddresses"` // initial enabled addresses } // Configure initializes the address space of [precompileAddr] by initializing the role of each of From 65bd9e8da49653987e99d18192dafb695afbca10 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 31 Aug 2022 16:29:53 +0300 Subject: [PATCH 12/31] rename test --- params/precompile_config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index b7c6a082ee..55445fbf9e 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestValidateWithChainConfig(t *testing.T) { +func TestVerifyWithChainConfig(t *testing.T) { admins := []common.Address{{1}} baseConfig := *SubnetEVMDefaultChainConfig config := &baseConfig From dfd41f57ade474a26a8d5a43925e1a7004d93511 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 31 Aug 2022 16:32:05 +0300 Subject: [PATCH 13/31] Update precompile/allow_list.go Co-authored-by: Darioush Jalali --- precompile/allow_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index 6717915266..59626ee653 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -72,7 +72,7 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { return true } -// Verify returns an error if there is an overlapping address between admins and enableds +// Verify returns an error if there is an overlapping address between admin and enabled roles func (c *AllowListConfig) Verify() error { // check if both lists are empty if len(c.EnabledAddresses) != 0 && len(c.AllowListAdmins) != 0 { From 8075ba12fbaa31ebd7cad5c9c7842dfaa4a0955e Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 31 Aug 2022 16:32:45 +0300 Subject: [PATCH 14/31] rename test & use func to create config --- params/precompile_config_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index 55445fbf9e..dd3e77a986 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -19,11 +19,7 @@ func TestVerifyWithChainConfig(t *testing.T) { baseConfig := *SubnetEVMDefaultChainConfig config := &baseConfig config.PrecompileUpgrade = PrecompileUpgrade{ - TxAllowListConfig: &precompile.TxAllowListConfig{ - UpgradeableConfig: precompile.UpgradeableConfig{ - BlockTimestamp: big.NewInt(2), - }, - }, + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), []common.Address{}), } config.PrecompileUpgrades = []PrecompileUpgrade{ { @@ -196,7 +192,7 @@ func TestVerifyPrecompiles(t *testing.T) { } } -func TestValidateRequiresSortedTimestamps(t *testing.T) { +func TestVerifyRequiresSortedTimestamps(t *testing.T) { admins := []common.Address{{1}} baseConfig := *SubnetEVMDefaultChainConfig config := &baseConfig From 3ffa6dcbcf524790a983200b0d2a764c27cc42b6 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 31 Aug 2022 16:58:39 +0300 Subject: [PATCH 15/31] Remove unnecessary role checks --- core/stateful_precompile_test.go | 107 +++++++++++-------------------- 1 file changed, 38 insertions(+), 69 deletions(-) diff --git a/core/stateful_precompile_test.go b/core/stateful_precompile_test.go index 8887a33fc9..bec0c8fece 100644 --- a/core/stateful_precompile_test.go +++ b/core/stateful_precompile_test.go @@ -90,10 +90,7 @@ func TestContractDeployerAllowListRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractDeployerAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - - res = precompile.GetContractDeployerAllowListStatus(state, noRoleAddr) + res := precompile.GetContractDeployerAllowListStatus(state, noRoleAddr) assert.Equal(t, precompile.AllowListAdmin, res) }, }, @@ -111,10 +108,7 @@ func TestContractDeployerAllowListRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractDeployerAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - - res = precompile.GetContractDeployerAllowListStatus(state, noRoleAddr) + res := precompile.GetContractDeployerAllowListStatus(state, noRoleAddr) assert.Equal(t, precompile.AllowListEnabled, res) }, }, @@ -215,10 +209,7 @@ func TestContractDeployerAllowListRun(t *testing.T) { suppliedGas: precompile.ReadAllowListGasCost, readOnly: false, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), - assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractDeployerAllowListStatus(state, noRoleAddr) - assert.Equal(t, precompile.AllowListNoRole, res) - }, + assertState: nil, }, "read allow list admin role": { caller: adminAddr, @@ -229,10 +220,7 @@ func TestContractDeployerAllowListRun(t *testing.T) { suppliedGas: precompile.ReadAllowListGasCost, readOnly: false, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), - assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractDeployerAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - }, + assertState: nil, }, "read allow list with readOnly enabled": { caller: adminAddr, @@ -243,10 +231,7 @@ func TestContractDeployerAllowListRun(t *testing.T) { suppliedGas: precompile.ReadAllowListGasCost, readOnly: true, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), - assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractDeployerAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - }, + assertState: nil, }, "read allow list out of gas": { caller: adminAddr, @@ -269,6 +254,9 @@ func TestContractDeployerAllowListRun(t *testing.T) { // Set up the state so that each address has the expected permissions at the start. precompile.SetContractDeployerAllowListStatus(state, adminAddr, precompile.AllowListAdmin) precompile.SetContractDeployerAllowListStatus(state, noRoleAddr, precompile.AllowListNoRole) + assert.Equal(t, precompile.AllowListAdmin, precompile.GetContractDeployerAllowListStatus(state, adminAddr)) + assert.Equal(t, precompile.AllowListNoRole, precompile.GetContractDeployerAllowListStatus(state, noRoleAddr)) + blockContext := &mockBlockContext{blockNumber: common.Big0} ret, remainingGas, err := precompile.ContractDeployerAllowListPrecompile.Run(&mockAccessibleState{state: state, blockContext: blockContext}, test.caller, test.precompileAddr, test.input(), test.suppliedGas, test.readOnly) if len(test.expectedErr) != 0 { @@ -326,10 +314,7 @@ func TestTxAllowListRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetTxAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - - res = precompile.GetTxAllowListStatus(state, noRoleAddr) + res := precompile.GetTxAllowListStatus(state, noRoleAddr) assert.Equal(t, precompile.AllowListAdmin, res) }, }, @@ -347,10 +332,7 @@ func TestTxAllowListRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetTxAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - - res = precompile.GetTxAllowListStatus(state, noRoleAddr) + res := precompile.GetTxAllowListStatus(state, noRoleAddr) assert.Equal(t, precompile.AllowListEnabled, res) }, }, @@ -451,10 +433,7 @@ func TestTxAllowListRun(t *testing.T) { suppliedGas: precompile.ReadAllowListGasCost, readOnly: false, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), - assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetTxAllowListStatus(state, noRoleAddr) - assert.Equal(t, precompile.AllowListNoRole, res) - }, + assertState: nil, }, "read allow list admin role": { caller: adminAddr, @@ -465,10 +444,7 @@ func TestTxAllowListRun(t *testing.T) { suppliedGas: precompile.ReadAllowListGasCost, readOnly: false, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), - assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetTxAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - }, + assertState: nil, }, "read allow list with readOnly enabled": { caller: adminAddr, @@ -479,10 +455,7 @@ func TestTxAllowListRun(t *testing.T) { suppliedGas: precompile.ReadAllowListGasCost, readOnly: true, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), - assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetTxAllowListStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - }, + assertState: nil, }, "read allow list out of gas": { caller: adminAddr, @@ -504,6 +477,8 @@ func TestTxAllowListRun(t *testing.T) { // Set up the state so that each address has the expected permissions at the start. precompile.SetTxAllowListStatus(state, adminAddr, precompile.AllowListAdmin) + assert.Equal(t, precompile.AllowListAdmin, precompile.GetTxAllowListStatus(state, adminAddr)) + blockContext := &mockBlockContext{blockNumber: common.Big0} ret, remainingGas, err := precompile.TxAllowListPrecompile.Run(&mockAccessibleState{state: state, blockContext: blockContext}, test.caller, test.precompileAddr, test.input(), test.suppliedGas, test.readOnly) if len(test.expectedErr) != 0 { @@ -547,6 +522,7 @@ func TestContractNativeMinterRun(t *testing.T) { adminAddr := common.HexToAddress("0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC") allowAddr := common.HexToAddress("0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B") noRoleAddr := common.HexToAddress("0xF60C45c607D0f41687c94C314d300f483661E13a") + testAddr := common.BigToAddress(common.Big2) for name, test := range map[string]test{ "mint funds from no role fails": { @@ -577,12 +553,25 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractNativeMinterStatus(state, allowAddr) - assert.Equal(t, precompile.AllowListEnabled, res) - assert.Equal(t, common.Big1, state.GetBalance(allowAddr), "expected minted funds") }, }, + "enabled role by config": { + caller: noRoleAddr, + precompileAddr: precompile.ContractNativeMinterAddress, + input: func() []byte { + return precompile.PackReadAllowList(testAddr) + }, + suppliedGas: precompile.ReadAllowListGasCost, + readOnly: false, + expectedRes: common.Hash(precompile.AllowListEnabled).Bytes(), + assertState: func(t *testing.T, state *state.StateDB) { + assert.Equal(t, precompile.AllowListEnabled, precompile.GetContractNativeMinterStatus(state, testAddr)) + }, + config: &precompile.ContractNativeMinterConfig{ + AllowListConfig: precompile.AllowListConfig{EnabledAddresses: []common.Address{testAddr}}, + }, + }, "initial mint funds": { caller: allowAddr, precompileAddr: precompile.ContractNativeMinterAddress, @@ -598,9 +587,6 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractNativeMinterStatus(state, allowAddr) - assert.Equal(t, precompile.AllowListEnabled, res) - assert.Equal(t, common.Big2, state.GetBalance(allowAddr), "expected minted funds") }, }, @@ -618,9 +604,6 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractNativeMinterStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - assert.Equal(t, common.Big1, state.GetBalance(adminAddr), "expected minted funds") }, }, @@ -638,9 +621,6 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractNativeMinterStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - assert.Equal(t, math.MaxBig256, state.GetBalance(adminAddr), "expected minted funds") }, }, @@ -746,10 +726,7 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetContractNativeMinterStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - - res = precompile.GetContractNativeMinterStatus(state, noRoleAddr) + res := precompile.GetContractNativeMinterStatus(state, noRoleAddr) assert.Equal(t, precompile.AllowListEnabled, res) }, }, @@ -778,6 +755,10 @@ func TestContractNativeMinterRun(t *testing.T) { precompile.SetContractNativeMinterStatus(state, adminAddr, precompile.AllowListAdmin) precompile.SetContractNativeMinterStatus(state, allowAddr, precompile.AllowListEnabled) precompile.SetContractNativeMinterStatus(state, noRoleAddr, precompile.AllowListNoRole) + assert.Equal(t, precompile.AllowListAdmin, precompile.GetContractNativeMinterStatus(state, adminAddr)) + assert.Equal(t, precompile.AllowListEnabled, precompile.GetContractNativeMinterStatus(state, allowAddr)) + assert.Equal(t, precompile.AllowListNoRole, precompile.GetContractNativeMinterStatus(state, noRoleAddr)) + blockContext := &mockBlockContext{blockNumber: common.Big0} if test.config != nil { test.config.Configure(params.TestChainConfig, state, blockContext) @@ -855,9 +836,6 @@ func TestFeeConfigManagerRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetFeeConfigManagerStatus(state, allowAddr) - assert.Equal(t, precompile.AllowListEnabled, res) - feeConfig := precompile.GetStoredFeeConfig(state) assert.Equal(t, testFeeConfig, feeConfig) }, @@ -879,9 +857,6 @@ func TestFeeConfigManagerRun(t *testing.T) { expectedRes: []byte{}, expectedErr: "cannot be greater than maxBlockGasCost", assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetFeeConfigManagerStatus(state, allowAddr) - assert.Equal(t, precompile.AllowListEnabled, res) - feeConfig := precompile.GetStoredFeeConfig(state) assert.Equal(t, testFeeConfig, feeConfig) }, @@ -900,9 +875,6 @@ func TestFeeConfigManagerRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetFeeConfigManagerStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - feeConfig := precompile.GetStoredFeeConfig(state) assert.Equal(t, testFeeConfig, feeConfig) lastChangedAt := precompile.GetFeeConfigLastChangedAt(state) @@ -1050,10 +1022,7 @@ func TestFeeConfigManagerRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - res := precompile.GetFeeConfigManagerStatus(state, adminAddr) - assert.Equal(t, precompile.AllowListAdmin, res) - - res = precompile.GetFeeConfigManagerStatus(state, noRoleAddr) + res := precompile.GetFeeConfigManagerStatus(state, noRoleAddr) assert.Equal(t, precompile.AllowListEnabled, res) }, }, From b844bbbabf3ea38b6511327dfb54090b5f86c4cb Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 1 Sep 2022 16:40:30 +0300 Subject: [PATCH 16/31] use config constructor for precompile tests --- core/genesis_test.go | 2 +- core/state_processor_test.go | 2 +- core/test_blockchain.go | 4 +- eth/gasprice/gasprice_test.go | 2 +- params/precompile_config_test.go | 53 ++++++++-------------- params/upgrade_config_test.go | 30 ++++++------ plugin/evm/vm_test.go | 8 ++-- plugin/evm/vm_upgrade_bytes_test.go | 2 +- precompile/config_test.go | 37 +++++---------- precompile/contract_deployer_allow_list.go | 9 ++-- precompile/contract_native_minter.go | 10 ++-- precompile/fee_config_manager.go | 10 ++-- precompile/tx_allow_list.go | 9 ++-- 13 files changed, 81 insertions(+), 97 deletions(-) diff --git a/core/genesis_test.go b/core/genesis_test.go index 5080d0baed..63762fd61d 100644 --- a/core/genesis_test.go +++ b/core/genesis_test.go @@ -183,7 +183,7 @@ func TestStatefulPrecompilesConfigure(t *testing.T) { "allow list enabled in genesis": { getConfig: func() *params.ChainConfig { config := *params.TestChainConfig - config.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(0), []common.Address{addr}) + config.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(0), []common.Address{addr}, nil) return &config }, assertState: func(t *testing.T, sdb *state.StateDB) { diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 42307dc0c1..3878641712 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -332,7 +332,7 @@ func TestBadTxAllowListBlock(t *testing.T) { SubnetEVMTimestamp: big.NewInt(0), }, PrecompileUpgrade: params.PrecompileUpgrade{ - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(0), []common.Address{}), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(0), nil, nil), }, } signer = types.LatestSigner(config) diff --git a/core/test_blockchain.go b/core/test_blockchain.go index 59ee062ce6..50ef515d62 100644 --- a/core/test_blockchain.go +++ b/core/test_blockchain.go @@ -1546,8 +1546,8 @@ func TestStatefulPrecompiles(t *testing.T, create func(db ethdb.Database, chainC genesisBalance := new(big.Int).Mul(big.NewInt(1000000), big.NewInt(params.Ether)) config := *params.TestChainConfig // Set all of the required config parameters - config.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(0), []common.Address{addr1}) - config.FeeManagerConfig = precompile.NewFeeManagerConfig(big.NewInt(0), []common.Address{addr1}) + config.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(0), []common.Address{addr1}, nil) + config.FeeManagerConfig = precompile.NewFeeManagerConfig(big.NewInt(0), []common.Address{addr1}, nil, nil) gspec := &Genesis{ Config: &config, Alloc: GenesisAlloc{addr1: {Balance: genesisBalance}}, diff --git a/eth/gasprice/gasprice_test.go b/eth/gasprice/gasprice_test.go index 32ab8f092e..074f6769ca 100644 --- a/eth/gasprice/gasprice_test.go +++ b/eth/gasprice/gasprice_test.go @@ -483,7 +483,7 @@ func TestSuggestGasPriceAfterFeeConfigUpdate(t *testing.T) { // create a chain config with fee manager enabled at genesis with [addr] as the admin chainConfig := *params.TestChainConfig - chainConfig.FeeManagerConfig = precompile.NewFeeManagerConfig(big.NewInt(0), []common.Address{addr}) + chainConfig.FeeManagerConfig = precompile.NewFeeManagerConfig(big.NewInt(0), []common.Address{addr}, nil, nil) // create a fee config with higher MinBaseFee and prepare it for inclusion in a tx signer := types.LatestSigner(params.TestChainConfig) diff --git a/params/precompile_config_test.go b/params/precompile_config_test.go index dd3e77a986..55b138b65a 100644 --- a/params/precompile_config_test.go +++ b/params/precompile_config_test.go @@ -19,7 +19,7 @@ func TestVerifyWithChainConfig(t *testing.T) { baseConfig := *SubnetEVMDefaultChainConfig config := &baseConfig config.PrecompileUpgrade = PrecompileUpgrade{ - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), []common.Address{}), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), nil, nil), } config.PrecompileUpgrades = []PrecompileUpgrade{ { @@ -28,7 +28,7 @@ func TestVerifyWithChainConfig(t *testing.T) { }, { // re-enable TxAllowList at timestamp 5 - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(5), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(5), admins, nil), }, } @@ -52,7 +52,7 @@ func TestVerifyWithChainConfig(t *testing.T) { badConfig.PrecompileUpgrades = append( badConfig.PrecompileUpgrades, PrecompileUpgrade{ - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(5), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(5), admins, nil), }, ) err = badConfig.Verify() @@ -70,7 +70,7 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { name: "enable and disable tx allow list", upgrades: []PrecompileUpgrade{ { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins, nil), }, { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(2)), @@ -82,16 +82,13 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { name: "invalid allow list config in tx allowlist", upgrades: []PrecompileUpgrade{ { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(1), admins, nil), }, { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(2)), }, { - TxAllowListConfig: &precompile.TxAllowListConfig{ - AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, - UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - }, + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(3), admins, admins), }, }, expectedError: "cannot set address", @@ -100,13 +97,10 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { name: "invalid initial fee manager config", upgrades: []PrecompileUpgrade{ { - FeeManagerConfig: &precompile.FeeConfigManagerConfig{ - AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins}, - UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - InitialFeeConfig: &commontype.FeeConfig{ + FeeManagerConfig: precompile.NewFeeManagerConfig(big.NewInt(3), admins, nil, + &commontype.FeeConfig{ GasLimit: big.NewInt(-1), - }, - }, + }), }, }, expectedError: "gasLimit = -1 cannot be less than or equal to 0", @@ -115,13 +109,10 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { name: "invalid initial fee manager config gas limit 0", upgrades: []PrecompileUpgrade{ { - FeeManagerConfig: &precompile.FeeConfigManagerConfig{ - AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins}, - UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - InitialFeeConfig: &commontype.FeeConfig{ + FeeManagerConfig: precompile.NewFeeManagerConfig(big.NewInt(3), admins, nil, + &commontype.FeeConfig{ GasLimit: big.NewInt(0), - }, - }, + }), }, }, expectedError: "gasLimit = 0 cannot be less than or equal to 0", @@ -154,23 +145,17 @@ func TestVerifyPrecompiles(t *testing.T) { { name: "invalid allow list config in tx allowlist", upgrade: PrecompileUpgrade{ - TxAllowListConfig: &precompile.TxAllowListConfig{ - AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, - UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - }, + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(3), admins, admins), }, expectedError: "cannot set address", }, { name: "invalid initial fee manager config", upgrade: PrecompileUpgrade{ - FeeManagerConfig: &precompile.FeeConfigManagerConfig{ - AllowListConfig: precompile.AllowListConfig{AllowListAdmins: admins}, - UpgradeableConfig: precompile.UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - InitialFeeConfig: &commontype.FeeConfig{ + FeeManagerConfig: precompile.NewFeeManagerConfig(big.NewInt(3), admins, nil, + &commontype.FeeConfig{ GasLimit: big.NewInt(-1), - }, - }, + }), }, expectedError: "gasLimit = -1 cannot be less than or equal to 0", }, @@ -198,10 +183,10 @@ func TestVerifyRequiresSortedTimestamps(t *testing.T) { config := &baseConfig config.PrecompileUpgrades = []PrecompileUpgrade{ { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), admins, nil), }, { - ContractDeployerAllowListConfig: precompile.NewContractDeployerAllowListConfig(big.NewInt(1), admins), + ContractDeployerAllowListConfig: precompile.NewContractDeployerAllowListConfig(big.NewInt(1), admins, nil), }, } @@ -215,7 +200,7 @@ func TestGetPrecompileConfig(t *testing.T) { baseConfig := *SubnetEVMDefaultChainConfig config := &baseConfig config.PrecompileUpgrade = PrecompileUpgrade{ - ContractDeployerAllowListConfig: precompile.NewContractDeployerAllowListConfig(big.NewInt(10), []common.Address{}), + ContractDeployerAllowListConfig: precompile.NewContractDeployerAllowListConfig(big.NewInt(10), nil, nil), } deployerConfig := config.GetContractDeployerAllowListConfig(big.NewInt(0)) diff --git a/params/upgrade_config_test.go b/params/upgrade_config_test.go index f5db3bd296..38c93f5386 100644 --- a/params/upgrade_config_test.go +++ b/params/upgrade_config_test.go @@ -15,7 +15,7 @@ import ( func TestVerifyUpgradeConfig(t *testing.T) { admins := []common.Address{{1}} chainConfig := *TestChainConfig - chainConfig.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(1), admins) + chainConfig.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(1), admins, nil) type test struct { upgrades []PrecompileUpgrade @@ -27,7 +27,7 @@ func TestVerifyUpgradeConfig(t *testing.T) { expectedErrorString: "disable should be [true]", upgrades: []PrecompileUpgrade{ { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(2), admins, nil), }, }, }, @@ -70,8 +70,8 @@ func TestVerifyUpgradeConfig(t *testing.T) { func TestCheckCompatibleUpgradeConfigs(t *testing.T) { admins := []common.Address{{1}} chainConfig := *TestChainConfig - chainConfig.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(1), admins) - chainConfig.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(10), admins) + chainConfig.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(1), admins, nil) + chainConfig.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(10), admins, nil) type test struct { configs []*UpgradeConfig @@ -89,7 +89,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -104,7 +104,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -114,7 +114,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(8), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(8), admins, nil), }, }, }, @@ -130,7 +130,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -140,7 +140,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(8), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(8), admins, nil), }, }, }, @@ -155,7 +155,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -178,7 +178,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -201,7 +201,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -212,7 +212,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { }, { // uses a different (empty) admin list, not allowed - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), []common.Address{}), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), []common.Address{}, nil), }, }, }, @@ -227,7 +227,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, @@ -237,7 +237,7 @@ func TestCheckCompatibleUpgradeConfigs(t *testing.T) { TxAllowListConfig: precompile.NewDisableTxAllowListConfig(big.NewInt(6)), }, { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(7), admins, nil), }, }, }, diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 428ec25f65..7229acc251 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -2109,7 +2109,7 @@ func TestBuildAllowListActivationBlock(t *testing.T) { if err := genesis.UnmarshalJSON([]byte(genesisJSONSubnetEVM)); err != nil { t.Fatal(err) } - genesis.Config.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(time.Now().Unix()), testEthAddrs) + genesis.Config.ContractDeployerAllowListConfig = precompile.NewContractDeployerAllowListConfig(big.NewInt(time.Now().Unix()), testEthAddrs, nil) genesisJSON, err := genesis.MarshalJSON() if err != nil { @@ -2173,7 +2173,7 @@ func TestTxAllowListSuccessfulTx(t *testing.T) { if err := genesis.UnmarshalJSON([]byte(genesisJSONSubnetEVM)); err != nil { t.Fatal(err) } - genesis.Config.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(0), testEthAddrs[0:1]) + genesis.Config.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(0), testEthAddrs[0:1], nil) genesisJSON, err := genesis.MarshalJSON() if err != nil { t.Fatal(err) @@ -2249,7 +2249,7 @@ func TestTxAllowListDisablePrecompile(t *testing.T) { t.Fatal(err) } enableAllowListTimestamp := time.Unix(0, 0) // enable at genesis - genesis.Config.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(enableAllowListTimestamp.Unix()), testEthAddrs[0:1]) + genesis.Config.TxAllowListConfig = precompile.NewTxAllowListConfig(big.NewInt(enableAllowListTimestamp.Unix()), testEthAddrs[0:1], nil) genesisJSON, err := genesis.MarshalJSON() if err != nil { t.Fatal(err) @@ -2356,7 +2356,7 @@ func TestFeeManagerChangeFee(t *testing.T) { if err := genesis.UnmarshalJSON([]byte(genesisJSONSubnetEVM)); err != nil { t.Fatal(err) } - genesis.Config.FeeManagerConfig = precompile.NewFeeManagerConfig(big.NewInt(0), testEthAddrs[0:1]) + genesis.Config.FeeManagerConfig = precompile.NewFeeManagerConfig(big.NewInt(0), testEthAddrs[0:1], nil, nil) // set a lower fee config now testLowFeeConfig := commontype.FeeConfig{ diff --git a/plugin/evm/vm_upgrade_bytes_test.go b/plugin/evm/vm_upgrade_bytes_test.go index 1b3d4673d7..a14bdeceb8 100644 --- a/plugin/evm/vm_upgrade_bytes_test.go +++ b/plugin/evm/vm_upgrade_bytes_test.go @@ -27,7 +27,7 @@ func TestVMUpgradeBytesPrecompile(t *testing.T) { upgradeConfig := ¶ms.UpgradeConfig{ PrecompileUpgrades: []params.PrecompileUpgrade{ { - TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(enableAllowListTimestamp.Unix()), testEthAddrs[0:1]), + TxAllowListConfig: precompile.NewTxAllowListConfig(big.NewInt(enableAllowListTimestamp.Unix()), testEthAddrs[0:1], nil), }, }, } diff --git a/precompile/config_test.go b/precompile/config_test.go index 2eea178f30..36af6fbf57 100644 --- a/precompile/config_test.go +++ b/precompile/config_test.go @@ -20,46 +20,31 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { expectedError string }{ { - name: "invalid allow list config in tx allowlist", - config: &TxAllowListConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, - UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - }, + name: "invalid allow list config in tx allowlist", + config: NewTxAllowListConfig(big.NewInt(3), admins, admins), expectedError: "cannot set address", }, { - name: "invalid allow list config in deployer allowlist", - config: &ContractDeployerAllowListConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, - UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - }, + name: "invalid allow list config in deployer allowlist", + config: NewTxAllowListConfig(big.NewInt(3), admins, admins), expectedError: "cannot set address", }, { - name: "invalid allow list config in native minter allowlist", - config: &ContractNativeMinterConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, - UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - }, + name: "invalid allow list config in native minter allowlist", + config: NewTxAllowListConfig(big.NewInt(3), admins, admins), expectedError: "cannot set address", }, { - name: "invalid allow list config in fee manager allowlist", - config: &FeeConfigManagerConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins, EnabledAddresses: admins}, - UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - }, + name: "invalid allow list config in fee manager allowlist", + config: NewTxAllowListConfig(big.NewInt(3), admins, admins), expectedError: "cannot set address", }, { name: "invalid initial fee manager config", - config: &FeeConfigManagerConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins}, - UpgradeableConfig: UpgradeableConfig{BlockTimestamp: big.NewInt(3)}, - InitialFeeConfig: &commontype.FeeConfig{ + config: NewFeeManagerConfig(big.NewInt(3), admins, nil, + &commontype.FeeConfig{ GasLimit: big.NewInt(0), - }, - }, + }), expectedError: "gasLimit = 0 cannot be less than or equal to 0", }, } diff --git a/precompile/contract_deployer_allow_list.go b/precompile/contract_deployer_allow_list.go index 97e1acd074..3b82efb01d 100644 --- a/precompile/contract_deployer_allow_list.go +++ b/precompile/contract_deployer_allow_list.go @@ -23,10 +23,13 @@ type ContractDeployerAllowListConfig struct { } // NewContractDeployerAllowListConfig returns a config for a network upgrade at [blockTimestamp] that enables -// ContractDeployerAllowList with the given [admins] as members of the allowlist. -func NewContractDeployerAllowListConfig(blockTimestamp *big.Int, admins []common.Address) *ContractDeployerAllowListConfig { +// ContractDeployerAllowList with the given [admins] and [enableds] as members of the allowlist. +func NewContractDeployerAllowListConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address) *ContractDeployerAllowListConfig { return &ContractDeployerAllowListConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins}, + AllowListConfig: AllowListConfig{ + AllowListAdmins: admins, + EnabledAddresses: enableds, + }, UpgradeableConfig: UpgradeableConfig{BlockTimestamp: blockTimestamp}, } } diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index 32d16544c7..d0c5c097e9 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -40,11 +40,15 @@ type ContractNativeMinterConfig struct { } // NewContractNativeMinterConfig returns a config for a network upgrade at [blockTimestamp] that enables -// ContractNativeMinter with the given [admins] as members of the allowlist. -func NewContractNativeMinterConfig(blockTimestamp *big.Int, admins []common.Address) *ContractNativeMinterConfig { +// ContractNativeMinter with the given [admins] and [enableds] as members of the allowlist with [initialMint] as initial mint operation. +func NewContractNativeMinterConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address, initialMint map[common.Address]*math.HexOrDecimal256) *ContractNativeMinterConfig { return &ContractNativeMinterConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins}, + AllowListConfig: AllowListConfig{ + AllowListAdmins: admins, + EnabledAddresses: enableds, + }, UpgradeableConfig: UpgradeableConfig{BlockTimestamp: blockTimestamp}, + InitialMint: initialMint, } } diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 066774c412..65ae35b262 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -60,11 +60,15 @@ type FeeConfigManagerConfig struct { } // NewFeeManagerConfig returns a config for a network upgrade at [blockTimestamp] that enables -// FeeConfigManager with the given [admins] as members of the allowlist. -func NewFeeManagerConfig(blockTimestamp *big.Int, admins []common.Address) *FeeConfigManagerConfig { +// FeeConfigManager with the given [admins] and [enableds] as members of the allowlist with [initialConfig] as initial fee config if specified. +func NewFeeManagerConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address, initialConfig *commontype.FeeConfig) *FeeConfigManagerConfig { return &FeeConfigManagerConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins}, + AllowListConfig: AllowListConfig{ + AllowListAdmins: admins, + EnabledAddresses: enableds, + }, UpgradeableConfig: UpgradeableConfig{BlockTimestamp: blockTimestamp}, + InitialFeeConfig: initialConfig, } } diff --git a/precompile/tx_allow_list.go b/precompile/tx_allow_list.go index 668d4eac41..f969ac2546 100644 --- a/precompile/tx_allow_list.go +++ b/precompile/tx_allow_list.go @@ -26,10 +26,13 @@ type TxAllowListConfig struct { } // NewTxAllowListConfig returns a config for a network upgrade at [blockTimestamp] that enables -// TxAllowList with the given [admins] as members of the allowlist. -func NewTxAllowListConfig(blockTimestamp *big.Int, admins []common.Address) *TxAllowListConfig { +// TxAllowList with the given [admins] and [enableds] as members of the allowlist. +func NewTxAllowListConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address) *TxAllowListConfig { return &TxAllowListConfig{ - AllowListConfig: AllowListConfig{AllowListAdmins: admins}, + AllowListConfig: AllowListConfig{ + AllowListAdmins: admins, + EnabledAddresses: enableds, + }, UpgradeableConfig: UpgradeableConfig{BlockTimestamp: blockTimestamp}, } } From 2777e0ec90e54eb6868a8488fccc4c793c36a985 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 1 Sep 2022 18:54:59 +0300 Subject: [PATCH 17/31] Update accounts/abi/bind/precompile_template.go Co-authored-by: Darioush Jalali --- accounts/abi/bind/precompile_template.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accounts/abi/bind/precompile_template.go b/accounts/abi/bind/precompile_template.go index 9a10f64bf7..71157f59ec 100644 --- a/accounts/abi/bind/precompile_template.go +++ b/accounts/abi/bind/precompile_template.go @@ -196,7 +196,7 @@ func (c *{{.Contract.Type}}Config) Contract() StatefulPrecompiledContract { func (c *{{.Contract.Type}}Config) Verify() error { {{if .Contract.AllowList}} // Verify AllowList first - if err := c.AllowListConfig.Verify(); err != nil{ + if err := c.AllowListConfig.Verify(); err != nil { return err } {{end}} From 83a9db64ea12500273da3d736d021488ca4215a3 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 1 Sep 2022 18:57:35 +0300 Subject: [PATCH 18/31] Update precompile/contract_native_minter.go Co-authored-by: Darioush Jalali --- precompile/contract_native_minter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index d0c5c097e9..1c5deffb5a 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -40,7 +40,7 @@ type ContractNativeMinterConfig struct { } // NewContractNativeMinterConfig returns a config for a network upgrade at [blockTimestamp] that enables -// ContractNativeMinter with the given [admins] and [enableds] as members of the allowlist with [initialMint] as initial mint operation. +// ContractNativeMinter with the given [admins] and [enableds] as members of the allowlist. Also mints balances according to [initialMint] when the upgrade activates. func NewContractNativeMinterConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address, initialMint map[common.Address]*math.HexOrDecimal256) *ContractNativeMinterConfig { return &ContractNativeMinterConfig{ AllowListConfig: AllowListConfig{ From 415a32bfe27d27ef231fed0c8abcfe2ee2bcfa14 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 1 Sep 2022 18:58:27 +0300 Subject: [PATCH 19/31] Update precompile/contract_deployer_allow_list.go Co-authored-by: Darioush Jalali --- precompile/contract_deployer_allow_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/contract_deployer_allow_list.go b/precompile/contract_deployer_allow_list.go index 3b82efb01d..aa01d9b8cd 100644 --- a/precompile/contract_deployer_allow_list.go +++ b/precompile/contract_deployer_allow_list.go @@ -23,7 +23,7 @@ type ContractDeployerAllowListConfig struct { } // NewContractDeployerAllowListConfig returns a config for a network upgrade at [blockTimestamp] that enables -// ContractDeployerAllowList with the given [admins] and [enableds] as members of the allowlist. +// ContractDeployerAllowList with [admins] and [enableds] as members of the allowlist. func NewContractDeployerAllowListConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address) *ContractDeployerAllowListConfig { return &ContractDeployerAllowListConfig{ AllowListConfig: AllowListConfig{ From 333aa8b97d5933f95c440c79a432523d0f466641 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 1 Sep 2022 18:58:35 +0300 Subject: [PATCH 20/31] Update precompile/allow_list.go Co-authored-by: Darioush Jalali --- precompile/allow_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index 418aabb0b4..dc8ba05a7e 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -74,7 +74,7 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { // Verify returns an error if there is an overlapping address between admin and enabled roles func (c *AllowListConfig) Verify() error { - // check if both lists are empty + // return early if either list is empty if len(c.EnabledAddresses) == 0 || len(c.AllowListAdmins) == 0 { return nil } From abfa39e6a9219f024423e57065432a349896b8e1 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 1 Sep 2022 19:52:44 +0300 Subject: [PATCH 21/31] add equals methods --- commontype/fee_config.go | 17 +++++++++++ core/stateful_precompile_test.go | 44 ++++++++++++++-------------- params/precompile_config.go | 2 +- precompile/allow_list.go | 14 +++++++-- precompile/contract_native_minter.go | 24 ++++++++++++++- precompile/fee_config_manager.go | 13 +++++++- 6 files changed, 86 insertions(+), 28 deletions(-) diff --git a/commontype/fee_config.go b/commontype/fee_config.go index 9843193ef9..f09eceeff9 100644 --- a/commontype/fee_config.go +++ b/commontype/fee_config.go @@ -7,6 +7,7 @@ import ( "fmt" "math/big" + "github.com/ava-labs/subnet-evm/utils" "github.com/ethereum/go-ethereum/common" ) @@ -81,6 +82,22 @@ func (f *FeeConfig) Verify() error { return f.checkByteLens() } +// Equal checks if given [other] is same with this FeeConfig. +func (f *FeeConfig) Equal(other *FeeConfig) bool { + if other == nil { + return false + } + + return utils.BigNumEqual(f.GasLimit, other.GasLimit) && + f.TargetBlockRate == other.TargetBlockRate && + utils.BigNumEqual(f.MinBaseFee, other.MinBaseFee) && + utils.BigNumEqual(f.TargetGas, other.TargetGas) && + utils.BigNumEqual(f.BaseFeeChangeDenominator, other.BaseFeeChangeDenominator) && + utils.BigNumEqual(f.MinBlockGasCost, other.MinBlockGasCost) && + utils.BigNumEqual(f.MaxBlockGasCost, other.MaxBlockGasCost) && + utils.BigNumEqual(f.BlockGasCostStep, other.BlockGasCostStep) +} + // checkByteLens checks byte lengths against common.HashLen (32 bytes) and returns error func (f *FeeConfig) checkByteLens() error { if isBiggerThanHashLen(f.GasLimit) { diff --git a/core/stateful_precompile_test.go b/core/stateful_precompile_test.go index bec0c8fece..782cb38a79 100644 --- a/core/stateful_precompile_test.go +++ b/core/stateful_precompile_test.go @@ -520,7 +520,7 @@ func TestContractNativeMinterRun(t *testing.T) { } adminAddr := common.HexToAddress("0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC") - allowAddr := common.HexToAddress("0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B") + enabledAddr := common.HexToAddress("0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B") noRoleAddr := common.HexToAddress("0xF60C45c607D0f41687c94C314d300f483661E13a") testAddr := common.BigToAddress(common.Big2) @@ -539,11 +539,11 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedErr: precompile.ErrCannotMint.Error(), }, - "mint funds from allow address": { - caller: allowAddr, + "mint funds from enabled address": { + caller: enabledAddr, precompileAddr: precompile.ContractNativeMinterAddress, input: func() []byte { - input, err := precompile.PackMintInput(allowAddr, common.Big1) + input, err := precompile.PackMintInput(enabledAddr, common.Big1) if err != nil { panic(err) } @@ -553,7 +553,7 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: []byte{}, assertState: func(t *testing.T, state *state.StateDB) { - assert.Equal(t, common.Big1, state.GetBalance(allowAddr), "expected minted funds") + assert.Equal(t, common.Big1, state.GetBalance(enabledAddr), "expected minted funds") }, }, "enabled role by config": { @@ -573,11 +573,11 @@ func TestContractNativeMinterRun(t *testing.T) { }, }, "initial mint funds": { - caller: allowAddr, + caller: enabledAddr, precompileAddr: precompile.ContractNativeMinterAddress, config: &precompile.ContractNativeMinterConfig{ InitialMint: map[common.Address]*math.HexOrDecimal256{ - allowAddr: math.NewHexOrDecimal256(2), + enabledAddr: math.NewHexOrDecimal256(2), }, }, input: func() []byte { @@ -587,7 +587,7 @@ func TestContractNativeMinterRun(t *testing.T) { readOnly: false, expectedRes: common.Hash(precompile.AllowListNoRole).Bytes(), assertState: func(t *testing.T, state *state.StateDB) { - assert.Equal(t, common.Big2, state.GetBalance(allowAddr), "expected minted funds") + assert.Equal(t, common.Big2, state.GetBalance(enabledAddr), "expected minted funds") }, }, "mint funds from admin address": { @@ -639,10 +639,10 @@ func TestContractNativeMinterRun(t *testing.T) { expectedErr: vmerrs.ErrWriteProtection.Error(), }, "readOnly mint with allow role fails": { - caller: allowAddr, + caller: enabledAddr, precompileAddr: precompile.ContractNativeMinterAddress, input: func() []byte { - input, err := precompile.PackMintInput(allowAddr, common.Big1) + input, err := precompile.PackMintInput(enabledAddr, common.Big1) if err != nil { panic(err) } @@ -670,7 +670,7 @@ func TestContractNativeMinterRun(t *testing.T) { caller: adminAddr, precompileAddr: precompile.ContractNativeMinterAddress, input: func() []byte { - input, err := precompile.PackMintInput(allowAddr, common.Big1) + input, err := precompile.PackMintInput(enabledAddr, common.Big1) if err != nil { panic(err) } @@ -731,7 +731,7 @@ func TestContractNativeMinterRun(t *testing.T) { }, }, "set allow role from non-admin fails": { - caller: allowAddr, + caller: enabledAddr, precompileAddr: precompile.ContractNativeMinterAddress, input: func() []byte { input, err := precompile.PackModifyAllowList(noRoleAddr, precompile.AllowListEnabled) @@ -753,10 +753,10 @@ func TestContractNativeMinterRun(t *testing.T) { } // Set up the state so that each address has the expected permissions at the start. precompile.SetContractNativeMinterStatus(state, adminAddr, precompile.AllowListAdmin) - precompile.SetContractNativeMinterStatus(state, allowAddr, precompile.AllowListEnabled) + precompile.SetContractNativeMinterStatus(state, enabledAddr, precompile.AllowListEnabled) precompile.SetContractNativeMinterStatus(state, noRoleAddr, precompile.AllowListNoRole) assert.Equal(t, precompile.AllowListAdmin, precompile.GetContractNativeMinterStatus(state, adminAddr)) - assert.Equal(t, precompile.AllowListEnabled, precompile.GetContractNativeMinterStatus(state, allowAddr)) + assert.Equal(t, precompile.AllowListEnabled, precompile.GetContractNativeMinterStatus(state, enabledAddr)) assert.Equal(t, precompile.AllowListNoRole, precompile.GetContractNativeMinterStatus(state, noRoleAddr)) blockContext := &mockBlockContext{blockNumber: common.Big0} @@ -804,7 +804,7 @@ func TestFeeConfigManagerRun(t *testing.T) { } adminAddr := common.HexToAddress("0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC") - allowAddr := common.HexToAddress("0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B") + enabledAddr := common.HexToAddress("0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B") noRoleAddr := common.HexToAddress("0xF60C45c607D0f41687c94C314d300f483661E13a") for name, test := range map[string]test{ @@ -822,8 +822,8 @@ func TestFeeConfigManagerRun(t *testing.T) { readOnly: false, expectedErr: precompile.ErrCannotChangeFee.Error(), }, - "set config from allow address": { - caller: allowAddr, + "set config from enabled address": { + caller: enabledAddr, precompileAddr: precompile.FeeConfigManagerAddress, input: func() []byte { input, err := precompile.PackSetFeeConfig(testFeeConfig) @@ -840,8 +840,8 @@ func TestFeeConfigManagerRun(t *testing.T) { assert.Equal(t, testFeeConfig, feeConfig) }, }, - "set invalid config from allow address": { - caller: allowAddr, + "set invalid config from enabled address": { + caller: enabledAddr, precompileAddr: precompile.FeeConfigManagerAddress, input: func() []byte { feeConfig := testFeeConfig @@ -967,7 +967,7 @@ func TestFeeConfigManagerRun(t *testing.T) { expectedErr: vmerrs.ErrWriteProtection.Error(), }, "readOnly setFeeConfig with allow role fails": { - caller: allowAddr, + caller: enabledAddr, precompileAddr: precompile.FeeConfigManagerAddress, input: func() []byte { input, err := precompile.PackSetFeeConfig(testFeeConfig) @@ -1027,7 +1027,7 @@ func TestFeeConfigManagerRun(t *testing.T) { }, }, "set allow role from non-admin fails": { - caller: allowAddr, + caller: enabledAddr, precompileAddr: precompile.FeeConfigManagerAddress, input: func() []byte { input, err := precompile.PackModifyAllowList(noRoleAddr, precompile.AllowListEnabled) @@ -1049,7 +1049,7 @@ func TestFeeConfigManagerRun(t *testing.T) { } // Set up the state so that each address has the expected permissions at the start. precompile.SetFeeConfigManagerStatus(state, adminAddr, precompile.AllowListAdmin) - precompile.SetFeeConfigManagerStatus(state, allowAddr, precompile.AllowListEnabled) + precompile.SetFeeConfigManagerStatus(state, enabledAddr, precompile.AllowListEnabled) precompile.SetFeeConfigManagerStatus(state, noRoleAddr, precompile.AllowListNoRole) if test.preCondition != nil { diff --git a/params/precompile_config.go b/params/precompile_config.go index 0c609e6b94..933b88a693 100644 --- a/params/precompile_config.go +++ b/params/precompile_config.go @@ -60,7 +60,7 @@ func (p *PrecompileUpgrade) getByKey(key precompileKey) (precompile.StatefulPrec } } -// VerifyPrecompileUpgrades checks [c.PrecompileUpgrades] is well formed: +// verifyPrecompileUpgrades checks [c.PrecompileUpgrades] is well formed: // - [upgrades] must specify exactly one key per PrecompileUpgrade // - the specified blockTimestamps must monotonically increase // - the specified blockTimestamps must be compatible with those diff --git a/precompile/allow_list.go b/precompile/allow_list.go index 418aabb0b4..8823e984f0 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -61,11 +61,19 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { if other == nil { return false } - if len(c.AllowListAdmins) != len(other.AllowListAdmins) { + if !checkList(c.AllowListAdmins, other.AllowListAdmins) { return false } - for i, admin := range c.AllowListAdmins { - if admin != other.AllowListAdmins[i] { + + return checkList(c.EnabledAddresses, other.EnabledAddresses) +} + +func checkList(current []common.Address, other []common.Address) bool { + if len(current) != len(other) { + return false + } + for i, address := range current { + if address != other[i] { return false } } diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index d0c5c097e9..ccc87eecbc 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -8,6 +8,7 @@ import ( "fmt" "math/big" + "github.com/ava-labs/subnet-evm/utils" "github.com/ava-labs/subnet-evm/vmerrs" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" @@ -92,7 +93,28 @@ func (c *ContractNativeMinterConfig) Equal(s StatefulPrecompileConfig) bool { if !ok { return false } - return c.UpgradeableConfig.Equal(&other.UpgradeableConfig) && c.AllowListConfig.Equal(&other.AllowListConfig) + eq := c.UpgradeableConfig.Equal(&other.UpgradeableConfig) && c.AllowListConfig.Equal(&other.AllowListConfig) + if !eq { + return false + } + + if len(c.InitialMint) != len(other.InitialMint) { + return false + } + + for address, amount := range c.InitialMint { + val, ok := other.InitialMint[address] + if !ok { + return false + } + bigIntAmount := (*big.Int)(amount) + bigIntVal := (*big.Int)(val) + if !utils.BigNumEqual(bigIntAmount, bigIntVal) { + return false + } + } + + return true } // GetContractNativeMinterStatus returns the role of [address] for the minter list. diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 65ae35b262..0df86b0977 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -95,7 +95,16 @@ func (c *FeeConfigManagerConfig) Equal(s StatefulPrecompileConfig) bool { if !ok { return false } - return c.UpgradeableConfig.Equal(&other.UpgradeableConfig) && c.AllowListConfig.Equal(&other.AllowListConfig) + eq := c.UpgradeableConfig.Equal(&other.UpgradeableConfig) && c.AllowListConfig.Equal(&other.AllowListConfig) + if !eq { + return false + } + + if c.InitialFeeConfig == nil { + return other.InitialFeeConfig == nil + } + + return c.InitialFeeConfig.Equal(other.InitialFeeConfig) } // Configure configures [state] with the desired admins based on [c]. @@ -103,10 +112,12 @@ func (c *FeeConfigManagerConfig) Configure(chainConfig ChainConfig, state StateD // Store the initial fee config into the state when the fee config manager activates. if c.InitialFeeConfig != nil { if err := StoreFeeConfig(state, *c.InitialFeeConfig, blockContext); err != nil { + // This should not happen since we already checked this config with Verify() panic(fmt.Sprintf("invalid feeConfig provided: %s", err)) } } else { if err := StoreFeeConfig(state, chainConfig.GetFeeConfig(), blockContext); err != nil { + // This should not happen since we already checked the chain config in the genesis creation. panic(fmt.Sprintf("fee config should have been verified in genesis: %s", err)) } } From 1013b59c544a4066a20a0217dc86dd8e164383e2 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 00:51:46 +0300 Subject: [PATCH 22/31] add new test cases for precompile configs --- precompile/config_test.go | 41 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/precompile/config_test.go b/precompile/config_test.go index 36af6fbf57..3d66c4f525 100644 --- a/precompile/config_test.go +++ b/precompile/config_test.go @@ -9,11 +9,13 @@ import ( "github.com/ava-labs/subnet-evm/commontype" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/math" "github.com/stretchr/testify/require" ) func TestVerifyPrecompileUpgrades(t *testing.T) { admins := []common.Address{{1}} + enableds := []common.Address{{2}} tests := []struct { name string config StatefulPrecompileConfig @@ -24,19 +26,34 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { config: NewTxAllowListConfig(big.NewInt(3), admins, admins), expectedError: "cannot set address", }, + { + name: "nil member allow list config in tx allowlist", + config: NewTxAllowListConfig(big.NewInt(3), nil, nil), + expectedError: "", + }, + { + name: "empty member allow list config in tx allowlist", + config: NewTxAllowListConfig(big.NewInt(3), []common.Address{}, []common.Address{}), + expectedError: "", + }, + { + name: "valid allow list config in tx allowlist", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + expectedError: "", + }, { name: "invalid allow list config in deployer allowlist", - config: NewTxAllowListConfig(big.NewInt(3), admins, admins), + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, admins), expectedError: "cannot set address", }, { name: "invalid allow list config in native minter allowlist", - config: NewTxAllowListConfig(big.NewInt(3), admins, admins), + config: NewContractNativeMinterConfig(big.NewInt(3), admins, admins, nil), expectedError: "cannot set address", }, { name: "invalid allow list config in fee manager allowlist", - config: NewTxAllowListConfig(big.NewInt(3), admins, admins), + config: NewFeeManagerConfig(big.NewInt(3), admins, admins, nil), expectedError: "cannot set address", }, { @@ -47,6 +64,24 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { }), expectedError: "gasLimit = 0 cannot be less than or equal to 0", }, + { + name: "nil amount in native minter config", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(123), + common.HexToAddress("0x02"): nil, + }), + expectedError: ErrAmountNil.Error(), + }, + { + name: "negative amount in native minter config", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(123), + common.HexToAddress("0x02"): math.NewHexOrDecimal256(-1), + }), + expectedError: ErrAmountNonPositive.Error(), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From a9d8b4985bc915c2152404645f63dcc58f42499e Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 00:52:19 +0300 Subject: [PATCH 23/31] use hex to address in tests --- core/stateful_precompile_test.go | 2 +- plugin/evm/vm_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/stateful_precompile_test.go b/core/stateful_precompile_test.go index 782cb38a79..23d5b0b0ed 100644 --- a/core/stateful_precompile_test.go +++ b/core/stateful_precompile_test.go @@ -522,7 +522,7 @@ func TestContractNativeMinterRun(t *testing.T) { adminAddr := common.HexToAddress("0x8db97C7cEcE249c2b98bDC0226Cc4C2A57BF52FC") enabledAddr := common.HexToAddress("0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B") noRoleAddr := common.HexToAddress("0xF60C45c607D0f41687c94C314d300f483661E13a") - testAddr := common.BigToAddress(common.Big2) + testAddr := common.HexToAddress("0x123456789") for name, test := range map[string]test{ "mint funds from no role fails": { diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 7229acc251..5fa6bc4d30 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -2484,7 +2484,7 @@ func TestAllowFeeRecipientDisabled(t *testing.T) { } issuer, vm, _, _ := GenesisVM(t, true, string(genesisJSON), "", "") - vm.miner.SetEtherbase(common.BigToAddress(common.Big1)) // set non-blackhole address by force + vm.miner.SetEtherbase(common.HexToAddress("0x0123456789")) // set non-blackhole address by force defer func() { if err := vm.Shutdown(); err != nil { t.Fatal(err) @@ -2533,7 +2533,7 @@ func TestAllowFeeRecipientEnabled(t *testing.T) { t.Fatal(err) } - etherBase := common.BigToAddress(common.Big1) + etherBase := common.HexToAddress("0x0123456789") c := Config{} c.SetDefaults() c.FeeRecipient = etherBase.String() From dad95da92ea2da3bd6e8cd498990d7ad20ca2c9f Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 00:52:32 +0300 Subject: [PATCH 24/31] rename helper function --- precompile/allow_list.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index 8d8026014a..e6ade243dd 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -61,14 +61,15 @@ func (c *AllowListConfig) Equal(other *AllowListConfig) bool { if other == nil { return false } - if !checkList(c.AllowListAdmins, other.AllowListAdmins) { + if !areEqualAddressLists(c.AllowListAdmins, other.AllowListAdmins) { return false } - return checkList(c.EnabledAddresses, other.EnabledAddresses) + return areEqualAddressLists(c.EnabledAddresses, other.EnabledAddresses) } -func checkList(current []common.Address, other []common.Address) bool { +// areEqualAddressLists returns true iff [a] and [b] have the same addresses in the same order. +func areEqualAddressLists(current []common.Address, other []common.Address) bool { if len(current) != len(other) { return false } From d754ae6ce927fbfa2ec49f5dc60dde9a80228d75 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 00:52:45 +0300 Subject: [PATCH 25/31] add verify to contract native minter --- precompile/contract_native_minter.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index 5e3b8d8d56..9843e0d6b8 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -28,8 +28,10 @@ var ( // Singleton StatefulPrecompiledContract for minting native assets by permissioned callers. ContractNativeMinterPrecompile StatefulPrecompiledContract = createNativeMinterPrecompile(ContractNativeMinterAddress) - mintSignature = CalculateFunctionSelector("mintNativeCoin(address,uint256)") // address, amount - ErrCannotMint = errors.New("non-enabled cannot mint") + mintSignature = CalculateFunctionSelector("mintNativeCoin(address,uint256)") // address, amount + ErrCannotMint = errors.New("non-enabled cannot mint") + ErrAmountNil = errors.New("initial mint amount cannot be nil") + ErrAmountNonPositive = errors.New("initial mint amount must be positive") ) // ContractNativeMinterConfig wraps [AllowListConfig] and uses it to implement the StatefulPrecompileConfig @@ -86,6 +88,23 @@ func (c *ContractNativeMinterConfig) Contract() StatefulPrecompiledContract { return ContractNativeMinterPrecompile } +func (c *ContractNativeMinterConfig) Verify() error { + if err := c.AllowListConfig.Verify(); err != nil { + return err + } + // ensure that all of the initial mint values in the map are non-nil positive values + for _, amount := range c.InitialMint { + if amount == nil { + return ErrAmountNil + } + bigIntAmount := (*big.Int)(amount) + if bigIntAmount.Sign() < 1 { + return ErrAmountNonPositive + } + } + return nil +} + // Equal returns true if [s] is a [*ContractNativeMinterConfig] and it has been configured identical to [c]. func (c *ContractNativeMinterConfig) Equal(s StatefulPrecompileConfig) bool { // typecast before comparison From 78e65f2863875fd5689ae47b513c432af2f52b0a Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 13:26:57 +0300 Subject: [PATCH 26/31] add fee config tests --- commontype/fee_config_test.go | 124 ++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 commontype/fee_config_test.go diff --git a/commontype/fee_config_test.go b/commontype/fee_config_test.go new file mode 100644 index 0000000000..892a85cbb4 --- /dev/null +++ b/commontype/fee_config_test.go @@ -0,0 +1,124 @@ +// (c) 2019-2022, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package commontype + +import ( + "math/big" + "testing" + + "github.com/stretchr/testify/require" +) + +var validFeeConfig = FeeConfig{ + GasLimit: big.NewInt(8_000_000), + TargetBlockRate: 2, // in seconds + + MinBaseFee: big.NewInt(25_000_000_000), + TargetGas: big.NewInt(15_000_000), + BaseFeeChangeDenominator: big.NewInt(36), + + MinBlockGasCost: big.NewInt(0), + MaxBlockGasCost: big.NewInt(1_000_000), + BlockGasCostStep: big.NewInt(200_000), +} + +func TestVerify(t *testing.T) { + tests := []struct { + name string + config *FeeConfig + expectedError string + }{ + { + name: "invalid GasLimit in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.GasLimit = big.NewInt(0); return &c }(), + expectedError: "gasLimit = 0 cannot be less than or equal to 0", + }, + { + name: "invalid TargetBlockRate in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.TargetBlockRate = 0; return &c }(), + expectedError: "targetBlockRate = 0 cannot be less than or equal to 0", + }, + { + name: "invalid MinBaseFee in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.MinBaseFee = big.NewInt(-1); return &c }(), + expectedError: "minBaseFee = -1 cannot be less than 0", + }, + { + name: "invalid TargetGas in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.TargetGas = big.NewInt(0); return &c }(), + expectedError: "targetGas = 0 cannot be less than or equal to 0", + }, + { + name: "invalid BaseFeeChangeDenominator in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.BaseFeeChangeDenominator = big.NewInt(0); return &c }(), + expectedError: "baseFeeChangeDenominator = 0 cannot be less than or equal to 0", + }, + { + name: "invalid MinBlockGasCost in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.MinBlockGasCost = big.NewInt(-1); return &c }(), + expectedError: "minBlockGasCost = -1 cannot be less than 0", + }, + { + name: "MinBlockGasCost bigger than MaxBlockGasCost in FeeConfig", + config: func() *FeeConfig { + c := validFeeConfig + c.MinBlockGasCost = big.NewInt(2) + c.MaxBlockGasCost = big.NewInt(1) + return &c + }(), + expectedError: "minBlockGasCost = 2 cannot be greater than maxBlockGasCost = 1", + }, + { + name: "invalid BlockGasCostStep in FeeConfig", + config: func() *FeeConfig { c := validFeeConfig; c.BlockGasCostStep = big.NewInt(-1); return &c }(), + expectedError: "blockGasCostStep = -1 cannot be less than 0", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := test.config.Verify() + if test.expectedError == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), test.expectedError) + } + }) + } +} + +func TestEqual(t *testing.T) { + tests := []struct { + name string + a *FeeConfig + b *FeeConfig + expected bool + }{ + { + name: "equal", + a: &validFeeConfig, + b: &validFeeConfig, + expected: true, + }, + { + name: "not equal", + a: &validFeeConfig, + b: func() *FeeConfig { c := validFeeConfig; c.GasLimit = big.NewInt(1); return &c }(), + expected: false, + }, + { + name: "not equal nil", + a: &validFeeConfig, + b: nil, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.expected, test.a.Equal(test.b)) + }) + } +} From 66a686e9f6f8854f908ddcac3ace8ae418ad1b06 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 13:30:51 +0300 Subject: [PATCH 27/31] add positive cases --- commontype/fee_config_test.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/commontype/fee_config_test.go b/commontype/fee_config_test.go index 892a85cbb4..306cffebda 100644 --- a/commontype/fee_config_test.go +++ b/commontype/fee_config_test.go @@ -59,6 +59,11 @@ func TestVerify(t *testing.T) { config: func() *FeeConfig { c := validFeeConfig; c.MinBlockGasCost = big.NewInt(-1); return &c }(), expectedError: "minBlockGasCost = -1 cannot be less than 0", }, + { + name: "valid FeeConfig", + config: &validFeeConfig, + expectedError: "", + }, { name: "MinBlockGasCost bigger than MaxBlockGasCost in FeeConfig", config: func() *FeeConfig { @@ -97,9 +102,20 @@ func TestEqual(t *testing.T) { expected bool }{ { - name: "equal", - a: &validFeeConfig, - b: &validFeeConfig, + name: "equal", + a: &validFeeConfig, + b: &FeeConfig{ + GasLimit: big.NewInt(8_000_000), + TargetBlockRate: 2, // in seconds + + MinBaseFee: big.NewInt(25_000_000_000), + TargetGas: big.NewInt(15_000_000), + BaseFeeChangeDenominator: big.NewInt(36), + + MinBlockGasCost: big.NewInt(0), + MaxBlockGasCost: big.NewInt(1_000_000), + BlockGasCostStep: big.NewInt(200_000), + }, expected: true, }, { From 3964fe59695f4329ff5741c32956bc596e389e1a Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 16 Sep 2022 14:09:16 +0300 Subject: [PATCH 28/31] add equal tesets for precompile configs --- precompile/config_test.go | 269 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 269 insertions(+) diff --git a/precompile/config_test.go b/precompile/config_test.go index 3d66c4f525..b774b98e57 100644 --- a/precompile/config_test.go +++ b/precompile/config_test.go @@ -13,6 +13,19 @@ import ( "github.com/stretchr/testify/require" ) +var validFeeConfig = commontype.FeeConfig{ + GasLimit: big.NewInt(8_000_000), + TargetBlockRate: 2, // in seconds + + MinBaseFee: big.NewInt(25_000_000_000), + TargetGas: big.NewInt(15_000_000), + BaseFeeChangeDenominator: big.NewInt(36), + + MinBlockGasCost: big.NewInt(0), + MaxBlockGasCost: big.NewInt(1_000_000), + BlockGasCostStep: big.NewInt(200_000), +} + func TestVerifyPrecompileUpgrades(t *testing.T) { admins := []common.Address{{1}} enableds := []common.Address{{2}} @@ -96,3 +109,259 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { }) } } + +func TestEqualTxAllowListConfig(t *testing.T) { + admins := []common.Address{{1}} + enableds := []common.Address{{2}} + tests := []struct { + name string + config StatefulPrecompileConfig + other StatefulPrecompileConfig + expected bool + }{ + { + name: "non-nil config and nil other", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + other: nil, + expected: false, + }, + { + name: "different type", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + other: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + expected: false, + }, + { + name: "different admin", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + other: NewTxAllowListConfig(big.NewInt(3), []common.Address{{3}}, enableds), + expected: false, + }, + { + name: "different enabled", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + other: NewTxAllowListConfig(big.NewInt(3), admins, []common.Address{{3}}), + expected: false, + }, + { + name: "different version", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + other: NewTxAllowListConfig(big.NewInt(4), admins, enableds), + expected: false, + }, + { + name: "same config", + config: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + other: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + require.Equal(tt.expected, tt.config.Equal(tt.other)) + }) + } +} + +func TestEqualContractDeployerAllowListConfig(t *testing.T) { + admins := []common.Address{{1}} + enableds := []common.Address{{2}} + tests := []struct { + name string + config StatefulPrecompileConfig + other StatefulPrecompileConfig + expected bool + }{ + { + name: "non-nil config and nil other", + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + other: nil, + expected: false, + }, + { + name: "different type", + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + other: NewTxAllowListConfig(big.NewInt(3), admins, enableds), + expected: false, + }, + { + name: "different admin", + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + other: NewContractDeployerAllowListConfig(big.NewInt(3), []common.Address{{3}}, enableds), + expected: false, + }, + { + name: "different enabled", + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + other: NewContractDeployerAllowListConfig(big.NewInt(3), admins, []common.Address{{3}}), + expected: false, + }, + { + name: "different version", + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + other: NewContractDeployerAllowListConfig(big.NewInt(4), admins, enableds), + expected: false, + }, + { + name: "same config", + config: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + other: NewContractDeployerAllowListConfig(big.NewInt(3), admins, enableds), + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + require.Equal(tt.expected, tt.config.Equal(tt.other)) + }) + } +} + +func TestEqualContractNativeMinterConfig(t *testing.T) { + admins := []common.Address{{1}} + enableds := []common.Address{{2}} + tests := []struct { + name string + config StatefulPrecompileConfig + other StatefulPrecompileConfig + expected bool + }{ + { + name: "non-nil config and nil other", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, enableds, nil), + other: nil, + expected: false, + }, + { + name: "different type", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, enableds, nil), + other: NewTxAllowListConfig(big.NewInt(3), []common.Address{{1}}, []common.Address{{2}}), + expected: false, + }, + { + name: "different version", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, nil), + other: NewContractNativeMinterConfig(big.NewInt(4), admins, nil, nil), + expected: false, + }, + { + name: "different enabled", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, nil), + other: NewContractNativeMinterConfig(big.NewInt(3), admins, enableds, nil), + expected: false, + }, + { + name: "different initial mint amounts", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(1), + }), + other: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(2), + }), + expected: false, + }, + { + name: "different initial mint addresses", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(1), + }), + other: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x02"): math.NewHexOrDecimal256(1), + }), + expected: false, + }, + + { + name: "same config", + config: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(1), + }), + other: NewContractNativeMinterConfig(big.NewInt(3), admins, nil, + map[common.Address]*math.HexOrDecimal256{ + common.HexToAddress("0x01"): math.NewHexOrDecimal256(1), + }), + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + require.Equal(tt.expected, tt.config.Equal(tt.other)) + }) + } +} + +func TestEqualFeeConfigManagerConfig(t *testing.T) { + admins := []common.Address{{1}} + enableds := []common.Address{{2}} + tests := []struct { + name string + config StatefulPrecompileConfig + other StatefulPrecompileConfig + expected bool + }{ + { + name: "non-nil config and nil other", + config: NewFeeManagerConfig(big.NewInt(3), admins, enableds, nil), + other: nil, + expected: false, + }, + { + name: "different type", + config: NewFeeManagerConfig(big.NewInt(3), admins, enableds, nil), + other: NewTxAllowListConfig(big.NewInt(3), []common.Address{{1}}, []common.Address{{2}}), + expected: false, + }, + { + name: "different version", + config: NewFeeManagerConfig(big.NewInt(3), admins, nil, nil), + other: NewFeeManagerConfig(big.NewInt(4), admins, nil, nil), + expected: false, + }, + { + name: "different enabled", + config: NewFeeManagerConfig(big.NewInt(3), admins, nil, nil), + other: NewFeeManagerConfig(big.NewInt(3), admins, enableds, nil), + expected: false, + }, + { + name: "non-nil initial config and nil initial config", + config: NewFeeManagerConfig(big.NewInt(3), admins, nil, &validFeeConfig), + other: NewFeeManagerConfig(big.NewInt(3), admins, nil, nil), + expected: false, + }, + { + name: "different initial config", + config: NewFeeManagerConfig(big.NewInt(3), admins, nil, &validFeeConfig), + other: NewFeeManagerConfig(big.NewInt(3), admins, nil, + func() *commontype.FeeConfig { + c := validFeeConfig + c.GasLimit = big.NewInt(123) + return &c + }()), + expected: false, + }, + { + name: "same config", + config: NewFeeManagerConfig(big.NewInt(3), admins, nil, &validFeeConfig), + other: NewFeeManagerConfig(big.NewInt(3), admins, nil, &validFeeConfig), + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + require.Equal(tt.expected, tt.config.Equal(tt.other)) + }) + } +} From 42139cdbca2d1750cf56dfd56c153eea4705ef94 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 20 Sep 2022 13:35:14 +0300 Subject: [PATCH 29/31] Update commontype/fee_config_test.go Co-authored-by: Darioush Jalali --- commontype/fee_config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commontype/fee_config_test.go b/commontype/fee_config_test.go index 306cffebda..57c8a58b06 100644 --- a/commontype/fee_config_test.go +++ b/commontype/fee_config_test.go @@ -60,7 +60,7 @@ func TestVerify(t *testing.T) { expectedError: "minBlockGasCost = -1 cannot be less than 0", }, { - name: "valid FeeConfig", + name: "valid FeeConfig", config: &validFeeConfig, expectedError: "", }, From 9affef3a782375e56c3ff04532d67f768a66c43a Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 20 Sep 2022 13:37:21 +0300 Subject: [PATCH 30/31] remove comment --- precompile/allow_list.go | 1 - 1 file changed, 1 deletion(-) diff --git a/precompile/allow_list.go b/precompile/allow_list.go index e6ade243dd..78e3d4b6c2 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -47,7 +47,6 @@ type AllowListConfig struct { // Configure initializes the address space of [precompileAddr] by initializing the role of each of // the addresses in [AllowListAdmins]. func (c *AllowListConfig) Configure(state StateDB, precompileAddr common.Address) { - // First set enabled roles so these can be upgraded to admin addresses below. for _, enabledAddr := range c.EnabledAddresses { setAllowListRole(state, precompileAddr, enabledAddr, AllowListEnabled) } From 56a6d05e7ae88e3baf3b24e1c8f3355a30e98402 Mon Sep 17 00:00:00 2001 From: aaronbuchwald Date: Thu, 22 Sep 2022 17:28:34 +0900 Subject: [PATCH 31/31] precompile: add formatted errors to precompile verify (#262) --- precompile/config_test.go | 4 ++-- precompile/contract_native_minter.go | 12 +++++------- precompile/fee_config_manager.go | 7 ++++--- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/precompile/config_test.go b/precompile/config_test.go index b774b98e57..56ebf979fb 100644 --- a/precompile/config_test.go +++ b/precompile/config_test.go @@ -84,7 +84,7 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { common.HexToAddress("0x01"): math.NewHexOrDecimal256(123), common.HexToAddress("0x02"): nil, }), - expectedError: ErrAmountNil.Error(), + expectedError: "initial mint cannot contain nil", }, { name: "negative amount in native minter config", @@ -93,7 +93,7 @@ func TestVerifyPrecompileUpgrades(t *testing.T) { common.HexToAddress("0x01"): math.NewHexOrDecimal256(123), common.HexToAddress("0x02"): math.NewHexOrDecimal256(-1), }), - expectedError: ErrAmountNonPositive.Error(), + expectedError: "initial mint cannot contain invalid amount", }, } for _, tt := range tests { diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index 9843e0d6b8..45301362c2 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -28,10 +28,8 @@ var ( // Singleton StatefulPrecompiledContract for minting native assets by permissioned callers. ContractNativeMinterPrecompile StatefulPrecompiledContract = createNativeMinterPrecompile(ContractNativeMinterAddress) - mintSignature = CalculateFunctionSelector("mintNativeCoin(address,uint256)") // address, amount - ErrCannotMint = errors.New("non-enabled cannot mint") - ErrAmountNil = errors.New("initial mint amount cannot be nil") - ErrAmountNonPositive = errors.New("initial mint amount must be positive") + mintSignature = CalculateFunctionSelector("mintNativeCoin(address,uint256)") // address, amount + ErrCannotMint = errors.New("non-enabled cannot mint") ) // ContractNativeMinterConfig wraps [AllowListConfig] and uses it to implement the StatefulPrecompileConfig @@ -93,13 +91,13 @@ func (c *ContractNativeMinterConfig) Verify() error { return err } // ensure that all of the initial mint values in the map are non-nil positive values - for _, amount := range c.InitialMint { + for addr, amount := range c.InitialMint { if amount == nil { - return ErrAmountNil + return fmt.Errorf("initial mint cannot contain nil amount for address %s", addr) } bigIntAmount := (*big.Int)(amount) if bigIntAmount.Sign() < 1 { - return ErrAmountNonPositive + return fmt.Errorf("initial mint cannot contain invalid amount %v for address %s", bigIntAmount, addr) } } return nil diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 0df86b0977..b7bd0d44b2 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -133,10 +133,11 @@ func (c *FeeConfigManagerConfig) Verify() error { if err := c.AllowListConfig.Verify(); err != nil { return err } - if c.InitialFeeConfig != nil { - return c.InitialFeeConfig.Verify() + if c.InitialFeeConfig == nil { + return nil } - return nil + + return c.InitialFeeConfig.Verify() } // GetFeeConfigManagerStatus returns the role of [address] for the fee config manager list.