✅ achow101 closed an issue: "No check is done whether the binding for Tor succeeded before using it for ADD_ONION"
(https://github.com/bitcoin/bitcoin/issues/22727)

🚀 achow101 merged a pull request: "Make it possible to disable Tor binds and abort startup on bind failure"
(https://github.com/bitcoin/bitcoin/pull/22729)

💬 achow101 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1680032048)
All of our PSBT updating operations should never remove any data from the PSBT. If the user decides to do something that invalidates a field in the PSBT, it's their duty to fix it themselves.

💬 achow101 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#issuecomment-2231778432)
ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902

💬 brunoerg commented on pull request "fuzz: add target for `CoinsResult`":
(https://github.com/bitcoin/bitcoin/pull/30461#issuecomment-2231778444)
I think fuzzing it directly is good and there are some `CoinsResult` functions that are not fuzzed (`All`, `Erase`, `Clear`), but tbh just noticed they're used only in unit test, so the only benefit would be the performance of fuzzing it directly. I'm ok on closing it if case.

💬 brunoerg commented on issue "Fuzz coverage for wallet RPCs is zero":
(https://github.com/bitcoin/bitcoin/issues/30458#issuecomment-2231782205)
Added this in https://github.com/bitcoin/bitcoin/issues/29901.

💬 hebasto commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2231830637)
> The first commit could be split up as a scripted-diff? Also, formatting:
>
> ```diff
> diff --git a/test/functional/interface_http.py b/test/functional/interface_http.py
> index 567aa33cbe..dbdceb52d1 100755
> --- a/test/functional/interface_http.py
> +++ b/test/functional/interface_http.py
> @@ -109 +109 @@ if __name__ == '__main__':
> - HTTPBasicsTest(__file__).main ()
> + HTTPBasicsTest(__file__).main()
> --
> diff --git a/test/functional/rpc_getchaintips.py b/test/functio
...

✅ achow101 closed an issue: "RPC: Internal bug in `walletprocesspsbt` when non_witness_utxo is not provided and a witness signature is invalid"
(https://github.com/bitcoin/bitcoin/issues/30077)

🚀 achow101 merged a pull request: "Fix cases of calls to `FillPSBT` errantly returning `complete=true`"
(https://github.com/bitcoin/bitcoin/pull/30357)

💬 ismaelsadeeq commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1679251068)
nit: typo
```suggestion
* before children), together with position information so transactions can be moved back to their
* correct position on deserialization.
```

📝 hebasto opened a pull request: "test, refactor: Fix MSVC warning C4101 "unreferenced local variable""
(https://github.com/bitcoin/bitcoin/pull/30464)
This PR is split from https://github.com/bitcoin/bitcoin/pull/30454 and addresses MSVC warning [C4101](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4101) "unreferenced local variable". The current MSVC build system in the master branch skips building univalue tests, so it is not affected.

No behaviour changes.

💬 hebasto commented on pull request "test, refactor: Fix MSVC warning C4101 "unreferenced local variable"":
(https://github.com/bitcoin/bitcoin/pull/30464#issuecomment-2231884900)
It is a part of upfront PR'ed commits from https://github.com/bitcoin/bitcoin/pull/30454, as suggested by @fanquake offline.

👍 theuni approved a pull request: "test, refactor: Fix MSVC warning C4101 "unreferenced local variable""
(https://github.com/bitcoin/bitcoin/pull/30464#pullrequestreview-2181381380)
trivial ACK 44f08786f435ed4284d39dc604c2a5fcbde9e602.

Seems weird to be catching a non-const ref, but it really doesn't matter here :)

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: cluster linearization algorithm"
(https://github.com/bitcoin/bitcoin/pull/30126#pullrequestreview-2181382355)
Reviewed:

- [x] Commit 4409a282d7fe7a0ebee67c8fb9fb4ef157ed5883
- [x] Commit f5fa49f589f477caa5a4ca41d8331acdca6d7298

I spent time understanding the [cluster linearization theory](https://delvingbitcoin.org/t/introduction-to-cluster-linearization/1032) and the serialization and de-serialization code in commit f5fa49f589f477caa5a4ca41d8331acdca6d7298 are well documented 💯 .

📝 hebasto opened a pull request: "depends: Set `CMAKE_SYSTEM_VERSION` for CMake builds"
(https://github.com/bitcoin/bitcoin/pull/30465)
From CMake [docs](https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_VERSION.html):
> When the [CMAKE_SYSTEM_NAME](https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html#variable:CMAKE_SYSTEM_NAME) variable is set explicitly to enable [cross compiling](https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#cross-compiling-toolchain) then the value of CMAKE_SYSTEM_VERSION must also be set explicitly to specify the target system version.

This PR is split from ht
...

💬 hebasto commented on pull request "depends: Set `CMAKE_SYSTEM_VERSION` for CMake builds":
(https://github.com/bitcoin/bitcoin/pull/30465#issuecomment-2231918125)
It is a part of upfront PR'ed commits from https://github.com/bitcoin/bitcoin/pull/30454, as suggested by @fanquake offline.

cc @theuni @TheCharlatan @ryanofsky

🤔 hodlinator reviewed a pull request: "blockstorage: XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052#pullrequestreview-2181323398)
Concept ACK fae8e0a9825d107431f3f33e4f70fe54d02ab3e3
(Haven't had time to look at the tests yet).

Appreciate the robustness of `InitBlocksdirXorKey()` after some testing of removing the `blocks/xor.dat` file, re-running `bitcoind`. `hexdump`ing it, removing the whole `blocks/` directory, rerunning, `hexdump`ing again etc.

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680128246)
Tested `bitcoind -blocksxor=01234567`, which does not throw. Evaluates to true. Discussion seems to have stalled since #12713 https://github.com/bitcoin/bitcoin/blob/6f9db1ebcab4064065ccd787161bf2b87e03cc1f/src/common/args.cpp#L43-L63

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680076253)
Error messages should be as specific as possible to aid troubleshooting.
```suggestion
return InitError(strprintf(Untranslated("Failed to initialize ChainstateManager: %s"), e.what()));
```

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680074476)
Including "KEY" makes me think the variable would itself contain the default XOR pattern.
```suggestion
static constexpr bool DEFAULT_XOR_BLOCKSDIR{true};
```
Same goes for `bool xor_key` below.. would call it `bool xored_on_disk` or something.