💬 maflcko commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2747808769)
lgtm ACK 1718da848a0a7591f0eab086e159f1e9e0f2c59b

This should also address the concern by @ryanofsky in https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2451865832 to some extent IIUC. Of course, it is still impossible to execute a fuzz test case normally in a release-mode non-fuzz build, but this pull offers one workaround:

* Picking a debug-mode build to build all binaries and manually overriding the cxx-flags to increase the optimization level to the level of a release-mode bu
...

💬 Eunovo commented on pull request "descriptors: taproot partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2747824269)
Rebased on master https://github.com/bitcoin/bitcoin/commit/770d39a37652d40885533fecce37e9f71cc0d051
- changed `Span` to `std::span`

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-2747830190)
Rebased on master https://github.com/bitcoin/bitcoin/commit/770d39a37652d40885533fecce37e9f71cc0d051
- fix failing functional test by changing `self.send_message` to `self.send_without_ping` due to https://github.com/bitcoin/bitcoin/commit/fa9cf38ab666b50b0d8e82cb17f9e5a8a613547d

💬 0xB10C commented on issue "RFC: Macro Regression Test Suite for Historical Reorgs":
(https://github.com/bitcoin/bitcoin/issues/32130#issuecomment-2747839536)
Instead of using mainnet blocks with a reorg maybe every 1000-3000 blocks, one could consider using testnet4 blocks as it often has multiple reorgs per block and blocks are a lot smaller and faster to load.

See for example https://fork.observer/?network=4

💬 maflcko commented on issue "RFC: Compact Block Reconstruction Macro Benchmark Suite":
(https://github.com/bitcoin/bitcoin/issues/32131#issuecomment-2747858696)
> * Setting up a node by syncing up to a known block height (e.g., block 840,000, ideally via quick AssumeUTXO seeding).
>
> * Fetching the next few blocks from the network (lazy-init from network, caching the blocks locally) and adding their transactions to the local mempool.
>
> * Replaying compact block announcements and measuring reconstruction performance

Maybe I am missing something obvious, but I don't think this is possible to detect the effects of mempoolfullrbf (one motivation for t
...

🤔 l0rinc requested changes to a pull request: "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script"
(https://github.com/bitcoin/bitcoin/pull/32116#pullrequestreview-2710061531)
Concept ACK - I think we can do a bit more cleanup, though

💬 l0rinc commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2009999146)
👍, `read_varint` and `deser_varint` implementations match exactly (except for description, which we may move over there)

💬 l0rinc commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2010014890)
Could we import the `MAGIC_BYTES` from `test_framework.messages` as well (we might have to extend it with Testnet4)?

💬 l0rinc commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2010017508)
👍, `decompress_amount` implementations match exactly

💬 l0rinc commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2010001864)
Seems to me `deser_compact_size` does and extra little endian conversion - for a single byte, which seems redundant, we might as well remove it from there to match this implementation

💬 l0rinc commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2010025936)
this is a bit hacky, but I don't mind if we can dedup

🤔 rkrux reviewed a pull request: "qa: make feature_assumeutxo.py test more robust"
(https://github.com/bitcoin/bitcoin/pull/32117#pullrequestreview-2710081823)
Concept ACK a7c57cf48a147c3ae3709f0630a3ba175aa4c841

This is much clearer compared to last time. I have reviewed this PR from the POV of someone new to the `assumeutxo` feature and asked a question because there appears to be two points that I have not been able to tie together yet.

💬 rkrux commented on pull request "qa: make feature_assumeutxo.py test more robust":
(https://github.com/bitcoin/bitcoin/pull/32117#discussion_r2010011383)
The comment says this is an invalid number of coins here but the validation code does actually read the said number of coins. Maybe mention the reason why the number of coins is invalid at this stage?
https://github.com/bitcoin/bitcoin/blob/770d39a37652d40885533fecce37e9f71cc0d051/src/validation.cpp#L5928-L5938

Mentioned this because the actual test failure error is caused by invalid height being passed in the serialized coin data, I'm trying to tie it up to why the number of coins is invali
...

💬 rkrux commented on pull request "qa: make feature_assumeutxo.py test more robust":
(https://github.com/bitcoin/bitcoin/pull/32117#discussion_r2010025399)
```diff
-ser_varint(300 * 2)
+ser_varint((SNAPSHOT_BASE_HEIGHT + 1) * 2)
```

💬 0xB10C commented on issue "RFC: Compact Block Reconstruction Macro Benchmark Suite":
(https://github.com/bitcoin/bitcoin/issues/32131#issuecomment-2747865062)
> Replaying compact block announcements and measuring reconstruction performance (multiple times for consistent and statistically meaningful results, given variability compared to stable micro-benchmarks)

What is the exact metric you are trying to measure? It's not 100% clear to me if you are trying to measure performance as in "speed" or performance as in "reconstructions without a round trip".

> Fetching the next few blocks from the network (lazy-init from network, caching the blocks locally
...

💬 rkrux commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2010029902)
+1, I felt the same but I feel it's worth getting rid of duplication.

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2709992001)
re-ACK 41b4434fed169570ce0976c6e58db0d4a9614aaa after https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2707582068

The new doc is better, and it's explicit that ref's can outlive their txgraph.

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2009970708)
As it is right now, it's unused in the main PR.

I've been thinking when `DoWork` can be called in the mempool lifecycle and whether that would make a difference because everything is done lazily and we only apply staged changes when we want to get the current state. (specifically a scenario where after some lazy additions it cheap to just call `DoWork` and not wait until we want to get a state.)


Maybe after trimming due to a reorg, after multiple additions of transactions e.g loading new
...

💬 l0rinc commented on issue "RFC: Macro Regression Test Suite for Historical Reorgs":
(https://github.com/bitcoin/bitcoin/issues/32130#issuecomment-2747870653)
Nice, we should do that as well!
I think it's important to be closer to the historical behavior as well - it may not be a tragedy if testnet behavior happens to change accidentally, but it is, if mainnet logic change isn't caught - that's why I insist on making it as realistic as possible.

💬 l0rinc commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2010045532)
Though this means this utility file isn't as self-contained anymore (i.e. you can't just copy it out of the project - not even sure that was possible before)