Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 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
...
💬 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.
💬 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
💬 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
...
💬 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.
👋 l0rinc's pull request is ready for review: "refactor: unify container presence checks"
(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.
📝 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)
maflcko closed a pull request: "ci: Remove unused ci/lint_run_all.sh"
(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.
👍 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
...
💬 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?
💬 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?
💬 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
💬 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.
💬 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
...
⚠️ 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
...
⚠️ 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.
==
...
💬 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.