👍 rkrux approved a pull request: "functional test: Additional package evaluation coverage"
(https://github.com/bitcoin/bitcoin/pull/31152#pullrequestreview-2395429428)
tACK f32c34d0c3d4041a301822b27e88d6db4cbf631e

Successful make and functional tests. In my system, 1 out of the 72 txs in the mempool was evicted when this package of 3 parents and 1 child was submitted.

Thanks for adding this one, it is a fantastic functional test to have that tests different pieces, I enjoyed going through it. Asked couple questions for my understanding.

📝 fanquake opened a pull request: "[27.x] rc2 or final"
(https://github.com/bitcoin/bitcoin/pull/31154)
Not expecting much more here. This backports one other change (that doesn't warrant an rc), which fixes noisey output from newer versions of Clang (19+). If no other changes are needed/added, I'll push the changes for final.

🤔 jonatack reviewed a pull request: "bench: Remove various extraneous benchmarks"
(https://github.com/bitcoin/bitcoin/pull/31153#pullrequestreview-2395650389)
Not sure about some of these. People can be asked to add benchmarks for performance improvements or to verify that a change to existing code doesn't degrade performance, and having existing benchmarks to refer to may make that easier. Seems odd to throw out work like https://github.com/bitcoin/bitcoin/pull/22284.

Are any of the benchmarks called by outside sites that track their results over time?

💬 jonatack commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#discussion_r1816916312)
Not sure about removing the logging benchmarks. Logging can be very high frequency. Even if that is more likely to occur for developers, it may be useful to be able to track this.

💬 fanquake commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438144997)
Concept ACK - agree with stickies-v re updating the doc.

💬 instagibbs commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#issuecomment-2438159398)
> Is the preference to reject undefined verbosity values for new code (e.g. -1, 5)? If so, we can adjust that and any future verbosity levels would be accompanied by updated test(s).

Rejection now means we can expand usage later if desired, and is more obvious what's happening.

💬 instagibbs commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1816951913)
A gap can/does happen naturally in that the highest descendant feerate evicted, plus minrelay(1 sat/vbyte) becomes the new floating fee.

I guess `fill_mempool` does it in such a way that all the leftover transactions are higher than the new minrelay?

I'll take a longer look into seeing if we can guarantee this more directly somehow

💬 instagibbs commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1816952041)
> Typo reintrodce

Will fix if I touch again

There's no direct way to query that a trimming happened, no. Note that even if it was directly reported, the results can't really be cached since we're evicting things.

💬 instagibbs commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1816952141)
will fix if retouched

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1816952259)
I think this should be fine for performance. Large transactions take orders of magnitude more time to validate than what is being added here (and for small transactions this is extremely fast).

📝 brunoerg converted_to_draft a pull request: "fuzz: speed up addrman"
(https://github.com/bitcoin/bitcoin/pull/30688)
There is no need to iterate 10'000 times and
call `Add` 10'000 times at once (we only add
1000 addresses at once in practice). These values
were introduced in 214d9055acdd72189a2f415477ce472ca8db4191.

I see an avg of 40 exec/s on master for this target and 115 exec/s on this branch.

💬 brunoerg commented on pull request "fuzz: speed up addrman":
(https://github.com/bitcoin/bitcoin/pull/30688#issuecomment-2438187212)
I just moved it to draft since I'm doing more experiments yet.

💬 theuni commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2438201709)
Woohoo! I like that much better than a bump :)

💬 brunoerg commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438202076)
Concept ACK

💬 theuni commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2438214017)
Getting on the Concept ACK train. Another dependency gone!

Agree with @laanwj about the refactor as a separate PR.

💬 instagibbs commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438261191)
> Perhaps also worth updating doc/benchmarking.md with a brief section on which benchmarks are typically useful to add, and which benchmarks should rather be ephemeral (e.g. provided on a separate branch, supplementary to a PR)?

I think this documentation update would be helpful to get in even without this PR.

👍 theuni approved a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903#pullrequestreview-2395817738)
Concept ACK and quick review ACK.

Tested with depends and verified that the fallback isn't used:
https://github.com/theuni/bitcoin/commit/d98c76c69e01ddacc79974fef6f7fd0f9df5ddcc

@hebasto want to pull that in here?

💬 theuni commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#discussion_r1817015584)
Doubtful it matters here, but external libs after internal libs please.

💬 dergoegge commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438279436)
> People can be asked to add benchmarks for performance improvements or to verify that a change to existing code doesn't degrade performance

Imo, performance improvements (and accompanying benchmarks) should only be made to code that makes critical things slower than they need to be. Otherwise we're just improving performance for the sake of doing it, which is not something we have the resources for. We should aim our resources with intention, and the benchmarks can act as the scope.

For e
...

💬 dergoegge commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438280776)
Will add the doc changes next week