Add ancestor-aware CPFP package pricing - #43
Conversation
d168359 to
ce09897
Compare
noahjoeris
left a comment
There was a problem hiding this comment.
What do you think of inverting the ancestor ↔ candidate edge?
Instead of Candidate.ancestors: Vec<usize>, store the link on the ancestor:
pub struct UnconfirmedAncestor {
pub weight: u64,
pub fee_paid: u64,
pub dependent_candidates: Vec<usize>, // candidate indices depending on this ancestor
}We could keep Candidate: Copy. I think it wouldn't change much for the caller.
Also avoids an unused ancestors field on Candidates if caller doesn't care about CPFP.
@noahjoeris Interesting idea. My main worry with this is in the ergonomics of constructing it. Worth an exploration. |
|
Ok I'll explore |
|
Here is the version with ancestors containing the |
ce09897 to
e381135
Compare
When spending unconfirmed UTXOs, miners evaluate the transaction as a package with its ancestors. This adds support for computing the package-level bump fee needed to bring ancestors up to the target feerate, with automatic deduplication of shared ancestors across candidates. - Add UnconfirmedAncestor struct (weight, fee_paid, dependent_candidates) - Add with_ancestors() builder on CoinSelector - Add selected_ancestor_bump_fee() with package-level computation - Add candidates_with_ancestors() to report the affected candidates - Fold the bump into the implied_package_fee_* helpers, so every excess calculation and implied_fee() price the package rather than the tx alone The ancestor -> candidate edge is stored on the ancestor rather than on the candidate, so `Candidate` stays `Copy` and each ancestor is counted at most once per selection without an intermediate dedup pass. Candidates with unconfirmed ancestors are banned from automatic selection. The bump is a property of the package, not of any one candidate: shared ancestors count once and an overpaying ancestor subsidizes an underpaying one, so the cost of adding a candidate depends on what else is selected. Every algorithm here ranks with per-candidate figures that cannot see ancestors, so a selectable ancestor candidate could *lower* the excess and break them silently -- select_until_target_met and select_srd reporting insufficient funds when a funding selection exists, is_fundable giving false negatives, LowestFee::bound no longer bounding, Changeless pruning branches that do hold changeless solutions. Banning keeps the reachable ancestor set, and hence the bump, identical across every selection an algorithm can reach. Bans do not block manual selection, so the CPFP flow is unaffected: select the unconfirmed UTXO you are bumping, then let the algorithms optimize the confirmed candidates around it. The invariant is noted at the sites that depend on it, and a debug_assert in selected_ancestor_bump_fee fires if a dependent candidate ever stops being banned. Co-Authored-By: Noah Joeris <noahjoeris@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e381135 to
6219c8f
Compare
|
After thinking further on this problem, turns out we overlooked #24 (comment). The only correct solution is something |
Summary
When spending unconfirmed UTXOs, miners evaluate the transaction as a package with its unconfirmed ancestors. If ancestors paid below the target feerate, the child must overpay (CPFP). This PR makes coin selection price that bump correctly, with automatic deduplication when multiple candidates share ancestors.
Closes #24. Competes with #38 and #42.
Scope: pricing, not selection
Worth stating up front, because it shapes everything below. This PR lets you say "I am spending these unconfirmed UTXOs" and have the resulting package priced correctly in every excess, effective-value and fee calculation. It does not let the selection algorithms decide for themselves whether spending an unconfirmed UTXO is worthwhile — candidates with unconfirmed ancestors are banned from automatic selection.
That matches the actual CPFP flow, where the input you're bumping is mandatory rather than optional: select it, then let branch and bound or
select_until_target_metoptimize the confirmed candidates around it, with the bump priced into every calculation along the way.The reason it isn't optional is in the next section, and it's the main thing I'd like review on.
Design
Each ancestor is tracked individually via
UnconfirmedAncestor { weight, fee_paid, dependent_candidates }, handed to the selector once throughCoinSelector::with_ancestors().Note the direction of the edge: the ancestor names the candidates that depend on it, rather than each candidate naming its ancestors.
Candidateis therefore untouched — it stays a plainCopystruct with no lifetime parameter, and existing code needs no changes. Deduplication also falls out for free: each ancestor is visited once per selection, with no intermediate collect/sort/dedup pass.The bump is computed at the package level: include every ancestor with at least one selected dependent, sum weights and fees, then
max(0, implied_fee(total_weight, feerate) - total_fees). High-feerate ancestors subsidize low-feerate ones within the package, matching Bitcoin Core's package relay approach from PR #27021.Why ancestor candidates are banned from automatic selection
The bump is a property of the package, not of any one candidate — shared ancestors count once, and an overpaying ancestor subsidizes an underpaying one. So the cost of adding a candidate depends on what else is selected.
Every selection algorithm in this crate ranks and accumulates using per-candidate figures (
Candidate::effective_value,Candidate::value_pwu) which structurally cannot see ancestors. If such a candidate were selectable, adding one could lower the excess, and every algorithm breaks in a way that produces no error:select_until_target_metandselect_srdcan report insufficient funds when a funding selection existsis_fundablereturns false negatives, so BnB misreportsNoBnbSolution::InsufficientFundsLowestFee::boundstops being a lower bound and silently prunes the optimal branchChangeless::change_unavoidableprunes branches that do contain changeless solutionsBanning keeps the reachable ancestor set — and hence the bump — identical across every selection an algorithm can produce, which is exactly what those algorithms need. Bans don't block manual selection, so the CPFP flow is unaffected.
Since that invariant is enforced in one place and depended on in several, the dependent sites carry a note pointing at
with_ancestors, and adebug_assertinselected_ancestor_bump_feefires if a dependent candidate ever stops being banned.Where the bump lives
Rather than modifying
weight()orfee()(which would need_without_packageescape hatches for RBF), the bump is folded into the privateimplied_package_fee_*helpers. Every caller of those needed it, so putting it inside means a new call site can't forget it.TargetFee::absolutestays bump-free, since it's a minimum fee floor rather than an additive charge.This makes
implied_fee()the exact counterpart ofexcess():Comparison with the alternatives
#42 (
ancestor_bump_feefield onCandidate) pre-computes the bump per candidate. Two consequences:But the honest counterpoint, which I got to while working through the above: #42's model is local, and that locality is precisely what makes it safe to select on. Per-candidate effective value tells the whole story there, so their algorithms can freely choose unconfirmed UTXOs. This PR's model is more accurate and, for exactly that reason, only usable for pricing. The overpayment #42 accepts is what buys them the ability to choose.
#38 (
Packagestruct) uses a single aggregatePackage { parent_fee, parent_weight }. That works for one parent but can't deduplicate shared ancestors as the selection changes. The per-ancestor model handles that naturally:Selecting either candidate counts A once; selecting both still counts it once.
API changes
Purely additive — no existing type or signature changes.
UnconfirmedAncestor— new:{ weight: u64, fee_paid: u64, dependent_candidates: Vec<usize> }CoinSelector::with_ancestors(&'a [UnconfirmedAncestor]) -> Self— records the ancestors and bans the dependent candidates. A selector without it behaves exactly as before. Panics if adependent_candidatesindex is out of bounds.CoinSelector::selected_ancestor_bump_fee(feerate) -> u64— package-level bump with deduplicationCoinSelector::candidates_with_ancestors() -> impl Iterator<Item = usize>— which candidates were banned, deduplicated, and distinct frombanned()(which also carries your own bans)rate_excess,rate_excess_wu,replacement_excess,replacement_excess_wu,effective_value,implied_fee— now price the packageTesting
tests/ancestor_aware.rscovers: no-ancestor backward compatibility, a single ancestor reducing excess, shared ancestors deduplicated, a high-feerate ancestor subsidizing a low-feerate one, an already-sufficient package contributing zero bump, bump scaling with feerate,effective_valueaccounting,implied_feecarrying the bump, theexcess/implied_feeidentity across ancestor sets × absolute fees × replacements × feerates × drains × selections, the ban taking effect throughunselected_indicesandselect_until_target_met, manual selection still working, andcandidates_with_ancestorsdeduplication.The suite passes in both debug and release — debug matters here, as that's the only build where the
debug_assertruns.Known follow-ups
rate_excess_wu/replacement_excess_wuexist to avoid vbyte rounding, but the bump they subtract still usesFeeRate::implied_fee(which rounds to whole vbytes) rather thanimplied_fee_wu. Small in sats, but it makes those bounds slightly over-tight. One line each to fix now that the bump is folded into the helpers.selected_ancestor_bump_feeisO(ancestors × dependents)per call. Storing each ancestor's dependents as aBitsetwould make it a bitwise AND. Pure optimization, no semantic change.implied_feerate()reports the child's standalone feerate rather than the package feerate.Co-authored with @noahjoeris, who contributed the inverted ancestor → candidate edge this design is built on.