diff --git a/backstop/src/backstop/deposit.rs b/backstop/src/backstop/deposit.rs index 33067042..583c6b74 100644 --- a/backstop/src/backstop/deposit.rs +++ b/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); + // }); + // } } diff --git a/backstop/src/backstop/fund_management.rs b/backstop/src/backstop/fund_management.rs index 18d68c4f..cd2374f7 100644 --- a/backstop/src/backstop/fund_management.rs +++ b/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); diff --git a/backstop/src/constants.rs b/backstop/src/constants.rs index 7c7b77f5..26e7f9ce 100644 --- a/backstop/src/constants.rs +++ b/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; diff --git a/backstop/src/contract.rs b/backstop/src/contract.rs index d3037c37..72f6ecf7 100644 --- a/backstop/src/contract.rs +++ b/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() diff --git a/test-suites/tests/test_backstop.rs b/test-suites/tests/test_backstop.rs index 31b0e252..aa465327 100644 --- a/test-suites/tests/test_backstop.rs +++ b/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), diff --git a/test-suites/tests/test_backstop_inflation_attack.rs b/test-suites/tests/test_backstop_inflation_attack.rs index 580ea7eb..799aa601 100644 --- a/test-suites/tests/test_backstop_inflation_attack.rs +++ b/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,30 +36,33 @@ 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); @@ -74,34 +70,4 @@ fn test_backstop_inflation_attack() { 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); }