Skip to content

Commit

Permalink
Fix: RPC calls (gasLimit, gasPrice) use the intial state of the block (…
Browse files Browse the repository at this point in the history 
…#2161)

* Fix: RPC calls (gasLimit, gasPrice) use the intial state of the block

* Fix tests
  • Loading branch information
@gastonponti
gastonponti authored Jul 13, 2023
1 parent b1bba65 commit bacdccf
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 34 deletions.
11 changes: 6 additions & 5 deletions e2e_test/e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
7 changes: 3 additions & 4 deletions e2e_test/e2e_transfer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
47 changes: 36 additions & 11 deletions eth/api_backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,23 +315,29 @@ 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)
}

func (b *EthAPIBackend) RealGasPriceMinimumForHeader(ctx context.Context, currencyAddress *common.Address, header *types.Header) (*big.Int, error) {
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)
}

Expand All @@ -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)
}
Expand Down
62 changes: 48 additions & 14 deletions les/api_backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -323,29 +342,44 @@ 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)
}

func (b *LesApiBackend) RealGasPriceMinimumForHeader(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.GetRealBaseFeeForCurrency(vmRunner, currencyAddress, header.BaseFee)
}

Expand Down

0 comments on commit bacdccf

Please sign in to comment.