[core] fix: Use time-based ordering to avoid spam

Closes harmony-one#4113 by sorting transactions by time received
instead of using a hashmap based transaction ordering. This sorting is
on top of the gas price and nonce sorting already implemented.

Effectively, this allows the production of almost a deterministic order
of transaction ordering, as opposed to a heap-based hash map ordering
which is affected by Golang's internal operations. Consequently,
arbitrageurs, who spam the network with a view to exist around
arbitrag-able transactions, will have to focus on latency instead of
network spamming.

See also bnb-chain/bsc#269 and ethereum/go-ethereum#21358 for related
issues in other chains.

core/types/eth_transaction.go

@@ -20,6 +20,7 @@ import (
 	"io"
 	"math/big"
 	"sync/atomic"
+	"time"
 
 	"github.com/harmony-one/harmony/internal/params"
 
@@ -43,6 +44,9 @@ type EthTransaction struct {
 	hash atomic.Value
 	size atomic.Value
 	from atomic.Value
+	// time at which the node received the tx
+	// and not the time set by the sender
+	time time.Time
 }
 
 type ethTxdata struct {
@@ -113,14 +117,19 @@ func newEthTransaction(nonce uint64, to *common.Address, amount *big.Int, gasLim
 		d.Price.Set(gasPrice)
 	}
 
-	return &EthTransaction{data: d}
+	return &EthTransaction{data: d, time: time.Now()}
 }
 
 // From returns the sender address of the transaction
 func (tx *EthTransaction) From() *atomic.Value {
 	return &tx.from
 }
 
+// Time returns the time at which the transaction was received by the node
+func (tx *EthTransaction) Time() time.Time {
+	return tx.time
+}
+
 // V value of the transaction signature
 func (tx *EthTransaction) V() *big.Int {
 	return tx.data.V
@@ -180,6 +189,7 @@ func (tx *EthTransaction) Protected() bool {
 func (tx *EthTransaction) Copy() *EthTransaction {
 	var tx2 EthTransaction
 	tx2.data.CopyFrom(&tx.data)
+	tx2.time = tx.time
 	return &tx2
 }
 
@@ -205,6 +215,8 @@ func (tx *EthTransaction) ConvertToHmy() *Transaction {
 	copy := tx2.Hash()
 	d2.Hash = &copy
 
+	tx2.time = tx.time
+
 	return &tx2
 }
 
@@ -219,6 +231,7 @@ func (tx *EthTransaction) 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

core/types/transaction.go

@@ -23,6 +23,7 @@ import (
 	"io"
 	"math/big"
 	"sync/atomic"
+	"time"
 
 	"github.com/harmony-one/harmony/internal/params"
 
@@ -100,6 +101,9 @@ type Transaction struct {
 	hash atomic.Value
 	size atomic.Value
 	from atomic.Value
+	// time at which the node received the tx
+	// and not the time set by the sender
+	time time.Time
 }
 
 // String print mode string
@@ -224,7 +228,7 @@ func newTransaction(nonce uint64, to *common.Address, shardID uint32, amount *bi
 		d.Price.Set(gasPrice)
 	}
 
-	return &Transaction{data: d}
+	return &Transaction{data: d, time: time.Now()}
 }
 
 func newCrossShardTransaction(nonce uint64, to *common.Address, shardID uint32, toShardID uint32, amount *big.Int, gasLimit uint64, gasPrice *big.Int, data []byte) *Transaction {
@@ -251,7 +255,7 @@ func newCrossShardTransaction(nonce uint64, to *common.Address, shardID uint32,
 		d.Price.Set(gasPrice)
 	}
 
-	return &Transaction{data: d}
+	return &Transaction{data: d, time: time.Now()}
 }
 
 // From returns the sender address of the transaction
@@ -309,6 +313,11 @@ func (tx *Transaction) ToShardID() uint32 {
 	return tx.data.ToShardID
 }
 
+// Time returns the time at which the transaction was received by the node
+func (tx *Transaction) Time() time.Time {
+	return tx.time
+}
+
 // Protected returns whether the transaction is protected from replay protection.
 func (tx *Transaction) Protected() bool {
 	return isProtectedV(tx.data.V)
@@ -334,6 +343,7 @@ 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
@@ -448,6 +458,8 @@ func (tx *Transaction) ConvertToEth() *EthTransaction {
 	copy := tx2.Hash()
 	d2.Hash = &copy
 
+	tx2.time = tx.time
+
 	return &tx2
 }
 
@@ -500,6 +512,7 @@ func (tx *Transaction) RawSignatureValues() (*big.Int, *big.Int, *big.Int) {
 func (tx *Transaction) Copy() *Transaction {
 	var tx2 Transaction
 	tx2.data.CopyFrom(&tx.data)
+	tx2.time = tx.time
 	return &tx2
 }
 
@@ -550,12 +563,40 @@ func (s *TxByPrice) Pop() interface{} {
 	return x
 }
 
+// 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 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 cmp > 0
+}
+func (s TxByPriceAndTime) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
+
+func (s *TxByPriceAndTime) Push(x interface{}) {
+	*s = append(*s, x.(*Transaction))
+}
+
+func (s *TxByPriceAndTime) Pop() interface{} {
+	old := *s
+	n := len(old)
+	x := old[n-1]
+	*s = old[0 : n-1]
+	return x
+}
+
 // TransactionsByPriceAndNonce represents a set of transactions that can return
 // transactions in a profit-maximizing sorted order, while supporting removing
 // 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
 	ethSigner Signer                          // Signer for the set of transactions
 }
@@ -567,7 +608,7 @@ type TransactionsByPriceAndNonce struct {
 // if after providing it to the constructor.
 func NewTransactionsByPriceAndNonce(hmySigner Signer, ethSigner Signer, txs map[common.Address]Transactions) *TransactionsByPriceAndNonce {
 	// Initialize a price based heap with the head transactions
-	heads := make(TxByPrice, 0, len(txs))
+	heads := make(TxByPriceAndTime, 0, len(txs))
 	for from, accTxs := range txs {
 		if accTxs.Len() == 0 {
 			continue

core/types/transaction_test.go

@@ -21,9 +21,11 @@ import (
 	"encoding/json"
 	"math/big"
 	"testing"
+	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/harmony-one/harmony/internal/params"
 )
 
 func defaultTestKey() (*ecdsa.PrivateKey, common.Address) {
@@ -133,3 +135,56 @@ func TestTransactionJSON(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{}, 0, 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
+	config := params.TestChainConfig
+	txset := NewTransactionsByPriceAndNonce(
+		NewEIP155Signer(config.ChainID),
+		NewEIP155Signer(config.EthCompatibleChainID),
+		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)
+			}
+		}
+	}
+}