Skip to content

feat: Add exclude_unconfirmed and exclude_below_confirmations#258

Merged
ValuedMammal merged 1 commit intobitcoindevkit:masterfrom
evanlinjin:feature/exclude_below_confirmations
Jul 3, 2025
Merged

feat: Add exclude_unconfirmed and exclude_below_confirmations#258
ValuedMammal merged 1 commit intobitcoindevkit:masterfrom
evanlinjin:feature/exclude_below_confirmations

Conversation

@evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented Jun 6, 2025

Fixes #143

Description

These are convenience methods on TxBuilder that can be used to filter out outpoints which do not meet a given confirmation threshold.

Previously, I advocated a feature freeze on TxBuilder, but I believe we can make an exception for these helper methods, as they do not broaden the TxBuilder interface’s error surface. Going forward, all other development should be directed to the bdk-tx repository and crate.

cc. @tnull

Changelog notice

Added:
- `TxBuilder::exclude_unconfirmed`
- `TxBuilder::exclude_below_confirmations`

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo +nightly fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@coveralls
Copy link

coveralls commented Jun 6, 2025

Pull Request Test Coverage Report for Build 15892909684

Details

  • 92 of 95 (96.84%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 85.673%

Changes Missing Coverage Covered Lines Changed/Added Lines %
wallet/src/wallet/tx_builder.rs 92 95 96.84%
Totals Coverage Status
Change from base Build 15719064446: 0.1%
Covered Lines: 7469
Relevant Lines: 8718

💛 - Coveralls

@evanlinjin evanlinjin force-pushed the feature/exclude_below_confirmations branch from 9bba85c to 059f6af Compare June 6, 2025 13:11
@evanlinjin evanlinjin self-assigned this Jun 6, 2025
@evanlinjin evanlinjin added this to the Wallet 2.1.0 milestone Jun 6, 2025
@evanlinjin evanlinjin moved this to Needs Review in BDK Wallet Jun 6, 2025
@evanlinjin evanlinjin force-pushed the feature/exclude_below_confirmations branch from 059f6af to 6400343 Compare June 6, 2025 13:14
@evanlinjin evanlinjin marked this pull request as ready for review June 6, 2025 13:15
@evanlinjin evanlinjin requested a review from ValuedMammal June 6, 2025 13:16
@ValuedMammal
Copy link
Collaborator

Looks good. I was a bit confused by the comment that refers to the exclude_unbroadcasted() method which is not actually defined anywhere.

/// Excludes any outpoints whose enclosing transaction has fewer than `min_confirms`
/// confirmations.
///
/// `min_confirms` is the minimum number of confirmations a transaction must have in order for
Copy link
Contributor

@nymius nymius Jun 18, 2025

Choose a reason for hiding this comment

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

suggestion(doc):

min_confirms is the minimum number of confirmations a transaction must have in order for its outpoints to be available for spend in the next block/available for selection in the current one.

Copy link
Member Author

Choose a reason for hiding this comment

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

@nymius could you highlight for me what detail you think is missing in the original docs?

Copy link
Contributor

@nymius nymius Jun 26, 2025

Choose a reason for hiding this comment

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

As you were using the same logic that the coinbase maturity check

.map_or(0, |h| tip_height.saturating_add(1).saturating_sub(h))

And the definition wasn't clear previously, I thought it was good to emphasize what spendable means: the outpoint is available for selection in the current block.

If you are expecting to make a transaction with a UTXO mined 4 blocks before, and you use exclude_below_confirmations with min_confirms = 4 you should know that same UTXO will be available for selection.

But don't consider it a blocker, if you think is not worth it.

@notmandatory notmandatory added the new feature New feature or request label Jun 25, 2025
These are convenience methods on `TxBuilder` that can be used to filter
out outpoints which do not meet a given confirmation threshold.
@evanlinjin evanlinjin force-pushed the feature/exclude_below_confirmations branch from 6400343 to 5a65422 Compare June 26, 2025 04:22
@evanlinjin evanlinjin requested a review from nymius June 26, 2025 04:36
Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

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

utACK 5a65422

I agree with nymius's comments, but I also don't see it as a blocker.

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 5a65422

@ValuedMammal
Copy link
Collaborator

ACK 5a65422

@ValuedMammal ValuedMammal merged commit 890eb05 into bitcoindevkit:master Jul 3, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Expose way to select only (n-)confirmed UTXOs for TX creation

6 participants