💬 maflcko commented on issue "Memory-only wallet (for faucets)":
(https://github.com/bitcoin/bitcoin/issues/31816#issuecomment-2655746412)
> Before I was doing also `scantxoutset` without any wallet. It actually provided me with unspent outputs that were confirmed in blocks and then I could just `signrawtransactionwithkey`. But using a wallet has many advantages

This would have been my suggestion as a workaround. Is there something that would be missing? I haven't tried, but `removeprunedfunds` could be another alternative to reduce the wallet file size?

👍 TheCharlatan approved a pull request: "cmake: add a component for each binary"
(https://github.com/bitcoin/bitcoin/pull/31844#pullrequestreview-2614109983)
ACK 686ebcfe93efa9ecca9b94e78da23a36c3b01f90

👍 TheCharlatan approved a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2614124544)
Re-ACK 0c1b29a05777

💬 maflcko commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#issuecomment-2655798240)
> @maflcko Mind taking a look at the first commit here to see if you'd prefer a different approach?

The first commit lgtm. Anything should be fine in the fuzz ci task as long as it compiles and runs the fuzz executable.

💬 hodlinator commented on pull request "qa debug: Add --debugnode/-waitfordebugger [DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/31723#issuecomment-2655806242)
- Added CMake option `WAIT_FOR_DEBUGGER`.
- Changed the Python logic from debugging specific nodes, to debugging specific node runs/executions, which is more specific.
- Use `prctl()` on Linux to allow any process to attach to `bitcoind`. This removes the necessity to disable `kernel.yama.ptrace_scope` on some systems, which may be seen as decreasing security, but is compiled out unless `WAIT_FOR_DEBUGGER=ON`.
- (Confirmed PR works on Windows).
- (Rebased).

👍 vasild approved a pull request: "cmake: Add `CheckLinkerSupportsPIE` module"
(https://github.com/bitcoin/bitcoin/pull/31359#pullrequestreview-2614092008)
ACK 81c174e3186efae084829dcde314b081cad3d3cb

I re-checked this. Without this PR I get the warning as mentioned in https://github.com/bitcoin/bitcoin/pull/31359#pullrequestreview-2465699855 and with this PR there is no warning. Further, without this PR, there is no `-fPIC` or `-fPIE` when compiling. With this PR `-fPIC` is added (not `-fPIE`).

💬 vasild commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#discussion_r1953962295)
> See the configure log

It would be nice to mention the actual file name of the "configure log" because that may not be obvious.
```suggestion
message(WARNING "PIE is not supported at link time. See ${CMAKE_BINARY_DIR}/CMakeFiles/CMakeConfigureLog.yaml for details.")
```

💬 hebasto commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#discussion_r1954013138)
It might depend on the used CMake version: https://github.com/bitcoin/bitcoin/blob/048ef98626b9ed62355923f089ad5cc5b6a898a3/ci/test/GetCMakeLogFiles.cmake#L5-L9

💬 vasild commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#discussion_r1954024412)
Ok, not so important. If this recurs then maybe move that snippet to a location where it can be reused by `ci/test/GetCMakeLogFiles.cmake` and by `cmake/module/CheckLinkerSupportsPIE.cmake`, so they can use `${log_files}`.

💬 hebasto commented on pull request "doc: build: Fix instructions for msvc gui builds":
(https://github.com/bitcoin/bitcoin/pull/31851#issuecomment-2655837115)
My only concern is that [` --x-buildtrees-root=<path>`](https://learn.microsoft.com/en-us/vcpkg/commands/common-options#buildtrees-root) is labeled as:
> an experimental feature of vcpkg which may change or be removed at any time.

💬 maflcko commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2655857008)
> i had the same problem, even in new test wallets I’ve created with same passpharse, the same problem

Does the problem on a freshly created wallet happen with any passphrase, or only one specific one?

👍 maflcko approved a pull request: "ci: switch MSAN to use prebuilt Clang binaries"
(https://github.com/bitcoin/bitcoin/pull/31850#pullrequestreview-2614234932)
lgtm. The clang compilation was done back when `APT_LLVM_V` wasn't available. Seems fine to use now and should be trivial to revert if there is an issue.

🤔 rkrux reviewed a pull request: "wallet: Automatically repair corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2611618216)
reACK 78cb0df4d5b9cc30dbe60a6cf24ab6cac5d3f421

I reviewed the range diff.
```
git range-diff e92dc1a3d36be02a8b16eed26159a43cfbe3dbcc...78cb0df4d5b9cc30dbe60a6cf24ab6cac5d3f421
```

Newer changes are related to using the constant `LAST_KEYPOOL_INDEX` as suggested and use `migratewallet` instead of `restorewallet` due to which the format of `good_deriv_path` is changed from `'` to 'h'. Using `migratewallet` is fine because it also ends up calling `LoadLegacyWalletRecords` internally where
...

💬 rkrux commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1954042039)
This was using 12 earlier, now 11. Fine since the test passes.

💬 rkrux commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1952466689)
I had in mind tying this 10 to the constant as well. Consider if you end up retouching, otherwise it's fine.

💬 maflcko commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1954103791)
> * I don't understand why "-342 Service unavailable, RPC server started but is shutting down due to error" comment is being changed to "-342 Service unavailable, could be starting up or shutting down". These descriptions seem inconsistent and I don't know which one is correct since I don't see where -342 is being returned

I'd guess that it is coming from authproxy and is pretty general:

```
$ git grep '\-342\>' ./test/
test/functional/feature_taproot.py:# Test Taproot softfork (BIPs 340
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2655986646)
`964b7b0ad6...ebc7ba0216`: rebase due to conflicts

💬 laanwj commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#discussion_r1954130643)
Right, the logic here has been swapped since this comment was written 😄

i think that sentence is redundant. That the length is checked is implied, why pass it otherwise? The line below already describes which length is used when it's not passed.

✅ fanquake closed an issue: "cmake -P $buildDir/Coverage.cmake: Test execution for coverage fails. Wrapper resource "cov_tool_wrapper.sh.in" missing from build cache folder."
(https://github.com/bitcoin/bitcoin/issues/31638)

🚀 fanquake merged a pull request: "cmake: Copy `cov_tool_wrapper.sh.in` to the build tree"
(https://github.com/bitcoin/bitcoin/pull/31722)