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

add custom marshaller for GetChainConfigResponse api #546

Merged
merged 8 commits into from
Mar 2, 2023
14 changes: 2 additions & 12 deletions internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,8 @@ func (s *EthereumAPI) Syncing() (interface{}, error) {
return false, nil
}

type GetChainConfigResponse struct {
*params.ChainConfig
params.UpgradeConfig `json:"upgrades"`
}

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.
Expand Down
49 changes: 49 additions & 0 deletions params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a custom unmarshaler too for them to use?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this may need to be a follow on to get this bug fix in for the release today.

// 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.
Expand Down
18 changes: 18 additions & 0 deletions params/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually, we'd want to ensure unmarshaled result == config?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't currently have a custom unmarshaller for this, so updating this test may need to be a follow on if we want to get this in for the release.

}