From e133a663c652f36f30c22ceb5a535251e9c3583b Mon Sep 17 00:00:00 2001 From: yrong Date: Tue, 9 Nov 2021 17:50:53 +0800 Subject: [PATCH 1/2] Add multisig account for salp lite --- runtime/bifrost/src/lib.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/runtime/bifrost/src/lib.rs b/runtime/bifrost/src/lib.rs index e21ef58990..789cbb2869 100644 --- a/runtime/bifrost/src/lib.rs +++ b/runtime/bifrost/src/lib.rs @@ -1278,6 +1278,7 @@ parameter_types! { pub ContributionWeight:XcmBaseWeight = XCM_WEIGHT.into(); pub AddProxyWeight:XcmBaseWeight = XCM_WEIGHT.into(); pub ConfirmMuitiSigAccount: AccountId = hex!["e4da05f08e89bf6c43260d96f26fffcfc7deae5b465da08669a9d008e64c2c63"].into(); + pub ConfirmSalpLiteConfirmAsMultiSig: AccountId = hex!["8bd7184f3c25082d1c6042685815a17bae2c2f853300b94ed58cfde8fca7a416"].into(); pub RelaychainSovereignSubAccount: MultiLocation = create_x2_multilocation(ParachainDerivedProxyAccountType::Salp as u16); pub SalpTransactType: ParachainTransactType = ParachainTransactType::Xcm; pub SalpProxyType: ParachainTransactProxyType = ParachainTransactProxyType::Derived; @@ -1314,6 +1315,28 @@ impl bifrost_salp::Config for Runtime { type RelayNetwork = RelayNetwork; } +pub struct EnsureSalpLiteConfirmAsMultiSig; +impl EnsureOrigin for EnsureSalpLiteConfirmAsMultiSig { + type Success = AccountId; + + fn try_origin(o: Origin) -> Result { + Into::, Origin>>::into(o).and_then(|o| match o { + RawOrigin::Signed(who) => + if who == ConfirmSalpLiteConfirmAsMultiSig::get() { + Ok(who) + } else { + Err(Origin::from(Some(who))) + }, + r => Err(Origin::from(r)), + }) + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> Origin { + Origin::from(RawOrigin::Signed(Default::default())) + } +} + impl bifrost_salp_lite::Config for Runtime { type BancorPool = (); type Event = Event; @@ -1328,7 +1351,7 @@ impl bifrost_salp_lite::Config for Runtime { type SlotLength = SlotLength; type WeightInfo = weights::bifrost_salp_lite::WeightInfo; type EnsureConfirmAsMultiSig = - EnsureOneOf; + EnsureOneOf; type EnsureConfirmAsGovernance = EnsureOneOf; } From 055405dc95c99712f2eaccd7e43605a15e6d1a7c Mon Sep 17 00:00:00 2001 From: yrong Date: Tue, 9 Nov 2021 17:51:25 +0800 Subject: [PATCH 2/2] Revamp salp flow for unlock&refund --- pallets/salp-lite/src/lib.rs | 80 ++++------- pallets/salp-lite/src/tests.rs | 146 ++++++++++--------- pallets/salp/src/lib.rs | 107 +++++--------- pallets/salp/src/tests.rs | 256 ++++++++++++++++----------------- 4 files changed, 259 insertions(+), 330 deletions(-) diff --git a/pallets/salp-lite/src/lib.rs b/pallets/salp-lite/src/lib.rs index 13dd5be9ca..72a1a9a05e 100644 --- a/pallets/salp-lite/src/lib.rs +++ b/pallets/salp-lite/src/lib.rs @@ -456,16 +456,8 @@ pub mod pallet { #[pallet::compact] index: ParaId, ) -> DispatchResult { let fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!( - fund.status == FundStatus::Success || - fund.status == FundStatus::Retired || - fund.status == FundStatus::RedeemWithdrew || - fund.status == FundStatus::End, - Error::::InvalidFundStatus - ); - let (contributed, status) = Self::contribution(fund.trie_index, &who); - ensure!(status == ContributionStatus::Idle, Error::::InvalidContributionStatus); + let (contributed, _) = Self::contribution(fund.trie_index, &who); #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); @@ -497,13 +489,6 @@ pub mod pallet { ensure_signed(origin)?; let fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!( - fund.status == FundStatus::Success || - fund.status == FundStatus::Retired || - fund.status == FundStatus::RedeemWithdrew || - fund.status == FundStatus::End, - Error::::InvalidFundStatus - ); let mut unlock_count = 0u32; let contributions = Self::contribution_iterator(fund.trie_index); @@ -519,10 +504,8 @@ pub mod pallet { if status != ContributionStatus::Unlocked { #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - let balance = T::MultiCurrency::unreserve(vsToken, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughBalanceToUnlock); - let balance = T::MultiCurrency::unreserve(vsBond, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughBalanceToUnlock); + T::MultiCurrency::unreserve(vsToken, &who, contributed); + T::MultiCurrency::unreserve(vsBond, &who, contributed); Self::put_contribution( fund.trie_index, @@ -676,10 +659,6 @@ pub mod pallet { let who = ensure_signed(origin.clone())?; let mut fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!( - fund.status == FundStatus::Failed || fund.status == FundStatus::RefundWithdrew, - Error::::InvalidFundStatus - ); let (contributed, status) = Self::contribution(fund.trie_index, &who); ensure!(contributed > Zero::zero(), Error::::ZeroContribution); @@ -689,21 +668,11 @@ pub mod pallet { #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - ensure!( - T::MultiCurrency::reserved_balance(vsToken, &who) >= contributed, - Error::::NotEnoughReservedAssetsToRefund - ); - ensure!( - T::MultiCurrency::reserved_balance(vsBond, &who) >= contributed, - Error::::NotEnoughReservedAssetsToRefund - ); fund.raised = fund.raised.saturating_sub(contributed); - let balance = T::MultiCurrency::slash_reserved(vsToken, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughReservedAssetsToRefund); - let balance = T::MultiCurrency::slash_reserved(vsBond, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughReservedAssetsToRefund); + T::MultiCurrency::slash_reserved(vsToken, &who, contributed); + T::MultiCurrency::slash_reserved(vsBond, &who, contributed); Self::kill_contribution(fund.trie_index, &who); @@ -722,32 +691,35 @@ pub mod pallet { let who = ensure_signed(origin.clone())?; let mut fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!(fund.status == FundStatus::RedeemWithdrew, Error::::InvalidFundStatus); - ensure!(fund.raised >= value, Error::::NotEnoughBalanceInRedeemPool); - - let (contributed, status) = Self::contribution(fund.trie_index, &who); ensure!( - status == ContributionStatus::Idle || - status == ContributionStatus::Unlocked || - status == ContributionStatus::Redeemed, - Error::::InvalidContributionStatus + fund.status == FundStatus::RefundWithdrew || + fund.status == FundStatus::RedeemWithdrew, + Error::::InvalidFundStatus ); + ensure!(fund.raised >= value, Error::::NotEnoughBalanceInRedeemPool); - ensure!(Self::redeem_pool() >= value, Error::::NotEnoughBalanceInRedeemPool); - + let (contributed, _) = Self::contribution(fund.trie_index, &who); #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - T::MultiCurrency::ensure_can_withdraw(vsToken, &who, value) - .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; - T::MultiCurrency::ensure_can_withdraw(vsBond, &who, value) - .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; - T::MultiCurrency::withdraw(vsToken, &who, value)?; - T::MultiCurrency::withdraw(vsBond, &who, value)?; + if fund.status == FundStatus::RedeemWithdrew { + ensure!(Self::redeem_pool() >= value, Error::::NotEnoughBalanceInRedeemPool); + T::MultiCurrency::ensure_can_withdraw(vsToken, &who, value) + .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; + T::MultiCurrency::ensure_can_withdraw(vsBond, &who, value) + .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; + } - fund.raised = fund.raised.saturating_sub(value); - RedeemPool::::set(Self::redeem_pool().saturating_sub(value)); + if fund.status == FundStatus::RedeemWithdrew { + T::MultiCurrency::withdraw(vsToken, &who, value)?; + T::MultiCurrency::withdraw(vsBond, &who, value)?; + RedeemPool::::set(Self::redeem_pool().saturating_sub(value)); + } else if fund.status == FundStatus::RefundWithdrew { + T::MultiCurrency::slash_reserved(vsToken, &who, contributed); + T::MultiCurrency::slash_reserved(vsBond, &who, contributed); + } + fund.raised = fund.raised.saturating_sub(value); let contributed_new = contributed.saturating_sub(value); Self::put_contribution( fund.trie_index, diff --git a/pallets/salp-lite/src/tests.rs b/pallets/salp-lite/src/tests.rs index eb7dfa4b12..45333a072d 100644 --- a/pallets/salp-lite/src/tests.rs +++ b/pallets/salp-lite/src/tests.rs @@ -18,7 +18,7 @@ // Ensure we're `no_std` when compiling for Wasm. -use frame_support::{assert_noop, assert_ok, dispatch::DispatchError, traits::BalanceStatus as BS}; +use frame_support::{assert_noop, assert_ok, dispatch::DispatchError}; use node_primitives::ContributionStatus; use orml_traits::{MultiCurrency, MultiReservableCurrency}; @@ -263,35 +263,6 @@ fn unlock_should_work() { }); } -#[test] -fn unlock_with_wrong_fund_status_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::issue(Some(ALICE).into(), BRUCE, 3_000, 100, CONTRIBUTON_INDEX,)); - assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); - - assert_noop!( - Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000), - Error::::InvalidFundStatus - ); - }); -} - -#[test] -fn unlock_when_already_unlocked_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::issue(Some(ALICE).into(), BRUCE, 3_000, 100, CONTRIBUTON_INDEX,)); - assert_ok!(Salp::fund_success(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000)); - - assert_noop!( - Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000), - Error::::InvalidContributionStatus - ); - }); -} - #[test] fn unlock_without_enough_reserved_vsassets_should_fail() { new_test_ext().execute_with(|| { @@ -554,26 +525,6 @@ fn refund_should_work() { }); } -#[test] -fn refund_without_enough_vsassets_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::issue(Some(ALICE).into(), BRUCE, 3_000, 100, CONTRIBUTON_INDEX)); - assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); - - #[allow(non_snake_case)] - let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); - assert_ok!(Tokens::repatriate_reserved(vsToken, &BRUCE, &ALICE, 50, BS::Reserved)); - assert_ok!(Tokens::repatriate_reserved(vsBond, &BRUCE, &ALICE, 50, BS::Reserved)); - - assert_noop!( - Salp::refund(Some(BRUCE).into(), 3_000), - Error::::NotEnoughReservedAssetsToRefund - ); - }); -} - #[test] fn refund_with_zero_contribution_should_fail() { new_test_ext().execute_with(|| { @@ -611,25 +562,6 @@ fn refund_with_wrong_para_id_should_fail() { }); } -#[test] -fn refund_with_wrong_fund_status_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::fund_end(Some(ALICE).into(), 3_000)); - - assert_noop!(Salp::refund(Some(BRUCE).into(), 3_000), Error::::InvalidFundStatus); - - assert_ok!(Salp::create(Some(ALICE).into(), 4_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::fund_success(Some(ALICE).into(), 4_000)); - assert_ok!(Salp::fund_retire(Some(ALICE).into(), 4_000)); - assert_ok!(Salp::withdraw(Some(ALICE).into(), 4_000)); - - assert_noop!(Salp::refund(Some(BRUCE).into(), 4_000), Error::::InvalidFundStatus); - }); -} - #[test] fn refund_with_when_ump_wrong_should_fail() { // TODO: Require an solution to settle with parallel test workflow @@ -972,3 +904,79 @@ fn batch_migrate_should_work() { assert_eq!(Tokens::accounts(BRUCE, vs_bond_new).reserved, 100); }) } + +#[test] +fn unlock_when_fund_ongoing_should_work() { + new_test_ext().execute_with(|| { + assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); + assert_ok!(Salp::issue(Some(ALICE).into(), BRUCE, 3_000, 100, CONTRIBUTON_INDEX)); + assert_ok!(Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000)); + + #[allow(non_snake_case)] + let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); + + assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 100); + assert_eq!(Tokens::accounts(BRUCE, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).free, 100); + assert_eq!(Tokens::accounts(BRUCE, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).reserved, 0); + }); +} + +#[test] +fn refund_when_fund_ongoing_should_work() { + new_test_ext().execute_with(|| { + assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); + assert_ok!(Salp::issue(Some(ALICE).into(), BRUCE, 3_000, 100, CONTRIBUTON_INDEX)); + assert_ok!(Salp::refund(Some(BRUCE).into(), 3_000)); + + #[allow(non_snake_case)] + let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); + assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).free, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).free, INIT_BALANCE); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).reserved, 0); + }); +} + +#[test] +fn redeem_when_fund_failed_should_work() { + new_test_ext().execute_with(|| { + assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); + assert_ok!(Salp::issue(Some(ALICE).into(), BRUCE, 3_000, 100, CONTRIBUTON_INDEX)); + + assert_ok!(Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000)); + assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); + assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); + + #[allow(non_snake_case)] + let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); + + assert_ok!(>::transfer(vsToken, &BRUCE, &CATHI, 50)); + assert_ok!(>::transfer(vsBond, &BRUCE, &CATHI, 50)); + + assert_ok!(Salp::redeem(Some(BRUCE).into(), 3_000, 50)); + + assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 50); + assert_eq!(Tokens::accounts(BRUCE, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).free, 50); + assert_eq!(Tokens::accounts(BRUCE, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).reserved, 0); + + assert_ok!(Salp::redeem(Some(CATHI).into(), 3_000, 50)); + + assert_eq!(Tokens::accounts(CATHI, vsToken).free, 50); + assert_eq!(Tokens::accounts(CATHI, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(CATHI, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(CATHI, vsBond).free, 50); + assert_eq!(Tokens::accounts(CATHI, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(CATHI, vsBond).reserved, 0); + }); +} diff --git a/pallets/salp/src/lib.rs b/pallets/salp/src/lib.rs index 947448196c..04883061e9 100644 --- a/pallets/salp/src/lib.rs +++ b/pallets/salp/src/lib.rs @@ -461,24 +461,14 @@ pub mod pallet { #[pallet::compact] index: ParaId, ) -> DispatchResult { let fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!( - fund.status == FundStatus::Success || - fund.status == FundStatus::Retired || - fund.status == FundStatus::RedeemWithdrew || - fund.status == FundStatus::End, - Error::::InvalidFundStatus - ); - let (contributed, status) = Self::contribution(fund.trie_index, &who); - ensure!(status == ContributionStatus::Idle, Error::::InvalidContributionStatus); + let (contributed, _) = Self::contribution(fund.trie_index, &who); #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - let balance = T::MultiCurrency::unreserve(vsToken, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughBalanceToUnlock); - let balance = T::MultiCurrency::unreserve(vsBond, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughBalanceToUnlock); + T::MultiCurrency::unreserve(vsToken, &who, contributed); + T::MultiCurrency::unreserve(vsBond, &who, contributed); Self::put_contribution( fund.trie_index, @@ -502,13 +492,6 @@ pub mod pallet { ensure_signed(origin)?; let fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!( - fund.status == FundStatus::Success || - fund.status == FundStatus::Retired || - fund.status == FundStatus::RedeemWithdrew || - fund.status == FundStatus::End, - Error::::InvalidFundStatus - ); let mut unlock_count = 0u32; let contributions = Self::contribution_iterator(fund.trie_index); @@ -524,10 +507,8 @@ pub mod pallet { if status != ContributionStatus::Unlocked { #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - let balance = T::MultiCurrency::unreserve(vsToken, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughBalanceToUnlock); - let balance = T::MultiCurrency::unreserve(vsBond, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughBalanceToUnlock); + T::MultiCurrency::unreserve(vsToken, &who, contributed); + T::MultiCurrency::unreserve(vsBond, &who, contributed); Self::put_contribution( fund.trie_index, @@ -755,8 +736,6 @@ pub mod pallet { let who = ensure_signed(origin.clone())?; let mut fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!(fund.status == FundStatus::RefundWithdrew, Error::::InvalidFundStatus); - let (contributed, status) = Self::contribution(fund.trie_index, &who); ensure!(contributed > Zero::zero(), Error::::ZeroContribution); ensure!(status == ContributionStatus::Idle, Error::::InvalidContributionStatus); @@ -765,21 +744,11 @@ pub mod pallet { #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - ensure!( - T::MultiCurrency::reserved_balance(vsToken, &who) >= contributed, - Error::::NotEnoughReservedAssetsToRefund - ); - ensure!( - T::MultiCurrency::reserved_balance(vsBond, &who) >= contributed, - Error::::NotEnoughReservedAssetsToRefund - ); fund.raised = fund.raised.saturating_sub(contributed); - let balance = T::MultiCurrency::slash_reserved(vsToken, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughReservedAssetsToRefund); - let balance = T::MultiCurrency::slash_reserved(vsBond, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughReservedAssetsToRefund); + T::MultiCurrency::slash_reserved(vsToken, &who, contributed); + T::MultiCurrency::slash_reserved(vsBond, &who, contributed); if T::TransactType::get() == ParachainTransactType::Xcm { T::MultiCurrency::transfer( @@ -789,12 +758,7 @@ pub mod pallet { contributed, )?; } - Self::put_contribution( - fund.trie_index, - &who, - Zero::zero(), - ContributionStatus::Refunded, - ); + Self::kill_contribution(fund.trie_index, &who); Self::deposit_event(Event::Refunded(who, index, contributed)); @@ -828,10 +792,8 @@ pub mod pallet { let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); fund.raised = fund.raised.saturating_sub(contributed); - let balance = T::MultiCurrency::slash_reserved(vsToken, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughReservedAssetsToRefund); - let balance = T::MultiCurrency::slash_reserved(vsBond, &who, contributed); - ensure!(balance == Zero::zero(), Error::::NotEnoughReservedAssetsToRefund); + T::MultiCurrency::slash_reserved(vsToken, &who, contributed); + T::MultiCurrency::slash_reserved(vsBond, &who, contributed); if T::TransactType::get() == ParachainTransactType::Xcm { T::MultiCurrency::transfer( @@ -841,12 +803,7 @@ pub mod pallet { contributed, )?; } - Self::put_contribution( - fund.trie_index, - &who, - Zero::zero(), - ContributionStatus::Refunded, - ); + Self::kill_contribution(fund.trie_index, &who); refund_count += 1; } } @@ -868,33 +825,37 @@ pub mod pallet { let who = ensure_signed(origin.clone())?; let mut fund = Self::funds(index).ok_or(Error::::InvalidParaId)?; - ensure!(fund.status == FundStatus::RedeemWithdrew, Error::::InvalidFundStatus); - ensure!(fund.raised >= value, Error::::NotEnoughBalanceInRedeemPool); - - let (contributed, status) = Self::contribution(fund.trie_index, &who); ensure!( - status == ContributionStatus::Idle || - status == ContributionStatus::Unlocked || - status == ContributionStatus::Redeemed, - Error::::InvalidContributionStatus + fund.status == FundStatus::RefundWithdrew || + fund.status == FundStatus::RedeemWithdrew, + Error::::InvalidFundStatus ); + ensure!(fund.raised >= value, Error::::NotEnoughBalanceInRedeemPool); - ensure!(Self::redeem_pool() >= value, Error::::NotEnoughBalanceInRedeemPool); - let cur_block = >::block_number(); - ensure!(!Self::is_expired(cur_block, fund.last_slot), Error::::VSBondExpired); - + let (contributed, _) = Self::contribution(fund.trie_index, &who); #[allow(non_snake_case)] let (vsToken, vsBond) = Self::vsAssets(index, fund.first_slot, fund.last_slot); - T::MultiCurrency::ensure_can_withdraw(vsToken, &who, value) - .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; - T::MultiCurrency::ensure_can_withdraw(vsBond, &who, value) - .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; - T::MultiCurrency::withdraw(vsToken, &who, value)?; - T::MultiCurrency::withdraw(vsBond, &who, value)?; + if fund.status == FundStatus::RedeemWithdrew { + ensure!(Self::redeem_pool() >= value, Error::::NotEnoughBalanceInRedeemPool); + let cur_block = >::block_number(); + ensure!(!Self::is_expired(cur_block, fund.last_slot), Error::::VSBondExpired); + T::MultiCurrency::ensure_can_withdraw(vsToken, &who, value) + .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; + T::MultiCurrency::ensure_can_withdraw(vsBond, &who, value) + .map_err(|_e| Error::::NotEnoughFreeAssetsToRedeem)?; + } + + if fund.status == FundStatus::RedeemWithdrew { + T::MultiCurrency::withdraw(vsToken, &who, value)?; + T::MultiCurrency::withdraw(vsBond, &who, value)?; + RedeemPool::::set(Self::redeem_pool().saturating_sub(value)); + } else if fund.status == FundStatus::RefundWithdrew { + T::MultiCurrency::slash_reserved(vsToken, &who, contributed); + T::MultiCurrency::slash_reserved(vsBond, &who, contributed); + } fund.raised = fund.raised.saturating_sub(value); - RedeemPool::::set(Self::redeem_pool().saturating_sub(value)); if T::TransactType::get() == ParachainTransactType::Xcm { T::MultiCurrency::transfer( diff --git a/pallets/salp/src/tests.rs b/pallets/salp/src/tests.rs index ee014988c6..5ea78839b6 100644 --- a/pallets/salp/src/tests.rs +++ b/pallets/salp/src/tests.rs @@ -18,9 +18,9 @@ // Ensure we're `no_std` when compiling for Wasm. -use frame_support::{assert_noop, assert_ok, dispatch::DispatchError, traits::BalanceStatus as BS}; +use frame_support::{assert_noop, assert_ok, dispatch::DispatchError}; use node_primitives::ContributionStatus; -use orml_traits::{MultiCurrency, MultiReservableCurrency}; +use orml_traits::MultiCurrency; use crate::{mock::*, Error, FundStatus}; @@ -270,82 +270,6 @@ fn unlock_should_work() { }); } -#[test] -fn unlock_with_wrong_fund_status_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); - assert_ok!(Salp::confirm_contribute( - Some(ALICE).into(), - BRUCE, - 3_000, - true, - CONTRIBUTON_INDEX - )); - assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); - - assert_noop!( - Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000), - Error::::InvalidFundStatus - ); - }); -} - -#[test] -fn unlock_when_already_unlocked_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); - assert_ok!(Salp::confirm_contribute( - Some(ALICE).into(), - BRUCE, - 3_000, - true, - CONTRIBUTON_INDEX - )); - assert_ok!(Salp::fund_success(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000)); - - assert_noop!( - Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000), - Error::::InvalidContributionStatus - ); - }); -} - -#[test] -fn unlock_without_enough_reserved_vsassets_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); - assert_ok!(Salp::confirm_contribute( - Some(ALICE).into(), - BRUCE, - 3_000, - true, - CONTRIBUTON_INDEX - )); - assert_ok!(Salp::fund_success(Some(ALICE).into(), 3_000)); - - // ABSOLUTELY NOT HAPPEN AT NORMAL PROCESS! - #[allow(non_snake_case)] - let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); - Tokens::slash_reserved(vsToken, &BRUCE, 50); - Tokens::slash_reserved(vsBond, &BRUCE, 50); - - // ``` - // // The following code will produce a supernatural bug. - // // DONT ASK WHY, I DONT KNOW! - // assert_noop!( - // Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000), - // Error::::NotEnoughBalanceToUnlock - // ); - // ``` - let result = Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000); - assert_noop!(result, Error::::NotEnoughBalanceToUnlock); - }); -} - #[test] fn contribute_should_work() { new_test_ext().execute_with(|| { @@ -749,11 +673,6 @@ fn refund_should_work() { assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); assert_ok!(Salp::refund(Some(BRUCE).into(), 3_000)); - let fund = Salp::funds(3_000).unwrap(); - let (contributed, status) = Salp::contribution(fund.trie_index, &BRUCE); - assert_eq!(contributed, 0); - assert_eq!(status, ContributionStatus::Refunded); - #[allow(non_snake_case)] let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 0); @@ -784,11 +703,6 @@ fn refund_when_xcm_error_should_work() { assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); assert_ok!(Salp::refund(Some(BRUCE).into(), 3_000)); - let fund = Salp::funds(3_000).unwrap(); - let (contributed, status) = Salp::contribution(fund.trie_index, &BRUCE); - assert_eq!(contributed, 0); - assert_eq!(status, ContributionStatus::Refunded); - #[allow(non_snake_case)] let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 0); @@ -819,33 +733,6 @@ fn double_refund_when_one_of_xcm_error_should_work() { }); } -#[test] -fn refund_without_enough_vsassets_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); - assert_ok!(Salp::confirm_contribute( - Some(ALICE).into(), - BRUCE, - 3_000, - true, - CONTRIBUTON_INDEX - )); - assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); - - #[allow(non_snake_case)] - let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); - assert_ok!(Tokens::repatriate_reserved(vsToken, &BRUCE, &ALICE, 50, BS::Reserved)); - assert_ok!(Tokens::repatriate_reserved(vsBond, &BRUCE, &ALICE, 50, BS::Reserved)); - - assert_noop!( - Salp::refund(Some(BRUCE).into(), 3_000), - Error::::NotEnoughReservedAssetsToRefund - ); - }); -} - #[test] fn refund_with_zero_contribution_should_fail() { new_test_ext().execute_with(|| { @@ -890,25 +777,6 @@ fn refund_with_wrong_para_id_should_fail() { }); } -#[test] -fn refund_with_wrong_fund_status_should_fail() { - new_test_ext().execute_with(|| { - assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); - assert_ok!(Salp::fund_end(Some(ALICE).into(), 3_000)); - - assert_noop!(Salp::refund(Some(BRUCE).into(), 3_000), Error::::InvalidFundStatus); - - assert_ok!(Salp::create(Some(ALICE).into(), 4_000, 1_000, 1, SlotLength::get())); - assert_ok!(Salp::fund_success(Some(ALICE).into(), 4_000)); - assert_ok!(Salp::fund_retire(Some(ALICE).into(), 4_000)); - assert_ok!(Salp::withdraw(Some(ALICE).into(), 4_000)); - - assert_noop!(Salp::refund(Some(BRUCE).into(), 4_000), Error::::InvalidFundStatus); - }); -} - #[test] fn refund_with_when_ump_wrong_should_fail() { // TODO: Require an solution to settle with parallel test workflow @@ -1347,3 +1215,123 @@ fn edit_fund_should_work() { assert_eq!(fund.status, FundStatus::Ongoing); }) } + +#[test] +fn unlock_when_fund_ongoing_should_work() { + new_test_ext().execute_with(|| { + assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); + assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); + assert_ok!(Salp::confirm_contribute( + Some(ALICE).into(), + BRUCE, + 3_000, + true, + CONTRIBUTON_INDEX + )); + assert_ok!(Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000)); + + #[allow(non_snake_case)] + let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); + + assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 100); + assert_eq!(Tokens::accounts(BRUCE, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).free, 100); + assert_eq!(Tokens::accounts(BRUCE, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).reserved, 0); + }); +} + +#[test] +fn refund_when_fund_ongoing_should_work() { + new_test_ext().execute_with(|| { + assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); + assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); + assert_ok!(Salp::confirm_contribute( + Some(ALICE).into(), + BRUCE, + 3_000, + true, + CONTRIBUTON_INDEX + )); + assert_ok!(Salp::refund(Some(BRUCE).into(), 3_000)); + + #[allow(non_snake_case)] + let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); + assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).free, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).free, INIT_BALANCE); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).reserved, 0); + }); +} + +#[test] +fn redeem_when_fund_failed_should_work() { + new_test_ext().execute_with(|| { + assert_ok!(Salp::create(Some(ALICE).into(), 3_000, 1_000, 1, SlotLength::get())); + assert_ok!(Salp::contribute(Some(BRUCE).into(), 3_000, 100)); + assert_ok!(Salp::confirm_contribute( + Some(ALICE).into(), + BRUCE, + 3_000, + true, + CONTRIBUTON_INDEX + )); + + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).free, INIT_BALANCE - 100); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).reserved, 0); + + assert_eq!( + Tokens::accounts(Salp::fund_account_id(3_000), RelayCurrencyId::get()).free, + 100 + ); + assert_eq!( + Tokens::accounts(Salp::fund_account_id(3_000), RelayCurrencyId::get()).frozen, + 0 + ); + assert_eq!( + Tokens::accounts(Salp::fund_account_id(3_000), RelayCurrencyId::get()).reserved, + 0 + ); + + assert_ok!(Salp::unlock(Some(BRUCE).into(), BRUCE, 3_000)); + assert_ok!(Salp::fund_fail(Some(ALICE).into(), 3_000)); + assert_ok!(Salp::withdraw(Some(ALICE).into(), 3_000)); + + #[allow(non_snake_case)] + let (vsToken, vsBond) = Salp::vsAssets(3_000, 1, SlotLength::get()); + + assert_ok!(>::transfer(vsToken, &BRUCE, &CATHI, 50)); + assert_ok!(>::transfer(vsBond, &BRUCE, &CATHI, 50)); + + assert_ok!(Salp::redeem(Some(BRUCE).into(), 3_000, 50)); + + assert_eq!(Tokens::accounts(BRUCE, vsToken).free, 50); + assert_eq!(Tokens::accounts(BRUCE, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).free, 50); + assert_eq!(Tokens::accounts(BRUCE, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, vsBond).reserved, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).free, INIT_BALANCE - 50); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).frozen, 0); + assert_eq!(Tokens::accounts(BRUCE, RelayCurrencyId::get()).reserved, 0); + + assert_ok!(Salp::redeem(Some(CATHI).into(), 3_000, 50)); + + assert_eq!(Tokens::accounts(CATHI, vsToken).free, 50); + assert_eq!(Tokens::accounts(CATHI, vsToken).frozen, 0); + assert_eq!(Tokens::accounts(CATHI, vsToken).reserved, 0); + assert_eq!(Tokens::accounts(CATHI, vsBond).free, 50); + assert_eq!(Tokens::accounts(CATHI, vsBond).frozen, 0); + assert_eq!(Tokens::accounts(CATHI, vsBond).reserved, 0); + assert_eq!(Tokens::accounts(CATHI, RelayCurrencyId::get()).free, INIT_BALANCE + 50); + assert_eq!(Tokens::accounts(CATHI, RelayCurrencyId::get()).frozen, 0); + assert_eq!(Tokens::accounts(CATHI, RelayCurrencyId::get()).reserved, 0); + }); +}