From bacdccfa93b53ae8064cb7acbf444363f0ecfebe Mon Sep 17 00:00:00 2001 From: Gaston Ponti Date: Thu, 13 Jul 2023 10:58:13 -0300 Subject: [PATCH] Fix: RPC calls (gasLimit, gasPrice) use the intial state of the block (#2161) * Fix: RPC calls (gasLimit, gasPrice) use the intial state of the block * Fix tests --- e2e_test/e2e_test.go | 11 ++++--- e2e_test/e2e_transfer_test.go | 7 ++-- eth/api_backend.go | 47 +++++++++++++++++++------- les/api_backend.go | 62 +++++++++++++++++++++++++++-------- 4 files changed, 93 insertions(+), 34 deletions(-) diff --git a/e2e_test/e2e_test.go b/e2e_test/e2e_test.go index 8004cb3738..a290d83669 100644 --- a/e2e_test/e2e_test.go +++ b/e2e_test/e2e_test.go @@ -541,7 +541,8 @@ func TestRPCDynamicTxGasPriceWithState(t *testing.T) { require.NoError(t, err) // Prune state - err = pruneStateOfBlock(ctx, network[0], *json.BlockHash) + // As the gasPrice of the block N, is the one from the state of the block N-1, we need to prune the parent block + err = pruneStateOfBlock(ctx, network[0], new(big.Int).Sub(json.BlockNumber.ToInt(), common.Big1)) require.NoError(t, err) var json2 *rpcCustomTransaction @@ -619,9 +620,9 @@ func testRPCDynamicTxGasPriceWithoutState(t *testing.T, afterGingerbread, altern // Create one block to be able to prune the last state _, err = accounts[0].SendCeloTracked(ctx, accounts[1].Address, 1, network[0]) require.NoError(t, err) - // Prune state - err = pruneStateOfBlock(ctx, network[0], *json.BlockHash) + // As the gasPrice of the block N, is the one from the state of the block N-1, we need to prune the parent block + err = pruneStateOfBlock(ctx, network[0], new(big.Int).Sub(json.BlockNumber.ToInt(), common.Big1)) require.NoError(t, err) var json2 *rpcCustomTransaction @@ -640,9 +641,9 @@ func testRPCDynamicTxGasPriceWithoutState(t *testing.T, afterGingerbread, altern } } -func pruneStateOfBlock(ctx context.Context, node *test.Node, blockHash common.Hash) error { +func pruneStateOfBlock(ctx context.Context, node *test.Node, blockNumber *big.Int) error { var block *types.Block - block, err := node.WsClient.BlockByHash(ctx, blockHash) + block, err := node.WsClient.BlockByNumber(ctx, blockNumber) if err != nil { return err } diff --git a/e2e_test/e2e_transfer_test.go b/e2e_test/e2e_transfer_test.go index 4b2dcecb30..3a937e00e0 100644 --- a/e2e_test/e2e_transfer_test.go +++ b/e2e_test/e2e_transfer_test.go @@ -52,10 +52,9 @@ func TestTransferCELO(t *testing.T) { gateWayFeeRecipient := devAccounts[2] // Get datum to set GasPrice/MaxFeePerGas/MaxPriorityFeePerGas to sensible values - header, err := network[0].WsClient.HeaderByNumber(ctx, common.Big0) - require.NoError(t, err) - datum, err := network[0].Eth.APIBackend.GasPriceMinimumForHeader(ctx, nil, header) + header, err := network[0].WsClient.HeaderByNumber(ctx, common.Big1) require.NoError(t, err) + datum := header.BaseFee testCases := []struct { name string @@ -254,7 +253,7 @@ func TestTransferCELOPreGingerbread(t *testing.T) { gateWayFeeRecipient := devAccounts[2] // Get datum to set GasPrice/MaxFeePerGas/MaxPriorityFeePerGas to sensible values - header, err := network[0].WsClient.HeaderByNumber(ctx, common.Big0) + header, err := network[0].WsClient.HeaderByNumber(ctx, common.Big1) require.NoError(t, err) datum, err := network[0].Eth.APIBackend.GasPriceMinimumForHeader(ctx, nil, header) require.NoError(t, err) diff --git a/eth/api_backend.go b/eth/api_backend.go index 221ec5c1cc..a51d983da2 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -315,11 +315,14 @@ func (b *EthAPIBackend) GasPriceMinimumForHeader(ctx context.Context, currencyAd if header.BaseFee != nil && currencyAddress == nil { return header.BaseFee, nil } - state, err := b.eth.blockchain.StateAt(header.Root) + // The gasPriceMinimum (celo or alternative currency) of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) if err != nil { return nil, err } - vmRunner := b.eth.BlockChain().NewEVMRunner(header, state) + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) return gp.GetBaseFeeForCurrency(vmRunner, currencyAddress, header.BaseFee) } @@ -327,11 +330,14 @@ func (b *EthAPIBackend) RealGasPriceMinimumForHeader(ctx context.Context, curren if header.BaseFee != nil && currencyAddress == nil { return header.BaseFee, nil } - state, err := b.eth.blockchain.StateAt(header.Root) + // The gasPriceMinimum (celo or alternative currency) of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) if err != nil { return nil, err } - vmRunner := b.eth.BlockChain().NewEVMRunner(header, state) + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) return gp.GetRealBaseFeeForCurrency(vmRunner, currencyAddress, header.BaseFee) } @@ -344,24 +350,43 @@ func (b *EthAPIBackend) SuggestPrice(ctx context.Context, currencyAddress *commo } func (b *EthAPIBackend) GetBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) uint64 { - statedb, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + header, err := b.HeaderByNumberOrHash(ctx, blockNrOrHash) + if err != nil { + log.Warn("Cannot retrieve the header for blockGasLimit", "err", err) + return params.DefaultGasLimit + } + if header.GasLimit > 0 { + return header.GasLimit + } + // The gasLimit of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) if err != nil { log.Warn("Cannot create evmCaller to get blockGasLimit", "err", err) return params.DefaultGasLimit } - - vmRunner := b.eth.BlockChain().NewEVMRunner(header, statedb) + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) return blockchain_parameters.GetBlockGasLimitOrDefault(vmRunner) } func (b *EthAPIBackend) GetRealBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (uint64, error) { - statedb, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + header, err := b.HeaderByNumberOrHash(ctx, blockNrOrHash) + if err != nil { + return 0, fmt.Errorf("EthApiBackend failed to retrieve header for gas limit %v: %w", blockNrOrHash, err) + } + if header.GasLimit > 0 { + return header.GasLimit, nil + } + // The gasLimit of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) if err != nil { return 0, fmt.Errorf("EthApiBackend failed to retrieve state for block gas limit for block %v: %w", blockNrOrHash, err) } - - caller := b.eth.BlockChain().NewEVMRunner(header, statedb) - limit, err := blockchain_parameters.GetBlockGasLimit(caller) + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) + limit, err := blockchain_parameters.GetBlockGasLimit(vmRunner) if err != nil { return 0, fmt.Errorf("EthApiBackend failed to retrieve block gas limit from blockchain parameters constract for block %v: %w", blockNrOrHash, err) } diff --git a/les/api_backend.go b/les/api_backend.go index d29643fa40..4dbeda23c2 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -286,24 +286,43 @@ func (b *LesApiBackend) GetIntrinsicGasForAlternativeFeeCurrency(ctx context.Con } func (b *LesApiBackend) GetBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) uint64 { - statedb, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + header, err := b.HeaderByNumberOrHash(ctx, blockNrOrHash) + if err != nil { + log.Warn("Cannot retrieve the header for blockGasLimit", "err", err) + return params.DefaultGasLimit + } + if header.GasLimit > 0 { + return header.GasLimit + } + // The gasLimit of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) if err != nil { log.Warn("Cannot create evmCaller to get blockGasLimit", "err", err) return params.DefaultGasLimit } - - caller := b.eth.BlockChain().NewEVMRunner(header, statedb) - return blockchain_parameters.GetBlockGasLimitOrDefault(caller) + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) + return blockchain_parameters.GetBlockGasLimitOrDefault(vmRunner) } func (b *LesApiBackend) GetRealBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (uint64, error) { - statedb, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + header, err := b.HeaderByNumberOrHash(ctx, blockNrOrHash) + if err != nil { + return 0, fmt.Errorf("LesApiBackend failed to retrieve header for gas limit %v: %w", blockNrOrHash, err) + } + if header.GasLimit > 0 { + return header.GasLimit, nil + } + // The gasLimit of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) if err != nil { return 0, fmt.Errorf("LesApiBackend failed to retrieve state for block gas limit for block %v: %w", blockNrOrHash, err) } - - caller := b.eth.BlockChain().NewEVMRunner(header, statedb) - limit, err := blockchain_parameters.GetBlockGasLimit(caller) + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) + limit, err := blockchain_parameters.GetBlockGasLimit(vmRunner) if err != nil { return 0, fmt.Errorf("LesApiBackend failed to retrieve block gas limit from blockchain parameters constract for block %v: %w", blockNrOrHash, err) } @@ -323,20 +342,29 @@ func (b *LesApiBackend) SuggestGasTipCap(ctx context.Context, currencyAddress *c } func (b *LesApiBackend) CurrentGasPriceMinimum(ctx context.Context, currencyAddress *common.Address) (*big.Int, error) { + header := b.CurrentHeader() + if header.BaseFee != nil && currencyAddress == nil { + return header.BaseFee, nil + } vmRunner, err := b.eth.BlockChain().NewEVMRunnerForCurrentBlock() if err != nil { return nil, err } - return gp.GetBaseFeeForCurrency(vmRunner, currencyAddress, b.CurrentHeader().BaseFee) + return gp.GetBaseFeeForCurrency(vmRunner, currencyAddress, header.BaseFee) } func (b *LesApiBackend) GasPriceMinimumForHeader(ctx context.Context, currencyAddress *common.Address, header *types.Header) (*big.Int, error) { if header.BaseFee != nil && currencyAddress == nil { return header.BaseFee, nil } - state := light.NewState(ctx, header, b.eth.odr) - vmRunner := b.eth.blockchain.NewEVMRunner(header, state) - + // The gasPriceMinimum (celo or alternative currency) of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) + if err != nil { + return nil, err + } + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) return gp.GetBaseFeeForCurrency(vmRunner, currencyAddress, header.BaseFee) } @@ -344,8 +372,14 @@ func (b *LesApiBackend) RealGasPriceMinimumForHeader(ctx context.Context, curren if header.BaseFee != nil && currencyAddress == nil { return header.BaseFee, nil } - state := light.NewState(ctx, header, b.eth.odr) - vmRunner := b.eth.blockchain.NewEVMRunner(header, state) + // The gasPriceMinimum (celo or alternative currency) of a specific block, is the one at the beginning of the block, + // not the end of it (the state_root of the header is the a state resulted of applying the block). So, the state to + // be used, MUST be the state result of the parent block + state, parent, err := b.StateAndHeaderByNumberOrHash(ctx, rpc.BlockNumberOrHash{BlockHash: &header.ParentHash}) + if err != nil { + return nil, err + } + vmRunner := b.eth.BlockChain().NewEVMRunner(parent, state) return gp.GetRealBaseFeeForCurrency(vmRunner, currencyAddress, header.BaseFee) }