💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2114271026)
Yes, see `verbose|verbosity` in `getblock` for example.

💬 fanquake commented on issue "Assertion failed: TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state) (wallet/wallet.cpp: AddToWallet: 1094)":
(https://github.com/bitcoin/bitcoin/issues/32625#issuecomment-2919878321)
> This was fixed in v29, specifically in https://github.com/bitcoin/bitcoin/pull/31757.

Is that the right PR? Those changes are not in the 29.x branch.

💬 luke-jr commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-2919900895)
I'm not sure it makes sense to adjust this based on dbcache size. Won't a given batch size use the same amount of memory regardless of the size of the dbcache?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2919920634)
The tidy job is failing because it doesn't like the logging jobs used in lambdas. It seems like this is pre-existing so I'm not sure why it's failing now. The windows cross-built job is failing on the `rate_limiting` test at every file-size comparison. I don't have a windows machine to debug this, but I think maybe `fs::file_size` is failing or some other quirk is showing up?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2114308684)
I can do that in a follow-up

👋 romanz's pull request is ready for review: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540)

💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2919935290)
Ran apples to apples out of curiosity. One example ran marginally slower on this PR (but still very fast), many are many multiples faster and some see 500x+ improvements.

💬 l0rinc commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-2919940651)
> Won't a given batch size use the same amount of memory regardless of the size of the dbcache

It would, the assumption was that the user should be able to signal how much leftover memory they have - if they start the app with dbcache of 40MiB, allocating an extra 16MiB can be acceptable, but allocating an extra 64MiB can push the node over the edge. Even though we're not (yet?) preallocating the batch string, doubling the size to accommodate the content would end up with a similar size.
How
...

💬 instagibbs commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#issuecomment-2919973134)
concept ACK

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2920042768)
> > While this approach introduces some duplication at present, it will ultimately enable us to consolidate how the IWYU tool is executed, allowing for a choice between using the iwyu_tool.py script or CMake's built-in functionality.
>
> Reading the PR description, it's not really clear why we'd need to retain using both tools? Either the CMake built-in should be better in some way, or at least equivalent in functionality, in which case we should just switch to using it?

I believe that the
...

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2114397076)
> Not sure if this is a valid concern, but an alternative approach would be to just call `git diff` below on the `TARGETS_WITH_FORCED_IWYU` folders (not targets). Then exit, when the diff is not empty?

Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2920042768).

👍 ryanofsky approved a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2878980302)
Code review ACK 1def3bc3773b3013a92f54daa75acb82c30beaac. Only change since previous review is implementing a previous suggestion and passing a single cache temporary coinsview instead of a double cache to ConnectBlock.

💬 ryanofsky commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2114374015)
re: https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2109284630

> I don't remember what the original was based on, so took the suggestion.

Suggestion seems like a good change since it reduces complexity and makes new code more similar to existing code.

But maybe one possible motivation or advantage of passing `ConnectBlock` a `CCoinsViewCache` on top of another `CCoinsViewCache` on top of `chainstate.CoinsTip()` instead of passing a single `CCoinsViewCache` on top of `chainstat
...

💬 l0rinc commented on pull request "Draft: CCoinMap Experiments":
(https://github.com/bitcoin/bitcoin/pull/32128#discussion_r2113859955)
`SipHash` is decent (I tried several other candidates and most were slower), but not particularly fast (even a general SHA256-shani is in the same ball-park).
`RapidHash` is much simpler and, on short inputs, dramatically faster:
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.06 | 16,529,344,089.83 | 0.2% | 11.00 | `RapidHash_32b`
| 0.96 |
...

👋 l0rinc's pull request is ready for review: "blocks: force hash validations of blocks read from disk"
(https://github.com/bitcoin/bitcoin/pull/32638)

💬 sipa commented on pull request "Draft: CCoinMap Experiments":
(https://github.com/bitcoin/bitcoin/pull/32128#discussion_r2114434674)
Perhaps I shouldn't have used the term "cryptographic"; I see it's referred to as "secure" instead of "cryptographic" in some places.

There are two properties that a (keyed) hash function needs to be cryptographic:
* Indistinguishable from a random function
* Have larger enough output to resist practical brute-force preimage or collision attacks.

RapidHash has neither. SipHash has the first (it was designed by cryptographers, and investigated cryptanalytically). HMAC-SHA256 has both.


...

💬 furszy commented on issue "Assertion failed: TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state) (wallet/wallet.cpp: AddToWallet: 1094)":
(https://github.com/bitcoin/bitcoin/issues/32625#issuecomment-2920129544)
> Is that the right PR? Those changes are not in the 29.x branch (I can add them to [#32589](https://github.com/bitcoin/bitcoin/pull/32589) if we want to backport).

Hmm yes, I thought we had included it on 29.x with [gui#864](https://github.com/bitcoin-core/gui/pull/864).. (these two combined cause the app to no longer open).

📝 ryanofsky opened a pull request: "Update libmultiprocess subtree to fix clang-tidy errors"
(https://github.com/bitcoin/bitcoin/pull/32641)
Includes:

- https://github.com/bitcoin-core/libmultiprocess/pull/165
- https://github.com/bitcoin-core/libmultiprocess/pull/173
- https://github.com/bitcoin-core/libmultiprocess/pull/172

These changes are needed to fix CI errors in #31802.

The changes can be verified by running `test/lint/git-subtree-check.sh src/ipc/libmultiprocess` as described in [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees) and [lint instructions](https://github.
...

💬 davidgumberg commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2114520701)
(Sorry for the churn here)

I think we actually want to drop this `if` guard here and above, the CI job should fail right here if the path to `vsdevcmd.bat` breaks, so that the cause/fix is obvious.

💬 gmaxwell commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2920277088)
@instagibbs I believe the only cases that should be slower with SFL are either very small examples (where their time is irrelevant because its very low per tx compared to bigger clusters) and ones with huge numbers of dependencies. Beyond huge dependencies being generally unrepresentative they're also expensive to generate because every dependency needs a vin, they also tend to be so slow with both that I guess neither will run to completion in practice.

A better way to compare across imp
...