💬 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 */
```

💬 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_r2241953374)
In b32c2b290f8e4dcb7fe54ee4c81714a3785920a7 _descriptor: ToPrivateString() pass if at least 1 priv key exists_:

To preserve existing behaviour:

```cpp
if (has_priv_key) *has_priv_key = true;
```

It's a bit worrying that the test passes whether I set this to `false` or `true`. But that's a pre-existing issue, as the following change to the original code doesn't break the test either:

```diff
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 0a32727f
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3135423918)
Rebased 3d6b3fd8f65e8d4ea2c26d61c3dee89f5da10fac -> 6a9fdf7ae58a85ccc08c5f6917f64f28f5a330ad ([kernelApi_48](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_48) -> [kernelApi_49](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_49), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_48..kernelApi_49))

* Fixed conflict with #33079

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2242080695)
> Set this to run on PRs in [fd32aa8](https://github.com/bitcoin/bitcoin/commit/fd32aa85b29f90c44e4a08f247ee6b1840ff7b69) ci: port lint by setting `CIRRUS_PR=1` via `if [ ${{ github.event_name }}" = "pull_request" ]; then` which I think should fix this?

Yes, this should fix it. This will correctly set the COMMIT_RANGE, just like it did previously. Thx.

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2242089031)
Heh, yeah. Somewhat it seems plausible that the environment may not be available before the vm boots up. A ~4 second delay should be acceptable. And it would be trivial to revert to hardcoding thrice, if there is need to.

(Feel free to resolve)

💬 ajtowns commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3135568435)
> @delta1 i would suggest picking up #13990 and #13922. In this order, or make a good argument why it would be fine to lower the settings without first making the fee estimation be compatible.

I don't think fee estimation should be a blocker for the relay change. If anything, the reverse: while sub-1sat/vb txs don't relay well, it would be annoying if your fee estimates dropped resulting in your txs also not relaying well.

You could get the best of both worlds by fixing fee estimation firs
...

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2242130550)
Correct, this is dropped in a81bf5c25a8