💬 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

💬 jonatack commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2438284288)
Approach ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0

👍 instagibbs approved a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2395838425)
reACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790

could use some fuzzing c @marcofleon

💬 sipa commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2438324004)
Concept ACK. While we don't actually know how many systems are out there which could be made reachable through UPnP but not through NAT-PMP/PCP, I suspect the number of them actually doing that is very low (given it being default off, and not widely advertized).

💬 sdaftuar commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#issuecomment-2438357266)
re-ACK f32c34d0c3d4041a301822b27e88d6db4cbf631e

💬 hebasto commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2438376673)
> Concept ACK and quick review ACK.
>
> Tested with depends and verified that the fallback isn't used: [theuni@d98c76c](https://github.com/theuni/bitcoin/commit/d98c76c69e01ddacc79974fef6f7fd0f9df5ddcc)
>
> @hebasto want to pull that in here?

Thanks! Pulled in.

💬 hebasto commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#discussion_r1817070022)
Sure. Reordered.