Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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_r1191211092)
> This is sometimes good for fingerprinting

fingerprinting seems bad, so another reason for removal?

> I don't think we can remove mapRelay -- we need it so we can continue to relay recently announced txs that we've removed from the mempool?

Sure that is what it does, but if the only use case is for spy nodes to fingerprint, it may be better to remove it?

See #27625. Feel free to NACK.
🤔 MarcoFalke reviewed a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610#pullrequestreview-1422225046)
first commit lgtm. Left one question. Didn't look too long at the second commit.
💬 MarcoFalke commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190938459)
question (feel free to ignore): I tried to figure out if there was a reason why this was re-ordered, but couldn't find one. An equivalent patch of simply changing the literals `false` to `true` and `true` to `false` in this function should have achieved the same, right?
💬 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_r1191220466)
Knowing when a tx is removed from the mempool means you know exactly when the tx that replaced it was added to the mempool... Txs are also removed from the mempool when a block comes in; if we process the block first, then see a GETDATA for a tx in the block, replying with the tx is better for block propagation (including compact blocks) that sending a NOTFOUND...
💬 sdaftuar commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1544047433)
I think the removed-for-block case is the main one we care about -- in a situation where a peer has requested a transaction where we have received a block containing the transaction in the time between announcement and relay, it seems likely that the peer is about to receive the block as well, and with the compact block protocol we may be helping our peer quite a bit (potentially saving a round trip) to deliver the transaction before the block arrives to them.
📝 instagibbs opened a pull request: "Parallel compact block downloads, take 3"
(https://github.com/bitcoin/bitcoin/pull/27626)
This is an attempt at mitigating https://github.com/bitcoin/bitcoin/issues/25258 , which is a revival of https://github.com/bitcoin/bitcoin/pull/10984, which is a revival of https://github.com/bitcoin/bitcoin/pull/9447.

This PR attempts to mitigate a single case, where high bandwidth peers can bail us out of a flakey
peer not completing blocks for us. We allow up to 2 additional getblocktxns requests per unique block.
This would hopefully allow the chance for an honest high bandwidth peer t
...
💬 hebasto commented on pull request "kernel: Remove args, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#issuecomment-1544060017)
Concept ACK.
💬 ajtowns commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191235815)
The `CompareTxMemPoolEntryByScore` comparator returns `false` on equality (ie, it's "less than" not "less than or equal"), so this preserves that behaviour.
📝 MarcoFalke converted_to_draft a pull request: "p2p: Stop relaying non-mempool txs"
(https://github.com/bitcoin/bitcoin/pull/27625)
`mapRelay` (used to relay announced transactions that are no longer in the mempool) has issues:

* It doesn't have an absolute memory limit, only an implicit one based on the rate of transaction announcements
* It doesn't have a use-case, and can be used to spy (see below)

Fix all issues by removing `mapRelay`.

For more context, on why a transaction may have been removed from the mempool, see https://github.com/bitcoin/bitcoin/blob/c2f2abd0a4f4bd18bfca41b632d21d803479f3f4/src/txmempool.
...
🤔 Sjors reviewed a pull request: "p2p: Avoid prematurely clearing download state for other peers"
(https://github.com/bitcoin/bitcoin/pull/27608#pullrequestreview-1422180036)
utACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4
💬 Sjors commented on pull request "p2p: Avoid prematurely clearing download state for other peers":
(https://github.com/bitcoin/bitcoin/pull/27608#discussion_r1190909435)
This comment could be improved:

```cpp
// When processing block related messages from our peers,
// we treat requested blocks different from unsolicited ones.
// When making this distinction we only keep track of the
// last peer we requested from.
```
💬 Sjors commented on pull request "p2p: Avoid prematurely clearing download state for other peers":
(https://github.com/bitcoin/bitcoin/pull/27608#discussion_r1191267648)
Can't we have a race condition where the block is not `new`? In any case this function is extremely fast if called twice.
🤔 glozow reviewed a pull request: "[24.1] Backports for rc3"
(https://github.com/bitcoin/bitcoin/pull/27614#pullrequestreview-1422778545)
ACK 97f5e28830dfa5786cdc5df62e245e03fa393e19
💬 dergoegge commented on pull request "[24.1] Backports for rc3":
(https://github.com/bitcoin/bitcoin/pull/27614#issuecomment-1544115933)
ACK 97f5e28830dfa5786cdc5df62e245e03fa393e19
💬 dergoegge commented on pull request "[23.2] Backports for rc1":
(https://github.com/bitcoin/bitcoin/pull/27624#issuecomment-1544118582)
ACK ce8f812b0ac0905c26edd826c57886a08079b4a7

Are you going to add https://github.com/bitcoin/bitcoin/pull/27610 to this?
👍 brunoerg approved a pull request: "[24.1] Backports for rc3"
(https://github.com/bitcoin/bitcoin/pull/27614#pullrequestreview-1422791729)
ACK 97f5e28830dfa5786cdc5df62e245e03fa393e19

Just ran the tests locally, no problem. Release notes ok.
📝 hebasto opened a pull request: "[24.x] qt: 24.1rc3 translations update"
(https://github.com/bitcoin/bitcoin/pull/27627)
This PR pulls the recent translations from the [Transifex.com](https://www.transifex.com/bitcoin/bitcoin) using the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) tool.

According to our [Release Process docs](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-every-release-candidate), it is supposed to be merged before `v24.1rc3` tagging.

Will keep this PR updated regul
...
💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1191301314)
I think it's a bit strange for policy/fees to have a dependency on scheduler. Maybe register `FlushFeeEstimates()` with the scheduler within `AppInitMain()` instead?
👍 hebasto approved a pull request: "[24.1] Backports for rc3"
(https://github.com/bitcoin/bitcoin/pull/27614#pullrequestreview-1422809452)
ACK 97f5e28830dfa5786cdc5df62e245e03fa393e19, commits were backported locally, got zero diff.
💬 achow101 commented on pull request "[24.1] Backports for rc3":
(https://github.com/bitcoin/bitcoin/pull/27614#issuecomment-1544146197)
ACK 97f5e28830dfa5786cdc5df62e245e03fa393e19