💬 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)

💬 ajtowns commented on pull request "refactor: Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3130725962)
ACK dd392a64bb0608847f771f8b1f09c2fcae146923

Shouldn't the copyright dates in these files be updated? `git blame` has some lines from 2022 in intro.h and 2024/25 in intro.cpp.

You could avoid the circularity with something like:

```diff
diff --git a/src/qt/freespacechecker.h b/src/qt/freespacechecker.h
index d3a61a11571..214324be7c8 100644
--- a/src/qt/freespacechecker.h
+++ b/src/qt/freespacechecker.h
@@ -9,8 +9,6 @@
#include <QString>
#include <QtGlobal>

-class Intro;
-

...

💬 luke-jr commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3130771830)
Concept NACK, Bitcoin invoice addresses are single-use, and it doesn't make sense to put them in DNS.

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2238645631)
nit (from the llm):

PACAKGES -> PACKAGES [env var name typo]

💬 maflcko commented on issue "build: Build warnings from deprecated std::get_temporary_buffer in std::stable_sort calls":
(https://github.com/bitcoin/bitcoin/issues/33080#issuecomment-3130890341)
Pretty sure this was already fixed upstream in GCC stdlib ? What are the exact steps to reproduce and what is your GCC version? It does not happen in the CI, looking at https://github.com/bitcoin/bitcoin/actions/runs/16582003563/job/46899945634?pr=32989

💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3130944764)
The other CI failure looks interesting. I wonder if it can be fixed by replacing the hard-coded `CLI_MAX_ARG_SIZE` with `python3 -c 'import os; print(os.sysconf("SC_ARG_MAX"))'`