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.

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

refactor: remove params dependency on modules #1261

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
af8ab3e
experiment: lazy-init precompiles in `ChainConfig`
ARR4N Jul 29, 2024
7f39a4c
refactor: move `modules`-dependent code out of `params` (only builds;…
ARR4N Jul 30, 2024
6acef2a
fix: change `ChainConfig.Fn()` method calls to `modules.Fn(ChainConfig)`
ARR4N Jul 30, 2024
8d9a7b6
refactor: HACKY! `./params/...` and `./precompile/...` tests passing
ARR4N Jul 30, 2024
574f91d
refactor: make `params.ChainConfig.Rules()` self-contained
ARR4N Jul 31, 2024
2722fb8
chore: remove unnecessary `nil` check
ARR4N Jul 31, 2024
87b3b8f
feat: `paramsjson` package for JSON unmarshalling `params.<T>` that d…
ARR4N Jul 31, 2024
ff2f78a
Merge branch 'master' into arr4n/circular-dep-types-precompiles
ARR4N Jul 31, 2024
3899608
chore: revert change to `params.TestRules`
ARR4N Aug 1, 2024
1f587b4
refactor: inject `paramsjson.Unmarshal()` as `UnmarshalJSON()` method…
ARR4N Aug 5, 2024
9b40ffd
Merge branch 'master' into arr4n/circular-dep-types-precompiles
ARR4N Aug 5, 2024
e21cf4b
refactor: use regular `json.Unmarshal()` and blank `import _ .../para…
ARR4N Aug 5, 2024
ae55c97
fix: _ import `paramsjson` in tests
ARR4N Aug 5, 2024
8207300
chore: blank _ import `paramsjson` in `plugin/main.go`
ARR4N Aug 5, 2024
143317d
chore: blank _ import `paramsjson` in `tests/utils/subnet.go`
ARR4N Aug 5, 2024
b19cc5c
refactor: `precompileconfig.Config.Address()` method instead of map o…
ARR4N Aug 5, 2024
03c5f9d
refactor: move `params.ChainConfig` methods back from `modules` package
ARR4N Aug 6, 2024
29aa763
refactor: move `params.ChainConfig` methods to their original locatio…
ARR4N Aug 6, 2024
afce466
chore: revert cosmetic changes
ARR4N Aug 6, 2024
ba12b54
chore: revert as many changes as possible in `params.ChainConfig.Rule…
ARR4N Aug 6, 2024
b55bba4
chore: add `Address()` method on test-double `precompileconfig.Config…
ARR4N Aug 6, 2024
eb4e226
fix: all unit tests passing locally
ARR4N Aug 7, 2024
b15507b
chore: update copyright years to keep linter happy
ARR4N Aug 7, 2024
6a7803e
refactor: `paramsjson` misc (see full message)
ARR4N Aug 7, 2024
0aa0922
refactor: add `Must` prefix and panics to `params.RegisterJSONUnmarsh…
ARR4N Aug 7, 2024
30e45f4
test: `params.UpgradeConfig` comprehensive testing
ARR4N Aug 7, 2024
8d7c002
chore: remove unnecessary variable declaration
ARR4N Aug 7, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import (
{{- if .Contract.AllowList}}
"github.com/ava-labs/subnet-evm/precompile/allowlist"

"github.com/ethereum/go-ethereum/common"
{{- end}}


"github.com/ethereum/go-ethereum/common"
)

var _ precompileconfig.Config = &Config{}
Expand Down Expand Up @@ -63,6 +63,10 @@ func NewDisableConfig(blockTimestamp *uint64) *Config {
// This should be the same key as used in the precompile module.
func (*Config) Key() string { return ConfigKey }

// Address returns the address for the {{.Contract.Type}} precompileconfig.
// This should be the same key as used in the precompile module.
func (*Config) Address() common.Address { return ContractAddress }

// Verify tries to verify Config and returns an error accordingly.
func (c *Config) Verify(chainConfig precompileconfig.ChainConfig) error {
{{- if .Contract.AllowList}}
Expand Down
8 changes: 8 additions & 0 deletions core/rawdb/accessors_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ import (
"github.com/ethereum/go-ethereum/ethdb"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/rlp"

// This side-effect import goes against recommendations for not using blank
// imports in libraries: https://google.github.io/styleguide/go/decisions#import-blank-import-_
// This is deliberate because the ReadChainConfig() method only logs errors
// and doesn't bubble them up; this caused hours of problems in development!
// The described cons of using a blank import here are _far_ outweighed by
// the benefits.
_ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency
)

// ReadDatabaseVersion retrieves the version number of the database.
Expand Down
2 changes: 2 additions & 0 deletions eth/tracers/internal/tracetest/calltrace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import (
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/common/math"
"github.com/ethereum/go-ethereum/rlp"

_ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency
)

type callContext struct {
Expand Down
39 changes: 20 additions & 19 deletions params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/version"
"github.com/ava-labs/subnet-evm/commontype"
"github.com/ava-labs/subnet-evm/precompile/modules"
"github.com/ava-labs/subnet-evm/precompile/precompileconfig"
"github.com/ava-labs/subnet-evm/utils"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -751,27 +750,29 @@ func (c *ChainConfig) rules(num *big.Int, timestamp uint64) Rules {
// Rules returns the Avalanche modified rules to support Avalanche
// network upgrades
func (c *ChainConfig) Rules(blockNum *big.Int, timestamp uint64) Rules {
rules := c.rules(blockNum, timestamp)

rules.AvalancheRules = c.GetAvalancheRules(timestamp)

// Initialize the stateful precompiles that should be enabled at [blockTimestamp].
rules.ActivePrecompiles = make(map[common.Address]precompileconfig.Config)
rules.Predicaters = make(map[common.Address]precompileconfig.Predicater)
rules.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter)
for _, module := range modules.RegisteredModules() {
if config := c.getActivePrecompileConfig(module.Address, timestamp); config != nil && !config.IsDisabled() {
rules.ActivePrecompiles[module.Address] = config
if predicater, ok := config.(precompileconfig.Predicater); ok {
rules.Predicaters[module.Address] = predicater
}
if precompileAccepter, ok := config.(precompileconfig.Accepter); ok {
rules.AccepterPrecompiles[module.Address] = precompileAccepter
}
r := c.rules(blockNum, timestamp)
r.AvalancheRules = c.GetAvalancheRules(timestamp)

r.ActivePrecompiles = make(map[common.Address]precompileconfig.Config)
r.Predicaters = make(map[common.Address]precompileconfig.Predicater)
r.AccepterPrecompiles = make(map[common.Address]precompileconfig.Accepter)

for _, addr := range c.allPrecompileAddresses() {
cfg := c.getActivePrecompileConfig(addr, timestamp)
if cfg == nil || cfg.IsDisabled() {
continue
}
r.ActivePrecompiles[addr] = cfg

if p, ok := cfg.(precompileconfig.Predicater); ok {
r.Predicaters[addr] = p
}
if a, ok := cfg.(precompileconfig.Accepter); ok {
r.AccepterPrecompiles[addr] = a
}
}

return rules
return r
}

// GetFeeConfig returns the original FeeConfig contained in the genesis ChainConfig.
Expand Down
39 changes: 0 additions & 39 deletions params/config_extra.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,6 @@ type AvalancheContext struct {
SnowCtx *snow.Context
}

// UnmarshalJSON parses the JSON-encoded data and stores the result in the
// object pointed to by c.
// This is a custom unmarshaler to handle the Precompiles field.
// Precompiles was presented as an inline object in the JSON.
// This custom unmarshaler ensures backwards compatibility with the old format.
func (c *ChainConfig) UnmarshalJSON(data []byte) error {
// Alias ChainConfig to avoid recursion
type _ChainConfig ChainConfig
tmp := _ChainConfig{}
if err := json.Unmarshal(data, &tmp); err != nil {
return err
}

// At this point we have populated all fields except PrecompileUpgrade
*c = ChainConfig(tmp)

// Unmarshal inlined PrecompileUpgrade
return json.Unmarshal(data, &c.GenesisPrecompiles)
}

// MarshalJSON returns the JSON encoding of c.
// This is a custom marshaler to handle the Precompiles field.
func (c ChainConfig) MarshalJSON() ([]byte, error) {
Expand Down Expand Up @@ -119,25 +99,6 @@ func (cu ChainConfigWithUpgradesJSON) MarshalJSON() ([]byte, error) {
return mergedJSON, nil
}

func (cu *ChainConfigWithUpgradesJSON) UnmarshalJSON(input []byte) error {
var cc ChainConfig
if err := json.Unmarshal(input, &cc); err != nil {
return err
}

type upgrades struct {
UpgradeConfig UpgradeConfig `json:"upgrades"`
}

var u upgrades
if err := json.Unmarshal(input, &u); err != nil {
return err
}
cu.ChainConfig = cc
cu.UpgradeConfig = u.UpgradeConfig
return nil
}

// ToWithUpgradesJSON converts the ChainConfig to ChainConfigWithUpgradesJSON with upgrades explicitly displayed.
// ChainConfig does not include upgrades in its JSON output.
// This is a workaround for showing upgrades in the JSON output.
Expand Down
20 changes: 12 additions & 8 deletions params/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.

package params
package params_test

import (
"encoding/json"
Expand All @@ -39,7 +39,11 @@ import (
"github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist"
"github.com/ava-labs/subnet-evm/utils"
"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

. "github.com/ava-labs/subnet-evm/params"
_ "github.com/ava-labs/subnet-evm/params/paramsjson" // registers JSON unmarshalers to avoid circular dependency
)

func TestCheckCompatible(t *testing.T) {
Expand Down Expand Up @@ -211,8 +215,8 @@ func TestConfigUnmarshalJSON(t *testing.T) {
}
}
`)
c := ChainConfig{}
err := json.Unmarshal(config, &c)
c := &ChainConfig{}
err := json.Unmarshal(config, c)
require.NoError(err)

require.Equal(c.ChainID, big.NewInt(43214))
Expand All @@ -230,14 +234,14 @@ func TestConfigUnmarshalJSON(t *testing.T) {
// Marshal and unmarshal again and check that the result is the same
marshaled, err := json.Marshal(c)
require.NoError(err)
c2 := ChainConfig{}
err = json.Unmarshal(marshaled, &c2)
c2 := &ChainConfig{}
err = json.Unmarshal(marshaled, c2)
require.NoError(err)
require.Equal(c, c2)
}

func TestActivePrecompiles(t *testing.T) {
config := ChainConfig{
config := &ChainConfig{
UpgradeConfig: UpgradeConfig{
PrecompileUpgrades: []PrecompileUpgrade{
{
Expand All @@ -251,10 +255,10 @@ func TestActivePrecompiles(t *testing.T) {
}

rules0 := config.Rules(common.Big0, 0)
require.True(t, rules0.IsPrecompileEnabled(nativeminter.Module.Address))
assert.True(t, rules0.IsPrecompileEnabled(nativeminter.Module.Address))

rules1 := config.Rules(common.Big0, 1)
require.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address))
assert.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address))
}

func TestChainConfigMarshalWithUpgrades(t *testing.T) {
Expand Down
82 changes: 82 additions & 0 deletions params/json.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package params

import (
"fmt"
)

// A JSONUnmarshaler is a type-safe function for unmarshalling a JSON buffer
// into a specific type.
type JSONUnmarshaler[T any] func([]byte, T) error

var jsonUmarshalers struct {
cc JSONUnmarshaler[*ChainConfig]
cu JSONUnmarshaler[*ChainConfigWithUpgradesJSON]
uc JSONUnmarshaler[*UpgradeConfig]
pu JSONUnmarshaler[*PrecompileUpgrade]
}

// MustRegisterJSONUnmarshalers registers the JSON unmarshalling functions for
// various types. This allows their unmarshalling behaviour to be injected by
// the [params/paramsjson] package, which can't be directly imported as it would
// result in a circular dependency.
//
// This function SHOULD NOT be called directly. Instead, blank import the
// [params/paramsjson] package, which registers unmarshalers in its init()
// function.
//
// MustRegisterJSONUnmarshalers panics if any functions are nil or if called
// more than once.
func MustRegisterJSONUnmarshalers(
cc JSONUnmarshaler[*ChainConfig],
cu JSONUnmarshaler[*ChainConfigWithUpgradesJSON],
uc JSONUnmarshaler[*UpgradeConfig],
pu JSONUnmarshaler[*PrecompileUpgrade],
) {
if jsonUmarshalers.cc != nil {
panic("JSON unmarshalers already registered")
}
panicIfNil(cc)
panicIfNil(cu)
panicIfNil(uc)
panicIfNil(pu)

jsonUmarshalers.cc = cc
jsonUmarshalers.cu = cu
jsonUmarshalers.uc = uc
jsonUmarshalers.pu = pu
}

func panicIfNil[T any](fn JSONUnmarshaler[T]) {
if fn == nil {
panic(fmt.Sprintf("registering nil %T", fn))
}
}

func unmarshalJSON[T any](u JSONUnmarshaler[T], data []byte, v T) error {
if u == nil {
return fmt.Errorf(`%T is nil; did you remember to import _ "github.com/ava-labs/subnet-evm/params/paramsjson"`, u)
}
return u(data, v)
}

// UnmarshalJSON parses the JSON-encoded data and stores the result in the
// object pointed to by c.
// This is a custom unmarshaler to handle the Precompiles field.
// Precompiles was presented as an inline object in the JSON.
// This custom unmarshaler ensures backwards compatibility with the old format.
// TODO(arr4n) update this method comment DO NOT MERGE
func (c *ChainConfig) UnmarshalJSON(data []byte) error {
return unmarshalJSON(jsonUmarshalers.cc, data, c)
}

func (cu *ChainConfigWithUpgradesJSON) UnmarshalJSON(data []byte) error {
return unmarshalJSON(jsonUmarshalers.cu, data, cu)
}

func (u *UpgradeConfig) UnmarshalJSON(data []byte) error {
return unmarshalJSON(jsonUmarshalers.uc, data, u)
}

func (u *PrecompileUpgrade) UnmarshalJSON(data []byte) error {
return unmarshalJSON(jsonUmarshalers.pu, data, u)
}
Loading
Loading