From f8601430fddfe44b9c1781957ab13578cf17ad9f Mon Sep 17 00:00:00 2001 From: cui <523516579@qq.com> Date: Sat, 3 Mar 2018 18:09:36 +0800 Subject: [PATCH 1/2] core: should enqueue the invalids tx anyway even the pending is empty we shoud enqueue the invalid txs --- core/tx_pool.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 0534fe57a..7d7ed334a 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -881,12 +881,13 @@ func (pool *TxPool) removeTx(hash common.Hash) { if pending.Empty() { delete(pool.pending, addr) delete(pool.beats, addr) - } else { - // Otherwise postpone any invalidated transactions - for _, tx := range invalids { - pool.enqueueTx(tx.Hash(), tx) - } } + + // Otherwise postpone any invalidated transactions + for _, tx := range invalids { + pool.enqueueTx(tx.Hash(), tx) + } + // Update the account nonce if needed if nonce := tx.Nonce(); pool.pendingState.GetNonce(addr) > nonce { pool.pendingState.SetNonce(addr, nonce) From 2b5d1a4a4cf33292267840aaf2952e2caa441447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 7 Mar 2018 10:55:59 +0200 Subject: [PATCH 2/2] core: update txpool tests for the removal fix --- core/tx_pool.go | 6 +- core/tx_pool_test.go | 159 ++++++++++++++++++++++++++++++------------- 2 files changed, 112 insertions(+), 53 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 7d7ed334a..089bd215a 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -877,17 +877,15 @@ func (pool *TxPool) removeTx(hash common.Hash) { // Remove the transaction from the pending lists and reset the account nonce if pending := pool.pending[addr]; pending != nil { if removed, invalids := pending.Remove(tx); removed { - // If no more transactions are left, remove the list + // If no more pending transactions are left, remove the list if pending.Empty() { delete(pool.pending, addr) delete(pool.beats, addr) } - - // Otherwise postpone any invalidated transactions + // Postpone any invalidated transactions for _, tx := range invalids { pool.enqueueTx(tx.Hash(), tx) } - // Update the account nonce if needed if nonce := tx.Nonce(); pool.pendingState.GetNonce(addr) > nonce { pool.pendingState.SetNonce(addr, nonce) diff --git a/core/tx_pool_test.go b/core/tx_pool_test.go index 158b9776b..1cf533aa6 100644 --- a/core/tx_pool_test.go +++ b/core/tx_pool_test.go @@ -557,74 +557,112 @@ func TestTransactionDropping(t *testing.T) { func TestTransactionPostponing(t *testing.T) { t.Parallel() - // Create a test account and fund it - pool, key := setupTxPool() + // Create the pool to test the postponing with + db, _ := ethdb.NewMemDatabase() + statedb, _ := state.New(common.Hash{}, state.NewDatabase(db)) + blockchain := &testBlockChain{statedb, 1000000, new(event.Feed)} + + pool := NewTxPool(testTxPoolConfig, params.TestChainConfig, blockchain) defer pool.Stop() - account, _ := deriveSender(transaction(0, 0, key)) - pool.currentState.AddBalance(account, big.NewInt(1000)) + // Create two test accounts to produce different gap profiles with + keys := make([]*ecdsa.PrivateKey, 2) + accs := make([]common.Address, len(keys)) + + for i := 0; i < len(keys); i++ { + keys[i], _ = crypto.GenerateKey() + accs[i] = crypto.PubkeyToAddress(keys[i].PublicKey) + pool.currentState.AddBalance(crypto.PubkeyToAddress(keys[i].PublicKey), big.NewInt(50100)) + } // Add a batch consecutive pending transactions for validation - txns := []*types.Transaction{} - for i := 0; i < 100; i++ { - var tx *types.Transaction - if i%2 == 0 { - tx = transaction(uint64(i), 100, key) - } else { - tx = transaction(uint64(i), 500, key) + txs := []*types.Transaction{} + for i, key := range keys { + + for j := 0; j < 100; j++ { + var tx *types.Transaction + if (i+j)%2 == 0 { + tx = transaction(uint64(j), 25000, key) + } else { + tx = transaction(uint64(j), 50000, key) + } + txs = append(txs, tx) + } + } + for i, err := range pool.AddRemotes(txs) { + if err != nil { + t.Fatalf("tx %d: failed to add transactions: %v", i, err) } - pool.promoteTx(account, tx.Hash(), tx) - txns = append(txns, tx) } // Check that pre and post validations leave the pool as is - if pool.pending[account].Len() != len(txns) { - t.Errorf("pending transaction mismatch: have %d, want %d", pool.pending[account].Len(), len(txns)) + if pending := pool.pending[accs[0]].Len() + pool.pending[accs[1]].Len(); pending != len(txs) { + t.Errorf("pending transaction mismatch: have %d, want %d", pending, len(txs)) } if len(pool.queue) != 0 { - t.Errorf("queued transaction mismatch: have %d, want %d", pool.queue[account].Len(), 0) + t.Errorf("queued accounts mismatch: have %d, want %d", len(pool.queue), 0) } - if len(pool.all) != len(txns) { - t.Errorf("total transaction mismatch: have %d, want %d", len(pool.all), len(txns)) + if len(pool.all) != len(txs) { + t.Errorf("total transaction mismatch: have %d, want %d", len(pool.all), len(txs)) } pool.lockedReset(nil, nil) - if pool.pending[account].Len() != len(txns) { - t.Errorf("pending transaction mismatch: have %d, want %d", pool.pending[account].Len(), len(txns)) + if pending := pool.pending[accs[0]].Len() + pool.pending[accs[1]].Len(); pending != len(txs) { + t.Errorf("pending transaction mismatch: have %d, want %d", pending, len(txs)) } if len(pool.queue) != 0 { - t.Errorf("queued transaction mismatch: have %d, want %d", pool.queue[account].Len(), 0) + t.Errorf("queued accounts mismatch: have %d, want %d", len(pool.queue), 0) } - if len(pool.all) != len(txns) { - t.Errorf("total transaction mismatch: have %d, want %d", len(pool.all), len(txns)) + if len(pool.all) != len(txs) { + t.Errorf("total transaction mismatch: have %d, want %d", len(pool.all), len(txs)) } // Reduce the balance of the account, and check that transactions are reorganised - pool.currentState.AddBalance(account, big.NewInt(-750)) + for _, addr := range accs { + pool.currentState.AddBalance(addr, big.NewInt(-1)) + } pool.lockedReset(nil, nil) - if _, ok := pool.pending[account].txs.items[txns[0].Nonce()]; !ok { - t.Errorf("tx %d: valid and funded transaction missing from pending pool: %v", 0, txns[0]) + // The first account's first transaction remains valid, check that subsequent + // ones are either filtered out, or queued up for later. + if _, ok := pool.pending[accs[0]].txs.items[txs[0].Nonce()]; !ok { + t.Errorf("tx %d: valid and funded transaction missing from pending pool: %v", 0, txs[0]) } - if _, ok := pool.queue[account].txs.items[txns[0].Nonce()]; ok { - t.Errorf("tx %d: valid and funded transaction present in future queue: %v", 0, txns[0]) + if _, ok := pool.queue[accs[0]].txs.items[txs[0].Nonce()]; ok { + t.Errorf("tx %d: valid and funded transaction present in future queue: %v", 0, txs[0]) } - for i, tx := range txns[1:] { + for i, tx := range txs[1:100] { if i%2 == 1 { - if _, ok := pool.pending[account].txs.items[tx.Nonce()]; ok { + if _, ok := pool.pending[accs[0]].txs.items[tx.Nonce()]; ok { t.Errorf("tx %d: valid but future transaction present in pending pool: %v", i+1, tx) } - if _, ok := pool.queue[account].txs.items[tx.Nonce()]; !ok { + if _, ok := pool.queue[accs[0]].txs.items[tx.Nonce()]; !ok { t.Errorf("tx %d: valid but future transaction missing from future queue: %v", i+1, tx) } } else { - if _, ok := pool.pending[account].txs.items[tx.Nonce()]; ok { + if _, ok := pool.pending[accs[0]].txs.items[tx.Nonce()]; ok { t.Errorf("tx %d: out-of-fund transaction present in pending pool: %v", i+1, tx) } - if _, ok := pool.queue[account].txs.items[tx.Nonce()]; ok { + if _, ok := pool.queue[accs[0]].txs.items[tx.Nonce()]; ok { t.Errorf("tx %d: out-of-fund transaction present in future queue: %v", i+1, tx) } } } - if len(pool.all) != len(txns)/2 { - t.Errorf("total transaction mismatch: have %d, want %d", len(pool.all), len(txns)/2) + // The second account's first transaction got invalid, check that all transactions + // are either filtered out, or queued up for later. + if pool.pending[accs[1]] != nil { + t.Errorf("invalidated account still has pending transactions") + } + for i, tx := range txs[100:] { + if i%2 == 1 { + if _, ok := pool.queue[accs[1]].txs.items[tx.Nonce()]; !ok { + t.Errorf("tx %d: valid but future transaction missing from future queue: %v", 100+i, tx) + } + } else { + if _, ok := pool.queue[accs[1]].txs.items[tx.Nonce()]; ok { + t.Errorf("tx %d: out-of-fund transaction present in future queue: %v", 100+i, tx) + } + } + } + if len(pool.all) != len(txs)/2 { + t.Errorf("total transaction mismatch: have %d, want %d", len(pool.all), len(txs)/2) } } @@ -949,11 +987,11 @@ func testTransactionLimitingEquivalency(t *testing.T, origin uint64) { account2, _ := deriveSender(transaction(0, 0, key2)) pool2.currentState.AddBalance(account2, big.NewInt(1000000)) - txns := []*types.Transaction{} + txs := []*types.Transaction{} for i := uint64(0); i < testTxPoolConfig.AccountQueue+5; i++ { - txns = append(txns, transaction(origin+i, 100000, key2)) + txs = append(txs, transaction(origin+i, 100000, key2)) } - pool2.AddRemotes(txns) + pool2.AddRemotes(txs) // Ensure the batch optimization honors the same pool mechanics if len(pool1.pending) != len(pool2.pending) { @@ -1124,7 +1162,7 @@ func TestTransactionPoolRepricing(t *testing.T) { defer sub.Unsubscribe() // Create a number of test accounts and fund them - keys := make([]*ecdsa.PrivateKey, 3) + keys := make([]*ecdsa.PrivateKey, 4) for i := 0; i < len(keys); i++ { keys[i], _ = crypto.GenerateKey() pool.currentState.AddBalance(crypto.PubkeyToAddress(keys[i].PublicKey), big.NewInt(1000000)) @@ -1136,24 +1174,28 @@ func TestTransactionPoolRepricing(t *testing.T) { txs = append(txs, pricedTransaction(1, 100000, big.NewInt(1), keys[0])) txs = append(txs, pricedTransaction(2, 100000, big.NewInt(2), keys[0])) + txs = append(txs, pricedTransaction(0, 100000, big.NewInt(1), keys[1])) txs = append(txs, pricedTransaction(1, 100000, big.NewInt(2), keys[1])) - txs = append(txs, pricedTransaction(2, 100000, big.NewInt(1), keys[1])) - txs = append(txs, pricedTransaction(3, 100000, big.NewInt(2), keys[1])) + txs = append(txs, pricedTransaction(2, 100000, big.NewInt(2), keys[1])) - ltx := pricedTransaction(0, 100000, big.NewInt(1), keys[2]) + txs = append(txs, pricedTransaction(1, 100000, big.NewInt(2), keys[2])) + txs = append(txs, pricedTransaction(2, 100000, big.NewInt(1), keys[2])) + txs = append(txs, pricedTransaction(3, 100000, big.NewInt(2), keys[2])) + + ltx := pricedTransaction(0, 100000, big.NewInt(1), keys[3]) // Import the batch and that both pending and queued transactions match up pool.AddRemotes(txs) pool.AddLocal(ltx) pending, queued := pool.Stats() - if pending != 4 { - t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 4) + if pending != 7 { + t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 7) } if queued != 3 { t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 3) } - if err := validateEvents(events, 4); err != nil { + if err := validateEvents(events, 7); err != nil { t.Fatalf("original event firing failed: %v", err) } if err := validateTxPoolInternals(pool); err != nil { @@ -1166,8 +1208,8 @@ func TestTransactionPoolRepricing(t *testing.T) { if pending != 2 { t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 2) } - if queued != 3 { - t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 3) + if queued != 5 { + t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 5) } if err := validateEvents(events, 0); err != nil { t.Fatalf("reprice event firing failed: %v", err) @@ -1179,7 +1221,10 @@ func TestTransactionPoolRepricing(t *testing.T) { if err := pool.AddRemote(pricedTransaction(1, 100000, big.NewInt(1), keys[0])); err != ErrUnderpriced { t.Fatalf("adding underpriced pending transaction error mismatch: have %v, want %v", err, ErrUnderpriced) } - if err := pool.AddRemote(pricedTransaction(2, 100000, big.NewInt(1), keys[1])); err != ErrUnderpriced { + if err := pool.AddRemote(pricedTransaction(0, 100000, big.NewInt(1), keys[1])); err != ErrUnderpriced { + t.Fatalf("adding underpriced pending transaction error mismatch: have %v, want %v", err, ErrUnderpriced) + } + if err := pool.AddRemote(pricedTransaction(2, 100000, big.NewInt(1), keys[2])); err != ErrUnderpriced { t.Fatalf("adding underpriced queued transaction error mismatch: have %v, want %v", err, ErrUnderpriced) } if err := validateEvents(events, 0); err != nil { @@ -1189,7 +1234,7 @@ func TestTransactionPoolRepricing(t *testing.T) { t.Fatalf("pool internal state corrupted: %v", err) } // However we can add local underpriced transactions - tx := pricedTransaction(1, 100000, big.NewInt(1), keys[2]) + tx := pricedTransaction(1, 100000, big.NewInt(1), keys[3]) if err := pool.AddLocal(tx); err != nil { t.Fatalf("failed to add underpriced local transaction: %v", err) } @@ -1202,6 +1247,22 @@ func TestTransactionPoolRepricing(t *testing.T) { if err := validateTxPoolInternals(pool); err != nil { t.Fatalf("pool internal state corrupted: %v", err) } + // And we can fill gaps with properly priced transactions + if err := pool.AddRemote(pricedTransaction(1, 100000, big.NewInt(2), keys[0])); err != nil { + t.Fatalf("failed to add pending transaction: %v", err) + } + if err := pool.AddRemote(pricedTransaction(0, 100000, big.NewInt(2), keys[1])); err != nil { + t.Fatalf("failed to add pending transaction: %v", err) + } + if err := pool.AddRemote(pricedTransaction(2, 100000, big.NewInt(2), keys[2])); err != nil { + t.Fatalf("failed to add queued transaction: %v", err) + } + if err := validateEvents(events, 5); err != nil { + t.Fatalf("post-reprice event firing failed: %v", err) + } + if err := validateTxPoolInternals(pool); err != nil { + t.Fatalf("pool internal state corrupted: %v", err) + } } // Tests that setting the transaction pool gas price to a higher value does not