Repository: ethereum/go-ethereum (language: go)

Issue:
TxTracker could miss the pending tx removed by txpool after tx tracker discards the txs under pendingNonce.
☝️Assume that my address's pending tx with nonce `N`, and my nonce in LegacyPool#pendingNonces should be`N+1`, and TxTracker has my nonce N transaction. 

✌️Here is the situation that TxTracker loop and call the recheck func below, assume that `sender` is my address and code `tracker.pool.Nonce(sender)` going to return the value `N+1`. And then `txs.Forward(tracker.pool.Nonce(sender))` is going to delete my nonce `N` transaction, right? 

```go
// recheck checks and returns any transactions that needs to be resubmitted.
func (tracker *TxTracker) recheck(journalCheck bool) (resubmits []*types.Transaction, rejournal map[common.Address]types.Transactions) {
	tracker.mu.Lock()
	defer tracker.mu.Unlock()

	var (
		numStales = 0
		numOk     = 0
	)
	for sender, txs := range tracker.byAddr {
		// Wipe the stales
		stales := txs.Forward(tracker.pool.Nonce(sender))
		for _, tx := range stales {
			delete(tracker.all, tx.Hash())
		}
// ......
	}
}
```

🙋Here is the key point, if the legacy pool removes my pending tx with nonce `N` after TxTracker delete the `N` tx tracked, this nonce N tx from my address is missed and I have to resend it. Is this right? please, I really need help🙏.


Discussion / hints:
🙋 Can TxTracker ensure that transactions tracked are included in a block? At least ensure that tx has been broadcast to network 🤔

For now from code, TxTracker can only ensure that tx has been put into legacy pool's pending queue, right?🤔

Fix patch:
diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go
index 3c00699dc70d..0f04193f8c29 100644
--- a/core/txpool/txpool.go
+++ b/core/txpool/txpool.go
@@ -466,9 +466,9 @@ func (p *TxPool) SubscribeTransactions(ch chan<- core.NewTxsEvent, reorgs bool)
 	return p.subs.Track(event.JoinSubscriptions(subs...))
 }
 
-// Nonce returns the next nonce of an account, with all transactions executable
+// PoolNonce returns the next nonce of an account, with all transactions executable
 // by the pool already applied on top.
-func (p *TxPool) Nonce(addr common.Address) uint64 {
+func (p *TxPool) PoolNonce(addr common.Address) uint64 {
 	// Since (for now) accounts are unique to subpools, only one pool will have
 	// (at max) a non-state nonce. To avoid stateful lookups, just return the
 	// highest nonce for now.
@@ -481,6 +481,15 @@ func (p *TxPool) Nonce(addr common.Address) uint64 {
 	return nonce
 }
 
+// Nonce returns the next nonce of an account at the current chain head. Unlike
+// PoolNonce, this function does not account for pending executable transactions.
+func (p *TxPool) Nonce(addr common.Address) uint64 {
+	p.stateLock.RLock()
+	defer p.stateLock.RUnlock()
+
+	return p.state.GetNonce(addr)
+}
+
 // Stats retrieves the current pool stats, namely the number of pending and the
 // number of queued (non-executable) transactions.
 func (p *TxPool) Stats() (int, int) {
diff --git a/eth/api_backend.go b/eth/api_backend.go
index b08e38afe5d6..c8c5ca707f94 100644
--- a/eth/api_backend.go
+++ b/eth/api_backend.go
@@ -327,7 +327,7 @@ func (b *EthAPIBackend) GetTransaction(ctx context.Context, txHash common.Hash)
 }
 
 func (b *EthAPIBackend) GetPoolNonce(ctx context.Context, addr common.Address) (uint64, error) {
-	return b.eth.txPool.Nonce(addr), nil
+	return b.eth.txPool.PoolNonce(addr), nil
 }
 
 func (b *EthAPIBackend) Stats() (runnable int, blocked int) {


Test patch:
diff --git a/core/txpool/locals/tx_tracker_test.go b/core/txpool/locals/tx_tracker_test.go
index cb6b9b345354..5585589b6cdd 100644
--- a/core/txpool/locals/tx_tracker_test.go
+++ b/core/txpool/locals/tx_tracker_test.go
@@ -108,6 +108,19 @@ func (env *testEnv) makeTx(nonce uint64, gasPrice *big.Int) *types.Transaction {
 	return tx
 }
 
+func (env *testEnv) makeTxs(n int) []*types.Transaction {
+	head := env.chain.CurrentHeader()
+	state, _ := env.chain.StateAt(head.Root)
+	nonce := state.GetNonce(address)
+
+	var txs []*types.Transaction
+	for i := 0; i < n; i++ {
+		tx, _ := types.SignTx(types.NewTransaction(nonce+uint64(i), common.Address{0x00}, big.NewInt(1000), params.TxGas, big.NewInt(params.GWei), nil), signer, key)
+		txs = append(txs, tx)
+	}
+	return txs
+}
+
 func (env *testEnv) commit() {
 	head := env.chain.CurrentBlock()
 	block := env.chain.GetBlock(head.Hash(), head.Number.Uint64())
@@ -177,3 +190,29 @@ func TestRejectInvalids(t *testing.T) {
 		}
 	}
 }
+
+func TestResubmit(t *testing.T) {
+	env := newTestEnv(t, 10, 0, "")
+	defer env.close()
+
+	txs := env.makeTxs(10)
+	txsA := txs[:len(txs)/2]
+	txsB := txs[len(txs)/2:]
+	env.pool.Add(txsA, true)
+	pending, queued := env.pool.ContentFrom(address)
+	if len(pending) != len(txsA) || len(queued) != 0 {
+		t.Fatalf("Unexpected txpool content: %d, %d", len(pending), len(queued))
+	}
+	env.tracker.TrackAll(txs)
+
+	resubmit, all := env.tracker.recheck(true)
+	if len(resubmit) != len(txsB) {
+		t.Fatalf("Unexpected transactions to resubmit, got: %d, want: %d", len(resubmit), len(txsB))
+	}
+	if len(all) == 0 || len(all[address]) == 0 {
+		t.Fatalf("Unexpected transactions being tracked, got: %d, want: %d", 0, len(txs))
+	}
+	if len(all[address]) != len(txs) {
+		t.Fatalf("Unexpected transactions being tracked, got: %d, want: %d", len(all[address]), len(txs))
+	}
+}