β
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.
π€ w0xlt reviewed a pull request: "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs"
(https://github.com/bitcoin/bitcoin/pull/32621#pullrequestreview-2876776591)
reACK https://github.com/bitcoin/bitcoin/pull/32621/commits/7378f27b4fb512567b6152f986f67d9263d08d7a
(https://github.com/bitcoin/bitcoin/pull/32621#pullrequestreview-2876776591)
reACK https://github.com/bitcoin/bitcoin/pull/32621/commits/7378f27b4fb512567b6152f986f67d9263d08d7a
π¬ pablomartin4btc commented on pull request "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/31423#issuecomment-2917980185)
I've tested this branch ([6d9bf36](https://github.com/bitcoin/bitcoin/commit/6d9bf36b2c061088abb0d839fee90683ff1525e1)) and I still got "**bad_function_call**", as mentioned in my previous [testing](https://github.com/bitcoin/bitcoin/pull/31423#pullrequestreview-2694630661), when I try to migrate a blank legacy wallet with an imported address. I've isolated the case and it's only happening on a blank wallet with private keys enabled (disabled private keys on blank wallet with imported address wo
...
(https://github.com/bitcoin/bitcoin/pull/31423#issuecomment-2917980185)
I've tested this branch ([6d9bf36](https://github.com/bitcoin/bitcoin/commit/6d9bf36b2c061088abb0d839fee90683ff1525e1)) and I still got "**bad_function_call**", as mentioned in my previous [testing](https://github.com/bitcoin/bitcoin/pull/31423#pullrequestreview-2694630661), when I try to migrate a blank legacy wallet with an imported address. I've isolated the case and it's only happening on a blank wallet with private keys enabled (disabled private keys on blank wallet with imported address wo
...
π¬ davidgumberg commented on pull request "build: Add resource file and manifest to `bitcoin.exe`":
(https://github.com/bitcoin/bitcoin/pull/32634#issuecomment-2918081325)
ACK https://github.com/bitcoin/bitcoin/pull/32634/commits/dbb2d4c3d54759f8c346882b6f98d26d5bb8e816"
I suggest adding a CI check like https://github.com/davidgumberg/bitcoin/commit/1b9816052b1d8ad1d9c17945530e14ead79fce33 to this PR to prevent future regressions, but OK for a separate PR if out of scope.
This is also a blocker for #32431.
(https://github.com/bitcoin/bitcoin/pull/32634#issuecomment-2918081325)
ACK https://github.com/bitcoin/bitcoin/pull/32634/commits/dbb2d4c3d54759f8c346882b6f98d26d5bb8e816"
I suggest adding a CI check like https://github.com/davidgumberg/bitcoin/commit/1b9816052b1d8ad1d9c17945530e14ead79fce33 to this PR to prevent future regressions, but OK for a separate PR if out of scope.
This is also a blocker for #32431.
π¬ davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2918101687)
Thanks for the patch @fanquake, I've rebased to include https://github.com/fanquake/bitcoin/tree/scikit_compat, but squashed to avoid a broken commit.
> It seems it [should](https://iscinumpy.dev/post/scikit-build-core-0-10/) be:
>
> ```
> Restore compat with python-scikit-build-core 0.9.x
> Can be dropped when using python-scikit-build-core 0.10.x
> ```
I see, in theory, it should be sufficient to fast-forward to [35c5f07e967155d2276c7ec58e5108e4da02c974](https://codeberg.org/guix/g
...
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2918101687)
Thanks for the patch @fanquake, I've rebased to include https://github.com/fanquake/bitcoin/tree/scikit_compat, but squashed to avoid a broken commit.
> It seems it [should](https://iscinumpy.dev/post/scikit-build-core-0-10/) be:
>
> ```
> Restore compat with python-scikit-build-core 0.9.x
> Can be dropped when using python-scikit-build-core 0.10.x
> ```
I see, in theory, it should be sufficient to fast-forward to [35c5f07e967155d2276c7ec58e5108e4da02c974](https://codeberg.org/guix/g
...
π¬ davidgumberg commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2918119652)
<details>
<summary>
#### AMD Ryzen 9 9950X 16-Core Processor </summary>
```bash
./build/bin/bench_bitcoin --filter="Linearize.*Example.*" -min-time=10000
```
| ns/cost | cost/s | err% | ins/cost | cyc/cost | IPC | bra/cost | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 1.19 |
...
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2918119652)
<details>
<summary>
#### AMD Ryzen 9 9950X 16-Core Processor </summary>
```bash
./build/bin/bench_bitcoin --filter="Linearize.*Example.*" -min-time=10000
```
| ns/cost | cost/s | err% | ins/cost | cyc/cost | IPC | bra/cost | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 1.19 |
...
β οΈ 602kuntry opened an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/32637)
Concept ACK
_Originally posted by @jnewbery in https://github.com/bitcoin/bitcoin/issues/22490#issuecomment-882506347_
(https://github.com/bitcoin/bitcoin/issues/32637)
Concept ACK
_Originally posted by @jnewbery in https://github.com/bitcoin/bitcoin/issues/22490#issuecomment-882506347_