From 44f2bccc7c0e24e1402131e2b8ca66a5ff7c3135 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Mon, 5 Aug 2024 11:45:15 +0200 Subject: [PATCH 01/15] feat: add FrozenInvestor permission --- libs/types/src/permissions.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/libs/types/src/permissions.rs b/libs/types/src/permissions.rs index 57eee60d24..21e40ead9b 100644 --- a/libs/types/src/permissions.rs +++ b/libs/types/src/permissions.rs @@ -36,6 +36,7 @@ pub enum PoolRole { LoanAdmin, TrancheInvestor(TrancheId, Seconds), PODReadAccess, + FrozenTrancheInvestor(TrancheId), } #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, TypeInfo, Debug)] @@ -110,6 +111,12 @@ pub struct PermissionedCurrencyHolderInfo { pub struct TrancheInvestorInfo { tranche_id: TrancheId, permissioned_till: Seconds, + is_frozen: bool, +} + +#[derive(Encode, Decode, TypeInfo, Debug, Clone, Eq, PartialEq, MaxEncodedLen)] +pub struct FrozenTrancheInvestorInfo { + tranche_id: TrancheId, } #[derive(Encode, Decode, TypeInfo, Debug, Clone, Eq, PartialEq, MaxEncodedLen)] @@ -214,6 +221,7 @@ where PoolRole::PODReadAccess => { self.pool_admin.contains(PoolAdminRoles::POD_READ_ACCESS) } + PoolRole::FrozenTrancheInvestor(id) => self.tranche_investor.contains_frozen(id), }, Role::PermissionedCurrencyRole(permissioned_currency_role) => { match permissioned_currency_role { @@ -255,6 +263,7 @@ where PoolRole::PODReadAccess => { Ok(self.pool_admin.remove(PoolAdminRoles::POD_READ_ACCESS)) } + PoolRole::FrozenTrancheInvestor(id) => self.tranche_investor.unfreeze(id), }, Role::PermissionedCurrencyRole(permissioned_currency_role) => { match permissioned_currency_role { @@ -289,6 +298,7 @@ where PoolRole::PODReadAccess => { Ok(self.pool_admin.insert(PoolAdminRoles::POD_READ_ACCESS)) } + PoolRole::FrozenTrancheInvestor(id) => self.tranche_investor.freeze(id), }, Role::PermissionedCurrencyRole(permissioned_currency_role) => { match permissioned_currency_role { @@ -410,6 +420,12 @@ where }) } + pub fn contains_frozen(&self, tranche: TrancheId) -> bool { + self.info + .iter() + .any(|info| info.tranche_id == tranche && info.is_frozen) + } + #[allow(clippy::result_unit_err)] pub fn remove(&mut self, tranche: TrancheId, delta: Seconds) -> Result<(), ()> { if let Some(index) = self.info.iter().position(|info| info.tranche_id == tranche) { @@ -443,10 +459,27 @@ where .try_push(TrancheInvestorInfo { tranche_id: tranche, permissioned_till: validity, + is_frozen: false, }) .map_err(|_| ()) } } + + #[allow(clippy::result_unit_err)] + pub fn freeze(&mut self, tranche: TrancheId) -> Result<(), ()> { + if let Some(investor) = self.info.iter_mut().find(|t| t.tranche_id == tranche) { + investor.is_frozen = true; + } + Ok(()) + } + + #[allow(clippy::result_unit_err)] + pub fn unfreeze(&mut self, tranche: TrancheId) -> Result<(), ()> { + if let Some(investor) = self.info.iter_mut().find(|t| t.tranche_id == tranche) { + investor.is_frozen = false; + } + Ok(()) + } } #[cfg(test)] From afb686d1a01e4ab7d54d846afeee3506a277722c Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Mon, 5 Aug 2024 11:46:24 +0200 Subject: [PATCH 02/15] feat: add freeze + unfreeze extrinsics --- pallets/liquidity-pools/src/inbound.rs | 23 +- pallets/liquidity-pools/src/lib.rs | 183 ++++++- pallets/liquidity-pools/src/tests.rs | 20 +- pallets/liquidity-pools/src/tests/inbound.rs | 482 ++++++++++++++++++- runtime/altair/src/lib.rs | 41 +- runtime/centrifuge/src/lib.rs | 47 +- runtime/development/src/lib.rs | 47 +- 7 files changed, 762 insertions(+), 81 deletions(-) diff --git a/pallets/liquidity-pools/src/inbound.rs b/pallets/liquidity-pools/src/inbound.rs index 2b7d5b442a..b2c904ca25 100644 --- a/pallets/liquidity-pools/src/inbound.rs +++ b/pallets/liquidity-pools/src/inbound.rs @@ -11,14 +11,8 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. -use cfg_traits::{ - investments::ForeignInvestment, liquidity_pools::OutboundMessageHandler, Permissions, - TimeAsSecs, -}; -use cfg_types::{ - domain_address::{Domain, DomainAddress}, - permissions::{PermissionScope, PoolRole, Role}, -}; +use cfg_traits::{investments::ForeignInvestment, liquidity_pools::OutboundMessageHandler}; +use cfg_types::domain_address::{Domain, DomainAddress}; use frame_support::{ ensure, traits::{fungibles::Mutate, tokens::Preservation, OriginTrait}, @@ -69,14 +63,11 @@ where let local_representation_of_receiver = T::DomainAddressToAccountId::convert(receiver.clone()); - ensure!( - T::Permission::has( - PermissionScope::Pool(pool_id), - local_representation_of_receiver.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::Time::now())), - ), - Error::::UnauthorizedTransfer - ); + Self::validate_investor_can_transfer( + local_representation_of_receiver.clone(), + pool_id, + tranche_id, + )?; let invest_id = Self::derive_invest_id(pool_id, tranche_id)?; diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index 8e55c470c3..c2a9ab7614 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -335,6 +335,12 @@ pub mod pallet { /// The account derived from the [Domain] and [DomainAddress] has not /// been whitelisted as a TrancheInvestor. InvestorDomainAddressNotAMember, + /// The account derived from the [Domain] and [DomainAddress] is frozen + /// and cannot transfer tranche tokens therefore. + InvestorDomainAddressFrozen, + /// The account derived from the [Domain] and [DomainAddress] is not + /// frozen and cannot be unfrozen therefore. + InvestorDomainAddressNotFrozen, /// Only the PoolAdmin can execute a given operation. NotPoolAdmin, /// The domain hook address could not be found. @@ -555,18 +561,14 @@ pub mod pallet { let who = ensure_signed(origin.clone())?; ensure!(!amount.is_zero(), Error::::InvalidTransferAmount); - ensure!( - T::Permission::has( - PermissionScope::Pool(pool_id), - T::DomainAddressToAccountId::convert(domain_address.clone()), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::Time::now())) - ), - Error::::UnauthorizedTransfer - ); + Self::validate_investor_can_transfer( + T::DomainAddressToAccountId::convert(domain_address.clone()), + pool_id, + tranche_id, + )?; // Ensure pool and tranche exist and derive invest id let invest_id = Self::derive_invest_id(pool_id, tranche_id)?; - T::PreTransferFilter::check((who.clone(), domain_address.clone(), invest_id.into()))?; // Transfer to the domain account for bookkeeping @@ -667,6 +669,7 @@ pub mod pallet { /// Add a currency to the set of known currencies on the domain derived /// from the given currency. + // TODO: Fix weights #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] #[pallet::call_index(8)] pub fn add_currency(origin: OriginFor, currency_id: T::CurrencyId) -> DispatchResult { @@ -691,6 +694,7 @@ pub mod pallet { /// Allow a currency to be used as a pool currency and to invest in a /// pool on the domain derived from the given currency. #[pallet::call_index(9)] + // TODO: Fix weights #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] pub fn allow_investment_currency( origin: OriginFor, @@ -795,7 +799,8 @@ pub mod pallet { /// Disallow a currency to be used as a pool currency and to invest in a /// pool on the domain derived from the given currency. #[pallet::call_index(13)] - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + // TODO: Add to weights + #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] pub fn disallow_investment_currency( origin: OriginFor, pool_id: T::PoolId, @@ -825,6 +830,112 @@ pub mod pallet { Ok(()) } + + // TODO: Add to weights + #[pallet::call_index(14)] + #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] + pub fn freeze_investor( + origin: OriginFor, + pool_id: T::PoolId, + tranche_id: T::TrancheId, + domain_address: DomainAddress, + ) -> DispatchResult { + let who = ensure_signed(origin.clone())?; + + ensure!( + T::PoolInspect::pool_exists(pool_id), + Error::::PoolNotFound + ); + ensure!( + T::PoolInspect::tranche_exists(pool_id, tranche_id), + Error::::TrancheNotFound + ); + + ensure!( + T::Permission::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::PoolAdmin) + ), + Error::::NotPoolAdmin + ); + Self::validate_investor_status( + domain_address.address().into(), + pool_id, + tranche_id, + false, + )?; + + T::OutboundMessageHandler::handle( + who, + domain_address.domain(), + Message::UpdateRestriction { + pool_id: pool_id.into(), + tranche_id: tranche_id.into(), + update: UpdateRestrictionMessage::Freeze { + address: domain_address.address(), + }, + }, + )?; + + Ok(()) + } + + // TODO: Add to weights + #[pallet::call_index(15)] + #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] + pub fn unfreeze_investor( + origin: OriginFor, + pool_id: T::PoolId, + tranche_id: T::TrancheId, + domain_address: DomainAddress, + ) -> DispatchResult { + let who = ensure_signed(origin.clone())?; + + ensure!( + T::PoolInspect::pool_exists(pool_id), + Error::::PoolNotFound + ); + ensure!( + T::PoolInspect::tranche_exists(pool_id, tranche_id), + Error::::TrancheNotFound + ); + + ensure!( + T::Permission::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::PoolAdmin) + ), + Error::::NotPoolAdmin + ); + Self::validate_investor_status( + domain_address.address().into(), + pool_id, + tranche_id, + true, + )?; + + T::Permission::remove( + PermissionScope::Pool(pool_id), + domain_address.address().into(), + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + )?; + + T::OutboundMessageHandler::handle( + who, + domain_address.domain(), + Message::UpdateRestriction { + pool_id: pool_id.into(), + tranche_id: tranche_id.into(), + update: UpdateRestrictionMessage::Unfreeze { + address: domain_address.address(), + }, + }, + )?; + + Ok(()) + } } impl Pallet { @@ -942,6 +1053,58 @@ pub mod pallet { let domain_address = T::DomainAccountToDomainAddress::convert(domain_account); T::DomainAddressToAccountId::convert(domain_address) } + + pub fn validate_investor_status( + investor: T::AccountId, + pool_id: T::PoolId, + tranche_id: T::TrancheId, + is_frozen: bool, + ) -> DispatchResult { + ensure!( + T::Permission::has( + PermissionScope::Pool(pool_id), + investor.clone(), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::Time::now())) + ), + Error::::InvestorDomainAddressNotAMember + ); + ensure!( + is_frozen + == T::Permission::has( + PermissionScope::Pool(pool_id), + investor, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) + ), + Error::::InvestorDomainAddressFrozen + ); + + Ok(()) + } + + pub fn validate_investor_can_transfer( + investor: T::AccountId, + pool_id: T::PoolId, + tranche_id: T::TrancheId, + ) -> DispatchResult { + ensure!( + T::Permission::has( + PermissionScope::Pool(pool_id), + investor.clone(), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::Time::now())) + ), + Error::::UnauthorizedTransfer + ); + ensure!( + !T::Permission::has( + PermissionScope::Pool(pool_id), + investor, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) + ), + Error::::InvestorDomainAddressFrozen + ); + + Ok(()) + } } impl InboundMessageHandler for Pallet diff --git a/pallets/liquidity-pools/src/tests.rs b/pallets/liquidity-pools/src/tests.rs index 47596ff243..6408457eaa 100644 --- a/pallets/liquidity-pools/src/tests.rs +++ b/pallets/liquidity-pools/src/tests.rs @@ -213,13 +213,21 @@ mod transfer_tranche_tokens { DomainAddressToAccountId::mock_convert(|_| CONTRACT_ACCOUNT_ID); Time::mock_now(|| NOW); Permissions::mock_has(move |scope, who, role| { - assert_eq!(who, CONTRACT_ACCOUNT_ID); assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - assert!(matches!( - role, - Role::PoolRole(PoolRole::TrancheInvestor(TRANCHE_ID, NOW_SECS)) - )); - true + assert_eq!(who, CONTRACT_ACCOUNT_ID); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(tranche_id, TRANCHE_ID); + // Default mock has unfrozen investor + false + } + _ => false, + } }); Pools::mock_pool_exists(|_| true); Pools::mock_tranche_exists(|_, _| true); diff --git a/pallets/liquidity-pools/src/tests/inbound.rs b/pallets/liquidity-pools/src/tests/inbound.rs index 886ceac3c2..dc634717b7 100644 --- a/pallets/liquidity-pools/src/tests/inbound.rs +++ b/pallets/liquidity-pools/src/tests/inbound.rs @@ -92,13 +92,21 @@ mod handle_tranche_tokens_transfer { DomainAddressToAccountId::mock_convert(move |_| ALICE); Time::mock_now(|| NOW); Permissions::mock_has(move |scope, who, role| { - assert_eq!(who, ALICE); assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - assert!(matches!( - role, - Role::PoolRole(PoolRole::TrancheInvestor(TRANCHE_ID, NOW_SECS)) - )); - true + assert_eq!(who, ALICE); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(tranche_id, TRANCHE_ID); + // Default mock has unfrozen investor + false + } + _ => false, + } }); Pools::mock_pool_exists(|_| true); Pools::mock_tranche_exists(|_, _| true); @@ -206,7 +214,7 @@ mod handle_tranche_tokens_transfer { } #[test] - fn with_wrong_permissions() { + fn without_investor_permissions() { System::externalities().execute_with(|| { config_mocks(CENTRIFUGE_DOMAIN_ADDRESS); Permissions::mock_has(|_, _, _| false); @@ -227,6 +235,47 @@ mod handle_tranche_tokens_transfer { }) } + #[test] + fn inbound_with_frozen_investor_permissions() { + System::externalities().execute_with(|| { + config_mocks(CENTRIFUGE_DOMAIN_ADDRESS); + Permissions::mock_has(|_, _, _| true); + + assert_noop!( + LiquidityPools::handle( + EVM_DOMAIN_ADDRESS, + Message::TransferTrancheTokens { + pool_id: POOL_ID, + tranche_id: TRANCHE_ID, + domain: CENTRIFUGE_DOMAIN_ADDRESS.domain().into(), + receiver: ALICE.into(), + amount: AMOUNT, + } + ), + Error::::InvestorDomainAddressFrozen, + ); + }) + } + + #[test] + fn outbound_with_frozen_investor_permissions() { + System::externalities().execute_with(|| { + config_mocks(EVM_DOMAIN_ADDRESS); + Permissions::mock_has(|_, _, _| true); + + assert_noop!( + LiquidityPools::transfer_tranche_tokens( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + EVM_DOMAIN_ADDRESS, + AMOUNT, + ), + Error::::InvestorDomainAddressFrozen, + ); + }) + } + #[test] fn with_wrong_pool() { System::externalities().execute_with(|| { @@ -773,3 +822,422 @@ mod handle_cancel_redeem_order { } } } + +mod freeze { + use sp_runtime::DispatchError; + + use super::*; + use crate::message::UpdateRestrictionMessage; + + fn config_mocks(receiver: DomainAddress) { + DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); + DomainAddressToAccountId::mock_convert(move |_| ALICE); + Time::mock_now(|| NOW); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(tranche_id, TRANCHE_ID); + // Default mock has unfrozen investor + false + } + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + Pools::mock_pool_exists(|_| true); + Pools::mock_tranche_exists(|_, _| true); + Gateway::mock_handle(|sender, destination, msg| { + assert_eq!(sender, ALICE); + assert_eq!(destination, ALICE_EVM_DOMAIN_ADDRESS.domain()); + assert_eq!( + msg, + Message::UpdateRestriction { + pool_id: POOL_ID, + tranche_id: TRANCHE_ID, + update: UpdateRestrictionMessage::Freeze { + address: ALICE_EVM_DOMAIN_ADDRESS.address().into() + } + } + ); + Ok(()) + }); + } + + #[test] + fn success() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + + assert_ok!(LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + )); + }); + } + + mod erroring_out { + use super::*; + + #[test] + fn with_bad_origin_unsigned_none() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::none(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + #[test] + fn with_bad_origin_unsigned_root() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::root(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + + #[test] + fn with_pool_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_pool_exists(|_| false); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::PoolNotFound + ); + }); + } + + #[test] + fn with_tranche_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_tranche_exists(|_, _| false); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::TrancheNotFound + ); + }); + } + + #[test] + fn with_origin_not_admin() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(|_, _, _| false); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::NotPoolAdmin + ); + }); + } + + #[test] + fn with_investor_not_member() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressNotAMember + ); + }); + } + + #[test] + fn with_investor_frozen() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |_, _, _| true); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressFrozen + ); + }); + } + } +} + +mod unfreeze { + use sp_runtime::DispatchError; + + use super::*; + use crate::message::UpdateRestrictionMessage; + + fn config_mocks(receiver: DomainAddress) { + DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); + DomainAddressToAccountId::mock_convert(move |_| ALICE); + Time::mock_now(|| NOW); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(tranche_id, TRANCHE_ID); + // Default mock has frozen investor + true + } + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + Permissions::mock_remove(|scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(tranche_id, TRANCHE_ID); + Ok(()) + } + _ => Err(DispatchError::Other( + "Must only remove FrozenTrancheInvestor", + )), + } + }); + Pools::mock_pool_exists(|_| true); + Pools::mock_tranche_exists(|_, _| true); + Gateway::mock_handle(|sender, destination, msg| { + assert_eq!(sender, ALICE); + assert_eq!(destination, ALICE_EVM_DOMAIN_ADDRESS.domain()); + assert_eq!( + msg, + Message::UpdateRestriction { + pool_id: POOL_ID, + tranche_id: TRANCHE_ID, + update: UpdateRestrictionMessage::Unfreeze { + address: ALICE_EVM_DOMAIN_ADDRESS.address().into() + } + } + ); + Ok(()) + }); + } + + #[test] + fn success() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + + assert_ok!(LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + )); + }); + } + + mod erroring_out { + use super::*; + + #[test] + fn with_bad_origin_unsigned_none() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::none(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + #[test] + fn with_bad_origin_unsigned_root() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::root(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + + #[test] + fn with_pool_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_pool_exists(|_| false); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::PoolNotFound + ); + }); + } + + #[test] + fn with_tranche_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_tranche_exists(|_, _| false); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::TrancheNotFound + ); + }); + } + + #[test] + fn with_origin_not_admin() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(|_, _, _| false); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::NotPoolAdmin + ); + }); + } + + #[test] + fn with_investor_not_member() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressNotAMember + ); + }); + } + + #[test] + fn with_investor_unfrozen() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(tranche_id, TRANCHE_ID); + false + } + _ => true, + } + }); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressFrozen + ); + }); + } + } +} diff --git a/runtime/altair/src/lib.rs b/runtime/altair/src/lib.rs index 69f2607ed3..d75eb825bd 100644 --- a/runtime/altair/src/lib.rs +++ b/runtime/altair/src/lib.rs @@ -1672,19 +1672,20 @@ impl pallet_investments::Config for Runtime { type CollectedRedemptionHook = pallet_foreign_investments::CollectedRedemptionHook; type InvestmentId = InvestmentId; type MaxOutstandingCollects = MaxOutstandingCollects; - type PreConditions = IsTrancheInvestor; + type PreConditions = IsUnfrozenTrancheInvestor; type RuntimeEvent = RuntimeEvent; type Tokens = Tokens; type WeightInfo = weights::pallet_investments::WeightInfo; } /// Checks whether the given `who` has the role -/// of a `TrancheInvestor` for the given pool. -pub struct IsTrancheInvestor(PhantomData<(P, T)>); +/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool +/// and tranche. +pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); impl< P: PermissionsT, Role = Role>, T: UnixTime, - > PreConditions> for IsTrancheInvestor + > PreConditions> for IsUnfrozenTrancheInvestor { type Result = DispatchResult; @@ -1694,20 +1695,32 @@ impl< who, investment_id: (pool_id, tranche_id), .. - } => P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ), + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } OrderType::Redemption { who, investment_id: (pool_id, tranche_id), .. - } => P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ), + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } }; if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { diff --git a/runtime/centrifuge/src/lib.rs b/runtime/centrifuge/src/lib.rs index 1deb9a249a..8e2c0ac14c 100644 --- a/runtime/centrifuge/src/lib.rs +++ b/runtime/centrifuge/src/lib.rs @@ -1630,12 +1630,13 @@ parameter_types! { } /// Checks whether the given `who` has the role -/// of a `TrancheInvestor` for the given pool. -pub struct IsTrancheInvestor(PhantomData<(P, T)>); +/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool +/// and tranche. +pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); impl< P: PermissionsT, Role = Role>, T: UnixTime, - > PreConditions> for IsTrancheInvestor + > PreConditions> for IsUnfrozenTrancheInvestor { type Result = DispatchResult; @@ -1645,20 +1646,38 @@ impl< who, investment_id: (pool_id, tranche_id), .. - } => P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ), + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor( + tranche_id.clone(), + T::now().as_secs(), + )), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } OrderType::Redemption { who, investment_id: (pool_id, tranche_id), .. - } => P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ), + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor( + tranche_id.clone(), + T::now().as_secs(), + )), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } }; if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { @@ -1682,7 +1701,7 @@ impl pallet_investments::Config for Runtime { type CollectedRedemptionHook = pallet_foreign_investments::CollectedRedemptionHook; type InvestmentId = InvestmentId; type MaxOutstandingCollects = MaxOutstandingCollects; - type PreConditions = IsTrancheInvestor; + type PreConditions = IsUnfrozenTrancheInvestor; type RuntimeEvent = RuntimeEvent; type Tokens = Tokens; type WeightInfo = weights::pallet_investments::WeightInfo; diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index 025c648d2d..3df9e78997 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -1689,19 +1689,20 @@ impl pallet_investments::Config for Runtime { type CollectedRedemptionHook = pallet_foreign_investments::CollectedRedemptionHook; type InvestmentId = InvestmentId; type MaxOutstandingCollects = MaxOutstandingCollects; - type PreConditions = IsTrancheInvestor; + type PreConditions = IsUnfrozenTrancheInvestor; type RuntimeEvent = RuntimeEvent; type Tokens = Tokens; type WeightInfo = (); } /// Checks whether the given `who` has the role -/// of a `TrancheInvestor` for the given pool. -pub struct IsTrancheInvestor(PhantomData<(P, T)>); +/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool +/// and tranche. +pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); impl< P: PermissionsT, Role = Role>, T: UnixTime, - > PreConditions> for IsTrancheInvestor + > PreConditions> for IsUnfrozenTrancheInvestor { type Result = DispatchResult; @@ -1711,20 +1712,38 @@ impl< who, investment_id: (pool_id, tranche_id), .. - } => P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ), + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor( + tranche_id.clone(), + T::now().as_secs(), + )), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } OrderType::Redemption { who, investment_id: (pool_id, tranche_id), .. - } => P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ), + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor( + tranche_id.clone(), + T::now().as_secs(), + )), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } }; if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { From ee4b1ba1e9b29133bf8f2f4a59c9e6da32f4f429 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Mon, 5 Aug 2024 18:35:22 +0200 Subject: [PATCH 03/15] chore: update investments logic --- pallets/investments/src/lib.rs | 7 +++++++ pallets/investments/src/mock.rs | 27 ++++++++++++++++++++++----- pallets/investments/src/tests.rs | 21 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/pallets/investments/src/lib.rs b/pallets/investments/src/lib.rs index ef9ecd6967..f12a427d66 100644 --- a/pallets/investments/src/lib.rs +++ b/pallets/investments/src/lib.rs @@ -627,6 +627,13 @@ impl Pallet { who: T::AccountId, investment_id: T::InvestmentId, ) -> DispatchResultWithPostInfo { + // Frozen investors must not be able to collect tranche tokens + T::PreConditions::check(OrderType::Investment { + who: who.clone(), + investment_id, + amount: Default::default(), + })?; + let _ = T::Accountant::info(investment_id).map_err(|_| Error::::UnknownInvestment)?; let (collected_investment, post_dispatch_info) = InvestOrders::::try_mutate( &who, diff --git a/pallets/investments/src/mock.rs b/pallets/investments/src/mock.rs index 57b7a287e1..3e32483601 100644 --- a/pallets/investments/src/mock.rs +++ b/pallets/investments/src/mock.rs @@ -20,6 +20,7 @@ use cfg_types::{ fixed_point::Quantity, investments::{InvestmentAccount, InvestmentInfo}, orders::{FulfillmentWithPrice, TotalOrder}, + permissions::PoolRole::PoolAdmin, tokens::CurrencyId, }; use frame_support::{ @@ -46,6 +47,7 @@ use sp_std::{ }; pub use crate as pallet_investments; +use crate::OrderType; pub type AccountId = u64; @@ -134,18 +136,30 @@ impl pallet_investments::Config for Runtime { type CollectedRedemptionHook = NoopCollectHook; type InvestmentId = InvestmentId; type MaxOutstandingCollects = MaxOutstandingCollect; - type PreConditions = Always; + type PreConditions = AlwaysWithOneException; type RuntimeEvent = RuntimeEvent; type Tokens = OrmlTokens; type WeightInfo = (); } -pub struct Always; -impl PreConditions for Always { +pub struct AlwaysWithOneException; +impl PreConditions for Always +where + T: Into> + Clone, +{ type Result = DispatchResult; - fn check(_: T) -> Self::Result { - Ok(()) + fn check(order: T) -> Self::Result { + match order.clone().into() { + OrderType::Investment { who, .. } | OrderType::Redemption { who, .. } + if who == NOT_INVESTOR => + { + Err(DispatchError::Other( + "PreCondition mock fails on u64::MAX account on purpose", + )) + } + _ => Ok(()), + } } } @@ -188,6 +202,9 @@ pub const UNKNOWN_INVESTMENT: InvestmentId = (1, TRANCHE_ID_0); /// The currency id for the AUSD token pub const AUSD_CURRENCY_ID: CurrencyId = CurrencyId::ForeignAsset(1); +/// The account for which the pre-condition mock fails +pub(crate) const NOT_INVESTOR: u64 = u64::MAX; + impl TestExternalitiesBuilder { // Build a genesis storage key/value store pub(crate) fn build() -> TestExternalities { diff --git a/pallets/investments/src/tests.rs b/pallets/investments/src/tests.rs index 771836b257..ed7eceaea6 100644 --- a/pallets/investments/src/tests.rs +++ b/pallets/investments/src/tests.rs @@ -2955,3 +2955,24 @@ fn collecting_over_max_works() { } }) } + +#[test] +fn collecting_investment_without_preconditions_fails() { + TestExternalitiesBuilder::build().execute_with(|| { + assert_noop!( + Investments::collect_investments(RuntimeOrigin::signed(NOT_INVESTOR), INVESTMENT_0_0), + DispatchError::Other("PreCondition mock fails on u64::MAX account on purpose") + ); + }) +} + +/// NOTE: Collecting redemptions does not check pre-conditions +#[test] +fn collecting_redemption_without_preconditions_success() { + TestExternalitiesBuilder::build().execute_with(|| { + assert_ok!(Investments::collect_redemptions( + RuntimeOrigin::signed(NOT_INVESTOR), + INVESTMENT_0_0 + ),); + }) +} From 787871d4918516352e3857f42afc7cb57881a949 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Mon, 5 Aug 2024 18:37:54 +0200 Subject: [PATCH 04/15] tests: fix UTs + add ITs --- pallets/investments/src/mock.rs | 3 +- pallets/liquidity-pools/src/lib.rs | 19 ++- pallets/liquidity-pools/src/mock.rs | 9 ++ pallets/liquidity-pools/src/tests/inbound.rs | 27 ++-- .../src/cases/lp/pool_management.rs | 119 ++++++++++++++++++ .../src/cases/lp/setup_lp.rs | 4 + 6 files changed, 167 insertions(+), 14 deletions(-) diff --git a/pallets/investments/src/mock.rs b/pallets/investments/src/mock.rs index 3e32483601..da3c6ed087 100644 --- a/pallets/investments/src/mock.rs +++ b/pallets/investments/src/mock.rs @@ -20,7 +20,6 @@ use cfg_types::{ fixed_point::Quantity, investments::{InvestmentAccount, InvestmentInfo}, orders::{FulfillmentWithPrice, TotalOrder}, - permissions::PoolRole::PoolAdmin, tokens::CurrencyId, }; use frame_support::{ @@ -143,7 +142,7 @@ impl pallet_investments::Config for Runtime { } pub struct AlwaysWithOneException; -impl PreConditions for Always +impl PreConditions for AlwaysWithOneException where T: Into> + Clone, { diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index c2a9ab7614..8912f1cb49 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -841,6 +841,7 @@ pub mod pallet { domain_address: DomainAddress, ) -> DispatchResult { let who = ensure_signed(origin.clone())?; + let local_address = T::DomainAddressToAccountId::convert(domain_address.clone()); ensure!( T::PoolInspect::pool_exists(pool_id), @@ -860,12 +861,19 @@ pub mod pallet { Error::::NotPoolAdmin ); Self::validate_investor_status( - domain_address.address().into(), + local_address.clone(), pool_id, tranche_id, + T::Time::now(), false, )?; + T::Permission::add( + PermissionScope::Pool(pool_id), + local_address, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + )?; + T::OutboundMessageHandler::handle( who, domain_address.domain(), @@ -891,6 +899,7 @@ pub mod pallet { domain_address: DomainAddress, ) -> DispatchResult { let who = ensure_signed(origin.clone())?; + let local_address = T::DomainAddressToAccountId::convert(domain_address.clone()); ensure!( T::PoolInspect::pool_exists(pool_id), @@ -910,15 +919,16 @@ pub mod pallet { Error::::NotPoolAdmin ); Self::validate_investor_status( - domain_address.address().into(), + local_address.clone(), pool_id, tranche_id, + T::Time::now(), true, )?; T::Permission::remove( PermissionScope::Pool(pool_id), - domain_address.address().into(), + local_address, Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), )?; @@ -1058,13 +1068,14 @@ pub mod pallet { investor: T::AccountId, pool_id: T::PoolId, tranche_id: T::TrancheId, + valid_until: Seconds, is_frozen: bool, ) -> DispatchResult { ensure!( T::Permission::has( PermissionScope::Pool(pool_id), investor.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::Time::now())) + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, valid_until)) ), Error::::InvestorDomainAddressNotAMember ); diff --git a/pallets/liquidity-pools/src/mock.rs b/pallets/liquidity-pools/src/mock.rs index 453a23aabb..8f62e37b82 100644 --- a/pallets/liquidity-pools/src/mock.rs +++ b/pallets/liquidity-pools/src/mock.rs @@ -27,6 +27,15 @@ pub const ALICE_32: [u8; 32] = [2; 32]; pub const ALICE: AccountId = AccountId::new(ALICE_32); pub const ALICE_ETH: [u8; 20] = [2; 20]; pub const ALICE_EVM_DOMAIN_ADDRESS: DomainAddress = DomainAddress::EVM(42, ALICE_ETH); +pub const ALICE_EVM_LOCAL_ACCOUNT: AccountId = { + let mut arr = [0u8; 32]; + let mut i = 0; + while i < 20 { + arr[i] = ALICE_ETH[i]; + i += 1; + } + AccountId::new(arr) +}; pub const CENTRIFUGE_DOMAIN_ADDRESS: DomainAddress = DomainAddress::Centrifuge(ALICE_32); pub const CONTRACT_ACCOUNT: [u8; 20] = [1; 20]; pub const CONTRACT_ACCOUNT_ID: AccountId = AccountId::new([1; 32]); diff --git a/pallets/liquidity-pools/src/tests/inbound.rs b/pallets/liquidity-pools/src/tests/inbound.rs index dc634717b7..9e0a63d9c5 100644 --- a/pallets/liquidity-pools/src/tests/inbound.rs +++ b/pallets/liquidity-pools/src/tests/inbound.rs @@ -831,19 +831,19 @@ mod freeze { fn config_mocks(receiver: DomainAddress) { DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); - DomainAddressToAccountId::mock_convert(move |_| ALICE); + DomainAddressToAccountId::mock_convert(move |_| ALICE_EVM_LOCAL_ACCOUNT); Time::mock_now(|| NOW); Permissions::mock_has(move |scope, who, role| { assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); match role { Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { - assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); assert_eq!(validity, NOW_SECS); true } Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); // Default mock has unfrozen investor false @@ -855,6 +855,17 @@ mod freeze { _ => false, } }); + Permissions::mock_add(|scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + Ok(()) + } + _ => Err(DispatchError::Other("Must only add FrozenTrancheInvestor")), + } + }); Pools::mock_pool_exists(|_| true); Pools::mock_tranche_exists(|_, _| true); Gateway::mock_handle(|sender, destination, msg| { @@ -1029,19 +1040,19 @@ mod unfreeze { fn config_mocks(receiver: DomainAddress) { DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); - DomainAddressToAccountId::mock_convert(move |_| ALICE); + DomainAddressToAccountId::mock_convert(move |_| ALICE_EVM_LOCAL_ACCOUNT); Time::mock_now(|| NOW); Permissions::mock_has(move |scope, who, role| { assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); match role { Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { - assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); assert_eq!(validity, NOW_SECS); true } Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); // Default mock has frozen investor true @@ -1057,7 +1068,7 @@ mod unfreeze { assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); match role { Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); Ok(()) } @@ -1220,7 +1231,7 @@ mod unfreeze { assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); match role { Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_DOMAIN_ADDRESS.address().into()); + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); false } diff --git a/runtime/integration-tests/src/cases/lp/pool_management.rs b/runtime/integration-tests/src/cases/lp/pool_management.rs index 45fb5f2190..e77a519b33 100644 --- a/runtime/integration-tests/src/cases/lp/pool_management.rs +++ b/runtime/integration-tests/src/cases/lp/pool_management.rs @@ -620,3 +620,122 @@ fn update_tranche_token_price() { assert_eq!(price.into_inner(), price_evm); }); } + +#[test_runtimes([centrifuge, development])] +fn freeze_member() { + let mut env = super::setup::(|evm| { + super::setup_currencies(evm); + super::setup_pools(evm); + super::setup_tranches(evm); + super::setup_investment_currencies(evm); + super::setup_deploy_lps(evm); + super::setup_investors(evm); + }); + + env.state(|evm| { + assert!(!Decoder::::decode( + &evm.view( + Keyring::Alice, + names::RESTRICTION_MANAGER, + "isFrozen", + Some(&[ + Token::Address(evm.deployed(names::POOL_A_T_1).address()), + Token::Address(Keyring::TrancheInvestor(2).into()) + ]), + ) + .unwrap() + .value + )); + }); + + env.state_mut(|_| { + assert_ok!(pallet_liquidity_pools::Pallet::::freeze_investor( + Keyring::Admin.as_origin(), + POOL_A, + pool_a_tranche_1_id::(), + DomainAddress::evm(EVM_DOMAIN_CHAIN_ID, Keyring::TrancheInvestor(2).into()), + )); + + utils::process_gateway_message::(utils::verify_gateway_message_success::); + }); + + env.state(|evm| { + assert!(Decoder::::decode( + &evm.view( + Keyring::Alice, + names::RESTRICTION_MANAGER, + "isFrozen", + Some(&[ + Token::Address(evm.deployed(names::POOL_A_T_1).address()), + Token::Address(Keyring::TrancheInvestor(2).into()) + ]), + ) + .unwrap() + .value + )); + }); +} + +#[test_runtimes([centrifuge, development])] +fn unfreeze_member() { + let mut env = super::setup::(|evm| { + super::setup_currencies(evm); + super::setup_pools(evm); + super::setup_tranches(evm); + super::setup_investment_currencies(evm); + super::setup_deploy_lps(evm); + super::setup_investors(evm); + }); + + env.state_mut(|_| { + assert_ok!(pallet_liquidity_pools::Pallet::::freeze_investor( + Keyring::Admin.as_origin(), + POOL_A, + pool_a_tranche_1_id::(), + DomainAddress::evm(EVM_DOMAIN_CHAIN_ID, Keyring::TrancheInvestor(2).into()), + )); + + utils::process_gateway_message::(utils::verify_gateway_message_success::); + }); + env.state(|evm| { + assert!(Decoder::::decode( + &evm.view( + Keyring::Alice, + names::RESTRICTION_MANAGER, + "isFrozen", + Some(&[ + Token::Address(evm.deployed(names::POOL_A_T_1).address()), + Token::Address(Keyring::TrancheInvestor(2).into()) + ]), + ) + .unwrap() + .value + )); + }); + + env.state_mut(|_| { + assert_ok!(pallet_liquidity_pools::Pallet::::unfreeze_investor( + Keyring::Admin.as_origin(), + POOL_A, + pool_a_tranche_1_id::(), + DomainAddress::evm(EVM_DOMAIN_CHAIN_ID, Keyring::TrancheInvestor(2).into()), + )); + + utils::process_gateway_message::(utils::verify_gateway_message_success::); + }); + env.state(|evm| { + assert!(!Decoder::::decode( + &evm.view( + Keyring::Alice, + names::RESTRICTION_MANAGER, + "isFrozen", + Some(&[ + Token::Address(evm.deployed(names::POOL_A_T_1).address()), + Token::Address(Keyring::TrancheInvestor(2).into()) + ]), + ) + .unwrap() + .value + )); + }); +} diff --git a/runtime/integration-tests/src/cases/lp/setup_lp.rs b/runtime/integration-tests/src/cases/lp/setup_lp.rs index f97463a81b..177fc25329 100644 --- a/runtime/integration-tests/src/cases/lp/setup_lp.rs +++ b/runtime/integration-tests/src/cases/lp/setup_lp.rs @@ -676,6 +676,7 @@ pub fn setup_currencies(evm: &mut impl EvmEnv) { /// Centrifuge Chain as well as EVM. Also mints default balance on both sides. pub fn setup_investors(evm: &mut impl EvmEnv) { default_investors().into_iter().for_each(|investor| { + // POOL A - Tranche 1/1 // Allow investor to locally invest crate::utils::pool::give_role::( investor.into(), @@ -696,6 +697,7 @@ pub fn setup_investors(evm: &mut impl EvmEnv) { SECONDS_PER_YEAR, )); + // POOL B - Tranche 1/2 // Allow investor to locally invest crate::utils::pool::give_role::( investor.into(), @@ -715,6 +717,7 @@ pub fn setup_investors(evm: &mut impl EvmEnv) { SECONDS_PER_YEAR, )); + // POOL B - Tranche 2/2 // Allow investor to locally invest crate::utils::pool::give_role::( investor.into(), @@ -734,6 +737,7 @@ pub fn setup_investors(evm: &mut impl EvmEnv) { SECONDS_PER_YEAR, )); + // POOL C - Tranche 1/1 // Allow investor to locally invest crate::utils::pool::give_role::( investor.into(), From c1b8f81436e01af46df70f32eaf5de49bc35ff7c Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Tue, 6 Aug 2024 09:09:39 +0200 Subject: [PATCH 05/15] docs: improve LP --- pallets/liquidity-pools/src/lib.rs | 46 +++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index 8912f1cb49..cb9c226944 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -352,7 +352,9 @@ pub mod pallet { where ::AccountId: From<[u8; 32]> + Into<[u8; 32]>, { - /// Add a pool to a given domain + /// Add a pool to a given domain. + /// + /// Origin: Pool admin #[pallet::weight(T::WeightInfo::add_pool())] #[pallet::call_index(2)] pub fn add_pool( @@ -386,7 +388,9 @@ pub mod pallet { Ok(()) } - /// Add a tranche to a given domain + /// Add a tranche to a given domain. + /// + /// Origin: Pool admin #[pallet::weight(T::WeightInfo::add_tranche())] #[pallet::call_index(3)] pub fn add_tranche( @@ -491,7 +495,8 @@ pub mod pallet { Ok(()) } - /// Update a member + /// Inform the recipient domain about a new or changed investor + /// validity. #[pallet::weight(T::WeightInfo::update_member())] #[pallet::call_index(5)] pub fn update_member( @@ -669,8 +674,9 @@ pub mod pallet { /// Add a currency to the set of known currencies on the domain derived /// from the given currency. - // TODO: Fix weights - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + /// + /// Origin: Anyone because transmitted data is queried from chain. + #[pallet::weight(T::WeightInfo::add_currency())] #[pallet::call_index(8)] pub fn add_currency(origin: OriginFor, currency_id: T::CurrencyId) -> DispatchResult { let who = ensure_signed(origin)?; @@ -693,6 +699,10 @@ pub mod pallet { /// Allow a currency to be used as a pool currency and to invest in a /// pool on the domain derived from the given currency. + /// + /// Origin: Pool admin for now + /// NOTE: In the future should be permissioned by new trait, see spec + /// #[pallet::call_index(9)] // TODO: Fix weights #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] @@ -701,9 +711,6 @@ pub mod pallet { pool_id: T::PoolId, currency_id: T::CurrencyId, ) -> DispatchResult { - // TODO(future): In the future, should be permissioned by trait which - // does not exist yet. - // See spec: https://centrifuge.hackmd.io/SERpps-URlG4hkOyyS94-w?view#fn-add_pool_currency let who = ensure_signed(origin)?; ensure!( @@ -729,8 +736,11 @@ pub mod pallet { Ok(()) } - /// Schedule an upgrade of an EVM-based liquidity pool contract instance - #[pallet::weight(::WeightInfo::schedule_upgrade())] + /// Schedule an upgrade of an EVM-based liquidity pool contract + /// instance. + /// + /// Origin: root + #[pallet::weight(T::WeightInfo::schedule_upgrade())] #[pallet::call_index(10)] pub fn schedule_upgrade( origin: OriginFor, @@ -747,6 +757,8 @@ pub mod pallet { } /// Schedule an upgrade of an EVM-based liquidity pool contract instance + /// + /// Origin: root #[pallet::weight(T::WeightInfo::cancel_upgrade())] #[pallet::call_index(11)] pub fn cancel_upgrade( @@ -767,7 +779,7 @@ pub mod pallet { /// /// NOTE: Pulls the metadata from the `AssetRegistry` and thus requires /// the pool admin to have updated the tranche tokens metadata there - /// beforehand. + /// beforehand. Therefore, no restrictions on calling origin. #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] #[pallet::call_index(12)] pub fn update_tranche_token_metadata( @@ -798,6 +810,8 @@ pub mod pallet { /// Disallow a currency to be used as a pool currency and to invest in a /// pool on the domain derived from the given currency. + /// + /// Origin: Pool admin #[pallet::call_index(13)] // TODO: Add to weights #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] @@ -831,7 +845,10 @@ pub mod pallet { Ok(()) } - // TODO: Add to weights + /// Block a remote investor from performing investment tasks until lock + /// is removed. + /// + /// Origin: Pool admin #[pallet::call_index(14)] #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] pub fn freeze_investor( @@ -889,7 +906,10 @@ pub mod pallet { Ok(()) } - // TODO: Add to weights + /// Unblock a previously locked remote investor from performing + /// investment tasks. + /// + /// Origin: Pool admin #[pallet::call_index(15)] #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] pub fn unfreeze_investor( From 1f7cf94fb2d735a60ff287111fbd725d4185df98 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Tue, 6 Aug 2024 09:09:57 +0200 Subject: [PATCH 06/15] weights: simplify for LP --- .../liquidity-pools/src/defensive_weights.rs | 134 +++++++----------- pallets/liquidity-pools/src/lib.rs | 15 +- 2 files changed, 62 insertions(+), 87 deletions(-) diff --git a/pallets/liquidity-pools/src/defensive_weights.rs b/pallets/liquidity-pools/src/defensive_weights.rs index 78501303a9..6c49292d45 100644 --- a/pallets/liquidity-pools/src/defensive_weights.rs +++ b/pallets/liquidity-pools/src/defensive_weights.rs @@ -19,9 +19,14 @@ pub trait WeightInfo { fn update_member() -> Weight; fn transfer() -> Weight; fn set_domain_router() -> Weight; + fn add_currency() -> Weight; + fn allow_investment_currency() -> Weight; + fn disallow_investment_currency() -> Weight; fn schedule_upgrade() -> Weight; fn cancel_upgrade() -> Weight; fn update_tranche_token_metadata() -> Weight; + fn freeze_investor() -> Weight; + fn unfreeze_investor() -> Weight; } // NOTE: We use temporary weights here. `execute_epoch` is by far our heaviest @@ -29,112 +34,79 @@ pub trait WeightInfo { // should be enough. const N: u64 = 4; +/// NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will +/// be replaced with real benchmark soon. + +fn default_defensive_weight() -> Weight { + Weight::from_parts(124_979_771, 19974) + .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) +} + impl WeightInfo for () { fn set_domain_router() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn add_pool() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn add_tranche() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn update_token_price() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn update_member() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn transfer() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn schedule_upgrade() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn cancel_upgrade() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() } fn update_tranche_token_metadata() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) + default_defensive_weight() + } + + fn freeze_investor() -> Weight { + default_defensive_weight() + } + + fn unfreeze_investor() -> Weight { + default_defensive_weight() + } + + fn add_currency() -> Weight { + // Reads: 2x AssetRegistry + // Writes: MessageNonceStore, MessageQueue + RocksDbWeight::get().reads_writes(2, 2) + } + + fn allow_investment_currency() -> Weight { + // Reads: 2x AssetRegistry + // Writes: MessageNonceStore, MessageQueue + RocksDbWeight::get().reads_writes(2, 2) + } + + fn disallow_investment_currency() -> Weight { + // Reads: 2x AssetRegistry + // Writes: MessageNonceStore, MessageQueue + RocksDbWeight::get().reads_writes(2, 2) } } diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index cb9c226944..a9216aa9e0 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -704,8 +704,7 @@ pub mod pallet { /// NOTE: In the future should be permissioned by new trait, see spec /// #[pallet::call_index(9)] - // TODO: Fix weights - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + #[pallet::weight(T::WeightInfo::allow_investment_currency())] pub fn allow_investment_currency( origin: OriginFor, pool_id: T::PoolId, @@ -813,8 +812,7 @@ pub mod pallet { /// /// Origin: Pool admin #[pallet::call_index(13)] - // TODO: Add to weights - #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] + #[pallet::weight(T::WeightInfo::disallow_investment_currency())] pub fn disallow_investment_currency( origin: OriginFor, pool_id: T::PoolId, @@ -850,7 +848,7 @@ pub mod pallet { /// /// Origin: Pool admin #[pallet::call_index(14)] - #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] + #[pallet::weight(T::WeightInfo::freeze_investor())] pub fn freeze_investor( origin: OriginFor, pool_id: T::PoolId, @@ -911,7 +909,7 @@ pub mod pallet { /// /// Origin: Pool admin #[pallet::call_index(15)] - #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] + #[pallet::weight(T::WeightInfo::unfreeze_investor())] pub fn unfreeze_investor( origin: OriginFor, pool_id: T::PoolId, @@ -1084,6 +1082,9 @@ pub mod pallet { T::DomainAddressToAccountId::convert(domain_address) } + /// Checks whether the given address has investor permissions with at + /// least the given validity timestamp. Moreover, checks whether the + /// investor is frozen or not. pub fn validate_investor_status( investor: T::AccountId, pool_id: T::PoolId, @@ -1112,6 +1113,8 @@ pub mod pallet { Ok(()) } + /// Checks whether the given address has investor permissions at least + /// to the current timestamp and whether it is not frozen. pub fn validate_investor_can_transfer( investor: T::AccountId, pool_id: T::PoolId, From 108341f9fdeafd302620145bafc1e492ae16f521 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Tue, 6 Aug 2024 09:10:08 +0200 Subject: [PATCH 07/15] fix: clippy --- runtime/centrifuge/src/lib.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/runtime/centrifuge/src/lib.rs b/runtime/centrifuge/src/lib.rs index 8e2c0ac14c..335c1e583d 100644 --- a/runtime/centrifuge/src/lib.rs +++ b/runtime/centrifuge/src/lib.rs @@ -1650,10 +1650,7 @@ impl< P::has( PermissionScope::Pool(pool_id), who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor( - tranche_id.clone(), - T::now().as_secs(), - )), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), ) && !P::has( PermissionScope::Pool(pool_id), who, @@ -1668,10 +1665,7 @@ impl< P::has( PermissionScope::Pool(pool_id), who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor( - tranche_id.clone(), - T::now().as_secs(), - )), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), ) && !P::has( PermissionScope::Pool(pool_id), who, From d613c75159d9a6983723e3a40541be28104e987b Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Thu, 8 Aug 2024 12:58:31 +0200 Subject: [PATCH 08/15] refactor: req perm to be set beforehand --- pallets/liquidity-pools/src/lib.rs | 22 +++----- pallets/liquidity-pools/src/tests/inbound.rs | 56 +++++++++---------- .../src/cases/lp/pool_management.rs | 25 +++++++++ runtime/integration-tests/src/utils/pool.rs | 11 ++++ 4 files changed, 70 insertions(+), 44 deletions(-) diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index a9216aa9e0..4be473b616 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -846,6 +846,9 @@ pub mod pallet { /// Block a remote investor from performing investment tasks until lock /// is removed. /// + /// NOTE: Assumes the remote investor's permissions have been updated to + /// reflect frozenness beforehand. + /// /// Origin: Pool admin #[pallet::call_index(14)] #[pallet::weight(T::WeightInfo::freeze_investor())] @@ -880,13 +883,7 @@ pub mod pallet { pool_id, tranche_id, T::Time::now(), - false, - )?; - - T::Permission::add( - PermissionScope::Pool(pool_id), - local_address, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + true, )?; T::OutboundMessageHandler::handle( @@ -907,6 +904,9 @@ pub mod pallet { /// Unblock a previously locked remote investor from performing /// investment tasks. /// + /// NOTE: Assumes the remote investor's permissions have been updated to + /// reflect an unfrozen state beforehand. + /// /// Origin: Pool admin #[pallet::call_index(15)] #[pallet::weight(T::WeightInfo::unfreeze_investor())] @@ -941,13 +941,7 @@ pub mod pallet { pool_id, tranche_id, T::Time::now(), - true, - )?; - - T::Permission::remove( - PermissionScope::Pool(pool_id), - local_address, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + false, )?; T::OutboundMessageHandler::handle( diff --git a/pallets/liquidity-pools/src/tests/inbound.rs b/pallets/liquidity-pools/src/tests/inbound.rs index 9e0a63d9c5..57e8e34a06 100644 --- a/pallets/liquidity-pools/src/tests/inbound.rs +++ b/pallets/liquidity-pools/src/tests/inbound.rs @@ -845,8 +845,8 @@ mod freeze { Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); - // Default mock has unfrozen investor - false + // Default mock has frozen investor + true } Role::PoolRole(PoolRole::PoolAdmin) => { assert_eq!(who, ALICE); @@ -855,17 +855,6 @@ mod freeze { _ => false, } }); - Permissions::mock_add(|scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - Ok(()) - } - _ => Err(DispatchError::Other("Must only add FrozenTrancheInvestor")), - } - }); Pools::mock_pool_exists(|_| true); Pools::mock_tranche_exists(|_, _| true); Gateway::mock_handle(|sender, destination, msg| { @@ -1016,7 +1005,27 @@ mod freeze { fn with_investor_frozen() { System::externalities().execute_with(|| { config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(move |_, _, _| true); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + false + } + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); assert_noop!( LiquidityPools::freeze_investor( @@ -1054,8 +1063,8 @@ mod unfreeze { Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); - // Default mock has frozen investor - true + // Default mock has unfrozen investor + false } Role::PoolRole(PoolRole::PoolAdmin) => { assert_eq!(who, ALICE); @@ -1064,19 +1073,6 @@ mod unfreeze { _ => false, } }); - Permissions::mock_remove(|scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - Ok(()) - } - _ => Err(DispatchError::Other( - "Must only remove FrozenTrancheInvestor", - )), - } - }); Pools::mock_pool_exists(|_| true); Pools::mock_tranche_exists(|_, _| true); Gateway::mock_handle(|sender, destination, msg| { @@ -1233,7 +1229,7 @@ mod unfreeze { Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); assert_eq!(tranche_id, TRANCHE_ID); - false + true } _ => true, } diff --git a/runtime/integration-tests/src/cases/lp/pool_management.rs b/runtime/integration-tests/src/cases/lp/pool_management.rs index e77a519b33..40f005d45a 100644 --- a/runtime/integration-tests/src/cases/lp/pool_management.rs +++ b/runtime/integration-tests/src/cases/lp/pool_management.rs @@ -35,6 +35,7 @@ use crate::{ utils::{ accounts::Keyring, currency::{register_currency, CurrencyInfo}, + pool::{give_role, remove_role}, }, }; @@ -649,6 +650,14 @@ fn freeze_member() { }); env.state_mut(|_| { + give_role::( + AccountConverter::convert_evm_address( + EVM_DOMAIN_CHAIN_ID, + Keyring::TrancheInvestor(2).into(), + ), + POOL_A, + PoolRole::FrozenTrancheInvestor(pool_a_tranche_1_id::()), + ); assert_ok!(pallet_liquidity_pools::Pallet::::freeze_investor( Keyring::Admin.as_origin(), POOL_A, @@ -688,6 +697,14 @@ fn unfreeze_member() { }); env.state_mut(|_| { + give_role::( + AccountConverter::convert_evm_address( + EVM_DOMAIN_CHAIN_ID, + Keyring::TrancheInvestor(2).into(), + ), + POOL_A, + PoolRole::FrozenTrancheInvestor(pool_a_tranche_1_id::()), + ); assert_ok!(pallet_liquidity_pools::Pallet::::freeze_investor( Keyring::Admin.as_origin(), POOL_A, @@ -714,6 +731,14 @@ fn unfreeze_member() { }); env.state_mut(|_| { + remove_role::( + AccountConverter::convert_evm_address( + EVM_DOMAIN_CHAIN_ID, + Keyring::TrancheInvestor(2).into(), + ), + POOL_A, + PoolRole::FrozenTrancheInvestor(pool_a_tranche_1_id::()), + ); assert_ok!(pallet_liquidity_pools::Pallet::::unfreeze_investor( Keyring::Admin.as_origin(), POOL_A, diff --git a/runtime/integration-tests/src/utils/pool.rs b/runtime/integration-tests/src/utils/pool.rs index ffa4fca521..ccdf417b1a 100644 --- a/runtime/integration-tests/src/utils/pool.rs +++ b/runtime/integration-tests/src/utils/pool.rs @@ -46,6 +46,17 @@ pub fn give_role(dest: AccountId, pool_id: PoolId, role: PoolRole) { .unwrap(); } +pub fn remove_role(dest: AccountId, pool_id: PoolId, role: PoolRole) { + pallet_permissions::Pallet::::remove( + RawOrigin::Root.into(), + Role::PoolRole(role), + dest, + PermissionScope::Pool(pool_id), + Role::PoolRole(role), + ) + .unwrap(); +} + pub fn create_empty(admin: AccountId, pool_id: PoolId, currency_id: CurrencyId) { create::( admin, From 28c3e528a1b7d78a3a325865c187be9277482c1e Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Thu, 8 Aug 2024 12:59:08 +0200 Subject: [PATCH 09/15] chore: apply suggestions from code review --- pallets/investments/src/mock.rs | 7 ++++--- pallets/investments/src/tests.rs | 2 +- pallets/liquidity-pools/src/mock.rs | 1 + runtime/development/src/lib.rs | 10 ++-------- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/pallets/investments/src/mock.rs b/pallets/investments/src/mock.rs index da3c6ed087..d88907e971 100644 --- a/pallets/investments/src/mock.rs +++ b/pallets/investments/src/mock.rs @@ -141,6 +141,9 @@ impl pallet_investments::Config for Runtime { type WeightInfo = (); } +pub(crate) const ERR_PRE_CONDITION: DispatchError = + DispatchError::Other("PreCondition mock fails on u64::MAX account on purpose"); + pub struct AlwaysWithOneException; impl PreConditions for AlwaysWithOneException where @@ -153,9 +156,7 @@ where OrderType::Investment { who, .. } | OrderType::Redemption { who, .. } if who == NOT_INVESTOR => { - Err(DispatchError::Other( - "PreCondition mock fails on u64::MAX account on purpose", - )) + Err(ERR_PRE_CONDITION) } _ => Ok(()), } diff --git a/pallets/investments/src/tests.rs b/pallets/investments/src/tests.rs index ed7eceaea6..f322aec830 100644 --- a/pallets/investments/src/tests.rs +++ b/pallets/investments/src/tests.rs @@ -2961,7 +2961,7 @@ fn collecting_investment_without_preconditions_fails() { TestExternalitiesBuilder::build().execute_with(|| { assert_noop!( Investments::collect_investments(RuntimeOrigin::signed(NOT_INVESTOR), INVESTMENT_0_0), - DispatchError::Other("PreCondition mock fails on u64::MAX account on purpose") + ERR_PRE_CONDITION ); }) } diff --git a/pallets/liquidity-pools/src/mock.rs b/pallets/liquidity-pools/src/mock.rs index 8f62e37b82..f76c5e26ab 100644 --- a/pallets/liquidity-pools/src/mock.rs +++ b/pallets/liquidity-pools/src/mock.rs @@ -27,6 +27,7 @@ pub const ALICE_32: [u8; 32] = [2; 32]; pub const ALICE: AccountId = AccountId::new(ALICE_32); pub const ALICE_ETH: [u8; 20] = [2; 20]; pub const ALICE_EVM_DOMAIN_ADDRESS: DomainAddress = DomainAddress::EVM(42, ALICE_ETH); +// TODO(future): Can be removed after domain conversion refactor pub const ALICE_EVM_LOCAL_ACCOUNT: AccountId = { let mut arr = [0u8; 32]; let mut i = 0; diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index 3df9e78997..6e80cb6741 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -1716,10 +1716,7 @@ impl< P::has( PermissionScope::Pool(pool_id), who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor( - tranche_id.clone(), - T::now().as_secs(), - )), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), ) && !P::has( PermissionScope::Pool(pool_id), who, @@ -1734,10 +1731,7 @@ impl< P::has( PermissionScope::Pool(pool_id), who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor( - tranche_id.clone(), - T::now().as_secs(), - )), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), ) && !P::has( PermissionScope::Pool(pool_id), who, From a5f2e6e272c2b8400190994a4529f97ca012ad46 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Thu, 8 Aug 2024 16:14:08 +0200 Subject: [PATCH 10/15] refactor: move some tests --- pallets/liquidity-pools/src/tests.rs | 462 ++++++++++++++++++- pallets/liquidity-pools/src/tests/inbound.rs | 445 ------------------ 2 files changed, 461 insertions(+), 446 deletions(-) diff --git a/pallets/liquidity-pools/src/tests.rs b/pallets/liquidity-pools/src/tests.rs index 6408457eaa..39d7d3c2ae 100644 --- a/pallets/liquidity-pools/src/tests.rs +++ b/pallets/liquidity-pools/src/tests.rs @@ -290,7 +290,7 @@ mod transfer_tranche_tokens { } #[test] - fn with_wrong_permissions() { + fn with_missing_investor_permissions() { System::externalities().execute_with(|| { config_mocks(); Permissions::mock_has(|_, _, _| false); @@ -308,6 +308,40 @@ mod transfer_tranche_tokens { }) } + #[test] + fn with_frozen_investor_permissions() { + System::externalities().execute_with(|| { + config_mocks(); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + assert_eq!(who, CONTRACT_ACCOUNT_ID); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(tranche_id, TRANCHE_ID); + true + } + _ => false, + } + }); + + assert_noop!( + LiquidityPools::transfer_tranche_tokens( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + EVM_DOMAIN_ADDRESS, + AMOUNT + ), + Error::::InvestorDomainAddressFrozen, + ); + }) + } + #[test] fn with_wrong_pool() { System::externalities().execute_with(|| { @@ -1388,3 +1422,429 @@ mod cancel_upgrade { } } } + +mod freeze { + use sp_runtime::DispatchError; + + use super::*; + use crate::message::UpdateRestrictionMessage; + + fn config_mocks(receiver: DomainAddress) { + DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); + DomainAddressToAccountId::mock_convert(move |_| ALICE_EVM_LOCAL_ACCOUNT); + Time::mock_now(|| NOW); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + // Default mock has frozen investor + true + } + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + Pools::mock_pool_exists(|_| true); + Pools::mock_tranche_exists(|_, _| true); + Gateway::mock_handle(|sender, destination, msg| { + assert_eq!(sender, ALICE); + assert_eq!(destination, ALICE_EVM_DOMAIN_ADDRESS.domain()); + assert_eq!( + msg, + Message::UpdateRestriction { + pool_id: POOL_ID, + tranche_id: TRANCHE_ID, + update: UpdateRestrictionMessage::Freeze { + address: ALICE_EVM_DOMAIN_ADDRESS.address().into() + } + } + ); + Ok(()) + }); + } + + #[test] + fn success() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + + assert_ok!(LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + )); + }); + } + + mod erroring_out { + use super::*; + + #[test] + fn with_bad_origin_unsigned_none() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::none(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + #[test] + fn with_bad_origin_unsigned_root() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::root(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + + #[test] + fn with_pool_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_pool_exists(|_| false); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::PoolNotFound + ); + }); + } + + #[test] + fn with_tranche_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_tranche_exists(|_, _| false); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::TrancheNotFound + ); + }); + } + + #[test] + fn with_origin_not_admin() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(|_, _, _| false); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::NotPoolAdmin + ); + }); + } + + #[test] + fn with_investor_not_member() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressNotAMember + ); + }); + } + + #[test] + fn with_investor_frozen() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + false + } + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + + assert_noop!( + LiquidityPools::freeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressFrozen + ); + }); + } + } +} + +mod unfreeze { + use sp_runtime::DispatchError; + + use super::*; + use crate::message::UpdateRestrictionMessage; + + fn config_mocks(receiver: DomainAddress) { + DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); + DomainAddressToAccountId::mock_convert(move |_| ALICE_EVM_LOCAL_ACCOUNT); + Time::mock_now(|| NOW); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + assert_eq!(validity, NOW_SECS); + true + } + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + // Default mock has unfrozen investor + false + } + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + Pools::mock_pool_exists(|_| true); + Pools::mock_tranche_exists(|_, _| true); + Gateway::mock_handle(|sender, destination, msg| { + assert_eq!(sender, ALICE); + assert_eq!(destination, ALICE_EVM_DOMAIN_ADDRESS.domain()); + assert_eq!( + msg, + Message::UpdateRestriction { + pool_id: POOL_ID, + tranche_id: TRANCHE_ID, + update: UpdateRestrictionMessage::Unfreeze { + address: ALICE_EVM_DOMAIN_ADDRESS.address().into() + } + } + ); + Ok(()) + }); + } + + #[test] + fn success() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + + assert_ok!(LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + )); + }); + } + + mod erroring_out { + use super::*; + + #[test] + fn with_bad_origin_unsigned_none() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::none(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + #[test] + fn with_bad_origin_unsigned_root() { + System::externalities().execute_with(|| { + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::root(), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + DispatchError::BadOrigin + ); + }); + } + + #[test] + fn with_pool_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_pool_exists(|_| false); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::PoolNotFound + ); + }); + } + + #[test] + fn with_tranche_dne() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Pools::mock_tranche_exists(|_, _| false); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::TrancheNotFound + ); + }); + } + + #[test] + fn with_origin_not_admin() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(|_, _, _| false); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::NotPoolAdmin + ); + }); + } + + #[test] + fn with_investor_not_member() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::PoolAdmin) => { + assert_eq!(who, ALICE); + true + } + _ => false, + } + }); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressNotAMember + ); + }); + } + + #[test] + fn with_investor_unfrozen() { + System::externalities().execute_with(|| { + config_mocks(ALICE_EVM_DOMAIN_ADDRESS); + Permissions::mock_has(move |scope, who, role| { + assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); + match role { + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { + assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); + assert_eq!(tranche_id, TRANCHE_ID); + true + } + _ => true, + } + }); + + assert_noop!( + LiquidityPools::unfreeze_investor( + RuntimeOrigin::signed(ALICE), + POOL_ID, + TRANCHE_ID, + ALICE_EVM_DOMAIN_ADDRESS + ), + Error::::InvestorDomainAddressFrozen + ); + }); + } + } +} diff --git a/pallets/liquidity-pools/src/tests/inbound.rs b/pallets/liquidity-pools/src/tests/inbound.rs index 57e8e34a06..bc0dd61cbd 100644 --- a/pallets/liquidity-pools/src/tests/inbound.rs +++ b/pallets/liquidity-pools/src/tests/inbound.rs @@ -257,25 +257,6 @@ mod handle_tranche_tokens_transfer { }) } - #[test] - fn outbound_with_frozen_investor_permissions() { - System::externalities().execute_with(|| { - config_mocks(EVM_DOMAIN_ADDRESS); - Permissions::mock_has(|_, _, _| true); - - assert_noop!( - LiquidityPools::transfer_tranche_tokens( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - EVM_DOMAIN_ADDRESS, - AMOUNT, - ), - Error::::InvestorDomainAddressFrozen, - ); - }) - } - #[test] fn with_wrong_pool() { System::externalities().execute_with(|| { @@ -822,429 +803,3 @@ mod handle_cancel_redeem_order { } } } - -mod freeze { - use sp_runtime::DispatchError; - - use super::*; - use crate::message::UpdateRestrictionMessage; - - fn config_mocks(receiver: DomainAddress) { - DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); - DomainAddressToAccountId::mock_convert(move |_| ALICE_EVM_LOCAL_ACCOUNT); - Time::mock_now(|| NOW); - Permissions::mock_has(move |scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - assert_eq!(validity, NOW_SECS); - true - } - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - // Default mock has frozen investor - true - } - Role::PoolRole(PoolRole::PoolAdmin) => { - assert_eq!(who, ALICE); - true - } - _ => false, - } - }); - Pools::mock_pool_exists(|_| true); - Pools::mock_tranche_exists(|_, _| true); - Gateway::mock_handle(|sender, destination, msg| { - assert_eq!(sender, ALICE); - assert_eq!(destination, ALICE_EVM_DOMAIN_ADDRESS.domain()); - assert_eq!( - msg, - Message::UpdateRestriction { - pool_id: POOL_ID, - tranche_id: TRANCHE_ID, - update: UpdateRestrictionMessage::Freeze { - address: ALICE_EVM_DOMAIN_ADDRESS.address().into() - } - } - ); - Ok(()) - }); - } - - #[test] - fn success() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - - assert_ok!(LiquidityPools::freeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - )); - }); - } - - mod erroring_out { - use super::*; - - #[test] - fn with_bad_origin_unsigned_none() { - System::externalities().execute_with(|| { - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::none(), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - DispatchError::BadOrigin - ); - }); - } - #[test] - fn with_bad_origin_unsigned_root() { - System::externalities().execute_with(|| { - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::root(), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - DispatchError::BadOrigin - ); - }); - } - - #[test] - fn with_pool_dne() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Pools::mock_pool_exists(|_| false); - - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::PoolNotFound - ); - }); - } - - #[test] - fn with_tranche_dne() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Pools::mock_tranche_exists(|_, _| false); - - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::TrancheNotFound - ); - }); - } - - #[test] - fn with_origin_not_admin() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(|_, _, _| false); - - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::NotPoolAdmin - ); - }); - } - - #[test] - fn with_investor_not_member() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(move |scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::PoolAdmin) => { - assert_eq!(who, ALICE); - true - } - _ => false, - } - }); - - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::InvestorDomainAddressNotAMember - ); - }); - } - - #[test] - fn with_investor_frozen() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(move |scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - assert_eq!(validity, NOW_SECS); - true - } - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - false - } - Role::PoolRole(PoolRole::PoolAdmin) => { - assert_eq!(who, ALICE); - true - } - _ => false, - } - }); - - assert_noop!( - LiquidityPools::freeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::InvestorDomainAddressFrozen - ); - }); - } - } -} - -mod unfreeze { - use sp_runtime::DispatchError; - - use super::*; - use crate::message::UpdateRestrictionMessage; - - fn config_mocks(receiver: DomainAddress) { - DomainAccountToDomainAddress::mock_convert(move |_| receiver.clone()); - DomainAddressToAccountId::mock_convert(move |_| ALICE_EVM_LOCAL_ACCOUNT); - Time::mock_now(|| NOW); - Permissions::mock_has(move |scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, validity)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - assert_eq!(validity, NOW_SECS); - true - } - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - // Default mock has unfrozen investor - false - } - Role::PoolRole(PoolRole::PoolAdmin) => { - assert_eq!(who, ALICE); - true - } - _ => false, - } - }); - Pools::mock_pool_exists(|_| true); - Pools::mock_tranche_exists(|_, _| true); - Gateway::mock_handle(|sender, destination, msg| { - assert_eq!(sender, ALICE); - assert_eq!(destination, ALICE_EVM_DOMAIN_ADDRESS.domain()); - assert_eq!( - msg, - Message::UpdateRestriction { - pool_id: POOL_ID, - tranche_id: TRANCHE_ID, - update: UpdateRestrictionMessage::Unfreeze { - address: ALICE_EVM_DOMAIN_ADDRESS.address().into() - } - } - ); - Ok(()) - }); - } - - #[test] - fn success() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - - assert_ok!(LiquidityPools::unfreeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - )); - }); - } - - mod erroring_out { - use super::*; - - #[test] - fn with_bad_origin_unsigned_none() { - System::externalities().execute_with(|| { - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::none(), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - DispatchError::BadOrigin - ); - }); - } - #[test] - fn with_bad_origin_unsigned_root() { - System::externalities().execute_with(|| { - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::root(), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - DispatchError::BadOrigin - ); - }); - } - - #[test] - fn with_pool_dne() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Pools::mock_pool_exists(|_| false); - - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::PoolNotFound - ); - }); - } - - #[test] - fn with_tranche_dne() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Pools::mock_tranche_exists(|_, _| false); - - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::TrancheNotFound - ); - }); - } - - #[test] - fn with_origin_not_admin() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(|_, _, _| false); - - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::NotPoolAdmin - ); - }); - } - - #[test] - fn with_investor_not_member() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(move |scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::PoolAdmin) => { - assert_eq!(who, ALICE); - true - } - _ => false, - } - }); - - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::InvestorDomainAddressNotAMember - ); - }); - } - - #[test] - fn with_investor_unfrozen() { - System::externalities().execute_with(|| { - config_mocks(ALICE_EVM_DOMAIN_ADDRESS); - Permissions::mock_has(move |scope, who, role| { - assert!(matches!(scope, PermissionScope::Pool(POOL_ID))); - match role { - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)) => { - assert_eq!(who, ALICE_EVM_LOCAL_ACCOUNT); - assert_eq!(tranche_id, TRANCHE_ID); - true - } - _ => true, - } - }); - - assert_noop!( - LiquidityPools::unfreeze_investor( - RuntimeOrigin::signed(ALICE), - POOL_ID, - TRANCHE_ID, - ALICE_EVM_DOMAIN_ADDRESS - ), - Error::::InvestorDomainAddressFrozen - ); - }); - } - } -} From 09c4b4c60e4820ecc3940781a3e6c6ccc1a51c55 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Thu, 8 Aug 2024 16:42:26 +0200 Subject: [PATCH 11/15] refactor: common runtime permissions --- runtime/altair/src/lib.rs | 68 +------------------ runtime/centrifuge/src/lib.rs | 63 +---------------- runtime/common/src/lib.rs | 39 +---------- runtime/common/src/permissions.rs | 108 ++++++++++++++++++++++++++++++ runtime/development/src/lib.rs | 63 +---------------- 5 files changed, 116 insertions(+), 225 deletions(-) create mode 100644 runtime/common/src/permissions.rs diff --git a/runtime/altair/src/lib.rs b/runtime/altair/src/lib.rs index 7e96221e91..a40f7517de 100644 --- a/runtime/altair/src/lib.rs +++ b/runtime/altair/src/lib.rs @@ -29,10 +29,7 @@ use cfg_primitives::{ }, LPGatewayQueueMessageNonce, }; -use cfg_traits::{ - investments::OrderManager, Millis, Permissions as PermissionsT, PoolUpdateGuard, PreConditions, - Seconds, -}; +use cfg_traits::{investments::OrderManager, Millis, PoolUpdateGuard, Seconds}; use cfg_types::{ fee_keys::{Fee, FeeKey}, fixed_point::{Quantity, Rate, Ratio}, @@ -80,7 +77,6 @@ use pallet_evm::{ Account as EVMAccount, EnsureAddressNever, EnsureAddressRoot, FeeCalculator, GasWeightMapping, Runner, }; -use pallet_investments::OrderType; use pallet_liquidity_pools_gateway::message::GatewayMessage; pub use pallet_loans::entities::{input::PriceCollectionInput, loans::ActiveLoanInfo}; use pallet_loans::types::cashflow::CashflowPayment; @@ -117,7 +113,7 @@ use runtime_common::{ }, PoolAdmin, Treasurer, }, - permissions::PoolAdminCheck, + permissions::{IsUnfrozenTrancheInvestor, PoolAdminCheck}, remarks::Remark, rewards::SingleCurrencyMovement, transfer_filter::{PreLpTransfer, PreNativeTransfer}, @@ -135,7 +131,7 @@ use sp_runtime::{ Dispatchable, IdentityLookup, PostDispatchInfoOf, UniqueSaturatedInto, Verify, Zero, }, transaction_validity::{TransactionSource, TransactionValidity, TransactionValidityError}, - ApplyExtrinsicResult, DispatchError, DispatchResult, FixedI128, Perbill, Permill, Perquintill, + ApplyExtrinsicResult, DispatchError, FixedI128, Perbill, Permill, Perquintill, }; use sp_staking::currency_to_vote::U128CurrencyToVote; use sp_std::{marker::PhantomData, prelude::*, vec::Vec}; @@ -1672,64 +1668,6 @@ impl pallet_investments::Config for Runtime { type WeightInfo = weights::pallet_investments::WeightInfo; } -/// Checks whether the given `who` has the role -/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool -/// and tranche. -pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); -impl< - P: PermissionsT, Role = Role>, - T: UnixTime, - > PreConditions> for IsUnfrozenTrancheInvestor -{ - type Result = DispatchResult; - - fn check(order: OrderType) -> Self::Result { - let is_tranche_investor = match order { - OrderType::Investment { - who, - investment_id: (pool_id, tranche_id), - .. - } => { - P::has( - PermissionScope::Pool(pool_id), - who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ) && !P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), - ) - } - OrderType::Redemption { - who, - investment_id: (pool_id, tranche_id), - .. - } => { - P::has( - PermissionScope::Pool(pool_id), - who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ) && !P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), - ) - } - }; - - if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { - Ok(()) - } else { - // TODO: We should adapt the permissions pallets interface to return an error - // instead of a boolean. This makes the redundant "does not have role" error, - // which downstream pallets always need to generate, not needed anymore. - Err(DispatchError::Other( - "Account does not have the TrancheInvestor permission.", - )) - } - } -} - parameter_types! { pub const MaxKeys: u32 = 10; pub const DefaultKeyDeposit: Balance = 100 * AIR; diff --git a/runtime/centrifuge/src/lib.rs b/runtime/centrifuge/src/lib.rs index 5db42bcaec..0b81fed741 100644 --- a/runtime/centrifuge/src/lib.rs +++ b/runtime/centrifuge/src/lib.rs @@ -54,7 +54,7 @@ use frame_support::{ construct_runtime, dispatch::DispatchClass, genesis_builder_helper::{build_config, create_default_config}, - pallet_prelude::{DispatchError, DispatchResult, RuntimeDebug}, + pallet_prelude::{DispatchError, RuntimeDebug}, parameter_types, traits::{ fungible::HoldConsideration, @@ -81,7 +81,6 @@ use pallet_evm::{ Account as EVMAccount, EnsureAddressNever, EnsureAddressRoot, FeeCalculator, GasWeightMapping, Runner, }; -use pallet_investments::OrderType; use pallet_liquidity_pools_gateway::message::GatewayMessage; pub use pallet_loans::entities::{input::PriceCollectionInput, loans::ActiveLoanInfo}; use pallet_loans::types::cashflow::CashflowPayment; @@ -115,7 +114,7 @@ use runtime_common::{ origins::gov::types::{ AllOfCouncil, EnsureRootOr, HalfOfCouncil, ThreeFourthOfCouncil, TwoThirdOfCouncil, }, - permissions::PoolAdminCheck, + permissions::{IsUnfrozenTrancheInvestor, PoolAdminCheck}, rewards::SingleCurrencyMovement, transfer_filter::{PreLpTransfer, PreNativeTransfer}, xcm::AccountIdToLocation, @@ -1623,64 +1622,6 @@ parameter_types! { pub const MaxOutstandingCollects: u32 = 10; } -/// Checks whether the given `who` has the role -/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool -/// and tranche. -pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); -impl< - P: PermissionsT, Role = Role>, - T: UnixTime, - > PreConditions> for IsUnfrozenTrancheInvestor -{ - type Result = DispatchResult; - - fn check(order: OrderType) -> Self::Result { - let is_tranche_investor = match order { - OrderType::Investment { - who, - investment_id: (pool_id, tranche_id), - .. - } => { - P::has( - PermissionScope::Pool(pool_id), - who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ) && !P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), - ) - } - OrderType::Redemption { - who, - investment_id: (pool_id, tranche_id), - .. - } => { - P::has( - PermissionScope::Pool(pool_id), - who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ) && !P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), - ) - } - }; - - if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { - Ok(()) - } else { - // TODO: We should adapt the permissions pallets interface to return an error - // instead of a boolean. This makes the redundant "does not have role" error, - // which downstream pallets always need to generate, not needed anymore. - Err(DispatchError::Other( - "Account does not have the TrancheInvestor permission.", - )) - } - } -} - impl pallet_investments::Config for Runtime { type Accountant = PoolSystem; type Amount = Balance; diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index 3ad18a91c4..7acc49d58f 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -40,6 +40,7 @@ pub mod gateway; pub mod migrations; pub mod oracle; pub mod origins; +pub mod permissions; pub mod pool; pub mod remarks; pub mod transfer_filter; @@ -725,44 +726,6 @@ pub mod origin { } } -pub mod permissions { - use cfg_primitives::{AccountId, PoolId}; - use cfg_traits::{Permissions, PreConditions}; - use cfg_types::{ - permissions::{PermissionScope, PoolRole, Role}, - tokens::CurrencyId, - }; - use sp_std::marker::PhantomData; - - /// Check if an account has a pool admin role - pub struct PoolAdminCheck

(PhantomData

); - - impl

PreConditions<(AccountId, PoolId)> for PoolAdminCheck

- where - P: Permissions, Role = Role>, - { - type Result = bool; - - fn check((account_id, pool_id): (AccountId, PoolId)) -> bool { - P::has( - PermissionScope::Pool(pool_id), - account_id, - Role::PoolRole(PoolRole::PoolAdmin), - ) - } - - #[cfg(feature = "runtime-benchmarks")] - fn satisfy((account_id, pool_id): (AccountId, PoolId)) { - P::add( - PermissionScope::Pool(pool_id), - account_id, - Role::PoolRole(PoolRole::PoolAdmin), - ) - .unwrap(); - } - } -} - pub mod rewards { frame_support::parameter_types! { #[derive(scale_info::TypeInfo)] diff --git a/runtime/common/src/permissions.rs b/runtime/common/src/permissions.rs new file mode 100644 index 0000000000..6dcd1d64c8 --- /dev/null +++ b/runtime/common/src/permissions.rs @@ -0,0 +1,108 @@ +// Copyright 2024 Centrifuge Foundation (centrifuge.io). +// +// This file is part of the Centrifuge chain project. +// Centrifuge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version (see http://www.gnu.org/licenses). +// Centrifuge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +use cfg_primitives::{AccountId, Balance, InvestmentId, PoolId}; +use cfg_traits::{Permissions, PreConditions}; +use cfg_types::{ + permissions::{PermissionScope, PoolRole, Role}, + tokens::CurrencyId, +}; +use frame_support::{dispatch::DispatchResult, traits::UnixTime}; +use pallet_investments::OrderType; +use sp_runtime::DispatchError; +use sp_std::marker::PhantomData; + +/// Check if an account has a pool admin role +pub struct PoolAdminCheck

(PhantomData

); + +impl

PreConditions<(AccountId, PoolId)> for PoolAdminCheck

+where + P: Permissions, Role = Role>, +{ + type Result = bool; + + fn check((account_id, pool_id): (AccountId, PoolId)) -> bool { + P::has( + PermissionScope::Pool(pool_id), + account_id, + Role::PoolRole(PoolRole::PoolAdmin), + ) + } + + #[cfg(feature = "runtime-benchmarks")] + fn satisfy((account_id, pool_id): (AccountId, PoolId)) { + P::add( + PermissionScope::Pool(pool_id), + account_id, + Role::PoolRole(PoolRole::PoolAdmin), + ) + .unwrap(); + } +} + +/// Checks whether the given `who` has the role +/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool +/// and tranche. +pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); +impl< + P: Permissions, Role = Role>, + T: UnixTime, + > PreConditions> for IsUnfrozenTrancheInvestor +{ + type Result = DispatchResult; + + fn check(order: OrderType) -> Self::Result { + let is_tranche_investor = match order { + OrderType::Investment { + who, + investment_id: (pool_id, tranche_id), + .. + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } + OrderType::Redemption { + who, + investment_id: (pool_id, tranche_id), + .. + } => { + P::has( + PermissionScope::Pool(pool_id), + who.clone(), + Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), + ) && !P::has( + PermissionScope::Pool(pool_id), + who, + Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), + ) + } + }; + + if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { + Ok(()) + } else { + // TODO: We should adapt the permissions pallets interface to return an error + // instead of a boolean. This makes the redundant "does not have role" error, + // which downstream pallets always need to generate, not needed anymore. + Err(DispatchError::Other( + "Account does not have the TrancheInvestor permission.", + )) + } + } +} diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index c6009fce4b..477c78515f 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -56,7 +56,7 @@ use frame_support::{ construct_runtime, dispatch::DispatchClass, genesis_builder_helper::{build_config, create_default_config}, - pallet_prelude::{DispatchError, DispatchResult, RuntimeDebug}, + pallet_prelude::{DispatchError, RuntimeDebug}, parameter_types, traits::{ fungible::HoldConsideration, @@ -83,7 +83,6 @@ use pallet_evm::{ Account as EVMAccount, EnsureAddressNever, EnsureAddressRoot, FeeCalculator, GasWeightMapping, Runner, }; -use pallet_investments::OrderType; use pallet_liquidity_pools_gateway::message::GatewayMessage; pub use pallet_loans::entities::{input::PriceCollectionInput, loans::ActiveLoanInfo}; use pallet_loans::types::cashflow::CashflowPayment; @@ -122,7 +121,7 @@ use runtime_common::{ }, PoolAdmin, Treasurer, }, - permissions::PoolAdminCheck, + permissions::{IsUnfrozenTrancheInvestor, PoolAdminCheck}, remarks::Remark, rewards::SingleCurrencyMovement, transfer_filter::{PreLpTransfer, PreNativeTransfer}, @@ -1689,64 +1688,6 @@ impl pallet_investments::Config for Runtime { type WeightInfo = (); } -/// Checks whether the given `who` has the role -/// of a `TrancheInvestor` without having `FrozenInvestor` for the given pool -/// and tranche. -pub struct IsUnfrozenTrancheInvestor(PhantomData<(P, T)>); -impl< - P: PermissionsT, Role = Role>, - T: UnixTime, - > PreConditions> for IsUnfrozenTrancheInvestor -{ - type Result = DispatchResult; - - fn check(order: OrderType) -> Self::Result { - let is_tranche_investor = match order { - OrderType::Investment { - who, - investment_id: (pool_id, tranche_id), - .. - } => { - P::has( - PermissionScope::Pool(pool_id), - who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ) && !P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), - ) - } - OrderType::Redemption { - who, - investment_id: (pool_id, tranche_id), - .. - } => { - P::has( - PermissionScope::Pool(pool_id), - who.clone(), - Role::PoolRole(PoolRole::TrancheInvestor(tranche_id, T::now().as_secs())), - ) && !P::has( - PermissionScope::Pool(pool_id), - who, - Role::PoolRole(PoolRole::FrozenTrancheInvestor(tranche_id)), - ) - } - }; - - if is_tranche_investor || cfg!(feature = "runtime-benchmarks") { - Ok(()) - } else { - // TODO: We should adapt the permissions pallets interface to return an error - // instead of a boolean. This makes the redundant "does not have role" error, - // which downstream pallets always need to generate, not needed anymore. - Err(DispatchError::Other( - "Account does not have the TrancheInvestor permission.", - )) - } - } -} - parameter_types! { pub const RewardsPalletId: PalletId = cfg_types::ids::BLOCK_REWARDS_PALLET_ID; pub const RewardCurrency: CurrencyId = CurrencyId::Native; From 9de643fc235ab41ffa53d86a3cb861547f0e1e1b Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Fri, 9 Aug 2024 12:16:08 +0200 Subject: [PATCH 12/15] feat: add perm migration --- libs/types/src/permissions.rs | 62 +++++++++- pallets/permissions/src/lib.rs | 3 + runtime/altair/src/migrations.rs | 18 +++ runtime/centrifuge/src/migrations.rs | 12 ++ runtime/common/src/migrations/mod.rs | 1 + .../common/src/migrations/permissions_v1.rs | 112 ++++++++++++++++++ runtime/development/src/migrations.rs | 18 +++ 7 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 runtime/common/src/migrations/permissions_v1.rs diff --git a/libs/types/src/permissions.rs b/libs/types/src/permissions.rs index 21e40ead9b..5d054d1486 100644 --- a/libs/types/src/permissions.rs +++ b/libs/types/src/permissions.rs @@ -114,11 +114,6 @@ pub struct TrancheInvestorInfo { is_frozen: bool, } -#[derive(Encode, Decode, TypeInfo, Debug, Clone, Eq, PartialEq, MaxEncodedLen)] -pub struct FrozenTrancheInvestorInfo { - tranche_id: TrancheId, -} - #[derive(Encode, Decode, TypeInfo, Debug, Clone, Eq, PartialEq, MaxEncodedLen)] pub struct PermissionedCurrencyHolders { info: Option, @@ -685,3 +680,60 @@ mod tests { assert!(!roles.exists(Role::PoolRole(PoolRole::PODReadAccess))); } } + +pub mod v0 { + use super::*; + + #[derive(Encode, Decode, TypeInfo, Clone, Eq, PartialEq, Debug, MaxEncodedLen)] + pub struct PermissionRoles> { + pool_admin: PoolAdminRoles, + currency_admin: CurrencyAdminRoles, + permissioned_asset_holder: PermissionedCurrencyHolders, + tranche_investor: TrancheInvestors, + } + + #[derive(Encode, Decode, TypeInfo, Debug, Clone, Eq, PartialEq, MaxEncodedLen)] + pub struct TrancheInvestors> { + info: BoundedVec, MaxTranches>, + _phantom: PhantomData<(Now, MinDelay)>, + } + + #[derive(Encode, Decode, TypeInfo, Debug, Clone, Eq, PartialEq, MaxEncodedLen)] + pub struct TrancheInvestorInfo { + tranche_id: TrancheId, + permissioned_till: Seconds, + } + + impl> + TrancheInvestors + { + fn migrate(self) -> super::TrancheInvestors { + super::TrancheInvestors:: { + info: BoundedVec::truncate_from( + self.info + .into_iter() + .map(|info| super::TrancheInvestorInfo { + tranche_id: info.tranche_id, + permissioned_till: info.permissioned_till, + is_frozen: false, + }) + .collect(), + ), + _phantom: self._phantom, + } + } + } + + impl> + PermissionRoles + { + pub fn migrate(self) -> super::PermissionRoles { + super::PermissionRoles { + pool_admin: self.pool_admin, + currency_admin: self.currency_admin, + permissioned_asset_holder: self.permissioned_asset_holder, + tranche_investor: self.tranche_investor.migrate(), + } + } + } +} diff --git a/pallets/permissions/src/lib.rs b/pallets/permissions/src/lib.rs index 4a31981702..9f9b67a17d 100644 --- a/pallets/permissions/src/lib.rs +++ b/pallets/permissions/src/lib.rs @@ -71,7 +71,10 @@ pub mod pallet { type WeightInfo: WeightInfo; } + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::storage] diff --git a/runtime/altair/src/migrations.rs b/runtime/altair/src/migrations.rs index 5152e11c79..6d7950c6f4 100644 --- a/runtime/altair/src/migrations.rs +++ b/runtime/altair/src/migrations.rs @@ -31,4 +31,22 @@ pub type UpgradeAltair1401 = ( 1, 2, >, + // Migrate TrancheInvestor permission role and storage version from v0 to v1 + frame_support::migrations::VersionedMigration< + 0, + 1, + frame_support::migrations::VersionedMigration< + 0, + 1, + runtime_common::migrations::permissions_v1::Migration< + Runtime, + crate::MinDelay, + crate::MaxTranches, + >, + pallet_permissions::Pallet, + ::DbWeight, + >, + pallet_permissions::Pallet, + ::DbWeight, + >, ); diff --git a/runtime/centrifuge/src/migrations.rs b/runtime/centrifuge/src/migrations.rs index d8bd54eab0..d8a378e7e4 100644 --- a/runtime/centrifuge/src/migrations.rs +++ b/runtime/centrifuge/src/migrations.rs @@ -31,4 +31,16 @@ pub type UpgradeCentrifuge1401 = ( 1, 2, >, + // Migrate TrancheInvestor permission role and storage version from v0 to v1 + frame_support::migrations::VersionedMigration< + 0, + 1, + runtime_common::migrations::permissions_v1::Migration< + Runtime, + crate::MinDelay, + crate::MaxTranches, + >, + pallet_permissions::Pallet, + ::DbWeight, + >, ); diff --git a/runtime/common/src/migrations/mod.rs b/runtime/common/src/migrations/mod.rs index aefb459ac9..8a2dbcc331 100644 --- a/runtime/common/src/migrations/mod.rs +++ b/runtime/common/src/migrations/mod.rs @@ -16,6 +16,7 @@ pub mod foreign_investments_v2; pub mod increase_storage_version; pub mod liquidity_pools_gateway; pub mod nuke; +pub mod permissions_v1; pub mod precompile_account_codes; pub mod restricted_location; pub mod technical_comittee; diff --git a/runtime/common/src/migrations/permissions_v1.rs b/runtime/common/src/migrations/permissions_v1.rs new file mode 100644 index 0000000000..b05db63ea4 --- /dev/null +++ b/runtime/common/src/migrations/permissions_v1.rs @@ -0,0 +1,112 @@ +// Copyright 2024 Centrifuge Foundation (centrifuge.io). +// +// This file is part of the Centrifuge chain project. +// Centrifuge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version (see http://www.gnu.org/licenses). +// Centrifuge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +use cfg_primitives::{PoolId, TrancheId}; +use cfg_types::{ + permissions::{v0::PermissionRoles as PermissionRolesV0, PermissionRoles, PermissionScope}, + time::TimeProvider, + tokens::CurrencyId, +}; +#[cfg(feature = "try-runtime")] +use frame_support::pallet_prelude::{Decode, Encode}; +use frame_support::{ + storage_alias, + traits::{Get, OnRuntimeUpgrade}, + weights::Weight, + Blake2_128Concat, +}; +use sp_arithmetic::traits::SaturatedConversion; +#[cfg(feature = "try-runtime")] +use sp_std::vec::Vec; + +mod v0 { + use super::*; + + #[storage_alias] + pub type Permission = + StorageDoubleMap< + pallet_permissions::Pallet, + Blake2_128Concat, + ::AccountId, + Blake2_128Concat, + PermissionScope, + PermissionRolesV0>, MinD, TrancheId, MaxT>, + >; +} + +const LOG_PREFIX: &str = "PermssionsV1"; + +pub struct Migration( + sp_std::marker::PhantomData<(T, MinDelay, MaxTranches)>, +); + +impl OnRuntimeUpgrade for Migration +where + T: pallet_permissions::Config< + Storage = PermissionRoles< + TimeProvider>, + MinDelay, + TrancheId, + MaxTranches, + >, + > + frame_system::Config + + pallet_timestamp::Config, + MinDelay: 'static, + MaxTranches: Get + 'static, +{ + fn on_runtime_upgrade() -> Weight { + let writes = v0::Permission::::iter_keys() + .count() + .saturated_into(); + + pallet_permissions::Permission::::translate_values::< + PermissionRolesV0< + TimeProvider>, + MinDelay, + TrancheId, + MaxTranches, + >, + _, + >(|role| Some(role.migrate().into())); + + log::info!("{LOG_PREFIX}: Migrated {writes} permissions!"); + T::DbWeight::get().reads_writes(1, writes) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + let count: u64 = v0::Permission::::iter_keys() + .count() + .saturated_into(); + + log::info!("{LOG_PREFIX}: Pre checks done!"); + + Ok(count.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(pre_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let pre_count: u64 = Decode::decode(&mut pre_state.as_slice()) + .expect("pre_upgrade provides a valid state; qed"); + let post_count: u64 = pallet_permissions::Permission::::iter_keys() + .count() + .saturated_into(); + assert_eq!( + pre_count, post_count, + "{LOG_PREFIX}: Mismatching number of permission roles after migration!" + ); + + log::info!("{LOG_PREFIX}: Post checks done!"); + + Ok(()) + } +} diff --git a/runtime/development/src/migrations.rs b/runtime/development/src/migrations.rs index f25201cc3f..2fd87478f0 100644 --- a/runtime/development/src/migrations.rs +++ b/runtime/development/src/migrations.rs @@ -29,4 +29,22 @@ pub type UpgradeDevelopment1401 = ( 1, 2, >, + // Migrate TrancheInvestor permission role and storage version from v0 to v1 + frame_support::migrations::VersionedMigration< + 0, + 1, + frame_support::migrations::VersionedMigration< + 0, + 1, + runtime_common::migrations::permissions_v1::Migration< + Runtime, + crate::MinDelay, + crate::MaxTranches, + >, + pallet_permissions::Pallet, + ::DbWeight, + >, + pallet_permissions::Pallet, + ::DbWeight, + >, ); From 73ab016f13c8056a6d6270466951dbe4e0f56b26 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Fri, 9 Aug 2024 12:16:41 +0200 Subject: [PATCH 13/15] feat-creep: rm nonce > 0 pre-upgrade condition in gateway migration --- runtime/common/src/migrations/liquidity_pools_gateway.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/runtime/common/src/migrations/liquidity_pools_gateway.rs b/runtime/common/src/migrations/liquidity_pools_gateway.rs index ab3914c43a..6677c2fc3d 100644 --- a/runtime/common/src/migrations/liquidity_pools_gateway.rs +++ b/runtime/common/src/migrations/liquidity_pools_gateway.rs @@ -73,11 +73,6 @@ where #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { // Extra check to confirm that the storage alias is correct. - assert!( - OutboundMessageNonceStore::::get() > 0, - "{LOG_PREFIX}: OutboundMessageNonce should be > 0" - ); - assert_eq!( OutboundMessageQueue::::iter_keys().count(), 0, From 13451f5a9396f2b540f187141026c70fdc12456e Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Fri, 9 Aug 2024 12:19:47 +0200 Subject: [PATCH 14/15] trigger CI after re-disabling DRAFT --- runtime/common/src/migrations/permissions_v1.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/common/src/migrations/permissions_v1.rs b/runtime/common/src/migrations/permissions_v1.rs index b05db63ea4..17831042a2 100644 --- a/runtime/common/src/migrations/permissions_v1.rs +++ b/runtime/common/src/migrations/permissions_v1.rs @@ -79,6 +79,7 @@ where >(|role| Some(role.migrate().into())); log::info!("{LOG_PREFIX}: Migrated {writes} permissions!"); + T::DbWeight::get().reads_writes(1, writes) } From c87eba6c96fbba0309a1bb012ca690ad8d49a0c2 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Fri, 9 Aug 2024 13:59:56 +0200 Subject: [PATCH 15/15] fix: unwanted nesting + fmt --- runtime/altair/src/migrations.rs | 14 ++++---------- runtime/common/src/migrations/permissions_v1.rs | 2 +- runtime/development/src/migrations.rs | 14 ++++---------- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/runtime/altair/src/migrations.rs b/runtime/altair/src/migrations.rs index 6d7950c6f4..c405dda164 100644 --- a/runtime/altair/src/migrations.rs +++ b/runtime/altair/src/migrations.rs @@ -35,16 +35,10 @@ pub type UpgradeAltair1401 = ( frame_support::migrations::VersionedMigration< 0, 1, - frame_support::migrations::VersionedMigration< - 0, - 1, - runtime_common::migrations::permissions_v1::Migration< - Runtime, - crate::MinDelay, - crate::MaxTranches, - >, - pallet_permissions::Pallet, - ::DbWeight, + runtime_common::migrations::permissions_v1::Migration< + Runtime, + crate::MinDelay, + crate::MaxTranches, >, pallet_permissions::Pallet, ::DbWeight, diff --git a/runtime/common/src/migrations/permissions_v1.rs b/runtime/common/src/migrations/permissions_v1.rs index 17831042a2..df0ab7da26 100644 --- a/runtime/common/src/migrations/permissions_v1.rs +++ b/runtime/common/src/migrations/permissions_v1.rs @@ -76,7 +76,7 @@ where MaxTranches, >, _, - >(|role| Some(role.migrate().into())); + >(|role| Some(role.migrate())); log::info!("{LOG_PREFIX}: Migrated {writes} permissions!"); diff --git a/runtime/development/src/migrations.rs b/runtime/development/src/migrations.rs index 2fd87478f0..257c6e8915 100644 --- a/runtime/development/src/migrations.rs +++ b/runtime/development/src/migrations.rs @@ -33,16 +33,10 @@ pub type UpgradeDevelopment1401 = ( frame_support::migrations::VersionedMigration< 0, 1, - frame_support::migrations::VersionedMigration< - 0, - 1, - runtime_common::migrations::permissions_v1::Migration< - Runtime, - crate::MinDelay, - crate::MaxTranches, - >, - pallet_permissions::Pallet, - ::DbWeight, + runtime_common::migrations::permissions_v1::Migration< + Runtime, + crate::MinDelay, + crate::MaxTranches, >, pallet_permissions::Pallet, ::DbWeight,