Merge pull request #21358 from hendrikhofstadt/fix/tx-sort-time
core: sort txs at the same gas price by received time
This commit is contained in:
commit
a00dc5095b
@ -22,6 +22,7 @@ import (
|
||||
"io"
|
||||
"math/big"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/ethereum/go-ethereum/common"
|
||||
"github.com/ethereum/go-ethereum/common/hexutil"
|
||||
@ -36,7 +37,9 @@ var (
|
||||
)
|
||||
|
||||
type Transaction struct {
|
||||
data txdata
|
||||
data txdata // Consensus contents of a transaction
|
||||
time time.Time // Time first seen locally (spam avoidance)
|
||||
|
||||
// caches
|
||||
hash atomic.Value
|
||||
size atomic.Value
|
||||
@ -100,8 +103,10 @@ func newTransaction(nonce uint64, to *common.Address, amount *big.Int, gasLimit
|
||||
if gasPrice != nil {
|
||||
d.Price.Set(gasPrice)
|
||||
}
|
||||
|
||||
return &Transaction{data: d}
|
||||
return &Transaction{
|
||||
data: d,
|
||||
time: time.Now(),
|
||||
}
|
||||
}
|
||||
|
||||
// ChainId returns which chain id this transaction was signed for (if at all)
|
||||
@ -134,8 +139,8 @@ func (tx *Transaction) DecodeRLP(s *rlp.Stream) error {
|
||||
err := s.Decode(&tx.data)
|
||||
if err == nil {
|
||||
tx.size.Store(common.StorageSize(rlp.ListSize(size)))
|
||||
tx.time = time.Now()
|
||||
}
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
@ -153,7 +158,6 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
|
||||
if err := dec.UnmarshalJSON(input); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
withSignature := dec.V.Sign() != 0 || dec.R.Sign() != 0 || dec.S.Sign() != 0
|
||||
if withSignature {
|
||||
var V byte
|
||||
@ -167,8 +171,10 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
|
||||
return ErrInvalidSig
|
||||
}
|
||||
}
|
||||
|
||||
*tx = Transaction{data: dec}
|
||||
*tx = Transaction{
|
||||
data: dec,
|
||||
time: time.Now(),
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -246,7 +252,10 @@ func (tx *Transaction) WithSignature(signer Signer, sig []byte) (*Transaction, e
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
cpy := &Transaction{data: tx.data}
|
||||
cpy := &Transaction{
|
||||
data: tx.data,
|
||||
time: tx.time,
|
||||
}
|
||||
cpy.data.R, cpy.data.S, cpy.data.V = r, s, v
|
||||
return cpy, nil
|
||||
}
|
||||
@ -306,19 +315,27 @@ func (s TxByNonce) Len() int { return len(s) }
|
||||
func (s TxByNonce) Less(i, j int) bool { return s[i].data.AccountNonce < s[j].data.AccountNonce }
|
||||
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
|
||||
// TxByPriceAndTime 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 TxByPriceAndTime Transactions
|
||||
|
||||
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 TxByPriceAndTime) Len() int { return len(s) }
|
||||
func (s TxByPriceAndTime) Less(i, j int) bool {
|
||||
// If the prices are equal, use the time the transaction was first seen for
|
||||
// deterministic sorting
|
||||
cmp := s[i].data.Price.Cmp(s[j].data.Price)
|
||||
if cmp == 0 {
|
||||
return s[i].time.Before(s[j].time)
|
||||
}
|
||||
return cmp > 0
|
||||
}
|
||||
func (s TxByPriceAndTime) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
|
||||
|
||||
func (s *TxByPrice) Push(x interface{}) {
|
||||
func (s *TxByPriceAndTime) Push(x interface{}) {
|
||||
*s = append(*s, x.(*Transaction))
|
||||
}
|
||||
|
||||
func (s *TxByPrice) Pop() interface{} {
|
||||
func (s *TxByPriceAndTime) Pop() interface{} {
|
||||
old := *s
|
||||
n := len(old)
|
||||
x := old[n-1]
|
||||
@ -331,7 +348,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 TxByPriceAndTime // Next transaction for each unique account (price heap)
|
||||
signer Signer // Signer for the set of transactions
|
||||
}
|
||||
|
||||
@ -341,8 +358,8 @@ 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 {
|
||||
// Initialize a price based heap with the head transactions
|
||||
heads := make(TxByPrice, 0, len(txs))
|
||||
// Initialize a price and received time based heap with the head transactions
|
||||
heads := make(TxByPriceAndTime, 0, len(txs))
|
||||
for from, accTxs := range txs {
|
||||
heads = append(heads, accTxs[0])
|
||||
// Ensure the sender address is from the signer
|
||||
|
@ -22,6 +22,7 @@ import (
|
||||
"encoding/json"
|
||||
"math/big"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/ethereum/go-ethereum/common"
|
||||
"github.com/ethereum/go-ethereum/crypto"
|
||||
@ -127,8 +128,8 @@ func TestTransactionPriceNonceSort(t *testing.T) {
|
||||
for i := 0; i < len(keys); i++ {
|
||||
keys[i], _ = crypto.GenerateKey()
|
||||
}
|
||||
|
||||
signer := HomesteadSigner{}
|
||||
|
||||
// Generate a batch of transactions with overlapping values, but shifted nonces
|
||||
groups := map[common.Address]Transactions{}
|
||||
for start, key := range keys {
|
||||
@ -155,12 +156,10 @@ func TestTransactionPriceNonceSort(t *testing.T) {
|
||||
// Make sure the nonce order is valid
|
||||
for j, txj := range txs[i+1:] {
|
||||
fromj, _ := Sender(signer, txj)
|
||||
|
||||
if fromi == fromj && txi.Nonce() > txj.Nonce() {
|
||||
t.Errorf("invalid nonce ordering: tx #%d (A=%x N=%v) < tx #%d (A=%x N=%v)", i, fromi[:4], txi.Nonce(), i+j, fromj[:4], txj.Nonce())
|
||||
}
|
||||
}
|
||||
|
||||
// If the next tx has different from account, the price must be lower than the current one
|
||||
if i+1 < len(txs) {
|
||||
next := txs[i+1]
|
||||
@ -172,6 +171,54 @@ func TestTransactionPriceNonceSort(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Tests that if multiple transactions have the same price, the ones seen earlier
|
||||
// are prioritized to avoid network spam attacks aiming for a specific ordering.
|
||||
func TestTransactionTimeSort(t *testing.T) {
|
||||
// Generate a batch of accounts to start with
|
||||
keys := make([]*ecdsa.PrivateKey, 5)
|
||||
for i := 0; i < len(keys); i++ {
|
||||
keys[i], _ = crypto.GenerateKey()
|
||||
}
|
||||
signer := HomesteadSigner{}
|
||||
|
||||
// Generate a batch of transactions with overlapping prices, but different creation times
|
||||
groups := map[common.Address]Transactions{}
|
||||
for start, key := range keys {
|
||||
addr := crypto.PubkeyToAddress(key.PublicKey)
|
||||
|
||||
tx, _ := SignTx(NewTransaction(0, common.Address{}, big.NewInt(100), 100, big.NewInt(1), nil), signer, key)
|
||||
tx.time = time.Unix(0, int64(len(keys)-start))
|
||||
|
||||
groups[addr] = append(groups[addr], tx)
|
||||
}
|
||||
// Sort the transactions and cross check the nonce ordering
|
||||
txset := NewTransactionsByPriceAndNonce(signer, groups)
|
||||
|
||||
txs := Transactions{}
|
||||
for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
|
||||
txs = append(txs, tx)
|
||||
txset.Shift()
|
||||
}
|
||||
if len(txs) != len(keys) {
|
||||
t.Errorf("expected %d transactions, found %d", len(keys), len(txs))
|
||||
}
|
||||
for i, txi := range txs {
|
||||
fromi, _ := Sender(signer, txi)
|
||||
if i+1 < len(txs) {
|
||||
next := txs[i+1]
|
||||
fromNext, _ := Sender(signer, next)
|
||||
|
||||
if 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())
|
||||
}
|
||||
// Make sure time order is ascending if the txs have the same gas price
|
||||
if txi.GasPrice().Cmp(next.GasPrice()) == 0 && txi.time.After(next.time) {
|
||||
t.Errorf("invalid received time ordering: tx #%d (A=%x T=%v) > tx #%d (A=%x T=%v)", i, fromi[:4], txi.time, i+1, fromNext[:4], next.time)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestTransactionJSON tests serializing/de-serializing to/from JSON.
|
||||
func TestTransactionJSON(t *testing.T) {
|
||||
key, err := crypto.GenerateKey()
|
||||
|
Loading…
Reference in New Issue
Block a user