locked_outpoints: Remove optional expiration height

- changeset: Changed SQLite to delete rows when an outpoint is
unlocked. Any outpoints that are present in the locked outpoints
table are logically assumed to be locked.

Author: ValuedMammal

wallet/src/wallet/changeset.rs

@@ -194,8 +194,6 @@ impl ChangeSet {
             "CREATE TABLE {} ( \
                 txid TEXT NOT NULL, \
                 vout INTEGER NOT NULL, \
-                is_locked INTEGER, \
-                expiration_height INTEGER, \
                 PRIMARY KEY(txid, vout) \
                 ) STRICT;",
             Self::WALLET_OUTPOINT_LOCK_TABLE_NAME,
@@ -247,27 +245,20 @@ impl ChangeSet {
 
         // Select locked outpoints.
         let mut stmt = db_tx.prepare(&format!(
-            "SELECT txid, vout, is_locked, expiration_height FROM {}",
+            "SELECT txid, vout FROM {}",
             Self::WALLET_OUTPOINT_LOCK_TABLE_NAME,
         ))?;
         let rows = stmt.query_map([], |row| {
             Ok((
                 row.get::<_, Impl<Txid>>("txid")?,
                 row.get::<_, u32>("vout")?,
-                row.get::<_, bool>("is_locked")?,
-                row.get::<_, Option<u32>>("expiration_height")?,
             ))
         })?;
-        let locked_outpoints = &mut changeset.locked_outpoints;
+        let locked_outpoints = &mut changeset.locked_outpoints.locked_outpoints;
         for row in rows {
-            let (Impl(txid), vout, is_locked, expiration_height) = row?;
+            let (Impl(txid), vout) = row?;
             let outpoint = OutPoint::new(txid, vout);
-            locked_outpoints
-                .locked_outpoints
-                .insert(outpoint, is_locked);
-            locked_outpoints
-                .expiration_heights
-                .insert(outpoint, expiration_height);
+            locked_outpoints.insert(outpoint, true);
         }
 
         changeset.local_chain = local_chain::ChangeSet::from_sqlite(db_tx)?;
@@ -318,33 +309,29 @@ impl ChangeSet {
             })?;
         }
 
-        // Insert locked outpoints.
-        let mut stmt = db_tx.prepare_cached(&format!(
-            "INSERT INTO {}(txid, vout, is_locked) VALUES(:txid, :vout, :is_locked) ON CONFLICT DO UPDATE SET is_locked=:is_locked",
-            Self::WALLET_OUTPOINT_LOCK_TABLE_NAME,
+        // Insert or delete locked outpoints.
+        let mut insert_stmt = db_tx.prepare_cached(&format!(
+            "REPLACE INTO {}(txid, vout) VALUES(:txid, :vout)",
+            Self::WALLET_OUTPOINT_LOCK_TABLE_NAME
         ))?;
-        let locked_outpoints = &self.locked_outpoints.locked_outpoints;
-        for (&outpoint, is_locked) in locked_outpoints.iter() {
-            let OutPoint { txid, vout } = outpoint;
-            stmt.execute(named_params! {
-                ":txid": Impl(txid),
-                ":vout": vout,
-                ":is_locked": is_locked,
-            })?;
-        }
-        // Insert locked outpoints expiration heights.
-        let mut stmt = db_tx.prepare_cached(&format!(
-            "INSERT INTO {}(txid, vout, expiration_height) VALUES(:txid, :vout, :expiration_height) ON CONFLICT DO UPDATE SET expiration_height=:expiration_height",
+        let mut delete_stmt = db_tx.prepare_cached(&format!(
+            "DELETE FROM {} WHERE txid=:txid AND vout=:vout",
             Self::WALLET_OUTPOINT_LOCK_TABLE_NAME,
         ))?;
-        let expiration_heights = &self.locked_outpoints.expiration_heights;
-        for (&outpoint, expiration_height) in expiration_heights.iter() {
+        let locked_outpoints = &self.locked_outpoints.locked_outpoints;
+        for (&outpoint, &is_locked) in locked_outpoints.iter() {
             let OutPoint { txid, vout } = outpoint;
-            stmt.execute(named_params! {
-                ":txid": Impl(txid),
-                ":vout": vout,
-                ":expiration_height": expiration_height,
-            })?;
+            if is_locked {
+                insert_stmt.execute(named_params! {
+                    ":txid": Impl(txid),
+                    ":vout": vout,
+                })?;
+            } else {
+                delete_stmt.execute(named_params! {
+                    ":txid": Impl(txid),
+                    ":vout": vout,
+                })?;
+            }
         }
 
         self.local_chain.persist_to_sqlite(db_tx)?;
@@ -390,3 +377,12 @@ impl From<keychain_txout::ChangeSet> for ChangeSet {
         }
     }
 }
+
+impl From<locked_outpoints::ChangeSet> for ChangeSet {
+    fn from(locked_outpoints: locked_outpoints::ChangeSet) -> Self {
+        Self {
+            locked_outpoints,
+            ..Default::default()
+        }
+    }
+}

wallet/src/wallet/locked_outpoints.rs

@@ -11,17 +11,16 @@ use crate::collections::BTreeMap;
 pub struct ChangeSet {
     /// The lock status of an outpoint, `true == is_locked`.
     pub locked_outpoints: BTreeMap<OutPoint, bool>,
-    /// The expiration height of the lock.
-    pub expiration_heights: BTreeMap<OutPoint, Option<u32>>,
 }
 
 impl Merge for ChangeSet {
     fn merge(&mut self, other: Self) {
+        // Extend self with other. Any entries in `self` that share the same
+        // outpoint are overwritten.
         self.locked_outpoints.extend(other.locked_outpoints);
-        self.expiration_heights.extend(other.expiration_heights);
     }
 
     fn is_empty(&self) -> bool {
-        self.locked_outpoints.is_empty() && self.expiration_heights.is_empty()
+        self.locked_outpoints.is_empty()
     }
 }

wallet/src/wallet/mod.rs

@@ -660,21 +660,19 @@ impl Wallet {
         indexed_graph.apply_changeset(changeset.tx_graph.into());
 
         // Apply locked outpoints
-        let mut locked_outpoints = BTreeMap::new();
-        let locked_outpoints::ChangeSet {
-            locked_outpoints: locked_utxos,
-            expiration_heights,
-        } = changeset.locked_outpoints;
-        for (outpoint, is_locked) in locked_utxos {
-            locked_outpoints.insert(
-                outpoint,
-                UtxoLock {
+        let locked_outpoints = changeset.locked_outpoints.locked_outpoints;
+        let locked_outpoints = locked_outpoints
+            .into_iter()
+            .map(|(outpoint, is_locked)| {
+                (
                     outpoint,
-                    is_locked,
-                    expiration_height: expiration_heights.get(&outpoint).cloned().flatten(),
-                },
-            );
-        }
+                    UtxoLock {
+                        outpoint,
+                        is_locked,
+                    },
+                )
+            })
+            .collect();
 
         let stage = ChangeSet::default();
 
@@ -2414,29 +2412,22 @@ impl Wallet {
             .map(|output| output.outpoint)
     }
 
-    /// Whether the `outpoint` is currently locked. See [`Wallet::lock_outpoint`] for more.
+    /// Whether the `outpoint` is locked. See [`Wallet::lock_outpoint`] for more.
     pub fn is_outpoint_locked(&self, outpoint: OutPoint) -> bool {
-        if let Some(utxo_lock) = self.locked_outpoints.get(&outpoint) {
-            if utxo_lock.is_locked {
-                return utxo_lock
-                    .expiration_height
-                    .map_or(true, |height| self.chain.tip().height() < height);
-            }
-        }
-        false
+        self.locked_outpoints
+            .get(&outpoint)
+            .map_or(false, |u| u.is_locked)
     }
 
     /// Lock a wallet output identified by the given `outpoint`.
     ///
     /// A locked UTXO will not be selected as an input to fund a transaction. This is useful
-    /// for excluding or reserving candidate inputs during transaction creation. You can optionally
-    /// specify the `expiration_height` of the lock that defines the height of the local chain at
-    /// which the outpoint becomes spendable.
+    /// for excluding or reserving candidate inputs during transaction creation.
     ///
-    /// You must persist the staged change for the lock status to be persistent. To unlock a
+    /// **You must persist the staged change for the lock status to be persistent**. To unlock a
     /// previously locked outpoint, see [`Wallet::unlock_outpoint`].
-    pub fn lock_outpoint(&mut self, outpoint: OutPoint, expiration_height: Option<u32>) {
-        use alloc::collections::btree_map;
+    pub fn lock_outpoint(&mut self, outpoint: OutPoint) {
+        use crate::collections::btree_map;
         let lock_value = true;
         let mut changeset = locked_outpoints::ChangeSet::default();
 
@@ -2449,36 +2440,26 @@ impl Wallet {
                     utxo.is_locked = lock_value;
                     changeset.locked_outpoints.insert(outpoint, lock_value);
                 }
-                if utxo.expiration_height != expiration_height {
-                    utxo.expiration_height = expiration_height;
-                    changeset
-                        .expiration_heights
-                        .insert(outpoint, expiration_height);
-                }
             }
             btree_map::Entry::Vacant(e) => {
                 e.insert(UtxoLock {
                     outpoint,
                     is_locked: lock_value,
-                    expiration_height,
                 });
                 changeset.locked_outpoints.insert(outpoint, lock_value);
-                changeset
-                    .expiration_heights
-                    .insert(outpoint, expiration_height);
             }
         };
 
-        self.stage.merge(ChangeSet {
-            locked_outpoints: changeset,
-            ..Default::default()
-        });
+        self.stage.merge(changeset.into());
     }
 
     /// Unlock the wallet output of the specified `outpoint`.
+    ///
+    /// **You must persist the staged changes.**
     pub fn unlock_outpoint(&mut self, outpoint: OutPoint) {
-        use alloc::collections::btree_map;
+        use crate::collections::btree_map;
         let lock_value = false;
+        let mut changeset = locked_outpoints::ChangeSet::default();
 
         match self.locked_outpoints.entry(outpoint) {
             btree_map::Entry::Vacant(..) => {}
@@ -2488,13 +2469,8 @@ impl Wallet {
                 let utxo = e.get_mut();
                 if utxo.is_locked {
                     utxo.is_locked = lock_value;
-                    self.stage.merge(ChangeSet {
-                        locked_outpoints: locked_outpoints::ChangeSet {
-                            locked_outpoints: [(outpoint, lock_value)].into(),
-                            ..Default::default()
-                        },
-                        ..Default::default()
-                    });
+                    changeset.locked_outpoints.insert(outpoint, lock_value);
+                    self.stage.merge(changeset.into());
                 }
             }
         }
@@ -2703,22 +2679,18 @@ impl AsRef<bdk_chain::tx_graph::TxGraph<ConfirmationBlockTime>> for Wallet {
     }
 }
 
-/// Records the lock status of a wallet UTXO (outpoint).
+/// Records the lock status of a wallet UTXO.
 ///
-/// Only 1 [`UtxoLock`] may be applied to a particular outpoint at a time. Refer to the docs
+/// Only a single [`UtxoLock`] may be assigned to a particular outpoint at a time. Refer to the docs
 /// for [`Wallet::lock_outpoint`]. Note that the lock status is user-defined and does not take
-/// into account any timelocks directly encoded by the descriptor.
+/// into account any timelocks encoded by the descriptor.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[non_exhaustive]
 pub struct UtxoLock {
     /// Outpoint.
     pub outpoint: OutPoint,
-    /// Whether the outpoint is currently locked.
+    /// Whether the outpoint is locked.
     pub is_locked: bool,
-    /// Height at which the UTXO lock expires. The outpoint may be selected when the
-    /// wallet chain tip is at or above this height. If `None`, the lock remains in
-    /// effect unless explicitly unlocked.
-    pub expiration_height: Option<u32>,
 }
 
 /// Deterministically generate a unique name given the descriptors defining the wallet

wallet/tests/wallet.rs

@@ -74,7 +74,7 @@ fn test_lock_outpoint_persist() -> anyhow::Result<()> {
     let unspent = wallet.list_unspent().collect::<Vec<_>>();
     assert!(!unspent.is_empty());
     for utxo in unspent {
-        wallet.lock_outpoint(utxo.outpoint, None);
+        wallet.lock_outpoint(utxo.outpoint);
         assert!(
             wallet.is_outpoint_locked(utxo.outpoint),
             "Expect outpoint is locked"
@@ -133,55 +133,6 @@ fn test_lock_outpoint_persist() -> anyhow::Result<()> {
         assert!(!wallet.locked_outpoints().values().any(|u| u.is_locked));
         assert!(wallet.list_locked_unspent().next().is_none());
         wallet.persist(&mut conn)?;
-
-        // Test: Update lock expiry
-        let outpoint = unspent.first().unwrap().outpoint;
-        let mut expiry: u32 = 100;
-        wallet.lock_outpoint(outpoint, Some(expiry));
-        let changeset = wallet.staged().unwrap();
-        assert_eq!(
-            changeset
-                .locked_outpoints
-                .expiration_heights
-                .get(&outpoint)
-                .cloned()
-                .unwrap(),
-            Some(expiry)
-        );
-
-        expiry *= 2;
-        wallet.lock_outpoint(outpoint, Some(expiry));
-        let changeset = wallet.staged().unwrap();
-        assert_eq!(
-            changeset
-                .locked_outpoints
-                .expiration_heights
-                .get(&outpoint)
-                .cloned()
-                .unwrap(),
-            Some(expiry)
-        );
-        wallet.persist(&mut conn)?;
-
-        // Now advance the local chain
-        let block_199 = BlockId {
-            height: expiry - 1,
-            hash: BlockHash::all_zeros(),
-        };
-        insert_checkpoint(&mut wallet, block_199);
-        assert!(
-            wallet.is_outpoint_locked(outpoint),
-            "outpoint should be locked before expiration height"
-        );
-        let block_200 = BlockId {
-            height: expiry,
-            hash: BlockHash::all_zeros(),
-        };
-        insert_checkpoint(&mut wallet, block_200);
-        assert!(
-            !wallet.is_outpoint_locked(outpoint),
-            "outpoint should unlock at expiration height"
-        );
     }
 
     Ok(())