Page MenuHomePhabricator

[p2p] opportunistically accept 1-parent-1-child packages
ClosedPublic

Authored by Fabien on Jul 17 2024, 19:20.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABCd0b3d2524b06: [p2p] opportunistically accept 1-parent-1-child packages
Summary
 This is "non-robust 1-parent-1-child package relay" which is immediately useful.

 [...]

(1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463.
(2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer.
(3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031.

Completes backport of core#28970.
https://github.com/bitcoin/bitcoin/pull/28970/commits/6c51e1d7d021ed6523107a6db87a865aaa8fc4c9
https://github.com/bitcoin/bitcoin/pull/28970/commits/87c5c524d63c833cf490c7f2f73d72695ad480df
https://github.com/bitcoin/bitcoin/pull/28970/commits/e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f

Depends on D16493.

Note that the tests have been minimally adapted to fit our codebase, but
they are still testing the same things.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Jul 17 2024, 19:20

Failed tests logs:

====== Bitcoin ABC functional tests: p2p_opportunistic_1p1c.py ======

------- Stderr: -------
File "/work/test/functional/p2p_opportunistic_1p1c.py", line 350
    f"removed orphan tx {high_fee_child["txid"]}",
                                         ^
SyntaxError: f-string: unmatched '['

Each failure log is accessible here:
Bitcoin ABC functional tests: p2p_opportunistic_1p1c.py

Failed tests logs:

====== Bitcoin ABC functional tests: p2p_opportunistic_1p1c.py ======

------- Stderr: -------
File "/work/test/functional/p2p_opportunistic_1p1c.py", line 350
    f"removed orphan tx {high_fee_child["txid"]}",
                                         ^
SyntaxError: f-string: unmatched '['
====== Bitcoin ABC functional tests with the next upgrade activated: p2p_opportunistic_1p1c.py ======

------- Stderr: -------
File "/work/test/functional/p2p_opportunistic_1p1c.py", line 350
    f"removed orphan tx {high_fee_child["txid"]}",
                                         ^
SyntaxError: f-string: unmatched '['

Each failure log is accessible here:
Bitcoin ABC functional tests: p2p_opportunistic_1p1c.py
Bitcoin ABC functional tests with the next upgrade activated: p2p_opportunistic_1p1c.py

Failed tests logs:

====== Bitcoin ABC functional tests: p2p_opportunistic_1p1c.py ======

------- Stderr: -------
File "/work/test/functional/p2p_opportunistic_1p1c.py", line 350
    f"removed orphan tx {high_fee_child["txid"]}",
                                         ^
SyntaxError: f-string: unmatched '['

Each failure log is accessible here:
Bitcoin ABC functional tests: p2p_opportunistic_1p1c.py

Using the same quote marks for an embedded dict key and its f-string is a Python 3.12+ feature, TIL

PiRK added a subscriber: PiRK.

One comment nit/typo.

test/functional/p2p_1p1c_network.py
30 ↗(On Diff #48708)

This has no effect as we are missing core#28970. But it does not hurt to leave it here, so we can't forget to add it later.

test/functional/p2p_opportunistic_1p1c.py
345 ↗(On Diff #48708)
This revision is now accepted and ready to land.Jul 18 2024, 08:58
test/functional/p2p_1p1c_network.py
30 ↗(On Diff #48708)

I meant core#27114

Fix comment nit, and remove the unecessary noban_tx_relay

test/functional/p2p_1p1c_network.py
30 ↗(On Diff #48708)

Yes I think I should remove it instead so we don't get puzzled when backporting that PR. I added the whitelist below that achieves the same anyway.