From c1740e454015e61e8207cdeb34c48eee3adbb4d3 Mon Sep 17 00:00:00 2001
From: Mark <markya0616@gmail.com>
Date: Thu, 7 Sep 2017 16:22:27 -0500
Subject: [PATCH] core/types, miner: avoid tx sender miscaching (#14773)
---
core/types/transaction.go | 22 ++++++++++++----------
core/types/transaction_test.go | 2 +-
miner/worker.go | 4 ++--
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/core/types/transaction.go b/core/types/transaction.go
index 947fc85d6..7f54860fc 100644
--- a/core/types/transaction.go
+++ b/core/types/transaction.go
@@ -381,28 +381,32 @@ func (s *TxByPrice) Pop() interface{} {
// transactions in a profit-maximising 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)
+ txs map[common.Address]Transactions // Per account nonce-sorted list of transactions
+ heads TxByPrice // Next transaction for each unique account (price heap)
+ signer Signer // Signer for the set of transactions
}
// NewTransactionsByPriceAndNonce creates a transaction set that can retrieve
// price sorted transactions in a nonce-honouring way.
//
// Note, the input map is reowned so the caller should not interact any more with
-// if after providng it to the constructor.
-func NewTransactionsByPriceAndNonce(txs map[common.Address]Transactions) *TransactionsByPriceAndNonce {
+// 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))
- for acc, accTxs := range txs {
+ for _, accTxs := range txs {
heads = append(heads, accTxs[0])
+ // Ensure the sender address is from the signer
+ acc, _ := Sender(signer, accTxs[0])
txs[acc] = accTxs[1:]
}
heap.Init(&heads)
// Assemble and return the transaction set
return &TransactionsByPriceAndNonce{
- txs: txs,
- heads: heads,
+ txs: txs,
+ heads: heads,
+ signer: signer,
}
}
@@ -416,9 +420,7 @@ func (t *TransactionsByPriceAndNonce) Peek() *Transaction {
// Shift replaces the current best head with the next one from the same account.
func (t *TransactionsByPriceAndNonce) Shift() {
- signer := deriveSigner(t.heads[0].data.V)
- // derive signer but don't cache.
- acc, _ := Sender(signer, t.heads[0]) // we only sort valid txs so this cannot fail
+ acc, _ := Sender(t.signer, t.heads[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)
diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go
index edbe171e6..df9d7ffd1 100644
--- a/core/types/transaction_test.go
+++ b/core/types/transaction_test.go
@@ -143,7 +143,7 @@ func TestTransactionPriceNonceSort(t *testing.T) {
}
}
// Sort the transactions and cross check the nonce ordering
- txset := NewTransactionsByPriceAndNonce(groups)
+ txset := NewTransactionsByPriceAndNonce(signer, groups)
txs := Transactions{}
for {
diff --git a/miner/worker.go b/miner/worker.go
index e1154ac06..2919d4a48 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -268,7 +268,7 @@ func (self *worker) update() {
self.currentMu.Lock()
acc, _ := types.Sender(self.current.signer, ev.Tx)
txs := map[common.Address]types.Transactions{acc: {ev.Tx}}
- txset := types.NewTransactionsByPriceAndNonce(txs)
+ txset := types.NewTransactionsByPriceAndNonce(self.current.signer, txs)
self.current.commitTransactions(self.mux, txset, self.chain, self.coinbase)
self.currentMu.Unlock()
@@ -471,7 +471,7 @@ func (self *worker) commitNewWork() {
log.Error("Failed to fetch pending transactions", "err", err)
return
}
- txs := types.NewTransactionsByPriceAndNonce(pending)
+ txs := types.NewTransactionsByPriceAndNonce(self.current.signer, pending)
work.commitTransactions(self.mux, txs, self.chain, self.coinbase)
// compute uncles for the new block.
--
GitLab