Skip to content

feat: Add Wallet::create_psbt#516

Merged
ValuedMammal merged 4 commits into
bitcoindevkit:masterfrom
ValuedMammal:feat/bdk-tx
Jul 22, 2026
Merged

feat: Add Wallet::create_psbt#516
ValuedMammal merged 4 commits into
bitcoindevkit:masterfrom
ValuedMammal:feat/bdk-tx

Conversation

@ValuedMammal

Copy link
Copy Markdown
Collaborator

Description

This PR introduces Wallet::create_psbt and Wallet::replace_by_fee - a new PSBT construction API built on top of bdk-tx. This is a second attempt at integrating the planning module that started with #297. The scope is mostly the same and incorporates a number of improvements noted on #297. Notably the entire surface area is gated behind two compile flags.

flags:

  • the bdk-tx Cargo feature
  • the --cfg bdk_wallet_unstable rustc flag

Enabling bdk-tx without also passing bdk_wallet_unstable is a compile error with a clear diagnostic. This allows us to ship the API for early adopters while reserving the right to break it in a minor release.

Users can opt in by adding the following to your project's .cargo/config.toml:

[build]
rustflags = ["--cfg", "bdk_wallet_unstable"]

and enable the feature in Cargo.toml:

bdk_wallet = { version = "...", features = ["bdk-tx"] }

New public API

Item Notes
Wallet::create_psbt Build a new PSBT with coin selection
Wallet::create_psbt_with_rng Same with caller-supplied entropy
Wallet::replace_by_fee Construct an RBF replacement for one or more transactions
Wallet::replace_by_fee_with_rng Same with caller-supplied entropy
PsbtParams<C> Builder for PSBT construction; C is CreateTx or ReplaceTx/Rbf
SelectionStrategy Enum: All, SingleRandomDraw, LowestFee, Custom
CreatePsbtError Error type for create_psbt
ReplaceByFeeError Error type for replace_by_fee
bdk_tx re-export For callers who need direct access to bdk_tx

TxOrdering is also generalized to TxOrdering<In, Out> to accommodate
bdk-tx's typed input/output types. TxOrdering<In, Out> adds default type parameters
(TxIn, TxOut) that are identical to the previous concrete type, so remains
semver-compatible.
Existing callers that don't use the Custom variant are unaffected.

Differences from #297

  • Non-essential API surface removed - Some wallet methods present in Implement create_psbt for Wallet #297 were trimmed; the public API is narrower and more focused.
  • bdk_tx re-export - bdk_wallet::bdk_tx is re-exported so callers who need bdk-tx types directly don't need a separate bdk_tx dependency.
  • SelectionStrategy::All replaces drain_wallet - modeling "sweep everything" as a strategy is cleaner than a PsbtParams option.
  • SelectionStrategy::Custom - users can supply their own coin-selection algorithm as a closure without having to implement a trait.
  • SingleRandomDraw correctness fix - shuffling now only runs when SingleRandomDraw is active; the extra shuffle_slice calls are removed.
  • bdk_wallet_unstable cfg gate + bdk-tx Cargo feature - the full create_psbt / replace_by_fee surface is hidden unless explicitly opted into.

Changelog notice

### Added
- feat(wallet): add unstable `Wallet::create_psbt` and `Wallet::replace_by_fee`
  behind the `bdk-tx` cargo feature and the `--cfg bdk_wallet_unstable` rustc
  flag. These APIs are explicitly **unstable**: breaking changes may land in
  minor releases without a semver bump. To opt in, enable the `bdk-tx` feature
  **and** pass `--cfg bdk_wallet_unstable` to rustc.

Before submitting

@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.18519% with 136 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.74%. Comparing base (75e0d30) to head (319df5e).

Files with missing lines Patch % Lines
src/psbt/params.rs 86.17% 50 Missing and 6 partials ⚠️
src/wallet/mod.rs 90.02% 31 Missing and 17 partials ⚠️
src/wallet/error.rs 0.00% 31 Missing ⚠️
src/lib.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #516      +/-   ##
==========================================
+ Coverage   81.15%   81.74%   +0.58%     
==========================================
  Files          24       25       +1     
  Lines        5552     6469     +917     
  Branches      247      295      +48     
==========================================
+ Hits         4506     5288     +782     
- Misses        970     1082     +112     
- Partials       76       99      +23     
Flag Coverage Δ
rust 81.74% <85.18%> (+0.58%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oleonardolima oleonardolima moved this to Needs Review in BDK Wallet Jul 18, 2026
@oleonardolima oleonardolima added this to the Wallet 3.2.0 milestone Jul 18, 2026
@oleonardolima oleonardolima added the new feature New feature or request label Jul 18, 2026

@oleonardolima oleonardolima left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I left a few comments, and questions. I'd like to do some manual testing too, not only with the examples in regtest.

The commit history could use some work, by rewriting and restructuring it, it'll help other reviewers.

Comment thread Cargo.toml Outdated
Comment thread src/psbt/params.rs
pub type Rbf = ReplaceTx;

/// Parameters to create a PSBT.
// TODO: Can we derive `Clone` for this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: are you looking forward to address this in a follow-up ?

Comment thread src/psbt/params.rs
Comment on lines +18 to +24
/// Marker type representing the PSBT creation state.
#[derive(Debug)]
pub struct CreateTx;

/// Marker type representing the Replace-By-Fee (RBF) state.
#[derive(Debug)]
pub struct ReplaceTx;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: food for thought, do you think having these structs is better than having an enum (as it's representing a state), for example ?

Comment thread src/psbt/params.rs Outdated
Comment thread src/psbt/params.rs
Comment thread src/wallet/mod.rs
I: IntoIterator<Item = FullTxOut<ConfirmationBlockTime>> + 'a,
F: Fn(&FullTxOut<ConfirmationBlockTime>) -> bool + 'a,
{
let current_height = params.maturity_height.unwrap_or(self.chain.tip().height());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: it'd be clear if you stick to maturity_height, or just height.

Comment thread src/wallet/mod.rs
Comment on lines +3090 to +3112
txos.into_iter().filter(move |txo| {
// Exclude outputs that are manually selected.
if params.set.contains(&txo.outpoint) {
return false;
}
// Filter outputs according to `policy` fn.
if !policy(txo) {
return false;
}
// Exclude locked UTXOs.
if self.is_outpoint_locked(txo.outpoint) {
return false;
}
// Exclude immature outputs.
if !txo.is_mature(current_height) {
return false;
}
// Exclude spent outputs.
if txo.spent_by.is_some() {
return false;
}
true
})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: IIRC you could do it lazily by chaining the .filter's instead.

Comment thread src/psbt/params.rs
pub struct PsbtParams<C> {
/// Set of selected UTXO outpoints.
/// Set of selected UTXO outpoints, `HashSet` ensures uniqueness
pub(crate) set: HashSet<OutPoint>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: woulndn't be using something as select be clearer ?

Comment thread src/wallet/mod.rs Outdated
Comment thread justfile Outdated

@evanlinjin evanlinjin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Outstanding review comments from #297.

Comment thread src/wallet/error.rs
Comment on lines +438 to +439
/// One of the transactions to be replaced is already confirmed
TransactionConfirmed(Txid),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this error variant earns it's keep.

  • Most callers would want to check whether the transaction is still unconfirmed before trying to replace it. A caller that doesn't do that obviously wants to create a transaction that conflicts with a confirmed one.

  • The guard is partial. It only catches "already confirmed at build time". The original transaction can be confirmed before the replacement is ready to broadcast.

Comment thread src/wallet/mod.rs
Comment on lines +3456 to +3458
// For each txid being replaced, verify that at least one of its original inputs
// remains in the selected set. A replacement must conflict with every transaction it
// replaces — two transactions cannot spend the same UTXO.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// For each txid being replaced, verify that at least one of its original inputs
// remains in the selected set. A replacement must conflict with every transaction it
// replaces — two transactions cannot spend the same UTXO.
// A replacement must conflict (double-spend) with each tx it replaces - so require at least
// one of each original's inputs to remain selected.

wdyt?

Comment thread src/wallet/mod.rs
.iter()
.filter_map(|&txid| {
let tx = self.tx_graph.graph().get_tx(txid)?;
self.calculate_fee(&tx).ok()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you think we should return an error if an original's descendant cannot have it's fee calculated? The current implementation has a risk of undershooting the RBF fee floor.

Comment thread src/wallet/mod.rs
let descendant_fee: Amount = descendants
.iter()
.filter_map(|&txid| {
let tx = self.tx_graph.graph().get_tx(txid)?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let tx = self.tx_graph.graph().get_tx(txid)?;
let tx = self.tx_graph.graph().get_tx(txid).expect("tx must exist as they were obtained via the TxGraph");

We obtained descendants directly from TxGraph. So they must exist.

Comment thread src/psbt/params.rs Outdated

@notmandatory notmandatory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I did an initial review (with LLM help) and new changes look good.

One other suggestion, but doesn't need to be done in this PR, is figure out how we want to document experimental and possibly unstable APIs in the rust docs.

The LLM suggested adding:

/// **Unstable**: requires the `bdk-tx` Cargo feature and the
/// `--cfg bdk_wallet_unstable` rustc flag; may change in any minor release.
///

Comment thread src/psbt/params.rs
Comment thread src/psbt/params.rs Outdated
Comment thread src/psbt/params.rs Outdated
Comment thread src/wallet/mod.rs
let err = bdk_coin_select::InsufficientFunds {
missing: target_amount.to_sat(),
};
return Err(CreatePsbtError::InsufficientFunds(err))?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: it looks strange to throw an error while returning an error. The below should be more idiomatic.

Suggested change
return Err(CreatePsbtError::InsufficientFunds(err))?;
return Err(CreatePsbtError::InsufficientFunds(err).into());

@sdmg15 sdmg15 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tAck 319df5e
Haven't spotted anything major. Would probably do a follow up and create examples on the book-of-bdk as suggested during the call which would include using planning, rbf and create_psbt.

Prepare the repository for upcoming unstable-gated API commits by
proving the cfg workflow end-to-end before any new surface is added.

This change wires bdk_wallet_unstable into CI coverage, testing,
linting, and docs checks, while keeping no-default-features and no_std
builds validated in a stable lane. It also splits local just tasks into
stable and unstable paths so pre-push validation correctly exercises
both.
Introduce the unstable create_psbt and replace_by_fee APIs behind
the bdk-tx feature and --cfg bdk_wallet_unstable.

This adds PsbtParams and SelectionStrategy, integrates bdk_tx into
wallet PSBT/RBF construction, re-exports bdk_tx when enabled, and
includes supporting error types needed by the new flow.
@ValuedMammal

Copy link
Copy Markdown
Collaborator Author

How is the nLockTime set?

First, bdk_tx takes the min_locktime (defaulting to LockTime::ZERO if not set) and the absolute timelock extracted from each input's spending plan, and returns the maximum of those values — as long as the units agree. After that, if anti-fee-sniping is used, the locktime may be raised further to the given chain tip, or instead sets a relative sequence on a randomly chosen Taproot input (BIP326 nSequence path). AFS hard-errors if the locktime at that point is time-based, so enabling both time-based CLTVs and AFS is a CreatePsbtError::Psbt(AntiFeeSnipingError::UnsupportedLockTime).

The four scenarios then fall out naturally:

Neither min_locktime nor anti_fee_sniping set: min_locktime = LockTime::ZERO. Final locktime is the maximum CLTV requirement across all inputs, or zero if none of the inputs carry a CLTV.

Only min_locktime set: The floor is raised from zero to the caller-specified value. Final locktime = max(min_locktime, max(input CLTVs)), unit-compatible. Useful for, e.g., enforcing a block-height floor independently of any script requirements.

Only anti_fee_sniping set: min_locktime = zero, so the pre-AFS locktime is just the input CLTVs (or zero). AFS then applies BIP326: with 50/50 probability it sets tx.lock_time to tip_height, otherwise it sets a relative sequence on a Taproot input. If the locktime after CLTV accumulation is already above tip_height (e.g., a CLTV that pins to a future height), AFS leaves it untouched. Note that AFS is not automatically derived from the wallet's chain tip; you must supply the current block height explicitly.

Both min_locktime and anti_fee_sniping set: min_locktime acts as the floor going into AFS. The final value is effectively max(min_locktime, input CLTVs, tip_height) — though the exact outcome depends on BIP326's nLockTime vs nSequence coin flip. If min_locktime is already above tip_height, AFS's locktime branch won't change it, so the min_locktime effectively wins.

Note that maturity_height has no bearing on nLockTime. It is used to decide whether coinbase UTXOs have reached the 100-confirmation threshold and are eligible for coin selection at all. It's a gate on which outputs may enter the candidate set, not a contributor to the transaction's locktime.

add_assets can be used to tell the planner "an OP_CLTV of up to this height will be satisfied." The planner uses that to choose a spending path. If it selects a branch that requires after(N), the resulting Plan carries absolute_timelock = Some(LockTime::Blocks(N)), and that is what Input::absolute_timelock() returns.


Some important nuances:

Assets::after() is a planner hint, not a direct locktime assertion. The height that ends up in the transaction's nLockTime is the descriptor's required CLTV value (from the plan), not the value you passed to Assets::after(). If you say Assets::after(500) but the descriptor has after(1000), the planner can't satisfy that branch anyway — it will either fail to plan or pick a different branch.

The fallback in plan_input is interesting. If you don't call add_assets at all, the wallet falls back to LockTime::from_consensus(current_height), i.e. the current chain tip height, as the inferred absolute timelock for planning. This means the wallet will automatically select CLTV branches that are currently satisfiable at the last known tip height, which is a sensible default. It also means that for a simple mandatory-CLTV policy like and_v(v:pk(A), after(1000)), the plan will carry absolute_timelock = Some(1000) regardless of what you pass to add_assets, as long as the tip is past height 1000.

So the full picture of locktime inputs is actually three sources, one of which is indirect:

  • min_locktime, a direct floor on tx.lock_time
  • Input CLTV requirements, which flow from the descriptor's spending plan. That plan may be shaped by add_assets, making CLTV branches reachable, and through that, indirectly controls whether and what absolute timelock an input contributes to the transaction.
  • anti_fee_sniping_height(tip_height), raises locktime toward the tip, or deflects to nSequence

@ValuedMammal

Copy link
Copy Markdown
Collaborator Author

Tested the PSBT and RBF happy path locally on a fork of satchl using a simple taproot descriptor (on signet). https://mempool.space/signet/tx/d45ebd30e2e2448f86ed8a32d3fcd859c3ab1bd5d1ccda6581246690d752509c

@ValuedMammal
ValuedMammal merged commit b01f2f6 into bitcoindevkit:master Jul 22, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in BDK Wallet Jul 22, 2026
@ValuedMammal
ValuedMammal deleted the feat/bdk-tx branch July 22, 2026 14:57
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

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants