diff --git a/core/tx_pool.go b/core/tx_pool.go
index 60ae3d72f1d17791976cc9805b9fe01dd635e937..16cd7076b02e778b294a4e435b823f66cc167781 100644
--- a/core/tx_pool.go
+++ b/core/tx_pool.go
@@ -589,7 +589,6 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
// whitelisted, preventing any associated transaction from being dropped out of the pool
// due to pricing constraints.
func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err error) {
- tx.SetReceivedTime(time.Now())
// If the transaction is already known, discard it
hash := tx.Hash()
if pool.all.Get(hash) != nil {
diff --git a/core/types/transaction.go b/core/types/transaction.go
index f4673b5c3e6c3178b9272a82105afdcdf79312b4..807517e5c5bf564601f2f9e8b6d7a77778e39ab6 100644
--- a/core/types/transaction.go
+++ b/core/types/transaction.go
@@ -39,10 +39,9 @@ var (
)
type Transaction struct {
- data txdata
+ data txdata // Consensus contents of a transaction
+ time time.Time // Time first seen locally (spam avoidance)
- // Time when the transaction was added to the txPool
- receivedTime *time.Time
// caches
hash atomic.Value
size atomic.Value
@@ -101,8 +100,10 @@ func newTransaction(nonce uint64, to *common.Address, amount *uint256.Int, gasLi
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)
@@ -135,8 +136,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
}
@@ -154,7 +155,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
@@ -168,8 +168,10 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
return ErrInvalidSig
}
}
-
- *tx = Transaction{data: dec}
+ *tx = Transaction{
+ data: dec,
+ time: time.Now(),
+ }
return nil
}
@@ -185,13 +187,6 @@ func (tx *Transaction) GasPriceIntCmp(other *uint256.Int) int {
func (tx *Transaction) Value() *uint256.Int { return new(uint256.Int).Set(&tx.data.Amount) }
func (tx *Transaction) Nonce() uint64 { return tx.data.AccountNonce }
func (tx *Transaction) CheckNonce() bool { return true }
-func (tx *Transaction) ReceivedTime() (time.Time, bool) {
- if tx.receivedTime == nil {
- return time.Time{}, false
- }
-
- return *tx.receivedTime, true
-}
// To returns the recipient address of the transaction.
// It returns nil if the transaction is a contract creation.
@@ -265,6 +260,10 @@ func (tx *Transaction) WithSignature(signer Signer, sig []byte) (*Transaction, e
if err != nil {
return nil, err
}
+ cpy := &Transaction{
+ data: tx.data,
+ time: tx.time,
+ }
cpy := &Transaction{data: tx.data}
cpy.data.R, cpy.data.S, cpy.data.V = *r, *s, *v
return cpy, nil
@@ -330,35 +329,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] }
-// TxByPriceAndReceiveTime 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 TxByPriceAndReceiveTime Transactions
-
-func (s TxByPriceAndReceiveTime) Len() int { return len(s) }
-func (s TxByPriceAndReceiveTime) Less(i, j int) bool {
- // If the price is equal, use the time the tx was received for deterministic sorting
- if s[i].data.Price.Cmp(s[j].data.Price) == 0 {
- recvI, ok := s[i].ReceivedTime()
- if !ok {
- return true
- }
-
- recvJ, ok := s[j].ReceivedTime()
- if !ok {
- return true
- }
- return recvI.UnixNano() < recvJ.UnixNano()
+type TxByPriceAndTime Transactions
+
+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 s[i].data.Price.Cmp(s[j].data.Price) > 0
+ return cmp > 0
}
-func (s TxByPriceAndReceiveTime) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
+func (s TxByPriceAndTime) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
-func (s *TxByPriceAndReceiveTime) Push(x interface{}) {
+func (s *TxByPriceAndTime) Push(x interface{}) {
*s = append(*s, x.(*Transaction))
}
-func (s *TxByPriceAndReceiveTime) Pop() interface{} {
+func (s *TxByPriceAndTime) Pop() interface{} {
old := *s
n := len(old)
x := old[n-1]
@@ -371,7 +362,7 @@ func (s *TxByPriceAndReceiveTime) 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 TxByPriceAndReceiveTime // 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
}
@@ -382,7 +373,7 @@ type TransactionsByPriceAndNonce struct {
// if after providing it to the constructor.
func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions) *TransactionsByPriceAndNonce {
// Initialize a price and received time based heap with the head transactions
- heads := make(TxByPriceAndReceiveTime, 0, len(txs))
+ heads := make(TxByPriceAndTime, 0, len(txs))
for from, accTxs := range txs {
heads = append(heads, accTxs[0])
// Ensure the sender address is from the signer
diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go
index 4b5dd226a228be9ef7cb294a661a802c19d321c7..39fc879b49e9dcabfb1ad3469e70118153de96c9 100644
--- a/core/types/transaction_test.go
+++ b/core/types/transaction_test.go
@@ -130,15 +130,14 @@ 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 {
addr := crypto.PubkeyToAddress(key.PublicKey)
for i := 0; i < 25; i++ {
tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, uint256.NewInt().SetUint64(100), 100, uint256.NewInt().SetUint64(uint64(start+i)), nil), signer, key)
- tx.SetReceivedTime(time.Unix(0, int64(start)))
groups[addr] = append(groups[addr], tx)
}
}
@@ -159,12 +158,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,10 +169,53 @@ func TestTransactionPriceNonceSort(t *testing.T) {
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())
}
+ }
+ }
+}
+
+// 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))
- // Make sure receivedTime order is ascending if the txs have the same gas price
- if txi.GasPrice().Cmp(next.GasPrice()) == 0 && fromi != fromNext && txi.receivedTime.UnixNano() > next.receivedTime.UnixNano() {
- t.Errorf("invalid received time ordering: tx #%d (A=%x T=%d) > tx #%d (A=%x T=%d)", i, fromi[:4], txi.receivedTime.UnixNano(), i+1, fromNext[:4], next.receivedTime.UnixNano())
+ 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)
}
}
}