📝 l0rinc opened a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094)
### Summary
Instead of counting occurrences in sets and maps, the C++20 `::contains` method expresses the intent unambiguously and can return early on first encounter.
### Context
Applied clang‑tidy's [readability‑container‑contains](https://clang.llvm.org/extra/clang-tidy/checks/readability/container-contains.html) check, though many cases required manual changes since tidy couldn't fix them automatically.
### Changes
The changes made here were:
| From | To
...
(https://github.com/bitcoin/bitcoin/pull/33094)
### Summary
Instead of counting occurrences in sets and maps, the C++20 `::contains` method expresses the intent unambiguously and can return early on first encounter.
### Context
Applied clang‑tidy's [readability‑container‑contains](https://clang.llvm.org/extra/clang-tidy/checks/readability/container-contains.html) check, though many cases required manual changes since tidy couldn't fix them automatically.
### Changes
The changes made here were:
| From | To
...
💬 Christewart commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134394012)
I believe this deserves a release note as it can break existing calls to `dumptxoutset` if you were previously omitting the `type` parameter?
> This parameter can be omitted if a separate \"rollback\" named parameter is specified indicating the height or hash of a specific historical block.
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134394012)
I believe this deserves a release note as it can break existing calls to `dumptxoutset` if you were previously omitting the `type` parameter?
> This parameter can be omitted if a separate \"rollback\" named parameter is specified indicating the height or hash of a specific historical block.
💬 kevkevinpal commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#issuecomment-3134529754)
ACK [c157438](https://github.com/bitcoin/bitcoin/pull/33083/commits/c1574381168573c561ebddf1945d2debefb340f7)
I was able to reproduce the errors with the diffs provided above, and was also able to validate that the master branch tests pass when the diffs are applied as well
(https://github.com/bitcoin/bitcoin/pull/33083#issuecomment-3134529754)
ACK [c157438](https://github.com/bitcoin/bitcoin/pull/33083/commits/c1574381168573c561ebddf1945d2debefb340f7)
I was able to reproduce the errors with the diffs provided above, and was also able to validate that the master branch tests pass when the diffs are applied as well
💬 ryanofsky commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134531569)
> I believe this deserves a release note as it can break existing calls to `dumptxoutset` if you were previously omitting the `type` parameter?
Yes it would be good to add a release note saying that `dumptxoutset` can no longer be called with just a filename to produce a snapshot with the latest UTXO state. It now needs an additional `type` argument that can be set to "latest" to match previous behavior. It can also take other arguments and is now capable of rolling back the chain and produci
...
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134531569)
> I believe this deserves a release note as it can break existing calls to `dumptxoutset` if you were previously omitting the `type` parameter?
Yes it would be good to add a release note saying that `dumptxoutset` can no longer be called with just a filename to produce a snapshot with the latest UTXO state. It now needs an additional `type` argument that can be set to "latest" to match previous behavior. It can also take other arguments and is now capable of rolling back the chain and produci
...
💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134929957)
29.0 is already tagged (archived), built, and released for a long time, so it isn't possible to change the release notes there now.
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134929957)
29.0 is already tagged (archived), built, and released for a long time, so it isn't possible to change the release notes there now.
👋 l0rinc's pull request is ready for review: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094)
(https://github.com/bitcoin/bitcoin/pull/33094)
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3135054321)
Thanks for addressing the comments, I will take one final look and then a-c-k.
Tagging @furszy for review as he noticed this issue as well few months back.
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3135054321)
Thanks for addressing the comments, I will take one final look and then a-c-k.
Tagging @furszy for review as he noticed this issue as well few months back.
📝 maflcko opened a pull request: "ci: Remove unused ci/lint_run_all.sh"
(https://github.com/bitcoin/bitcoin/pull/33095)
As per the file content, this can be deleted now. (Stale re-runs are no longer performed for `lint` right now and the CI will be reworked anyway)
(https://github.com/bitcoin/bitcoin/pull/33095)
As per the file content, this can be deleted now. (Stale re-runs are no longer performed for `lint` right now and the CI will be reworked anyway)
✅ maflcko closed a pull request: "ci: Remove unused ci/lint_run_all.sh"
(https://github.com/bitcoin/bitcoin/pull/33095)
(https://github.com/bitcoin/bitcoin/pull/33095)
💬 maflcko commented on pull request "ci: Remove unused ci/lint_run_all.sh":
(https://github.com/bitcoin/bitcoin/pull/33095#issuecomment-3135184921)
Hmm, let's just do it in the other pull.
(https://github.com/bitcoin/bitcoin/pull/33095#issuecomment-3135184921)
Hmm, let's just do it in the other pull.
👍 maflcko approved a pull request: "Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3070236084)
everything looks good and this is probably ready for final review before merge? I just found some nits, which can be done along with follow-ups like https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236533948. Also, the merge_base issue mentioned above can be addressed in a follow-up.
However, I am not familiar with the GitHub-style CI, including the (docker) caching, but the pull request motivation claims that many are familiar with this, so it should be easy to find reviewers for i
...
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3070236084)
everything looks good and this is probably ready for final review before merge? I just found some nits, which can be done along with follow-ups like https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236533948. Also, the merge_base issue mentioned above can be addressed in a follow-up.
However, I am not familiar with the GitHub-style CI, including the (docker) caching, but the pull request motivation claims that many are familiar with this, so it should be easy to find reviewers for i
...
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241797039)
nit in 01746e60eb82b9bedb3afa1e791b5abce75ebf75: Unused? This task is using nproc?
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241797039)
nit in 01746e60eb82b9bedb3afa1e791b5abce75ebf75: Unused? This task is using nproc?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241816383)
nit in 4a1c5b04691266f1441fd966a4be65140f9a5cc4: Outdated doc?
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241816383)
nit in 4a1c5b04691266f1441fd966a4be65140f9a5cc4: Outdated doc?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241782043)
You can also remove the now unused stuff:
```
ci/lint_run.sh:# Only used in .cirrus.yml. Refer to test/lint/README.md on how to run locally.
ci/lint_run_all.sh:# Only used in .cirrus.yml for stale re-runs of old pull request tasks. This
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241782043)
You can also remove the now unused stuff:
```
ci/lint_run.sh:# Only used in .cirrus.yml. Refer to test/lint/README.md on how to run locally.
ci/lint_run_all.sh:# Only used in .cirrus.yml for stale re-runs of old pull request tasks. This
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241802051)
nit in https://github.com/bitcoin/bitcoin/commit/01746e60eb82b9bedb3afa1e791b5abce75ebf75: Any reason to change this setting for the other tasks, but not for this one? For reference, the task has 3 mac-arm cpu threads.
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241802051)
nit in https://github.com/bitcoin/bitcoin/commit/01746e60eb82b9bedb3afa1e791b5abce75ebf75: Any reason to change this setting for the other tasks, but not for this one? For reference, the task has 3 mac-arm cpu threads.
💬 ajtowns commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3135258539)
> TLDR, this PR removes the rejection cache filtering part of all txid-based tx requests.
There's three cases here I think:
* orphan resolution from wtxidrelay peers (WTXIDRELAY)
* tx relay from segwit-supporting, non-wtxidrelay peers (NODE_WITNESS, no WTXIDRELAY, 0.13.1 to 0.20.x, Jan 2021)
* tx relay from non-segwit peers
The last case case is trivial -- non-segwit peers can be treated as doing wtxid relay for caching because all the txs they relay will have the same value for tx
...
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3135258539)
> TLDR, this PR removes the rejection cache filtering part of all txid-based tx requests.
There's three cases here I think:
* orphan resolution from wtxidrelay peers (WTXIDRELAY)
* tx relay from segwit-supporting, non-wtxidrelay peers (NODE_WITNESS, no WTXIDRELAY, 0.13.1 to 0.20.x, Jan 2021)
* tx relay from non-segwit peers
The last case case is trivial -- non-segwit peers can be treated as doing wtxid relay for caching because all the txs they relay will have the same value for tx
...
💬 maflcko commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3135294275)
This seems to be causing CI failures:
* https://cirrus-ci.com/task/5499213573783552?logs=ci#L1556
* https://cirrus-ci.com/task/5576386364047360?logs=ci#L1556
* https://cirrus-ci.com/task/6693205086830592?logs=ci#L1556
* ...
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3135294275)
This seems to be causing CI failures:
* https://cirrus-ci.com/task/5499213573783552?logs=ci#L1556
* https://cirrus-ci.com/task/5576386364047360?logs=ci#L1556
* https://cirrus-ci.com/task/6693205086830592?logs=ci#L1556
* ...
⚠️ fanquake opened an issue: "ci: failure in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/issues/33096)
https://cirrus-ci.com/task/5576386364047360?logs=ci#L1549:
```bash
[15:21:12.678] node0 2025-07-29T19:21:12.075826Z [httpworker.4] [rpc/request.cpp:243] [void JSONRPCRequest::parse(const UniValue&)] [rpc] ThreadRPCServer method=listwalletdir user=__cookie__
[15:21:12.678] test 2025-07-29T19:21:12.085334Z TestFramework (ERROR): Unexpected exception
[15:21:12.678] Traceback (most recent call last):
[15:21:12.678] File "/c
...
(https://github.com/bitcoin/bitcoin/issues/33096)
https://cirrus-ci.com/task/5576386364047360?logs=ci#L1549:
```bash
[15:21:12.678] node0 2025-07-29T19:21:12.075826Z [httpworker.4] [rpc/request.cpp:243] [void JSONRPCRequest::parse(const UniValue&)] [rpc] ThreadRPCServer method=listwalletdir user=__cookie__
[15:21:12.678] test 2025-07-29T19:21:12.085334Z TestFramework (ERROR): Unexpected exception
[15:21:12.678] Traceback (most recent call last):
[15:21:12.678] File "/c
...
⚠️ dergoegge opened an issue: "fuzz: `txgraph`: Assertion `cmp == 0' failed"
(https://github.com/bitcoin/bitcoin/issues/33097)
```
$ echo "oK+goKCgArv/GMG0oAkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAOTkAr6AAAgIC/wICAgICAgICCAAAAAAAAAAAAAgAAP8A/wACAAAAAAAAAAAAAAAAAAAAAAAAAAAAABACAgICf/8AoKAAAAAAAAAAAAgAAP8AAgAAAAAAABACAgICf/8AAAAAIAAAAAAAAAAAAAAACAAA/wACgIAAAAAAgIAAAAAAAAAAAAAACAAA/wACAAAAAAAAAAAAAAAAAAAAAAAAAAAAABACAgICf/8AoF4AFgAAAAAAAAAIAAD/AAIAAICAAAA=" | base64 --decode > txgraph.crash
$ FUZZ=txgraph ./fuzz txgraph.crash
fuzz: test/fuzz/txgraph.cpp:1057: void txgraph_fuzz_target(FuzzBufferType): Assertion `cmp == 0' failed.
==
...
(https://github.com/bitcoin/bitcoin/issues/33097)
```
$ echo "oK+goKCgArv/GMG0oAkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAOTkAr6AAAgIC/wICAgICAgICCAAAAAAAAAAAAAgAAP8A/wACAAAAAAAAAAAAAAAAAAAAAAAAAAAAABACAgICf/8AoKAAAAAAAAAAAAgAAP8AAgAAAAAAABACAgICf/8AAAAAIAAAAAAAAAAAAAAACAAA/wACgIAAAAAAgIAAAAAAAAAAAAAACAAA/wACAAAAAAAAAAAAAAAAAAAAAAAAAAAAABACAgICf/8AoF4AFgAAAAAAAAAIAAD/AAIAAICAAAA=" | base64 --decode > txgraph.crash
$ FUZZ=txgraph ./fuzz txgraph.crash
fuzz: test/fuzz/txgraph.cpp:1057: void txgraph_fuzz_target(FuzzBufferType): Assertion `cmp == 0' failed.
==
...
💬 fanquake commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3135301238)
Yea, opened #33096 for this.
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3135301238)
Yea, opened #33096 for this.