Merge pull request #208 from blend-capital/audit-updates

Audit updates

Cargo.toml

@@ -26,7 +26,7 @@ codegen-units = 1
 lto = true
 
 [workspace.dependencies.soroban-sdk]
-version = "20.0.0"
+version = "20.3.2"
 
 [workspace.dependencies.soroban-fixed-point-math]
 version = "1.0.0"

backstop/src/backstop/deposit.rs

@@ -20,6 +20,9 @@ pub fn execute_deposit(e: &Env, from: &Address, pool_address: &Address, amount:
     backstop_token_client.transfer(from, &e.current_contract_address(), &amount);
 
     let to_mint = pool_balance.convert_to_shares(amount);
+    if to_mint == 0 {
+        panic_with_error!(e, &BackstopError::InvalidShareMintAmount);
+    }
     pool_balance.deposit(amount, to_mint);
     user_balance.add_shares(to_mint);
 
@@ -35,6 +38,7 @@ mod tests {
 
     use crate::{
         backstop::execute_donate,
+        constants::SCALAR_7,
         testutils::{create_backstop, create_backstop_token, create_mock_pool_factory},
     };
 
@@ -208,4 +212,65 @@ mod tests {
             execute_deposit(&e, &samwise, &pool_0_id, 100);
         });
     }
+
+    #[test]
+    #[should_panic(expected = "Error(Contract, #1005)")]
+    fn test_execute_deposit_zero_share_mint() {
+        let e = Env::default();
+        e.budget().reset_unlimited();
+        e.mock_all_auths_allowing_non_root_auth();
+
+        let backstop_address = create_backstop(&e);
+        let bombadil = Address::generate(&e);
+        let samwise = Address::generate(&e);
+        let frodo = Address::generate(&e);
+        let pool_0_id = Address::generate(&e);
+        let pool_1_id = Address::generate(&e);
+
+        let (_, backstop_token_client) = create_backstop_token(&e, &backstop_address, &bombadil);
+        backstop_token_client.mint(&samwise, &100_0000000);
+        backstop_token_client.mint(&frodo, &100_000_000_0000000);
+
+        let (_, mock_pool_factory_client) = create_mock_pool_factory(&e, &backstop_address);
+        mock_pool_factory_client.set_pool(&pool_0_id);
+        mock_pool_factory_client.set_pool(&pool_1_id);
+
+        // initialize pool 0 with funds + some profit
+        e.as_contract(&backstop_address, || {
+            execute_deposit(&e, &frodo, &pool_0_id, SCALAR_7);
+            execute_donate(&e, &frodo, &pool_0_id, 10_000_000 * SCALAR_7);
+        });
+
+        e.as_contract(&backstop_address, || {
+            execute_deposit(&e, &samwise, &pool_0_id, SCALAR_7);
+        });
+    }
+
+    // #[test]
+    // #[should_panic(expected = "Error(Contract, #1005)")]
+    // fn test_execute_deposit_small_initial_mint() {
+    //     let e = Env::default();
+    //     e.budget().reset_unlimited();
+    //     e.mock_all_auths_allowing_non_root_auth();
+
+    //     let backstop_address = create_backstop(&e);
+    //     let bombadil = Address::generate(&e);
+    //     let samwise = Address::generate(&e);
+    //     let frodo = Address::generate(&e);
+    //     let pool_0_id = Address::generate(&e);
+    //     let pool_1_id = Address::generate(&e);
+
+    //     let (_, backstop_token_client) = create_backstop_token(&e, &backstop_address, &bombadil);
+    //     backstop_token_client.mint(&samwise, &100_0000000);
+    //     backstop_token_client.mint(&frodo, &100_0000000);
+
+    //     let (_, mock_pool_factory_client) = create_mock_pool_factory(&e, &backstop_address);
+    //     mock_pool_factory_client.set_pool(&pool_0_id);
+    //     mock_pool_factory_client.set_pool(&pool_1_id);
+
+    //     e.as_contract(&backstop_address, || {
+    //         execute_donate(&e, &frodo, &pool_0_id, SCALAR_7);
+    //         execute_deposit(&e, &samwise, &pool_0_id, SCALAR_7 / 10 - 1);
+    //     });
+    // }
 }

backstop/src/backstop/fund_management.rs

@@ -9,6 +9,8 @@ use soroban_sdk::{panic_with_error, unwrap::UnwrapOptimized, Address, Env};
 use super::require_is_from_pool_factory;
 
 /// Perform a draw from a pool's backstop
+///
+/// `pool_address` MUST be authenticated before calling
 pub fn execute_draw(e: &Env, pool_address: &Address, amount: i128, to: &Address) {
     require_nonnegative(e, amount);
 

backstop/src/backstop/pool.rs

@@ -57,36 +57,26 @@ pub fn require_is_from_pool_factory(e: &Env, address: &Address, balance: i128) {
     }
 }
 
-/// TODO: Duplicated from pool/pool/status.rs. Can this be moved to a common location?
-///
 /// Calculate the threshold for the pool's backstop balance
 ///
 /// Returns true if the pool's backstop balance is above the threshold
-/// NOTE: The calculation is the percentage^5 to simplify the calculation of the pools product constant.
-///       Some useful calculation results:
-///         - greater than 1 = 100+%
-///         - 1_0000000 = 100%
-///         - 0_0000100 = ~10%
-///         - 0_0000003 = ~5%
-///         - 0_0000000 = ~0-4%
 pub fn require_pool_above_threshold(pool_backstop_data: &PoolBackstopData) -> bool {
     // @dev: Calculation for pools product constant of underlying will often overflow i128
     //       so saturating mul is used. This is safe because the threshold is below i128::MAX and the
     //       protocol does not need to differentiate between pools over the threshold product constant.
     //       The calculation is:
     //        - Threshold % = (bal_blnd^4 * bal_usdc) / PC^5 such that PC is 200k
     let threshold_pc = 320_000_000_000_000_000_000_000_000i128; // 3.2e26 (200k^5)
-                                                                // floor balances to nearest full unit and calculate saturated pool product constant
-                                                                // and scale to SCALAR_7 to get final division result in SCALAR_7 points
+
+    // floor balances to nearest full unit and calculate saturated pool product constant
     let bal_blnd = pool_backstop_data.blnd / SCALAR_7;
     let bal_usdc = pool_backstop_data.usdc / SCALAR_7;
     let saturating_pool_pc = bal_blnd
         .saturating_mul(bal_blnd)
         .saturating_mul(bal_blnd)
         .saturating_mul(bal_blnd)
-        .saturating_mul(bal_usdc)
-        .saturating_mul(SCALAR_7); // 10^7 * 10^7
-    saturating_pool_pc / threshold_pc >= SCALAR_7
+        .saturating_mul(bal_usdc);
+    saturating_pool_pc >= threshold_pc
 }
 
 /// The pool's backstop balances
@@ -134,8 +124,6 @@ impl PoolBalance {
 
     /// Deposit tokens and shares into the pool
     ///
-    /// If this is the first time
-    ///
     /// ### Arguments
     /// * `tokens` - The amount of tokens to add
     /// * `shares` - The amount of shares to add

backstop/src/backstop/withdrawal.rs

@@ -1,6 +1,6 @@
-use crate::{contract::require_nonnegative, emissions, storage};
+use crate::{contract::require_nonnegative, emissions, storage, BackstopError};
 use sep_41_token::TokenClient;
-use soroban_sdk::{unwrap::UnwrapOptimized, Address, Env};
+use soroban_sdk::{panic_with_error, unwrap::UnwrapOptimized, Address, Env};
 
 use super::Q4W;
 
@@ -56,6 +56,9 @@ pub fn execute_withdraw(e: &Env, from: &Address, pool_address: &Address, amount:
     user_balance.dequeue_shares_for_withdrawal(e, amount, true);
 
     let to_return = pool_balance.convert_to_tokens(amount);
+    if to_return == 0 {
+        panic_with_error!(e, &BackstopError::InvalidTokenWithdrawAmount);
+    }
     pool_balance.withdraw(e, to_return, amount);
 
     storage::set_user_balance(e, pool_address, from, &user_balance);
@@ -75,7 +78,7 @@ mod tests {
     };
 
     use crate::{
-        backstop::{execute_deposit, execute_donate},
+        backstop::{execute_deposit, execute_donate, execute_draw},
         testutils::{
             assert_eq_vec_q4w, create_backstop, create_backstop_token, create_mock_pool_factory,
         },
@@ -363,6 +366,7 @@ mod tests {
             assert_eq!(backstop_token_client.balance(&samwise), tokens);
         });
     }
+
     #[test]
     #[should_panic(expected = "Error(Contract, #8)")]
     fn test_execute_withdrawal_negative_amount() {
@@ -413,4 +417,58 @@ mod tests {
             execute_withdraw(&e, &samwise, &pool_address, -42_0000000);
         });
     }
+
+    #[test]
+    #[should_panic(expected = "Error(Contract, #1006)")]
+    fn test_execute_withdrawal_zero_tokens() {
+        let e = Env::default();
+        e.mock_all_auths_allowing_non_root_auth();
+
+        let backstop_address = create_backstop(&e);
+        let pool_address = Address::generate(&e);
+        let bombadil = Address::generate(&e);
+        let samwise = Address::generate(&e);
+        let frodo = Address::generate(&e);
+
+        let (_, backstop_token_client) = create_backstop_token(&e, &backstop_address, &bombadil);
+        backstop_token_client.mint(&samwise, &150_0000000);
+        backstop_token_client.mint(&frodo, &150_0000000);
+
+        let (_, mock_pool_factory_client) = create_mock_pool_factory(&e, &backstop_address);
+        mock_pool_factory_client.set_pool(&pool_address);
+
+        e.ledger().set(LedgerInfo {
+            protocol_version: 20,
+            sequence_number: 200,
+            timestamp: 10000,
+            network_id: Default::default(),
+            base_reserve: 10,
+            min_temp_entry_ttl: 10,
+            min_persistent_entry_ttl: 10,
+            max_entry_ttl: 2000000,
+        });
+
+        // setup pool with queue for withdrawal and allow the backstop to incur a profit
+        e.as_contract(&backstop_address, || {
+            execute_deposit(&e, &frodo, &pool_address, 1_0000001);
+            execute_deposit(&e, &samwise, &pool_address, 1_0000000);
+            execute_queue_withdrawal(&e, &samwise, &pool_address, 1_0000000);
+            execute_draw(&e, &pool_address, 1_9999999, &frodo);
+        });
+
+        e.ledger().set(LedgerInfo {
+            protocol_version: 20,
+            sequence_number: 200,
+            timestamp: 10000 + 21 * 24 * 60 * 60 + 1,
+            network_id: Default::default(),
+            base_reserve: 10,
+            min_temp_entry_ttl: 10,
+            min_persistent_entry_ttl: 10,
+            max_entry_ttl: 2000000,
+        });
+
+        e.as_contract(&backstop_address, || {
+            execute_withdraw(&e, &samwise, &pool_address, 1_0000000);
+        });
+    }
 }