💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2112813722)
In 0341e021fe6edfa21252f4bd93b9e1a209383891 "Implement rescan stop with timestamp as never for import descriptors"

Please use `snake_case` for new variables. Additionally, when the type is not annoying to type out, please give the full type rather than use `auto`.

Same for `requestTimestamp` below.

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2112814780)
In 0341e021fe6edfa21252f4bd93b9e1a209383891 "Implement rescan stop with timestamp as never for import descriptors"

nit: space between `now` and `:`

```suggestion
const int64_t timestamp = requestTimestamp < 0 ? now : requestTimestamp;
```

💬 achow101 commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#issuecomment-2917732757)
fe0432b1c4a10b74844c2dedefccfe340c0d3b10

💬 achow101 commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2112856923)
```suggestion
"Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only.\n"
```

💬 achow101 commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2112856236)
Comma should be a period.

```suggestion
"if the transaction has unconfirmed inputs. This is because the wallet will attempt to make the\n"
```

🤔 w0xlt reviewed a pull request: "wallet: init, don't error out when loading legacy wallets"
(https://github.com/bitcoin/bitcoin/pull/32449#pullrequestreview-2876603197)
ACK https://github.com/bitcoin/bitcoin/pull/32449/commits/86e1111239cdb39dd32cfb5178653c608fa30515

nit: Perhaps the comment in `HandleWalletError` could also explain why `DatabaseStatus::FAILED_LEGACY_DISABLED` is also being mapped to `RPC_WALLET_NOT_FOUND`. While it seems clear from looking at this PR, it might be a useful for future reference.

⚠️ harryvik990 opened an issue: "My phone f"
(https://github.com/bitcoin/bitcoin/issues/32635)

✅ fanquake closed an issue: "My phone f"
(https://github.com/bitcoin/bitcoin/issues/32635)

💬 achow101 commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-2917784928)
ACK f6517df210f5e940d87823c86358976743de2641

💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2917799485)
> OP_RETURN fallback if no address or descriptor set is provided.

Why? I think it would be simpler to just require that at least one output address/descriptor is specified.

💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2112898425)
Descriptors are accepted too.

💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2112898168)
Although this RPC is only used by tests, I don't think we should be breaking backwards compatibility here. It's equally easy to determine whether this parameter is a string, so we can accept a string here for backwards compatibility.

The reason to keep compatibility is because this RPC has a high likelihood of being used in scripts and automated tests. Changing the type of the parameter will break all of those.

🤔 1440000bytes reviewed a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876724595)
ACK 83df64d7491b8271f7dfa2aea30f055102e3ff39

<details>
<summary>Signature</summary>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 83df64d7491b8271f7dfa2aea30f055102e3ff39
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaDekrAAKCRAtIwUgISpZ
Aa86AP0cOFEVIMWCd7bhNekXwpmdgIVlMV9LdU2YQ40r6TdZJQEA8hVtCHmD2sMN
pkHDM0gALAeez7FyJQplX2HYYqh/tgU=
=ldiS
-----END PGP SIGNATURE-----

</details>

🤔 w0xlt reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2876745493)
ACK https://github.com/bitcoin/bitcoin/pull/32594/commits/fdabfc7b01ec2cff5f2356e1b13b5537d30dbaf8

💬 w0xlt commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2112963998)
```suggestion
# 1) Find the first “[ … ]”-wrapped bit and capture what’s inside it
origin_match = re.search(r'\[([^\]]*)\]', desc_verify)
# 2) If we got a match, pull out the inner text; otherwise use empty string
origin_part = origin_match.group(1) if origin_match else ""
# 3) Remove everything up to and including the first ‘]’, leaving only what follows
after_origin = re.sub(r'^.*?\]', '', desc_verify) if origin_match else desc_verify

...

💬 w0xlt commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2112964574)
nit

💬 andrewtoth commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2917909516)
@l0rinc yes, I agree with other reviewers a feature like this should be added directly to bitcoind. If needed, this tool is still available to be run at [github.com/andrewtoth/blocks-xor](https://github.com/andrewtoth/blocks-xor).

📝 davidgumberg opened a pull request: "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`"
(https://github.com/bitcoin/bitcoin/pull/32636)
This PR is mostly a refactor which splits out logic used for creating wallets and for loading wallets, both of which are presently contained in `CWallet::Create()` into `CWallet::CreateNew()` and `CWallet::LoadExisting()`

The real win of this PR is that `CWallet::Create()` uses a very bad heuristic for trying to guess whether or not it is supposed to be creating a new wallet or loading an existing wallet:

https://github.com/bitcoin/bitcoin/blob/370c59261269fd9043674e0f4fd782a89e724473/src/
...

💬 davidgumberg commented on issue "wallet: migratewallet crashes "Assertion `legacy_spkm' failed"":
(https://github.com/bitcoin/bitcoin/issues/32112#issuecomment-2917920181)
Opened #32636 to fix this.

💬 davidgumberg commented on issue "wallet: migratewallet crashes "Assertion `m_wallet_flags == 0' failed"":
(https://github.com/bitcoin/bitcoin/issues/32111#issuecomment-2917920494)
Opened #32636 to fix this