diff --git a/core/tx_list.go b/core/tx_list.go index 6b87b2d667d57..257bc9e24ab8c 100644 --- a/core/tx_list.go +++ b/core/tx_list.go @@ -401,8 +401,8 @@ type priceHeap struct { baseFee *big.Int } -func (h priceHeap) Len() int { return len(h.txs) } -func (h priceHeap) Swap(i, j int) { h.txs[i], h.txs[j] = h.txs[j], h.txs[i] } +func (h priceHeap) Len() int { return len(h.txs) } +func (h *priceHeap) Swap(i, j int) { h.txs[i], h.txs[j] = h.txs[j], h.txs[i] } func (h priceHeap) Less(i, j int) bool { // Sort primarily by price, returning the cheaper one diff --git a/core/types/transaction.go b/core/types/transaction.go index 651d12c142227..598e6f5830dab 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -438,21 +438,43 @@ func (s TxByNonce) Swap(i, j int) { s[i], s[j] = s[j], s[i] } // TxByPrice implements both the sort and the heap interface, making it useful // for all at once sorting as well as individually adding and removing elements. -type TxByPrice Transactions +type TxByPrice struct { + txs Transactions + baseFee *big.Int +} + +func (s TxByPrice) Len() int { return len(s.txs) } + +// Note that this returns true if j is less than i, as the ordering needs to be from highest price to lowest +func (s TxByPrice) Less(i, j int) bool { + iPrice := s.txs[i].data.Price + jPrice := s.txs[j].data.Price + if iPrice == nil { + iPrice = new(big.Int).Add(s.baseFee, s.txs[i].data.GasPremium) + if iPrice.Cmp(s.txs[i].data.FeeCap) > 0 { + iPrice.Set(s.txs[i].data.FeeCap) + } + } + if jPrice == nil { + jPrice = new(big.Int).Add(s.baseFee, s.txs[j].data.GasPremium) + if jPrice.Cmp(s.txs[j].data.FeeCap) > 0 { + jPrice.Set(s.txs[j].data.FeeCap) + } + } + return iPrice.Cmp(jPrice) > 0 +} -func (s TxByPrice) Len() int { return len(s) } -func (s TxByPrice) Less(i, j int) bool { return s[i].data.Price.Cmp(s[j].data.Price) > 0 } -func (s TxByPrice) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s *TxByPrice) Swap(i, j int) { s.txs[i], s.txs[j] = s.txs[j], s.txs[i] } func (s *TxByPrice) Push(x interface{}) { - *s = append(*s, x.(*Transaction)) + s.txs = append(s.txs, x.(*Transaction)) } func (s *TxByPrice) Pop() interface{} { - old := *s + old := s.txs n := len(old) x := old[n-1] - *s = old[0 : n-1] + s.txs = old[0 : n-1] return x } @@ -461,7 +483,7 @@ func (s *TxByPrice) Pop() interface{} { // entire batches of transactions for non-executable accounts. type TransactionsByPriceAndNonce struct { txs map[common.Address]Transactions // Per account nonce-sorted list of transactions - heads TxByPrice // Next transaction for each unique account (price heap) + heads *TxByPrice // Next transaction for each unique account (price heap) signer Signer // Signer for the set of transactions } @@ -470,11 +492,13 @@ type TransactionsByPriceAndNonce struct { // // Note, the input map is reowned so the caller should not interact any more with // if after providing it to the constructor. -func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions) *TransactionsByPriceAndNonce { +func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions, baseFee *big.Int) *TransactionsByPriceAndNonce { // Initialize a price based heap with the head transactions - heads := make(TxByPrice, 0, len(txs)) + heads := new(TxByPrice) + heads.txs = make(Transactions, 0, len(txs)) + heads.baseFee = baseFee for from, accTxs := range txs { - heads = append(heads, accTxs[0]) + heads.txs = append(heads.txs, accTxs[0]) // Ensure the sender address is from the signer acc, _ := Sender(signer, accTxs[0]) txs[acc] = accTxs[1:] @@ -482,7 +506,7 @@ func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transa delete(txs, from) } } - heap.Init(&heads) + heap.Init(heads) // Assemble and return the transaction set return &TransactionsByPriceAndNonce{ @@ -494,20 +518,20 @@ func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transa // Peek returns the next transaction by price. func (t *TransactionsByPriceAndNonce) Peek() *Transaction { - if len(t.heads) == 0 { + if len(t.heads.txs) == 0 { return nil } - return t.heads[0] + return t.heads.txs[0] } // Shift replaces the current best head with the next one from the same account. func (t *TransactionsByPriceAndNonce) Shift() { - acc, _ := Sender(t.signer, t.heads[0]) + acc, _ := Sender(t.signer, t.heads.txs[0]) if txs, ok := t.txs[acc]; ok && len(txs) > 0 { - t.heads[0], t.txs[acc] = txs[0], txs[1:] - heap.Fix(&t.heads, 0) + t.heads.txs[0], t.txs[acc] = txs[0], txs[1:] + heap.Fix(t.heads, 0) } else { - heap.Pop(&t.heads) + heap.Pop(t.heads) } } @@ -515,7 +539,7 @@ func (t *TransactionsByPriceAndNonce) Shift() { // the same account. This should be used when a transaction cannot be executed // and hence all subsequent ones should be discarded from the same account. func (t *TransactionsByPriceAndNonce) Pop() { - heap.Pop(&t.heads) + heap.Pop(t.heads) } // Message is a fully derived transaction and implements core.Message diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index 747644292f75c..08472c427eff0 100644 --- a/core/types/transaction_test.go +++ b/core/types/transaction_test.go @@ -330,12 +330,18 @@ func TestTransactionPriceNonceSort(t *testing.T) { for start, key := range keys { addr := crypto.PubkeyToAddress(key.PublicKey) for i := 0; i < 25; i++ { - tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, big.NewInt(int64(start+i)), nil, nil, nil), signer, key) - groups[addr] = append(groups[addr], tx) + if i%2 == 0 { + tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, big.NewInt(int64(start+i)), nil, nil, nil), signer, key) + groups[addr] = append(groups[addr], tx) + } else { + tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, nil, nil, big.NewInt(int64(start+i)), big.NewInt(int64(start+i+1))), signer, key) + groups[addr] = append(groups[addr], tx) + } } } // Sort the transactions and cross check the nonce ordering - txset := NewTransactionsByPriceAndNonce(signer, groups) + baseFee := big.NewInt(1) + txset := NewTransactionsByPriceAndNonce(signer, groups, baseFee) txs := Transactions{} for tx := txset.Peek(); tx != nil; tx = txset.Peek() { @@ -361,8 +367,22 @@ func TestTransactionPriceNonceSort(t *testing.T) { if i+1 < len(txs) { next := txs[i+1] fromNext, _ := Sender(signer, next) - if fromi != fromNext && txi.GasPrice().Cmp(next.GasPrice()) < 0 { - t.Errorf("invalid gasprice ordering: tx #%d (A=%x P=%v) < tx #%d (A=%x P=%v)", i, fromi[:4], txi.GasPrice(), i+1, fromNext[:4], next.GasPrice()) + iPrice := txi.GasPrice() + nextPrice := next.GasPrice() + if iPrice == nil { + iPrice = new(big.Int).Add(baseFee, txi.GasPremium()) + if iPrice.Cmp(txi.FeeCap()) > 0 { + iPrice.Set(txi.FeeCap()) + } + } + if nextPrice == nil { + nextPrice = new(big.Int).Add(baseFee, next.GasPremium()) + if nextPrice.Cmp(next.FeeCap()) > 0 { + nextPrice.Set(next.FeeCap()) + } + } + if fromi != fromNext && iPrice.Cmp(nextPrice) < 0 { + t.Errorf("invalid gasprice ordering: tx #%d (A=%x P=%v) < tx #%d (A=%x P=%v)", i, fromi[:4], iPrice.Uint64(), i+1, fromNext[:4], nextPrice.Uint64()) } } } @@ -379,10 +399,9 @@ func TestTransactionJSON(t *testing.T) { transactions := make([]*Transaction, 0, 50) for i := uint64(0); i < 25; i++ { var tx *Transaction - switch i % 2 { - case 0: + if i%2 == 0 { tx = NewTransaction(i, common.Address{1}, common.Big0, 1, common.Big2, []byte("abcdef"), nil, nil) - case 1: + } else { tx = NewContractCreation(i, common.Big0, 1, common.Big2, []byte("abcdef"), nil, nil) } transactions = append(transactions, tx) @@ -427,10 +446,9 @@ func TestEIP1559TransactionJSON(t *testing.T) { transactions := make([]*Transaction, 0, 50) for i := uint64(0); i < 25; i++ { var tx *Transaction - switch i % 2 { - case 0: + if i%2 == 0 { tx = NewTransaction(i, common.Address{1}, common.Big0, 1, nil, []byte("abcdef"), big.NewInt(200000), big.NewInt(800000)) - case 1: + } else { tx = NewContractCreation(i, common.Big0, 1, nil, []byte("abcdef"), big.NewInt(200000), big.NewInt(800000)) } transactions = append(transactions, tx) diff --git a/miner/worker.go b/miner/worker.go index 6df55e4d01fc8..d3412e6dc2cdf 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -458,11 +458,11 @@ func (w *worker) mainLoop() { legacyGasPool := w.current.gasPool eip1559GasPool := w.current.gp1559 // If EIP1559 is finalized we only accept 1559 transactions so if that pool is exhausted the block is full - if w.chainConfig.IsEIP1559Finalized(w.chain.CurrentBlock().Number()) && eip1559GasPool != nil && eip1559GasPool.Gas() < params.TxGas { + if w.chainConfig.IsEIP1559Finalized(w.current.header.Number) && eip1559GasPool != nil && eip1559GasPool.Gas() < params.TxGas { continue } // If EIP1559 has not been initialized we only accept legacy transaction so if that pool is exhausted the block is full - if !w.chainConfig.IsEIP1559(w.chain.CurrentBlock().Number()) && legacyGasPool != nil && legacyGasPool.Gas() < params.TxGas { + if !w.chainConfig.IsEIP1559(w.current.header.Number) && legacyGasPool != nil && legacyGasPool.Gas() < params.TxGas { continue } // When we are between EIP1559 activation and finalization we can received transactions of both types @@ -481,7 +481,7 @@ func (w *worker) mainLoop() { acc, _ := types.Sender(w.current.signer, tx) txs[acc] = append(txs[acc], tx) } - txset := types.NewTransactionsByPriceAndNonce(w.current.signer, txs) + txset := types.NewTransactionsByPriceAndNonce(w.current.signer, txs, w.current.header.BaseFee) tcount := w.current.tcount w.commitTransactions(txset, coinbase, nil) // Only update the snapshot if any new transactons were added @@ -1015,13 +1015,13 @@ func (w *worker) commitNewWork(interrupt *int32, noempty bool, timestamp int64) } } if len(localTxs) > 0 { - txs := types.NewTransactionsByPriceAndNonce(w.current.signer, localTxs) + txs := types.NewTransactionsByPriceAndNonce(w.current.signer, localTxs, baseFee) if w.commitTransactions(txs, w.coinbase, interrupt) { return } } if len(remoteTxs) > 0 { - txs := types.NewTransactionsByPriceAndNonce(w.current.signer, remoteTxs) + txs := types.NewTransactionsByPriceAndNonce(w.current.signer, remoteTxs, baseFee) if w.commitTransactions(txs, w.coinbase, interrupt) { return } diff --git a/signer/core/api_test.go b/signer/core/api_test.go index 87b13b6213372..3f83566313e2c 100644 --- a/signer/core/api_test.go +++ b/signer/core/api_test.go @@ -223,25 +223,34 @@ func TestNewAcc(t *testing.T) { } } -func mkTestTx(from common.MixedcaseAddress) core.SendTxArgs { +func mkTestTx(from common.MixedcaseAddress, eip1559 bool) core.SendTxArgs { to := common.NewMixedcaseAddress(common.HexToAddress("0x1337")) gas := hexutil.Uint64(21000) - gasPrice := (*hexutil.Big)(big.NewInt(2000000000)) value := (hexutil.Big)(*big.NewInt(1e18)) nonce := (hexutil.Uint64)(0) data := hexutil.Bytes(common.Hex2Bytes("01020304050607080a")) tx := core.SendTxArgs{ - From: from, - To: &to, - Gas: gas, - GasPrice: gasPrice, - Value: value, - Data: &data, - Nonce: nonce} + From: from, + To: &to, + Gas: gas, + Value: value, + Data: &data, + Nonce: nonce} + if eip1559 { + tx.GasPremium = (*hexutil.Big)(big.NewInt(1000000000)) + tx.FeeCap = (*hexutil.Big)(big.NewInt(2000000000)) + } else { + tx.GasPrice = (*hexutil.Big)(big.NewInt(2000000000)) + } return tx } func TestSignTx(t *testing.T) { + testSignTx(t, false) + testSignTx(t, true) +} + +func testSignTx(t *testing.T, eip1559 bool) { var ( list []common.Address res, res2 *ethapi.SignTransactionResult @@ -258,7 +267,7 @@ func TestSignTx(t *testing.T) { a := common.NewMixedcaseAddress(list[0]) methodSig := "test(uint)" - tx := mkTestTx(a) + tx := mkTestTx(a, eip1559) control.approveCh <- "Y" control.inputCh <- "wrongpassword" @@ -321,5 +330,4 @@ func TestSignTx(t *testing.T) { if bytes.Equal(res.Raw, res2.Raw) { t.Error("Expected tx to be modified by UI") } - }