fix: rebroadcast txs recovered from reorg cache#3900
Conversation
After a reorg we re-applied cached transactions to the local mempool but never announced them to peers, so recovered txs could remain private to the node. Call tx_accepted for each successfully re-accepted entry so the network learns about them again. Closes mimblewimble#3489
|
|
||
| /// Re-apply cached transactions to the mempool after a reorg. | ||
| /// | ||
| /// Txs that are successfully re-accepted are also rebroadcast via the pool |
There was a problem hiding this comment.
| // Only rebroadcast txs that were actually re-accepted (already-present | ||
| // or invalid-on-new-tip txs are skipped without network spam). | ||
| if self.add_to_txpool(&entry, header).is_ok() { | ||
| self.adapter.tx_accepted(&entry); |
There was a problem hiding this comment.
tx_accepted() is fine for a single tx, but here it runs in a loop under the pool write lock. Each peer queue has at most 100 free slots and silently drops overflow while reporting success, so a reorg recovering more than SEND_CHANNEL_CAP txs can lose later announcements. Could we schedule these through a paced bulk-relay path outside the pool lock?
| assert_eq!(pool.reorg_cache.read().len(), 2); | ||
|
|
||
| // Simulate reorg: empty the pool while leaving reorg_cache intact. | ||
| pool.txpool.entries.clear(); |
There was a problem hiding this comment.
The recording adapter only counts callbacks. Could we add a real fork/reorg test for the production trigger, plus a bounded-queue adapter using SEND_CHANNEL_CAP to cover overflow? Otherwise the test misses the actual delivery failure mode.
| @@ -0,0 +1,97 @@ | |||
| // Copyright 2021 The Grin Developers | |||
| } | ||
| } | ||
|
|
||
| pub fn init_transaction_pool_with_adapter<B, P>( |
There was a problem hiding this comment.
this duplicates the full PoolConfig literal from init_transaction_pool for one caller. Could the existing helper delegate here, or both use a shared test config, so they cannot drift?
2 participants
Summary
On reorg, the reorg cache correctly re-adds previously seen txs to the local mempool, but it never rebroadcast them. Peers that did not keep those txs (or never saw them) would not learn about them until someone else re-announced.
As noted on #3489 by @antiochp: every tx successfully re-accepted from the reorg cache should also be treated as new and rebroadcast.
Closes #3489
Changes
reconcile_reorg_cache: calladapter.tx_acceptedafter a successfuladd_to_txpoolRetention period remains configurable via
reorg_cache_period(default 30 minutes). This PR focuses on the missing rebroadcast path rather than changing the default window.Test plan
cargo test -p grin_pool --test reorg_cache_rebroadcast