From f6caa45fa5b9551bf884c86396ff04f8cb2c7f95 Mon Sep 17 00:00:00 2001 From: MaxMustermann2 <82761650+MaxMustermann2@users.noreply.github.com> Date: Mon, 4 Apr 2022 22:06:37 +0000 Subject: [PATCH] [core] fix: Use time-based ordering to avoid spam Closes harmony-one/harmony#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 | 15 +++++++++- core/types/transaction.go | 49 +++++++++++++++++++++++++++--- core/types/transaction_test.go | 55 ++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 5 deletions(-) diff --git a/core/types/eth_transaction.go b/core/types/eth_transaction.go index 77b721880e..82e4a58805 100644 --- a/core/types/eth_transaction.go +++ b/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,7 +117,7 @@ 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 @@ -121,6 +125,11 @@ 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 = © + 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 diff --git a/core/types/transaction.go b/core/types/transaction.go index 8863a33aa3..bee69a393f 100644 --- a/core/types/transaction.go +++ b/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 = © + 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 diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index 8b17fa3a7b..dd2f1c891e 100644 --- a/core/types/transaction_test.go +++ b/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) + } + } + } +}