📝 hebasto opened a pull request: "build: Fix `ENABLE_WALLET` option"
(https://github.com/bitcoin/bitcoin/pull/30867)
The removed commands were left over from the transition from autodetection to explicit options in the CMake staging branch. These commands prevented the `-DENABLE_WALLET=OFF` option from being work properly when building with depends.

How to test:
```
$ make -C depends NO_QT=1
```

On the master branch @ c66c68345efb0bb3d5613ebac703cde779fa0f01:
```
$ rm -rf build && cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake -DENABLE_WALLET=OFF
< snip >
Optional features:
...

💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#issuecomment-2342178439)
> Replacing PushDataSize size_t parameter with uint32_t parameter

Good observation, thanks.
I was being overly cautious with modifying that code.

> Making the char operator call the std::byte operator to make it smaller, deduplicate code, and make it obvious the operators are equivalent.

My goal was to avoid extra casting and wrapping, especially in performance-critical areas like this.

Right now, we avoid intermediary vectors, but with this change, we'd wrap the data in a `std::spa
...

💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1752854915)
I think it makes sense to throw an error in this situation. Although it's not necessarily due to disk corruption, it would also be possible that the block/undo data is pruning event right after the blockindex check (`cs_main` is released) and before the `UndoReadFromDisk` call.

We also have a functional test for just this situation that wouldn't work anymore ([here](https://github.com/bitcoin/bitcoin/blob/c66c68345efb0bb3d5613ebac703cde779fa0f01/test/functional/rpc_blockchain.py#L615-L628)) b
...

💬 kevkevinpal commented on pull request "test: Drop no longer needed workarounds":
(https://github.com/bitcoin/bitcoin/pull/30847#issuecomment-2342307179)
ACK [5c80192](https://github.com/bitcoin/bitcoin/pull/30847/commits/5c80192ff6b982ee3a75be4142fe942b8206f2cd)

`cmake -B build -DENABLE_EXTERNAL_SIGNER=OFF (and commented out banman_tests)`
`cmake --build build`
`ctest --test-dir build`

Got the following snippit
```
<snip>
Start 13: banman_tests
13/133 Test #13: banman_tests .........................***Skipped 0.01 sec
</snip>

<snip>
Start 99: system_tests
99/133 Test #99: system_tests ...............
...

💬 kevkevinpal commented on pull request "docs: Updated debug build instructions for cmake":
(https://github.com/bitcoin/bitcoin/pull/30840#issuecomment-2342354487)
ACK [0b003e1](https://github.com/bitcoin/bitcoin/pull/30840/commits/0b003e1ff7e09b56cd013b15f1998495994b7211)

Also in this parity list `--enable-debug` is replaced with `-DCMAKE_BUILD_TYPE=Debug` and defaults to `RelWithDebInfo`
https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e

🤔 glozow reviewed a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805#pullrequestreview-2294829330)
ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c

🚀 glozow merged a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805)

📝 romanz opened a pull request: "http: Use 'starts_with' for matching URI prefix"
(https://github.com/bitcoin/bitcoin/pull/30868)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 maflcko commented on pull request "http: Use 'starts_with' for matching URI prefix":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2342686200)
If there is support for this change, it may be better to use and enable https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-starts-ends-with.html . Otherwise, follow-up pull requests may be created for each instance individually. Also, newly introduced code may re-introduce new instances.

💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2342749010)
I successfully tested this code as part https://github.com/Sjors/bitcoin/pull/48#issuecomment-2342745012

💬 maflcko commented on pull request "build: Fix `ENABLE_WALLET` option":
(https://github.com/bitcoin/bitcoin/pull/30867#issuecomment-2342790419)
review ACK 0037d53d1a21ec8a5a97a83ab716d68030446021

Just noting that the test config will be a bit confusing, albeit harmless:

```
$ grep -C1 USE_SQLITE ./bld-cmake/test/config.ini
#ENABLE_WALLET=true
USE_SQLITE=true
#USE_BDB=true

💬 maflcko commented on pull request "ci: Post CMake-migration fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30841#issuecomment-2342828286)
review ACK c45186ca548362b75a6640393ccf79b11ff727da

💬 maflcko commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2342833531)
review ACK b07fe666f27e2b2515d2cb5a0339512045e64761

Seems fine to always buffer everything in memory twice, because it is already buffered in memory once (as the full `hex_content`) and the data should be small (generally less than 10 MB?)

👍 vasild approved a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-2295868986)
ACK e4e3b44e9cc7227b3ad765397c884999f57bac2e

💬 maflcko commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2342845494)
re-ACK 1cc93fe7b40f10a7d1d1189058af98a2bce31381

Only change since last review is MSVC fixups as well as dropping dead (and broken) code.

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1753486683)
@stratospher thanks for the suggestion , update [https://github.com/bitcoin/bitcoin/pull/29858/commits/fcf0ead6cd358fd35909e1102bdae2c176b0ace6](https://github.com/bitcoin/bitcoin/pull/29858/commits/fcf0ead6cd358fd35909e1102bdae2c176b0ace6)

👍 naumenkogs approved a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-2296005895)
ACK e4e3b44e9cc7227b3ad765397c884999f57bac2e

💬 naumenkogs commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2342970431)
ACK 08cd29f5020995294a4f6274e72afab58bf69aa2

The first two commits make logging cleaner and more uniform.
I'm not sure the last commit is useful, but it doesn't hurt either.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1753752267)
> is there a reason we'd want to actively discourage this function from being used?

Mostly just to avoid someone from using the function over `sizeof...(Args)`, which would render everything pointless, because calculating the same value at compile time is obviously going to always pass (modulo parsing errors)

👍 naumenkogs approved a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2296198023)
ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac