Skip to content

Commit d844b5e

Browse files
committed
Merge bitcoin/bitcoin#24152: policy / validation: CPFP fee bumping within packages
9bebf35 [validation] don't package validate if not policy or missing inputs (glozow) 51edcff [unit test] package feerate and package cpfp (glozow) 1b93748 [validation] try individual validation before package validation (glozow) 17a8ffd [packages/policy] use package feerate in package validation (glozow) 09f32cf [docs] package feerate (glozow) Pull request description: Part of #22290, aka [Package Mempool Accept](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a). This enables CPFP fee bumping in child-with-unconfirmed-parents packages by introducing [package feerate](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a#fee-related-checks-use-package-feerate) (total modified fees divided by total virtual size) and using it in place of individual feerate. We also always [validate individual transactions first](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a#always-try-individual-submission-first) to avoid incentive-incompatible policies like "parents pay for children" or "siblings pay for siblings" behavior. ACKs for top commit: instagibbs: reACK bitcoin/bitcoin@9bebf35 mzumsande: Code review ACK 9bebf35 t-bast: ACK bitcoin/bitcoin@9bebf35 Tree-SHA512: 5117cfcc3ce55c00384d9e8003a0589ceac1e6f738b1c299007d9cd9cdd2d7c530d31cfd23658b041a6604d39073bcc6e81f0639a300082a92097682a6ea8c8f
2 parents 41720a1 + 9bebf35 commit d844b5e

File tree

5 files changed

+362
-21
lines changed

5 files changed

+362
-21
lines changed

doc/policy/packages.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,48 @@ test accepts):
7272
a competing package or transaction with a mutated witness, even though the two
7373
same-txid-different-witness transactions are conflicting and cannot replace each other, the
7474
honest package should still be considered for acceptance.
75+
76+
### Package Fees and Feerate
77+
78+
*Package Feerate* is the total modified fees (base fees + any fee delta from
79+
`prioritisetransaction`) divided by the total virtual size of all transactions in the package.
80+
If any transactions in the package are already in the mempool, they are not submitted again
81+
("deduplicated") and are thus excluded from this calculation.
82+
83+
To meet the two feerate requirements of a mempool, i.e., the pre-configured minimum relay feerate
84+
(`minRelayTxFee`) and the dynamic mempool minimum feerate, the total package feerate is used instead
85+
of the individual feerate. The individual transactions are allowed to be below the feerate
86+
requirements if the package meets the feerate requirements. For example, the parent(s) in the
87+
package can pay no fees but be paid for by the child.
88+
89+
*Rationale*: This can be thought of as "CPFP within a package," solving the issue of a parent not
90+
meeting minimum fees on its own. This would allow contracting applications to adjust their fees at
91+
broadcast time instead of overshooting or risking becoming stuck or pinned.
92+
93+
*Rationale*: It would be incorrect to use the fees of transactions that are already in the mempool, as
94+
we do not want a transaction's fees to be double-counted.
95+
96+
Implementation Note: Transactions within a package are always validated individually first, and
97+
package validation is used for the transactions that failed. Since package feerate is only
98+
calculated using transactions that are not in the mempool, this implementation detail affects the
99+
outcome of package validation.
100+
101+
*Rationale*: Packages are intended for incentive-compatible fee-bumping: transaction B is a
102+
"legitimate" fee-bump for transaction A only if B is a descendant of A and has a *higher* feerate
103+
than A. We want to prevent "parents pay for children" behavior; fees of parents should not help
104+
their children, since the parents can be mined without the child. More generally, if transaction A
105+
is not needed in order for transaction B to be mined, A's fees cannot help B. In a
106+
child-with-parents package, simply excluding any parent transactions that meet feerate requirements
107+
individually is sufficient to ensure this.
108+
109+
*Rationale*: We must not allow a low-feerate child to prevent its parent from being accepted; fees
110+
of children should not negatively impact their parents, since they are not necessary for the parents
111+
to be mined. More generally, if transaction B is not needed in order for transaction A to be mined,
112+
B's fees cannot harm A. In a child-with-parents package, simply validating parents individually
113+
first is sufficient to ensure this.
114+
115+
*Rationale*: As a principle, we want to avoid accidentally restricting policy in order to be
116+
backward-compatible for users and applications that rely on p2p transaction relay. Concretely,
117+
package validation should not prevent the acceptance of a transaction that would otherwise be
118+
policy-valid on its own. By always accepting a transaction that passes individual validation before
119+
trying package validation, we prevent any unintentional restriction of policy.

src/test/fuzz/tx_pool.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,14 +234,18 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
234234
const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
235235
::fRequireStandard = fuzzed_data_provider.ConsumeBool();
236236

237-
// Make sure ProcessNewPackage on one transaction works and always fully validates the transaction.
237+
// Make sure ProcessNewPackage on one transaction works.
238238
// The result is not guaranteed to be the same as what is returned by ATMP.
239239
const auto result_package = WITH_LOCK(::cs_main,
240240
return ProcessNewPackage(chainstate, tx_pool, {tx}, true));
241-
auto it = result_package.m_tx_results.find(tx->GetWitnessHash());
242-
Assert(it != result_package.m_tx_results.end());
243-
Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
244-
it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
241+
// If something went wrong due to a package-specific policy, it might not return a
242+
// validation result for the transaction.
243+
if (result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) {
244+
auto it = result_package.m_tx_results.find(tx->GetWitnessHash());
245+
Assert(it != result_package.m_tx_results.end());
246+
Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
247+
it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
248+
}
245249

246250
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
247251
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;

0 commit comments

Comments
 (0)