Skip to content

p2p tx broadcasting improvements and some other changes#2052

Open
ImplOfAnImpl wants to merge 10 commits intoupdate_rust_editionfrom
p2p_improve_tx_broadcasting
Open

p2p tx broadcasting improvements and some other changes#2052
ImplOfAnImpl wants to merge 10 commits intoupdate_rust_editionfrom
p2p_improve_tx_broadcasting

Conversation

@ImplOfAnImpl
Copy link
Copy Markdown
Contributor

  • exponential_rand's implementation was slightly wrong - it used a distribution that could produce zero and then manually checked for zero replacing it with f64::MIN_POSITIVE, thus introducing an artificially high upper bound on what exponential_rand can return (the actual minimum non-zero value that the f64 uniform distributions can produce is f64::EPSILON/2, which is much bigger than f64::MIN_POSITIVE). So I replaced the usage of the StandardUniform distribution (used by Rng::random) with Open01, which can't produce zero. The max result of exponential_rand is now about 36, so I removed the MAX_DELAY_FACTOR constant in peer manager that was used to clamp the result.

  • I renamed some event types in mempool, e.g. TransactionProcessed is now TransactionProcessedEvent, NewTip is NewTipEvent etc.

  • When a duplicate tx is added to the mempool and it has local origin, TransactionProcessedEvent will now be sent for it again. P2p will re-announce such a tx if its relayble.

  • Tx announcement was revamped - previously we would generate a random delay for each individual tx announcement and the txs would be announced in the order defined by the generated delays. This had 2 problems - a guaranteed lag before each tx and the arbitrary order of tx announcements. Now PeerTransactionSyncManager will collect all tx ids that must be announced into a set and announce them all in one batch, ordering them according to the mempool's understanding of what tx is best. The interval between the batch announcements is still chosen at random, but the base delay now depends on the connection direction - it's 2s for outbound connections and 5s for inbound ones (same values are used in bitcoin).

  • SyncManager now tracks txs with local origin that have been announced but not sent to any peer and queues them for re-announcement every few minutes.

  • The delays that SyncManager and PeerTransactionSyncManager generate are based on the monotonic clock (std::time::Instant). Normally, tokio::time::advance should be used to manipulate this time in tests; unfortunately it only works with the current_thread runtime and the tests need the multithreaded one, so I had to introduce a MonotonicTimeGetter in addition to the ordinary TimeGetter that we had.

…t will be broadcast to the peers again. Rename mempool event types.
…r tx relay delay for outbound connections. Replace "inbound: bool" with "direction: ConnectionDirection" in P2pEvent for consistency.
… instead of adding a random delay before each tx). Track which txs with local origin were broadcast but not sent, and rebroadcast them.
…ime_to_announce_txs are now delayed at the start.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant