💬 achow101 commented on pull request "test: Disable legacy wallet for mempool_packages.py":
(https://github.com/bitcoin/bitcoin/pull/27735#issuecomment-1561578795)
Is there a reason that this test needs the wallet in the first place?

💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1204513106)
> -DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_INCLUDE_TESTS=OFF to reduce CPU?

These two control the generation of build targets, however nothing should be getting compiled, as `LLVM_BUILD_TESTS` and `LLVM_BUILD_BENCHMARKS` [both default to `OFF`](https://llvm.org/docs/CMake.html). However I've also added some additional `-DLLVM_INCLUDE_*` options that should reduce compilation.

> -DLLVM_USE_LINKER=lld and/or -DLLVM_PARALLEL_LINK_JOBS=1 to reduce change of OOM?

I could follow up with a change
...

💬 achow101 commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#discussion_r1204518144)
AFAIK there aren't any other external signer software yet.

💬 achow101 commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1561612282)
ACK e3d02f8ef8cfe1d3c502ede863d14f82212a0ce6

👋 brunoerg's pull request is ready for review: "net, refactor: net_processing, add `ProcessCompactBlockTxns`"
(https://github.com/bitcoin/bitcoin/pull/26969)

💬 instagibbs commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561640287)
@ajtowns

💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1204534315)
updated test and also included warning in release note

🤔 Sjors reviewed a pull request: "Parallel compact block downloads, take 3"
(https://github.com/bitcoin/bitcoin/pull/27626#pullrequestreview-1442325175)
Post merge utACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83 modulo question about missing `return`. Other comments can wait for a followup.

Conceptually this PR still relies on our node immediately reacting to an incoming compact block message. This forces us to be a bit impatient and potentially use more bandwidth than needed.

In a followup we could instead track which peers have announced a block and then request block transactions from other peers if the first peer takes too long. This co
...

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204480914)
03423f8bd12b95a06a4a9d8377e781625dd38aae This deleted comment line is still relevant when deleting the request:

```cpp
// Forget about all prior requests. If a block was already in-flight
// for a different peer, its BLOCKTXN response will be dropped.
```

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204478988)
03423f8bd12b95a06a4a9d8377e781625dd38aae: some of the above deleted comment is still relevant.

```cpp
// If the peer does not send us a block, vBlocksInFlight remains non-empty,
// causing us to timeout and disconnect.
```

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204497452)
03423f8bd12b95a06a4a9d8377e781625dd38aae could add a static assert `MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK > 1` in a followup.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894)
03423f8bd12b95a06a4a9d8377e781625dd38aae: why not `return`?

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204506528)
03423f8bd12b95a06a4a9d8377e781625dd38aae: "Give up" implies that this peer could have done something to prevent this, but that's often not the case (`Nodes sending cmpctblock messages SHOULD limit prefilledtxn to 10KB of transactions.` - BIP152 footnote 5).

```cpp
// Do not followup with `GETBLOCKTXN` to this peer,
// wait for an outbound peer to announce the compact block instead.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204514939)
d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83: maybe explain what's going on here. We send an unsolicited compact block from these peers to our node. Because the block adds to our tip the message doesn't get ignored. Once the block is successfully processed we mark the peer as high bandwidth. This is done on a first-come-first-serve basis. This results in the `[False, True, True, True]` pattern below.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204495961)
03423f8bd12b95a06a4a9d8377e781625dd38aae "on failure" -> "when missing transactions" (the word failure is ambiguous, this is different from `READ_STATUS_FAILED`)

💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561670353)
> This is really cool and I'd love to have it as an option in Bitcoin Core. However, most CoinJoin implementations are more advanced in this regard – for example, WabiSabi performs both divisions and consolidations. And it makes me wonder to which extent it's possible to decentralize coordination to the point where clients randomly take turns in doing it.
>

Yes, CoinJoin is definitely more powerful, however it has some drawbacks.
In particular, if it's possible to detect that an UTXO has
...

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204554984)
Not anymore, we completely forget we expect it from a peer now. No disconnect should occur.

💬 Sjors commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1561678286)
I compiled on cce96182ba2457335868c65dc16b081c3dee32ee, but can reproduce on master. Updated description.

💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561680006)
> It seems like a feature like this would be better to integrate into the wallet directly rather than something that is done through and managed by the GUI.

Do you mean moving some of the core logic inside the interfaces::Wallet API, or the CWallet implementation?

Yes, certainly, that could have benefits. I had to workaround certain things that weren't in the Wallet API (namely address reservation, bump fee handling etc). So that would clean up the wallet side for sure.

Since this was m
...

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204558056)
if it's 1, that's fine, it just won't allow additional slots