💬 ajtowns commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/27630#issuecomment-1547072913)
> In the presence of smaller transactions on the network, blocks can sustain a higher relay rate than 7tx/second.
I agree this limit is too small after segwit. I think plausible datapoints for the rate at which we can sustain confirmable txs across the network might be:
* pre-segwit, 2-in, 2-out p2pkh: 374 vb, ~4.5 tx/s
* 2-in, 2-out p2wpkh: 208.5 vb, ~8 tx/s
* pre-segwit, 1-in, 1-out p2pkh: 192 vb, ~8.5 tx/s
* 1-in, 1-out p2wpkh: 109.5 vb, ~15 tx/s
* 1-in p2tr, 1-out p2wpkh:
...
(https://github.com/bitcoin/bitcoin/pull/27630#issuecomment-1547072913)
> In the presence of smaller transactions on the network, blocks can sustain a higher relay rate than 7tx/second.
I agree this limit is too small after segwit. I think plausible datapoints for the rate at which we can sustain confirmable txs across the network might be:
* pre-segwit, 2-in, 2-out p2pkh: 374 vb, ~4.5 tx/s
* 2-in, 2-out p2wpkh: 208.5 vb, ~8 tx/s
* pre-segwit, 1-in, 1-out p2pkh: 192 vb, ~8.5 tx/s
* 1-in, 1-out p2wpkh: 109.5 vb, ~15 tx/s
* 1-in p2tr, 1-out p2wpkh:
...
💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193429237)
I was trying to say that if any false negative or false positive arises as a result of a mempool message in this pull request, the same should already be observable (on master or releases) in the absence of a mempool message with normal tx relay? So addressing it here seems unrelated? I presume #27630 is addressing it.
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193429237)
I was trying to say that if any false negative or false positive arises as a result of a mempool message in this pull request, the same should already be observable (on master or releases) in the absence of a mempool message with normal tx relay? So addressing it here seems unrelated? I presume #27630 is addressing it.
💬 vasild commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1547398410)
One more thing to consider - this PR is a followup to another one (not an out of the blue, standalone one). We use the will-do-in-a-followup practice to avoid invalidating ACKs on the original PR with non-critical changes. If such followup PRs start getting frowned upon or ignored, then that could create a backward pressure to insist to include the small/non-critical suggestions in the original PR because the followup PR will not make it.
@stratospher, @mzumsande, maybe you want to review thi
...
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1547398410)
One more thing to consider - this PR is a followup to another one (not an out of the blue, standalone one). We use the will-do-in-a-followup practice to avoid invalidating ACKs on the original PR with non-critical changes. If such followup PRs start getting frowned upon or ignored, then that could create a backward pressure to insist to include the small/non-critical suggestions in the original PR because the followup PR will not make it.
@stratospher, @mzumsande, maybe you want to review thi
...
📝 MarcoFalke opened a pull request: "doc: Remove unused NO_BLOOM_VERSION constant"
(https://github.com/bitcoin/bitcoin/pull/27657)
This source code is the wrong place to document historic and now irrelevant details. Also, while touching the docs, clarify that the BIP 35 `mempool` message type is currently also guarded by the BIP 111 `NODE_BLOOM` flag, even though BIP 111 does not mention the `mempool` message type.
(https://github.com/bitcoin/bitcoin/pull/27657)
This source code is the wrong place to document historic and now irrelevant details. Also, while touching the docs, clarify that the BIP 35 `mempool` message type is currently also guarded by the BIP 111 `NODE_BLOOM` flag, even though BIP 111 does not mention the `mempool` message type.
💬 MarcoFalke commented on pull request "doc: clarify processing of mempool-msgs when NODE_BLOOM":
(https://github.com/bitcoin/bitcoin/pull/27559#discussion_r1193500972)
#27657
(https://github.com/bitcoin/bitcoin/pull/27559#discussion_r1193500972)
#27657
👍 vasild approved a pull request: "Raise on invalid -debug and -loglevel config options"
(https://github.com/bitcoin/bitcoin/pull/27632#pullrequestreview-1426041286)
ACK 9c74c94714465ac66d82a7e233312d188c9b2841
(https://github.com/bitcoin/bitcoin/pull/27632#pullrequestreview-1426041286)
ACK 9c74c94714465ac66d82a7e233312d188c9b2841
📝 hebasto opened a pull request: "test: Drop `deadlock:libdb` TSan suppression"
(https://github.com/bitcoin/bitcoin/pull/27658)
I'm not able to reproduce the TSan's deadlock warning for `libdb` for `24.x`, `25.x` and master branches neither in CI nor in different local environments (Ubuntu, Fedora).
If Tsan complains again, we can obtain a call stack from TSan, and this change can be easily reverted back.
Noticed while reviewing https://github.com/bitcoin/bitcoin/pull/27556.
(https://github.com/bitcoin/bitcoin/pull/27658)
I'm not able to reproduce the TSan's deadlock warning for `libdb` for `24.x`, `25.x` and master branches neither in CI nor in different local environments (Ubuntu, Fedora).
If Tsan complains again, we can obtain a call stack from TSan, and this change can be easily reverted back.
Noticed while reviewing https://github.com/bitcoin/bitcoin/pull/27556.
🤔 vasild reviewed a pull request: "p2p: Increase tx relay rate"
(https://github.com/bitcoin/bitcoin/pull/27630#pullrequestreview-1426068855)
Concept ACK on the increase.
I don't feel confident enough to judge the exact numbers proposed.
(https://github.com/bitcoin/bitcoin/pull/27630#pullrequestreview-1426068855)
Concept ACK on the increase.
I don't feel confident enough to judge the exact numbers proposed.
💬 0xB10C commented on pull request "doc: Remove unused NO_BLOOM_VERSION constant":
(https://github.com/bitcoin/bitcoin/pull/27657#issuecomment-1547444768)
ACK facbcd3
(https://github.com/bitcoin/bitcoin/pull/27657#issuecomment-1547444768)
ACK facbcd3
💬 ajtowns commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193531102)
The scenario here is:
* issue MEMPOOL with a large , returning 6000 txs from the mempool, all of which are added to the bloom filter, which then causes the first ~750 or more to be removed from the rolling bloom filter as it rolls over
* request one of those txs via GETDATA, which then fails if the tx hasn't been in the mempool for more than 2 minutes
In normal relay, over the course of two minutes we'd only advertise ~840 txs (~2100 for an outbound), and only add the txs to the bloom
...
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193531102)
The scenario here is:
* issue MEMPOOL with a large , returning 6000 txs from the mempool, all of which are added to the bloom filter, which then causes the first ~750 or more to be removed from the rolling bloom filter as it rolls over
* request one of those txs via GETDATA, which then fails if the tx hasn't been in the mempool for more than 2 minutes
In normal relay, over the course of two minutes we'd only advertise ~840 txs (~2100 for an outbound), and only add the txs to the bloom
...
💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193547718)
Whoops. I was incorrectly assuming that a reply to a `mempool` message was rate-limited and size-limited. (Which on a second thought, probably doesn't make sense to do). So a single reply can hold up to 50'000 inv entries, which will certainly overflow. Sorry for the distraction.
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193547718)
Whoops. I was incorrectly assuming that a reply to a `mempool` message was rate-limited and size-limited. (Which on a second thought, probably doesn't make sense to do). So a single reply can hold up to 50'000 inv entries, which will certainly overflow. Sorry for the distraction.
💬 kroese commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1547489470)
I have run into an issue where I cannot verify the signatures for test releases (like v24rc3 or v25rc2). When I execute:
`gpg --verify SHA256SUMS.asc SHA256SUMS`
It returns exitcode 2, while on the final releases it works fine.
What could be causing this behaviour? Or is it to be expected that test releases are not signed?
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1547489470)
I have run into an issue where I cannot verify the signatures for test releases (like v24rc3 or v25rc2). When I execute:
`gpg --verify SHA256SUMS.asc SHA256SUMS`
It returns exitcode 2, while on the final releases it works fine.
What could be causing this behaviour? Or is it to be expected that test releases are not signed?
⚠️ GregTonoski opened an issue: ""Create Unsigned" should not show the message: "The amount exceeds you balance" without suggesting alternatives"
(https://github.com/bitcoin/bitcoin/issues/27659)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
"The amount exceeds you balance." error message after clicking "Create Unsigned" button even though there are funds on an address.

### Expected behaviour
A message about steps to workaround or solve the issue, e.g. "Balance must be above 0 in order to create PSBT. Do you want to s
...
(https://github.com/bitcoin/bitcoin/issues/27659)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
"The amount exceeds you balance." error message after clicking "Create Unsigned" button even though there are funds on an address.

### Expected behaviour
A message about steps to workaround or solve the issue, e.g. "Balance must be above 0 in order to create PSBT. Do you want to s
...
💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193582239)
Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.
<img src=https://user-images.githubusercontent.com/19157360/235448790-6a1448bd-64b3-4c41-89d3-1ca5bf0cf76d.png></img>
Image stolen from the comment https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529678174 by 0xB10C .
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193582239)
Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.
<img src=https://user-images.githubusercontent.com/19157360/235448790-6a1448bd-64b3-4c41-89d3-1ca5bf0cf76d.png></img>
Image stolen from the comment https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529678174 by 0xB10C .
📝 fanquake opened a pull request: "[24.1] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27660)
Final changes for `v24.1`.
PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
(https://github.com/bitcoin/bitcoin/pull/27660)
Final changes for `v24.1`.
PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
👍 hebasto approved a pull request: "[24.1] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426164333)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426164333)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187, I have reviewed the code and it looks OK.
👍 TheCharlatan approved a pull request: "ci: Remove CI_EXEC bloat"
(https://github.com/bitcoin/bitcoin/pull/27616#pullrequestreview-1426207043)
ACK fa01c3c59cbe28be0751c2956609907ecfbcbe49
(https://github.com/bitcoin/bitcoin/pull/27616#pullrequestreview-1426207043)
ACK fa01c3c59cbe28be0751c2956609907ecfbcbe49
🚀 fanquake merged a pull request: "ci: Remove CI_EXEC bloat"
(https://github.com/bitcoin/bitcoin/pull/27616)
(https://github.com/bitcoin/bitcoin/pull/27616)
👍 stickies-v approved a pull request: "[24.1] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426234284)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426234284)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187
🚀 fanquake merged a pull request: "build, doc: Adjust comment after PR27254"
(https://github.com/bitcoin/bitcoin/pull/27656)
(https://github.com/bitcoin/bitcoin/pull/27656)