From f37f3823103ea4a88d1d2e164f19cddc7f352a24 Mon Sep 17 00:00:00 2001 From: mootz12 Date: Fri, 29 Dec 2023 14:10:38 -0500 Subject: [PATCH] pool: chore: move max positions check logic to once per request process --- pool/src/pool/actions.rs | 86 +++++++++++++++++++++++++++++-- pool/src/pool/pool.rs | 107 +++++++++++++++++++-------------------- pool/src/pool/submit.rs | 2 +- pool/src/pool/user.rs | 8 +++ 4 files changed, 143 insertions(+), 60 deletions(-) diff --git a/pool/src/pool/actions.rs b/pool/src/pool/actions.rs index d9933da5..17cc192a 100644 --- a/pool/src/pool/actions.rs +++ b/pool/src/pool/actions.rs @@ -71,6 +71,7 @@ pub fn build_actions_from_request( ) -> (Actions, User, bool) { let mut actions = Actions::new(e); let mut from_state = User::load(e, from); + let prev_positions_count = from_state.positions.effective_count(); let mut check_health = false; for request in requests.iter() { // verify the request is allowed @@ -280,9 +281,11 @@ pub fn build_actions_from_request( } _ => panic_with_error!(e, PoolError::BadRequest), } - // Verify max positions haven't been exceeded - pool.require_under_max(e, from_state.positions.clone(), request.request_type) } + + // Verify max positions haven't been exceeded + pool.require_under_max(e, &from_state.positions, prev_positions_count); + (actions, from_state, check_health) } @@ -1133,7 +1136,7 @@ mod tests { oracle: oracle_address, bstop_rate: 0_100_000_000, status: 0, - max_positions: 2, + max_positions: 4, }; let positions: Positions = Positions { collateral: map![ @@ -1515,6 +1518,69 @@ mod tests { assert_eq!(actions.spender_transfer.len(), 0); }); } + + /********** positions_under_max **********/ + + #[test] + fn test_actions_requires_positions_under_max_with_decrease() { + let e = Env::default(); + e.mock_all_auths(); + + let bombadil = Address::generate(&e); + let samwise = Address::generate(&e); + let pool = testutils::create_pool(&e); + + let (underlying, _) = testutils::create_token_contract(&e, &bombadil); + let (reserve_config, reserve_data) = testutils::default_reserve_meta(); + testutils::create_reserve(&e, &pool, &underlying, &reserve_config, &reserve_data); + + let (underlying_1, _) = testutils::create_token_contract(&e, &bombadil); + let (reserve_config, reserve_data) = testutils::default_reserve_meta(); + testutils::create_reserve(&e, &pool, &underlying_1, &reserve_config, &reserve_data); + + e.ledger().set(LedgerInfo { + timestamp: 600, + protocol_version: 20, + sequence_number: 1234, + network_id: Default::default(), + base_reserve: 10, + min_temp_entry_ttl: 10, + min_persistent_entry_ttl: 10, + max_entry_ttl: 2000000, + }); + + let pool_config = PoolConfig { + oracle: Address::generate(&e), + bstop_rate: 0_200_000_000, + status: 0, + max_positions: 2, + }; + + let user_positions = Positions { + liabilities: map![&e, (0, 5_0000000), (1, 1_0000000)], + collateral: map![&e, (0, 20_0000000), (1, 10)], + supply: map![&e], + }; + e.as_contract(&pool, || { + storage::set_pool_config(&e, &pool_config); + storage::set_user_positions(&e, &samwise, &user_positions); + + let mut pool = Pool::load(&e); + + let requests = vec![ + &e, + Request { + request_type: 3, + address: underlying_1.clone(), + amount: 20, + }, + ]; + + let (_, user, _) = build_actions_from_request(&e, &mut pool, &samwise, requests); + assert_eq!(user.positions.effective_count(), 3) + }); + } + #[test] #[should_panic(expected = "Error(Contract, #13)")] fn test_actions_requires_positions_under_max() { @@ -1528,6 +1594,7 @@ mod tests { let (underlying, _) = testutils::create_token_contract(&e, &bombadil); let (reserve_config, reserve_data) = testutils::default_reserve_meta(); testutils::create_reserve(&e, &pool, &underlying, &reserve_config, &reserve_data); + e.ledger().set(LedgerInfo { timestamp: 600, protocol_version: 20, @@ -1538,14 +1605,22 @@ mod tests { min_persistent_entry_ttl: 10, max_entry_ttl: 2000000, }); + let pool_config = PoolConfig { oracle: Address::generate(&e), bstop_rate: 0_200_000_000, status: 0, - max_positions: 0, + max_positions: 1, + }; + + let user_positions = Positions { + liabilities: map![&e], + collateral: map![&e, (0, 20_0000000)], + supply: map![&e], }; e.as_contract(&pool, || { storage::set_pool_config(&e, &pool_config); + storage::set_user_positions(&e, &samwise, &user_positions); let mut pool = Pool::load(&e); @@ -1554,9 +1629,10 @@ mod tests { Request { request_type: 4, address: underlying.clone(), - amount: 10_1234567, + amount: 1_0000000, }, ]; + build_actions_from_request(&e, &mut pool, &samwise, requests); }); } diff --git a/pool/src/pool/pool.rs b/pool/src/pool/pool.rs index 02c0319d..bdb8eb11 100644 --- a/pool/src/pool/pool.rs +++ b/pool/src/pool/pool.rs @@ -76,16 +76,18 @@ impl Pool { } } - /// Require that an action does not violate the maximum number of positions, or panic. + /// Require that a position does not violate the maximum number of positions, or panic. /// /// ### Arguments /// * `positions` - The user's positions - /// * `action_type` - The type of action being performed - pub fn require_under_max(&self, e: &Env, positions: Positions, action_type: u32) { - if self.config.max_positions - < positions.liabilities.len() + positions.collateral.len() as u32 - && (action_type == 2 || action_type == 4) - { + /// * `previous_num` - The number of positions the user previously had + /// + /// ### Panics + /// If the user has more positions than the maximum allowed and they are not + /// decreasing their number of positions + pub fn require_under_max(&self, e: &Env, positions: &Positions, previous_num: u32) { + let new_num = positions.effective_count(); + if new_num > previous_num && self.config.max_positions < new_num { panic_with_error!(e, PoolError::MaxPositionsExceeded) } } @@ -541,8 +543,9 @@ mod tests { assert!(false); }); } + #[test] - fn test_require_under_max_passes() { + fn test_require_under_max_empty() { let e = Env::default(); let samwise = Address::generate(&e); let pool = testutils::create_pool(&e); @@ -560,105 +563,97 @@ mod tests { max_positions: 2, }; e.as_contract(&pool, || { - user.add_collateral(&e, &mut reserve_0, 123); storage::set_pool_config(&e, &pool_config); + let prev_positions = user.positions.effective_count(); + let pool = Pool::load(&e); + user.add_collateral(&e, &mut reserve_0, 1); - pool.require_under_max(&e, user.positions, 4); + pool.require_under_max(&e, &user.positions, prev_positions); }); } + #[test] - #[should_panic(expected = "Error(Contract, #13)")] - fn test_require_under_max_fails_borrow() { + fn test_require_under_max_ignores_supply() { let e = Env::default(); let samwise = Address::generate(&e); let pool = testutils::create_pool(&e); let mut reserve_0 = testutils::default_reserve(&e); + let mut reserve_1 = testutils::default_reserve(&e); + reserve_1.index = 1; + let (oracle, _) = testutils::create_mock_oracle(&e); let mut user = User { address: samwise.clone(), positions: Positions::env_default(&e), }; - let (oracle, _) = testutils::create_mock_oracle(&e); let pool_config = PoolConfig { oracle, bstop_rate: 0_200_000_000, status: 0, - max_positions: 1, + max_positions: 2, }; e.as_contract(&pool, || { storage::set_pool_config(&e, &pool_config); - user.add_collateral(&e, &mut reserve_0, 123); - user.add_liabilities(&e, &mut reserve_0, 789); + user.add_supply(&e, &mut reserve_0, 42); + user.add_supply(&e, &mut reserve_1, 42); + user.add_collateral(&e, &mut reserve_1, 1); + let prev_positions = user.positions.effective_count(); + let pool = Pool::load(&e); + user.add_liabilities(&e, &mut reserve_1, 2); - pool.require_under_max(&e, user.positions, 4); + pool.require_under_max(&e, &user.positions, prev_positions); }); } - #[test] - #[should_panic(expected = "Error(Contract, #13)")] - fn test_require_under_max_fails_deposit() { - let e = Env::default(); - let samwise = Address::generate(&e); - let pool = testutils::create_pool(&e); - let mut reserve_0 = testutils::default_reserve(&e); - - let mut user = User { - address: samwise.clone(), - positions: Positions::env_default(&e), - }; - let (oracle, _) = testutils::create_mock_oracle(&e); - let pool_config = PoolConfig { - oracle, - bstop_rate: 0_200_000_000, - status: 0, - max_positions: 1, - }; - e.as_contract(&pool, || { - storage::set_pool_config(&e, &pool_config); - let pool = Pool::load(&e); - user.add_collateral(&e, &mut reserve_0, 123); - user.add_liabilities(&e, &mut reserve_0, 789); - pool.require_under_max(&e, user.positions, 2); - }); - } #[test] - fn test_require_under_max_passes_withdraw() { + fn test_require_under_max_allows_decreasing_change() { let e = Env::default(); let samwise = Address::generate(&e); let pool = testutils::create_pool(&e); let mut reserve_0 = testutils::default_reserve(&e); + let mut reserve_1 = testutils::default_reserve(&e); + reserve_1.index = 1; + let (oracle, _) = testutils::create_mock_oracle(&e); let mut user = User { address: samwise.clone(), positions: Positions::env_default(&e), }; - let (oracle, _) = testutils::create_mock_oracle(&e); let pool_config = PoolConfig { oracle, bstop_rate: 0_200_000_000, status: 0, - max_positions: 1, + max_positions: 2, }; e.as_contract(&pool, || { storage::set_pool_config(&e, &pool_config); + user.add_collateral(&e, &mut reserve_0, 42); + user.add_collateral(&e, &mut reserve_1, 42); + user.add_liabilities(&e, &mut reserve_0, 123); + user.add_liabilities(&e, &mut reserve_1, 123); + let prev_positions = user.positions.effective_count(); + let pool = Pool::load(&e); - user.add_collateral(&e, &mut reserve_0, 123); - user.add_liabilities(&e, &mut reserve_0, 789); + user.remove_collateral(&e, &mut reserve_1, 42); - pool.require_under_max(&e, user.positions, 3); + pool.require_under_max(&e, &user.positions, prev_positions); }); } + #[test] - fn test_require_under_max_passes_repay() { + #[should_panic(expected = "Error(Contract, #13)")] + fn test_require_under_max_panics_if_over() { let e = Env::default(); let samwise = Address::generate(&e); let pool = testutils::create_pool(&e); let mut reserve_0 = testutils::default_reserve(&e); + let mut reserve_1 = testutils::default_reserve(&e); + reserve_1.index = 1; let mut user = User { address: samwise.clone(), @@ -669,14 +664,18 @@ mod tests { oracle, bstop_rate: 0_200_000_000, status: 0, - max_positions: 1, + max_positions: 2, }; e.as_contract(&pool, || { storage::set_pool_config(&e, &pool_config); - let pool = Pool::load(&e); user.add_collateral(&e, &mut reserve_0, 123); user.add_liabilities(&e, &mut reserve_0, 789); - pool.require_under_max(&e, user.positions, 5); + let prev_positions = user.positions.effective_count(); + + let pool = Pool::load(&e); + user.add_liabilities(&e, &mut reserve_1, 42); + + pool.require_under_max(&e, &user.positions, prev_positions); }); } } diff --git a/pool/src/pool/submit.rs b/pool/src/pool/submit.rs index 810ea590..f28dbe75 100644 --- a/pool/src/pool/submit.rs +++ b/pool/src/pool/submit.rs @@ -161,7 +161,7 @@ mod tests { assert_eq!(underlying_1_client.balance(&merry), 1_5000000); }); } - + #[test] #[should_panic(expected = "Error(Contract, #10)")] fn test_submit_requires_healhty() { diff --git a/pool/src/pool/user.rs b/pool/src/pool/user.rs index d755ce2f..0244150b 100644 --- a/pool/src/pool/user.rs +++ b/pool/src/pool/user.rs @@ -22,6 +22,14 @@ impl Positions { supply: Map::new(e), } } + + /// Get the number of effective (impacts health factor) posiitons the user holds. + /// + /// This function ignores non-collateralized supply positions, as they are not relevant to the + /// max number of allowed positions by the pool. + pub fn effective_count(&self) -> u32 { + self.liabilities.len() + self.collateral.len() + } } /// A user / contracts position's with the pool