💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237910584)
Removed in e62bd1b24b6 ci: port tsan-depends-gui

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237906512)
I am actually not 100% sure why this is needed. Warp have it [documented in their docs](https://docs.warpbuild.com/cache/docker_layer_caching#step-1-set-up-docker-buildx-action), but IIRC Cirrus also advised us to set it.

My understanding is that it was necessary for the `gha` caching using custom URL, but perhaps @m3dwards or @fkorotkov could clarify/confirm?

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237908036)
Added a helper job (runtime of ~4 seconds) which has an output whether or not to use cirrus to the subsequent jobs in 8758b6ec333 ci: add job to determine runner type

This reduces the hardcoding to a single env var in the main ci.yml file, which can trivially be patched by forks if desired.

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237910261)
Removed dead code no-longer needed in 3825f903585 ci: port arm 32-bit job (and subsequent job commits)

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

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237950268)
Fixed, and now using `$PACKAGES` in 8bb213c64c3 ci: force reinstall of kernel headers in asan

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237909204)
Thanks, I agree.

Removed the input entirely and use `$CONTAINER_NAME` in d6ed832034b ci: add caching actions

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237935705)
Fixed in fd32aa85b29 ci: port lint

💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2237982299)
Removed the global makejobs in 89f177406c9 ci: dynamically match makejobs with cores

🤔 jonatack reviewed a pull request: "doc: add note for watch-only wallet migration"
(https://github.com/bitcoin/bitcoin/pull/32866#pullrequestreview-3064816090)
Approach ACK

💬 jonatack commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#discussion_r2238023764)
```suggestion
contains all of the watchonly scripts. `<name>_solvables` contains any scripts that the wallet
```

💬 jonatack commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#discussion_r2238023510)
```suggestion
wallets do not support having both private keys and watch-only scripts, there may be up to two
```

💬 luke-jr commented on pull request "[IBD] log the start of script validation past `assumevalid` block":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238060331)
fScriptChecks isn't a one-way gate. It would be very confusing to see "Started validation" due to a block not an ancestor of the assumevalid block, and then have validation of the real chain continue without validation and no log informing the user it's being skipped again.

💬 HowHsu commented on pull request "truc: optimize the in package relation calculation":
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2238103933)
> Would you consider using DepGraph instead of a matrix of indices (it's basically the same thing but more compact)?

Sure, I'll look into it.

💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#issuecomment-3130283784)
@sipa, my comments are still relevant as far as I can tell, I think it's a good change, how can I help to advance it?

💬 l0rinc commented on pull request "[IBD] log the start of script validation past `assumevalid` block":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238145186)
Thanks, updated the code to log any such change, let me know what you think!

💬 ajtowns commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2238379741)
`GetIter` is already exposed, it's used in validation.cpp for looking up txs. This would also expose `GetInfo` which converts an iterator into a index-neutral summary of info about the tx.

💬 luke-jr commented on pull request "assumevalid: log every script validation state change":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238396232)
Probably fine/better to log this the first time through for clarity.

💬 l0rinc commented on pull request "assumevalid: log every script validation state change":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238494034)
I don't mind doing that, but we're already either logging:
> Assuming ancestors of block 00000000000000000001b658dd1120e82e66d2790811f89ede9742ada3ed6d77 have valid signatures.

or
> Validating signatures for all blocks.

Wouldn't it be redundant to always state if we're validating scripts or not and not just when the state changes?

💬 luke-jr commented on pull request "test: add option to skip large re-org test in feature_block":
(https://github.com/bitcoin/bitcoin/pull/33003#issuecomment-3130694316)
Seems like it would be better to split it into two, so the quicker part can be run by test_runner every time (without running it twice when the long reorg test is also desired)