💬 Sjors commented on pull request "test: Fix test by checking the actual exception instance":
(https://github.com/bitcoin/bitcoin/pull/28989#issuecomment-1843472589)
Looks like a reasonable fix, thanks!

💬 brunoerg commented on pull request "fuzz: p2p: Detect peer deadlocks":
(https://github.com/bitcoin/bitcoin/pull/29009#issuecomment-1843473953)
Concept ACK

💬 maflcko commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843481759)
> clang can obviously do the right thing.

Are you sure? When I tried yesterday, it did not (https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841445629)

🤔 ryanofsky reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1767990381)
Updated 8a02ce59ffa16c73611aeda6caf8b54d85c6208f -> 4a3a2651420ca1808cb25aed8a33d57dfcd627f5 ([`pr/noshut.19`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.19) -> [`pr/noshut.20`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.20), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noshut.19..pr/noshut.20)) with Qt windows bugfix

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417585924)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417160346

> Calling `app.node()` here before `app.createNode` results in a crash on windows when running `bitcoin-qt.exe`.

Great catch! This should be fixed now by passing a shutdown callback and delaying the call to app.node(). I didn't want to move the createNode call, because the interfaces::Node object is intentionally created after the splash screen, so the splash screen can show up without a delay. The delay wouldn't be s
...

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417833003)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417160672

> Nit: Could this be private?

Sure, switched to a private member in the latest push

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843504462)
> > clang can obviously do the right thing.
>
> Are you sure?

Absolutely not! I'm making this up as I go.

> When I tried yesterday, it did not ([#28674 (comment)](https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841445629))

Thanks, I guess that's the test I just asked for.

Ok, I'll keep poking. But it seems like this approach likely won't work :(

💬 maflcko commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843519987)
> Ok, I'll keep poking. But it seems like this approach likely won't work :(

What about hiding the function inside a compilation unit, where they are turned into `bswap`, and then rely on LTO to replace the `call internal_bswap_64` with `bswap`? (Haven't tried this)

💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1843570199)
Post-merge ACK

💬 hebasto commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1843624931)
Rebased on top of the merged #28989.

💬 maflcko commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1843630668)
I am trying to compile this on riscv64, however, it still does not seem to work.

```
dpkg --print-architecture && HOSTS="arm64-apple-darwin" V=1 VERBOSE=1 MAX_JOBS=$(nproc) ./contrib/guix/guix-build
riscv64
Found macOS SDK at '/bitcoin-core/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers', using...
Checking that we can connect to the guix-daemon...

Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.

accepted connection from pi
...

⚠️ hebasto opened an issue: "The `streams_tests/xor_file` test fails on Windows"
(https://github.com/bitcoin/bitcoin/issues/29014)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
>.\bitcoin-26.0\bin\test_bitcoin.exe -t streams_tests/xor_file
Running 1 test case...
unknown location(0): fatal error: in "streams_tests/xor_file": std::ios_base::failure[abi:cxx11]: AutoFile::write: file handle is nullptr: iostream error
test/streams_tests.cpp(29): last checkpoint

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

```

### Expected behav
...

💬 fanquake commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1843638198)
> Download https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip

> What version of Bitcoin Core are you using?
> v26.0

Which version is it?

💬 hebasto commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1843642317)
> > Download [bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip](https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip)
>
> > What version of Bitcoin Core are you using?
> > v26.0
>
> Which version is it?

https://bitcoincore.org/bin/bitcoin-core-26.0/bitcoin-26.0-win64.zip

💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1843661159)
Rebased 2086d1d4c3f40cce34647995ead4bff22967ffd8 -> 34970bde23dd87fc0fea5a1970880761f7184774 ([kernelInternalLib_11](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_11) -> [kernelInternalLib_12](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_11..kernelInternalLib_12))
* Fixed conflict with #27581

💬 maflcko commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1417977573)
Is this commit needed? I guess there could be issues if the same translation unit is compiled twice?

Not sure if Hex is correct to put into "crypto".

If this isn't needed, maybe leave this for a follow-up?

An alternative would be to just remove it from the util library and keep it only in consensus?

🤔 pablomartin4btc reviewed a pull request: "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block"
(https://github.com/bitcoin/bitcoin/pull/26415#pullrequestreview-1768668905)
Concept ACK

I'd like to run the bench using #26684 which I have pending of reviewing.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1418007227)
> 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.

Replicate issues without information is by far more challenging than replicating them with information. It is a blind guess.

Also, what you are saying works (
...

💬 TheCharlatan commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1418034290)
Thanks for the rationale, I indeed did not think of the future IPC delay. If we only start processing events after `app.exec()`, why not move this to just before `registerShutdownBlockReason`? Looking at the history it looks to me like this was put where it is to be in the general vicinity of the `installEvent*` calls.

📝 ryanofsky opened a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015)
Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically:

- Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity.
- Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing.
- Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of
...