💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2232053247)
6d252266e40 and 3e757e511a3 now hash `FILE_ENV` too for the caches, using:
```bash
DEPENDS_HASH=$(git ls-tree HEAD depends "ci/test/$FILE_ENV" | sha256sum | cut -d' ' -f1)
```
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2232053247)
6d252266e40 and 3e757e511a3 now hash `FILE_ENV` too for the caches, using:
```bash
DEPENDS_HASH=$(git ls-tree HEAD depends "ci/test/$FILE_ENV" | sha256sum | cut -d' ' -f1)
```
💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3120460571)
Thanks for the review on this so far.
Whilst we had tested the docker registry caching on PRs successfully, because I was opening them myself (and was owner of the parent repo) repo-level variables were available to me which were *not* available to 3rd party pull requests.
The short of this is that this meant the docker registry cache setup could _not_ pull from the registry on pull requests and we didn't think it was therefore suitable for our purposes.
We have switched to the `gha` ca
...
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3120460571)
Thanks for the review on this so far.
Whilst we had tested the docker registry caching on PRs successfully, because I was opening them myself (and was owner of the parent repo) repo-level variables were available to me which were *not* available to 3rd party pull requests.
The short of this is that this meant the docker registry cache setup could _not_ pull from the registry on pull requests and we didn't think it was therefore suitable for our purposes.
We have switched to the `gha` ca
...
👋 willcl-ark's pull request is ready for review: "Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989)
(https://github.com/bitcoin/bitcoin/pull/32989)
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232064980)
Done. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232064980)
Done. Thanks.
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232065117)
Done. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232065117)
Done. Thanks.
🚀 achow101 merged a pull request: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
(https://github.com/bitcoin/bitcoin/pull/31576)
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232069312)
Done. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232069312)
Done. Thanks.
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232069363)
Done. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232069363)
Done. Thanks.
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232069783)
I will look into this.
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2232069783)
I will look into this.
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#issuecomment-3120479453)
Rebased. Ready for reviews.
(https://github.com/bitcoin/bitcoin/pull/32977#issuecomment-3120479453)
Rebased. Ready for reviews.
🤔 pablomartin4btc reviewed a pull request: "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`"
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3056872157)
utACK e3ba0757a9410336e904a1b108d86165f347fc52
What about these 2 (`-addresstype` and `-changetype`) in `src/wallet/init.cpp`?
https://github.com/bitcoin/bitcoin/blob/2e97541396b998f6dfb47f9acef51e3ca69e97b2/src/wallet/init.cpp#L44-L51
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3056872157)
utACK e3ba0757a9410336e904a1b108d86165f347fc52
What about these 2 (`-addresstype` and `-changetype`) in `src/wallet/init.cpp`?
https://github.com/bitcoin/bitcoin/blob/2e97541396b998f6dfb47f9acef51e3ca69e97b2/src/wallet/init.cpp#L44-L51
💬 achow101 commented on pull request "rpc, doc: clarify watch-only wallets balances in RPCHelp":
(https://github.com/bitcoin/bitcoin/pull/32761#issuecomment-3120529045)
I don't really think this change is necessary, especially with the removal of watch-only within a wallet.
(https://github.com/bitcoin/bitcoin/pull/32761#issuecomment-3120529045)
I don't really think this change is necessary, especially with the removal of watch-only within a wallet.
💬 achow101 commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#discussion_r2232105186)
The first sentences starts oddly to me. I would write it as
> If the legacy wallet only contains watch-only scripts and no private keys, then only `<name>_watchonly` ...
(https://github.com/bitcoin/bitcoin/pull/32866#discussion_r2232105186)
The first sentences starts oddly to me. I would write it as
> If the legacy wallet only contains watch-only scripts and no private keys, then only `<name>_watchonly` ...
💬 theStack commented on pull request "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`":
(https://github.com/bitcoin/bitcoin/pull/33065#issuecomment-3120565816)
> What about these 2 (`-addresstype` and `-changetype`) in `src/wallet/init.cpp`?
Thanks, missed those due to `grep`ping too tightly, added another commit.
(https://github.com/bitcoin/bitcoin/pull/33065#issuecomment-3120565816)
> What about these 2 (`-addresstype` and `-changetype`) in `src/wallet/init.cpp`?
Thanks, missed those due to `grep`ping too tightly, added another commit.
💬 martinatime commented on issue "getbestblockhash is sometimes taking a very long time":
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3120567532)
I'm using the 64bit version of PiOS and I also seem to have more than half my memory free.
$ free -mh
total used free shared buff/cache available
Mem: 3.7Gi 1.1Gi 38Mi 0.0Ki 2.5Gi 2.5Gi
Swap: 2.0Gi 2.0Gi 0.0Ki
$ uname -a
Linux raspibolt2 6.1.21-v8+ #1642 SMP PREEMPT Mon Apr 3 17:24:16 BST 2023 aarch64 GNU/Linux
$ getconf LONG_BIT
64
I can't imagine that the 32/64 bit allocations are the reas
...
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3120567532)
I'm using the 64bit version of PiOS and I also seem to have more than half my memory free.
$ free -mh
total used free shared buff/cache available
Mem: 3.7Gi 1.1Gi 38Mi 0.0Ki 2.5Gi 2.5Gi
Swap: 2.0Gi 2.0Gi 0.0Ki
$ uname -a
Linux raspibolt2 6.1.21-v8+ #1642 SMP PREEMPT Mon Apr 3 17:24:16 BST 2023 aarch64 GNU/Linux
$ getconf LONG_BIT
64
I can't imagine that the 32/64 bit allocations are the reas
...
🤔 cedwies reviewed a pull request: "p2p: never check tx rejections by txid"
(https://github.com/bitcoin/bitcoin/pull/33066#pullrequestreview-3056948992)
Built PR 33066 on macOS 14 (Apple Silicon) with CMake + Ninja, Debug.
- unit tests: 141/141 pass, 9 min wall time
what I checked:
‒ AlreadyHaveTx(): tx-id path now skips both reject filters; wtx-id path unchanged.
‒ MempoolRejectedTx(): parent-txid rejection bail-out removed → orphans kept even if parents sit in a reject cache
‒ updated tests (expected_behaviors table + orphan-handling cases) run green.
Question (still wrapping my head around this):
MempoolRejectedTx still inserts
...
(https://github.com/bitcoin/bitcoin/pull/33066#pullrequestreview-3056948992)
Built PR 33066 on macOS 14 (Apple Silicon) with CMake + Ninja, Debug.
- unit tests: 141/141 pass, 9 min wall time
what I checked:
‒ AlreadyHaveTx(): tx-id path now skips both reject filters; wtx-id path unchanged.
‒ MempoolRejectedTx(): parent-txid rejection bail-out removed → orphans kept even if parents sit in a reject cache
‒ updated tests (expected_behaviors table + orphan-handling cases) run green.
Question (still wrapping my head around this):
MempoolRejectedTx still inserts
...
🤔 achow101 reviewed a pull request: "wallet: Fix relative path backup during migration."
(https://github.com/bitcoin/bitcoin/pull/32273#pullrequestreview-3056922125)
ACK 76fe0e59ec4a5b0c5b18f46bfcbbf99628d74d77
(https://github.com/bitcoin/bitcoin/pull/32273#pullrequestreview-3056922125)
ACK 76fe0e59ec4a5b0c5b18f46bfcbbf99628d74d77
💬 achow101 commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2232112912)
In 63c6d364376907c10b9baa3c6f4d72e3f1881abc "test: Migration of a wallet with `../` in path."
This is unnecessary since `migrate_and_get_rpc` also sets and unsets the mocktime.
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2232112912)
In 63c6d364376907c10b9baa3c6f4d72e3f1881abc "test: Migration of a wallet with `../` in path."
This is unnecessary since `migrate_and_get_rpc` also sets and unsets the mocktime.
👍 pablomartin4btc approved a pull request: "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`"
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3056964820)
re-utACK 251d02084688c67523e9ec92ec79ee657454ab93
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3056964820)
re-utACK 251d02084688c67523e9ec92ec79ee657454ab93
💬 murchandamus commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#discussion_r2232211925)
The phrasing "Test that when the max weight is exceeded, only the most valuable encountered UTXOs are returned" sounds potentially misleading to me, because it could be understood as the algorithm changing to largest-first if the max weight is exceeded or the input set getting pruned beyond dropping below the weight limit, but this only tests that a solution is found that stays within the limit.
(https://github.com/bitcoin/bitcoin/pull/33058#discussion_r2232211925)
The phrasing "Test that when the max weight is exceeded, only the most valuable encountered UTXOs are returned" sounds potentially misleading to me, because it could be understood as the algorithm changing to largest-first if the max weight is exceeded or the input set getting pruned beyond dropping below the weight limit, but this only tests that a solution is found that stays within the limit.