feat!: Persist utxo lock status #259

ValuedMammal · 2025-06-07T19:23:41Z

Description

Added methods on Wallet to lock/unlock an outpoint, to query the locked outpoints, and updated the wallet ChangeSet to persist the lock status of an outpoint.

based on #277
may resolve #166.

Changelog notice

Added the following methods to Wallet

lock_outpoint
unlock_outpoint
locked_outpoints
list_locked_unspent
is_outpoint_locked

Added

New type wallet::locked_outpoints::ChangeSet
New member field wallet::ChangeSet::locked_outpoints

Checklists

All Submissions:

New Features:

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

Bugfixes:

This pull request breaks the existing API
I've added tests to reproduce the issue which are now passing
I'm linking the issue being fixed by this PR

coveralls · 2025-06-07T19:27:49Z

Pull Request Test Coverage Report for Build 16273558170

Details

112 of 119 (94.12%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.1%) to 84.83%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
wallet/src/wallet/mod.rs	56	63	88.89%

Totals
Change from base Build 16215822724:	0.1%
Covered Lines:	6688
Relevant Lines:	7884

💛 - Coveralls

ChidiChuks · 2025-06-08T14:20:31Z

How do you handle UTXO locking in concurrent wallet instances?

nymius

Could you expand a little bit more on how this mechanism could be used in broadcast scenarios?
From the test I guess all the logic of why a UTxO becomes available again is kept by the lib user? Like unlock UTxO after a height in the future if there is not transaction spending it in the blockchain.

This is a competing solution for #257, right?

evanlinjin · 2025-06-09T01:15:56Z

How do you handle UTXO locking in concurrent wallet instances?

@ChidiChuks could you be a bit more specific with this question? Do you mean sharing lock status between multiple instances of the same wallet?

evanlinjin · 2025-06-09T02:15:58Z

This is a competing solution for #257, right?

@nymius My understanding is that this is NOT a competing solution with #257. UTXO-locking will not fully solve #166 as we want unbroadcasted outputs to be available for selection. In fact, UTXO-locking is already available by using TxBuilder::add_unspendable. The only difference here is that locked outpoints are persisted with BDK. So this is more of a convenience feature that users are requesting.

evanlinjin

Thanks for this work.

I propose adding the following methods:

list_locked_outpoints - Lists all locked outpoints.
list_locked_unspents - Lists locked outpoints that are unspent.

Imagine a situation where the caller broadcasts a tx and locks the prevouts. Then the tx gets evicted from the mempool. It will be helpful to list locked unspents at this point to recover those prevouts.

wallet/src/wallet/mod.rs

ValuedMammal · 2025-06-16T17:45:25Z

@evanlinjin I took all of your suggestions other than I think the proposed Wallet::list_locked_unspents is not yet implemented. edit: I have some ideas for this though.

wallet/src/wallet/changeset.rs

evanlinjin · 2025-06-19T21:29:11Z

wallet/src/wallet/changeset.rs

+                txid TEXT NOT NULL, \
+                vout INTEGER NOT NULL, \
+                is_locked INTEGER, \
+                expiration_height INTEGER, \


What would be the usecases for this expiration_height feature?

If the only goal is to forget about locked outpoints if a transaction fails to be included in a block or is dropped from the mempool due to insufficient feerates, it may result in the caller creating a second transaction that is intended to replace the original payment which does not conflict the original. Now we have a situation where the original and replacement can belong in the same history resulting in double-payment. As suggested in #257, it is only safe to forget about a tx if there is a conflict which is x-confirmations deep (where x is defined by the caller, based on their assumptions).

Let me know if there is any other usecase that I am unaware of. Otherwise, I recommend removing this feature as it seems dangerous.

cc. @tnull

References:

Original feature suggestion: Let TxBuilder avoid previously used UTXOs (UTXO locking) #166 (comment)

Proposed no-footgun API: Introduce IntentTracker #257 (comment). Note that this does not automatically "untrack" transactions if a conflict has reached x-confirmations. If automation is important, it can be implemented in a separate PR. The API would be: "automatically forget if a conflicting tx reasons x-confirmations deep". For transactions that are evicted from the mempool due to insufficient fee, the safe thing to do would be to explicitly replace, or cancel with a replacement transaction.

Yes, I agree with everything you said. It's just that if you don't offer that API, the user will implement such a thing themselves, as at some point the user would need unlock previously-locked UTXOs for which the spend 'failed'. And usually that means that they'd do a worse job as they have less context, less access to wallet internals, and presumably less time spent thinking through the issues. So while we often tend to leave things we can't decide on to our users, it unfortunately also means that we set them up to do a worse job than us.

All that said, I agree that it might be tricky to get the API right to avoid any footguns, and maybe it could happen in a follow-up/separate PR, especially if it's undecided yet and would block this PR.

The use case I had in mind was to prevent a utxo from being selected for a specific duration of time measured in blocks. As mentioned, it frees the user from having to explicitly unlock it in the future.

Yes, I agree with everything you said. It's just that if you don't offer that API, the user will implement such a thing themselves, as at some point the user would need unlock previously-locked UTXOs for which the spend 'failed'.

The purpose of the IntentTracker API (as proposed in #257) is to address this issue.

The use case I had in mind was to prevent a utxo from being selected for a specific duration of time measured in blocks. As mentioned, it frees the user from having to explicitly unlock it in the future.

I think we need a more thorough explanation on how this would be useful just because we have to maintain what we include.

One potential example I can think of is in payjoin. The sender keeps around the Original PSBT until a given timeout or they receive the Proposal PSBT. During this time, they lock the outpoints that fund these PSBTs. However, once the PJ transaction is broadcasted, there will be no need to keep these outpoints locked - and we can unlock them instantly. In other words, no need for auto-unlocking of outpoints.

wallet/src/wallet/mod.rs

notmandatory

I'm happy to see this feature moving along. In addition to helping L2 users like @tnull it also helps support one of my personal goals of improving feature parity with the bitcoin core wallet.

clippy.toml

wallet/src/wallet/changeset.rs

notmandatory · 2025-06-25T21:47:07Z

wallet/src/wallet/mod.rs

+    }
+
+    /// List unspent outpoints that are currently locked.
+    pub fn list_locked_unspent(&self) -> impl Iterator<Item = OutPoint> + '_ {


⛏️ a nit, it's a little confusing to refer to the outpoints as "unspent" here and "outpoints" in the other new functions. Since we already have the "list_unspent" function would it make sense to use "unspent" for all the related functions (ie. locked_unspent, is_unspent_locked, lock_unspent, unlock_unspent)? Even though we all know what "outpoints" mean for those unfamiliar with bitcoin internals it's not a very descriptive term.

wallet/src/wallet/mod.rs

evanlinjin

Implementation looks good. Just need a bit more clarity on expiration_height before ACK.

evanlinjin · 2025-06-26T03:10:34Z

wallet/src/wallet/mod.rs

+            }
+        }
+    }
+


What do you think about an unlock_all_outpoints method? Just in case the caller needs to reset their state.

evanlinjin · 2025-06-26T03:40:39Z

wallet/src/wallet/changeset.rs

+                txid TEXT NOT NULL, \
+                vout INTEGER NOT NULL, \
+                is_locked INTEGER, \
+                expiration_height INTEGER, \


Yes, I agree with everything you said. It's just that if you don't offer that API, the user will implement such a thing themselves, as at some point the user would need unlock previously-locked UTXOs for which the spend 'failed'.

The purpose of the IntentTracker API (as proposed in #257) is to address this issue.

The use case I had in mind was to prevent a utxo from being selected for a specific duration of time measured in blocks. As mentioned, it frees the user from having to explicitly unlock it in the future.

I think we need a more thorough explanation on how this would be useful just because we have to maintain what we include.

One potential example I can think of is in payjoin. The sender keeps around the Original PSBT until a given timeout or they receive the Proposal PSBT. During this time, they lock the outpoints that fund these PSBTs. However, once the PJ transaction is broadcasted, there will be no need to keep these outpoints locked - and we can unlock them instantly. In other words, no need for auto-unlocking of outpoints.

wallet/src/wallet/mod.rs

wallet: - Add pub struct `UtxoLock` - Add `Wallet::lock_outpoint` - Add `Wallet::unlock_outpoint` - Add `Wallet::is_outpoint_locked` - Add `Wallet::locked_outpoints` - Add `Wallet::list_locked_unspent` changeset: - Add member `locked_outpoints` to ChangeSet `tests/persisted_wallet.rs`: - Add test `test_lock_outpoint_persist`

github-project-automation bot added this to BDK Wallet Jun 7, 2025

ValuedMammal moved this to Todo in BDK Wallet Jun 7, 2025

ValuedMammal added this to the Wallet 3.0.0 milestone Jun 7, 2025

ValuedMammal self-assigned this Jun 7, 2025

nymius reviewed Jun 8, 2025

View reviewed changes

evanlinjin requested changes Jun 9, 2025

View reviewed changes

wallet/src/wallet/mod.rs Outdated Show resolved Hide resolved

wallet/src/wallet/mod.rs Outdated Show resolved Hide resolved

wallet/src/wallet/mod.rs Outdated Show resolved Hide resolved

notmandatory mentioned this pull request Jun 12, 2025

Introduce IntentTracker #257

Draft

14 tasks

ValuedMammal commented Jun 16, 2025

View reviewed changes

wallet/src/wallet/mod.rs Outdated Show resolved Hide resolved

ValuedMammal force-pushed the feat/lock-unspent branch from b083dff to a6b0f5d Compare June 16, 2025 17:23

ValuedMammal marked this pull request as ready for review June 16, 2025 17:29

ValuedMammal moved this from Todo to Needs Review in BDK Wallet Jun 16, 2025

ValuedMammal force-pushed the feat/lock-unspent branch from a6b0f5d to 13e024d Compare June 18, 2025 13:19

ValuedMammal moved this from Needs Review to In Progress in BDK Wallet Jun 18, 2025

evanlinjin reviewed Jun 19, 2025

View reviewed changes

ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Jun 23, 2025

notmandatory reviewed Jun 25, 2025

View reviewed changes

notmandatory added new feature New feature or request api A breaking API change labels Jun 25, 2025

evanlinjin reviewed Jun 26, 2025

View reviewed changes

ValuedMammal moved this from Needs Review to In Progress in BDK Wallet Jun 27, 2025

ValuedMammal force-pushed the feat/lock-unspent branch from 6e11db6 to 37fac7d Compare July 9, 2025 21:13

ValuedMammal force-pushed the feat/lock-unspent branch from 37fac7d to ee0a88b Compare July 14, 2025 17:26

feat!: Persist utxo lock status #259

Are you sure you want to change the base?

feat!: Persist utxo lock status #259

Uh oh!

Conversation

ValuedMammal commented Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog notice

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

coveralls commented Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 16273558170

Details

💛 - Coveralls

Uh oh!

ChidiChuks commented Jun 8, 2025

Uh oh!

nymius left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evanlinjin commented Jun 9, 2025

Uh oh!

evanlinjin commented Jun 9, 2025

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ValuedMammal commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

evanlinjin Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

tnull Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

evanlinjin Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

notmandatory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

notmandatory Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

evanlinjin Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

evanlinjin Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ValuedMammal commented Jun 7, 2025 •

edited

Loading

coveralls commented Jun 7, 2025 •

edited

Loading

nymius left a comment •

edited

Loading

ValuedMammal commented Jun 16, 2025 •

edited

Loading

tnull Jun 20, 2025 •

edited

Loading

notmandatory Jun 25, 2025 •

edited

Loading