From f4efe97f0443d22762677c2e1dd9e83d8520b3fe Mon Sep 17 00:00:00 2001 From: Igor Crevar Date: Tue, 11 Jul 2023 11:25:25 +0200 Subject: [PATCH] EVM-728 Gas price comparison for executable transactions in `pricedQueue` (#1696) --- gasprice/feehistory.go | 3 +- gasprice/gasprice.go | 12 +-- txpool/queue_priced.go | 86 ++++++++------------- txpool/queue_priced_test.go | 26 +++---- txpool/txpool.go | 21 +----- txpool/txpool_test.go | 145 ++++++++++++++++++++++++------------ types/transaction.go | 48 ++++++++---- 7 files changed, 182 insertions(+), 159 deletions(-) diff --git a/gasprice/feehistory.go b/gasprice/feehistory.go index 89b941b3f9..5d28b2177c 100644 --- a/gasprice/feehistory.go +++ b/gasprice/feehistory.go @@ -130,12 +130,13 @@ func (g *GasHelper) FeeHistory(blockCount uint64, newestBlock uint64, rewardPerc } sorter := make([]*txGasAndReward, len(block.Transactions)) + baseFee := new(big.Int).SetUint64(block.Header.BaseFee) for j, tx := range block.Transactions { cost := tx.Cost() sorter[j] = &txGasAndReward{ gasUsed: cost.Sub(cost, tx.Value), - reward: tx.EffectiveTip(block.Header.BaseFee), + reward: tx.EffectiveGasTip(baseFee), } } diff --git a/gasprice/gasprice.go b/gasprice/gasprice.go index ed6f3680b6..7c04dfd299 100644 --- a/gasprice/gasprice.go +++ b/gasprice/gasprice.go @@ -140,7 +140,7 @@ func (g *GasHelper) MaxPriorityFeePerGas() (*big.Int, error) { var allPrices []*big.Int collectPrices := func(block *types.Block) error { - baseFee := block.Header.BaseFee + baseFee := new(big.Int).SetUint64(block.Header.BaseFee) txSorter := newTxByEffectiveTipSorter(block.Transactions, baseFee) sort.Sort(txSorter) @@ -150,7 +150,7 @@ func (g *GasHelper) MaxPriorityFeePerGas() (*big.Int, error) { blockTxPrices := make([]*big.Int, 0) for _, tx := range txSorter.txs { - tip := tx.EffectiveTip(baseFee) + tip := tx.EffectiveGasTip(baseFee) if tip.Cmp(g.ignorePrice) == -1 { // ignore transactions with tip lower than ignore price @@ -238,11 +238,11 @@ func (g *GasHelper) MaxPriorityFeePerGas() (*big.Int, error) { // txSortedByEffectiveTip sorts transactions by effective tip from smallest to largest type txSortedByEffectiveTip struct { txs []*types.Transaction - baseFee uint64 + baseFee *big.Int } // newTxByEffectiveTipSorter is constructor function for txSortedByEffectiveTip -func newTxByEffectiveTipSorter(txs []*types.Transaction, baseFee uint64) *txSortedByEffectiveTip { +func newTxByEffectiveTipSorter(txs []*types.Transaction, baseFee *big.Int) *txSortedByEffectiveTip { return &txSortedByEffectiveTip{ txs: txs, baseFee: baseFee, @@ -259,8 +259,8 @@ func (t *txSortedByEffectiveTip) Swap(i, j int) { // Less is implementation of sort.Interface func (t *txSortedByEffectiveTip) Less(i, j int) bool { - tip1 := t.txs[i].EffectiveTip(t.baseFee) - tip2 := t.txs[j].EffectiveTip(t.baseFee) + tip1 := t.txs[i].EffectiveGasTip(t.baseFee) + tip2 := t.txs[j].EffectiveGasTip(t.baseFee) return tip1.Cmp(tip2) < 0 } diff --git a/txpool/queue_priced.go b/txpool/queue_priced.go index 88390a11fd..0bdf12a72f 100644 --- a/txpool/queue_priced.go +++ b/txpool/queue_priced.go @@ -3,7 +3,6 @@ package txpool import ( "container/heap" "math/big" - "sync/atomic" "github.com/0xPolygon/polygon-edge/types" ) @@ -12,19 +11,18 @@ type pricedQueue struct { queue *maxPriceQueue } -func newPricedQueue() *pricedQueue { - q := pricedQueue{ - queue: &maxPriceQueue{}, +// newPricesQueue creates the priced queue with initial transactions and base fee +func newPricesQueue(baseFee uint64, initialTxs []*types.Transaction) *pricedQueue { + q := &pricedQueue{ + queue: &maxPriceQueue{ + baseFee: new(big.Int).SetUint64(baseFee), + txs: initialTxs, + }, } heap.Init(q.queue) - return &q -} - -// clear empties the underlying queue. -func (q *pricedQueue) clear() { - q.queue.txs = q.queue.txs[:0] + return q } // Pushes the given transactions onto the queue. @@ -48,13 +46,13 @@ func (q *pricedQueue) pop() *types.Transaction { } // length returns the number of transactions in the queue. -func (q *pricedQueue) length() uint64 { - return uint64(q.queue.Len()) +func (q *pricedQueue) length() int { + return q.queue.Len() } // transactions sorted by gas price (descending) type maxPriceQueue struct { - baseFee uint64 + baseFee *big.Int txs []*types.Transaction } @@ -76,17 +74,6 @@ func (q *maxPriceQueue) Swap(i, j int) { q.txs[i], q.txs[j] = q.txs[j], q.txs[i] } -func (q *maxPriceQueue) Less(i, j int) bool { - switch q.cmp(q.txs[i], q.txs[j]) { - case -1: - return true - case 1: - return false - default: - return q.txs[i].Nonce > q.txs[j].Nonce - } -} - func (q *maxPriceQueue) Push(x interface{}) { transaction, ok := x.(*types.Transaction) if !ok { @@ -105,45 +92,32 @@ func (q *maxPriceQueue) Pop() interface{} { return x } -// cmp compares the given transactions by their fees and returns: -// - 0 if they have same fees -// - 1 if a has higher fees than b -// - -1 if b has higher fees than a -func (q *maxPriceQueue) cmp(a, b *types.Transaction) int { - baseFee := atomic.LoadUint64(&q.baseFee) - effectiveTipA := a.EffectiveTip(baseFee) - effectiveTipB := b.EffectiveTip(baseFee) - - // Compare effective tips if baseFee is specified - if c := effectiveTipA.Cmp(effectiveTipB); c != 0 { - return c - } - - aGasFeeCap, bGasFeeCap := new(big.Int), new(big.Int) - - if a.GasFeeCap != nil { - aGasFeeCap = aGasFeeCap.Set(a.GasFeeCap) +// @see https://github.com/etclabscore/core-geth/blob/4e2b0e37f89515a4e7b6bafaa40910a296cb38c0/core/txpool/list.go#L458 +// for details why is something implemented like it is +func (q *maxPriceQueue) Less(i, j int) bool { + switch cmp(q.txs[i], q.txs[j], q.baseFee) { + case -1: + return false + case 1: + return true + default: + return q.txs[i].Nonce < q.txs[j].Nonce } +} - if b.GasFeeCap != nil { - bGasFeeCap = bGasFeeCap.Set(b.GasFeeCap) +func cmp(a, b *types.Transaction, baseFee *big.Int) int { + if baseFee.BitLen() > 0 { + // Compare effective tips if baseFee is specified + if c := a.EffectiveGasTip(baseFee).Cmp(b.EffectiveGasTip(baseFee)); c != 0 { + return c + } } // Compare fee caps if baseFee is not specified or effective tips are equal - if c := aGasFeeCap.Cmp(bGasFeeCap); c != 0 { + if c := a.GetGasFeeCap().Cmp(b.GetGasFeeCap()); c != 0 { return c } - aGasTipCap, bGasTipCap := new(big.Int), new(big.Int) - - if a.GasTipCap != nil { - aGasTipCap = aGasTipCap.Set(a.GasTipCap) - } - - if b.GasTipCap != nil { - bGasTipCap = bGasTipCap.Set(b.GasTipCap) - } - // Compare tips if effective tips and fee caps are equal - return aGasTipCap.Cmp(bGasTipCap) + return a.GetGasTipCap().Cmp(b.GetGasTipCap()) } diff --git a/txpool/queue_priced_test.go b/txpool/queue_priced_test.go index 7e8aea3695..e5b3dd7300 100644 --- a/txpool/queue_priced_test.go +++ b/txpool/queue_priced_test.go @@ -3,12 +3,10 @@ package txpool import ( "math/big" "math/rand" - "sort" "testing" - "github.com/stretchr/testify/assert" - "github.com/0xPolygon/polygon-edge/types" + "github.com/stretchr/testify/assert" ) func Test_maxPriceQueue(t *testing.T) { @@ -224,6 +222,12 @@ func Test_maxPriceQueue(t *testing.T) { }, }, }, + { + name: "empty", + baseFee: 0, + unsorted: nil, + sorted: []*types.Transaction{}, + }, } for _, tt := range testTable { @@ -232,15 +236,10 @@ func Test_maxPriceQueue(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - queue := &maxPriceQueue{ - baseFee: tt.baseFee, - txs: tt.unsorted, - } - - sort.Sort(queue) + queue := newPricesQueue(tt.baseFee, tt.unsorted) for _, tx := range tt.sorted { - actual := queue.Pop() + actual := queue.pop() assert.Equal(t, tx, actual) } }) @@ -269,12 +268,7 @@ func Benchmark_pricedQueue(t *testing.B) { for _, tt := range testTable { t.Run(tt.name, func(b *testing.B) { for i := 0; i < t.N; i++ { - q := newPricedQueue() - q.queue.baseFee = uint64(i) - - for _, tx := range tt.unsortedTxs { - q.push(tx) - } + q := newPricesQueue(uint64(100), tt.unsortedTxs) for q.length() > 0 { _ = q.pop() diff --git a/txpool/txpool.go b/txpool/txpool.go index b5a2aca714..41c5ca4008 100644 --- a/txpool/txpool.go +++ b/txpool/txpool.go @@ -202,7 +202,7 @@ func NewTxPool( logger: logger.Named("txpool"), forks: forks, store: store, - executables: newPricedQueue(), + executables: newPricesQueue(0, nil), accounts: accountsMap{maxEnqueuedLimit: config.MaxAccountEnqueued}, index: lookupMap{all: make(map[types.Hash]*types.Transaction)}, gauge: slotGauge{height: 0, max: config.MaxSlots}, @@ -325,21 +325,14 @@ func (p *TxPool) AddTx(tx *types.Transaction) error { // Prepare generates all the transactions // ready for execution. (primaries) func (p *TxPool) Prepare(baseFee uint64) { - // clear from previous round - if p.executables.length() != 0 { - p.executables.clear() - } - // set base fee - p.updateBaseFee(baseFee) + atomic.StoreUint64(&p.baseFee, baseFee) // fetch primary from each account primaries := p.accounts.getPrimaries() - // push primaries to the executables queue - for _, tx := range primaries { - p.executables.push(tx) - } + // create new executables queue with base fee and initial transactions (primaries) + p.executables = newPricesQueue(baseFee, primaries) } // Peek returns the best-price selected @@ -1027,12 +1020,6 @@ func (p *TxPool) Length() uint64 { return p.accounts.promoted() } -// updateBaseFee updates base fee in the tx pool and priced queue -func (p *TxPool) updateBaseFee(baseFee uint64) { - atomic.StoreUint64(&p.baseFee, baseFee) - atomic.StoreUint64(&p.executables.queue.baseFee, baseFee) -} - // toHash returns the hash(es) of given transaction(s) func toHash(txs ...*types.Transaction) (hashes []types.Hash) { for _, tx := range txs { diff --git a/txpool/txpool_test.go b/txpool/txpool_test.go index 8e80a83d04..2a7acca20c 100644 --- a/txpool/txpool_test.go +++ b/txpool/txpool_test.go @@ -2612,9 +2612,20 @@ func TestResetAccounts_Enqueued(t *testing.T) { func TestExecutablesOrder(t *testing.T) { t.Parallel() - newPricedTx := func(addr types.Address, nonce, gasPrice uint64) *types.Transaction { + newPricedTx := func( + addr types.Address, nonce, gasPrice uint64, gasFeeCap uint64, value uint64) *types.Transaction { tx := newTx(addr, nonce, 1) - tx.GasPrice.SetUint64(gasPrice) + tx.Value = new(big.Int).SetUint64(value) + + if gasPrice == 0 { + tx.Type = types.DynamicFeeTx + tx.GasFeeCap = new(big.Int).SetUint64(gasFeeCap) + tx.GasTipCap = new(big.Int).SetUint64(2) + tx.GasPrice = big.NewInt(0) + } else { + tx.Type = types.LegacyTx + tx.GasPrice = new(big.Int).SetUint64(gasPrice) + } return tx } @@ -2622,87 +2633,118 @@ func TestExecutablesOrder(t *testing.T) { testCases := []struct { name string allTxs map[types.Address][]*types.Transaction - expectedPriceOrder []uint64 + expectedPriceOrder [][2]uint64 }{ { name: "case #1", allTxs: map[types.Address][]*types.Transaction{ addr1: { - newPricedTx(addr1, 0, 1), + newPricedTx(addr1, 0, 1, 0, 400), }, addr2: { - newPricedTx(addr2, 0, 2), + newPricedTx(addr2, 0, 2, 0, 500), }, addr3: { - newPricedTx(addr3, 0, 3), + newPricedTx(addr3, 0, 3, 0, 200), }, addr4: { - newPricedTx(addr4, 0, 4), + newPricedTx(addr4, 0, 4, 0, 300), }, addr5: { - newPricedTx(addr5, 0, 5), + newPricedTx(addr5, 0, 5, 0, 100), }, }, - expectedPriceOrder: []uint64{ - 5, - 4, - 3, - 2, - 1, + expectedPriceOrder: [][2]uint64{ + {5, 100}, + {4, 300}, + {3, 200}, + {2, 500}, + {1, 400}, }, }, { name: "case #2", allTxs: map[types.Address][]*types.Transaction{ addr1: { - newPricedTx(addr1, 0, 3), - newPricedTx(addr1, 1, 3), - newPricedTx(addr1, 2, 3), + newPricedTx(addr1, 1, 3, 0, 200), + newPricedTx(addr1, 2, 3, 0, 500), + newPricedTx(addr1, 0, 3, 0, 300), }, addr2: { - newPricedTx(addr2, 0, 2), - newPricedTx(addr2, 1, 2), - newPricedTx(addr2, 2, 2), + newPricedTx(addr2, 0, 2, 0, 700), + newPricedTx(addr2, 1, 2, 0, 900), + newPricedTx(addr2, 2, 2, 0, 800), }, addr3: { - newPricedTx(addr3, 0, 1), - newPricedTx(addr3, 1, 1), - newPricedTx(addr3, 2, 1), + newPricedTx(addr3, 2, 1, 0, 100), + newPricedTx(addr3, 1, 1, 0, 50), + newPricedTx(addr3, 0, 1, 0, 75), }, }, - expectedPriceOrder: []uint64{ - 3, - 3, - 3, - 2, - 2, - 2, - 1, - 1, - 1, + expectedPriceOrder: [][2]uint64{ + {3, 300}, + {3, 200}, + {3, 500}, + {2, 700}, + {2, 900}, + {2, 800}, + {1, 75}, + {1, 50}, + {1, 100}, }, }, { name: "case #3", allTxs: map[types.Address][]*types.Transaction{ addr1: { - newPricedTx(addr1, 0, 9), - newPricedTx(addr1, 1, 5), - newPricedTx(addr1, 2, 3), + newPricedTx(addr1, 1, 5, 0, 100), + newPricedTx(addr1, 0, 9, 0, 100), + newPricedTx(addr1, 2, 3, 0, 100), + }, + addr2: { + newPricedTx(addr2, 0, 9, 0, 100), + newPricedTx(addr2, 1, 3, 0, 100), + newPricedTx(addr2, 2, 1, 0, 100), + }, + }, + expectedPriceOrder: [][2]uint64{ + {9, 100}, + {9, 100}, + {5, 100}, + {3, 100}, + {3, 100}, + {1, 100}, + }, + }, + { + name: "case #4", + allTxs: map[types.Address][]*types.Transaction{ + addr1: { + newPricedTx(addr1, 0, 0, 70, 1), + newPricedTx(addr1, 1, 0, 50, 2), + newPricedTx(addr1, 2, 0, 20, 3), }, addr2: { - newPricedTx(addr2, 0, 9), - newPricedTx(addr2, 1, 3), - newPricedTx(addr2, 2, 1), + newPricedTx(addr2, 0, 0, 90, 4), + newPricedTx(addr2, 1, 0, 80, 5), + newPricedTx(addr2, 2, 0, 10, 6), + }, + addr3: { + newPricedTx(addr3, 0, 0, 100, 7), + newPricedTx(addr3, 1, 0, 60, 8), + newPricedTx(addr3, 2, 0, 40, 9), }, }, - expectedPriceOrder: []uint64{ - 9, - 9, - 5, - 3, - 3, - 1, + expectedPriceOrder: [][2]uint64{ + {0, 7}, + {0, 4}, + {0, 5}, + {0, 1}, + {0, 8}, + {0, 2}, + {0, 9}, + {0, 3}, + {0, 6}, }, }, } @@ -2736,8 +2778,10 @@ func TestExecutablesOrder(t *testing.T) { defer cancelFn() // All txns should get added - assert.Len(t, waitForEvents(ctx, subscription, expectedPromotedTx), expectedPromotedTx) - assert.Equal(t, uint64(len(test.expectedPriceOrder)), pool.accounts.promoted()) + require.Len(t, waitForEvents(ctx, subscription, expectedPromotedTx), expectedPromotedTx) + require.Equal(t, uint64(len(test.expectedPriceOrder)), pool.accounts.promoted()) + + pool.Prepare(1000) var successful []*types.Transaction for { @@ -2750,10 +2794,13 @@ func TestExecutablesOrder(t *testing.T) { successful = append(successful, tx) } + require.Len(t, successful, expectedPromotedTx) + // verify the highest priced transactions // were processed first for i, tx := range successful { - assert.Equal(t, test.expectedPriceOrder[i], tx.GasPrice.Uint64()) + require.Equal(t, test.expectedPriceOrder[i][0], tx.GasPrice.Uint64()) + require.Equal(t, test.expectedPriceOrder[i][1], tx.Value.Uint64()) } }) } diff --git a/types/transaction.go b/types/transaction.go index a66a94a3ba..9cb2b2ea80 100644 --- a/types/transaction.go +++ b/types/transaction.go @@ -153,7 +153,7 @@ func (t *Transaction) GetGasPrice(baseFee uint64) *big.Int { if t.GasPrice != nil && t.GasPrice.BitLen() > 0 { return new(big.Int).Set(t.GasPrice) } else if baseFee == 0 { - return new(big.Int) + return big.NewInt(0) } gasFeeCap := new(big.Int) @@ -166,18 +166,17 @@ func (t *Transaction) GetGasPrice(baseFee uint64) *big.Int { gasTipCap = gasTipCap.Set(t.GasTipCap) } - gasPrice := new(big.Int) if gasFeeCap.BitLen() > 0 || gasTipCap.BitLen() > 0 { - gasPrice = common.BigMin( - new(big.Int).Add( + return common.BigMin( + gasTipCap.Add( gasTipCap, new(big.Int).SetUint64(baseFee), ), - new(big.Int).Set(gasFeeCap), + gasFeeCap, ) } - return gasPrice + return big.NewInt(0) } func (t *Transaction) Size() uint64 { @@ -191,17 +190,38 @@ func (t *Transaction) Size() uint64 { return size } -// EffectiveTip defines effective tip based on tx type. +// EffectiveGasTip defines effective tip based on tx type. // Spec: https://eips.ethereum.org/EIPS/eip-1559#specification // We use EIP-1559 fields of the tx if the london hardfork is enabled. // Effective tip be came to be either gas tip cap or (gas fee cap - current base fee) -func (t *Transaction) EffectiveTip(baseFee uint64) *big.Int { - if t.GasFeeCap != nil && t.GasTipCap != nil { - return common.BigMin( - new(big.Int).Sub(t.GasFeeCap, new(big.Int).SetUint64(baseFee)), - new(big.Int).Set(t.GasTipCap), - ) +func (t *Transaction) EffectiveGasTip(baseFee *big.Int) *big.Int { + if baseFee == nil || baseFee.BitLen() == 0 { + return t.GetGasTipCap() } - return t.GetGasPrice(baseFee) + return common.BigMin( + new(big.Int).Set(t.GetGasTipCap()), + new(big.Int).Sub(t.GetGasFeeCap(), baseFee)) +} + +// GetGasTipCap gets gas tip cap depending on tx type +// Spec: https://eips.ethereum.org/EIPS/eip-1559#specification +func (t *Transaction) GetGasTipCap() *big.Int { + switch t.Type { + case DynamicFeeTx: + return t.GasTipCap + default: + return t.GasPrice + } +} + +// GetGasFeeCap gets gas fee cap depending on tx type +// Spec: https://eips.ethereum.org/EIPS/eip-1559#specification +func (t *Transaction) GetGasFeeCap() *big.Int { + switch t.Type { + case DynamicFeeTx: + return t.GasFeeCap + default: + return t.GasPrice + } }