Merge pull request #210 from blend-capital/deposit-slippage

backstop: block user donations to prevent inflation attack vectors (#196)

backstop/src/backstop/deposit.rs

@@ -1,6 +1,4 @@
-use crate::{
-    constants::MIN_INITIAL_SHARES, contract::require_nonnegative, emissions, storage, BackstopError,
-};
+use crate::{contract::require_nonnegative, emissions, storage, BackstopError};
 use sep_41_token::TokenClient;
 use soroban_sdk::{panic_with_error, Address, Env};
 
@@ -22,7 +20,7 @@ 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 || (pool_balance.shares == 0 && to_mint < MIN_INITIAL_SHARES) {
+    if to_mint == 0 {
         panic_with_error!(e, &BackstopError::InvalidShareMintAmount);
     }
     pool_balance.deposit(amount, to_mint);
@@ -248,31 +246,31 @@ mod tests {
         });
     }
 
-    #[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);
-        });
-    }
+    // #[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/constants.rs

@@ -1,8 +1,5 @@
 /// Fixed-point scalar for 7 decimal numbers
 pub const SCALAR_7: i128 = 1_0000000;
 
-/// The minimum number of shares that can be created
-pub const MIN_INITIAL_SHARES: i128 = SCALAR_7 / 10;
-
 // The approximate deployment date of the backstop module TODO: pick one
 pub const BACKSTOP_EPOCH: u64 = 1441065600;

backstop/src/contract.rs

@@ -135,7 +135,7 @@ pub trait Backstop {
 
     /********** Fund Management *********/
 
-    /// Take backstop token from a pools backstop
+    /// (Only Pool) Take backstop token from a pools backstop
     ///
     /// ### Arguments
     /// * `from` - The address of the pool drawing tokens from the backstop
@@ -144,10 +144,11 @@ pub trait Backstop {
     /// * `to` - The address to send the backstop tokens to
     ///
     /// ### Errors
-    /// If the pool does not have enough backstop tokens
+    /// If the pool does not have enough backstop tokens, or if the pool does
+    /// not authorize the call
     fn draw(e: Env, pool_address: Address, amount: i128, to: Address);
 
-    /// Sends backstop tokens from "from" to a pools backstop
+    /// (Only Pool) Sends backstop tokens from "from" to a pools backstop
     ///
     /// NOTE: This is not a deposit, and "from" will permanently lose access to the funds
     ///
@@ -157,7 +158,8 @@ pub trait Backstop {
     /// * `amount` - The amount of BLND to add
     ///
     /// ### Errors
-    /// If the `pool_address` is not valid
+    /// If the `pool_address` is not valid, or if the pool does not
+    /// authorize the call
     fn donate(e: Env, from: Address, pool_address: Address, amount: i128);
 
     /// Updates the underlying value of 1 backstop token
@@ -193,6 +195,8 @@ impl Backstop for BackstopContract {
         storage::set_blnd_token(&e, &blnd_token);
         storage::set_usdc_token(&e, &usdc_token);
         storage::set_pool_factory(&e, &pool_factory);
+        // NOTE: For a replacement backstop, this value likely needs to be stored in persistent storage to avoid
+        //       an expiration occuring before a backstop swap is finalized.
         storage::set_drop_list(&e, &drop_list);
         storage::set_emitter(&e, &emitter);
 
@@ -324,6 +328,7 @@ impl Backstop for BackstopContract {
     fn donate(e: Env, from: Address, pool_address: Address, amount: i128) {
         storage::extend_instance(&e);
         from.require_auth();
+        pool_address.require_auth();
 
         backstop::execute_donate(&e, &from, &pool_address, amount);
         e.events()

test-suites/tests/test_backstop.rs

@@ -173,6 +173,25 @@ fn test_backstop() {
             }
         )
     );
+    assert_eq!(
+        fixture.env.auths()[1],
+        (
+            pool.address.clone(),
+            AuthorizedInvocation {
+                function: AuthorizedFunction::Contract((
+                    fixture.backstop.address.clone(),
+                    Symbol::new(&fixture.env, "donate"),
+                    vec![
+                        &fixture.env,
+                        frodo.to_val(),
+                        pool.address.to_val(),
+                        amount.into_val(&fixture.env)
+                    ]
+                )),
+                sub_invocations: std::vec![]
+            }
+        )
+    );
     assert_eq!(bstop_token.balance(&frodo), frodo_bstop_token_balance);
     assert_eq!(
         bstop_token.balance(&fixture.backstop.address),

test-suites/tests/test_backstop_inflation_attack.rs

@@ -6,13 +6,6 @@ use test_suites::{
     test_fixture::{TestFixture, TokenIndex, SCALAR_7},
 };
 
-// This test showcases an inflation attack during a backstops initialization stage. This is a
-// high risk inflation attack for the attacker, and requires significant capital to execute.
-// If the donation does not happen before the victim deposits, the attacker will lose the majority
-// of the capital they donated to the backstop.
-//
-// However, since the attack is feasible, this test exists to ensure it's effects are minimized
-// and common pitfalls are avoided.
 #[test]
 fn test_backstop_inflation_attack() {
     let mut fixture = TestFixture::create(false);
@@ -43,65 +36,38 @@ fn test_backstop_inflation_attack() {
     fixture.lp.transfer(&whale, &pippen, &starting_balance);
 
     // 1. Attacker deposits a small amount as the initial depositor
-
-    // contracts stop very small initial deposits
-    let bad_init_deposit =
-        fixture
-            .backstop
-            .try_deposit(&sauron, &pool_address, &(SCALAR_7 / 10 - 1));
-    assert_eq!(
-        bad_init_deposit.err(),
-        Some(Ok(Error::from_contract_error(1005)))
-    );
-
-    let sauron_deposit_amount = SCALAR_7 / 10;
+    let sauron_deposit_amount = 100;
     let sauron_shares = fixture
         .backstop
         .deposit(&sauron, &pool_address, &sauron_deposit_amount);
 
-    // 2. Attacker donates a large amount to the backstop before the victim can perform a deposit
-    let inflation_amount = 100_000 * SCALAR_7;
+    // 2. Attacker tries to send a large amount to the backstop before the victim can perform a deposit
+    let inflation_amount = 10_000 * SCALAR_7;
+    fixture
+        .lp
+        .transfer(&sauron, &pool_address, &inflation_amount);
+
+    // contract correctly mints share amounts regardless of the token balance
+    let deposit_amount = 100;
+    let pippen_shares = fixture
+        .backstop
+        .deposit(&pippen, &pool_address, &deposit_amount);
+    assert_eq!(pippen_shares, 100);
+    assert_eq!(sauron_shares, pippen_shares);
+
+    // 2b. Attacker tries to donate a large amount to the backstop before the victim can perform a deposit
+    //    #! NOTE - Contract will stop a random address from donating. This can ONLY come from the pool.
+    //              However, authorizations are mocked during intergation tests, so this will succeed.
     fixture
         .backstop
         .donate(&sauron, &pool_address, &inflation_amount);
 
     // contracts stop any zero share deposits
-    let mut deposit_amount = 1000;
     let bad_deposit_result = fixture
         .backstop
         .try_deposit(&pippen, &pool_address, &deposit_amount);
     assert_eq!(
         bad_deposit_result.err(),
         Some(Ok(Error::from_contract_error(1005)))
     );
-
-    // user can still be in a situation where they get adversely affected by the inflation attacks
-    // but to a small extent
-    deposit_amount = SCALAR_7;
-    let pippen_shares = fixture
-        .backstop
-        .deposit(&pippen, &pool_address, &deposit_amount);
-    assert_eq!(pippen_shares, 9); // actual is 9.99...
-
-    // 3. Attacker and victim withdraw funds
-    fixture
-        .backstop
-        .queue_withdrawal(&sauron, &pool_address, &sauron_shares);
-    fixture
-        .backstop
-        .queue_withdrawal(&pippen, &pool_address, &pippen_shares);
-
-    // wait enough time so all shares can be withdrawn
-    fixture.jump(21 * 24 * 60 * 60 + 1);
-
-    fixture
-        .backstop
-        .withdraw(&sauron, &pool_address, &sauron_shares);
-    fixture
-        .backstop
-        .withdraw(&pippen, &pool_address, &pippen_shares);
-
-    // pippen loses less than 10% of initial deposit due to rounding
-    assert!(fixture.lp.balance(&sauron) < starting_balance + SCALAR_7 / 10);
-    assert!(fixture.lp.balance(&pippen) > starting_balance - SCALAR_7 / 10);
 }