💬 pablomartin4btc commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2230304020)
nit: this could be just GetDescriptors? you can list them or export them later?

💬 Sjors commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-3116624817)
re-utACK 3d099384a60c8f64c8d67a9f1e7b8649a9c54313

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2230329744)
I'm regretting that assumption in the context of multisig :-)

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3116675604)
re-ACK 604955304dfca14b1c0bb5ed1044141a5d1b2d79

💬 Sjors commented on pull request "Have createwalletdescriptor auto-detect an unused(KEY)":
(https://github.com/bitcoin/bitcoin/pull/32861#issuecomment-3116687436)
> The linked comment also states that any RPC using the unused descriptor should delete it immediately afterwords.

That should be done in #29136 which introduces `createwalletdescriptor `, or in a followup, but not in this PR.

👍 rkrux approved a pull request: "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively"
(https://github.com/bitcoin/bitcoin/pull/28333#pullrequestreview-3054401144)
ACK 8e9de762f66f3e99e02944006999fe5afb1896e4

Thanks for addressing comments.

💬 rkrux commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2230372657)
I had something like this in mind and I should have left a diff earlier for clarity. It seemed to me that only for encrypted wallets the blank flag was set by the code & hence, believed this flag unsetting could be nested inside the below condition.

```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index bd1b36b4a9..5a3ecc1d19 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -381,9 +381,6 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const st
...

🤔 hebasto reviewed a pull request: "depends: fix libevent `_WIN32_WINNT` usage"
(https://github.com/bitcoin/bitcoin/pull/32837#pullrequestreview-3054470510)
Post-merge ACK f5647c6c5ae85e9469cfc5df6fcac23752e1695a.

> > ACK [524dd98](https://github.com/bitcoin/bitcoin/commit/524dd98cb1cfa3a7917b94e13a91827842e7aba8), tested on macOS 15.5 (x64) using Homebrew's mingw-w64 13.0.0.
>
> Retracting my ACK basing on non-reproducibilty of the Windows builds.

I can confirm reproducibility across `x86_64`, `aarch64` and `riscv64` platforms:
```
71847e8121e14afe0ef221b40faa9b07d5ea948b5ab2082cb4d4e449e74eb87b guix-build-f5647c6c5ae8/output/dist-archi
...

💬 maflcko commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2230430411)
> I'm not against this change, I just don't fully understand why some of the `LogPrintf` weren't migrated, e.g.

thx, done httpserver. The others you mention are not init logs (those that happen only once during startup and shutdown), because they are called in a loop for as long as the program runs.

💬 maflcko commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2230431049)
thx, done

💬 maflcko commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2230431379)
thx, done

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2230469671)
> Thanks, will do this in the next push

I don't think you did?

📝 Sjors opened a pull request: "subprocess: always check result of close() "
(https://github.com/bitcoin/bitcoin/pull/33061)
Suggested in https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3071393448

💬 maflcko commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3116865212)
review ACK d89c6fa4a71810cdb28395d4609632e1b22249b3 🤙

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK d89c6fa4a718
...

💬 Sjors commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3116867038)
@tnndbtc I opened #33061 to do this

💬 Sjors commented on pull request "subprocess: always check result of close()":
(https://github.com/bitcoin/bitcoin/pull/33061#issuecomment-3116873333)
I suppose we should catch `OSError` and not crash if this ever happens...

🤔 maflcko reviewed a pull request: "subprocess: always check result of close()"
(https://github.com/bitcoin/bitcoin/pull/33061#pullrequestreview-3054567458)
This "fix" is just introducing other bugs down the line, no?

💬 maflcko commented on pull request "subprocess: always check result of close()":
(https://github.com/bitcoin/bitcoin/pull/33061#discussion_r2230497364)
(same below)

💬 maflcko commented on pull request "subprocess: always check result of close()":
(https://github.com/bitcoin/bitcoin/pull/33061#discussion_r2230496925)
returning early here will leak fds if the exception is caught and everything is tried again?

💬 maflcko commented on pull request "subprocess: always check result of close()":
(https://github.com/bitcoin/bitcoin/pull/33061#discussion_r2230494730)
This will just confusingly change the error message, claiming that the close failed, because the fd is not available? How would the fd be available if the open failed?