💬 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"))'`

💬 maflcko commented on pull request "assumevalid: log every script validation state change":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238733043)
This is racy and will fail CI. You can reproduce this by adding a `time.sleep(9)` before the `with`?

💬 l0rinc commented on pull request "assumevalid: log every script validation state change":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238737896)
Thanks, was wondering why it's failing on some platforms. Do you have a better alternative?

💬 maflcko commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3130976782)
The remaining ones don't speed up IBD, do they?

💬 maflcko commented on pull request "assumevalid: log every script validation state change":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238745438)
The `with` scope will have to be increased, it you want to keep it

💬 l0rinc commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3130990941)
The big ones are merged, thanks for your help.
These are smaller ones - at least from an IBD perspective, but both are quite simple.

💬 l0rinc commented on pull request "assumevalid: log every script validation state change":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2238768642)
Makes sense, thanks for the pointer!

⚠️ Sjors opened an issue: "wallet: control which taproot script path to spend"
(https://github.com/bitcoin/bitcoin/issues/33084)
When there's multiple script paths we can spend, the wallet just (partially) signs all of them. We then pick the one with the lowest fees:

https://github.com/bitcoin/bitcoin/blob/321984705dbcc5982013c8cfb52389cf3f2e75f6/src/script/sign.cpp#L373-L389

The user may want to override this, e.g.:

1. For privacy reasons they don't wish to reveal the less secure (but cheaper) fallback condition.
2. Their co-signer software trips up if it sees something in the `taproot_script_path_sigs` it can't han
...