Skip to content

Commit

Permalink
EVM-749 Transaction pool Base Fee fix (#1747)
Browse files Browse the repository at this point in the history
  • Loading branch information
igorcrevar authored Jul 27, 2023
1 parent b430680 commit 787cbe4
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 45 deletions.
6 changes: 3 additions & 3 deletions consensus/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@ type transitionInterface interface {
Write(txn *types.Transaction) error
}

func (d *Dev) writeTransactions(baseFee, gasLimit uint64, transition transitionInterface) []*types.Transaction {
func (d *Dev) writeTransactions(gasLimit uint64, transition transitionInterface) []*types.Transaction {
var successful []*types.Transaction

d.txpool.Prepare(baseFee)
d.txpool.Prepare()

for {
tx := d.txpool.Peek()
Expand Down Expand Up @@ -183,7 +183,7 @@ func (d *Dev) writeNewBlock(parent *types.Header) error {
return err
}

txns := d.writeTransactions(baseFee, gasLimit, transition)
txns := d.writeTransactions(gasLimit, transition)

// Commit the changes
_, root, err := transition.Commit()
Expand Down
6 changes: 1 addition & 5 deletions consensus/ibft/consensus_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,6 @@ func (i *backendIBFT) buildBlock(parent *types.Header) (*types.Block, error) {
}

// calculate base fee
baseFee := i.blockchain.CalculateBaseFee(parent)

header.GasLimit = gasLimit

if err := i.currentHooks.ModifyHeader(header, i.currentSigner.Address()); err != nil {
Expand Down Expand Up @@ -209,7 +207,6 @@ func (i *backendIBFT) buildBlock(parent *types.Header) (*types.Block, error) {

txs := i.writeTransactions(
writeCtx,
baseFee,
gasLimit,
header.Number,
transition,
Expand Down Expand Up @@ -297,7 +294,6 @@ type transitionInterface interface {

func (i *backendIBFT) writeTransactions(
writeCtx context.Context,
baseFee,
gasLimit,
blockNumber uint64,
transition transitionInterface,
Expand All @@ -324,7 +320,7 @@ func (i *backendIBFT) writeTransactions(
)
}()

i.txpool.Prepare(baseFee)
i.txpool.Prepare()

write:
for {
Expand Down
2 changes: 1 addition & 1 deletion consensus/ibft/ibft.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var (
)

type txPoolInterface interface {
Prepare(uint64)
Prepare()
Length() uint64
Peek() *types.Transaction
Pop(tx *types.Transaction)
Expand Down
2 changes: 1 addition & 1 deletion consensus/polybft/block_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (b *BlockBuilder) WriteTx(tx *types.Transaction) error {
func (b *BlockBuilder) Fill() {
blockTimer := time.NewTimer(b.params.BlockTime)

b.params.TxPool.Prepare(b.params.BaseFee)
b.params.TxPool.Prepare()
write:
for {
select {
Expand Down
2 changes: 1 addition & 1 deletion consensus/polybft/block_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestBlockBuilder_BuildBlockTxOneFailedTxAndOneTakesTooMuchGas(t *testing.T)
parentHeader := &types.Header{StateRoot: hash, GasLimit: 1_000_000_000_000_000}

txPool := &txPoolMock{}
txPool.On("Prepare", uint64(0)).Once()
txPool.On("Prepare").Once()

for i, acc := range accounts {
receiver := types.Address(acc.Ecdsa.Address())
Expand Down
2 changes: 1 addition & 1 deletion consensus/polybft/consensus_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var (

// txPoolInterface is an abstraction of transaction pool
type txPoolInterface interface {
Prepare(uint64)
Prepare()
Length() uint64
Peek() *types.Transaction
Pop(*types.Transaction)
Expand Down
4 changes: 2 additions & 2 deletions consensus/polybft/mocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ type txPoolMock struct {
mock.Mock
}

func (tp *txPoolMock) Prepare(baseFee uint64) {
tp.Called(baseFee)
func (tp *txPoolMock) Prepare() {
tp.Called()
}

func (tp *txPoolMock) Length() uint64 {
Expand Down
1 change: 1 addition & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ func NewServer(config *Config) (*Server, error) {
}
}

m.txpool.SetBaseFee(m.blockchain.Header())
m.txpool.Start()

return m, nil
Expand Down
26 changes: 24 additions & 2 deletions txpool/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@ var mockHeader = &types.Header{

type defaultMockStore struct {
DefaultHeader *types.Header

getBlockByHashFn func(types.Hash, bool) (*types.Block, bool)
calculateBaseFeeFn func(*types.Header) uint64
}

func NewDefaultMockStore(header *types.Header) defaultMockStore {
return defaultMockStore{
header,
DefaultHeader: header,
calculateBaseFeeFn: func(h *types.Header) uint64 {
return h.BaseFee
},
}
}

Expand All @@ -31,7 +37,11 @@ func (m defaultMockStore) GetNonce(types.Hash, types.Address) uint64 {
return 0
}

func (m defaultMockStore) GetBlockByHash(types.Hash, bool) (*types.Block, bool) {
func (m defaultMockStore) GetBlockByHash(hash types.Hash, full bool) (*types.Block, bool) {
if m.getBlockByHashFn != nil {
return m.getBlockByHashFn(hash, full)
}

return nil, false
}

Expand All @@ -41,6 +51,14 @@ func (m defaultMockStore) GetBalance(types.Hash, types.Address) (*big.Int, error
return balance, nil
}

func (m defaultMockStore) CalculateBaseFee(header *types.Header) uint64 {
if m.calculateBaseFeeFn != nil {
return m.calculateBaseFeeFn(header)
}

return 0
}

type faultyMockStore struct {
}

Expand All @@ -60,6 +78,10 @@ func (fms faultyMockStore) GetBalance(root types.Hash, addr types.Address) (*big
return nil, fmt.Errorf("unable to fetch account state")
}

func (fms faultyMockStore) CalculateBaseFee(*types.Header) uint64 {
return 0
}

type mockSigner struct {
}

Expand Down
9 changes: 6 additions & 3 deletions txpool/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,15 @@ func (p *TxPool) GetPendingTx(txHash types.Hash) (*types.Transaction, bool) {
func (p *TxPool) GetTxs(inclQueued bool) (
allPromoted, allEnqueued map[types.Address][]*types.Transaction,
) {
allPromoted, allEnqueued = p.accounts.allTxs(inclQueued)

return
return p.accounts.allTxs(inclQueued)
}

// GetBaseFee returns current base fee
func (p *TxPool) GetBaseFee() uint64 {
return atomic.LoadUint64(&p.baseFee)
}

// SetBaseFee calculates base fee from the (current) header and sets value into baseFee field
func (p *TxPool) SetBaseFee(header *types.Header) {
atomic.StoreUint64(&p.baseFee, p.store.CalculateBaseFee(header))
}
32 changes: 18 additions & 14 deletions txpool/txpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type store interface {
GetNonce(root types.Hash, addr types.Address) uint64
GetBalance(root types.Hash, addr types.Address) (*big.Int, error)
GetBlockByHash(types.Hash, bool) (*types.Block, bool)
CalculateBaseFee(parent *types.Header) uint64
}

type signer interface {
Expand Down Expand Up @@ -331,15 +332,12 @@ func (p *TxPool) AddTx(tx *types.Transaction) error {

// Prepare generates all the transactions
// ready for execution. (primaries)
func (p *TxPool) Prepare(baseFee uint64) {
// set base fee
atomic.StoreUint64(&p.baseFee, baseFee)

func (p *TxPool) Prepare() {
// fetch primary from each account
primaries := p.accounts.getPrimaries()

// create new executables queue with base fee and initial transactions (primaries)
p.executables = newPricesQueue(baseFee, primaries)
p.executables = newPricesQueue(p.GetBaseFee(), primaries)
}

// Peek returns the best-price selected
Expand Down Expand Up @@ -532,6 +530,11 @@ func (p *TxPool) processEvent(event *blockchain.Event) {
}
}

// update base fee
if ln := len(event.NewChain); ln > 0 {
p.SetBaseFee(event.NewChain[ln-1])
}

// reset accounts with the new state
p.resetAccounts(stateNonces)

Expand Down Expand Up @@ -596,6 +599,13 @@ func (p *TxPool) validateTx(tx *types.Transaction) error {
return runtime.ErrMaxCodeSizeExceeded
}

// Grab the state root, current block number and block gas limit for the latest block
currentHeader := p.store.Header()
stateRoot := currentHeader.StateRoot
currentBlockNumber := currentHeader.Number
latestBlockGasLimit := currentHeader.GasLimit
baseFee := p.GetBaseFee() // base fee is calculated for the next block

if tx.Type == types.DynamicFeeTx {
// Reject dynamic fee tx if london hardfork is not enabled
if !p.forks.London {
Expand All @@ -606,7 +616,7 @@ func (p *TxPool) validateTx(tx *types.Transaction) error {

// DynamicFeeTx should be rejected if TxHashWithType fork is registered but not enabled for current block
blockNumber, err := forkmanager.GetInstance().GetForkBlock(chain.TxHashWithType)
if err == nil && blockNumber > p.store.Header().Number {
if err == nil && blockNumber > currentBlockNumber {
metrics.IncrCounter([]string{txPoolMetrics, "dynamic_tx_not_allowed"}, 1)

return ErrDynamicTxNotAllowed
Expand Down Expand Up @@ -638,23 +648,20 @@ func (p *TxPool) validateTx(tx *types.Transaction) error {
}

// Reject underpriced transactions
if tx.GasFeeCap.Cmp(new(big.Int).SetUint64(p.GetBaseFee())) < 0 {
if tx.GasFeeCap.Cmp(new(big.Int).SetUint64(baseFee)) < 0 {
metrics.IncrCounter([]string{txPoolMetrics, "underpriced_tx"}, 1)

return ErrUnderpriced
}
} else {
// Legacy approach to check if the given tx is not underpriced
if tx.GetGasPrice(p.GetBaseFee()).Cmp(big.NewInt(0).SetUint64(p.priceLimit)) < 0 {
if tx.GetGasPrice(baseFee).Cmp(big.NewInt(0).SetUint64(p.priceLimit)) < 0 {
metrics.IncrCounter([]string{txPoolMetrics, "underpriced_tx"}, 1)

return ErrUnderpriced
}
}

// Grab the state root for the latest block
stateRoot := p.store.Header().StateRoot

// Check nonce ordering
if p.store.GetNonce(stateRoot, tx.From) > tx.Nonce {
metrics.IncrCounter([]string{txPoolMetrics, "nonce_too_low_tx"}, 1)
Expand Down Expand Up @@ -690,9 +697,6 @@ func (p *TxPool) validateTx(tx *types.Transaction) error {
return ErrIntrinsicGas
}

// Grab the block gas limit for the latest block
latestBlockGasLimit := p.store.Header().GasLimit

if tx.Gas > latestBlockGasLimit {
metrics.IncrCounter([]string{txPoolMetrics, "block_gas_limit_exceeded_tx"}, 1)

Expand Down
Loading

0 comments on commit 787cbe4

Please sign in to comment.