Validate amount when creating withdrawal request

common/src/borrow.rs

@@ -455,6 +455,9 @@ impl<M: BorrowMut<Market>> LinkedBorrowPositionMut<M> {
         .emit();
     }
 
+    /// Returns the amount that is left over after repaying the whole
+    /// position. That is, the return value is the number of tokens that may
+    /// be returned to the owner of the borrow position.
     pub fn record_repay(
         &mut self,
         proof: InterestAccumulationProof,

common/src/market/impl.rs

@@ -235,19 +235,15 @@ impl Market {
         let (account_id, requested_amount) = self.withdrawal_queue.try_lock()?;
 
         let Some((amount, mut supply_position)) = self
-            .get_linked_supply_position_mut(account_id.clone())
+            .get_linked_supply_position_mut(account_id)
             .and_then(|supply_position| {
                 // Cap withdrawal amount to deposit amount at most.
                 let amount = supply_position
                     .inner()
                     .get_borrow_asset_deposit()
                     .min(requested_amount);
 
-                if amount.is_zero() {
-                    None
-                } else {
-                    Some((amount, supply_position))
-                }
+                (!amount.is_zero()).then_some((amount, supply_position))
             })
         else {
             // The amount that the entry is eligible to withdraw is zero, so skip it.

common/src/supply.rs

@@ -168,8 +168,10 @@ impl<M: Borrow<Market>> LinkedSupplyPosition<M> {
             reason = "Assume # of snapshots is never >u32::MAX"
         )]
         for (i, snapshot) in it.enumerate().skip(next_snapshot_index as usize) {
-            accumulated += amount * Decimal::from(snapshot.yield_distribution)
-                / Decimal::from(snapshot.deposited);
+            if !snapshot.deposited.is_zero() {
+                accumulated += amount * Decimal::from(snapshot.yield_distribution)
+                    / Decimal::from(snapshot.deposited);
+            }
 
             next_snapshot_index = i as u32 + 1;
         }

contract/market/src/impl_helper.rs

@@ -44,14 +44,8 @@ impl Contract {
         amount: BorrowAssetAmount,
     ) -> BorrowAssetAmount {
         if let Some(mut borrow_position) = self.get_linked_borrow_position_mut(account_id) {
-            // TODO:
-            // Due to the slightly imprecise calculation of yield and
-            // other fees, the returning of the excess should be
-            // anything >1%, for example, over the total amount
-            // borrowed + fees/interest.
-            // -- https://github.com/Templar-Protocol/contract-mvp/pull/6#discussion_r1923876327
-
             let proof = borrow_position.accumulate_interest();
+            // Returns the amount that should be returned to the borrower.
             borrow_position.record_repay(proof, amount)
         } else {
             // No borrow exists: just return the whole amount.
@@ -227,7 +221,10 @@ impl Contract {
     }
 
     #[private]
-    pub fn after_execute_next_withdrawal(&mut self, withdrawal_resolution: WithdrawalResolution) {
+    pub fn execute_next_supply_withdrawal_request_01_finalize(
+        &mut self,
+        withdrawal_resolution: WithdrawalResolution,
+    ) {
         // TODO: Is this check even necessary in a #[private] function?
         require!(env::promise_results_count() == 1);
 

contract/market/src/impl_market_external.rs

@@ -157,17 +157,23 @@ impl MarketExternalInterface for Contract {
             "Amount to withdraw must be greater than zero",
         );
         let predecessor = env::predecessor_account_id();
-        if self
-            .get_linked_supply_position(predecessor.clone())
-            .filter(|supply_position| !supply_position.inner().get_borrow_asset_deposit().is_zero())
-            .is_none()
-        {
+        let Some(supply_position) =
+            self.get_linked_supply_position(predecessor.clone())
+                .filter(|supply_position| {
+                    !supply_position.inner().get_borrow_asset_deposit().is_zero()
+                })
+        else {
             env::panic_str("Supply position does not exist");
-        }
+        };
+
+        // We do check here, as well as during the execution.
+        // This check really only ensures that the `depth` reported by
+        // get_supply_withdrawal_queue_status() is realistically accurate.
+        require!(
+            supply_position.inner().get_borrow_asset_deposit() >= amount,
+            "Attempt to withdraw more than current deposit",
+        );
 
-        // TODO: Check that amount is a sane value? i.e. within the amount actually deposited?
-        // Probably not, since this should be checked during the actual execution of the withdrawal.
-        // No sense duplicating the check, probably.
         self.withdrawal_queue.remove(&predecessor);
         self.withdrawal_queue.insert_or_update(&predecessor, amount);
     }
@@ -192,7 +198,10 @@ impl MarketExternalInterface for Contract {
                     withdrawal_resolution.account_id.clone(),
                     withdrawal_resolution.amount_to_account,
                 )
-                .then(self_ext!().after_execute_next_withdrawal(withdrawal_resolution)),
+                .then(
+                    self_ext!()
+                        .execute_next_supply_withdrawal_request_01_finalize(withdrawal_resolution),
+                ),
         )
     }
 

contract/market/tests/supply_withdrawal_queue.rs

@@ -0,0 +1,85 @@
+use rstest::rstest;
+
+use templar_common::withdrawal_queue::WithdrawalQueueStatus;
+use test_utils::*;
+
+#[rstest]
+#[tokio::test]
+async fn successful_withdrawal() {
+    let SetupEverything { c, supply_user, .. } = setup_everything(|_| {}).await;
+
+    c.supply(&supply_user, 10_000).await;
+
+    let balance_before = c.borrow_asset_balance_of(supply_user.id()).await;
+    c.create_supply_withdrawal_request(&supply_user, 10_000)
+        .await;
+    let status = c.get_supply_withdrawal_queue_status().await;
+    assert_eq!(
+        status,
+        WithdrawalQueueStatus {
+            depth: 10_000.into(),
+            length: 1
+        },
+    );
+    c.execute_next_supply_withdrawal_request(&supply_user).await;
+    let balance_after = c.borrow_asset_balance_of(supply_user.id()).await;
+    assert_eq!(
+        balance_before + 10_000,
+        balance_after,
+        "Supply user should receive full deposit back"
+    );
+}
+
+#[rstest]
+#[tokio::test]
+async fn unsuccessful_withdrawal() {
+    let SetupEverything {
+        c,
+        supply_user,
+        borrow_user,
+        ..
+    } = setup_everything(|_| {}).await;
+
+    c.supply(&supply_user, 10_000).await;
+    c.collateralize(&borrow_user, 20_000).await;
+    c.borrow(&borrow_user, 5_000).await;
+
+    let balance_before = c.borrow_asset_balance_of(supply_user.id()).await;
+    c.create_supply_withdrawal_request(&supply_user, 10_000)
+        .await;
+    let status = c.get_supply_withdrawal_queue_status().await;
+    assert_eq!(
+        status,
+        WithdrawalQueueStatus {
+            depth: 10_000.into(),
+            length: 1
+        },
+    );
+    c.execute_next_supply_withdrawal_request(&supply_user).await;
+    let balance_after = c.borrow_asset_balance_of(supply_user.id()).await;
+    assert_eq!(
+        balance_before, balance_after,
+        "Supply user does not receive anything"
+    );
+
+    let status = c.get_supply_withdrawal_queue_status().await;
+    assert_eq!(
+        status,
+        WithdrawalQueueStatus {
+            depth: 10_000.into(),
+            length: 1
+        },
+        "Status of queue remains unchanged",
+    );
+}
+
+#[rstest]
+#[tokio::test]
+#[should_panic = "Smart contract panicked: Attempt to withdraw more than current deposit"]
+async fn attempt_to_withdraw_more_than_deposit() {
+    let SetupEverything { c, supply_user, .. } = setup_everything(|_| {}).await;
+
+    c.supply(&supply_user, 10_000).await;
+    c.create_supply_withdrawal_request(&supply_user, 12_000)
+        .await;
+}