Skip to content

Add ancestor_bump_fee to Candidate#42

Closed
aagbotemi wants to merge 2 commits intobitcoindevkit:masterfrom
aagbotemi:feat/ancestor-aware-candidate
Closed

Add ancestor_bump_fee to Candidate#42
aagbotemi wants to merge 2 commits intobitcoindevkit:masterfrom
aagbotemi:feat/ancestor-aware-candidate

Conversation

@aagbotemi
Copy link
Copy Markdown
Contributor

Description

Add ancestor_bump_fee field to Candidate, allowing callers to
express the additional fee a child transaction must pay to bring
its unconfirmed ancestors up to the target feerate.

Note to Reviewers

Cross-candidate ancestor deduplication is not handled at this layer.
Shared ancestors across candidates are counted per-candidate, which
may overcount when multiple selected candidates share an ancestor.
This is conservative (overpays, never underpays). The wallet layer
deduplicates post-selection.

Closes #24

Changelog notice

  • Added ancestor_bump_fee field to Candidate for ancestor-aware coin selection

Checklists

All Submissions:

  • I followed the contribution guidelines
  • I've added tests for the new feature
  • I've added docs for the new feature
  • I'm linking the issue being fixed by this PR

Copy link
Copy Markdown
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NACK

Thanks for working on this. However, this solution is flawed.

  • ancestor_bump_fee is a function of the target feerate. This is incorrect as Candidate should only have fixed properties of an UTXO.
  • This solution does not take ancestor overlaps into account. Please refer to #24 (comment)

Also, next time, please remember to reference pre-existing PRs addressing similar problems. This PR is essentially a competing solution to #38.

@aagbotemi
Copy link
Copy Markdown
Contributor Author

Thanks for the review.

Both points are valid.

Also, next time, please remember to reference pre-existing PRs addressing similar problems. This PR is essentially a competing solution to #38.

I looked at #38 before opening this PR and concluded they addressed different layers. #38 operates at the transaction level with a single Package aggregate, this PR at the candidate level. I should have commented on #38 explaining this reasoning rather than opening a separate PR.

@aagbotemi
Copy link
Copy Markdown
Contributor Author

Closing in favor of #43

@aagbotemi aagbotemi closed this Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CPFP: Allow telling coin selector that a candidate has unconfirmed parents

2 participants