📝 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
...

💬 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
* ...

⚠️ 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.

💬 fanquake commented on issue "ci: failure in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3135307621)
Other failures:

https://cirrus-ci.com/task/6693205086830592?logs=ci#L1556
https://cirrus-ci.com/task/5499213573783552?logs=ci#L1556

💬 dergoegge commented on issue "fuzz: `txgraph`: Assertion `cmp == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/33097#issuecomment-3135309059)
I'm guessing this is new after #32263 was merged yesterday. cc @sipa @glozow

💬 HowHsu commented on pull request "truc: optimize the in package relation calculation":
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2241946108)
@glozow Hi glozow, seems DepGraph stores all ancestors of a tx, while here:
```
if (mempool_ancestors.size() + in_package_parents.size() + 1 > TRUC_ANCESTOR_LIMIT)
```

shows `PackageTRUCChecks()` want the parent (the direct ancestor).
Using DepGraph changes the semantics of this function. Any hints about the above code: Do we actually want `mempool_ancestors.size() + in_package_parents.size()` or `mempool_ancestors.size + in_package_ancestors.size()` here ?
Using the Bitset directly is
...

💬 maflcko commented on issue "ci: failure in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3135379716)
The failures are so frequent that I've disabled the DrahtBot silent-merge-conflict detection for now

🤔 Sjors reviewed a 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#pullrequestreview-3070321358)
Mostly happy with d82dcf2eaa7efb3809ae559c8f97f74403a2ccd6, but in b32c2b290f8e4dcb7fe54ee4c81714a3785920a7 _descriptor: ToPrivateString() pass if at least 1 priv key exists_ I find the miniscript change hard to follow (mostly because I find the miniscript code itself hard to follow). Can you split that out in a seperate commit that doesn't change behaviour?

Tested with https://github.com/Sjors/bitcoin/pull/91.

Also left some code style suggestions.

💬 Sjors 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#discussion_r2241891588)
In https://github.com/bitcoin/bitcoin/commit/2c0a84f1e2ebe98bcf5066acb0de0ba5138c2101 descriptors: add IsWatchOnly(): since c++17 you can safely compare std::optional<T> with T, see item 21 here: https://en.cppreference.com/w/cpp/utility/optional/operator_cmp.html

I also find it easier to read if `privkey_count` and `pubkey_count` are tracked in the same manner, but feel free to ignore:

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 98ae29ceb3..a86b4743b
...

💬 Sjors 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#discussion_r2241845825)
In 2c0a84f1e2ebe98bcf5066acb0de0ba5138c2101 _descriptors: add IsWatchOnly()_: this doesn't describe the actual behavior. IIUC:

```cpp
/** Whether this descriptor is missing any private key material */
```