💬 stickies-v commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1204065433)
nit: Do we need this?

💬 stickies-v commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1204082373)
nit: could use `warn()` to avoid having an identical fn and param name (also more intuitive for a fn name imo, but that's personal)

💬 stickies-v commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1204148265)
nit: I think this include is no longer necessary

💬 stickies-v commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1204278405)
nit: I think adding line number is unmaintainable, would suggest leaving it out (+ a few lines down). Even the filename is probably not even necessary, but less of a problem.

💬 achow101 commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561460052)
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.

🚀 achow101 merged a pull request: "rpc: Fix invalid bech32 address handling"
(https://github.com/bitcoin/bitcoin/pull/27727)

💬 achow101 commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1561508601)
ACK ac8d72668c7bbd9f8771442fe25fd37f08c3f5ae

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204429122)
cce96182ba2457335868c65dc16b081c3dee32ee: the goal of the above loop seems to be to find one entry: the `vBlocksInFlight` entry for this particular block and `from_peer`. A followup could move that into its own function, and/or have the subsequent operations on the entry live _outside_ the loop.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204472811)
It ended up in 03423f8bd12b95a06a4a9d8377e781625dd38aae

💬 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.