💬 ismaelsadeeq commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2215623353)
This is not surprising at all you can easily see that by looking at the subscribers of this notification.
Fwiw, I like the change, but I think the last comment in the sentence should be removed. We're not updating the fee logic inside the subroutine; it's done in the scheduler thread in the background. Hence, I’m approaching ~0 as-is.
Based on https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#conceptual-review

💬 maflcko commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3088867703)
review ACK 96da68a38fa295d2414685739c41b8626e198d27 🚋

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 96da68a38fa2
...

💬 maflcko commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2215637564)
It is also updating the fee logic (`lastRollingFee`), so the comment isn't wrong. No strong opinion, though 😅

💬 maflcko commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2215643371)
Could also use `mask|=non_empty;` (Maybe the `+` was a typo and meant `|`?)

💬 ismaelsadeeq commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2215648241)
Okay, I see that :)
Yeah the comment matches that update, I guess that ~0 does not apply any more, can be resolved.

👍 ismaelsadeeq approved a pull request: "mempool: Avoid needless vtx iteration during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827#pullrequestreview-3032959527)
Code review ACK 54f9cb85c4be2c30be0eb89f29b76a4cbf6f1c50

👍 stickies-v approved a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001#pullrequestreview-3032970350)
re-ACK faa3e684118bffa7a98cf76eeeb59243219df900

💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#issuecomment-3088912601)
Follow ups to these last few comments can be found here: https://github.com/bitcoin/bitcoin/pull/33005

💬 rkrux commented on pull request "Have createwalletdescriptor auto-detect an unused(KEY)":
(https://github.com/bitcoin/bitcoin/pull/32861#issuecomment-3088939545)
Thanks, I recall reading this comment earlier but forgot about it later. If I am not missing anything, I don't suppose this point is orthogonal to this PR?

The linked comment also states that any RPC using the unused descriptor should delete it immediately afterwords.
> add logic to importdescriptors and generatewalletdescriptor to delete any unused(KEY) descriptor as soon as the KEY which it references is used by an new descriptor (taking into account both public and private parts of the k
...

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3088988915)
Pushed c126475ed7a17ec9030066056e31846c7124dcf3 with a CI run on master branch at https://github.com/testing-cirrus-runners/bitcoin2/actions/runs/16368410249

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215709407)
switched to `job-id` in 9458cd0e66d

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215710431)
This now uses `git rev-parse HEAD:depends` in both 868ddf55718 and 1b689cf7811

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215711488)
comment removed in 9458cd0e66d

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215715420)
All references to `master` have been switched to `github.event.repository.default_branch` where appropriate. Removed the `ref_name` from the save/restore keys, as it is indeed unnecessary.

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215717870)
9458cd0e66d now uses a depends sources cache per job_id to avoid this pitfall (thanks!):

```
key: depends-sources-${{ inputs.job-id }}-${{ env.DEPENDS_HASH }}
```

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215719446)
Moved this code into the main `ci.yml` file, so that the cache actions are *only* performing caching (rather than being responsible for setting env vars), both in 868ddf55718 and 1b689cf7811

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215719957)
Comment added in db78efac768

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2215720797)
Registry env vars updated in 7272f56a755

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3089009002)
Thanks for the review @maflcko, I hope I addressed all your current review comments.

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2215722531)
Maybe not because comment above explicitly mentioned add, but that can be modified to match |.

see https://github.com/ismaelsadeeq/bitcoin/commit/95ea4e737612c094dfcb6414f8005c79c6a34551