From 0b8437789047cefbeb42dc7ca87d9a4015c12b76 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Fri, 20 Oct 2023 11:31:07 +0000 Subject: [PATCH 1/7] compute: emit nft id in WithdrawalQueued --- pallets/phala/src/compute/base_pool.rs | 36 ++++++++++++++++---------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/pallets/phala/src/compute/base_pool.rs b/pallets/phala/src/compute/base_pool.rs index 421e8449c6..50d8da21da 100644 --- a/pallets/phala/src/compute/base_pool.rs +++ b/pallets/phala/src/compute/base_pool.rs @@ -148,8 +148,11 @@ pub mod pallet { pid: u64, user: T::AccountId, shares: BalanceOf, + /// Target NFT to withdraw nft_id: NftId, as_vault: Option, + /// Splitted NFT for withdrawing + withdrawing_nft_id: NftId, }, /// Some stake was withdrawn from a pool /// @@ -653,13 +656,14 @@ pub mod pallet { nft: &mut NftAttr>, account_id: T::AccountId, shares: BalanceOf, - ) -> DispatchResult { + ) -> Result, DispatchError> { if pool.share_price().is_none() { + // Pool bankrupt. Reduce the NFT share without doing real task. nft.shares = nft .shares .checked_sub(&shares) .ok_or(Error::::InvalidShareToWithdraw)?; - return Ok(()); + return Ok(None); } // Remove the existing withdraw request in the queue if there is any. @@ -679,7 +683,7 @@ pub mod pallet { Self::burn_nft(&pallet_id(), pool.cid, withdrawinfo.nft_id) .expect("burn nft attr should always success; qed."); } - + // Move the shares in the NFT to the withdrawing NFT. let split_nft_id = Self::mint_nft(pool.cid, pallet_id(), shares, pool.pid) .expect("mint nft should always success"); nft.shares = nft @@ -696,7 +700,7 @@ pub mod pallet { nft_id: split_nft_id, }); - Ok(()) + Ok(Some(split_nft_id)) } /// Returns the new pid that will assigned to the creating pool @@ -1006,16 +1010,20 @@ pub mod pallet { nft_id: NftId, as_vault: Option, ) -> DispatchResult { - Self::push_withdraw_in_queue(pool_info, nft, userid.clone(), shares)?; - Self::deposit_event(Event::::WithdrawalQueued { - pid: pool_info.pid, - user: userid, - shares, - nft_id, - as_vault, - }); - Self::try_process_withdraw_queue(pool_info); - + let maybe_split_nft_id = + Self::push_withdraw_in_queue(pool_info, nft, userid.clone(), shares)?; + if let Some(split_nft_id) = maybe_split_nft_id { + // The pool is operating normally. Emit event and try to process withdraw queue. + Self::deposit_event(Event::::WithdrawalQueued { + pid: pool_info.pid, + user: userid, + shares, + nft_id, + as_vault, + withdrawing_nft_id: split_nft_id, + }); + Self::try_process_withdraw_queue(pool_info); + } Ok(()) } From 1dcba083e3ca311b39ee86f0a6b0e0d707d131b1 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Sun, 22 Oct 2023 09:53:54 +0000 Subject: [PATCH 2/7] compute: add burnt shares to withdrawal event Also rewrite withdraw tests --- pallets/phala/src/compute/base_pool.rs | 167 ++++++----- .../phala_pallets__test__withdraw1.snap | 224 +++++++++++++++ .../phala_pallets__test__withdraw2.snap | 232 +++++++++++++++ .../phala_pallets__test__withdraw3.snap | 269 ++++++++++++++++++ .../phala_pallets__test__withdraw4.snap | 224 +++++++++++++++ .../phala_pallets__test__withdraw5.snap | 264 +++++++++++++++++ .../phala_pallets__test__withdraw6.snap | 231 +++++++++++++++ .../phala_pallets__test__withdraw7.snap | 212 ++++++++++++++ .../phala_pallets__test__withdraw8.snap | 224 +++++++++++++++ .../phala_pallets__test__withdraw9.snap | 112 ++++++++ pallets/phala/src/test.rs | 171 +++++------ 11 files changed, 2158 insertions(+), 172 deletions(-) create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw1.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw2.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw3.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw4.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw5.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw6.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw7.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__withdraw9.snap diff --git a/pallets/phala/src/compute/base_pool.rs b/pallets/phala/src/compute/base_pool.rs index 50d8da21da..1f6fe8dab3 100644 --- a/pallets/phala/src/compute/base_pool.rs +++ b/pallets/phala/src/compute/base_pool.rs @@ -57,6 +57,14 @@ pub mod pallet { pub nft_id: NftId, } + /// Current withdrawing stats for a user + #[derive(Default)] + struct WithdrawEventStats { + amount: Balance, + shares: Balance, + burnt_shares: Balance, + } + #[pallet::config] pub trait Config: frame_system::Config @@ -167,6 +175,7 @@ pub mod pallet { user: T::AccountId, amount: BalanceOf, shares: BalanceOf, + burnt_shares: BalanceOf, }, /// A pool contribution whitelist is added /// @@ -1027,6 +1036,9 @@ pub mod pallet { Ok(()) } + /// Removes the share from the pool total_shares if it's a dust + /// + /// Return true if the dust is removed pub fn maybe_remove_dust( pool_info: &mut BasePool>, nft: &NftAttr>, @@ -1044,93 +1056,102 @@ pub mod pallet { true } - /// Removes withdrawing_shares from the nft - pub fn do_withdraw_shares( - withdrawing_shares: BalanceOf, - pool_info: &mut BasePool>, - nft: &mut NftAttr>, - userid: T::AccountId, - ) { - // Overflow warning: remove_stake is carefully written to avoid precision error. - // (I hope so) - let (reduced, withdrawn_shares) = - Self::remove_stake_from_nft(pool_info, withdrawing_shares, nft, &userid) - .expect("There are enough withdrawing_shares; qed."); - Self::deposit_event(Event::::Withdrawal { - pid: pool_info.pid, - user: userid, - amount: reduced, - shares: withdrawn_shares, - }); - } - /// Tries to fulfill the withdraw queue with the newly freed stake pub fn try_process_withdraw_queue(pool_info: &mut BasePool>) { + use sp_std::collections::btree_map::BTreeMap; // The share price shouldn't change at any point in this function. So we can calculate // only once at the beginning. let price = match pool_info.share_price() { Some(price) => price, None => return, }; - let wpha_min = wrapped_balances::Pallet::::min_balance(); + // Note: This function aggregates all the withdrawal stats and emit the events at the + // end. It's supposed the withdrawal process won't be interrupted by any error. + let mut withdrawing = BTreeMap::>>::new(); while pool_info.get_free_stakes::() > wpha_min { - if let Some(withdraw) = pool_info.withdraw_queue.front().cloned() { - // Must clear the pending reward before any stake change - let mut withdraw_nft_guard = - Self::get_nft_attr_guard(pool_info.cid, withdraw.nft_id) - .expect("get nftattr should always success; qed."); - let mut withdraw_nft = withdraw_nft_guard.attr.clone(); - if Self::maybe_remove_dust(pool_info, &withdraw_nft) { - pool_info.withdraw_queue.pop_front(); - Self::burn_nft(&pallet_id(), pool_info.cid, withdraw.nft_id) - .expect("burn nft should always success"); - continue; - } - // Try to fulfill the withdraw requests as much as possible - let free_shares = if price == fp!(0) { - withdraw_nft.shares // 100% slashed - } else { - bdiv(pool_info.get_free_stakes::(), &price) - }; - // This is the shares to withdraw immedately. It should NOT contain any dust - // because we ensure (1) `free_shares` is not dust earlier, and (2) the shares - // in any withdraw request mustn't be dust when inserting and updating it. - let withdrawing_shares = free_shares.min(withdraw_nft.shares); - debug_assert!( - is_nondust_balance(withdrawing_shares), - "withdrawing_shares must be positive" - ); - // Actually remove the fulfilled withdraw request. Dust in the user shares is - // considered but it in the request is ignored. - Self::do_withdraw_shares( - withdrawing_shares, - pool_info, - &mut withdraw_nft, - withdraw.user.clone(), - ); - withdraw_nft_guard.attr = withdraw_nft.clone(); - withdraw_nft_guard - .save() - .expect("save nft should always success"); - // Update if the withdraw is partially fulfilled, otherwise pop it out of the - // queue - if withdraw_nft.shares == Zero::zero() - || Self::maybe_remove_dust(pool_info, &withdraw_nft) - { - pool_info.withdraw_queue.pop_front(); - Self::burn_nft(&pallet_id(), pool_info.cid, withdraw.nft_id) - .expect("burn nft should always success"); - } else { - *pool_info - .withdraw_queue - .front_mut() - .expect("front exists as just checked; qed.") = withdraw; + let Some(withdraw) = pool_info.withdraw_queue.front().cloned() else { + // Stop if the queue is already empty + break; + }; + let mut nft = Self::get_nft_attr_guard(pool_info.cid, withdraw.nft_id) + .expect("get nftattr should always success; qed."); + // Fast track to remove existing dust in the queue + if Self::maybe_remove_dust(pool_info, &nft.attr) { + // Account the burning NFT to emit events + withdrawing + .entry(withdraw.user.clone()) + .or_default() + .burnt_shares += nft.attr.shares; + nft.unlock(); + // Actually burn the NFT + pool_info.withdraw_queue.pop_front(); + Self::burn_nft(&pallet_id(), pool_info.cid, withdraw.nft_id) + .expect("burn nft should always success"); + continue; + } + // Try to fulfill the withdraw requests as much as possible + let free_shares = if price == fp!(0) { + nft.attr.shares // 100% slashed + } else { + bdiv(pool_info.get_free_stakes::(), &price) + }; + // This is the shares to withdraw immedately. It should NOT contain any dust + // because we ensure (1) `free_shares` is not dust earlier, and (2) the shares + // in any withdraw request mustn't be dust when inserting and updating it. + let withdrawing_shares = free_shares.min(nft.attr.shares); + debug_assert!( + is_nondust_balance(withdrawing_shares), + "withdrawing_shares must be positive" + ); + // Actually remove the fulfilled withdraw request. Dust in the user shares is + // considered but it in the request is ignored. + let (reduced, withdrawn_shares) = Self::remove_stake_from_nft( + pool_info, + withdrawing_shares, + &mut nft.attr, + &withdraw.user, + ) + .expect("There are enough withdrawing_shares; qed."); + // Account the withdrawn NFT to emit events + let event = withdrawing.entry(withdraw.user.clone()).or_default(); + event.amount += reduced; + event.shares += withdrawn_shares; + // Update the withdraw NFT shares + let processed_nft = nft.attr.clone(); + nft.save().expect("save nft should always success"); + // Update if the withdraw is partially fulfilled, otherwise pop it out of the + // queue + if processed_nft.shares == Zero::zero() + || Self::maybe_remove_dust(pool_info, &processed_nft) + { + if processed_nft.shares != Zero::zero() { + // Account the burning NFT to emit events + withdrawing + .entry(withdraw.user.clone()) + .or_default() + .burnt_shares += processed_nft.shares; } + pool_info.withdraw_queue.pop_front(); + Self::burn_nft(&pallet_id(), pool_info.cid, withdraw.nft_id) + .expect("burn nft should always success"); } else { - break; + *pool_info + .withdraw_queue + .front_mut() + .expect("front exists as just checked; qed.") = withdraw; } } + // Emit the aggregated withdrawal events + for (user, stats) in withdrawing.into_iter() { + Self::deposit_event(Event::::Withdrawal { + pid: pool_info.pid, + user, + amount: stats.amount, + shares: stats.shares, + burnt_shares: stats.burnt_shares, + }) + } } } diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw1.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw1.snap new file mode 100644 index 0000000000..27970a8cba --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw1.snap @@ -0,0 +1,224 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 3, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10000, + nft_id: 3, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 3, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 192, + 110, + 49, + 217, + 16, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 3, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 3, + owner: 7813586407040180578, + shares: 300000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 0, + user: 2, + shares: 300000000000000, + nft_id: 0, + as_vault: None, + withdrawing_nft_id: 3, + }, + ), + RuntimeEvent::System( + Event::KilledAccount { + account: 16637257129592320098, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 16637257129592320098, + to: 2, + amount: 200000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 3, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 0, + user: 2, + amount: 200000000000000, + shares: 200000000000000, + burnt_shares: 0, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 0, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw2.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw2.snap new file mode 100644 index 0000000000..f555a65038 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw2.snap @@ -0,0 +1,232 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 4, + owner: 99, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 99, + ), + collection_id: 10000, + nft_id: 4, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 4, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 192, + 110, + 49, + 217, + 16, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 4, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 4, + owner: 99, + shares: 300000000000000, + }, + ), + RuntimeEvent::System( + Event::NewAccount { + account: 16637257129592320098, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 99, + to: 16637257129592320098, + amount: 300000000000000, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 16637257129592320098, + to: 2, + amount: 100000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 3, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertiesRemoved { + collection_id: 10000, + maybe_nft_id: Some( + 3, + ), + }, + ), + RuntimeEvent::Uniques( + Event::Burned { + collection: 10000, + item: 3, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NFTBurned { + owner: 7813586407040180578, + collection_id: 10000, + nft_id: 3, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertyRemoved { + collection_id: 10000, + maybe_nft_id: Some( + 3, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 0, + user: 2, + amount: 100000000000000, + shares: 100000000000000, + burnt_shares: 0, + }, + ), + RuntimeEvent::PhalaStakePoolv2( + Event::Contribution { + pid: 0, + user: 99, + amount: 300000000000000, + shares: 300000000000000, + as_vault: None, + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw3.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw3.snap new file mode 100644 index 0000000000..f7a7237522 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw3.snap @@ -0,0 +1,269 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 5, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10000, + nft_id: 5, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 5, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 128, + 244, + 32, + 230, + 181, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 5, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 5, + owner: 7813586407040180578, + shares: 200000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 0, + user: 1, + shares: 200000000000000, + nft_id: 1, + as_vault: None, + withdrawing_nft_id: 5, + }, + ), + RuntimeEvent::System( + Event::KilledAccount { + account: 16637257129592320098, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 16637257129592320098, + to: 1, + amount: 200000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 5, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertiesRemoved { + collection_id: 10000, + maybe_nft_id: Some( + 5, + ), + }, + ), + RuntimeEvent::Uniques( + Event::Burned { + collection: 10000, + item: 5, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NFTBurned { + owner: 7813586407040180578, + collection_id: 10000, + nft_id: 5, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertyRemoved { + collection_id: 10000, + maybe_nft_id: Some( + 5, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 0, + user: 1, + amount: 200000000000000, + shares: 200000000000000, + burnt_shares: 0, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw4.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw4.snap new file mode 100644 index 0000000000..a36b42de16 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw4.snap @@ -0,0 +1,224 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10001, + item: 2, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10001, + nft_id: 2, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10001, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 128, + 244, + 32, + 230, + 181, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10001, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 1, + cid: 10001, + nft_id: 2, + owner: 7813586407040180578, + shares: 200000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 1, + user: 1, + shares: 200000000000000, + nft_id: 0, + as_vault: None, + withdrawing_nft_id: 2, + }, + ), + RuntimeEvent::System( + Event::KilledAccount { + account: 13009150994509951074, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 13009150994509951074, + to: 1, + amount: 100000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10001, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 1, + user: 1, + amount: 100000000000000, + shares: 100000000000000, + burnt_shares: 0, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10001, + maybe_nft_id: Some( + 0, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw5.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw5.snap new file mode 100644 index 0000000000..fb389dba67 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw5.snap @@ -0,0 +1,264 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 7, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10000, + nft_id: 7, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 7, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 128, + 244, + 32, + 230, + 181, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 7, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 7, + owner: 7813586407040180578, + shares: 200000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 0, + user: 3, + shares: 200000000000000, + nft_id: 2, + as_vault: None, + withdrawing_nft_id: 7, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 16637257129592320098, + to: 3, + amount: 200000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 7, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertiesRemoved { + collection_id: 10000, + maybe_nft_id: Some( + 7, + ), + }, + ), + RuntimeEvent::Uniques( + Event::Burned { + collection: 10000, + item: 7, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NFTBurned { + owner: 7813586407040180578, + collection_id: 10000, + nft_id: 7, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertyRemoved { + collection_id: 10000, + maybe_nft_id: Some( + 7, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 0, + user: 3, + amount: 200000000000000, + shares: 200000000000000, + burnt_shares: 0, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw6.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw6.snap new file mode 100644 index 0000000000..711f36b96a --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw6.snap @@ -0,0 +1,231 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 8, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10000, + nft_id: 8, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 8, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 233, + 65, + 204, + 107, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 8, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 8, + owner: 7813586407040180578, + shares: 400000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 0, + user: 13009150994509951074, + shares: 400000000000000, + nft_id: 6, + as_vault: Some( + 1, + ), + withdrawing_nft_id: 8, + }, + ), + RuntimeEvent::System( + Event::KilledAccount { + account: 16637257129592320098, + }, + ), + RuntimeEvent::System( + Event::NewAccount { + account: 13009150994509951074, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 16637257129592320098, + to: 13009150994509951074, + amount: 300000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 8, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 0, + user: 13009150994509951074, + amount: 300000000000000, + shares: 300000000000000, + burnt_shares: 0, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 6, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw7.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw7.snap new file mode 100644 index 0000000000..8687e93b73 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw7.snap @@ -0,0 +1,212 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10002, + item: 1, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10002, + nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 160, + 134, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 2, + cid: 10002, + nft_id: 1, + owner: 7813586407040180578, + shares: 100000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 2, + user: 99, + shares: 100000, + nft_id: 0, + as_vault: None, + withdrawing_nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertiesRemoved { + collection_id: 10002, + maybe_nft_id: Some( + 1, + ), + }, + ), + RuntimeEvent::Uniques( + Event::Burned { + collection: 10002, + item: 1, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NFTBurned { + owner: 7813586407040180578, + collection_id: 10002, + nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertyRemoved { + collection_id: 10002, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 2, + user: 99, + amount: 0, + shares: 0, + burnt_shares: 100000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 0, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 192, + 110, + 49, + 217, + 16, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap new file mode 100644 index 0000000000..3b9e4d7941 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap @@ -0,0 +1,224 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10002, + item: 2, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10002, + nft_id: 2, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 192, + 110, + 49, + 217, + 16, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 48, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 2, + cid: 10002, + nft_id: 2, + owner: 7813586407040180578, + shares: 300000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 2, + user: 99, + shares: 300000000000000, + nft_id: 0, + as_vault: None, + withdrawing_nft_id: 2, + }, + ), + RuntimeEvent::System( + Event::KilledAccount { + account: 615996557710291042, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 615996557710291042, + to: 99, + amount: 10000099999, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 100, + 85, + 97, + 221, + 214, + 16, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 2, + user: 99, + amount: 10000099999, + shares: 10000099996, + burnt_shares: 0, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10002, + maybe_nft_id: Some( + 0, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw9.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw9.snap new file mode 100644 index 0000000000..243634ce4c --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw9.snap @@ -0,0 +1,112 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 13009150994509951074, + to: 1, + amount: 100000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10001, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertiesRemoved { + collection_id: 10001, + maybe_nft_id: Some( + 2, + ), + }, + ), + RuntimeEvent::Uniques( + Event::Burned { + collection: 10001, + item: 2, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NFTBurned { + owner: 7813586407040180578, + collection_id: 10001, + nft_id: 2, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertyRemoved { + collection_id: 10001, + maybe_nft_id: Some( + 2, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 1, + user: 1, + amount: 100000000000000, + shares: 100000000000000, + burnt_shares: 0, + }, + ), +] diff --git a/pallets/phala/src/test.rs b/pallets/phala/src/test.rs index 4afb5dff0b..6072a786e6 100644 --- a/pallets/phala/src/test.rs +++ b/pallets/phala/src/test.rs @@ -1680,6 +1680,7 @@ fn test_withdraw() { set_block_1(); setup_workers(2); setup_stake_pool_with_workers(1, &[1, 2]); // pid = 0 + // Contribute 300 x3 and use 700 to compute assert_ok!(PhalaStakePoolv2::contribute( RuntimeOrigin::signed(2), 0, @@ -1710,12 +1711,15 @@ fn test_withdraw() { worker_pubkey(2), 300 * DOLLARS )); + // Partial withdraw 300 PHA. 200 withdrawn and 100 left in the queue. + let _ = take_events(); assert_ok!(PhalaStakePoolv2::withdraw( RuntimeOrigin::signed(2), 0, 300 * DOLLARS, None )); + insta::assert_debug_snapshot!("withdraw1", take_events()); let pool = ensure_stake_pool::(0).unwrap(); let item = pool .basepool @@ -1731,76 +1735,50 @@ fn test_withdraw() { .clone(); assert_eq!(nft_attr.shares, 100 * DOLLARS); } - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10000).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10000, x).unwrap(); - nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(2) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(pool.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 0); - } + // Check account2 has no share in its NFT + assert_user_has_share(10000, 2, 0); assert_eq!(get_balance(2), 400 * DOLLARS); + // Add another 300 PHA, expect 100 PHA queued withdrawal being fulfilled. + let _ = take_events(); assert_ok!(PhalaStakePoolv2::contribute( RuntimeOrigin::signed(99), 0, 300 * DOLLARS, None )); + insta::assert_debug_snapshot!("withdraw2", take_events()); let pool = ensure_stake_pool::(0).unwrap(); assert_eq!(pool.basepool.withdraw_queue.len(), 0); assert_eq!(get_balance(2), 500 * DOLLARS); - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10000).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10000, x).unwrap(); - nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(2) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(pool.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 0); - } + // Account 2 has no share in the NFT + assert_user_has_share(10000, 2, 0); + // Account 1 can withdraw 200 PHA with free stake + let _ = take_events(); assert_ok!(PhalaStakePoolv2::withdraw( RuntimeOrigin::signed(1), 0, 200 * DOLLARS, None )); + insta::assert_debug_snapshot!("withdraw3", take_events()); let pool = ensure_stake_pool::(0).unwrap(); assert_eq!(pool.basepool.withdraw_queue.len(), 0); assert_eq!(get_balance(1), 400 * DOLLARS); - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10000).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10000, x).unwrap(); - nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(1) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(pool.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 100 * DOLLARS); - } - let _pid = setup_vault(99); + // Account 1 has 100 shares left + assert_user_has_share(10000, 1, 100 * DOLLARS); + // Vault 1 tests case: + // - 300 x2 PHA contribution to vault1 + // - 500 from vault1 to sp0 + let vault1 = setup_vault(99); + assert_eq!(vault1, 1); assert_ok!(PhalaVault::contribute( RuntimeOrigin::signed(1), - 1, + vault1, 300 * DOLLARS, )); assert_ok!(PhalaVault::contribute( RuntimeOrigin::signed(99), - 1, + vault1, 300 * DOLLARS, )); assert_ok!(PhalaStakePoolv2::contribute( @@ -1809,12 +1787,15 @@ fn test_withdraw() { 500 * DOLLARS, Some(1) )); + // Account1 can withdraw 100 now and left 100 in the queue from vault1 + let _ = take_events(); assert_ok!(PhalaVault::withdraw( RuntimeOrigin::signed(1), - 1, + vault1, 200 * DOLLARS, )); - let pool = ensure_vault::(1).unwrap(); + insta::assert_debug_snapshot!("withdraw4", take_events()); + let pool = ensure_vault::(vault1).unwrap(); let item = pool .basepool .withdraw_queue @@ -1829,37 +1810,30 @@ fn test_withdraw() { .clone(); assert_eq!(nft_attr.shares, 100 * DOLLARS); } - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10001).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10001, x).unwrap(); - nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(1) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(pool.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 100 * DOLLARS); - } + assert_user_has_share(10001, 1, 100 * DOLLARS); assert_eq!(get_balance(1), 200 * DOLLARS); + // Account3 withdraw 200 with 500 contribution from vault1 + let _ = take_events(); assert_ok!(PhalaStakePoolv2::withdraw( RuntimeOrigin::signed(3), 0, 200 * DOLLARS, None )); + insta::assert_debug_snapshot!("withdraw5", take_events()); let pool = ensure_stake_pool::(0).unwrap(); assert_eq!(get_balance(pool.basepool.pool_account_id), 300 * DOLLARS); + // Vault1 withdraw 400, only get 300, left 100 in the queue and 100 in share + let _ = take_events(); assert_ok!(PhalaStakePoolv2::withdraw( RuntimeOrigin::signed(99), 0, 400 * DOLLARS, Some(1) )); + insta::assert_debug_snapshot!("withdraw6", take_events()); let pool = ensure_stake_pool::(0).unwrap(); - let vault = ensure_vault::(1).unwrap(); + let vault = ensure_vault::(vault1).unwrap(); let item = pool .basepool .withdraw_queue @@ -1874,61 +1848,44 @@ fn test_withdraw() { .clone(); assert_eq!(nft_attr.shares, 100 * DOLLARS); } - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10000).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10000, x).unwrap(); - nft.owner - == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId( - vault.basepool.pool_account_id, - ) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(pool.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 100 * DOLLARS); - } + assert_user_has_share(10000, vault.basepool.pool_account_id, 100 * DOLLARS); assert_eq!(get_balance(vault.basepool.pool_account_id), 300 * DOLLARS); - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10001).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10001, x).unwrap(); - nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(1) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(vault.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 100 * DOLLARS); - } - let _pid = setup_vault(99); + // Vault 2 test case: + // - Account99 contribute (300 + eps), eps = 1e5 pico PHA + let vault2 = setup_vault(99); assert_ok!(PhalaVault::contribute( RuntimeOrigin::signed(99), - 2, + vault2, 300000000100000, )); - assert_ok!(PhalaVault::withdraw(RuntimeOrigin::signed(99), 2, 100000,)); + // Account99 withdraw eps from vault2 + // Note: 100000 shares were burnt because the amount to withdraw is smaller than WPHA min. + let _ = take_events(); + assert_ok!(PhalaVault::withdraw(RuntimeOrigin::signed(99), vault2, 100000)); + insta::assert_debug_snapshot!("withdraw7", take_events()); + // Vault2 can contribute 299.99 to pool0 assert_ok!(PhalaStakePoolv2::contribute( RuntimeOrigin::signed(99), 0, 299990000000000, Some(2) )); + // Account 99 withdraw 300 PHA from vault2 + // ~(0.01 + eps) PHA can be withdrawn, leaving the rest in the queue + let _ = take_events(); assert_ok!(PhalaVault::withdraw( RuntimeOrigin::signed(99), - 2, + vault2, 300 * DOLLARS, )); - let pool = ensure_vault::(1).unwrap(); + insta::assert_debug_snapshot!("withdraw8", take_events()); + // With 299.99 contribution from vault2, vault1 can clear its queue + let _ = take_events(); assert_ok!(PhalaVault::check_and_maybe_force_withdraw( RuntimeOrigin::signed(4), - pool.basepool.pid + vault1, )); + insta::assert_debug_snapshot!("withdraw9", take_events()); }); } @@ -2179,3 +2136,19 @@ fn set_balance_proposal(value: u128) -> BoundedCallOf { let outer = RuntimeCall::Balances(inner); Preimage::bound(outer).unwrap() } + +fn assert_user_has_share(cid: u32, owner: u64, shares: u128) { + let mut nftid_arr: Vec = pallet_rmrk_core::Nfts::::iter_key_prefix(cid).collect(); + nftid_arr.retain(|x| { + let nft = pallet_rmrk_core::Nfts::::get(cid, x).unwrap(); + nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(owner) + }); + assert_eq!(nftid_arr.len(), 1, "owner not found"); + assert_eq!( + PhalaBasePool::get_nft_attr_guard(cid, nftid_arr[0]) + .unwrap() + .attr + .shares, + shares + ); +} From 26fd290307a8cf3accfa3de18823bf407ea9a009 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Sun, 22 Oct 2023 09:58:51 +0000 Subject: [PATCH 3/7] cargo fmt --- pallets/phala/src/mock.rs | 8 ++++---- pallets/phala/src/test.rs | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pallets/phala/src/mock.rs b/pallets/phala/src/mock.rs index dcb3762280..8fd60e9d8e 100644 --- a/pallets/phala/src/mock.rs +++ b/pallets/phala/src/mock.rs @@ -10,9 +10,7 @@ use frame_support::{ ord_parameter_types, pallet_prelude::ConstU32, parameter_types, - traits::{ - AsEnsureOriginWithArg, ConstU128, ConstU64, EqualPrivilegeOnly, SortedMembers, - }, + traits::{AsEnsureOriginWithArg, ConstU128, ConstU64, EqualPrivilegeOnly, SortedMembers}, }; use frame_support_test::TestRandomness; use frame_system::{self as system, EnsureRoot, EnsureSigned, EnsureSignedBy}; @@ -426,7 +424,9 @@ impl AttestationValidator for MockValidator { // This function basically just builds a genesis storage key/value store according to // our desired mockup. pub fn new_test_ext() -> sp_io::TestExternalities { - let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + let mut t = frame_system::GenesisConfig::::default() + .build_storage() + .unwrap(); // Inject genesis storage let zero_pubkey = sp_core::sr25519::Public::from_raw([0u8; 32]); diff --git a/pallets/phala/src/test.rs b/pallets/phala/src/test.rs index 6072a786e6..0ba1b1bc28 100644 --- a/pallets/phala/src/test.rs +++ b/pallets/phala/src/test.rs @@ -1680,6 +1680,7 @@ fn test_withdraw() { set_block_1(); setup_workers(2); setup_stake_pool_with_workers(1, &[1, 2]); // pid = 0 + // Contribute 300 x3 and use 700 to compute assert_ok!(PhalaStakePoolv2::contribute( RuntimeOrigin::signed(2), @@ -1861,7 +1862,11 @@ fn test_withdraw() { // Account99 withdraw eps from vault2 // Note: 100000 shares were burnt because the amount to withdraw is smaller than WPHA min. let _ = take_events(); - assert_ok!(PhalaVault::withdraw(RuntimeOrigin::signed(99), vault2, 100000)); + assert_ok!(PhalaVault::withdraw( + RuntimeOrigin::signed(99), + vault2, + 100000 + )); insta::assert_debug_snapshot!("withdraw7", take_events()); // Vault2 can contribute 299.99 to pool0 assert_ok!(PhalaStakePoolv2::contribute( From 24c0ad5b5a06590984cd2332da183614da1a9fd6 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Sun, 22 Oct 2023 16:29:24 +0000 Subject: [PATCH 4/7] compute: improve vault force withdraw - Add ForceShutdown event with shutdown reason - Allow partial fill of force withdraw - Fallsafe shutdown for withdrawal > 3x grace period --- pallets/phala/src/compute/vault.rs | 166 ++++++++++------- ..._force_withdraw_after_3x_grace_period.snap | 172 ++++++++++++++++++ ..._test__vault_partial_force_withdraw-2.snap | 72 ++++++++ ...s__test__vault_partial_force_withdraw.snap | 171 +++++++++++++++++ pallets/phala/src/test.rs | 134 ++++++++++++-- 5 files changed, 637 insertions(+), 78 deletions(-) create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__vault_force_withdraw_after_3x_grace_period.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw-2.snap create mode 100644 pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw.snap diff --git a/pallets/phala/src/compute/vault.rs b/pallets/phala/src/compute/vault.rs index 47c1b0b689..ea37bc6125 100644 --- a/pallets/phala/src/compute/vault.rs +++ b/pallets/phala/src/compute/vault.rs @@ -116,6 +116,16 @@ pub mod pallet { amount: BalanceOf, shares: BalanceOf, }, + ForceShutdown { + pid: u64, + reason: ForceShutdownReason, + } + } + + #[derive(PartialEq, Eq, Clone, Debug, Encode, Decode, scale_info::TypeInfo)] + pub enum ForceShutdownReason { + NoEnoughReleasingStake, + Waiting3xGracePeriod, } #[pallet::error] @@ -340,7 +350,6 @@ pub mod pallet { /// /// If the shutdown condition is met, all shares owned by the vault will be forced withdraw. /// Note: This function doesn't guarantee no-op when there's error. - /// TODO(mingxuan): add more detail comment later. #[pallet::call_index(4)] #[pallet::weight({0})] #[frame_support::transactional] @@ -349,48 +358,37 @@ pub mod pallet { vault_pid: u64, ) -> DispatchResult { ensure_signed(origin.clone())?; - let now = ::UnixTime::now() - .as_secs() - .saturated_into::(); let mut vault = ensure_vault::(vault_pid)?; + // Try to process withdraw queue with the free token. + // Unlock the vault if the withdrawals are clear. base_pool::Pallet::::try_process_withdraw_queue(&mut vault.basepool); - let grace_period = T::GracePeriod::get(); - let mut releasing_stake = Zero::zero(); - for pid in vault.invest_pools.iter() { - let stake_pool = ensure_stake_pool::(*pid)?; - let withdraw_vec: VecDeque<_> = stake_pool - .basepool - .withdraw_queue - .iter() - .filter(|x| x.user == vault.basepool.pool_account_id) - .collect(); - // the length of vec should be 1 - for withdraw in withdraw_vec { - let nft_guard = base_pool::Pallet::::get_nft_attr_guard( - stake_pool.basepool.cid, - withdraw.nft_id, - )?; - let price = stake_pool - .basepool - .share_price() - .ok_or(Error::::VaultBankrupt)?; - releasing_stake += bmul(nft_guard.attr.shares, &price); - } - } if vault.basepool.withdraw_queue.is_empty() { VaultLocks::::remove(vault_pid); } base_pool::pallet::Pools::::insert(vault_pid, PoolProxy::Vault(vault.clone())); - if base_pool::Pallet::::has_expired_withdrawal( - &vault.basepool, - now, - grace_period, - Some(T::VaultQueuePeriod::get()), - releasing_stake, - ) { + + // Trigger force withdrawal and lock if there's any expired withdrawal. + // If already locked, we don't need to trigger it again. + if VaultLocks::::contains_key(vault_pid) { + return Ok(()) + } + // Case 1: There's a request over 3x GracePeriod old, the pool must be shutdown. + let mut shutdown_reason: Option = None; + let now = ::UnixTime::now() + .as_secs() + .saturated_into::(); + let grace_period = T::GracePeriod::get(); + if let Some(withdraw) = vault.basepool.withdraw_queue.front() { + if withdraw.start_time + 3 * grace_period < now { + shutdown_reason = Some(ForceShutdownReason::Waiting3xGracePeriod); + } + } + // Case 2: There's no enough releasing stake to address the expired withdrawals. + // Sum up the releasing stake + if shutdown_reason.is_none() { + let mut releasing_stake = Zero::zero(); for pid in vault.invest_pools.iter() { let stake_pool = ensure_stake_pool::(*pid)?; - let mut total_shares = Zero::zero(); let withdraw_vec: VecDeque<_> = stake_pool .basepool .withdraw_queue @@ -403,42 +401,82 @@ pub mod pallet { stake_pool.basepool.cid, withdraw.nft_id, )?; - total_shares += nft_guard.attr.shares; + let price = stake_pool + .basepool + .share_price() + .ok_or(Error::::VaultBankrupt)?; + releasing_stake += bmul(nft_guard.attr.shares, &price); } - pallet_uniques::Pallet::::owned_in_collection( - &stake_pool.basepool.cid, - &vault.basepool.pool_account_id, + } + // Check if the releasing stake can cover the expired withdrawals + if base_pool::Pallet::::has_expired_withdrawal( + &vault.basepool, + now, + grace_period, + Some(T::VaultQueuePeriod::get()), + releasing_stake, + ) { + shutdown_reason = Some(ForceShutdownReason::NoEnoughReleasingStake); + } + } + let Some(shutdown_reason) = shutdown_reason else { + // No need to shutdown. + return Ok(()) + }; + // Try to withdraw from the upstream stake pools + for pid in vault.invest_pools.iter() { + let stake_pool = ensure_stake_pool::(*pid)?; + let mut total_shares = Zero::zero(); + let withdraw_vec: VecDeque<_> = stake_pool + .basepool + .withdraw_queue + .iter() + .filter(|x| x.user == vault.basepool.pool_account_id) + .collect(); + // the length of vec should be 1 + for withdraw in withdraw_vec { + let nft_guard = base_pool::Pallet::::get_nft_attr_guard( + stake_pool.basepool.cid, + withdraw.nft_id, + )?; + total_shares += nft_guard.attr.shares; + } + pallet_uniques::Pallet::::owned_in_collection( + &stake_pool.basepool.cid, + &vault.basepool.pool_account_id, + ) + .for_each(|nftid| { + let property_guard = base_pool::Pallet::::get_nft_attr_guard( + stake_pool.basepool.cid, + nftid, ) - .for_each(|nftid| { - let property_guard = base_pool::Pallet::::get_nft_attr_guard( + .expect("get nft should not fail: qed."); + let property = &property_guard.attr; + if !base_pool::is_nondust_balance(property.shares) { + let _ = base_pool::Pallet::::burn_nft( + &base_pool::pallet_id::(), stake_pool.basepool.cid, nftid, - ) - .expect("get nft should not fail: qed."); - let property = &property_guard.attr; - if !base_pool::is_nondust_balance(property.shares) { - let _ = base_pool::Pallet::::burn_nft( - &base_pool::pallet_id::(), - stake_pool.basepool.cid, - nftid, - ); - return; - } - total_shares += property.shares; - }); - if !base_pool::is_nondust_balance(total_shares) { - continue; + ); + return; } - stake_pool_v2::Pallet::::withdraw( - Origin::::Signed(vault.basepool.owner.clone()).into(), - stake_pool.basepool.pid, - total_shares, - Some(vault_pid), - )?; + total_shares += property.shares; + }); + if !base_pool::is_nondust_balance(total_shares) { + continue; } - VaultLocks::::insert(vault_pid, ()); + stake_pool_v2::Pallet::::withdraw( + Origin::::Signed(vault.basepool.owner.clone()).into(), + stake_pool.basepool.pid, + total_shares, + Some(vault_pid), + )?; } - + VaultLocks::::insert(vault_pid, ()); + Self::deposit_event(Event::::ForceShutdown { + pid: vault_pid, + reason: shutdown_reason, + }); Ok(()) } diff --git a/pallets/phala/src/snapshots/phala_pallets__test__vault_force_withdraw_after_3x_grace_period.snap b/pallets/phala/src/snapshots/phala_pallets__test__vault_force_withdraw_after_3x_grace_period.snap new file mode 100644 index 0000000000..4bb34e10ac --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__vault_force_withdraw_after_3x_grace_period.snap @@ -0,0 +1,172 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 1, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10000, + nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 128, + 244, + 32, + 230, + 181, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 49, + 56, + 49, + 52, + 52, + 48, + 49, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 1, + owner: 7813586407040180578, + shares: 200000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 0, + user: 13009150994509951074, + shares: 200000000000000, + nft_id: 0, + as_vault: Some( + 1, + ), + withdrawing_nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 0, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaVault( + Event::ForceShutdown { + pid: 1, + reason: Waiting3xGracePeriod, + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw-2.snap b/pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw-2.snap new file mode 100644 index 0000000000..8a881c9c79 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw-2.snap @@ -0,0 +1,72 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::System( + Event::KilledAccount { + account: 13009150994509951074, + }, + ), + RuntimeEvent::Assets( + Event::Transferred { + asset_id: 1, + from: 13009150994509951074, + to: 1, + amount: 100000000000000, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10001, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 64, + 122, + 16, + 243, + 90, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::Withdrawal { + pid: 1, + user: 1, + amount: 100000000000000, + shares: 100000000000000, + burnt_shares: 0, + }, + ), +] diff --git a/pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw.snap b/pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw.snap new file mode 100644 index 0000000000..4eac2d7063 --- /dev/null +++ b/pallets/phala/src/snapshots/phala_pallets__test__vault_partial_force_withdraw.snap @@ -0,0 +1,171 @@ +--- +source: pallets/phala/src/test.rs +expression: take_events() +--- +[ + RuntimeEvent::Uniques( + Event::Issued { + collection: 10000, + item: 1, + owner: 7813586407040180578, + }, + ), + RuntimeEvent::RmrkCore( + Event::NftMinted { + owner: AccountId( + 7813586407040180578, + ), + collection_id: 10000, + nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 128, + 244, + 32, + 230, + 181, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 1, + ), + key: BoundedVec( + [ + 99, + 114, + 101, + 97, + 116, + 101, + 116, + 105, + 109, + 101, + ], + 32000, + ), + value: BoundedVec( + [ + 54, + 48, + 52, + 56, + 48, + 49, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaBasePool( + Event::NftCreated { + pid: 0, + cid: 10000, + nft_id: 1, + owner: 7813586407040180578, + shares: 200000000000000, + }, + ), + RuntimeEvent::PhalaBasePool( + Event::WithdrawalQueued { + pid: 0, + user: 13009150994509951074, + shares: 200000000000000, + nft_id: 0, + as_vault: Some( + 1, + ), + withdrawing_nft_id: 1, + }, + ), + RuntimeEvent::RmrkCore( + Event::PropertySet { + collection_id: 10000, + maybe_nft_id: Some( + 0, + ), + key: BoundedVec( + [ + 115, + 116, + 97, + 107, + 101, + 45, + 105, + 110, + 102, + 111, + ], + 32000, + ), + value: BoundedVec( + [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + 512000, + ), + }, + ), + RuntimeEvent::PhalaVault( + Event::ForceShutdown { + pid: 1, + reason: NoEnoughReleasingStake, + }, + ), +] diff --git a/pallets/phala/src/test.rs b/pallets/phala/src/test.rs index 0ba1b1bc28..543a0699ee 100644 --- a/pallets/phala/src/test.rs +++ b/pallets/phala/src/test.rs @@ -1972,20 +1972,7 @@ fn test_check_and_maybe_force_withdraw() { .clone(); assert_eq!(nft_attr.shares, 100 * DOLLARS); } - let mut nftid_arr: Vec = - pallet_rmrk_core::Nfts::::iter_key_prefix(10000).collect(); - nftid_arr.retain(|x| { - let nft = pallet_rmrk_core::Nfts::::get(10000, x).unwrap(); - nft.owner == rmrk_traits::AccountIdOrCollectionNftTuple::AccountId(2) - }); - assert_eq!(nftid_arr.len(), 1); - { - let user_nft_attr = PhalaBasePool::get_nft_attr_guard(pool.basepool.cid, nftid_arr[0]) - .unwrap() - .attr - .clone(); - assert_eq!(user_nft_attr.shares, 0); - } + assert_user_has_share(10000, 2, 0); assert_eq!(get_balance(2), 400 * DOLLARS); assert_ok!(PhalaStakePoolv2::check_and_maybe_force_withdraw( RuntimeOrigin::signed(3), @@ -2097,6 +2084,125 @@ fn test_check_and_maybe_force_withdraw() { }); } +#[test] +fn vault_partial_force_withdraw() { + new_test_ext().execute_with(|| { + mock_asset_id(); + assert_ok!(PhalaWrappedBalances::wrap( + RuntimeOrigin::signed(1), + 500 * DOLLARS + )); + set_block_1(); + setup_workers(2); + setup_stake_pool_with_workers(1, &[1, 2]); // pid = 0 + let vault1 = setup_vault(99); + // - Account1 contribute 200 to vault1 + // - Vault1 contribute 200 to pool0 + // - Pool0 start worker1 and worker 2 with each 100 PHA + assert_ok!(PhalaVault::contribute( + RuntimeOrigin::signed(1), + 1, + 200 * DOLLARS, + )); + assert_ok!(PhalaStakePoolv2::contribute( + RuntimeOrigin::signed(99), + 0, + 200 * DOLLARS, + Some(vault1) + )); + assert_ok!(PhalaStakePoolv2::start_computing( + RuntimeOrigin::signed(1), + 0, + worker_pubkey(1), + 100 * DOLLARS + )); + assert_ok!(PhalaStakePoolv2::start_computing( + RuntimeOrigin::signed(1), + 0, + worker_pubkey(2), + 100 * DOLLARS + )); + // Trigger force withdraw (7d + 1s) with NoEnoughReleasingStake + assert_ok!(PhalaVault::withdraw( + RuntimeOrigin::signed(1), + 1, + 200 * DOLLARS, + )); + elapse_cool_down(); + elapse_seconds(1); + let _ = take_events(); + assert_ok!(PhalaVault::check_and_maybe_force_withdraw(RuntimeOrigin::signed(1), 1)); + insta::assert_debug_snapshot!(take_events()); + assert!(vault::VaultLocks::::contains_key(vault1)); + // Pool0 stop first worker first + assert_ok!(PhalaStakePoolv2::stop_computing( + RuntimeOrigin::signed(1), + 0, + worker_pubkey(1), + )); + elapse_cool_down(); + assert_ok!(PhalaStakePoolv2::reclaim_pool_worker( + RuntimeOrigin::signed(1), + 0, + worker_pubkey(1), + )); + // Partial fill 100 PHA (out of 200) + let _ = take_events(); + assert_ok!(PhalaVault::check_and_maybe_force_withdraw(RuntimeOrigin::signed(1), 1)); + insta::assert_debug_snapshot!(take_events()); + assert!(vault::VaultLocks::::contains_key(vault1)); + }); +} + +#[test] +fn vault_force_withdraw_after_3x_grace_period() { + new_test_ext().execute_with(|| { + mock_asset_id(); + assert_ok!(PhalaWrappedBalances::wrap( + RuntimeOrigin::signed(1), + 500 * DOLLARS + )); + set_block_1(); + setup_workers(1); + setup_stake_pool_with_workers(1, &[1]); // pid = 0 + let vault1 = setup_vault(99); + // - Account1 contribute 200 to vault1 + // - Vault1 contribute 200 to pool0 + // - Pool0 start worker1 with 200 PHA + assert_ok!(PhalaVault::contribute( + RuntimeOrigin::signed(1), + 1, + 200 * DOLLARS, + )); + assert_ok!(PhalaStakePoolv2::contribute( + RuntimeOrigin::signed(99), + 0, + 200 * DOLLARS, + Some(vault1) + )); + assert_ok!(PhalaStakePoolv2::start_computing( + RuntimeOrigin::signed(1), + 0, + worker_pubkey(1), + 200 * DOLLARS + )); + // Trigger force withdraw (21d + 1s) with Waiting3xGracePeriod + assert_ok!(PhalaVault::withdraw( + RuntimeOrigin::signed(1), + 1, + 200 * DOLLARS, + )); + elapse_cool_down(); + elapse_cool_down(); + elapse_cool_down(); + elapse_seconds(1); + let _ = take_events(); + assert_ok!(PhalaVault::check_and_maybe_force_withdraw(RuntimeOrigin::signed(1), 1)); + insta::assert_debug_snapshot!(take_events()); + assert!(vault::VaultLocks::::contains_key(vault1)); + }); +} + fn mock_asset_id() { as Create>::create( ::WPhaAssetId::get(), From ec514b28ce7300c04824b0ed5bf18376261ea1b0 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Sun, 22 Oct 2023 16:34:59 +0000 Subject: [PATCH 5/7] cargo fmt --- pallets/phala/src/compute/vault.rs | 14 ++++++-------- pallets/phala/src/test.rs | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/pallets/phala/src/compute/vault.rs b/pallets/phala/src/compute/vault.rs index ea37bc6125..8e077636ea 100644 --- a/pallets/phala/src/compute/vault.rs +++ b/pallets/phala/src/compute/vault.rs @@ -119,7 +119,7 @@ pub mod pallet { ForceShutdown { pid: u64, reason: ForceShutdownReason, - } + }, } #[derive(PartialEq, Eq, Clone, Debug, Encode, Decode, scale_info::TypeInfo)] @@ -370,7 +370,7 @@ pub mod pallet { // Trigger force withdrawal and lock if there's any expired withdrawal. // If already locked, we don't need to trigger it again. if VaultLocks::::contains_key(vault_pid) { - return Ok(()) + return Ok(()); } // Case 1: There's a request over 3x GracePeriod old, the pool must be shutdown. let mut shutdown_reason: Option = None; @@ -421,7 +421,7 @@ pub mod pallet { } let Some(shutdown_reason) = shutdown_reason else { // No need to shutdown. - return Ok(()) + return Ok(()); }; // Try to withdraw from the upstream stake pools for pid in vault.invest_pools.iter() { @@ -446,11 +446,9 @@ pub mod pallet { &vault.basepool.pool_account_id, ) .for_each(|nftid| { - let property_guard = base_pool::Pallet::::get_nft_attr_guard( - stake_pool.basepool.cid, - nftid, - ) - .expect("get nft should not fail: qed."); + let property_guard = + base_pool::Pallet::::get_nft_attr_guard(stake_pool.basepool.cid, nftid) + .expect("get nft should not fail: qed."); let property = &property_guard.attr; if !base_pool::is_nondust_balance(property.shares) { let _ = base_pool::Pallet::::burn_nft( diff --git a/pallets/phala/src/test.rs b/pallets/phala/src/test.rs index 543a0699ee..57d71f89d4 100644 --- a/pallets/phala/src/test.rs +++ b/pallets/phala/src/test.rs @@ -2131,7 +2131,10 @@ fn vault_partial_force_withdraw() { elapse_cool_down(); elapse_seconds(1); let _ = take_events(); - assert_ok!(PhalaVault::check_and_maybe_force_withdraw(RuntimeOrigin::signed(1), 1)); + assert_ok!(PhalaVault::check_and_maybe_force_withdraw( + RuntimeOrigin::signed(1), + 1 + )); insta::assert_debug_snapshot!(take_events()); assert!(vault::VaultLocks::::contains_key(vault1)); // Pool0 stop first worker first @@ -2148,7 +2151,10 @@ fn vault_partial_force_withdraw() { )); // Partial fill 100 PHA (out of 200) let _ = take_events(); - assert_ok!(PhalaVault::check_and_maybe_force_withdraw(RuntimeOrigin::signed(1), 1)); + assert_ok!(PhalaVault::check_and_maybe_force_withdraw( + RuntimeOrigin::signed(1), + 1 + )); insta::assert_debug_snapshot!(take_events()); assert!(vault::VaultLocks::::contains_key(vault1)); }); @@ -2197,7 +2203,10 @@ fn vault_force_withdraw_after_3x_grace_period() { elapse_cool_down(); elapse_seconds(1); let _ = take_events(); - assert_ok!(PhalaVault::check_and_maybe_force_withdraw(RuntimeOrigin::signed(1), 1)); + assert_ok!(PhalaVault::check_and_maybe_force_withdraw( + RuntimeOrigin::signed(1), + 1 + )); insta::assert_debug_snapshot!(take_events()); assert!(vault::VaultLocks::::contains_key(vault1)); }); From f8629041ad1062233178572e24fd34054ed0af34 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Fri, 27 Oct 2023 02:29:12 +0000 Subject: [PATCH 6/7] compute: settle vault reward when necessary --- pallets/phala/src/compute/vault.rs | 92 +++++++++++++++++++----------- pallets/phala/src/test.rs | 79 +++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 34 deletions(-) diff --git a/pallets/phala/src/compute/vault.rs b/pallets/phala/src/compute/vault.rs index 8e077636ea..df69215b37 100644 --- a/pallets/phala/src/compute/vault.rs +++ b/pallets/phala/src/compute/vault.rs @@ -309,41 +309,12 @@ pub mod pallet { #[pallet::weight({0})] pub fn maybe_gain_owner_shares(origin: OriginFor, vault_pid: u64) -> DispatchResult { let who = ensure_signed(origin)?; - let mut pool_info = ensure_vault::(vault_pid)?; - // Add pool owner's reward if applicable + let pool_info = ensure_vault::(vault_pid)?; ensure!( who == pool_info.basepool.owner, Error::::UnauthorizedPoolOwner ); - let current_price = match pool_info.basepool.share_price() { - Some(price) => BalanceOf::::from_fixed(&price), - None => return Ok(()), - }; - if pool_info.last_share_price_checkpoint == Zero::zero() { - pool_info.last_share_price_checkpoint = current_price; - base_pool::pallet::Pools::::insert(vault_pid, PoolProxy::Vault(pool_info)); - return Ok(()); - } - if current_price <= pool_info.last_share_price_checkpoint { - return Ok(()); - } - let delta_price = pool_info.commission.unwrap_or_default() - * (current_price - pool_info.last_share_price_checkpoint); - let new_price = current_price - delta_price; - let adjust_shares = bdiv(pool_info.basepool.total_value, &new_price.to_fixed()) - - pool_info.basepool.total_shares; - pool_info.basepool.total_shares += adjust_shares; - pool_info.owner_shares += adjust_shares; - pool_info.last_share_price_checkpoint = new_price; - - base_pool::pallet::Pools::::insert(vault_pid, PoolProxy::Vault(pool_info)); - Self::deposit_event(Event::::OwnerSharesGained { - pid: vault_pid, - shares: adjust_shares, - checkout_price: new_price, - }); - - Ok(()) + Self::do_gain_owner_share(vault_pid) } /// Let any user to launch a vault withdraw. Then check if the vault need to be forced withdraw all its contributions. @@ -488,9 +459,7 @@ pub mod pallet { #[frame_support::transactional] pub fn contribute(origin: OriginFor, pid: u64, amount: BalanceOf) -> DispatchResult { let who = ensure_signed(origin)?; - let mut pool_info = ensure_vault::(pid)?; let a = amount; // Alias to reduce confusion in the code below - ensure!( a >= T::MinContribution::get(), Error::::InsufficientContribution @@ -502,11 +471,15 @@ pub mod pallet { .ok_or(Error::::AssetAccountNotExist)?; ensure!(free >= a, Error::::InsufficientBalance); + // Trigger owner reward share distribution before contribution to ensure no harm to the + // contributor. + Self::do_gain_owner_share(pid)?; + let mut pool_info = ensure_vault::(pid)?; + let shares = base_pool::Pallet::::contribute(&mut pool_info.basepool, who.clone(), amount)?; // We have new free stake now, try to handle the waiting withdraw queue - base_pool::Pallet::::try_process_withdraw_queue(&mut pool_info.basepool); // Persist @@ -542,7 +515,12 @@ pub mod pallet { #[frame_support::transactional] pub fn withdraw(origin: OriginFor, pid: u64, shares: BalanceOf) -> DispatchResult { let who = ensure_signed(origin)?; + + // Trigger owner reward share distribution before withdrawal to ensure no harm to the + // pool owner. + Self::do_gain_owner_share(pid)?; let mut pool_info = ensure_vault::(pid)?; + let maybe_nft_id = base_pool::Pallet::::merge_nft_for_staker( pool_info.basepool.cid, who.clone(), @@ -605,4 +583,50 @@ pub mod pallet { Self::check_and_maybe_force_withdraw(origin, pid) } } + + impl Pallet + where + BalanceOf: sp_runtime::traits::AtLeast32BitUnsigned + Copy + FixedPointConvert + Display, + T: pallet_rmrk_core::Config, + T: pallet_assets::Config>, + { + /// Triggers owner reward share distribution + /// + /// Note 1: This function does mutate the pool info. After calling this function, the caller + /// must read the pool info again if it's accessed. + /// + /// Note 2: This function guarantees no-op when it returns error. + fn do_gain_owner_share(vault_pid: u64) -> DispatchResult { + let mut pool_info = ensure_vault::(vault_pid)?; + let current_price = match pool_info.basepool.share_price() { + Some(price) => BalanceOf::::from_fixed(&price), + None => return Ok(()), + }; + if pool_info.last_share_price_checkpoint == Zero::zero() { + pool_info.last_share_price_checkpoint = current_price; + base_pool::pallet::Pools::::insert(vault_pid, PoolProxy::Vault(pool_info)); + return Ok(()); + } + if current_price <= pool_info.last_share_price_checkpoint { + return Ok(()); + } + let delta_price = pool_info.commission.unwrap_or_default() + * (current_price - pool_info.last_share_price_checkpoint); + let new_price = current_price - delta_price; + let adjust_shares = bdiv(pool_info.basepool.total_value, &new_price.to_fixed()) + - pool_info.basepool.total_shares; + pool_info.basepool.total_shares += adjust_shares; + pool_info.owner_shares += adjust_shares; + pool_info.last_share_price_checkpoint = new_price; + + base_pool::pallet::Pools::::insert(vault_pid, PoolProxy::Vault(pool_info)); + Self::deposit_event(Event::::OwnerSharesGained { + pid: vault_pid, + shares: adjust_shares, + checkout_price: new_price, + }); + + Ok(()) + } + } } diff --git a/pallets/phala/src/test.rs b/pallets/phala/src/test.rs index 57d71f89d4..f25e8e0f29 100644 --- a/pallets/phala/src/test.rs +++ b/pallets/phala/src/test.rs @@ -2212,6 +2212,85 @@ fn vault_force_withdraw_after_3x_grace_period() { }); } +#[test] +fn vault_owner_reward_settle_when_contribute_withdraw() { + use crate::compute::computation::pallet::OnReward; + new_test_ext().execute_with(|| { + mock_asset_id(); + assert_ok!(PhalaWrappedBalances::wrap( + RuntimeOrigin::signed(1), + 500 * DOLLARS + )); + set_block_1(); + setup_workers(1); + setup_stake_pool_with_workers(1, &[1]); // pid = 0 + let vault1 = setup_vault(99); + assert_ok!(PhalaVault::set_payout_pref( + RuntimeOrigin::signed(99), + vault1, + Some(Permill::from_percent(100)), + )); + assert_ok!(PhalaVault::contribute( + RuntimeOrigin::signed(1), + 1, + 100 * DOLLARS, + )); + assert_ok!(PhalaStakePoolv2::contribute( + RuntimeOrigin::signed(99), + 0, + 100 * DOLLARS, + Some(vault1), + )); + // Checkpoint price = 1 + assert_ok!(PhalaVault::maybe_gain_owner_shares( + RuntimeOrigin::signed(99), + vault1, + )); + let pool0 = ensure_stake_pool::(0).unwrap(); + assert_eq!(pool0.basepool.share_price(), Some(fp!(1))); + let pool1 = ensure_vault::(1).unwrap(); + assert_eq!(pool1.basepool.share_price(), Some(fp!(1))); + assert_eq!(pool1.last_share_price_checkpoint, 1 * DOLLARS); + + // Current price = 2 + PhalaStakePoolv2::on_reward(&[SettleInfo { + pubkey: worker_pubkey(1), + v: FixedPoint::from_num(1u32).to_bits(), + payout: FixedPoint::from_num(100u32).to_bits(), + treasury: 0, + }]); + let pool1 = ensure_vault::(1).unwrap(); + assert_eq!(pool1.basepool.share_price(), Some(fp!(2))); + // Contribution should bring price back to 1 + assert_ok!(PhalaVault::contribute( + RuntimeOrigin::signed(1), + 1, + 100 * DOLLARS, + )); + let pool1 = ensure_vault::(1).unwrap(); + assert_eq!(pool1.basepool.share_price(), Some(fp!(1))); + + // Double the stake pool asset by adding 300 reward + // Current price = 2 again + PhalaStakePoolv2::on_reward(&[SettleInfo { + pubkey: worker_pubkey(1), + v: FixedPoint::from_num(1u32).to_bits(), + payout: FixedPoint::from_num(300u32).to_bits(), + treasury: 0, + }]); + let pool1 = ensure_vault::(1).unwrap(); + assert_eq!(pool1.basepool.share_price(), Some(fp!(2))); + // Withdrawal should bring the price back to 1 + assert_ok!(PhalaVault::withdraw( + RuntimeOrigin::signed(1), + 1, + 100 * DOLLARS, + )); + let pool1 = ensure_vault::(1).unwrap(); + assert_eq!(pool1.basepool.share_price(), Some(fp!(1))); + }); +} + fn mock_asset_id() { as Create>::create( ::WPhaAssetId::get(), From ba0b8372170aba7dd144fb7ae94e9cdda8a73c91 Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Fri, 27 Oct 2023 10:33:20 +0000 Subject: [PATCH 7/7] compute: update insta snap --- .../src/snapshots/phala_pallets__test__withdraw8.snap | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap b/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap index 3b9e4d7941..a31045e780 100644 --- a/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap +++ b/pallets/phala/src/snapshots/phala_pallets__test__withdraw8.snap @@ -3,6 +3,13 @@ source: pallets/phala/src/test.rs expression: take_events() --- [ + RuntimeEvent::PhalaVault( + Event::OwnerSharesGained { + pid: 2, + shares: 100, + checkout_price: 1000000000333, + }, + ), RuntimeEvent::Uniques( Event::Issued { collection: 10002,