Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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_r1191170092)
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?

How about this approach: when we get a MEMPOOL message, at the next `fSendTrickle` time, for each tx that matches the filter, if it's older than 2 minutes, announce it immediately, otherwise just put it into `m_tx_inventory_to_send` and let it be sent out at the usual rate. Only use `m_recently_announced_invs` for txs relayed via `m_tx_inventory_to_send`
...
👋 fanquake's pull request is ready for review: "depends: no-longer nuke libc++abi.so* in native_clang package"
(https://github.com/bitcoin/bitcoin/pull/27493)
💬 fanquake commented on pull request "depends: no-longer nuke libc++abi.so* in native_clang package":
(https://github.com/bitcoin/bitcoin/pull/27493#issuecomment-1543996017)
Guix Build:
```bash
f05e6ab973d999e7fd8445c580bf919a6c7ffcdabbfac30cefaa8a81c1c8d804 guix-build-9ae854da193f/output/aarch64-linux-gnu/SHA256SUMS.part
4294c97d12f0c29ad9d3211fb455ac2a2b9933c13efa061b1753917a11dbbaf5 guix-build-9ae854da193f/output/aarch64-linux-gnu/bitcoin-9ae854da193f-aarch64-linux-gnu-debug.tar.gz
cf80ca7545331b69ca2c1496eab26122fcf7499e6674e9cabf168b3cfe3d10d8 guix-build-9ae854da193f/output/aarch64-linux-gnu/bitcoin-9ae854da193f-aarch64-linux-gnu.tar.gz
816d7fc4bbc6deba
...
🚀 fanquake merged a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610)
👍 brunoerg approved a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610#pullrequestreview-1422604159)
crACK 5b3406094f2679dfb3763de4414257268565b943
💬 brunoerg commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191178712)
Could we have these cases covered by a unit test?
💬 instagibbs commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1544006512)
without #26711 we are still letting things into mempool that are possibly problematic, no?
💬 willcl-ark 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_r1191189563)
```suggestion
self.log.info("Check all transactions are returned by the match-all filter")
```
💬 willcl-ark 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_r1191190564)
I think this log line could be removed as this seems part of the previous check?
💬 brunoerg commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1544020183)
@hebasto I got an error when trying to access [25.0 Draft Release Notes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Notes-draft)
💬 sr-gi 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#issuecomment-1544020927)
> > What's the proportion of announced vs non-announced transactions? How much would be left out with this patch?
>
> The PR (as it is right now) still announces everything in response to mempool. The client just cannot request extremely recent things, which will be announced normally every ~5 seconds. This is "I haven't even announced this to you yet, why are you requesting it from me? seems suspicious. Just wait 5 seconds and I'll announce it to you."
>
> A normal client, who presumably
...
💬 josibake commented on pull request "[24.1] Backports for rc3":
(https://github.com/bitcoin/bitcoin/pull/27614#issuecomment-1544027466)
ACK https://github.com/bitcoin/bitcoin/pull/27614/commits/97f5e28830dfa5786cdc5df62e245e03fa393e19

Verified that the commits added are the correct ones from the PRs linked, compiled and ran the tests. Document changes look good. Looks like the CI failures are just github being crappy
🤔 hebasto reviewed a pull request: "depends: no-longer nuke libc++abi.so* in native_clang package"
(https://github.com/bitcoin/bitcoin/pull/27493#pullrequestreview-1422646806)
ACK 9ae854da193f3c4bda38a75e96f9b989b289baab

Actually, this PR introduces no changes to the `native_clang` package at all:
```
$ git fetch origin pull/27493/head
$ git checkout FETCH_HEAD
$ make -C depends clean-all
$ make -C depends native_clang HOST=x86_64-apple-darwin
$ cat depends/built/x86_64-apple-darwin/native_clang/native_clang-10.0.1-*.tar.gz.hash
300b13fa3db495f3ad33f729a2fa2688e84d928a17ef8f4eb1164e062ad924c6 native_clang-10.0.1-a7086a951bd.tar.gz
$ git checkout HEAD~1
$
...
📝 MarcoFalke opened 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.
...
💬 glozow commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191209626)
I've written a unit test here: https://github.com/glozow/bitcoin/commits/2023-05-comparedepthandscore-test
🤔 jonatack reviewed a pull request: "p2p: Allow whitelisting outgoing connections"
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1422614006)
Approach ACK, modulo comments and https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186378760 -- it might be better for that commit to be a pure refactoring. Thanks for addressing the previous feedback.
💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191201370)
Why this change?
💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191183121)
0a77e6ab37a9b23bd293 I would suggest placing this new helper with the other ones standalone ones, i.e. after `IsLocal()` or higher, not between the `CConnman::FindNode()` class members (unless there's a reason that I didn't see).
💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191191757)
ea46500ece2
```suggestion
"Additional flags \"in\" and \"out\" control whether permissions apply to incoming connections and/or outgoing (default: %s). "
```
💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191197012)
f99912850e2a50b1a
```suggestion
# Whitelist peers to speed up tx relay / mempool sync. Don't use if testing tx relay or timing.
```