From 3edc4ff88ed8e0d5621758c38a3da4b0cf2997c9 Mon Sep 17 00:00:00 2001 From: Allen Pocket Date: Fri, 24 Sep 2021 11:24:36 +0800 Subject: [PATCH 1/6] :sparkles: ($PALLET) Add option value to redeem --- pallets/liquidity-mining/src/lib.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/pallets/liquidity-mining/src/lib.rs b/pallets/liquidity-mining/src/lib.rs index 012494d959..c07dc71019 100644 --- a/pallets/liquidity-mining/src/lib.rs +++ b/pallets/liquidity-mining/src/lib.rs @@ -783,7 +783,7 @@ pub mod pallet { /// /// The extrinsic will: /// - Try to retire the liquidity-pool which has reached the end of life. - /// - Try to settle the rewards when the liquidity-pool in `Ongoing`. + /// - Try to settle the rewards. /// - Try to unreserve the remaining rewards to the pool investor when the deposit in the /// liquidity-pool is clear. /// - Try to delete the liquidity-pool in which the deposit becomes zero. @@ -792,9 +792,16 @@ pub mod pallet { /// The condition to redeem: /// - User should have some deposit in the liquidity-pool; /// - The liquidity-pool should be in special state: `Ongoing`, `Retired`; + /// + /// NOTE: All deposit will be redeemed when the pool is being retired, no matter the `value` + /// is. #[transactional] #[pallet::weight(T::WeightInfo::redeem())] - pub fn redeem(origin: OriginFor, pid: PoolId) -> DispatchResultWithPostInfo { + pub fn redeem( + origin: OriginFor, + pid: PoolId, + value: Option>, + ) -> DispatchResultWithPostInfo { let user = ensure_signed(origin)?; let mut pool: PoolInfo = @@ -816,10 +823,16 @@ pub mod pallet { let minimum_in_pool = match pool.state { PoolState::Ongoing => T::MinimumDepositOfUser::get(), PoolState::Retired => Zero::zero(), - _ => return Err(Error::::InvalidPoolState.into()), + _ => return Err(Error::::Unexpected.into()), + }; + + let try_redeemed = match pool.state { + PoolState::Ongoing => + min(value.unwrap_or(deposit_data.deposit), deposit_data.deposit), + PoolState::Retired => deposit_data.deposit, + _ => return Err(Error::::Unexpected.into()), }; - let try_redeemed = deposit_data.deposit; let left_in_pool = max(pool.deposit.saturating_sub(try_redeemed), minimum_in_pool); let can_redeemed = pool.deposit.saturating_sub(left_in_pool); let left_in_user = deposit_data.deposit.saturating_sub(can_redeemed); @@ -907,7 +920,7 @@ pub mod pallet { }, }; - Self::redeem(origin, pid)?; + Self::redeem(origin, pid, None)?; Ok(().into()) } From d143f0075ce78f21e2f95a55e3517bf8a2ecc65e Mon Sep 17 00:00:00 2001 From: Allen Pocket Date: Fri, 24 Sep 2021 11:25:38 +0800 Subject: [PATCH 2/6] :white_check_mark: ($PALLET) Cover unit-tests on last modification --- pallets/liquidity-mining/src/tests.rs | 162 ++++++++++++++++---------- 1 file changed, 100 insertions(+), 62 deletions(-) diff --git a/pallets/liquidity-mining/src/tests.rs b/pallets/liquidity-mining/src/tests.rs index bc5c026e19..82fbf1e253 100644 --- a/pallets/liquidity-mining/src/tests.rs +++ b/pallets/liquidity-mining/src/tests.rs @@ -947,24 +947,60 @@ fn redeem_from_pool_ongoing_should_work() { // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); - assert_ok!(LM::deposit(Some(USER_1).into(), 0, UNIT)); - assert_ok!(LM::deposit(Some(USER_2).into(), 0, UNIT)); + assert_ok!(LM::deposit(Some(USER_1).into(), 0, DEPOSIT_AMOUNT)); + assert_ok!(LM::deposit(Some(USER_2).into(), 0, DEPOSIT_AMOUNT)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, UNIT); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).reserved, 0); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, UNIT); + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).reserved, 0); run_to_block(100); let per_block = REWARD_AMOUNT / DAYS as Balance; - let pbpd = FixedU128::from((per_block, 2 * UNIT)); - let rewarded = (pbpd * (100 * UNIT).into()).into_inner() / FixedU128::accuracy(); + let pbpd = FixedU128::from((per_block, 2 * DEPOSIT_AMOUNT)); + let rewarded = (pbpd * (100 * DEPOSIT_AMOUNT).into()).into_inner() / FixedU128::accuracy(); + + let redeemed = DEPOSIT_AMOUNT / 2; + let deposit_left = DEPOSIT_AMOUNT - redeemed; + + assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(redeemed))); + + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, deposit_left); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).reserved, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_1).free, rewarded); + assert_eq!(Tokens::accounts(USER_1, REWARD_1).frozen, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_1).reserved, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_2).free, rewarded); + assert_eq!(Tokens::accounts(USER_1, REWARD_2).frozen, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); + assert_eq!(LM::user_deposit_data(0, USER_1).unwrap().deposit, deposit_left); + + assert_ok!(LM::redeem(Some(USER_2).into(), 0, Some(redeemed))); + + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, deposit_left); + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).reserved, 0); + assert_eq!(Tokens::accounts(USER_2, REWARD_1).free, rewarded); + assert_eq!(Tokens::accounts(USER_2, REWARD_1).frozen, 0); + assert_eq!(Tokens::accounts(USER_2, REWARD_1).reserved, 0); + assert_eq!(Tokens::accounts(USER_2, REWARD_2).free, rewarded); + assert_eq!(Tokens::accounts(USER_2, REWARD_2).frozen, 0); + assert_eq!(Tokens::accounts(USER_2, REWARD_2).reserved, 0); + assert_eq!(LM::user_deposit_data(0, USER_2).unwrap().deposit, deposit_left); + + assert_eq!(LM::pool(0).unwrap().deposit, 2 * deposit_left); + + run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); + let pbpd = FixedU128::from((per_block, 2 * deposit_left)); + let rewarded = (pbpd * (100 * deposit_left).into()).into_inner() / FixedU128::accuracy(); + + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); @@ -977,7 +1013,7 @@ fn redeem_from_pool_ongoing_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); assert!(LM::user_deposit_data(0, USER_1).is_none()); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, Some(Balance::MAX))); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, MinimumDeposit::get()); @@ -989,8 +1025,6 @@ fn redeem_from_pool_ongoing_should_work() { assert_eq!(Tokens::accounts(USER_2, REWARD_2).frozen, 0); assert_eq!(Tokens::accounts(USER_2, REWARD_2).reserved, 0); assert_eq!(LM::user_deposit_data(0, USER_2).unwrap().deposit, MinimumDeposit::get()); - - assert_eq!(LM::pool(0).unwrap().deposit, MinimumDeposit::get()); }); } @@ -1010,15 +1044,15 @@ fn redeem_from_pool_retired_should_work() { // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); - assert_ok!(LM::deposit(Some(USER_1).into(), 0, UNIT)); - assert_ok!(LM::deposit(Some(USER_2).into(), 0, UNIT)); + assert_ok!(LM::deposit(Some(USER_1).into(), 0, DEPOSIT_AMOUNT)); + assert_ok!(LM::deposit(Some(USER_2).into(), 0, DEPOSIT_AMOUNT)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, UNIT); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).reserved, 0); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, UNIT); + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).reserved, 0); run_to_block(DAYS); @@ -1028,7 +1062,8 @@ fn redeem_from_pool_retired_should_work() { let rewarded = (pbpd * (DAYS as Balance * UNIT).into()).into_inner() / FixedU128::accuracy(); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); + let whatever: Balance = 13; + assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(whatever))); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); @@ -1041,7 +1076,7 @@ fn redeem_from_pool_retired_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); assert!(LM::user_deposit_data(0, USER_1).is_none()); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, 0); @@ -1081,15 +1116,15 @@ fn double_redeem_from_pool_in_diff_state_should_work() { // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); - assert_ok!(LM::deposit(Some(USER_1).into(), 0, UNIT)); - assert_ok!(LM::deposit(Some(USER_2).into(), 0, UNIT)); + assert_ok!(LM::deposit(Some(USER_1).into(), 0, DEPOSIT_AMOUNT)); + assert_ok!(LM::deposit(Some(USER_2).into(), 0, DEPOSIT_AMOUNT)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, UNIT); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).reserved, 0); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, UNIT); + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).reserved, 0); run_to_block(100); @@ -1098,10 +1133,13 @@ fn double_redeem_from_pool_in_diff_state_should_work() { let pbpd = FixedU128::from((per_block, 2 * UNIT)); let old_rewarded = (pbpd * (100 * UNIT).into()).into_inner() / FixedU128::accuracy(); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); + let redeemed = DEPOSIT_AMOUNT / 2; + let deposit_left = DEPOSIT_AMOUNT - redeemed; + + assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(redeemed))); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, deposit_left); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).reserved, 0); assert_eq!(Tokens::accounts(USER_1, REWARD_1).free, old_rewarded); assert_eq!(Tokens::accounts(USER_1, REWARD_1).frozen, 0); @@ -1109,12 +1147,12 @@ fn double_redeem_from_pool_in_diff_state_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).free, old_rewarded); assert_eq!(Tokens::accounts(USER_1, REWARD_2).frozen, 0); assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); - assert!(LM::user_deposit_data(0, USER_1).is_none()); + assert_eq!(LM::user_deposit_data(0, USER_1).unwrap().deposit, deposit_left); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, MinimumDeposit::get()); + assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, 0); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).reserved, 0); assert_eq!(Tokens::accounts(USER_2, REWARD_1).free, old_rewarded); assert_eq!(Tokens::accounts(USER_2, REWARD_1).frozen, 0); @@ -1122,29 +1160,29 @@ fn double_redeem_from_pool_in_diff_state_should_work() { assert_eq!(Tokens::accounts(USER_2, REWARD_2).free, old_rewarded); assert_eq!(Tokens::accounts(USER_2, REWARD_2).frozen, 0); assert_eq!(Tokens::accounts(USER_2, REWARD_2).reserved, 0); - assert_eq!(LM::user_deposit_data(0, USER_2).unwrap().deposit, MinimumDeposit::get()); + assert!(LM::user_deposit_data(0, USER_2).is_none()); - assert_eq!(LM::pool(0).unwrap().deposit, MinimumDeposit::get()); + assert_eq!(LM::pool(0).unwrap().deposit, deposit_left); - // USER_2 didn't remember to redeem until the seventh day + // USER_1 didn't remember to redeem until the seventh day run_to_block(7 * DAYS); - let pbpd = FixedU128::from((per_block, MinimumDeposit::get())); - let new_rewarded = (pbpd * ((DAYS - 100) as Balance * MinimumDeposit::get()).into()) - .into_inner() / + let pbpd = FixedU128::from((per_block, deposit_left)); + let new_rewarded = (pbpd * ((DAYS - 100) as Balance * deposit_left).into()).into_inner() / FixedU128::accuracy(); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + let whatever: Balance = 13; + assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(whatever))); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, 0); - assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).reserved, 0); - assert_eq!(Tokens::accounts(USER_2, REWARD_1).free, old_rewarded + new_rewarded); - assert_eq!(Tokens::accounts(USER_2, REWARD_1).frozen, 0); - assert_eq!(Tokens::accounts(USER_2, REWARD_1).reserved, 0); - assert_eq!(Tokens::accounts(USER_2, REWARD_2).free, old_rewarded + new_rewarded); - assert_eq!(Tokens::accounts(USER_2, REWARD_2).frozen, 0); - assert_eq!(Tokens::accounts(USER_2, REWARD_2).reserved, 0); - assert!(LM::user_deposit_data(0, USER_2).is_none()); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); + assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).reserved, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_1).free, old_rewarded + new_rewarded); + assert_eq!(Tokens::accounts(USER_1, REWARD_1).frozen, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_1).reserved, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_2).free, old_rewarded + new_rewarded); + assert_eq!(Tokens::accounts(USER_1, REWARD_2).frozen, 0); + assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); + assert!(LM::user_deposit_data(0, USER_1).is_none()); assert!(LM::pool(0).is_none()); @@ -1166,8 +1204,8 @@ fn double_redeem_from_pool_in_diff_state_should_work() { #[test] fn redeem_with_wrong_origin_should_fail() { new_test_ext().execute_with(|| { - assert_noop!(LM::redeem(Origin::root(), 0), DispatchError::BadOrigin); - assert_noop!(LM::redeem(Origin::none(), 0), DispatchError::BadOrigin); + assert_noop!(LM::redeem(Origin::root(), 0, None), DispatchError::BadOrigin); + assert_noop!(LM::redeem(Origin::none(), 0, None), DispatchError::BadOrigin); }); } @@ -1191,7 +1229,7 @@ fn redeem_with_wrong_pid_should_fail() { run_to_block(100); - assert_noop!(LM::redeem(Some(USER_1).into(), 1), Error::::InvalidPoolId); + assert_noop!(LM::redeem(Some(USER_1).into(), 1, None), Error::::InvalidPoolId); }); } @@ -1208,13 +1246,13 @@ fn redeem_with_wrong_state_should_fail() { 0 )); - assert_noop!(LM::redeem(Some(USER_1).into(), 0), Error::::InvalidPoolState); + assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); assert_ok!(LM::deposit(Some(USER_1).into(), 0, UNIT)); - assert_noop!(LM::redeem(Some(USER_1).into(), 0), Error::::InvalidPoolState); + assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); }); } @@ -1231,7 +1269,7 @@ fn redeem_without_deposit_should_fail() { 0 )); - assert_noop!(LM::redeem(Some(USER_1).into(), 0), Error::::InvalidPoolState); + assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); @@ -1240,8 +1278,8 @@ fn redeem_without_deposit_should_fail() { run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); - assert_noop!(LM::redeem(Some(USER_1).into(), 0), Error::::NoDepositOfUser); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::NoDepositOfUser); }); } @@ -1258,7 +1296,7 @@ fn redeem_all_deposit_from_pool_ongoing_should_fail() { 0 )); - assert_noop!(LM::redeem(Some(USER_1).into(), 0), Error::::InvalidPoolState); + assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); @@ -1266,8 +1304,8 @@ fn redeem_all_deposit_from_pool_ongoing_should_fail() { run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); - let result = LM::redeem(Some(USER_1).into(), 0); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + let result = LM::redeem(Some(USER_1).into(), 0, None); assert_noop!(result, Error::::TooLowDepositInPoolToRedeem); }); } @@ -1625,8 +1663,8 @@ fn force_retire_pool_charged_should_work() { assert_noop!(LM::claim(Some(USER_1).into(), 0), Error::::InvalidPoolState); assert_noop!(LM::claim(Some(USER_2).into(), 0), Error::::InvalidPoolState); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); assert_eq!(Tokens::accounts(USER_1, REWARD_1).free, 0); assert_eq!(Tokens::accounts(USER_1, REWARD_1).frozen, 0); @@ -1724,8 +1762,8 @@ fn force_retire_pool_ongoing_should_work() { assert_noop!(LM::claim(Some(USER_1).into(), 0), Error::::InvalidPoolState); assert_noop!(LM::claim(Some(USER_2).into(), 0), Error::::InvalidPoolState); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); let pbpd_1 = FixedU128::from((PER_BLOCK, UNIT)); let reward_step_1 = (pbpd_1 * (100 * UNIT).into()).into_inner() / FixedU128::accuracy(); @@ -1932,7 +1970,7 @@ fn redeem_from_eb_farming_should_work() { run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); let pbpd = FixedU128::from((PER_BLOCK, DEPOSIT_AMOUNT)); let reward_to_user_1 = @@ -1949,7 +1987,7 @@ fn redeem_from_eb_farming_should_work() { run_to_block(DAYS); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); let reward_to_user_2 = (pbpd * ((DAYS - 100) as Balance * DEPOSIT_AMOUNT).into()) .into_inner() / @@ -2017,7 +2055,7 @@ fn claim_from_eb_farming_should_work() { let result = LM::claim(Some(USER_1).into(), 0); assert_noop!(result, Error::::InvalidPoolState); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); let reward_to_user_2 = (pbpd * ((DAYS - 100) as Balance * DEPOSIT_AMOUNT).into()) .into_inner() / @@ -2111,8 +2149,8 @@ fn simple_integration_test() { run_to_block(DAYS); - assert_ok!(LM::redeem(Some(USER_1).into(), 0)); - assert_ok!(LM::redeem(Some(USER_2).into(), 0)); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); let reward_step_3 = (pbpd_2 * ((DAYS - 200) as Balance * UNIT).into()).into_inner() / FixedU128::accuracy(); From 55511903fe4bc9221f6a7b37c763c6ba897c5cfa Mon Sep 17 00:00:00 2001 From: Allen Pocket Date: Fri, 24 Sep 2021 11:27:31 +0800 Subject: [PATCH 3/6] :bug: ($PALLET) Fix benchmarks after modification --- pallets/liquidity-mining/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/liquidity-mining/src/benchmarking.rs b/pallets/liquidity-mining/src/benchmarking.rs index 6dbaf0329d..e560d2c05d 100644 --- a/pallets/liquidity-mining/src/benchmarking.rs +++ b/pallets/liquidity-mining/src/benchmarking.rs @@ -122,7 +122,7 @@ benchmarks! { // Run to block run_to_block::(duration); - }: _(RawOrigin::Signed(caller.clone()), 0) + }: _(RawOrigin::Signed(caller.clone()), 0, None) verify { let pool = LM::::pool(0); let deposit_data = LM::::user_deposit_data(0, caller.clone()); From 960b011408896c26a30fee5d8438c9ee65a6c41d Mon Sep 17 00:00:00 2001 From: Allen Pocket Date: Fri, 24 Sep 2021 15:36:23 +0800 Subject: [PATCH 4/6] :sparkles: ($PALLET) Add redeem_all --- pallets/liquidity-mining/src/lib.rs | 251 ++++++++++++++++------------ 1 file changed, 141 insertions(+), 110 deletions(-) diff --git a/pallets/liquidity-mining/src/lib.rs b/pallets/liquidity-mining/src/lib.rs index c07dc71019..fc62983e8e 100644 --- a/pallets/liquidity-mining/src/lib.rs +++ b/pallets/liquidity-mining/src/lib.rs @@ -326,8 +326,6 @@ type PoolId = u32; #[frame_support::pallet] pub mod pallet { - use frame_system::RawOrigin; - use super::*; #[pallet::config] @@ -399,8 +397,8 @@ pub mod pallet { NoDepositOfUser, /// Too low balance to deposit TooLowToDeposit, - /// The deposit in liquidity-pool ongoing should be greater than `T::MinimumDeposit` - TooLowDepositInPoolToRedeem, + /// User doesnt have enough deposit to redeem + TooLowToRedeem, /// The interval between two claims is short TooShortBetweenTwoClaim, /// The pool has been charged @@ -513,9 +511,7 @@ pub mod pallet { duration, min_deposit_to_start, after_block_to_start, - )?; - - Ok(().into()) + ) } #[pallet::weight(( @@ -547,9 +543,7 @@ pub mod pallet { duration, min_deposit_to_start, after_block_to_start, - )?; - - Ok(().into()) + ) } #[pallet::weight(( @@ -581,9 +575,7 @@ pub mod pallet { duration, min_deposit_to_start, after_block_to_start, - )?; - - Ok(().into()) + ) } #[pallet::weight(T::WeightInfo::charge())] @@ -776,7 +768,7 @@ pub mod pallet { Ok(().into()) } - /// User redeems all deposit from a liquidity-pool. + /// User redeems some deposit from a liquidity-pool. /// The deposit in the liquidity-pool should be greater than `T::MinimumDeposit` when the /// liquidity-pool is on `Ongoing` state; So user may not be able to redeem completely /// until the liquidity-pool is on `Retire` state. @@ -793,106 +785,46 @@ pub mod pallet { /// - User should have some deposit in the liquidity-pool; /// - The liquidity-pool should be in special state: `Ongoing`, `Retired`; /// - /// NOTE: All deposit will be redeemed when the pool is being retired, no matter the `value` - /// is. + /// NOTE: All deposit will be redeemed when the pool is being `Retired`, no matter the + /// `value` is. #[transactional] #[pallet::weight(T::WeightInfo::redeem())] pub fn redeem( origin: OriginFor, pid: PoolId, - value: Option>, + value: BalanceOf, ) -> DispatchResultWithPostInfo { - let user = ensure_signed(origin)?; - - let mut pool: PoolInfo = - Self::pool(pid).ok_or(Error::::InvalidPoolId)?.try_retire().try_update(); - - ensure!( - pool.state == PoolState::Ongoing || pool.state == PoolState::Retired, - Error::::InvalidPoolState - ); - - let mut deposit_data: DepositData = - Self::user_deposit_data(pid, user.clone()).ok_or(Error::::NoDepositOfUser)?; - - if pool.update_b != deposit_data.update_b { - pool.try_settle_and_transfer(&mut deposit_data, user.clone())?; + if value == Zero::zero() { + return Ok(().into()); } - // Keep minimum deposit in pool when the pool is ongoing. - let minimum_in_pool = match pool.state { - PoolState::Ongoing => T::MinimumDepositOfUser::get(), - PoolState::Retired => Zero::zero(), - _ => return Err(Error::::Unexpected.into()), - }; - - let try_redeemed = match pool.state { - PoolState::Ongoing => - min(value.unwrap_or(deposit_data.deposit), deposit_data.deposit), - PoolState::Retired => deposit_data.deposit, - _ => return Err(Error::::Unexpected.into()), - }; - - let left_in_pool = max(pool.deposit.saturating_sub(try_redeemed), minimum_in_pool); - let can_redeemed = pool.deposit.saturating_sub(left_in_pool); - let left_in_user = deposit_data.deposit.saturating_sub(can_redeemed); - - ensure!(can_redeemed != Zero::zero(), Error::::TooLowDepositInPoolToRedeem); - - // To unlock the deposit - match pool.r#type { - PoolType::Mining => { - let lpt = Self::convert_to_lptoken(pool.trading_pair)?; - match left_in_user.saturated_into() { - 0u128 => T::MultiCurrency::remove_lock(DEPOSIT_ID, lpt, &user)?, - _ => T::MultiCurrency::set_lock(DEPOSIT_ID, lpt, &user, left_in_user)?, - } - }, - PoolType::Farming => { - let (token_a, token_b) = pool.trading_pair; - match left_in_user.saturated_into() { - 0u128 => { - T::MultiCurrency::remove_lock(DEPOSIT_ID, token_a, &user)?; - T::MultiCurrency::remove_lock(DEPOSIT_ID, token_b, &user)?; - }, - _ => { - T::MultiCurrency::set_lock(DEPOSIT_ID, token_a, &user, left_in_user)?; - T::MultiCurrency::set_lock(DEPOSIT_ID, token_b, &user, left_in_user)?; - }, - } - }, - PoolType::EBFarming => {}, - }; - - deposit_data.deposit = left_in_user; - pool.deposit = left_in_pool; - - if pool.state == PoolState::Retired && pool.deposit == Zero::zero() { - let investor = pool.investor.clone().ok_or(Error::::Unexpected)?; - for (rtoken, reward) in pool.rewards.iter() { - let remain = reward.total.saturating_sub(reward.claimed); - T::MultiCurrency::transfer(*rtoken, &pool.keeper, &investor, remain)?; - } - - pool.state = PoolState::Dead; - } - - let r#type = pool.r#type; - let trading_pair = pool.trading_pair; - - match pool.deposit.saturated_into() { - 0u128 => TotalPoolInfos::::remove(pid), - _ => TotalPoolInfos::::insert(pid, pool), - } + let user = ensure_signed(origin)?; - match deposit_data.deposit.saturated_into() { - 0u128 => TotalDepositData::::remove(pid, user.clone()), - _ => TotalDepositData::::insert(pid, user.clone(), deposit_data), - } + Self::redeem_inner(user, pid, Some(value)) + } - Self::deposit_event(Event::UserRedeemed(pid, r#type, trading_pair, try_redeemed, user)); + /// User redeems all deposit from a liquidity-pool. + /// The deposit in the liquidity-pool should be greater than `T::MinimumDeposit` when the + /// liquidity-pool is on `Ongoing` state; So user may not be able to redeem completely + /// until the liquidity-pool is on `Retire` state. + /// + /// The extrinsic will: + /// - Try to retire the liquidity-pool which has reached the end of life. + /// - Try to settle the rewards. + /// - Try to unreserve the remaining rewards to the pool investor when the deposit in the + /// liquidity-pool is clear. + /// - Try to delete the liquidity-pool in which the deposit becomes zero. + /// - Try to delete the deposit-data in which the deposit becomes zero. + /// + /// The condition to redeem: + /// - User should have some deposit in the liquidity-pool; + /// - The liquidity-pool should be in special state: `Ongoing`, `Retired`; + #[transactional] + #[pallet::weight(1_000)] + pub fn redeem_all(origin: OriginFor, pid: PoolId) -> DispatchResultWithPostInfo { + let user = ensure_signed(origin)?; - Ok(().into()) + Self::redeem_inner(user, pid, None) } /// Help someone to redeem the deposit whose deposited in a liquidity-pool. @@ -909,20 +841,18 @@ pub mod pallet { ensure!(pool.state == PoolState::Retired, Error::::InvalidPoolState); - let origin = match account { - Some(account) => RawOrigin::Signed(account).into(), + let user = match account { + Some(account) => account, None => { let (account, _) = TotalDepositData::::iter_prefix(pid) .next() .ok_or(Error::::NoDepositOfUser)?; - RawOrigin::Signed(account).into() + account }, }; - Self::redeem(origin, pid, None)?; - - Ok(().into()) + Self::redeem_inner(user, pid, None) } /// User claims the rewards from a liquidity-pool. @@ -965,7 +895,7 @@ pub mod pallet { duration: BlockNumberFor, min_deposit_to_start: BalanceOf, after_block_to_start: BlockNumberFor, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { // Check the trading-pair ensure!(trading_pair.0 != trading_pair.1, Error::::InvalidTradingPair); @@ -1024,6 +954,107 @@ pub mod pallet { Ok(().into()) } + pub(crate) fn redeem_inner( + user: AccountIdOf, + pid: PoolId, + value: Option>, + ) -> DispatchResultWithPostInfo { + let mut pool: PoolInfo = + Self::pool(pid).ok_or(Error::::InvalidPoolId)?.try_retire().try_update(); + + ensure!( + pool.state == PoolState::Ongoing || pool.state == PoolState::Retired, + Error::::InvalidPoolState + ); + + let mut deposit_data: DepositData = + Self::user_deposit_data(pid, user.clone()).ok_or(Error::::NoDepositOfUser)?; + + if pool.update_b != deposit_data.update_b { + pool.try_settle_and_transfer(&mut deposit_data, user.clone())?; + } + + // Keep minimum deposit in pool when the pool is ongoing. + let minimum_in_pool = match pool.state { + PoolState::Ongoing => T::MinimumDepositOfUser::get(), + PoolState::Retired => Zero::zero(), + _ => return Err(Error::::Unexpected.into()), + }; + + let pool_can_redeem = pool.deposit.saturating_sub(minimum_in_pool); + let user_can_redeem = min(deposit_data.deposit, pool_can_redeem); + + let try_redeem = match value { + Some(value) if pool.state == PoolState::Ongoing => value, + Some(_) if pool.state == PoolState::Retired => user_can_redeem, + None => user_can_redeem, + _ => return Err(Error::::Unexpected.into()), + }; + + ensure!( + user_can_redeem >= try_redeem && user_can_redeem != Zero::zero(), + Error::::TooLowToRedeem + ); + + let left_in_pool = pool.deposit.saturating_sub(try_redeem); + let left_in_user = deposit_data.deposit.saturating_sub(try_redeem); + + // To unlock the deposit + match pool.r#type { + PoolType::Mining => { + let lpt = Self::convert_to_lptoken(pool.trading_pair)?; + match left_in_user.saturated_into() { + 0u128 => T::MultiCurrency::remove_lock(DEPOSIT_ID, lpt, &user)?, + _ => T::MultiCurrency::set_lock(DEPOSIT_ID, lpt, &user, left_in_user)?, + } + }, + PoolType::Farming => { + let (token_a, token_b) = pool.trading_pair; + match left_in_user.saturated_into() { + 0u128 => { + T::MultiCurrency::remove_lock(DEPOSIT_ID, token_a, &user)?; + T::MultiCurrency::remove_lock(DEPOSIT_ID, token_b, &user)?; + }, + _ => { + T::MultiCurrency::set_lock(DEPOSIT_ID, token_a, &user, left_in_user)?; + T::MultiCurrency::set_lock(DEPOSIT_ID, token_b, &user, left_in_user)?; + }, + } + }, + PoolType::EBFarming => {}, + }; + + deposit_data.deposit = left_in_user; + pool.deposit = left_in_pool; + + if pool.state == PoolState::Retired && pool.deposit == Zero::zero() { + let investor = pool.investor.clone().ok_or(Error::::Unexpected)?; + for (rtoken, reward) in pool.rewards.iter() { + let remain = reward.total.saturating_sub(reward.claimed); + T::MultiCurrency::transfer(*rtoken, &pool.keeper, &investor, remain)?; + } + + pool.state = PoolState::Dead; + } + + let r#type = pool.r#type; + let trading_pair = pool.trading_pair; + + match pool.deposit.saturated_into() { + 0u128 => TotalPoolInfos::::remove(pid), + _ => TotalPoolInfos::::insert(pid, pool), + } + + match deposit_data.deposit.saturated_into() { + 0u128 => TotalDepositData::::remove(pid, user.clone()), + _ => TotalDepositData::::insert(pid, user.clone(), deposit_data), + } + + Self::deposit_event(Event::UserRedeemed(pid, r#type, trading_pair, try_redeem, user)); + + Ok(().into()) + } + pub(crate) fn next_pool_id() -> PoolId { let next_pool_id = Self::pool_id(); NextPoolId::::mutate(|current| *current = current.saturating_add(1)); From d2ddc249cdf4d1c8ee2d826c146697e6ef5b18cf Mon Sep 17 00:00:00 2001 From: Allen Pocket Date: Fri, 24 Sep 2021 15:36:42 +0800 Subject: [PATCH 5/6] :white_check_mark: ($PALLET) Cover unit-tests --- pallets/liquidity-mining/src/tests.rs | 91 +++++++++++++++++---------- 1 file changed, 59 insertions(+), 32 deletions(-) diff --git a/pallets/liquidity-mining/src/tests.rs b/pallets/liquidity-mining/src/tests.rs index 82fbf1e253..404fd814f4 100644 --- a/pallets/liquidity-mining/src/tests.rs +++ b/pallets/liquidity-mining/src/tests.rs @@ -967,7 +967,7 @@ fn redeem_from_pool_ongoing_should_work() { let redeemed = DEPOSIT_AMOUNT / 2; let deposit_left = DEPOSIT_AMOUNT - redeemed; - assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(redeemed))); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, redeemed)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, deposit_left); @@ -980,7 +980,7 @@ fn redeem_from_pool_ongoing_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); assert_eq!(LM::user_deposit_data(0, USER_1).unwrap().deposit, deposit_left); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, Some(redeemed))); + assert_ok!(LM::redeem(Some(USER_2).into(), 0, redeemed)); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, deposit_left); @@ -1000,7 +1000,7 @@ fn redeem_from_pool_ongoing_should_work() { let pbpd = FixedU128::from((per_block, 2 * deposit_left)); let rewarded = (pbpd * (100 * deposit_left).into()).into_inner() / FixedU128::accuracy(); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); @@ -1013,7 +1013,7 @@ fn redeem_from_pool_ongoing_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); assert!(LM::user_deposit_data(0, USER_1).is_none()); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, Some(Balance::MAX))); + assert_ok!(LM::redeem_all(Some(USER_2).into(), 0)); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, MinimumDeposit::get()); @@ -1062,8 +1062,8 @@ fn redeem_from_pool_retired_should_work() { let rewarded = (pbpd * (DAYS as Balance * UNIT).into()).into_inner() / FixedU128::accuracy(); - let whatever: Balance = 13; - assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(whatever))); + let minimum: Balance = 1; + assert_ok!(LM::redeem(Some(USER_1).into(), 0, minimum)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); @@ -1076,7 +1076,7 @@ fn redeem_from_pool_retired_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); assert!(LM::user_deposit_data(0, USER_1).is_none()); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_2).into(), 0)); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, 0); @@ -1136,7 +1136,7 @@ fn double_redeem_from_pool_in_diff_state_should_work() { let redeemed = DEPOSIT_AMOUNT / 2; let deposit_left = DEPOSIT_AMOUNT - redeemed; - assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(redeemed))); + assert_ok!(LM::redeem(Some(USER_1).into(), 0, redeemed)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, deposit_left); @@ -1149,7 +1149,7 @@ fn double_redeem_from_pool_in_diff_state_should_work() { assert_eq!(Tokens::accounts(USER_1, REWARD_2).reserved, 0); assert_eq!(LM::user_deposit_data(0, USER_1).unwrap().deposit, deposit_left); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_2).into(), 0)); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_2, MINING_DEPOSIT).frozen, 0); @@ -1170,8 +1170,8 @@ fn double_redeem_from_pool_in_diff_state_should_work() { let new_rewarded = (pbpd * ((DAYS - 100) as Balance * deposit_left).into()).into_inner() / FixedU128::accuracy(); - let whatever: Balance = 13; - assert_ok!(LM::redeem(Some(USER_1).into(), 0, Some(whatever))); + let minimum: Balance = 1; + assert_ok!(LM::redeem(Some(USER_1).into(), 0, minimum)); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).free, DEPOSIT_AMOUNT); assert_eq!(Tokens::accounts(USER_1, MINING_DEPOSIT).frozen, 0); @@ -1204,8 +1204,8 @@ fn double_redeem_from_pool_in_diff_state_should_work() { #[test] fn redeem_with_wrong_origin_should_fail() { new_test_ext().execute_with(|| { - assert_noop!(LM::redeem(Origin::root(), 0, None), DispatchError::BadOrigin); - assert_noop!(LM::redeem(Origin::none(), 0, None), DispatchError::BadOrigin); + assert_noop!(LM::redeem_all(Origin::root(), 0), DispatchError::BadOrigin); + assert_noop!(LM::redeem_all(Origin::none(), 0), DispatchError::BadOrigin); }); } @@ -1229,7 +1229,7 @@ fn redeem_with_wrong_pid_should_fail() { run_to_block(100); - assert_noop!(LM::redeem(Some(USER_1).into(), 1, None), Error::::InvalidPoolId); + assert_noop!(LM::redeem_all(Some(USER_1).into(), 1), Error::::InvalidPoolId); }); } @@ -1246,13 +1246,13 @@ fn redeem_with_wrong_state_should_fail() { 0 )); - assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); + assert_noop!(LM::redeem_all(Some(USER_1).into(), 0), Error::::InvalidPoolState); // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); assert_ok!(LM::deposit(Some(USER_1).into(), 0, UNIT)); - assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); + assert_noop!(LM::redeem_all(Some(USER_1).into(), 0), Error::::InvalidPoolState); }); } @@ -1269,7 +1269,7 @@ fn redeem_without_deposit_should_fail() { 0 )); - assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); + assert_noop!(LM::redeem_all(Some(USER_1).into(), 0), Error::::InvalidPoolState); // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); @@ -1278,8 +1278,8 @@ fn redeem_without_deposit_should_fail() { run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); - assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::NoDepositOfUser); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); + assert_noop!(LM::redeem_all(Some(USER_1).into(), 0), Error::::NoDepositOfUser); }); } @@ -1296,7 +1296,7 @@ fn redeem_all_deposit_from_pool_ongoing_should_fail() { 0 )); - assert_noop!(LM::redeem(Some(USER_1).into(), 0, None), Error::::InvalidPoolState); + assert_noop!(LM::redeem_all(Some(USER_1).into(), 0), Error::::InvalidPoolState); // It is unable to call Collective::execute(..) which is private; assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); @@ -1304,9 +1304,36 @@ fn redeem_all_deposit_from_pool_ongoing_should_fail() { run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); - let result = LM::redeem(Some(USER_1).into(), 0, None); - assert_noop!(result, Error::::TooLowDepositInPoolToRedeem); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); + let result = LM::redeem_all(Some(USER_1).into(), 0); + assert_noop!(result, Error::::TooLowToRedeem); + }); +} + +#[test] +fn redeem_some_more_than_user_can_redeem_should_fail() { + new_test_ext().execute_with(|| { + assert_ok!(LM::create_mining_pool( + pallet_collective::RawOrigin::Member(TC_MEMBER_1).into(), + MINING_TRADING_PAIR, + (REWARD_1, REWARD_AMOUNT), + vec![(REWARD_2, REWARD_AMOUNT)].try_into().unwrap(), + DAYS, + 1 * UNIT, + 0 + )); + + // It is unable to call Collective::execute(..) which is private; + assert_ok!(LM::charge(Some(INVESTOR).into(), 0)); + + assert_ok!(LM::deposit(Some(USER_1).into(), 0, UNIT)); + + run_to_block(100); + + assert_noop!( + LM::redeem(Some(USER_1).into(), 0, UNIT - MinimumDeposit::get() + 1), + Error::::TooLowToRedeem + ); }); } @@ -1663,8 +1690,8 @@ fn force_retire_pool_charged_should_work() { assert_noop!(LM::claim(Some(USER_1).into(), 0), Error::::InvalidPoolState); assert_noop!(LM::claim(Some(USER_2).into(), 0), Error::::InvalidPoolState); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); + assert_ok!(LM::redeem_all(Some(USER_2).into(), 0)); assert_eq!(Tokens::accounts(USER_1, REWARD_1).free, 0); assert_eq!(Tokens::accounts(USER_1, REWARD_1).frozen, 0); @@ -1762,8 +1789,8 @@ fn force_retire_pool_ongoing_should_work() { assert_noop!(LM::claim(Some(USER_1).into(), 0), Error::::InvalidPoolState); assert_noop!(LM::claim(Some(USER_2).into(), 0), Error::::InvalidPoolState); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); + assert_ok!(LM::redeem_all(Some(USER_2).into(), 0)); let pbpd_1 = FixedU128::from((PER_BLOCK, UNIT)); let reward_step_1 = (pbpd_1 * (100 * UNIT).into()).into_inner() / FixedU128::accuracy(); @@ -1970,7 +1997,7 @@ fn redeem_from_eb_farming_should_work() { run_to_block(100); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); let pbpd = FixedU128::from((PER_BLOCK, DEPOSIT_AMOUNT)); let reward_to_user_1 = @@ -1987,7 +2014,7 @@ fn redeem_from_eb_farming_should_work() { run_to_block(DAYS); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); let reward_to_user_2 = (pbpd * ((DAYS - 100) as Balance * DEPOSIT_AMOUNT).into()) .into_inner() / @@ -2055,7 +2082,7 @@ fn claim_from_eb_farming_should_work() { let result = LM::claim(Some(USER_1).into(), 0); assert_noop!(result, Error::::InvalidPoolState); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); let reward_to_user_2 = (pbpd * ((DAYS - 100) as Balance * DEPOSIT_AMOUNT).into()) .into_inner() / @@ -2149,8 +2176,8 @@ fn simple_integration_test() { run_to_block(DAYS); - assert_ok!(LM::redeem(Some(USER_1).into(), 0, None)); - assert_ok!(LM::redeem(Some(USER_2).into(), 0, None)); + assert_ok!(LM::redeem_all(Some(USER_1).into(), 0)); + assert_ok!(LM::redeem_all(Some(USER_2).into(), 0)); let reward_step_3 = (pbpd_2 * ((DAYS - 200) as Balance * UNIT).into()).into_inner() / FixedU128::accuracy(); From 446f821e68f0310849d0edbe7f3f98839343948f Mon Sep 17 00:00:00 2001 From: Allen Pocket Date: Fri, 24 Sep 2021 16:01:00 +0800 Subject: [PATCH 6/6] :bug: ($PALLET) Fix benchmark and weights --- pallets/liquidity-mining/src/benchmarking.rs | 112 ++++++++++++------ pallets/liquidity-mining/src/lib.rs | 2 +- pallets/liquidity-mining/src/weights.rs | 5 + .../src/weights/bifrost_liquidity_mining.rs | 18 ++- 4 files changed, 97 insertions(+), 40 deletions(-) diff --git a/pallets/liquidity-mining/src/benchmarking.rs b/pallets/liquidity-mining/src/benchmarking.rs index e560d2c05d..5894271157 100644 --- a/pallets/liquidity-mining/src/benchmarking.rs +++ b/pallets/liquidity-mining/src/benchmarking.rs @@ -49,15 +49,15 @@ benchmarks! { let duration = T::MinimumDuration::get().saturating_add(1u128.saturated_into()); let min_deposit_to_start = T::MinimumDepositOfUser::get(); - let reward_amount: BalanceOf = UNIT.saturated_into(); + let amount: BalanceOf = UNIT.saturated_into(); - assert_ok!(::MultiCurrency::deposit(REWARD_1, &caller, reward_amount)); - assert_ok!(::MultiCurrency::deposit(REWARD_2, &caller, reward_amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_1, &caller, amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_2, &caller, amount)); assert_ok!(LM::::create_pool( (FARMING_DEPOSIT_1, FARMING_DEPOSIT_2), - (REWARD_1, reward_amount), - vec![(REWARD_2, reward_amount)].try_into().unwrap(), + (REWARD_1, amount), + vec![(REWARD_2, amount)].try_into().unwrap(), PoolType::Farming, duration, min_deposit_to_start, @@ -68,20 +68,20 @@ benchmarks! { deposit { let duration = T::MinimumDuration::get().saturating_add(1u128.saturated_into()); let min_deposit_to_start = T::MinimumDepositOfUser::get(); - let reward_amount: BalanceOf = UNIT.saturated_into(); + let amount: BalanceOf = UNIT.saturated_into(); let investor: T::AccountId = account("lm", 0, 0); - assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, reward_amount)); - assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, reward_amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, amount)); let caller: T::AccountId = whitelisted_caller(); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, reward_amount)); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, reward_amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, amount)); assert_ok!(LM::::create_pool( (FARMING_DEPOSIT_1, FARMING_DEPOSIT_2), - (REWARD_1, reward_amount), - vec![(REWARD_2, reward_amount)].try_into().unwrap(), + (REWARD_1, amount), + vec![(REWARD_2, amount)].try_into().unwrap(), PoolType::Farming, duration, min_deposit_to_start, @@ -95,20 +95,20 @@ benchmarks! { redeem { let duration = T::MinimumDuration::get().saturating_add(1u128.saturated_into()); let min_deposit_to_start = T::MinimumDepositOfUser::get(); - let reward_amount: BalanceOf = UNIT.saturated_into(); + let amount: BalanceOf = UNIT.saturated_into(); let investor: T::AccountId = account("lm", 0, 0); - assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, reward_amount)); - assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, reward_amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, amount)); let caller: T::AccountId = whitelisted_caller(); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, reward_amount)); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, reward_amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, amount)); assert_ok!(LM::::create_pool( (FARMING_DEPOSIT_1, FARMING_DEPOSIT_2), - (REWARD_1, reward_amount), - vec![(REWARD_2, reward_amount)].try_into().unwrap(), + (REWARD_1, amount), + vec![(REWARD_2, amount)].try_into().unwrap(), PoolType::Farming, duration, min_deposit_to_start, @@ -117,12 +117,50 @@ benchmarks! { assert_ok!(LM::::charge(RawOrigin::Signed(investor).into(), 0)); - assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, reward_amount)); + assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, amount)); // Run to block run_to_block::(duration); - }: _(RawOrigin::Signed(caller.clone()), 0, None) + }: _(RawOrigin::Signed(caller.clone()), 0, amount) + verify { + let pool = LM::::pool(0); + let deposit_data = LM::::user_deposit_data(0, caller.clone()); + assert!(pool.is_none()); + assert!(deposit_data.is_none()); + } + + redeem_all { + let duration = T::MinimumDuration::get().saturating_add(1u128.saturated_into()); + let min_deposit_to_start = T::MinimumDepositOfUser::get(); + let amount: BalanceOf = UNIT.saturated_into(); + + let investor: T::AccountId = account("lm", 0, 0); + assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, amount)); + + let caller: T::AccountId = whitelisted_caller(); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, amount)); + + assert_ok!(LM::::create_pool( + (FARMING_DEPOSIT_1, FARMING_DEPOSIT_2), + (REWARD_1, amount), + vec![(REWARD_2, amount)].try_into().unwrap(), + PoolType::Farming, + duration, + min_deposit_to_start, + 0u128.saturated_into() + )); + + assert_ok!(LM::::charge(RawOrigin::Signed(investor).into(), 0)); + + assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, amount)); + + // Run to block + run_to_block::(duration); + + }: _(RawOrigin::Signed(caller.clone()), 0) verify { let pool = LM::::pool(0); let deposit_data = LM::::user_deposit_data(0, caller.clone()); @@ -133,20 +171,20 @@ benchmarks! { volunteer_to_redeem { let duration = T::MinimumDuration::get().saturating_add(1u128.saturated_into()); let min_deposit_to_start = T::MinimumDepositOfUser::get(); - let reward_amount: BalanceOf = UNIT.saturated_into(); + let amount: BalanceOf = UNIT.saturated_into(); let investor: T::AccountId = account("lm", 0, 0); - assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, reward_amount)); - assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, reward_amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, amount)); let caller: T::AccountId = whitelisted_caller(); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, reward_amount)); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, reward_amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, amount)); assert_ok!(LM::::create_pool( (FARMING_DEPOSIT_1, FARMING_DEPOSIT_2), - (REWARD_1, reward_amount), - vec![(REWARD_2, reward_amount)].try_into().unwrap(), + (REWARD_1, amount), + vec![(REWARD_2, amount)].try_into().unwrap(), PoolType::Farming, duration, min_deposit_to_start, @@ -155,7 +193,7 @@ benchmarks! { assert_ok!(LM::::charge(RawOrigin::Signed(investor).into(), 0)); - assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, reward_amount)); + assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, amount)); // Run to block run_to_block::(duration); @@ -173,20 +211,20 @@ benchmarks! { claim { let duration = T::MinimumDuration::get().saturating_add(1u128.saturated_into()); let min_deposit_to_start = T::MinimumDepositOfUser::get(); - let reward_amount: BalanceOf = UNIT.saturated_into(); + let amount: BalanceOf = UNIT.saturated_into(); let investor: T::AccountId = account("lm", 0, 0); - assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, reward_amount)); - assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, reward_amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_1, &investor, amount)); + assert_ok!(::MultiCurrency::deposit(REWARD_2, &investor, amount)); let caller: T::AccountId = whitelisted_caller(); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, reward_amount)); - assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, reward_amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_1, &caller, amount)); + assert_ok!(::MultiCurrency::deposit(FARMING_DEPOSIT_2, &caller, amount)); assert_ok!(LM::::create_pool( (FARMING_DEPOSIT_1, FARMING_DEPOSIT_2), - (REWARD_1, reward_amount), - vec![(REWARD_2, reward_amount)].try_into().unwrap(), + (REWARD_1, amount), + vec![(REWARD_2, amount)].try_into().unwrap(), PoolType::Farming, duration, min_deposit_to_start, @@ -195,7 +233,7 @@ benchmarks! { assert_ok!(LM::::charge(RawOrigin::Signed(investor).into(), 0)); - assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, reward_amount)); + assert_ok!(LM::::deposit(RawOrigin::Signed(caller.clone()).into(), 0, amount)); // Run to block run_to_block::(1u128.saturated_into()); diff --git a/pallets/liquidity-mining/src/lib.rs b/pallets/liquidity-mining/src/lib.rs index fc62983e8e..920c34c618 100644 --- a/pallets/liquidity-mining/src/lib.rs +++ b/pallets/liquidity-mining/src/lib.rs @@ -820,7 +820,7 @@ pub mod pallet { /// - User should have some deposit in the liquidity-pool; /// - The liquidity-pool should be in special state: `Ongoing`, `Retired`; #[transactional] - #[pallet::weight(1_000)] + #[pallet::weight(T::WeightInfo::redeem_all())] pub fn redeem_all(origin: OriginFor, pid: PoolId) -> DispatchResultWithPostInfo { let user = ensure_signed(origin)?; diff --git a/pallets/liquidity-mining/src/weights.rs b/pallets/liquidity-mining/src/weights.rs index 363226cc5d..cd96dcf5c8 100644 --- a/pallets/liquidity-mining/src/weights.rs +++ b/pallets/liquidity-mining/src/weights.rs @@ -30,6 +30,7 @@ pub trait WeightInfo { fn charge() -> Weight; fn deposit() -> Weight; fn redeem() -> Weight; + fn redeem_all() -> Weight; fn volunteer_to_redeem() -> Weight; fn claim() -> Weight; } @@ -48,6 +49,10 @@ impl WeightInfo for () { (50_000_000 as Weight) } + fn redeem_all() -> Weight { + (50_000_000 as Weight) + } + fn volunteer_to_redeem() -> Weight { (50_000_000 as Weight) } diff --git a/runtime/bifrost/src/weights/bifrost_liquidity_mining.rs b/runtime/bifrost/src/weights/bifrost_liquidity_mining.rs index 2065b80c0c..16fdd81419 100644 --- a/runtime/bifrost/src/weights/bifrost_liquidity_mining.rs +++ b/runtime/bifrost/src/weights/bifrost_liquidity_mining.rs @@ -19,7 +19,7 @@ //! Autogenerated weights for `bifrost_liquidity_mining` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2021-09-24, STEPS: `50`, REPEAT: 1, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2021-09-23, STEPS: `50`, REPEAT: 1, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("bifrost-local"), DB CACHE: 128 // Executed Command: @@ -34,7 +34,7 @@ // --wasm-execution=compiled // --heap-pages=4096 // --header=./HEADER-GPL3 -// --output=./runtime/bifrost/src/weights/bifrost-liquidity-mining.rs +// --output=./runtime/bifrost/src/weights/bifrost_liquidity_mining #![cfg_attr(rustfmt, rustfmt_skip)] @@ -89,6 +89,20 @@ impl bifrost_liquidity_mining::WeightInfo for WeightInf } // Storage: LiquidityMining TotalPoolInfos (r:1 w:1) // Storage: System Number (r:1 w:0) + // Storage: LiquidityMining TotalDepositData (r:1 w:1) + // Storage: System Account (r:1 w:1) + // Storage: Tokens Accounts (r:4 w:4) + // Storage: System ExecutionPhase (r:1 w:0) + // Storage: System EventCount (r:1 w:1) + // Storage: System Events (r:1 w:1) + // Storage: Tokens Locks (r:2 w:2) + fn redeem_all() -> Weight { + (175_000_000 as Weight) + .saturating_add(T::DbWeight::get().reads(13 as Weight)) + .saturating_add(T::DbWeight::get().writes(11 as Weight)) + } + // Storage: LiquidityMining TotalPoolInfos (r:1 w:1) + // Storage: System Number (r:1 w:0) // Storage: LiquidityMining TotalDepositData (r:2 w:1) // Storage: System Account (r:1 w:1) // Storage: Tokens Accounts (r:4 w:4)