Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename configs: alternative #520

Merged
merged 5 commits into from
Feb 17, 2023
Merged

Rename configs: alternative #520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
99b628a
alternative renaming for precompile configs
darioush Feb 17, 2023
a42e486
fixes
darioush Feb 17, 2023
5a00ece
update naming
darioush Feb 17, 2023
5f686a4
rename to AllowListConfig
darioush Feb 17, 2023
9e32126
simplify
darioush Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions params/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (

"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/subnet-evm/commontype"
"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ava-labs/subnet-evm/precompile/modules"
"github.com/ava-labs/subnet-evm/utils"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -558,7 +558,7 @@ type Rules struct {
// for this rule set.
// Note: none of these addresses should conflict with the address space used by
// any existing precompiles.
ActivePrecompiles map[common.Address]config.Config
ActivePrecompiles map[common.Address]precompileConfig.Config
}

// IsPrecompileEnabled returns true if the precompile at [addr] is enabled for this rule set.
Expand Down Expand Up @@ -594,7 +594,7 @@ func (c *ChainConfig) AvalancheRules(blockNum, blockTimestamp *big.Int) Rules {
rules.IsSubnetEVM = c.IsSubnetEVM(blockTimestamp)

// Initialize the stateful precompiles that should be enabled at [blockTimestamp].
rules.ActivePrecompiles = make(map[common.Address]config.Config)
rules.ActivePrecompiles = make(map[common.Address]precompileConfig.Config)
for _, module := range modules.RegisteredModules() {
if config := c.GetActivePrecompileConfig(module.Address, blockTimestamp); config != nil && !config.IsDisabled() {
rules.ActivePrecompiles[module.Address] = config
Expand Down
9 changes: 4 additions & 5 deletions params/precompile_upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"fmt"
"math/big"

"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ava-labs/subnet-evm/precompile/modules"
"github.com/ava-labs/subnet-evm/utils"
Expand All @@ -23,7 +22,7 @@ var errNoKey = errors.New("PrecompileUpgrade cannot be empty")
// based on the key. Keys are defined in each precompile module, and registered in
// precompile/registry/registry.go.
type PrecompileUpgrade struct {
config.Config
precompileConfig.Config
}

// UnmarshalJSON unmarshals the json into the correct precompile config type
Expand Down Expand Up @@ -152,7 +151,7 @@ func (c *ChainConfig) verifyPrecompileUpgrades() error {

// GetActivePrecompileConfig returns the most recent precompile config corresponding to [address].
// If none have occurred, returns nil.
func (c *ChainConfig) GetActivePrecompileConfig(address common.Address, blockTimestamp *big.Int) config.Config {
func (c *ChainConfig) GetActivePrecompileConfig(address common.Address, blockTimestamp *big.Int) precompileConfig.Config {
configs := c.GetActivatingPrecompileConfigs(address, nil, blockTimestamp, c.PrecompileUpgrades)
if len(configs) == 0 {
return nil
Expand All @@ -162,13 +161,13 @@ func (c *ChainConfig) GetActivePrecompileConfig(address common.Address, blockTim

// GetActivatingPrecompileConfigs returns all upgrades configured to activate during the state transition from a block with timestamp [from]
// to a block with timestamp [to].
func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, from *big.Int, to *big.Int, upgrades []PrecompileUpgrade) []config.Config {
func (c *ChainConfig) GetActivatingPrecompileConfigs(address common.Address, from *big.Int, to *big.Int, upgrades []PrecompileUpgrade) []precompileConfig.Config {
// Get key from address.
module, ok := modules.GetPrecompileModuleByAddress(address)
if !ok {
return nil
}
configs := make([]config.Config, 0)
configs := make([]precompileConfig.Config, 0)
key := module.ConfigKey
// First check the embedded [upgrade] for precompiles configured
// in the genesis chain config.
Expand Down
4 changes: 2 additions & 2 deletions params/precompiles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ package params
import (
"encoding/json"

"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ava-labs/subnet-evm/precompile/modules"
)

type Precompiles map[string]config.Config
type Precompiles map[string]precompileConfig.Config

// UnmarshalJSON parses the JSON-encoded data into the ChainConfigPrecompiles.
// ChainConfigPrecompiles is a map of precompile module keys to their
Expand Down
10 changes: 5 additions & 5 deletions precompile/allowlist/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import (
"github.com/ethereum/go-ethereum/common"
)

// Config specifies the initial set of addresses with Admin or Enabled roles.
type Config struct {
// AllowList specifies the initial set of addresses with Admin or Enabled roles.
type AllowList struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer this to be AllowListConfig, since AllowList is more like the collection of these contract functions.

darioush marked this conversation as resolved.
Show resolved Hide resolved
AdminAddresses []common.Address `json:"adminAddresses,omitempty"` // initial admin addresses
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 *Config) Configure(state contract.StateDB, precompileAddr common.Address) error {
func (c *AllowList) Configure(state contract.StateDB, precompileAddr common.Address) error {
for _, enabledAddr := range c.EnabledAddresses {
SetAllowListRole(state, precompileAddr, enabledAddr, EnabledRole)
}
Expand All @@ -29,7 +29,7 @@ func (c *Config) Configure(state contract.StateDB, precompileAddr common.Address
}

// Equal returns true iff [other] has the same admins in the same order in its allow list.
func (c *Config) Equal(other *Config) bool {
func (c *AllowList) Equal(other *AllowList) bool {
if other == nil {
return false
}
Expand All @@ -52,7 +52,7 @@ func areEqualAddressLists(current []common.Address, other []common.Address) bool
}

// Verify returns an error if there is an overlapping address between admin and enabled roles
func (c *Config) Verify() error {
func (c *AllowList) Verify() error {
addressMap := make(map[common.Address]Role) // tracks which addresses we have seen and their role

// check for duplicates in enabled list
Expand Down
2 changes: 1 addition & 1 deletion precompile/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// See the file LICENSE for licensing terms.

// Defines the stateless interface for unmarshalling an arbitrary config of a precompile
package config
package precompileConfig
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we want to change the package name, we should also change the folder name

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not %100 sure about Go best practices but I feel like package name should be precompileconfig

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree it should be all lowercase. Generally, I think multi-word package names are discouraged. I'd prefer to leave as config and import under the alias precompileConfig, but maybe that's too much manual work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that before but I think the lowercase option is better (not a fan of aliasing), feel free to have a look at the previous commit and lmk what you think @aaronbuchwald .

This package is going to be a bit hard to fit with the standard rules.


import (
"math/big"
Expand Down
3 changes: 1 addition & 2 deletions precompile/config/mock_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
// See the file LICENSE for licensing terms.

// TODO: replace with gomock

package config
package precompileConfig

import (
"math/big"
Expand Down
2 changes: 1 addition & 1 deletion precompile/config/upgradeable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// (c) 2022 Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package config
package precompileConfig

import (
"math/big"
Expand Down
6 changes: 3 additions & 3 deletions precompile/contract/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/subnet-evm/commontype"
"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ethereum/go-ethereum/common"
)

Expand Down Expand Up @@ -69,10 +69,10 @@ type BlockContext interface {
}

type Configurator interface {
NewConfig() config.Config
NewConfig() precompileConfig.Config
Configure(
chainConfig ChainConfig,
precompileConfig config.Config,
precompileConfig precompileConfig.Config,
state StateDB,
blockContext BlockContext,
) error
Expand Down
4 changes: 2 additions & 2 deletions precompile/contract/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"testing"

"github.com/ava-labs/subnet-evm/accounts/abi"
"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ava-labs/subnet-evm/vmerrs"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
Expand Down Expand Up @@ -39,7 +39,7 @@ type PrecompileTest struct {
// It should be the same precompile config that is used in the
// precompile's configurator.
// If nil, Configure on the Configurator will not be called.
Config config.Config
Config precompileConfig.Config
// BeforeHook is called before the precompile is called.
BeforeHook func(t *testing.T, state StateDB)
// AfterHook is called after the precompile is called.
Expand Down
20 changes: 10 additions & 10 deletions precompile/contracts/deployerallowlist/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,36 @@ import (
"math/big"

"github.com/ava-labs/subnet-evm/precompile/allowlist"
"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ethereum/go-ethereum/common"
)

var _ config.Config = &Config{}
var _ precompileConfig.Config = &Config{}

// Config contains the configuration for the ContractDeployerAllowList precompile,
// consisting of the initial allowlist and the timestamp for the network upgrade.
type Config struct {
allowlist.Config
config.Upgrade
allowlist.AllowList
precompileConfig.Upgrade
}

// NewConfig returns a config for a network upgrade at [blockTimestamp] that enables
// ContractDeployerAllowList with [admins] and [enableds] as members of the allowlist.
func NewConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address) *Config {
return &Config{
Config: allowlist.Config{
AllowList: allowlist.AllowList{
AdminAddresses: admins,
EnabledAddresses: enableds,
},
Upgrade: config.Upgrade{BlockTimestamp: blockTimestamp},
Upgrade: precompileConfig.Upgrade{BlockTimestamp: blockTimestamp},
}
}

// NewDisableConfig returns config for a network upgrade at [blockTimestamp]
// that disables ContractDeployerAllowList.
func NewDisableConfig(blockTimestamp *big.Int) *Config {
return &Config{
Upgrade: config.Upgrade{
Upgrade: precompileConfig.Upgrade{
BlockTimestamp: blockTimestamp,
Disable: true,
},
Expand All @@ -46,13 +46,13 @@ func NewDisableConfig(blockTimestamp *big.Int) *Config {
func (*Config) Key() string { return ConfigKey }

// Equal returns true if [cfg] is a [*ContractDeployerAllowListConfig] and it has been configured identical to [c].
func (c *Config) Equal(cfg config.Config) bool {
func (c *Config) Equal(cfg precompileConfig.Config) bool {
// typecast before comparison
other, ok := (cfg).(*Config)
if !ok {
return false
}
return c.Upgrade.Equal(&other.Upgrade) && c.Config.Equal(&other.Config)
return c.Upgrade.Equal(&other.Upgrade) && c.AllowList.Equal(&other.AllowList)
}

func (c *Config) Verify() error { return c.Config.Verify() }
func (c *Config) Verify() error { return c.AllowList.Verify() }
8 changes: 4 additions & 4 deletions precompile/contracts/deployerallowlist/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package deployerallowlist
import (
"fmt"

"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ava-labs/subnet-evm/precompile/contract"
"github.com/ava-labs/subnet-evm/precompile/modules"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -35,15 +35,15 @@ func init() {
}
}

func (*configurator) NewConfig() config.Config {
func (*configurator) NewConfig() precompileConfig.Config {
return &Config{}
}

// Configure configures [state] with the given [cfg] config.
func (c *configurator) Configure(_ contract.ChainConfig, cfg config.Config, state contract.StateDB, _ contract.BlockContext) error {
func (c *configurator) Configure(_ contract.ChainConfig, cfg precompileConfig.Config, state contract.StateDB, _ contract.BlockContext) error {
config, ok := cfg.(*Config)
if !ok {
return fmt.Errorf("incorrect config %T: %v", config, config)
}
return config.Config.Configure(state, ContractAddress)
return config.AllowList.Configure(state, ContractAddress)
}
20 changes: 10 additions & 10 deletions precompile/contracts/feemanager/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ import (

"github.com/ava-labs/subnet-evm/commontype"
"github.com/ava-labs/subnet-evm/precompile/allowlist"
"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ethereum/go-ethereum/common"
)

var _ config.Config = &Config{}
var _ precompileConfig.Config = &Config{}

// Config implements the StatefulPrecompileConfig interface while adding in the
// FeeManager specific precompile config.
type Config struct {
allowlist.Config // Config for the fee config manager allow list
config.Upgrade
allowlist.AllowList // Config for the fee config manager allow list
precompileConfig.Upgrade
InitialFeeConfig *commontype.FeeConfig `json:"initialFeeConfig,omitempty"` // initial fee config to be immediately activated
}

Expand All @@ -27,11 +27,11 @@ type Config struct {
// allowlist with [initialConfig] as initial fee config if specified.
func NewConfig(blockTimestamp *big.Int, admins []common.Address, enableds []common.Address, initialConfig *commontype.FeeConfig) *Config {
return &Config{
Config: allowlist.Config{
AllowList: allowlist.AllowList{
AdminAddresses: admins,
EnabledAddresses: enableds,
},
Upgrade: config.Upgrade{BlockTimestamp: blockTimestamp},
Upgrade: precompileConfig.Upgrade{BlockTimestamp: blockTimestamp},
InitialFeeConfig: initialConfig,
}
}
Expand All @@ -40,7 +40,7 @@ func NewConfig(blockTimestamp *big.Int, admins []common.Address, enableds []comm
// that disables FeeManager.
func NewDisableConfig(blockTimestamp *big.Int) *Config {
return &Config{
Upgrade: config.Upgrade{
Upgrade: precompileConfig.Upgrade{
BlockTimestamp: blockTimestamp,
Disable: true,
},
Expand All @@ -50,13 +50,13 @@ func NewDisableConfig(blockTimestamp *big.Int) *Config {
func (*Config) Key() string { return ConfigKey }

// Equal returns true if [cfg] is a [*FeeManagerConfig] and it has been configured identical to [c].
func (c *Config) Equal(cfg config.Config) bool {
func (c *Config) Equal(cfg precompileConfig.Config) bool {
// typecast before comparison
other, ok := (cfg).(*Config)
if !ok {
return false
}
eq := c.Upgrade.Equal(&other.Upgrade) && c.Config.Equal(&other.Config)
eq := c.Upgrade.Equal(&other.Upgrade) && c.AllowList.Equal(&other.AllowList)
if !eq {
return false
}
Expand All @@ -69,7 +69,7 @@ func (c *Config) Equal(cfg config.Config) bool {
}

func (c *Config) Verify() error {
if err := c.Config.Verify(); err != nil {
if err := c.AllowList.Verify(); err != nil {
return err
}
if c.InitialFeeConfig == nil {
Expand Down
8 changes: 4 additions & 4 deletions precompile/contracts/feemanager/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package feemanager
import (
"fmt"

"github.com/ava-labs/subnet-evm/precompile/config"
precompileConfig "github.com/ava-labs/subnet-evm/precompile/config"
"github.com/ava-labs/subnet-evm/precompile/contract"
"github.com/ava-labs/subnet-evm/precompile/modules"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -35,12 +35,12 @@ func init() {
}
}

func (*configurator) NewConfig() config.Config {
func (*configurator) NewConfig() precompileConfig.Config {
return &Config{}
}

// Configure configures [state] with the desired admins based on [configIface].
func (*configurator) Configure(chainConfig contract.ChainConfig, cfg config.Config, state contract.StateDB, blockContext contract.BlockContext) error {
func (*configurator) Configure(chainConfig contract.ChainConfig, cfg precompileConfig.Config, state contract.StateDB, blockContext contract.BlockContext) error {
config, ok := cfg.(*Config)
if !ok {
return fmt.Errorf("incorrect config %T: %v", config, config)
Expand All @@ -57,5 +57,5 @@ func (*configurator) Configure(chainConfig contract.ChainConfig, cfg config.Conf
return fmt.Errorf("cannot configure fee config in chain config: %w", err)
}
}
return config.Config.Configure(state, ContractAddress)
return config.AllowList.Configure(state, ContractAddress)
}
Loading