💬 mzumsande commented on pull request "validation: Do less work in NeedsRedownload":
(https://github.com/bitcoin/bitcoin/pull/31714#issuecomment-2905395016)
I just thought that there should be a point in time where this edge-case scenario with multiple switches between pre-segwit and post-segwit datadirs would not be sufficiently realistic anymore to justify that each node runs these extra checks with each startup.

I could change the PR to describe this switching back-and-forth scenario explicitly (it alludes to it with "less thorough" / "most typical case"), but the currently saved time during startup (~40milliseconds on mainnet for me) does not
...

💬 achow101 commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#issuecomment-2905414873)
> nit: "prividing" -> "providing"

Fixed

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2105211527)
`musig()` expressions are a key expression, which itself contains multiple key expressions. It needs to be able to increment `key_exp_index` such that the caller will also know what the `key_exp_index` has been incremented to. The easiest way to do this was a reference.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2105211725)
Done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2105211791)
Done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2105211849)
Done

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2105212407)
Would this be more descriptive?
```suggestion
LogDebug(BCLog::COINDB, "Writing chainstate to disk: flush mode=%s, prune=%d, cache_large=%d, cache_critical=%d, periodic=%d",

```

💬 jonatack commented on issue "test: `tool_wallet.py` references no-longer used CI":
(https://github.com/bitcoin/bitcoin/issues/32576#issuecomment-2905417856)
@m3dwards those comments can be dropped completely; see #28116, which I will update and open.

💬 achow101 commented on pull request "walletdb: Log additional exception error messages for corrupted wallets":
(https://github.com/bitcoin/bitcoin/pull/32598#issuecomment-2905423874)
> Should also change [HasLegacyRecords()](https://github.com/bitcoin/bitcoin/blob/2df824f4e62b6bc569044819cd64f66f3839ba13/src/wallet/walletdb.cpp#L544) as it would log the same error twice with this change.

Done

💬 davidgumberg commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2105223515)
nit for here and above:
```suggestion
$installationPath = & $vswherePath -latest -property installationPath
```

or could drop the line where `$vswherePath` is defined, whatever your preference is.

💬 jonatack commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2105227345)
That does seem nicer.

🤔 jonatack reviewed a pull request: "blocks: avoid recomputing block header hash in `ReadBlock`"
(https://github.com/bitcoin/bitcoin/pull/32487#pullrequestreview-2865412103)
Concept ACK

💬 jonatack commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2105238684)
This error log change no longer provides info about the index that doesn't match.

Examples from the unit test log:

`$ ./build/bin/test_bitcoin --run_test=blockmanager_tests -l test_suite -- DEBUG_LOG_OUT`

before
```
025-05-23T18:42:55.808136Z (mocktime: 2020-08-31T15:34:12Z) [test] [node/blockstorage.cpp:1033] [ReadBlock] [error] GetHash() doesn't match index for CBlockIndex(pprev=0x6000019085e0, nHeight=100, merkle=851b8e14be513191d6dd7df5ac3c997c2b0b1f58c99143905eac61b296d01169, has
...

💬 jonatack commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2105241036)
(Thank you for adding the unit test!)

🤔 jonatack reviewed a pull request: "walletdb: Log additional exception error messages for corrupted wallets"
(https://github.com/bitcoin/bitcoin/pull/32598#pullrequestreview-2865443270)
Concept ACK, would be great to add test coverage for this change.

💬 pinheadmz commented on issue "rpc: Wrong `gettransaction` info for a coinjoin":
(https://github.com/bitcoin/bitcoin/issues/14136#issuecomment-2905521838)
I'm digging in to this issue, it has two closed PRs: #16199 and #19002 and a test asserting the current, somewhat unexpected, behavior: #18919

There are two things to consider:

1. We don't store input amounts in the wallet, only output amounts -- and only output amounts we create. That means we can't compute the fee for a tx unless we created all the outputs the tx is spending.
2. In the context of a coinjoin a "fee" field could refer to either the actual network fee paid to the miner, OR just
...

💬 gmaxwell commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2905537657)
I don't recall if the vulnerability related to requesting the transaction was closed (relay pool limited its scope at least to recently announced txn) but there were complications related to orphan txn (e.g. that relaying a transaction ought to imply permission to fetch its parents if they are still in your memory pool). But if that vulnerablity hasn't been closed it ought to be closed.

Ignoring the unrequested CB is desirable for non-privacy reasons, specifically because if it doesn't someone
...

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2105273746)
Yes, there is no index available, only the desired hash, wherever it comes from.
Would it help if we included the two different hashes in the error?

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2105277605)
Good call, changed, rebased, added you as co-author.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2105280293)
Ah. I've added a test which exercises this code.