refund user & remove pending amount if xcm failed (#1058)

* refund user & remove pending amount if xcm failed Signed-off-by: Cheng JIANG <[email protected]> * only allow migrate_pending to be executed in Pending phase Signed-off-by: Cheng JIANG <[email protected]> * prepare for Succeeded phase Signed-off-by: Cheng JIANG <[email protected]>
parallel-finance · Dec 16, 2021 · 3b0862b · 3b0862b
1 parent 723c7c4
commit 3b0862b
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 47 deletions.
diff --git a/pallets/crowdloans/src/benchmarking.rs b/pallets/crowdloans/src/benchmarking.rs
@@ -104,7 +104,7 @@ benchmarks! {
         ctoken,
         ContributionStrategy::XCM,
         CAP,
-        END_BLOCK
+        END_BLOCK.into()
     )
     verify {
         assert_last_event::<T>(Event::<T>::VaultCreated(crowdloan, ctoken).into())
@@ -116,12 +116,12 @@ benchmarks! {
         let caller: T::AccountId = whitelisted_caller();
         initial_set_up::<T>(caller, ctoken);
         // create vault before update
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into()));
     }: _(
         SystemOrigin::Root,
         crowdloan,
         Some(1_000_000_000_001),
-        Some(1_000_000_001u32),
+        Some(1_000_000_001u32.into()),
         Some(ContributionStrategy::XCM)
     )
     verify {
@@ -134,7 +134,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1335u32);
 
         initial_set_up::<T>(caller.clone(), ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
     }: _(
         SystemOrigin::Signed(caller.clone()),
@@ -152,7 +152,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1336u32);
 
         initial_set_up::<T>(caller, ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into()));
     }: _(
         SystemOrigin::Root,
         crowdloan
@@ -167,7 +167,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1337u32);
 
         initial_set_up::<T>(caller, ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
     }: _(
         SystemOrigin::Root,
@@ -183,7 +183,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1338u32);
 
         initial_set_up::<T>(caller, ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into()));
     }: _(
         SystemOrigin::Root,
         vec![ParaId::from(1336u32), ParaId::from(1337u32)]
@@ -199,7 +199,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1339u32);
 
         initial_set_up::<T>(caller, ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
         assert_ok!(Crowdloans::<T>::close(SystemOrigin::Root.into(), crowdloan));
     }: _(
@@ -216,7 +216,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1340u32);
 
         initial_set_up::<T>(caller.clone(), ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
         assert_ok!(Crowdloans::<T>::contribute(SystemOrigin::Signed(caller).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new()));
         assert_ok!(Crowdloans::<T>::close(SystemOrigin::Root.into(), crowdloan));
@@ -235,7 +235,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1341u32);
 
         initial_set_up::<T>(caller.clone(), ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
         assert_ok!(Crowdloans::<T>::contribute(SystemOrigin::Signed(caller.clone()).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new()));
         assert_ok!(Crowdloans::<T>::notification_received(
@@ -265,7 +265,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1342u32);
 
         initial_set_up::<T>(caller.clone(), ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
         assert_ok!(Crowdloans::<T>::contribute(SystemOrigin::Signed(caller).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new()));
         assert_ok!(Crowdloans::<T>::close(SystemOrigin::Root.into(), crowdloan));
@@ -283,7 +283,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1343u32);
 
         initial_set_up::<T>(caller.clone(), ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into()));
         for _ in 0..10 {
             assert_ok!(Crowdloans::<T>::contribute(SystemOrigin::Signed(caller.clone()).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new()));
         }
@@ -301,7 +301,7 @@ benchmarks! {
         let crowdloan = ParaId::from(1344u32);
 
         initial_set_up::<T>(caller.clone(), ctoken);
-        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK));
+        assert_ok!(Crowdloans::<T>::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into()));
         assert_ok!(Crowdloans::<T>::open(SystemOrigin::Root.into(), crowdloan));
         assert_ok!(Crowdloans::<T>::contribute(SystemOrigin::Signed(caller).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new()));
     }: _(

diff --git a/pallets/crowdloans/src/lib.rs b/pallets/crowdloans/src/lib.rs
@@ -48,9 +48,12 @@ pub mod pallet {
         },
         transactional, Blake2_128Concat, BoundedVec, PalletId,
     };
-    use frame_system::{ensure_signed, pallet_prelude::OriginFor};
+    use frame_system::{
+        ensure_signed,
+        pallet_prelude::{BlockNumberFor, OriginFor},
+    };
     use pallet_xcm::ensure_response;
-    use primitives::{ump::*, Balance, BlockNumber, CurrencyId, ParaId, TrieIndex};
+    use primitives::{ump::*, Balance, CurrencyId, ParaId, TrieIndex};
     use sp_runtime::{
         traits::{AccountIdConversion, BlockNumberProvider, Convert, Hash, Zero},
         ArithmeticError, DispatchError,
@@ -143,9 +146,7 @@ pub mod pallet {
         type WeightInfo: WeightInfo;
 
         /// The relay's BlockNumber provider
-        type RelayChainBlockNumberProvider: BlockNumberProvider<
-            BlockNumber = primitives::BlockNumber,
-        >;
+        type RelayChainBlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
 
         /// To expose XCM helper functions
         type XCM: XcmHelper<Self, BalanceOf<Self>, AssetIdOf<Self>, Self::AccountId>;
@@ -253,7 +254,7 @@ pub mod pallet {
             ctoken: AssetIdOf<T>,
             contribution_strategy: ContributionStrategy,
             #[pallet::compact] cap: BalanceOf<T>,
-            end_block: BlockNumber,
+            end_block: BlockNumberFor<T>,
         ) -> DispatchResult {
             T::CreateVaultOrigin::ensure_origin(origin)?;
 
@@ -319,7 +320,7 @@ pub mod pallet {
             origin: OriginFor<T>,
             crowdloan: ParaId,
             cap: Option<BalanceOf<T>>,
-            end_block: Option<BlockNumber>,
+            end_block: Option<BlockNumberFor<T>>,
             contribution_strategy: Option<ContributionStrategy>,
         ) -> DispatchResult {
             T::UpdateVaultOrigin::ensure_origin(origin)?;
@@ -425,7 +426,7 @@ pub mod pallet {
                 Self::do_contribute(&who, crowdloan, amount)?;
             }
 
-            Self::do_pending_contribution(&who, &mut vault, amount)?;
+            Self::do_update_pending(&who, &mut vault, amount, true)?;
 
             Vaults::<T>::insert(crowdloan, vault.id, vault);
 
@@ -592,6 +593,10 @@ pub mod pallet {
             T::MigrateOrigin::ensure_origin(origin)?;
 
             let vault = Self::current_vault(crowdloan).ok_or(Error::<T>::VaultDoesNotExist)?;
+            ensure!(
+                vault.phase == VaultPhase::Pending,
+                Error::<T>::IncorrectVaultPhase
+            );
             let contributions = Self::contribution_iterator(vault.trie_index, true);
             let mut migrated_count: u32 = 0u32;
             let mut all_migrated = true;
@@ -704,19 +709,30 @@ pub mod pallet {
         }
 
         #[require_transactional]
-        fn do_pending_contribution(
+        fn do_update_pending(
             who: &AccountIdOf<T>,
             vault: &mut Vault<T>,
             amount: BalanceOf<T>,
+            addition: bool,
         ) -> DispatchResult {
-            vault.pending = vault
-                .pending
-                .checked_add(amount)
-                .ok_or(ArithmeticError::Overflow)?;
             let (pending, _) = Self::contribution_get(vault.trie_index, who, true);
-            let new_pending = pending
-                .checked_add(amount)
-                .ok_or(ArithmeticError::Overflow)?;
+            let new_pending = if addition {
+                vault.pending = vault
+                    .pending
+                    .checked_add(amount)
+                    .ok_or(ArithmeticError::Overflow)?;
+                pending
+                    .checked_add(amount)
+                    .ok_or(ArithmeticError::Overflow)?
+            } else {
+                vault.pending = vault
+                    .pending
+                    .checked_sub(amount)
+                    .ok_or(ArithmeticError::Underflow)?;
+                pending
+                    .checked_sub(amount)
+                    .ok_or(ArithmeticError::Underflow)?
+            };
             Self::contribution_put(vault.trie_index, who, &new_pending, true);
             Ok(())
         }
@@ -779,24 +795,27 @@ pub mod pallet {
                     Vaults::<T>::insert(crowdloan, vault.id, vault);
                 }
                 XcmInflightRequest::Contribute {
-                    crowdloan: index,
+                    crowdloan,
                     who,
                     amount,
                 } if !executed => {
-                    // refund
+                    let mut vault =
+                        Self::current_vault(crowdloan).ok_or(Error::<T>::VaultDoesNotExist)?;
                     T::Assets::transfer(
                         T::RelayCurrency::get(),
-                        &Self::vault_account_id(index),
+                        &Self::vault_account_id(crowdloan),
                         &who,
                         amount,
                         true,
                     )?;
+                    Self::do_update_pending(&who, &mut vault, amount, false)?;
+                    Vaults::<T>::insert(crowdloan, vault.id, vault);
                 }
                 XcmInflightRequest::Withdraw {
                     crowdloan,
                     amount,
                     target_phase,
-                } => {
+                } if executed => {
                     let mut vault =
                         Self::current_vault(crowdloan).ok_or(Error::<T>::VaultDoesNotExist)?;
                     T::Assets::mint_into(

diff --git a/pallets/crowdloans/src/mock.rs b/pallets/crowdloans/src/mock.rs
@@ -13,7 +13,7 @@ use polkadot_parachain::primitives::Sibling;
 use primitives::{currency::MultiCurrencyAdapter, tokens::*, Balance, ParaId};
 use sp_core::H256;
 use sp_runtime::{
-    testing::Header,
+    generic,
     traits::{
         AccountIdConversion, AccountIdLookup, BlakeTwo256, BlockNumberProvider, Convert, Zero,
     },
@@ -259,11 +259,12 @@ impl orml_xtokens::Config for Test {
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
-type BlockNumber = u64;
+type BlockNumber = u32;
+type Index = u32;
 pub const DOT_DECIMAL: u128 = 10u128.pow(10);
 
 parameter_types! {
-    pub const BlockHashCount: u64 = 250;
+    pub const BlockHashCount: u32 = 250;
     pub const SS58Prefix: u8 = 42;
 }
 
@@ -274,13 +275,13 @@ impl frame_system::Config for Test {
     type DbWeight = ();
     type Origin = Origin;
     type Call = Call;
-    type Index = u64;
+    type Index = Index;
     type BlockNumber = BlockNumber;
     type Hash = H256;
     type Hashing = BlakeTwo256;
     type AccountId = AccountId;
     type Lookup = AccountIdLookup<AccountId, ()>;
-    type Header = Header;
+    type Header = generic::Header<BlockNumber, BlakeTwo256>;
     type Event = Event;
     type BlockHashCount = BlockHashCount;
     type Version = ();

diff --git a/pallets/crowdloans/src/types.rs b/pallets/crowdloans/src/types.rs
@@ -18,29 +18,30 @@ use super::{AccountIdOf, AssetIdOf, BalanceOf, Config};
 
 use codec::{Decode, Encode};
 
+use frame_system::pallet_prelude::BlockNumberFor;
 use scale_info::TypeInfo;
 use sp_runtime::{traits::Zero, RuntimeDebug};
 
-use primitives::{BlockNumber, ParaId, TrieIndex};
+use primitives::{ParaId, TrieIndex};
 
 #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
 pub enum VaultPhase {
     /// Vault is open for contributions but wont execute contribute call on relaychain
-    Pending,
+    Pending = 0,
     /// Vault is open for contributions
-    Contributing,
+    Contributing = 1,
     /// The vault is closed and we should avoid future contributions. This happens when
     /// - there are no contribution
     /// - user cancelled
     /// - crowdloan reached its cap
     /// - parachain won the slot
-    Closed,
+    Closed = 2,
     /// The vault's crowdloan failed, we have to distribute its assets back
     /// to the contributors
-    Failed,
+    Failed = 3,
     /// The vault's crowdloan and its associated parachain slot expired, it is
     /// now possible to get back the money we put in
-    Expired,
+    Expired = 5,
 }
 
 #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
@@ -62,7 +63,7 @@ pub struct Vault<T: Config> {
     /// parallel enforced limit
     pub cap: BalanceOf<T>,
     /// block that vault ends
-    pub end_block: BlockNumber,
+    pub end_block: BlockNumberFor<T>,
     /// child storage trie index where we store all contributions
     pub trie_index: TrieIndex,
 }
@@ -74,7 +75,7 @@ impl<T: Config> Vault<T> {
         ctoken: AssetIdOf<T>,
         contribution_strategy: ContributionStrategy,
         cap: BalanceOf<T>,
-        end_block: BlockNumber,
+        end_block: BlockNumberFor<T>,
         trie_index: TrieIndex,
     ) -> Self {
         Self {
@@ -93,7 +94,7 @@ impl<T: Config> Vault<T> {
 
 #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
 pub enum ContributionStrategy {
-    XCM,
+    XCM = 0,
 }
 
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]