From 1fdc23e7711d539a0d6042c98c14445cf5d0d905 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 20 Dec 2022 21:28:45 +0300 Subject: [PATCH 01/10] rework on panics in precompiles --- accounts/abi/bind/precompile_template.go | 35 ++++++++---- core/genesis.go | 5 +- core/state_processor.go | 6 +- miner/worker.go | 5 +- params/precompile_config.go | 9 ++- precompile/allow_list.go | 10 +++- precompile/contract.go | 11 ++-- precompile/contract_deployer_allow_list.go | 4 +- precompile/contract_native_minter.go | 15 +++-- precompile/fee_config_manager.go | 32 +++++++---- precompile/reward_manager.go | 65 +++++++++------------- precompile/stateful_precompile_config.go | 6 +- precompile/tx_allow_list.go | 4 +- precompile/utils.go | 10 ++-- 14 files changed, 126 insertions(+), 91 deletions(-) diff --git a/accounts/abi/bind/precompile_template.go b/accounts/abi/bind/precompile_template.go index 2f4cd39ad1..596d96c351 100644 --- a/accounts/abi/bind/precompile_template.go +++ b/accounts/abi/bind/precompile_template.go @@ -46,6 +46,7 @@ Typically, custom codes are required in only those areas. package precompile import ( + "encoding/json" "math/big" "errors" "fmt" @@ -75,6 +76,7 @@ var ( _ = big.NewInt _ = strings.NewReader _ = fmt.Printf + _ = json.Unmarshal ) {{$contract := .Contract}} @@ -148,7 +150,10 @@ func init() { } {{.Contract.Type}}ABI = parsed - {{.Contract.Type}}Precompile = create{{.Contract.Type}}Precompile({{.Contract.Type}}Address) + {{.Contract.Type}}Precompile, err = create{{.Contract.Type}}Precompile({{.Contract.Type}}Address) + if err != nil { + panic(err) + } } // New{{.Contract.Type}}Config returns a config for a network upgrade at [blockTimestamp] that enables @@ -198,9 +203,10 @@ func (c *{{.Contract.Type}}Config) Address() common.Address { } // Configure configures [state] with the initial configuration. -func (c *{{.Contract.Type}}Config) Configure(_ ChainConfig, state StateDB, _ BlockContext) { +func (c *{{.Contract.Type}}Config) Configure(_ ChainConfig, state StateDB, _ BlockContext) error { {{if .Contract.AllowList}}c.AllowListConfig.Configure(state, {{.Contract.Type}}Address){{end}} // CUSTOM CODE STARTS HERE + return nil } // Contract returns the singleton stateful precompiled contract to be used for {{.Contract.Type}}. @@ -404,27 +410,32 @@ func {{decapitalise $contract.Type}}Fallback (accessibleState PrecompileAccessib // create{{.Contract.Type}}Precompile returns a StatefulPrecompiledContract with getters and setters for the precompile. {{if .Contract.AllowList}} // Access to the getters/setters is controlled by an allow list for [precompileAddr].{{end}} -func create{{.Contract.Type}}Precompile(precompileAddr common.Address) StatefulPrecompiledContract { +func create{{.Contract.Type}}Precompile(precompileAddr common.Address) (StatefulPrecompiledContract, error) { var functions []*statefulPrecompileFunction {{- if .Contract.AllowList}} functions = append(functions, createAllowListFunctions(precompileAddr)...) {{- end}} - {{range .Contract.Funcs}} - method{{.Normalized.Name}}, ok := {{$contract.Type}}ABI.Methods["{{.Original.Name}}"] - if !ok{ - panic("given method does not exist in the ABI") + abiFunctionMap := map[string]RunStatefulPrecompileFunc{ + {{- range .Contract.Funcs}} + "{{.Original.Name}}": {{decapitalise .Normalized.Name}}, + {{- end}} + } + + for name, function := range abiFunctionMap { + method, ok := {{$contract.Type}}ABI.Methods[name] + if !ok { + return nil, fmt.Errorf("given method (%s) does not exist in the ABI", name) + } + functions = append(functions, newStatefulPrecompileFunction(method.ID, function)) } - functions = append(functions, newStatefulPrecompileFunction(method{{.Normalized.Name}}.ID, {{decapitalise .Normalized.Name}})) - {{end}} {{- if .Contract.Fallback}} // Construct the contract with the fallback function. - contract := newStatefulPrecompileWithFunctionSelectors({{decapitalise $contract.Type}}Fallback, functions) + return NewStatefulPrecompileContract({{decapitalise $contract.Type}}Fallback, functions) {{- else}} // Construct the contract with no fallback function. - contract := newStatefulPrecompileWithFunctionSelectors(nil, functions) + return NewStatefulPrecompileContract(nil, functions) {{- end}} - return contract } ` diff --git a/core/genesis.go b/core/genesis.go index eb88bc82f4..1ab970ad2e 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -307,7 +307,10 @@ func (g *Genesis) ToBlock(db ethdb.Database) *types.Block { } // Configure any stateful precompiles that should be enabled in the genesis. - g.Config.CheckConfigurePrecompiles(nil, types.NewBlockWithHeader(head), statedb) + err = g.Config.ConfigurePrecompiles(nil, types.NewBlockWithHeader(head), statedb) + if err != nil { + panic(fmt.Sprintf("unable to configure precompiles in genesis block: %v", err)) + } // Do custom allocation after airdrop in case an address shows up in standard // allocation diff --git a/core/state_processor.go b/core/state_processor.go index 2b28f8b082..6ae2bddbbf 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -78,7 +78,11 @@ func (p *StateProcessor) Process(block *types.Block, parent *types.Header, state ) // Configure any stateful precompiles that should go into effect during this block. - p.config.CheckConfigurePrecompiles(new(big.Int).SetUint64(parent.Time), block, statedb) + err := p.config.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time), block, statedb) + // ASK: Should we panic instead? + if err != nil { + return nil, nil, 0, fmt.Errorf("could not configure precompiles: %w", err) + } blockContext := NewEVMBlockContext(header, p.bc, nil) vmenv := vm.NewEVM(blockContext, vm.TxContext{}, statedb, p.config, cfg) diff --git a/miner/worker.go b/miner/worker.go index 050cf663be..976940a243 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -184,7 +184,10 @@ func (w *worker) commitNewWork() (*types.Block, error) { return nil, fmt.Errorf("failed to create new current environment: %w", err) } // Configure any stateful precompiles that should go into effect during this block. - w.chainConfig.CheckConfigurePrecompiles(new(big.Int).SetUint64(parent.Time()), types.NewBlockWithHeader(header), env.state) + err = w.chainConfig.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time()), types.NewBlockWithHeader(header), env.state) + if err != nil { + return nil, fmt.Errorf("failed to configure precompiles: %w", err) + } // Fill the block with all available pending transactions. pending := w.eth.TxPool().Pending(true) diff --git a/params/precompile_config.go b/params/precompile_config.go index daa457b03b..02788afd69 100644 --- a/params/precompile_config.go +++ b/params/precompile_config.go @@ -346,13 +346,13 @@ func (c *ChainConfig) EnabledStatefulPrecompiles(blockTimestamp *big.Int) []prec return statefulPrecompileConfigs } -// CheckConfigurePrecompiles checks if any of the precompiles specified by the chain config are enabled or disabled by the block +// ConfigurePrecompiles checks if any of the precompiles specified by the chain config are enabled or disabled by the block // transition from [parentTimestamp] to the timestamp set in [blockContext]. If this is the case, it calls [Configure] // or [Deconfigure] to apply the necessary state transitions for the upgrade. // This function is called: // - within genesis setup to configure the starting state for precompiles enabled at genesis, // - during block processing to update the state before processing the given block. -func (c *ChainConfig) CheckConfigurePrecompiles(parentTimestamp *big.Int, blockContext precompile.BlockContext, statedb precompile.StateDB) { +func (c *ChainConfig) ConfigurePrecompiles(parentTimestamp *big.Int, blockContext precompile.BlockContext, statedb precompile.StateDB) error { blockTimestamp := blockContext.Timestamp() for _, key := range precompileKeys { // Note: configure precompiles in a deterministic order. for _, config := range c.getActivatingPrecompileConfigs(parentTimestamp, blockTimestamp, key, c.PrecompileUpgrades) { @@ -368,8 +368,11 @@ func (c *ChainConfig) CheckConfigurePrecompiles(parentTimestamp *big.Int, blockC statedb.Finalise(true) } else { log.Info("Activating new precompile", "name", key, "config", config) - precompile.Configure(c, blockContext, config, statedb) + if err := precompile.Configure(c, blockContext, config, statedb); err != nil { + return err + } } } } + return nil } diff --git a/precompile/allow_list.go b/precompile/allow_list.go index e411b5eba4..c580106db8 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -46,13 +46,14 @@ 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) { +func (c *AllowListConfig) Configure(state StateDB, precompileAddr common.Address) error { for _, enabledAddr := range c.EnabledAddresses { setAllowListRole(state, precompileAddr, enabledAddr, AllowListEnabled) } for _, adminAddr := range c.AllowListAdmins { setAllowListRole(state, precompileAddr, adminAddr, AllowListAdmin) } + return nil } // Equal returns true iff [other] has the same admins in the same order in its allow list. @@ -219,7 +220,12 @@ func createReadAllowList(precompileAddr common.Address) RunStatefulPrecompileFun func createAllowListPrecompile(precompileAddr common.Address) StatefulPrecompiledContract { // Construct the contract with no fallback function. allowListFuncs := createAllowListFunctions(precompileAddr) - contract := newStatefulPrecompileWithFunctionSelectors(nil, allowListFuncs) + contract, err := NewStatefulPrecompileContract(nil, allowListFuncs) + // Change this to be returned as an error after refactoring this precompile + // to use the new precompile template. + if err != nil { + panic(err) + } return contract } diff --git a/precompile/contract.go b/precompile/contract.go index 0e2d720273..9dadb02ab9 100644 --- a/precompile/contract.go +++ b/precompile/contract.go @@ -98,9 +98,9 @@ type statefulPrecompileWithFunctionSelectors struct { functions map[string]*statefulPrecompileFunction } -// newStatefulPrecompileWithFunctionSelectors generates new StatefulPrecompile using [functions] as the available functions and [fallback] +// NewStatefulPrecompileContract generates new StatefulPrecompile using [functions] as the available functions and [fallback] // as an optional fallback if there is no input data. Note: the selector of [fallback] will be ignored, so it is required to be left empty. -func newStatefulPrecompileWithFunctionSelectors(fallback RunStatefulPrecompileFunc, functions []*statefulPrecompileFunction) StatefulPrecompiledContract { +func NewStatefulPrecompileContract(fallback RunStatefulPrecompileFunc, functions []*statefulPrecompileFunction) (StatefulPrecompiledContract, error) { // Construct the contract and populate [functions]. contract := &statefulPrecompileWithFunctionSelectors{ fallback: fallback, @@ -109,12 +109,15 @@ func newStatefulPrecompileWithFunctionSelectors(fallback RunStatefulPrecompileFu for _, function := range functions { _, exists := contract.functions[string(function.selector)] if exists { - panic(fmt.Errorf("cannot create stateful precompile with duplicated function selector: %q", function.selector)) + return nil, fmt.Errorf("cannot create stateful precompile with duplicated function selector: %q", function.selector) + } + if function == nil { + return nil, fmt.Errorf("cannot create stateful precompile with nil function, selector: %q", function.selector) } contract.functions[string(function.selector)] = function } - return contract + return contract, nil } // Run selects the function using the 4 byte function selector at the start of the input and executes the underlying function on the diff --git a/precompile/contract_deployer_allow_list.go b/precompile/contract_deployer_allow_list.go index 7d42a270b9..11df6ff3fe 100644 --- a/precompile/contract_deployer_allow_list.go +++ b/precompile/contract_deployer_allow_list.go @@ -52,8 +52,8 @@ func (c *ContractDeployerAllowListConfig) Address() common.Address { } // Configure configures [state] with the desired admins based on [c]. -func (c *ContractDeployerAllowListConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) { - c.AllowListConfig.Configure(state, ContractDeployerAllowListAddress) +func (c *ContractDeployerAllowListConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) error { + return c.AllowListConfig.Configure(state, ContractDeployerAllowListAddress) } // Contract returns the singleton stateful precompiled contract to be used for the allow list. diff --git a/precompile/contract_native_minter.go b/precompile/contract_native_minter.go index 2da1b3bbaa..26ccaf8e68 100644 --- a/precompile/contract_native_minter.go +++ b/precompile/contract_native_minter.go @@ -71,7 +71,7 @@ func (c *ContractNativeMinterConfig) Address() common.Address { } // Configure configures [state] with the desired admins based on [c]. -func (c *ContractNativeMinterConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) { +func (c *ContractNativeMinterConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) error { for to, amount := range c.InitialMint { if amount != nil { bigIntAmount := (*big.Int)(amount) @@ -79,7 +79,7 @@ func (c *ContractNativeMinterConfig) Configure(_ ChainConfig, state StateDB, _ B } } - c.AllowListConfig.Configure(state, ContractNativeMinterAddress) + return c.AllowListConfig.Configure(state, ContractNativeMinterAddress) } // Contract returns the singleton stateful precompiled contract to be used for the native minter. @@ -157,12 +157,12 @@ func SetContractNativeMinterStatus(stateDB StateDB, address common.Address, role func PackMintInput(address common.Address, amount *big.Int) ([]byte, error) { // function selector (4 bytes) + input(hash for address + hash for amount) res := make([]byte, selectorLen+mintInputLen) - packOrderedHashesWithSelector(res, mintSignature, []common.Hash{ + err := packOrderedHashesWithSelector(res, mintSignature, []common.Hash{ address.Hash(), common.BigToHash(amount), }) - return res, nil + return res, err } // UnpackMintInput attempts to unpack [input] into the arguments to the mint precompile @@ -217,6 +217,11 @@ func createNativeMinterPrecompile(precompileAddr common.Address) StatefulPrecomp enabledFuncs = append(enabledFuncs, mintFunc) // Construct the contract with no fallback function. - contract := newStatefulPrecompileWithFunctionSelectors(nil, enabledFuncs) + contract, err := NewStatefulPrecompileContract(nil, enabledFuncs) + // Change this to be returned as an error after refactoring this precompile + // to use the new precompile template. + if err != nil { + panic(err) + } return contract } diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 3436ba360c..97bdbfa327 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -109,20 +109,20 @@ 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) { +func (c *FeeConfigManagerConfig) Configure(chainConfig ChainConfig, state StateDB, blockContext BlockContext) error { // 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)) + return fmt.Errorf("invalid feeConfig provided: %w", 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)) + return fmt.Errorf("invalid feeConfig provided in chainConfig: %w", err) } } - c.AllowListConfig.Configure(state, FeeConfigManagerAddress) + return c.AllowListConfig.Configure(state, FeeConfigManagerAddress) } // Contract returns the singleton stateful precompiled contract to be used for the fee manager. @@ -171,16 +171,16 @@ func PackGetLastChangedAtInput() []byte { // PackFeeConfig packs [feeConfig] without the selector into the appropriate arguments for fee config operations. func PackFeeConfig(feeConfig commontype.FeeConfig) ([]byte, error) { // input(feeConfig) - return packFeeConfigHelper(feeConfig, false), nil + return packFeeConfigHelper(feeConfig, false) } // PackSetFeeConfig packs [feeConfig] with the selector into the appropriate arguments for setting fee config operations. func PackSetFeeConfig(feeConfig commontype.FeeConfig) ([]byte, error) { // function selector (4 bytes) + input(feeConfig) - return packFeeConfigHelper(feeConfig, true), nil + return packFeeConfigHelper(feeConfig, true) } -func packFeeConfigHelper(feeConfig commontype.FeeConfig, useSelector bool) []byte { +func packFeeConfigHelper(feeConfig commontype.FeeConfig, useSelector bool) ([]byte, error) { hashes := []common.Hash{ common.BigToHash(feeConfig.GasLimit), common.BigToHash(new(big.Int).SetUint64(feeConfig.TargetBlockRate)), @@ -194,13 +194,13 @@ func packFeeConfigHelper(feeConfig commontype.FeeConfig, useSelector bool) []byt if useSelector { res := make([]byte, len(setFeeConfigSignature)+feeConfigInputLen) - packOrderedHashesWithSelector(res, setFeeConfigSignature, hashes) - return res + err := packOrderedHashesWithSelector(res, setFeeConfigSignature, hashes) + return res, err } res := make([]byte, len(hashes)*common.HashLength) - packOrderedHashes(res, hashes) - return res + err := packOrderedHashes(res, hashes) + return res, err } // UnpackFeeConfigInput attempts to unpack [input] into the arguments to the fee config precompile @@ -231,6 +231,7 @@ func UnpackFeeConfigInput(input []byte) (commontype.FeeConfig, error) { case blockGasCostStepKey: feeConfig.BlockGasCostStep = new(big.Int).SetBytes(packedElement) default: + // this should not ever happen. keep this as panic. panic(fmt.Sprintf("unknown fee config key: %d", i)) } } @@ -260,6 +261,7 @@ func GetStoredFeeConfig(stateDB StateDB) commontype.FeeConfig { case blockGasCostStepKey: feeConfig.BlockGasCostStep = new(big.Int).Set(val.Big()) default: + // this should not ever happen. keep this as panic. panic(fmt.Sprintf("unknown fee config key: %d", i)) } } @@ -298,6 +300,7 @@ func StoreFeeConfig(stateDB StateDB, feeConfig commontype.FeeConfig, blockContex case blockGasCostStepKey: input = common.BigToHash(feeConfig.BlockGasCostStep) default: + // this should not ever happen. keep this as panic. panic(fmt.Sprintf("unknown fee config key: %d", i)) } stateDB.SetState(FeeConfigManagerAddress, common.Hash{byte(i)}, input) @@ -386,6 +389,11 @@ func createFeeConfigManagerPrecompile(precompileAddr common.Address) StatefulPre feeConfigManagerFunctions = append(feeConfigManagerFunctions, setFeeConfigFunc, getFeeConfigFunc, getFeeConfigLastChangedAtFunc) // Construct the contract with no fallback function. - contract := newStatefulPrecompileWithFunctionSelectors(nil, feeConfigManagerFunctions) + contract, err := NewStatefulPrecompileContract(nil, feeConfigManagerFunctions) + // Change this to be returned as an error after refactoring this precompile + // to use the new precompile template. + if err != nil { + panic(err) + } return contract } diff --git a/precompile/reward_manager.go b/precompile/reward_manager.go index f57b0f72a8..7262efa0b8 100644 --- a/precompile/reward_manager.go +++ b/precompile/reward_manager.go @@ -76,7 +76,7 @@ func (c *InitialRewardConfig) Equal(other *InitialRewardConfig) bool { return c.AllowFeeRecipients == other.AllowFeeRecipients && c.RewardAddress == other.RewardAddress } -func (i *InitialRewardConfig) Configure(state StateDB) { +func (i *InitialRewardConfig) Configure(state StateDB) error { // enable allow fee recipients if i.AllowFeeRecipients { EnableAllowFeeRecipients(state) @@ -86,10 +86,9 @@ func (i *InitialRewardConfig) Configure(state StateDB) { DisableFeeRewards(state) } else { // set reward address - if err := StoreRewardAddress(state, i.RewardAddress); err != nil { - panic(err) - } + return StoreRewardAddress(state, i.RewardAddress) } + return nil } // RewardManagerConfig implements the StatefulPrecompileConfig @@ -106,7 +105,10 @@ func init() { panic(err) } RewardManagerABI = parsed - RewardManagerPrecompile = createRewardManagerPrecompile(RewardManagerAddress) + RewardManagerPrecompile, err = createRewardManagerPrecompile(RewardManagerAddress) + if err != nil { + panic(err) + } } // NewRewardManagerConfig returns a config for a network upgrade at [blockTimestamp] that enables @@ -161,11 +163,11 @@ func (c *RewardManagerConfig) Address() common.Address { } // Configure configures [state] with the initial configuration. -func (c *RewardManagerConfig) Configure(chainConfig ChainConfig, state StateDB, _ BlockContext) { +func (c *RewardManagerConfig) Configure(chainConfig ChainConfig, state StateDB, _ BlockContext) error { c.AllowListConfig.Configure(state, RewardManagerAddress) // configure the RewardManager with the given initial configuration if c.InitialRewardConfig != nil { - c.InitialRewardConfig.Configure(state) + return c.InitialRewardConfig.Configure(state) } else if chainConfig.AllowedFeeRecipients() { // configure the RewardManager according to chainConfig EnableAllowFeeRecipients(state) @@ -175,6 +177,7 @@ func (c *RewardManagerConfig) Configure(chainConfig ChainConfig, state StateDB, // default to disabling rewards DisableFeeRewards(state) } + return nil } // Contract returns the singleton stateful precompiled contract to be used for RewardManager. @@ -419,41 +422,25 @@ func disableRewards(accessibleState PrecompileAccessibleState, caller common.Add // createRewardManagerPrecompile returns a StatefulPrecompiledContract with getters and setters for the precompile. // Access to the getters/setters is controlled by an allow list for [precompileAddr]. -func createRewardManagerPrecompile(precompileAddr common.Address) StatefulPrecompiledContract { +func createRewardManagerPrecompile(precompileAddr common.Address) (StatefulPrecompiledContract, error) { var functions []*statefulPrecompileFunction functions = append(functions, createAllowListFunctions(precompileAddr)...) - - methodAllowFeeRecipients, ok := RewardManagerABI.Methods["allowFeeRecipients"] - if !ok { - panic("given method does not exist in the ABI") - } - functions = append(functions, newStatefulPrecompileFunction(methodAllowFeeRecipients.ID, allowFeeRecipients)) - - methodAreFeeRecipientsAllowed, ok := RewardManagerABI.Methods["areFeeRecipientsAllowed"] - if !ok { - panic("given method does not exist in the ABI") - } - functions = append(functions, newStatefulPrecompileFunction(methodAreFeeRecipientsAllowed.ID, areFeeRecipientsAllowed)) - - methodCurrentRewardAddress, ok := RewardManagerABI.Methods["currentRewardAddress"] - if !ok { - panic("given method does not exist in the ABI") - } - functions = append(functions, newStatefulPrecompileFunction(methodCurrentRewardAddress.ID, currentRewardAddress)) - - methodDisableRewards, ok := RewardManagerABI.Methods["disableRewards"] - if !ok { - panic("given method does not exist in the ABI") - } - functions = append(functions, newStatefulPrecompileFunction(methodDisableRewards.ID, disableRewards)) - - methodSetRewardAddress, ok := RewardManagerABI.Methods["setRewardAddress"] - if !ok { - panic("given method does not exist in the ABI") + abiFunctionMap := map[string]RunStatefulPrecompileFunc{ + "allowFeeRecipients": allowFeeRecipients, + "areFeeRecipientsAllowed": areFeeRecipientsAllowed, + "currentRewardAddress": currentRewardAddress, + "disableRewards": disableRewards, + "setRewardAddress": setRewardAddress, + } + + for name, function := range abiFunctionMap { + method, ok := RewardManagerABI.Methods[name] + if !ok { + return nil, fmt.Errorf("given method (%s) does not exist in the ABI", name) + } + functions = append(functions, newStatefulPrecompileFunction(method.ID, function)) } - functions = append(functions, newStatefulPrecompileFunction(methodSetRewardAddress.ID, setRewardAddress)) // Construct the contract with no fallback function. - contract := newStatefulPrecompileWithFunctionSelectors(nil, functions) - return contract + return NewStatefulPrecompileContract(nil, functions) } diff --git a/precompile/stateful_precompile_config.go b/precompile/stateful_precompile_config.go index 7db30060e4..6955b8d448 100644 --- a/precompile/stateful_precompile_config.go +++ b/precompile/stateful_precompile_config.go @@ -32,7 +32,7 @@ type StatefulPrecompileConfig interface { // Configure is called on the first block where the stateful precompile should be enabled. This // provides the config the ability to set its initial state and should only modify the state within // its own address space. - Configure(ChainConfig, StateDB, BlockContext) + Configure(ChainConfig, StateDB, BlockContext) error // Contract returns a thread-safe singleton that can be used as the StatefulPrecompiledContract when // this config is enabled. Contract() StatefulPrecompiledContract @@ -45,7 +45,7 @@ type StatefulPrecompileConfig interface { // Configure sets the nonce and code to non-empty values then calls Configure on [precompileConfig] to make the necessary // state update to enable the StatefulPrecompile. // Assumes that [precompileConfig] is non-nil. -func Configure(chainConfig ChainConfig, blockContext BlockContext, precompileConfig StatefulPrecompileConfig, state StateDB) { +func Configure(chainConfig ChainConfig, blockContext BlockContext, precompileConfig StatefulPrecompileConfig, state StateDB) error { // Set the nonce of the precompile's address (as is done when a contract is created) to ensure // that it is marked as non-empty and will not be cleaned up when the statedb is finalized. state.SetNonce(precompileConfig.Address(), 1) @@ -53,5 +53,5 @@ func Configure(chainConfig ChainConfig, blockContext BlockContext, precompileCon // can be called from within Solidity contracts. Solidity adds a check before invoking a contract to ensure // that it does not attempt to invoke a non-existent contract. state.SetCode(precompileConfig.Address(), []byte{0x1}) - precompileConfig.Configure(chainConfig, state, blockContext) + return precompileConfig.Configure(chainConfig, state, blockContext) } diff --git a/precompile/tx_allow_list.go b/precompile/tx_allow_list.go index c67e07011f..4ab39a8f04 100644 --- a/precompile/tx_allow_list.go +++ b/precompile/tx_allow_list.go @@ -55,8 +55,8 @@ func (c *TxAllowListConfig) Address() common.Address { } // Configure configures [state] with the desired admins based on [c]. -func (c *TxAllowListConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) { - c.AllowListConfig.Configure(state, TxAllowListAddress) +func (c *TxAllowListConfig) Configure(_ ChainConfig, state StateDB, _ BlockContext) error { + return c.AllowListConfig.Configure(state, TxAllowListAddress) } // Contract returns the singleton stateful precompiled contract to be used for the allow list. diff --git a/precompile/utils.go b/precompile/utils.go index 2476a97e34..6d779c251e 100644 --- a/precompile/utils.go +++ b/precompile/utils.go @@ -17,6 +17,7 @@ var functionSignatureRegex = regexp.MustCompile(`[\w]+\(((([\w]+)?)|((([\w]+),)+ // CalculateFunctionSelector returns the 4 byte function selector that results from [functionSignature] // Ex. the function setBalance(addr address, balance uint256) should be passed in as the string: // "setBalance(address,uint256)" +// TODO: remove this after moving to ABI based function selectors. func CalculateFunctionSelector(functionSignature string) []byte { if !functionSignatureRegex.MatchString(functionSignature) { panic(fmt.Errorf("invalid function signature: %q", functionSignature)) @@ -36,16 +37,16 @@ func deductGas(suppliedGas uint64, requiredGas uint64) (uint64, error) { // packOrderedHashesWithSelector packs the function selector and ordered list of hashes into [dst] // byte slice. // assumes that [dst] has sufficient room for [functionSelector] and [hashes]. -func packOrderedHashesWithSelector(dst []byte, functionSelector []byte, hashes []common.Hash) { +func packOrderedHashesWithSelector(dst []byte, functionSelector []byte, hashes []common.Hash) error { copy(dst[:len(functionSelector)], functionSelector) - packOrderedHashes(dst[len(functionSelector):], hashes) + return packOrderedHashes(dst[len(functionSelector):], hashes) } // packOrderedHashes packs the ordered list of [hashes] into the [dst] byte buffer. // assumes that [dst] has sufficient space to pack [hashes] or else this function will panic. -func packOrderedHashes(dst []byte, hashes []common.Hash) { +func packOrderedHashes(dst []byte, hashes []common.Hash) error { if len(dst) != len(hashes)*common.HashLength { - panic(fmt.Sprintf("destination byte buffer has insufficient length (%d) for %d hashes", len(dst), len(hashes))) + return fmt.Errorf("destination byte buffer has insufficient length (%d) for %d hashes", len(dst), len(hashes)) } var ( @@ -57,6 +58,7 @@ func packOrderedHashes(dst []byte, hashes []common.Hash) { start += common.HashLength end += common.HashLength } + return nil } // returnPackedHash returns packed the byte slice with common.HashLength from [packed] From 4d011d9a0d49b4fa7310be7666677e9ce2b54e88 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 21 Dec 2022 18:41:54 +0300 Subject: [PATCH 02/10] 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 c580106db8..b4f75becdb 100644 --- a/precompile/allow_list.go +++ b/precompile/allow_list.go @@ -221,7 +221,7 @@ func createAllowListPrecompile(precompileAddr common.Address) StatefulPrecompile // Construct the contract with no fallback function. allowListFuncs := createAllowListFunctions(precompileAddr) contract, err := NewStatefulPrecompileContract(nil, allowListFuncs) - // Change this to be returned as an error after refactoring this precompile + // TODO Change this to be returned as an error after refactoring this precompile // to use the new precompile template. if err != nil { panic(err) From 2f90260717dd72aca5564fffb132b718bb1c5936 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 21 Dec 2022 18:42:06 +0300 Subject: [PATCH 03/10] Update precompile/fee_config_manager.go Co-authored-by: aaronbuchwald --- precompile/fee_config_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 97bdbfa327..4059f1f590 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -231,7 +231,7 @@ func UnpackFeeConfigInput(input []byte) (commontype.FeeConfig, error) { case blockGasCostStepKey: feeConfig.BlockGasCostStep = new(big.Int).SetBytes(packedElement) default: - // this should not ever happen. keep this as panic. + // This should never encounter an unknown fee config key panic(fmt.Sprintf("unknown fee config key: %d", i)) } } From 5991c6ea4b3b730f9a0b1066cb377877195b7c33 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 21 Dec 2022 18:49:27 +0300 Subject: [PATCH 04/10] Update precompile/fee_config_manager.go Co-authored-by: aaronbuchwald --- precompile/fee_config_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 4059f1f590..1affa533cb 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -390,7 +390,7 @@ func createFeeConfigManagerPrecompile(precompileAddr common.Address) StatefulPre feeConfigManagerFunctions = append(feeConfigManagerFunctions, setFeeConfigFunc, getFeeConfigFunc, getFeeConfigLastChangedAtFunc) // Construct the contract with no fallback function. contract, err := NewStatefulPrecompileContract(nil, feeConfigManagerFunctions) - // Change this to be returned as an error after refactoring this precompile + // TODO Change this to be returned as an error after refactoring this precompile // to use the new precompile template. if err != nil { panic(err) From 8f83fe584746a1568a92ead116523cec4d37ab35 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 22 Dec 2022 12:50:00 +0300 Subject: [PATCH 05/10] fix reviews --- core/state_processor.go | 6 ++++-- miner/worker.go | 4 +++- precompile/contract.go | 3 --- precompile/fee_config_manager.go | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/core/state_processor.go b/core/state_processor.go index 6ae2bddbbf..39c99461b3 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -37,6 +37,7 @@ import ( "github.com/ava-labs/subnet-evm/params" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/log" ) // StateProcessor is a basic Processor, which takes care of transitioning @@ -79,9 +80,10 @@ func (p *StateProcessor) Process(block *types.Block, parent *types.Header, state // Configure any stateful precompiles that should go into effect during this block. err := p.config.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time), block, statedb) - // ASK: Should we panic instead? if err != nil { - return nil, nil, 0, fmt.Errorf("could not configure precompiles: %w", err) + werr := fmt.Errorf("could not configure precompiles: %w", err) + log.Error("failed to process the state changes", "err", werr) + return nil, nil, 0, err } blockContext := NewEVMBlockContext(header, p.bc, nil) diff --git a/miner/worker.go b/miner/worker.go index 976940a243..eba402601a 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -186,7 +186,9 @@ func (w *worker) commitNewWork() (*types.Block, error) { // Configure any stateful precompiles that should go into effect during this block. err = w.chainConfig.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time()), types.NewBlockWithHeader(header), env.state) if err != nil { - return nil, fmt.Errorf("failed to configure precompiles: %w", err) + werr := fmt.Errorf("failed to configure precompiles: %w", err) + log.Error("failed to commit new work", "err", werr) + return nil, werr } // Fill the block with all available pending transactions. diff --git a/precompile/contract.go b/precompile/contract.go index 9dadb02ab9..69ed968641 100644 --- a/precompile/contract.go +++ b/precompile/contract.go @@ -111,9 +111,6 @@ func NewStatefulPrecompileContract(fallback RunStatefulPrecompileFunc, functions if exists { return nil, fmt.Errorf("cannot create stateful precompile with duplicated function selector: %q", function.selector) } - if function == nil { - return nil, fmt.Errorf("cannot create stateful precompile with nil function, selector: %q", function.selector) - } contract.functions[string(function.selector)] = function } diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 1affa533cb..de23cbde7b 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -261,7 +261,7 @@ func GetStoredFeeConfig(stateDB StateDB) commontype.FeeConfig { case blockGasCostStepKey: feeConfig.BlockGasCostStep = new(big.Int).Set(val.Big()) default: - // this should not ever happen. keep this as panic. + // This should never encounter an unknown fee config key panic(fmt.Sprintf("unknown fee config key: %d", i)) } } @@ -300,7 +300,7 @@ func StoreFeeConfig(stateDB StateDB, feeConfig commontype.FeeConfig, blockContex case blockGasCostStepKey: input = common.BigToHash(feeConfig.BlockGasCostStep) default: - // this should not ever happen. keep this as panic. + // this should not ever happen. panic(fmt.Sprintf("unknown fee config key: %d", i)) } stateDB.SetState(FeeConfigManagerAddress, common.Hash{byte(i)}, input) From ccc78106aea0d0c511ddd624d0c9707675ecde88 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 23 Dec 2022 16:02:17 +0300 Subject: [PATCH 06/10] wrap errors in ConfigurePrecompiles --- core/state_processor.go | 3 +-- miner/worker.go | 5 ++--- params/precompile_config.go | 2 +- precompile/fee_config_manager.go | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/state_processor.go b/core/state_processor.go index 39c99461b3..6f6454fd0b 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -81,8 +81,7 @@ func (p *StateProcessor) Process(block *types.Block, parent *types.Header, state // Configure any stateful precompiles that should go into effect during this block. err := p.config.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time), block, statedb) if err != nil { - werr := fmt.Errorf("could not configure precompiles: %w", err) - log.Error("failed to process the state changes", "err", werr) + log.Error("failed to process the state changes", "err", err) return nil, nil, 0, err } diff --git a/miner/worker.go b/miner/worker.go index eba402601a..7eb6c8ae69 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -186,9 +186,8 @@ func (w *worker) commitNewWork() (*types.Block, error) { // Configure any stateful precompiles that should go into effect during this block. err = w.chainConfig.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time()), types.NewBlockWithHeader(header), env.state) if err != nil { - werr := fmt.Errorf("failed to configure precompiles: %w", err) - log.Error("failed to commit new work", "err", werr) - return nil, werr + log.Error("failed to commit new work", "err", err) + return nil, err } // Fill the block with all available pending transactions. diff --git a/params/precompile_config.go b/params/precompile_config.go index f58ac8fa84..cd629ced55 100644 --- a/params/precompile_config.go +++ b/params/precompile_config.go @@ -286,7 +286,7 @@ func (c *ChainConfig) ConfigurePrecompiles(parentTimestamp *big.Int, blockContex } else { log.Info("Activating new precompile", "precompileAddress", address, "config", config) if err := precompile.Configure(c, blockContext, config, statedb); err != nil { - return err + return fmt.Errorf("could not configure precompile, precompileAddress: %s, reason: %w", address, err) } } } diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index de23cbde7b..471e7ca68a 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -300,7 +300,7 @@ func StoreFeeConfig(stateDB StateDB, feeConfig commontype.FeeConfig, blockContex case blockGasCostStepKey: input = common.BigToHash(feeConfig.BlockGasCostStep) default: - // this should not ever happen. + // This should never encounter an unknown fee config key panic(fmt.Sprintf("unknown fee config key: %d", i)) } stateDB.SetState(FeeConfigManagerAddress, common.Hash{byte(i)}, input) From 93296c659fd768aafc9dadec809b40d2f35de8cd Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 23 Dec 2022 16:06:45 +0300 Subject: [PATCH 07/10] cleaner errors --- precompile/fee_config_manager.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/precompile/fee_config_manager.go b/precompile/fee_config_manager.go index 471e7ca68a..56567d840a 100644 --- a/precompile/fee_config_manager.go +++ b/precompile/fee_config_manager.go @@ -114,12 +114,12 @@ func (c *FeeConfigManagerConfig) Configure(chainConfig ChainConfig, state StateD 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() - return fmt.Errorf("invalid feeConfig provided: %w", err) + return fmt.Errorf("cannot configure given initial fee config: %w", 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. - return fmt.Errorf("invalid feeConfig provided in chainConfig: %w", err) + return fmt.Errorf("cannot configure fee config in chain config: %w", err) } } return c.AllowListConfig.Configure(state, FeeConfigManagerAddress) @@ -277,7 +277,7 @@ func GetFeeConfigLastChangedAt(stateDB StateDB) *big.Int { // A validation on [feeConfig] is done before storing. func StoreFeeConfig(stateDB StateDB, feeConfig commontype.FeeConfig, blockContext BlockContext) error { if err := feeConfig.Verify(); err != nil { - return err + return fmt.Errorf("cannot verify fee config: %w", err) } for i := minFeeConfigFieldKey; i <= numFeeConfigField; i++ { From 2d4794f263cec8fbac7d5542cba5e84b68200e55 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 27 Dec 2022 20:26:16 +0300 Subject: [PATCH 08/10] Update utils.go --- precompile/utils.go | 1 - 1 file changed, 1 deletion(-) diff --git a/precompile/utils.go b/precompile/utils.go index 6d779c251e..6da328f9c8 100644 --- a/precompile/utils.go +++ b/precompile/utils.go @@ -17,7 +17,6 @@ var functionSignatureRegex = regexp.MustCompile(`[\w]+\(((([\w]+)?)|((([\w]+),)+ // CalculateFunctionSelector returns the 4 byte function selector that results from [functionSignature] // Ex. the function setBalance(addr address, balance uint256) should be passed in as the string: // "setBalance(address,uint256)" -// TODO: remove this after moving to ABI based function selectors. func CalculateFunctionSelector(functionSignature string) []byte { if !functionSignatureRegex.MatchString(functionSignature) { panic(fmt.Errorf("invalid function signature: %q", functionSignature)) From 5b7940c12e76a4e861fcf4bceb085ac74c557387 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 27 Dec 2022 20:26:44 +0300 Subject: [PATCH 09/10] Update miner/worker.go Co-authored-by: aaronbuchwald --- miner/worker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miner/worker.go b/miner/worker.go index 7eb6c8ae69..c0715c0ba2 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -186,7 +186,7 @@ func (w *worker) commitNewWork() (*types.Block, error) { // Configure any stateful precompiles that should go into effect during this block. err = w.chainConfig.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time()), types.NewBlockWithHeader(header), env.state) if err != nil { - log.Error("failed to commit new work", "err", err) + log.Error("failed to configure precompiles mining new block", "parent", parent.Hash(), "number", header.Number, "timestamp", header.Time, "err", err) return nil, err } From 679d4e03313a6de58feb319a9a67bdc1723b5a71 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 27 Dec 2022 20:26:51 +0300 Subject: [PATCH 10/10] Update core/state_processor.go Co-authored-by: aaronbuchwald --- core/state_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state_processor.go b/core/state_processor.go index 6f6454fd0b..58d7df6fd7 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -81,7 +81,7 @@ func (p *StateProcessor) Process(block *types.Block, parent *types.Header, state // Configure any stateful precompiles that should go into effect during this block. err := p.config.ConfigurePrecompiles(new(big.Int).SetUint64(parent.Time), block, statedb) if err != nil { - log.Error("failed to process the state changes", "err", err) + log.Error("failed to configure precompiles processing block", "hash", block.Hash(), "number", block.NumberU64(), "timestamp", block.Time(), "err", err) return nil, nil, 0, err }