💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818600499)
Update: I realized that at least for now, there is a requirement that any added transactions are added in a topological order, with parents coming before children -- this is so that we can add things to the mempool's multi_index at `Apply()` time without creating any missing dependencies. But removals can happen in any order and can be interleaved with the additions.

💬 maflcko commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2440916567)
Hex parsing is used in mining via `DecodeHexBlk`, no?

Also, while some of the benchmarks do not exist to be optimized, but merely to give a feeling and to avoid a regression where an algorithm is swapped to go from `O(n)` to `O(n^2)`. So in a sense, they are a bit of a unit test.



In any case, updating the docs should be uncontroversial and useful going forward.

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2440960117)
review ACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53 🗜

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 9f243cd7fa66
...

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2441018301)
Looks like that fixed it. However, it would be good to understand *why* it fixed it. Looking at the diff that triggered it, I fail to see how (https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)

💬 maflcko commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2441027190)
Can this be closed, or is it waiting on something?

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818548647)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
Not sure it's worth exposing this external method if the only reason to have it is testing.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818553463)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5

worth adding a comment on what would happen if there are 3 colliding transactions (probably fine but some text would be helpful) :)

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818568903)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
could *this* be used to game collisions (for *wtxid* you have here)?

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818583090)
392facfce1be6d0395abecc2e049ff39635bd1a5

you can technically feed it arbitrary transactions. I think the comment should say it, and then later say "we normally pass a list of parents to determine whether [... ... ...]".

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818575030)
392facfce1be6d0395abecc2e049ff39635bd1a5

why do you think force relay should influence whether we reconcile or flood? The transaction is relayed in either case (the original meaning of force relay)

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818555798)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
what would happen if a descendant was already flooded?

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818577080)
392facfce1be6d0395abecc2e049ff39635bd1a5

these kind of refactors we better apply where the code was introduced, in the older commits of this PR

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818598067)
392facfce1be6d0395abecc2e049ff39635bd1a5

what about the order of the parents here? Deserves a comment at least. With the current non-cluster mempool it might break something?

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818670032)
20772c077832592265bd6a2876aa6b4cb7dbde7d

with this code, and especially the 5/2 trickle delay (very long), previous performance measurements become outdated.

My idea was to [delay the responses](https://github.com/bitcoin/bitcoin/pull/21515/commits/f99fa469115856aefa9799b3fc62ba09985933ad). And by this time, the hope was flooding does enough job so that scanning the sets through many connections does not work.

Also, adding to a set already happens on a trickle... so that's another level
...

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818584774)
392facfce1be6d0395abecc2e049ff39635bd1a5

why not modifying the former commit that introduces this code?

💬 fanquake commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2441225040)
Guix Build:
```bash
541e5a857759e6e34aa8020e70b2b89be7cadfb358aecb0d83c10df214c5b1f7 guix-build-40e5f26a3ff7/output/aarch64-linux-gnu/SHA256SUMS.part
3973c1c93d969f4b1a338c734df151db3bdfdca0fcb5800f126890a426d5b35d guix-build-40e5f26a3ff7/output/aarch64-linux-gnu/bitcoin-40e5f26a3ff7-aarch64-linux-gnu-debug.tar.gz
4cdeb68207b4f71819618b53f1c463f368ccca2912617945caceb49049f30a80 guix-build-40e5f26a3ff7/output/aarch64-linux-gnu/bitcoin-40e5f26a3ff7-aarch64-linux-gnu.tar.gz
5da6d6e1ab2ac471
...

🚀 fanquake merged a pull request: "Drop miniupnp dependency"
(https://github.com/bitcoin/bitcoin/pull/31130)

✅ fanquake closed an issue: "p2p_headers_presync fuzz target times out when FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION isn't set"
(https://github.com/bitcoin/bitcoin/issues/30950)

🚀 fanquake merged a pull request: "Introduce `g_fuzzing` global for fuzzing checks"
(https://github.com/bitcoin/bitcoin/pull/31093)

💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2441276551)
`a8e531f92f...f42561e667`: rebase and address suggestions

💬 brunoerg commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818861531)
In c1a5d5c100b1628456acfa6129e303737f0ad4d3: Any specific reason to remove the comment?