💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114104811)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

Single scenario that boots out 2+ txns with a single `CheckNumEvictions` seems apt

💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2919737881)
Actually, we can now use 0.16.6: https://github.com/lief-project/LIEF/releases/tag/0.16.6, which includes that change.

💬 ryanofsky commented on pull request "Mining: Avoid copying template CBlocks":
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-2919753784)
> The constness of methods is a property of the interface. It is a way to say calling it will not change the object

Yes, I understand the intent of marking `interfaces::BlockTemplate` methods `const` is to say calling those methods will not change the object. And I agree using `const` to prevent changing the object would be a nice goal, but unfortunately that's not what `const` keyword does here.

Marking the methods `const` does not mark the block template data `const` or do anything that
...

📝 fanquake converted_to_draft a pull request: "Mining: Avoid copying template CBlocks"
(https://github.com/bitcoin/bitcoin/pull/32547)
Refactoring to avoid unnecessary copies/complexity, at least in the mainnet paths.

Also abstracts out a new `TemplateToJSON` function to keep track of variable scoping better.

💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2919777106)
Backported to `27.x` in #32479.

💬 fanquake commented on pull request "[28.x] Backport guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32639#issuecomment-2919815304)
Guix Build:
```bash
f68d83b36bec27fd42c754af357d205c9abf7e2e90b69e7d1f9136813a191745 guix-build-5c2ba9f583e2/output/aarch64-linux-gnu/SHA256SUMS.part
e29852e1e6905d2c90734c39cf2994680b83228148342e25b0f44b44a6962a1a guix-build-5c2ba9f583e2/output/aarch64-linux-gnu/bitcoin-5c2ba9f583e2-aarch64-linux-gnu-debug.tar.gz
949df5bf91e782332065362d02b77ea8f2686433eb461e2458b91996cf97cd8f guix-build-5c2ba9f583e2/output/aarch64-linux-gnu/bitcoin-5c2ba9f583e2-aarch64-linux-gnu.tar.gz
eb18222f14ad9cec
...

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114242408)
what if the peer sent us the first non-reconsidered orphan? would that not be considered below due to sequence number being 0?

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2114255993)
I have benchmarked both approaches (reading block undo data directly from storage and returning re-serialized spent txouts) using https://github.com/bitcoin/bitcoin/pull/32540/commits/d5828eadc39772bf825eb4141340a281f4a490a7 with https://github.com/romanz/bench-rest/commit/f90aaddf1c3a0c0687f373cc89b0c343e35df739:

## Fetching block undo data (using `ReadRawBlockUndo`)
```
$ time cargo run --release --bin blockundo
Finished `release` profile [optimized] target(s) in 0.02s
Running
...

✅ fanquake closed an issue: "Bug in PartiallyDownloadedBlock fuzz test logic or behavior"
(https://github.com/bitcoin/bitcoin/issues/32640)

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