ZIP 239 preparations 1#5265
Conversation
Current master crashes on OSX with an exception: "boost: mutex lock failed in pthread_mutex_lock: Invalid argument" (cherry picked from commit 0d699fc) Zcash: Also adds the `clear` call that this was fixing. Upstream added it in bitcoin/bitcoin#6722 which we never backported (instead implementing our own mempool limiting logic).
We used to have a trickle node, a node which was chosen in each iteration of the send loop that was privileged and allowed to send out queued up non-time critical messages. Since the removal of the fixed sleeps in the network code, this resulted in fast and attackable treatment of such broadcasts. This pull request changes the 3 remaining trickle use cases by random delays: * Local address broadcast (while also removing the the wiping of the seen filter) * Address relay * Inv relay (for transactions; blocks are always relayed immediately) The code is based on older commits by Patrick Strateman. (cherry picked from commit 5400ef6)
Previously we used the CInv that would be sent to the peer announcing the transaction as the key, but using the txid instead allows us to decouple the p2p layer from the application logic (which relies on this map to avoid duplicate tx requests). (cherry picked from commit 7e91f63)
(cherry picked from commit 38c3102)
|
@zkbot try |
ZIP 239 preparations 1 This is the first of several backports to prepare for ZIP 239. The primary change is altering `mapRelay` to store `CTransaction`s, which we need because ZIP 239 requires changing `Inv` messages based on transaction versions. The other changes are mainly for conflict removal but are also independently useful. Backports the following upstream PRs: - bitcoin/bitcoin#6889 - bitcoin/bitcoin#7125 - bitcoin/bitcoin#7862 - bitcoin/bitcoin#7877
| } | ||
|
|
||
| int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds) { | ||
| return nNow + (int64_t)(log1p(GetRand(1ULL << 48) * -0.0000000000000035527136788 /* -1/2^48 */) * average_interval_seconds * -1000000.0 + 0.5); |
There was a problem hiding this comment.
I checked that this formula is correct for inverse transform sampling of an exponential distribution.
(I would call it ExponentialNextSend, but I guess it doesn't matter.)
|
💔 Test failed - pr-try |
|
CI failures were transient; this PR doesn't cause any hard test failures. |
|
@zkbot r+ |
|
📌 Commit 465c249 has been approved by |
|
⌛ Testing commit 465c249 with merge 6f389625778a56ace5bb9a28f376f3d0e8d54e9b... |
|
💔 Test failed - pr-merge |
This was previously a transient test failure, that started reliably failing after the move to Poisson delays.
|
This PR caused a transient failure to become reliable, enabling me to find the cause: we weren't calling @zkbot r+ |
|
📌 Commit 6be1a2a has been approved by |
ZIP 239 preparations 1 This is the first of several backports to prepare for ZIP 239. The primary change is altering `mapRelay` to store `CTransaction`s, which we need because ZIP 239 requires changing `Inv` messages based on transaction versions. The other changes are mainly for conflict removal but are also independently useful. Backports the following upstream PRs: - bitcoin/bitcoin#6889 - bitcoin/bitcoin#7125 - bitcoin/bitcoin#7862 - bitcoin/bitcoin#7877
|
💔 Test failed - pr-merge |
|
The race isn't reliable; I can't reproduce it locally. @zkbot retry |
|
⌛ Testing commit 6be1a2a with merge 8c3d670c8a39eda55a72dd99d8694ad50e6bf82f... |
|
💔 Test failed - pr-merge |
The recent changes to mempool inv logic mean that nodes are much less likely to immediately return an `inv` message in response to a `mempool` message. The `p2p_txexpiringsoon` RPC test was relying on the prior behaviour. `TestNode.sync_with_ping` now takes an optional `waiting_for` closure that allows the caller to require that a specific message kind is received prior to the timeout.
|
Did some digging, and I think I've figured out the race condition in @zkbot r+ |
|
📌 Commit 75b9fc4 has been approved by |
|
⌛ Testing commit 75b9fc4 with merge 1dc1ababa970f633616d0c1fc60c65fb9a9231ef... |
|
💔 Test failed - pr-merge |
|
Transient failure that I'm going to try and fix in a separate PR. For now I'll just hope this works. @zkbot retry |
|
⌛ Testing commit 75b9fc4 with merge 18fe0a377bc66e3b0f8735624be8958ef93b75be... |
|
💔 Test failed - pr-merge |
|
Failed again. We need to merge #5268 before this PR. |
|
Okay, next attempt: @zkbot retry |
This is the first of several backports to prepare for ZIP 239. The primary
change is altering
mapRelayto storeCTransactions, which we needbecause ZIP 239 requires changing
Invmessages based on transactionversions. The other changes are mainly for conflict removal but are also
independently useful.
Backports the following upstream PRs: