feat: Add lock_utxos option to prevent double-spending unbroadcasted txs - #534
dorinengabdoh wants to merge 1 commit into
Conversation
|
Hi everyone, I've worked on a fix for this issue. Since BDK 3.0 already supports persistent UTXO locking via lock_outpoint, I have opened a Pull Request that introduces a lock_utxos(bool) option on both TxBuilder and PsbtParams (disabled by default to maintain backward compatibility). When enabled, it automatically locks the selected inputs of the generated transaction so that subsequent transaction creations do not double-spend them. I've also added integration tests to verify this behavior. I would love to get your feedback on the PR! |
There was a problem hiding this comment.
This could serve as a convenient wrapper around lock_outpoint, applying the lock automatically when creating the transaction rather than leaving it to the caller. I also noticed there was an earlier design discussion around this in #257.
Since locks don't expire automatically, what is the expected way to handle the case where the created transaction is abandoned or never broadcast? Is the caller expected to explicitly call unlock_outpoint in that case? If so, it might be worth documenting.
aagbotemi
left a comment
There was a problem hiding this comment.
Thanks for working on this. I left some comments.
This PR doesn't build under RUSTFLAGS="--cfg bdk_wallet_unstable" cargo build --all-features: into_replace_params constructs PsbtParams<ReplaceTx> field by field, so the new field (lock_utxos) has to be listed there too.
| if lock_utxos { | ||
| for input in &psbt.unsigned_tx.input { | ||
| self.lock_outpoint(input.previous_output); | ||
| } | ||
| } |
There was a problem hiding this comment.
This locks every input, including foreign UTXOs. lock_outpoint is documented as locking a wallet output, so worth filtering to outputs the wallet actually tracks first.
| if params.lock_utxos { | ||
| for input in &psbt.unsigned_tx.input { | ||
| self.lock_outpoint(input.previous_output); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same here for filtering wallet owned UTXOs.
Description
Title: feat: Add lock_utxos option to prevent double-spending created transactions
Description: This PR addresses #40 by introducing an option to automatically lock the selected UTXOs (inputs) of a transaction when it is created.
Added lock_utxos option (disabled by default) to TxParams and PsbtParams.
Implemented lock_utxos(bool) builder methods on TxBuilder and PsbtParams.
Updated Wallet::create_tx and Wallet::create_psbt_with_rng to call lock_outpoint on the transaction inputs if lock_utxos is enabled.
Verification: Added the following integration tests to verify that locked UTXOs are excluded from subsequent transaction creations:
test_tx_builder_lock_utxos in tests/wallet.rs
test_create_psbt_lock_utxos in tests/create_psbt.rs
Run tests with: RUSTFLAGS="--cfg bdk_wallet_unstable" cargo test