#35591 [DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation)
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35591 [DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation) | Waiting on author | 0 | Neutral | 1,864 + 2,562 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
This pull request provides a complete implementation of Erlay (BIP 330) transaction reconciliation. It introduces an `OUTBOUND_FULL_RECONCILIATION` connection type, protocol message handlers (`reqtxrcncl`, `sketch`, `reconcildiff`, `reqsketchext`), sketch computation and decoding logic via minisketch, connection eviction handling, and functional and benchmark tests.
Problem: Transaction announcement via flood and trickle fanout consumes significant network bandwidth and limits how many outbound peers a node can maintain without excessive overhead. Erlay replaces redundant inv announcements with set reconciliation, lowering bandwidth consumption and allowing nodes to maintain additional outbound connections to resist eclipse attacks.
Reviewability
Paused · Waiting on author
Open objections from bartoli with no author reply for 37 days; review would repeat what has been asked.
Waiting on (from the thread read):
- author: response to bartoli's C++ code suggestions 2026-09-25
Agreement
Neutral
objections settled, nobody has spoken for the PR
Thread notes: Added bartoli as a neutral participant and recorded two C++ suggestions regarding const ref return and vector sizing.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| brunoerg (member) | objection, correctness | test coverage gaps in extension handling, boundary conditions, INV relay side effects, and state cleanup leave edge case bugs undetected | fixed · fix pushed · author covered relevant mutants in tests · v1, not yet re-checked | answered | yes | 2026-06-29: 'The survivors are mostly about what happens around the edges of the protocol, not the central flow.' Status from: 2026-08-17: sr-gi: 'Finished covering the most relevant mutants. Left out the ones regarding extensions, as I'm still debating whether those' |
| brunoerg (member) | objection, correctness | reconciliation peers could exceed m_max_outbound_full_recon during v2-to-v1 fallback due to a race condition lacking eviction handling | fixed · fix pushed · outbound reconciliations now have eviction handling · v1, not yet re-checked | answered | yes | 2026-08-31: 'Could reconcilitation peers exceed `m_max_outbound_full_recon` during v2-to-v1 fallback? Thinking about a possible race.' Status from: 2026-09-04: sr-gi: 'With this, I should have also covered your concern about v2->v1 downgrades, as outbound reconciliations now have evictio' |
| brunoerg (member) | objection, safety | malicious peers can trigger expensive minisketch decoding taking 30+ seconds, blocking message processing and causing a DoS | answered · author replied in rc:3902587961 · v1, not yet re-checked | answered | yes | 2026-08-31: 'I got 31.2 seconds on my machine (Ryzen 9 7900). Wondering if a malicious peer could use it to cause a DoS since it would block all msgs processings?' Status from: 2026-09-01: sr-gi: '' |
| brunoerg (member) | objection, interface | non-conformance with BIP 330 by accepting any non-zero byte as true instead of strictly 0 or 1 | fixed · fix pushed · recon_result parsing updated and test added · v1, not yet re-checked | answered | yes | 2026-08-31: 'nit: I think that, according to the BIP330, we must accept 0 or 1 here. This way it would treat any nonzero byte as true.' Status from: 2026-09-01: sr-gi: 'I changed the way we parse `recon_result` to mimic the check we already have in place for BIP152. Also added a test for ' |
| bartoli (none) | suggestion, style | return a const reference instead of making a copy | open | answered | no | 2026-09-25: 'This could return a const ref (even though all current calls would make a copy anyway)' |
| bartoli (none) | suggestion, style | set size once to skdata.size() + m_remote_sketch_snapshot.size() before copying values | open | answered | no | 2026-09-25: 'first set size once to skdata.size()+m_remote_sketch_snapshot.size then copy values?' |
Participants:
- brunoerg (member, objection, 4 statements 2026-06-29 to 2026-08-31): reported mutation testing gaps, raised DoS concern regarding quadratic minisketch decoding times, noted potential connection overage during v2-v1 downgrade, and noted BIP 330 recon_result parsing
- bartoli (none, neutral, 3 statements 2026-09-25 to 2026-09-25): made generic C++ suggestions on const ref return and vector sizing
Files
2592 lines under test/bench/ci.
- src/test/txreconciliation_tests.cpp +1109/-24
- src/node/txreconciliation_impl.cpp +848/-0
- src/net_processing.cpp +465/-114
- test/functional/test_framework/p2p_txrecon.py +330/-0
- test/functional/p2p_txrecon_responder.py +328/-0
- src/node/txreconciliation.h +214/-58
- test/functional/p2p_txrecon_initiator.py +266/-0
- src/node/txreconciliation_impl.h +231/-0
- src/node/txreconciliation.cpp +0/-170
- src/test/denialofservice_tests.cpp +117/-0
- test/functional/test_framework/messages.py +113/-1
- test/functional/p2p_reqtxrcncl.py +111/-0
- src/bench/txreconciliation.cpp +64/-0
- test/functional/p2p_sendtxrcncl.py +49/-2
- test/functional/p2p_txrecon_disabled.py +51/-0
- src/net.h +36/-13
- src/net.cpp +20/-13
- src/protocol.h +25/-0
- test/functional/test_framework/p2p.py +15/-1
- src/node/connection_types.h +10/-0
- src/init.cpp +6/-1
- src/rpc/net.cpp +4/-1
- test/functional/test_runner.py +4/-0
- src/qt/guiutil.cpp +2/-1
- src/CMakeLists.txt +1/-1
- src/bench/CMakeLists.txt +2/-0
- src/node/connection_types.cpp +2/-0
- src/test/fuzz/connman.cpp +1/-1
- test/functional/test_framework/test_node.py +1/-1
- src/test/util/net.h +1/-0
Card
The short text the ranking pass compares across PRs.
This pull request implements the full Erlay transaction reconciliation protocol (BIP 330) along with an outbound-full-recon connection type to dramatically reduce transaction announcement bandwidth while increasing outbound connectivity against eclipse attacks. The change is strategically important (P2 in p2p) as a network-wide scaling and security enhancement. Technical review is positive, with brunoerg providing mutation test reports and raising DoS concerns regarding quadratic minisketch decode times. The PR currently needs a rebase and the author intends to carve it into smaller reviewable PRs for actual merging.
Data
thread read through 2026-09-25 (24 statements), head 37912ec178
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Needs rebase
Categories
P2P
P2 · new feature (ranking pass: Problem is major, but this artifact is marked DO NOT MERGE and will be split; its benefit as a reviewable change is the least certain.)
Implements BIP 330 Erlay transaction reconciliation to drastically reduce transaction announcement bandwidth and allow nodes to maintain more outbound connections against eclipse attacks. While a major strategic protocol upgrade, delay does not risk immediate network correctness or fund safety.
Membership: Modifies net_processing.cpp, net.cpp/h, and protocol.h to implement Erlay (BIP 330) transaction reconciliation and new connection types.
Factors: security/stability 1, bug 0, performance 2, user value 2, leverage 1