From 11617d2d89844575f3b4f89ae00d8befe5f624c4 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Sat, 25 Feb 2023 00:56:04 +0300 Subject: [PATCH 1/6] add yet another custom marshaller to GetChainConfigResponse api --- internal/ethapi/api.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 8ace36596f..e8812d2921 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -28,6 +28,7 @@ package ethapi import ( "context" + "encoding/json" "errors" "fmt" "math/big" @@ -140,16 +141,44 @@ func (s *EthereumAPI) Syncing() (interface{}, error) { type GetChainConfigResponse struct { *params.ChainConfig - params.UpgradeConfig `json:"upgrades"` + UpgradeConfig params.UpgradeConfig `json:"upgrades"` } -func (s *BlockChainAPI) GetChainConfig(ctx context.Context) GetChainConfigResponse { +// MarshalJSON implements json.Marshaler. This is a workaround for the fact that +// the embedded ChainConfig struct has a MarshalJSON method, which prevents +// the default JSON marshalling from working for UpgradeConfig. +// TODO: consider removing this method by allowing external tag for the embedded +// ChainConfig struct. +func (s *GetChainConfigResponse) MarshalJSON() ([]byte, error) { + // embed the ChainConfig struct into the response + chainConfigJSON, err := json.Marshal(s.ChainConfig) + if err != nil { + return nil, err + } + + type upgrades struct { + UpgradeConfig params.UpgradeConfig `json:"upgrades"` + } + + upgradeJSON, err := json.Marshal(upgrades{s.UpgradeConfig}) + if err != nil { + return nil, err + } + // merge the two JSON objects + mergedJSON := make([]byte, 0, len(chainConfigJSON)+len(upgradeJSON)+1) + mergedJSON = append(mergedJSON, chainConfigJSON[:len(chainConfigJSON)-1]...) + mergedJSON = append(mergedJSON, ',') + mergedJSON = append(mergedJSON, upgradeJSON[1:]...) + return mergedJSON, nil +} + +func (s *BlockChainAPI) GetChainConfig(ctx context.Context) *GetChainConfigResponse { config := s.b.ChainConfig() resp := GetChainConfigResponse{ ChainConfig: config, UpgradeConfig: config.UpgradeConfig, } - return resp + return &resp } // TxPoolAPI offers and API for the transaction pool. It only operates on data that is non confidential. From 4a388f3d1be6c304ea54511ac6716fd0fadd2893 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Sat, 25 Feb 2023 02:50:02 +0300 Subject: [PATCH 2/6] enforce a json len before allocation --- internal/ethapi/api.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index e8812d2921..a0665b2c1f 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -57,6 +57,8 @@ import ( "github.com/tyler-smith/go-bip39" ) +const maxJSONLen = 64 * 1024 * 1024 // 64MB + // EthereumAPI provides an API to access Ethereum related information. type EthereumAPI struct { b Backend @@ -141,7 +143,7 @@ func (s *EthereumAPI) Syncing() (interface{}, error) { type GetChainConfigResponse struct { *params.ChainConfig - UpgradeConfig params.UpgradeConfig `json:"upgrades"` + UpgradeConfig params.UpgradeConfig } // MarshalJSON implements json.Marshaler. This is a workaround for the fact that @@ -155,6 +157,9 @@ func (s *GetChainConfigResponse) MarshalJSON() ([]byte, error) { if err != nil { return nil, err } + if len(chainConfigJSON) > maxJSONLen { + return nil, errors.New("value too large") + } type upgrades struct { UpgradeConfig params.UpgradeConfig `json:"upgrades"` @@ -164,6 +169,10 @@ func (s *GetChainConfigResponse) MarshalJSON() ([]byte, error) { if err != nil { return nil, err } + if len(upgradeJSON) > maxJSONLen { + return nil, errors.New("value too large") + } + // merge the two JSON objects mergedJSON := make([]byte, 0, len(chainConfigJSON)+len(upgradeJSON)+1) mergedJSON = append(mergedJSON, chainConfigJSON[:len(chainConfigJSON)-1]...) From aaa52f4afa9660a3e98424befbcb0cf8d088e144 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Wed, 1 Mar 2023 14:54:33 -0800 Subject: [PATCH 3/6] Move chain config wrapper type to params/ and add test --- internal/ethapi/api.go | 52 ++---------------------------------------- params/config.go | 49 +++++++++++++++++++++++++++++++++++++++ params/config_test.go | 19 +++++++++++++++ 3 files changed, 70 insertions(+), 50 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a0665b2c1f..947fd32bb1 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -28,7 +28,6 @@ package ethapi import ( "context" - "encoding/json" "errors" "fmt" "math/big" @@ -57,8 +56,6 @@ import ( "github.com/tyler-smith/go-bip39" ) -const maxJSONLen = 64 * 1024 * 1024 // 64MB - // EthereumAPI provides an API to access Ethereum related information. type EthereumAPI struct { b Backend @@ -141,53 +138,8 @@ func (s *EthereumAPI) Syncing() (interface{}, error) { return false, nil } -type GetChainConfigResponse struct { - *params.ChainConfig - UpgradeConfig params.UpgradeConfig -} - -// MarshalJSON implements json.Marshaler. This is a workaround for the fact that -// the embedded ChainConfig struct has a MarshalJSON method, which prevents -// the default JSON marshalling from working for UpgradeConfig. -// TODO: consider removing this method by allowing external tag for the embedded -// ChainConfig struct. -func (s *GetChainConfigResponse) MarshalJSON() ([]byte, error) { - // embed the ChainConfig struct into the response - chainConfigJSON, err := json.Marshal(s.ChainConfig) - if err != nil { - return nil, err - } - if len(chainConfigJSON) > maxJSONLen { - return nil, errors.New("value too large") - } - - type upgrades struct { - UpgradeConfig params.UpgradeConfig `json:"upgrades"` - } - - upgradeJSON, err := json.Marshal(upgrades{s.UpgradeConfig}) - if err != nil { - return nil, err - } - if len(upgradeJSON) > maxJSONLen { - return nil, errors.New("value too large") - } - - // merge the two JSON objects - mergedJSON := make([]byte, 0, len(chainConfigJSON)+len(upgradeJSON)+1) - mergedJSON = append(mergedJSON, chainConfigJSON[:len(chainConfigJSON)-1]...) - mergedJSON = append(mergedJSON, ',') - mergedJSON = append(mergedJSON, upgradeJSON[1:]...) - return mergedJSON, nil -} - -func (s *BlockChainAPI) GetChainConfig(ctx context.Context) *GetChainConfigResponse { - config := s.b.ChainConfig() - resp := GetChainConfigResponse{ - ChainConfig: config, - UpgradeConfig: config.UpgradeConfig, - } - return &resp +func (s *BlockChainAPI) GetChainConfig(ctx context.Context) *params.ChainConfigWithUpgradesMarshalled { + return s.b.ChainConfig().ToWithUpgradesMarshalled() } // TxPoolAPI offers and API for the transaction pool. It only operates on data that is non confidential. diff --git a/params/config.go b/params/config.go index 1f2289f2a9..81067168f3 100644 --- a/params/config.go +++ b/params/config.go @@ -40,6 +40,8 @@ import ( "github.com/ethereum/go-ethereum/common" ) +const maxJSONLen = 64 * 1024 * 1024 // 64MB + var ( errNonGenesisForkByHeight = errors.New("subnet-evm only supports forking by height at the genesis block") @@ -211,6 +213,53 @@ func (c ChainConfig) MarshalJSON() ([]byte, error) { return json.Marshal(raw) } +type ChainConfigWithUpgradesMarshalled struct { + *ChainConfig + UpgradeConfig UpgradeConfig `json:"upgrades,omitempty"` +} + +// MarshalJSON implements json.Marshaler. This is a workaround for the fact that +// the embedded ChainConfig struct has a MarshalJSON method, which prevents +// the default JSON marshalling from working for UpgradeConfig. +// TODO: consider removing this method by allowing external tag for the embedded +// ChainConfig struct. +func (s *ChainConfigWithUpgradesMarshalled) MarshalJSON() ([]byte, error) { + // embed the ChainConfig struct into the response + chainConfigJSON, err := json.Marshal(s.ChainConfig) + if err != nil { + return nil, err + } + if len(chainConfigJSON) > maxJSONLen { + return nil, errors.New("value too large") + } + + type upgrades struct { + UpgradeConfig UpgradeConfig `json:"upgrades"` + } + + upgradeJSON, err := json.Marshal(upgrades{s.UpgradeConfig}) + if err != nil { + return nil, err + } + if len(upgradeJSON) > maxJSONLen { + return nil, errors.New("value too large") + } + + // merge the two JSON objects + mergedJSON := make([]byte, 0, len(chainConfigJSON)+len(upgradeJSON)+1) + mergedJSON = append(mergedJSON, chainConfigJSON[:len(chainConfigJSON)-1]...) + mergedJSON = append(mergedJSON, ',') + mergedJSON = append(mergedJSON, upgradeJSON[1:]...) + return mergedJSON, nil +} + +func (c *ChainConfig) ToWithUpgradesMarshalled() *ChainConfigWithUpgradesMarshalled { + return &ChainConfigWithUpgradesMarshalled{ + ChainConfig: c, + UpgradeConfig: c.UpgradeConfig, + } +} + // UpgradeConfig includes the following configs that may be specified in upgradeBytes: // - Timestamps that enable avalanche network upgrades, // - Enabling or disabling precompiles as network upgrades. diff --git a/params/config_test.go b/params/config_test.go index a407c81bae..19e4344eae 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -34,6 +34,7 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" + "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) @@ -227,3 +228,21 @@ func TestActivePrecompiles(t *testing.T) { rules1 := config.AvalancheRules(common.Big0, common.Big1) require.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address)) } + +func TestChainConfigMarshalWithUpgrades(t *testing.T) { + config := ChainConfigWithUpgradesMarshalled{ + ChainConfig: TestChainConfig, + UpgradeConfig: UpgradeConfig{ + PrecompileUpgrades: []PrecompileUpgrade{ + { + Config: txallowlist.NewConfig(big.NewInt(100), nil, nil), + }, + }, + }, + } + result, err := json.Marshal(config) + require.NoError(t, err) + configBytes := []byte(`{"chainId":1,"feeConfig":{"gasLimit":8000000,"targetBlockRate":2,"minBaseFee":25000000000,"targetGas":15000000,"baseFeeChangeDenominator":36,"minBlockGasCost":0,"maxBlockGasCost":1000000,"blockGasCostStep":200000},"homesteadBlock":0,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"subnetEVMTimestamp":0,"UpgradeConfig":{"precompileUpgrades":[{"txAllowListConfig":{"blockTimestamp":100}}]}}`) + require.Equal(t, configBytes, result) + +} From 228c979fd51900fad95e18f3dbfcf579e54299c7 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Wed, 1 Mar 2023 15:06:07 -0800 Subject: [PATCH 4/6] Fix trailing newline --- params/config_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/params/config_test.go b/params/config_test.go index 19e4344eae..d5f690f128 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -244,5 +244,4 @@ func TestChainConfigMarshalWithUpgrades(t *testing.T) { require.NoError(t, err) configBytes := []byte(`{"chainId":1,"feeConfig":{"gasLimit":8000000,"targetBlockRate":2,"minBaseFee":25000000000,"targetGas":15000000,"baseFeeChangeDenominator":36,"minBlockGasCost":0,"maxBlockGasCost":1000000,"blockGasCostStep":200000},"homesteadBlock":0,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"subnetEVMTimestamp":0,"UpgradeConfig":{"precompileUpgrades":[{"txAllowListConfig":{"blockTimestamp":100}}]}}`) require.Equal(t, configBytes, result) - } From eafd5918414421200eb5c7b3f06173412d5126a1 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Wed, 1 Mar 2023 16:06:59 -0800 Subject: [PATCH 5/6] Remove flaky unit test --- params/config_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/params/config_test.go b/params/config_test.go index d5f690f128..a407c81bae 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -34,7 +34,6 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" - "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) @@ -228,20 +227,3 @@ func TestActivePrecompiles(t *testing.T) { rules1 := config.AvalancheRules(common.Big0, common.Big1) require.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address)) } - -func TestChainConfigMarshalWithUpgrades(t *testing.T) { - config := ChainConfigWithUpgradesMarshalled{ - ChainConfig: TestChainConfig, - UpgradeConfig: UpgradeConfig{ - PrecompileUpgrades: []PrecompileUpgrade{ - { - Config: txallowlist.NewConfig(big.NewInt(100), nil, nil), - }, - }, - }, - } - result, err := json.Marshal(config) - require.NoError(t, err) - configBytes := []byte(`{"chainId":1,"feeConfig":{"gasLimit":8000000,"targetBlockRate":2,"minBaseFee":25000000000,"targetGas":15000000,"baseFeeChangeDenominator":36,"minBlockGasCost":0,"maxBlockGasCost":1000000,"blockGasCostStep":200000},"homesteadBlock":0,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"subnetEVMTimestamp":0,"UpgradeConfig":{"precompileUpgrades":[{"txAllowListConfig":{"blockTimestamp":100}}]}}`) - require.Equal(t, configBytes, result) -} From 9302f75d4971d2228a1daafce7dc68c5f9cf813c Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Wed, 1 Mar 2023 16:28:54 -0800 Subject: [PATCH 6/6] Add back unit test with JSONeq --- params/config_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/params/config_test.go b/params/config_test.go index a407c81bae..1b4fadbdb3 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -34,6 +34,7 @@ import ( "github.com/ava-labs/subnet-evm/precompile/contracts/nativeminter" "github.com/ava-labs/subnet-evm/precompile/contracts/rewardmanager" + "github.com/ava-labs/subnet-evm/precompile/contracts/txallowlist" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" ) @@ -227,3 +228,20 @@ func TestActivePrecompiles(t *testing.T) { rules1 := config.AvalancheRules(common.Big0, common.Big1) require.False(t, rules1.IsPrecompileEnabled(nativeminter.Module.Address)) } + +func TestChainConfigMarshalWithUpgrades(t *testing.T) { + config := ChainConfigWithUpgradesMarshalled{ + ChainConfig: TestChainConfig, + UpgradeConfig: UpgradeConfig{ + PrecompileUpgrades: []PrecompileUpgrade{ + { + Config: txallowlist.NewConfig(big.NewInt(100), nil, nil), + }, + }, + }, + } + result, err := json.Marshal(&config) + require.NoError(t, err) + expectedJSON := `{"chainId":1,"feeConfig":{"gasLimit":8000000,"targetBlockRate":2,"minBaseFee":25000000000,"targetGas":15000000,"baseFeeChangeDenominator":36,"minBlockGasCost":0,"maxBlockGasCost":1000000,"blockGasCostStep":200000},"homesteadBlock":0,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"subnetEVMTimestamp":0,"upgrades":{"precompileUpgrades":[{"txAllowListConfig":{"blockTimestamp":100}}]}}` + require.JSONEq(t, expectedJSON, string(result)) +}