From 3b0862b979c7f095cc30e6332ccf4ccd8e8b8512 Mon Sep 17 00:00:00 2001 From: Cheng JIANG Date: Thu, 16 Dec 2021 10:10:44 -0600 Subject: [PATCH] refund user & remove pending amount if xcm failed (#1058) * refund user & remove pending amount if xcm failed Signed-off-by: Cheng JIANG * only allow migrate_pending to be executed in Pending phase Signed-off-by: Cheng JIANG * prepare for Succeeded phase Signed-off-by: Cheng JIANG --- pallets/crowdloans/src/benchmarking.rs | 26 ++++++------ pallets/crowdloans/src/lib.rs | 59 +++++++++++++++++--------- pallets/crowdloans/src/mock.rs | 11 ++--- pallets/crowdloans/src/types.rs | 19 +++++---- 4 files changed, 68 insertions(+), 47 deletions(-) diff --git a/pallets/crowdloans/src/benchmarking.rs b/pallets/crowdloans/src/benchmarking.rs index ade5aa44e..261549543 100644 --- 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::(Event::::VaultCreated(crowdloan, ctoken).into()) @@ -116,12 +116,12 @@ benchmarks! { let caller: T::AccountId = whitelisted_caller(); initial_set_up::(caller, ctoken); // create vault before update - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK)); + assert_ok!(Crowdloans::::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::(caller.clone(), ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); }: _( SystemOrigin::Signed(caller.clone()), @@ -152,7 +152,7 @@ benchmarks! { let crowdloan = ParaId::from(1336u32); initial_set_up::(caller, ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK)); + assert_ok!(Crowdloans::::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::(caller, ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); }: _( SystemOrigin::Root, @@ -183,7 +183,7 @@ benchmarks! { let crowdloan = ParaId::from(1338u32); initial_set_up::(caller, ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK)); + assert_ok!(Crowdloans::::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::(caller, ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); assert_ok!(Crowdloans::::close(SystemOrigin::Root.into(), crowdloan)); }: _( @@ -216,7 +216,7 @@ benchmarks! { let crowdloan = ParaId::from(1340u32); initial_set_up::(caller.clone(), ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); assert_ok!(Crowdloans::::contribute(SystemOrigin::Signed(caller).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new())); assert_ok!(Crowdloans::::close(SystemOrigin::Root.into(), crowdloan)); @@ -235,7 +235,7 @@ benchmarks! { let crowdloan = ParaId::from(1341u32); initial_set_up::(caller.clone(), ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); assert_ok!(Crowdloans::::contribute(SystemOrigin::Signed(caller.clone()).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new())); assert_ok!(Crowdloans::::notification_received( @@ -265,7 +265,7 @@ benchmarks! { let crowdloan = ParaId::from(1342u32); initial_set_up::(caller.clone(), ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); assert_ok!(Crowdloans::::contribute(SystemOrigin::Signed(caller).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new())); assert_ok!(Crowdloans::::close(SystemOrigin::Root.into(), crowdloan)); @@ -283,7 +283,7 @@ benchmarks! { let crowdloan = ParaId::from(1343u32); initial_set_up::(caller.clone(), ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into())); for _ in 0..10 { assert_ok!(Crowdloans::::contribute(SystemOrigin::Signed(caller.clone()).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new())); } @@ -301,7 +301,7 @@ benchmarks! { let crowdloan = ParaId::from(1344u32); initial_set_up::(caller.clone(), ctoken); - assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK)); + assert_ok!(Crowdloans::::create_vault(SystemOrigin::Root.into(), crowdloan, ctoken, ContributionStrategy::XCM, LARGE_CAP, END_BLOCK.into())); assert_ok!(Crowdloans::::open(SystemOrigin::Root.into(), crowdloan)); assert_ok!(Crowdloans::::contribute(SystemOrigin::Signed(caller).into(), crowdloan, CONTRIBUTE_AMOUNT, Vec::new())); }: _( diff --git a/pallets/crowdloans/src/lib.rs b/pallets/crowdloans/src/lib.rs index 47f00b258..4e6c1b0d9 100644 --- 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>; /// To expose XCM helper functions type XCM: XcmHelper, AssetIdOf, Self::AccountId>; @@ -253,7 +254,7 @@ pub mod pallet { ctoken: AssetIdOf, contribution_strategy: ContributionStrategy, #[pallet::compact] cap: BalanceOf, - end_block: BlockNumber, + end_block: BlockNumberFor, ) -> DispatchResult { T::CreateVaultOrigin::ensure_origin(origin)?; @@ -319,7 +320,7 @@ pub mod pallet { origin: OriginFor, crowdloan: ParaId, cap: Option>, - end_block: Option, + end_block: Option>, contribution_strategy: Option, ) -> 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::::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::::VaultDoesNotExist)?; + ensure!( + vault.phase == VaultPhase::Pending, + Error::::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, vault: &mut Vault, amount: BalanceOf, + 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::::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::::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::::insert(crowdloan, vault.id, vault); } XcmInflightRequest::Withdraw { crowdloan, amount, target_phase, - } => { + } if executed => { let mut vault = Self::current_vault(crowdloan).ok_or(Error::::VaultDoesNotExist)?; T::Assets::mint_into( diff --git a/pallets/crowdloans/src/mock.rs b/pallets/crowdloans/src/mock.rs index 639396b2e..9ab5a35d7 100644 --- 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; type Block = frame_system::mocking::MockBlock; -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; - type Header = Header; + type Header = generic::Header; type Event = Event; type BlockHashCount = BlockHashCount; type Version = (); diff --git a/pallets/crowdloans/src/types.rs b/pallets/crowdloans/src/types.rs index c3e763d6b..1a508f3d0 100644 --- 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 { /// parallel enforced limit pub cap: BalanceOf, /// block that vault ends - pub end_block: BlockNumber, + pub end_block: BlockNumberFor, /// child storage trie index where we store all contributions pub trie_index: TrieIndex, } @@ -74,7 +75,7 @@ impl Vault { ctoken: AssetIdOf, contribution_strategy: ContributionStrategy, cap: BalanceOf, - end_block: BlockNumber, + end_block: BlockNumberFor, trie_index: TrieIndex, ) -> Self { Self { @@ -93,7 +94,7 @@ impl Vault { #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] pub enum ContributionStrategy { - XCM, + XCM = 0, } #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]