🤔 hebasto reviewed a pull request: "build: Introduce internal kernel library"
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1767977106)
Approach ACK 2086d1d4c3f40cce34647995ead4bff22967ffd8.

Still reviewing the 4th commit (983d0978a973a12c3128b2c8e13b73ed08155e67).

🤔 sr-gi reviewed a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765#pullrequestreview-1767980730)
tACK [dca7990](https://github.com/bitcoin/bitcoin/pull/28765/commits/dca7990c3b8b8ecb963c068b4a8a215e0c2b3cd0)

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1417583518)
Ups, I forgot to add this.

nit: Shouldn't this also be `Wtxid&` for consistency with the rest of the interface?

This is non-blocking, but I realized when reviewing the last changes to the tests

💬 dergoegge commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843194507)
> Are you sure on this? I don't know how afl or fuzz-introspector detect reachable code paths, but I'd be surprised if the static analysis can follow a function pointer hidden in a map.

Yea maybe not... should be quite easy to avoid the map.

I'll try to setup a fuzz-introspector instance at some point, I have a feeling it'll still be a while before we can use oss-fuzz's instance.

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843211066)
Hmm, playing around with this code on godbolt, it hardly ever compiles down to bswaps. I'm not sure what changed from when I was initially investigating. Will have a look.

💬 dergoegge commented on pull request "fuzz: p2p: Detect peer deadlocks":
(https://github.com/bitcoin/bitcoin/pull/29009#issuecomment-1843215697)
> Happy to review a pull request, or happy to include any patch here, that compiles, if someone writes it.

feel free to pick https://github.com/dergoegge/bitcoin/commit/9f265d88253ed464413dea5614fa13dea0d8cfd5

👍 dergoegge approved a pull request: "fuzz: Avoid timeout in bitdeque"
(https://github.com/bitcoin/bitcoin/pull/29012#pullrequestreview-1768029847)
utACK fad1903b8a85506378101c1f857ba47b4a058fb4

💬 achow101 commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#issuecomment-1843222722)
ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417608844)
Feels like we are talking past each other a bit here. My point is that for developers, we already have tracepoints as a tool for debugging and observability into low-level details of how the software is working. In many cases, a developer will try to reproduce an issue reported by a user on their own node, at which point debuggers and tracepoints are going to be more useful than logs.

For node runners, if someone provided us with this log I don't see how having this would have helped any of u
...

💬 achow101 commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417614132)
The logs are useful for figuring out what happened when a user asks about why their wallet chose a crazy number of UTXOs when one would have been sufficient. As we add more algos, being able to see in the logs what algo was used, and what the resulting waste score was, is helpful for investigating such questions.

🚀 achow101 merged a pull request: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581)

👍 pablomartin4btc approved a pull request: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581#pullrequestreview-1768047102)
ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3

👍 brunoerg approved a pull request: "fuzz: Avoid timeout in bitdeque"
(https://github.com/bitcoin/bitcoin/pull/29012#pullrequestreview-1768071879)
crACK fad1903b8a85506378101c1f857ba47b4a058fb4

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417638654)
> wallet chose a crazy number of UTXOs when one would have been sufficient

I don't see how this example is relevant to this PR, which is why I think the logging discussion would be better moved to a logging PR.

This also presumes the user already had the logging enabled before the issue happened. I'm not sure the costs outweigh the benefits of having this log always on to be able to answer questions like your example.

💬 luke-jr commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1843276390)
Hypothetical future issues is not an excuse to stagnate harmfully. You're making the perfect the enemy of the good.

💬 achow101 commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417656406)
This log line is on by default.

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417662670)
Again, not convinced having this log on all the time is worth it, and don't see how this log is relevant to this PR.

💬 ismaelsadeeq commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1843292726)
Concept ACK
Did a light review this looks good to me

💬 maflcko commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1843313196)
Building 17.0.5 failed for me on riscv64: `/var/log/guix/drvs/9k/xxqy3shpf5b3ajmw1ar2zrbm2a8pyp-llvm-17.0.5.drv.gz`:

```
...
[ 93%] Linking CXX executable ../../bin/dsymutil
cd /tmp/guix-build-llvm-17.0.5.drv-0/source/build/tools/dsymutil && /gnu/store/v9d5kgjpkcgfwc5k1jsq8a6i2qxmp9fw-cmake-minimal-3.24.2/bin/cmake -E cmake_link_script CMakeFiles/dsymutil.dir/link.txt --verbose=1
/gnu/store/dcrdgy1rjkh0kibpkw6v2mia7f921b27-gcc-11.3.0/bin/c++ -fPIC -fno-semantic-interposition -fvisibility
...

🚀 fanquake merged a pull request: "fuzz: Avoid timeout in bitdeque"
(https://github.com/bitcoin/bitcoin/pull/29012)