👍 pablomartin4btc approved a pull request: "test: Add coverage for dumptxoutset failure robustness"
(https://github.com/bitcoin/bitcoin/pull/30817#pullrequestreview-2283624689)
cr & tACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7

💬 ryanofsky commented on pull request "cmake: decouple `FORTIFY_SOURCE` check from `Debug` build type":
(https://github.com/bitcoin/bitcoin/pull/30824#discussion_r1745887633)
The check above reminds me of the part of the XZ backdoor where the attacker inserted a single . character to disable a linux security feature in a similar cmake check:

https://git.tukaani.org/?p=xz.git;a=blobdiff;f=CMakeLists.txt;h=d2b1af7ab0ab759b6805ced3dff2555e2a4b3f8e;hp=76700591059711e3a4da5b45cf58474dac4e12a7;hb=328c52da8a2bbb81307644efdb58db2c422d9ba7;hpb=eb8ad59e9bab32a8d655796afd39597ea6dcc64d

So I think it might be useful to add a sanity check to confirm that `cxx_supports_forti
...

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2332233857)
@sdaftuar Added your benchmarks as a commit to this PR (I've reworked it to auto-detect the transaction count).

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745897934)
Taken, thanks.

💬 fanquake commented on pull request "cmake: decouple `FORTIFY_SOURCE` check from `Debug` build type":
(https://github.com/bitcoin/bitcoin/pull/30824#discussion_r1745898836)
I am certainly open to additional approaches in regards to the (potential fragility) of online source.

> So I think it might be useful to add a sanity check to confirm that cxx_supports_fortify_source is actually true in release builds:

I have a PR to try and do this in #27038.

👍 ryanofsky approved a pull request: "cmake: decouple `FORTIFY_SOURCE` check from `Debug` build type"
(https://github.com/bitcoin/bitcoin/pull/30824#pullrequestreview-2283657342)
Code review ACK 30803a35d54acda19ded88474c205f8954fea5e1

💬 ryanofsky commented on pull request "cmake: decouple `FORTIFY_SOURCE` check from `Debug` build type":
(https://github.com/bitcoin/bitcoin/pull/30824#discussion_r1745902560)
> I have a PR to try and do this in #27038.

Nice that seems potentially more reliable than suggested sanity check

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745904431)
Will comment elsewhere as well, but I just updated this limit to be a limit on the number of distinct clusters that are conflicted.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745906461)
Taken

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745907045)
Taken

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745907713)
Removed

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745908827)
Deleted.

💬 instagibbs commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1745889618)
I think it's clearer to name this `p2p_headers_presync` as well as the file.

💬 instagibbs commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1745881799)
won't this always build off genesis?

💬 instagibbs commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1745887683)
I modified this harness to use `REGTEST` chainparams with a minimum chainwork setting of `0x0` and the harness has yet to fail, fwiw.

Might need to make sure this harness is getting deep enough to actually check what's being intended?

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745913307)
Thanks, took this.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-2332274663)
Thanks @glozow for the test improvements. I've also made a change to an RBF limit that I think is worth mentioning. Rather than limit the number of direct conflicts that a transaction can have, I've now implemented the limit to be on the number of distinct clusters that the conflicts of a transaction belong to.

The idea is still to create some bound on the amount of CPU we might spend linearizing clusters and doing feerate diagram checks when processing a single transaction. Limiting the n
...

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1745941441)
is f94c546ac02ad11ba73174cb8d25220515cee893 the first commit that you can assert optimality? I'm not sure having the benchmark in prior to that is so meaningful

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1745945489)
also seems like the assertion fails for me at that commit

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1745962506)
Nice catch. Apparently the example graphs were obtained by finding 100k-1M iterations-optimal clusters *with LIMO* (so while passing in an already-good linearization as input), while the benchmarks didn't pass in an existing linearization.

@sdaftuar is going to construct new ones.