π¬ 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;
```
(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
(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"
```
(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"
```
(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.
(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)
(https://github.com/bitcoin/bitcoin/issues/32635)
β
fanquake closed an issue: "My phone f"
(https://github.com/bitcoin/bitcoin/issues/32635)
(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
(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.
(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.
(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.
(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>
(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
(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
...
(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
(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).
(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/
...
(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.
(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
(https://github.com/bitcoin/bitcoin/issues/32111#issuecomment-2917920494)
Opened #32636 to fix this
π¬ theStack commented on pull request "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs":
(https://github.com/bitcoin/bitcoin/pull/32621#discussion_r2112985346)
Agree, done.
(https://github.com/bitcoin/bitcoin/pull/32621#discussion_r2112985346)
Agree, done.