WIP validation [ci skip]

signer/src/stacks/contracts.rs

@@ -18,6 +18,7 @@
 
 use std::collections::BTreeSet;
 use std::future::Future;
+use std::ops::Deref;
 use std::sync::OnceLock;
 
 use bitcoin::hashes::Hash as _;
@@ -42,8 +43,9 @@ use blockstack_lib::types::chainstate::StacksAddress;
 use crate::error::Error;
 use crate::keys::PublicKey;
 use crate::stacks::wallet::SignerWallet;
+use crate::storage::model::BitcoinBlockHash;
+use crate::storage::model::BitcoinTxId;
 use crate::storage::DbRead;
-use crate::transaction_signer::ChainContext;
 
 /// A struct describing any transaction post-execution conditions that we'd
 /// like to enforce.
@@ -180,7 +182,9 @@ pub struct CompleteDepositV1 {
     /// The outpoint of the bitcoin UTXO that was spent as a deposit for
     /// sBTC.
     pub outpoint: OutPoint,
-    /// The amount of sats associated with the above UTXO.
+    /// The amount of sats swept in by the signers when they moved in the
+    /// above UTXO. This amount is less than the amount associated with the
+    /// above UTXO because of bitcoin mining fees.
     pub amount: u64,
     /// The address where the newly minted sBTC will be deposited.
     pub recipient: PrincipalData,
@@ -229,6 +233,25 @@ impl AsContractCall for CompleteDepositV1 {
     }
 }
 
+///
+pub struct DepositContext {
+    /// The block hash on the bitcoin blockchain with the greatest height.
+    /// On ties we sort by the block_hash descending and take the first
+    /// one.
+    pub chain_tip: BitcoinBlockHash,
+    /// How many bitcoin blocks back from the chain tip the signer will
+    /// look for requests.
+    pub context_window: u16,
+    /// The transaction ID for the sweep transaction that moved the deposit
+    /// UTXO into the signers' UTXO.
+    pub txid: BitcoinTxId,
+    /// The public key of the signer that created the deposit request
+    /// transaction.
+    pub origin_signer: PublicKey,
+    /// The number of signatures required for an accepted deposit request.
+    pub signatures_required: u16,
+}
+
 impl CompleteDepositV1 {
     /// Validates that the Complete deposit request satisfies the following
     /// criteria:
@@ -238,29 +261,73 @@ impl CompleteDepositV1 {
     ///    transaction.
     /// 3. That the signer sweep transaction exists on the canonical
     ///    bitcoin blockchain.
-    /// 5. That the `amount` matches the amount in the `outpoint` less
+    /// 4. That the `amount` matches the amount in the `outpoint` less
     ///    their portion of fees spent in the sweep transaction.
-    /// 4. That the principal matches the principal embedded in the deposit
+    /// 5. That the principal matches the principal embedded in the deposit
     ///    script locked in the outpoint.
-    pub async fn validate2<S>(&self, storage: &S, ctx: &ChainContext) -> Result<bool, Error>
+    pub async fn validate2<S>(&self, storage: &S, ctx: &DepositContext) -> Result<bool, Error>
     where
         S: DbRead + Send + Sync,
         Error: From<<S as DbRead>::Error>,
     {
-        let chain_tip = ctx.bitcoin_chain_tip.into();
+        // The `complete-deposit-wrapper` public function will not mint to
+        // the user again if we mistakenly submit two transactions for the
+        // same deposit outpoint. This means we do not need to do a check
+        // for existance of a similar transaction in the stacks mempool. This is
+        // fortunate, because even if we wanted to, the only view into the
+        // stacks-core mempool is through the `POST /new_mempool_tx`
+        // webhooks, which we do not currently injest and could be
+        // incomplete.
+
+        // Check that this is actually a pending and accepted deposit
+        // request.
         let deposit_requests = storage
-            .get_pending_accepted_deposit_requests(&chain_tip, ctx.context_window, ctx.threshold)
+            .get_pending_accepted_deposit_requests(
+                &ctx.chain_tip,
+                ctx.context_window,
+                ctx.signatures_required,
+            )
             .await?;
 
         let deposit_request = deposit_requests.into_iter().find(|req| {
             req.txid == self.outpoint.txid.into() && req.output_index == self.outpoint.vout
         });
 
-        let Some(_deposit_request) = deposit_request else {
+        let Some(deposit_request) = deposit_request else {
             // We don't have a record of this outpoint being accepted in
-            // our database. This must be invalid to us then.
+            // our database. This looks invalid to me.
+            tracing::warn!(
+                "We do not have a record of the associated deposit request in our database"
+            );
             return Ok(false);
         };
+
+        if deposit_request.outpoint() != self.outpoint {
+            // Seriously, how did we messed this one up?
+            return Ok(false);
+        }
+
+        if &self.recipient != deposit_request.recipient.deref() {
+            // Whoa, how is the outpoint right but the recipient is wrong?.
+            // TODO: serialize this struct and the deposit request to a
+            // JSON and put it in the logs. Or store.
+            tracing::warn!("Recipient in transaction does not match deposit request");
+            return Ok(false);
+        }
+
+        if self.amount >= deposit_request.amount {
+            // The amount here must always be less than the original UTXO
+            // amount because bitcoin mining fees are always non-zero.
+            return Ok(false);
+        }
+
+        if deposit_request.amount - self.amount > deposit_request.max_fee {
+            // Well this is bad and it should never happen. The smart
+            // contract does not care about us exceeding the max fee
+            // though.
+            tracing::warn!(%self.outpoint, "fee paid for deposit exceeded the max-fee");
+        }
+
         Ok(false)
     }
 }

signer/src/transaction_signer.rs

@@ -129,22 +129,6 @@ pub struct TxSignerEventLoop<Network, Storage, BlocklistChecker, Rng> {
     pub test_observer_tx: Option<tokio::sync::mpsc::Sender<TxSignerEvent>>,
 }
 
-/// Contains the essential context of the current state of the bitcoin
-/// blockchain and the current signer set.
-#[derive(Debug, Copy, Clone)]
-pub struct ChainContext {
-    /// The block hash on the bitcoin blockchain with the greatest height.
-    /// On ties we sort by the block_hash descending and take the first
-    /// one.
-    pub bitcoin_chain_tip: bitcoin::BlockHash,
-    /// How many bitcoin blocks back from the chain tip the signer will
-    /// look for requests.
-    pub context_window: u16,
-    /// The number of signatures necessary for the signers' wallet on the
-    /// stacks blockchain.
-    pub threshold: u16,
-}
-
 /// Event useful for tests
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum TxSignerEvent {