💬 maflcko commented on pull request "fuzz: Make process_message(s) more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32822#issuecomment-3116570133)
fuzz-only pull with two acks and no conflicts with reviewed pulls, rfm?

💬 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_r2230275124)
out of curiosity... why do we need this here now? what's the issue if we don't (currently in `master`)? could we not use `UpgradeDescriptorCache()` from an upper level?

💬 Sjors commented on pull request "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32":
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3116588223)
It might be useful to open an issue to discuss paper backups in general.

I think conceptually the are three different things to backup, and each has different requirements and frequencies:

1. Private key material: backed up once at wallet creation time (or never, in some multisig setups where recovery consists of using the other keys and moving to a fresh wallet). Needs to be kept secure against theft.
2. Output descriptors: for anything more complicated than a [BIP48 multisig](https://
...

💬 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_r2230279837)
nit: shouldn't be something like GetDescriptorsInfo... ? or better naming... it's not doing an "export"...

🤔 pablomartin4btc reviewed a pull request: "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet"
(https://github.com/bitcoin/bitcoin/pull/32489#pullrequestreview-3054273346)
light cr ACK e6ac2015a852caa0f50e44becf9cd4b7592c48e7

I like the refactoring in moving the logic of extracting descriptors info out of `listdescriptors` RPC to `CWallet` itself.

I'll continue reviewing and will test it soon.

Left a couple of comments.

💬 Sjors commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2230294157)
Let's not (re)build a full fletched CPU miner :-)

💬 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
...