feat: feature wait before rejecting a withdrawal request

signer/src/stacks/contracts.rs

@@ -59,6 +59,7 @@ use crate::storage::model::ToLittleEndianOrder as _;
 use crate::storage::DbRead;
 use crate::DEPOSIT_DUST_LIMIT;
 use crate::WITHDRAWAL_BLOCKS_EXPIRY;
+use crate::WITHDRAWAL_MIN_CONFIRMATIONS;
 
 use super::api::StacksInteract;
 
@@ -1029,6 +1030,10 @@ pub enum WithdrawalRejectErrorMsg {
     /// been either accepted or rejected already.
     #[error("the smart contract has been updated to indicate that the request has been completed")]
     RequestCompleted,
+    /// A withdrawal request is still active if it's reasonably possible
+    /// for the request to be fulfilled by a sweep transaction on bitcoin.
+    #[error("the withdrawal request is still active, we cannot reject it yet")]
+    RequestStillActive,
     /// Withdrawal request fulfilled
     #[error("Withdrawal request fulfilled")]
     RequestFulfilled,
@@ -1109,6 +1114,9 @@ impl AsContractCall for RejectWithdrawalV1 {
     /// 6. Whether the withdrawal request has expired. Fail if it hasn't.
     /// 7. Whether the withdrawal request is being serviced by a sweep
     ///    transaction that is in the mempool.
+    /// 8. Whether we need to worry about forks causing the withdrawal to
+    ///    be confirmed by a sweep that was broadcst changing the status of
+    ///    the request from rejected to accepted.
     async fn validate<C>(&self, ctx: &C, req_ctx: &ReqContext) -> Result<(), Error>
     where
         C: Context + Send + Sync,
@@ -1191,6 +1199,16 @@ impl AsContractCall for RejectWithdrawalV1 {
             return Err(WithdrawalRejectErrorMsg::RequestBeingFulfilled.into_error(req_ctx, self));
         }
 
+        // 8. Check whether the withdrawal request is still active, as in
+        //    it could still be fulfilled.
+        let withdrawal_is_active = db
+            .is_withdrawal_active(&self.id, &req_ctx.chain_tip, WITHDRAWAL_MIN_CONFIRMATIONS)
+            .await?;
+
+        if withdrawal_is_active {
+            return Err(WithdrawalRejectErrorMsg::RequestStillActive.into_error(req_ctx, self));
+        }
+
         Ok(())
     }
 }

signer/src/storage/in_memory.rs

@@ -822,6 +822,15 @@ impl super::DbRead for SharedStore {
         unimplemented!()
     }
 
+    async fn is_withdrawal_active(
+        &self,
+        _: &model::QualifiedRequestId,
+        _: &model::BitcoinBlockRef,
+        _: u64,
+    ) -> Result<bool, Error> {
+        unimplemented!()
+    }
+
     async fn get_bitcoin_tx(
         &self,
         txid: &model::BitcoinTxId,

signer/src/storage/mod.rs

@@ -336,6 +336,17 @@ pub trait DbRead {
         bitcoin_chain_tip: &model::BitcoinBlockHash,
     ) -> impl Future<Output = Result<bool, Error>> + Send;
 
+    /// Returns whether we should consider the withdrawal active. A
+    /// withdrawal request is considered active if there is a reasonable
+    /// risk of the withdrawal being confirmed from a fork of blocks less
+    /// than `min_confirmations`.
+    fn is_withdrawal_active(
+        &self,
+        id: &model::QualifiedRequestId,
+        bitcoin_chain_tip: &model::BitcoinBlockRef,
+        min_confirmations: u64,
+    ) -> impl Future<Output = Result<bool, Error>> + Send;
+
     /// Fetch the bitcoin transaction that is included in the block
     /// identified by the block hash.
     fn get_bitcoin_tx(

signer/src/storage/postgres.rs

@@ -171,6 +171,8 @@ struct PgSignerUtxo {
     #[sqlx(try_from = "i64")]
     amount: u64,
     aggregate_key: PublicKey,
+    #[sqlx(try_from = "i64")]
+    block_height: u64,
 }
 
 impl From<PgSignerUtxo> for SignerUtxo {
@@ -401,11 +403,13 @@ impl PgStore {
         chain_tip: &model::BitcoinBlockHash,
         output_type: model::TxOutputType,
         min_block_height: i64,
-    ) -> Result<Option<SignerUtxo>, Error> {
-        let pg_utxo = sqlx::query_as::<_, PgSignerUtxo>(
+    ) -> Result<Option<PgSignerUtxo>, Error> {
+        sqlx::query_as::<_, PgSignerUtxo>(
             r#"
             WITH bitcoin_blockchain AS (
-                SELECT block_hash
+                SELECT 
+                    block_hash
+                  , block_height
                 FROM bitcoin_blockchain_until($1, $2)
             ),
             confirmed_sweeps AS (
@@ -422,6 +426,7 @@ impl PgStore {
               , bo.output_index
               , bo.amount
               , ds.aggregate_key
+              , bb.block_height
             FROM sbtc_signer.bitcoin_tx_outputs AS bo
             JOIN sbtc_signer.bitcoin_transactions AS bt USING (txid)
             JOIN bitcoin_blockchain AS bb USING (block_hash)
@@ -440,9 +445,7 @@ impl PgStore {
         .bind(output_type)
         .fetch_optional(&self.0)
         .await
-        .map_err(Error::SqlxQuery)?;
-
-        Ok(pg_utxo.map(SignerUtxo::from))
+        .map_err(Error::SqlxQuery)
     }
 
     /// Return the height of the earliest block in which a donation UTXO
@@ -480,6 +483,7 @@ impl PgStore {
         let output_type = model::TxOutputType::Donation;
         self.get_utxo(chain_tip, output_type, min_block_height)
             .await
+            .map(|pg_utxo| pg_utxo.map(SignerUtxo::from))
     }
     /// Return a block height that is less than or equal to the block that
     /// confirms the signers' UTXO.
@@ -1980,7 +1984,7 @@ impl super::DbRead for PgStore {
         // happens we try searching for a donation.
         let output_type = model::TxOutputType::SignersOutput;
         let fut = self.get_utxo(chain_tip, output_type, min_block_height);
-        match fut.await? {
+        match fut.await?.map(SignerUtxo::from) {
             res @ Some(_) => Ok(res),
             None => self.get_donation_utxo(chain_tip).await,
         }
@@ -2121,6 +2125,46 @@ impl super::DbRead for PgStore {
         .map_err(Error::SqlxQuery)
     }
 
+    async fn is_withdrawal_active(
+        &self,
+        id: &model::QualifiedRequestId,
+        bitcoin_chain_tip: &model::BitcoinBlockRef,
+        min_confirmations: u64,
+    ) -> Result<bool, Error> {
+        let min_block_height = sqlx::query_scalar::<_, Option<i64>>(
+            r#"
+            SELECT MAX(bb.block_height) + 1
+            FROM sbtc_signer.bitcoin_withdrawals_outputs AS bwo
+            JOIN sbtc_signer.bitcoin_blocks AS bb
+              ON bb.block_hash = bwo.bitcoin_chain_tip
+            WHERE bwo.request_id = $1
+              AND bwo.stacks_block_hash = $2
+            "#,
+        )
+        .bind(i64::try_from(id.request_id).map_err(Error::ConversionDatabaseInt)?)
+        .bind(id.block_hash)
+        .fetch_one(&self.0)
+        .await
+        .map_err(Error::SqlxQuery)?;
+
+        // This means that there are no rows associated with the ID in the
+        // `bitcoin_withdrawals_outputs` table.
+        let Some(min_block_height) = min_block_height else {
+            return Ok(false);
+        };
+
+        let output_type = model::TxOutputType::SignersOutput;
+        let chain_tip_hash = &bitcoin_chain_tip.block_hash;
+        let signer_utxo_fut = self.get_utxo(chain_tip_hash, output_type, min_block_height);
+        // If this returns None, then the sweep itself could be in the
+        // mempool. If that's the case then this is definitely active.
+        let Some(signer_utxo) = signer_utxo_fut.await? else {
+            return Ok(true);
+        };
+
+        Ok(signer_utxo.block_height + min_confirmations >= bitcoin_chain_tip.block_height)
+    }
+
     async fn get_bitcoin_tx(
         &self,
         txid: &model::BitcoinTxId,

signer/src/transaction_coordinator.rs

@@ -66,6 +66,7 @@ use crate::storage::DbRead as _;
 use crate::wsts_state_machine::FireCoordinator;
 use crate::wsts_state_machine::FrostCoordinator;
 use crate::wsts_state_machine::WstsCoordinator;
+use crate::WITHDRAWAL_MIN_CONFIRMATIONS;
 
 use bitcoin::hashes::Hash as _;
 use wsts::net::SignatureType;
@@ -781,13 +782,25 @@ where
         // the given withdrawal has been included in a sweep transaction
         // that could have been submitted. With this check we are more
         // confident that it is safe to reject the withdrawal.
+        let qualified_id = request.qualified_id();
         let withdrawal_inflight = db
-            .is_withdrawal_inflight(&request.qualified_id(), &chain_tip.block_hash)
+            .is_withdrawal_inflight(&qualified_id, &chain_tip.block_hash)
             .await?;
         if withdrawal_inflight {
             return Ok(());
         }
 
+        // The `DbRead::is_withdrawal_inflight` function considers whether
+        // we need to worry about a fork making a sweep fulfilling
+        // withdrawal active in the mempool.
+        let withdrawal_is_active = db
+            .is_withdrawal_active(&qualified_id, chain_tip, WITHDRAWAL_MIN_CONFIRMATIONS)
+            .await?;
+
+        if withdrawal_is_active {
+            return Ok(());
+        }
+
         let sign_request_fut = self.construct_withdrawal_reject_stacks_sign_request(
             &request,
             bitcoin_aggregate_key,

signer/tests/integration/setup.rs

@@ -25,6 +25,7 @@ use signer::bitcoin::rpc::BitcoinCoreClient;
 use signer::bitcoin::rpc::BitcoinTxInfo;
 use signer::bitcoin::rpc::GetTxResponse;
 use signer::bitcoin::utxo;
+use signer::bitcoin::utxo::Fees;
 use signer::bitcoin::utxo::SbtcRequests;
 use signer::bitcoin::utxo::SignerBtcState;
 use signer::bitcoin::utxo::SignerUtxo;
@@ -621,6 +622,15 @@ pub struct SweepTxInfo {
     pub tx_info: BitcoinTxInfo,
 }
 
+#[derive(Debug, Clone)]
+pub struct BroadcastSweepTxInfo {
+    /// The block hash of the bitcoin chain tip when the sweep transaction
+    /// was broadcast
+    pub block_hash: bitcoin::BlockHash,
+    /// The transaction that swept in the deposit transaction.
+    pub txid: bitcoin::Txid,
+}
+
 #[derive(Debug, Clone, Copy)]
 pub struct SweepAmounts {
     pub amount: u64,
@@ -655,6 +665,8 @@ pub struct TestSweepSetup2 {
     pub donation: OutPoint,
     /// The transaction that swept in the deposit transaction.
     pub sweep_tx_info: Option<SweepTxInfo>,
+    /// Information about the sweep transaction when it was broadcast.
+    pub broadcast_info: Option<BroadcastSweepTxInfo>,
     /// The stacks blocks confirming the withdrawal requests, along with a
     /// genesis block.
     pub stacks_blocks: Vec<model::StacksBlock>,
@@ -783,6 +795,7 @@ impl TestSweepSetup2 {
             deposit_block_hash,
             deposits,
             sweep_tx_info: None,
+            broadcast_info: None,
             donation,
             signers,
             stacks_blocks,
@@ -847,13 +860,31 @@ impl TestSweepSetup2 {
 
     /// This function generates a sweep transaction that sweeps in the
     /// deposited funds and sweeps out the withdrawal funds in a proper
-    /// sweep transaction, that is also confirmed on bitcoin.
-    pub fn submit_sweep_tx(&mut self, rpc: &Client, faucet: &Faucet) {
+    /// sweep transaction, it broadcasts this transaction to the bitcoin
+    /// network.
+    pub fn broadcast_sweep_tx(&mut self, rpc: &Client) {
         // Okay now we try to peg-in the deposit by making a transaction.
         // Let's start by getting the signer's sole UTXO.
         let aggregated_signer = &self.signers.signer;
         let signer_utxo = aggregated_signer.get_utxos(rpc, None).pop().unwrap();
 
+        // Well we want a BitcoinCoreClient, so we create one using the
+        // context. Not, the best thing to do, sorry. TODO: pass in a
+        // bitcoin core client object.
+        let settings = Settings::new_from_default_config().unwrap();
+        let btc = BitcoinCoreClient::try_from(&settings.bitcoin.rpc_endpoints[0]).unwrap();
+        let outpoint = OutPoint::new(signer_utxo.txid, signer_utxo.vout);
+        let txids = btc.get_tx_spending_prevout(&outpoint).unwrap();
+
+        let last_fees = txids
+            .iter()
+            .filter_map(|txid| btc.get_mempool_entry(txid).unwrap())
+            .map(|entry| Fees {
+                total: entry.fees.base.to_sat(),
+                rate: entry.fees.base.to_sat() as f64 / entry.vsize as f64,
+            })
+            .max_by(|fee1, fee2| fee1.rate.total_cmp(&fee2.rate));
+
         let withdrawals = self
             .withdrawals
             .iter()
@@ -875,7 +906,7 @@ impl TestSweepSetup2 {
                 },
                 fee_rate: 10.0,
                 public_key: aggregated_signer.keypair.x_only_public_key().0,
-                last_fees: None,
+                last_fees,
                 magic_bytes: [b'T', b'3'],
             },
             accept_threshold: 4,
@@ -898,7 +929,22 @@ impl TestSweepSetup2 {
             unsigned.tx.compute_txid()
         };
 
-        // Let's sweep in the transaction
+        let block_header = rpc.get_blockchain_info().unwrap();
+
+        self.broadcast_info = Some(BroadcastSweepTxInfo {
+            block_hash: block_header.best_block_hash,
+            txid,
+        });
+    }
+
+    /// This function generates a sweep transaction that sweeps in the
+    /// deposited funds and sweeps out the withdrawal funds in a proper
+    /// sweep transaction, that is also confirmed on bitcoin.
+    pub fn submit_sweep_tx(&mut self, rpc: &Client, faucet: &Faucet) {
+        self.broadcast_sweep_tx(rpc);
+        let txid = self.broadcast_info.as_ref().unwrap().txid;
+
+        // Let's confirm the sweep transaction
         let block_hash = faucet.generate_blocks(1).pop().unwrap();
         let block_header = rpc.get_block_header_info(&block_hash).unwrap();
 
@@ -941,18 +987,18 @@ impl TestSweepSetup2 {
     /// validation, where we write to the `bitcoin_withdrawals_outputs`
     /// table at the end.
     pub async fn store_bitcoin_withdrawals_outputs(&self, db: &PgStore) {
-        let sweep = self.sweep_tx_info.as_ref().expect("no sweep tx info set");
+        let sweep = self.broadcast_info.as_ref().expect("no sweep tx info set");
 
         for (index, withdrawal) in self.withdrawals.iter().enumerate() {
             let swept_output = BitcoinWithdrawalOutput {
                 request_id: withdrawal.request.request_id,
                 stacks_txid: withdrawal.request.txid,
                 stacks_block_hash: withdrawal.request.block_hash,
-                bitcoin_chain_tip: sweep.block_hash,
+                bitcoin_chain_tip: sweep.block_hash.into(),
                 is_valid_tx: true,
                 validation_result: WithdrawalValidationResult::Ok,
                 output_index: index as u32 + 2,
-                bitcoin_txid: sweep.tx_info.txid.into(),
+                bitcoin_txid: sweep.txid.into(),
             };
             db.write_bitcoin_withdrawals_outputs(&[swept_output])
                 .await

signer/tests/integration/transaction_coordinator.rs

@@ -3056,6 +3056,7 @@ fn create_test_setup(
         deposit_block_hash,
         deposits: vec![(deposit_info, deposit_request, tx_info)],
         sweep_tx_info: None,
+        broadcast_info: None,
         donation,
         stacks_blocks: vec![stacks_block],
         signers: test_signers,