From 12566d0652653614cdabb7898f7d92519af2560a Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Mon, 4 Mar 2024 14:07:49 +0100 Subject: [PATCH] refactor: split PoolFees trait into *Mutate and *Inspect --- libs/traits/src/fee.rs | 13 ++++++- pallets/pool-fees/src/benchmarking.rs | 2 +- pallets/pool-fees/src/lib.rs | 18 ++++++++-- pallets/pool-fees/src/tests.rs | 47 +++++++++++++++++++++++++ pallets/pool-registry/src/lib.rs | 46 ++++++++++++++++++------ pallets/pool-registry/src/mock.rs | 31 ++++++++++++++-- pallets/pool-system/src/benchmarking.rs | 2 +- pallets/pool-system/src/impls.rs | 2 +- pallets/pool-system/src/lib.rs | 18 +++++----- runtime/altair/src/lib.rs | 1 + runtime/centrifuge/src/lib.rs | 1 + runtime/development/src/lib.rs | 1 + 12 files changed, 154 insertions(+), 28 deletions(-) diff --git a/libs/traits/src/fee.rs b/libs/traits/src/fee.rs index b53ffd6106..da2ebae8c7 100644 --- a/libs/traits/src/fee.rs +++ b/libs/traits/src/fee.rs @@ -29,7 +29,7 @@ pub enum PoolFeeBucket { } /// Trait to add fees to a pool -pub trait PoolFees { +pub trait PoolFeesMutate { type PoolId; type FeeInfo; @@ -37,11 +37,22 @@ pub trait PoolFees { /// /// NOTE: Assumes call permissions are separately checked beforehand. fn add_fee(pool_id: Self::PoolId, bucket: PoolFeeBucket, fee: Self::FeeInfo) -> DispatchResult; +} + +/// Trait to get pool fees counts +pub trait PoolFeesInspect { + type PoolId; + + /// Returns the maximum number of pool fees required for accurate weights + fn get_max_fee_count() -> u32; /// Returns the maximum number of pool fees per bucket required for accurate /// weights fn get_max_fees_per_bucket() -> u32; + /// Returns the current amount of active fees for the given pool + fn get_pool_fee_count(pool: Self::PoolId) -> u32; + /// Returns the current amount of active fees for the given pool and bucket /// pair fn get_pool_fee_bucket_count(pool: Self::PoolId, bucket: PoolFeeBucket) -> u32; diff --git a/pallets/pool-fees/src/benchmarking.rs b/pallets/pool-fees/src/benchmarking.rs index 197885700f..4aef893183 100644 --- a/pallets/pool-fees/src/benchmarking.rs +++ b/pallets/pool-fees/src/benchmarking.rs @@ -14,7 +14,7 @@ use cfg_traits::{ benchmarking::{PoolBenchmarkHelper, PoolFeesBenchmarkHelper}, changes::ChangeGuard, - fee::{PoolFeeBucket, PoolFees as _}, + fee::{PoolFeeBucket, PoolFeesMutate as _}, }; use cfg_types::pools::PoolFeeEditor; use frame_benchmarking::v2::*; diff --git a/pallets/pool-fees/src/lib.rs b/pallets/pool-fees/src/lib.rs index 6ff04e79c4..718577ea3d 100644 --- a/pallets/pool-fees/src/lib.rs +++ b/pallets/pool-fees/src/lib.rs @@ -30,7 +30,7 @@ pub mod pallet { use cfg_traits::benchmarking::PoolFeesBenchmarkHelper; use cfg_traits::{ changes::ChangeGuard, - fee::{FeeAmountProration, PoolFeeBucket, PoolFees}, + fee::{FeeAmountProration, PoolFeeBucket, PoolFeesInspect, PoolFeesMutate}, EpochTransitionHook, PoolInspect, PoolNAV, PoolReserve, PreConditions, Seconds, TimeAsSecs, }; use cfg_types::{ @@ -803,7 +803,7 @@ pub mod pallet { } } - impl PoolFees for Pallet { + impl PoolFeesMutate for Pallet { type FeeInfo = PoolFeeInfoOf; type PoolId = T::PoolId; @@ -815,6 +815,14 @@ pub mod pallet { let fee_id = Self::generate_fee_id()?; Self::add_fee_with_id(pool_id, fee_id, bucket, fee) } + } + + impl PoolFeesInspect for Pallet { + type PoolId = T::PoolId; + + fn get_max_fee_count() -> u32 { + T::MaxFeesPerPool::get() + } fn get_max_fees_per_bucket() -> u32 { T::MaxPoolFeesPerBucket::get() @@ -823,6 +831,12 @@ pub mod pallet { fn get_pool_fee_bucket_count(pool: Self::PoolId, bucket: PoolFeeBucket) -> u32 { ActiveFees::::get(pool, bucket).len().saturated_into() } + + fn get_pool_fee_count(pool: Self::PoolId) -> u32 { + PoolFeeBucket::iter().fold(0u32, |count, bucket| { + count.saturating_add(Self::get_pool_fee_bucket_count(pool, bucket)) + }) + } } impl EpochTransitionHook for Pallet { diff --git a/pallets/pool-fees/src/tests.rs b/pallets/pool-fees/src/tests.rs index a7040cc315..06285e0a4e 100644 --- a/pallets/pool-fees/src/tests.rs +++ b/pallets/pool-fees/src/tests.rs @@ -1365,3 +1365,50 @@ mod disbursements { } } } + +mod inspect { + use cfg_traits::fee::PoolFeesInspect; + + use super::*; + use crate::mock::{default_chargeable_fee, NAV}; + + #[test] + fn max_fee_count() { + ExtBuilder::default().set_aum(NAV).build().execute_with(|| { + assert_eq!( + PoolFees::get_max_fee_count(), + ::MaxFeesPerPool::get() + ); + }) + } + + #[test] + fn max_pool_fees_per_bucket() { + ExtBuilder::default().set_aum(NAV).build().execute_with(|| { + assert_eq!( + PoolFees::get_max_fees_per_bucket(), + ::MaxPoolFeesPerBucket::get() + ); + }) + } + + #[test] + fn pool_fee_count() { + ExtBuilder::default().set_aum(NAV).build().execute_with(|| { + assert_eq!(PoolFees::get_pool_fee_count(POOL), 0u32); + + add_fees(vec![default_chargeable_fee()]); + assert_eq!(PoolFees::get_pool_fee_count(POOL), 1u32); + }) + } + + #[test] + fn bucket_pool_fee_count() { + ExtBuilder::default().set_aum(NAV).build().execute_with(|| { + assert_eq!(PoolFees::get_pool_fee_bucket_count(POOL, BUCKET), 0u32); + + add_fees(vec![default_chargeable_fee()]); + assert_eq!(PoolFees::get_pool_fee_bucket_count(POOL, BUCKET), 1u32); + }) + } +} diff --git a/pallets/pool-registry/src/lib.rs b/pallets/pool-registry/src/lib.rs index 751eb34565..c1e7e2e632 100644 --- a/pallets/pool-registry/src/lib.rs +++ b/pallets/pool-registry/src/lib.rs @@ -14,8 +14,9 @@ #![allow(clippy::too_many_arguments)] use cfg_traits::{ - fee::PoolFeeBucket, investments::TrancheCurrency, Permissions, PoolMutate, - PoolWriteOffPolicyMutate, UpdateState, + fee::{PoolFeeBucket, PoolFeesInspect}, + investments::TrancheCurrency, + Permissions, PoolMutate, PoolWriteOffPolicyMutate, UpdateState, }; use cfg_types::{ permissions::{PermissionScope, PoolRole, Role}, @@ -166,6 +167,9 @@ pub mod pallet { CustomMetadata = CustomMetadata, >; + /// The source of truth for the pool fees counters; + type PoolFeesInspect: PoolFeesInspect; + /// Weight Information type WeightInfo: WeightInfo; } @@ -239,7 +243,10 @@ pub mod pallet { /// Returns an error if the requested pool ID is already in /// use, or if the tranche configuration cannot be used. #[allow(clippy::too_many_arguments)] - #[pallet::weight(T::WeightInfo::register(tranche_inputs.len().try_into().unwrap_or(u32::MAX)))] + #[pallet::weight(T::WeightInfo::register( + tranche_inputs.len().try_into().unwrap_or(u32::MAX), + pool_fees.len().try_into().unwrap_or(u32::MAX)) + )] #[transactional] #[pallet::call_index(0)] pub fn register( @@ -308,7 +315,13 @@ pub mod pallet { /// /// The caller must have the `PoolAdmin` role in order to /// invoke this extrinsic. - #[pallet::weight(T::WeightInfo::update_no_execution(T::MaxTranches::get()).max(T::WeightInfo::update_and_execute(T::MaxTranches::get())))] + #[pallet::weight(T::WeightInfo::update_no_execution( + T::MaxTranches::get(), + T::PoolFeesInspect::get_max_fee_count()).max( + T::WeightInfo::update_and_execute(T::MaxTranches::get(), + T::PoolFeesInspect::get_max_fee_count() + )) + )] #[pallet::call_index(1)] pub fn update( origin: OriginFor, @@ -339,14 +352,20 @@ pub mod pallet { Self::deposit_event(Event::UpdateRegistered { pool_id }); let weight = match state { - UpdateState::NoExecution => T::WeightInfo::update_no_execution(0), + UpdateState::NoExecution => T::WeightInfo::update_no_execution(0, 0), UpdateState::Executed(num_tranches) => { Self::deposit_event(Event::UpdateExecuted { pool_id }); - T::WeightInfo::update_and_execute(num_tranches) + T::WeightInfo::update_and_execute( + num_tranches, + T::PoolFeesInspect::get_pool_fee_count(pool_id), + ) } UpdateState::Stored(num_tranches) => { Self::deposit_event(Event::UpdateStored { pool_id }); - T::WeightInfo::update_no_execution(num_tranches) + T::WeightInfo::update_no_execution( + num_tranches, + T::PoolFeesInspect::get_pool_fee_count(pool_id), + ) } }; Ok(Some(weight).into()) @@ -358,7 +377,10 @@ pub mod pallet { /// and, if required, if there are no outstanding /// redeem orders. If both apply, then the scheduled /// changes are applied. - #[pallet::weight(T::WeightInfo::execute_update(T::MaxTranches::get()))] + #[pallet::weight(T::WeightInfo::execute_update( + T::MaxTranches::get(), + T::PoolFeesInspect::get_max_fee_count() + ))] #[pallet::call_index(2)] pub fn execute_update( origin: OriginFor, @@ -367,14 +389,18 @@ pub mod pallet { ensure_signed(origin)?; let num_tranches = T::ModifyPool::execute_update(pool_id)?; - Ok(Some(T::WeightInfo::execute_update(num_tranches)).into()) + let num_fees = T::PoolFeesInspect::get_pool_fee_count(pool_id); + Ok(Some(T::WeightInfo::execute_update(num_tranches, num_fees)).into()) } /// Sets the IPFS hash for the pool metadata information. /// /// The caller must have the `PoolAdmin` role in order to /// invoke this extrinsic. - #[pallet::weight(T::WeightInfo::set_metadata(metadata.len().try_into().unwrap_or(u32::MAX)))] + #[pallet::weight(T::WeightInfo::set_metadata( + metadata.len().try_into().unwrap_or(u32::MAX), + T::PoolFeesInspect::get_max_fee_count() + ))] #[pallet::call_index(3)] pub fn set_metadata( origin: OriginFor, diff --git a/pallets/pool-registry/src/mock.rs b/pallets/pool-registry/src/mock.rs index fae60dce5f..a17826fe8f 100644 --- a/pallets/pool-registry/src/mock.rs +++ b/pallets/pool-registry/src/mock.rs @@ -19,8 +19,9 @@ use cfg_primitives::{ TrancheWeight, }; use cfg_traits::{ - fee::PoolFeeBucket, investments::OrderManager, Millis, PoolMutate, PoolUpdateGuard, - PreConditions, Seconds, UpdateState, + fee::{PoolFeeBucket, PoolFeesInspect}, + investments::OrderManager, + Millis, PoolMutate, PoolUpdateGuard, PreConditions, Seconds, UpdateState, }; use cfg_types::{ fixed_point::{Quantity, Rate}, @@ -212,6 +213,7 @@ impl pallet_pool_fees::Config for Test { type RuntimeEvent = RuntimeEvent; type Time = Timestamp; type Tokens = OrmlTokens; + type WeightInfo = (); } parameter_types! { @@ -251,7 +253,7 @@ where >; type PoolFeeInput = ( PoolFeeBucket, - <::PoolFees as cfg_traits::fee::PoolFees>::FeeInfo, + <::PoolFees as cfg_traits::fee::PoolFeesMutate>::FeeInfo, ); type TrancheInput = TrancheInput< ::Rate, @@ -297,6 +299,28 @@ impl PreConditions for Always { } } +/// NOTE: Amounts only used for weight determination +pub struct MockPoolFeesInspect; +impl PoolFeesInspect for MockPoolFeesInspect { + type PoolId = PoolId; + + fn get_max_fee_count() -> u32 { + 100 + } + + fn get_max_fees_per_bucket() -> u32 { + 100 + } + + fn get_pool_fee_count(_pool: Self::PoolId) -> u32 { + 100 + } + + fn get_pool_fee_bucket_count(_pool: Self::PoolId, _bucket: PoolFeeBucket) -> u32 { + 100 + } +} + impl Config for Test { type AssetRegistry = RegistryMock; type Balance = Balance; @@ -310,6 +334,7 @@ impl Config for Test { type ModifyWriteOffPolicy = MockWriteOffPolicy; type Permission = PermissionsMock; type PoolCreateOrigin = EnsureSigned; + type PoolFeesInspect = MockPoolFeesInspect; type PoolId = u64; type RuntimeEvent = RuntimeEvent; type TrancheCurrency = TrancheCurrency; diff --git a/pallets/pool-system/src/benchmarking.rs b/pallets/pool-system/src/benchmarking.rs index 0d6fc39a6f..63a4792ea9 100644 --- a/pallets/pool-system/src/benchmarking.rs +++ b/pallets/pool-system/src/benchmarking.rs @@ -15,7 +15,7 @@ use cfg_primitives::PoolEpochId; use cfg_traits::{ benchmarking::PoolFeesBenchmarkHelper, - fee::{PoolFeeBucket, PoolFees}, + fee::{PoolFeeBucket, PoolFeesInspect, PoolFeesMutate}, investments::TrancheCurrency as _, UpdateState, }; diff --git a/pallets/pool-system/src/impls.rs b/pallets/pool-system/src/impls.rs index 984d2d53e5..b8ba8c2c1f 100644 --- a/pallets/pool-system/src/impls.rs +++ b/pallets/pool-system/src/impls.rs @@ -12,7 +12,7 @@ use cfg_traits::{ changes::ChangeGuard, - fee::{PoolFeeBucket, PoolFees}, + fee::{PoolFeeBucket, PoolFeesMutate}, investments::{InvestmentAccountant, TrancheCurrency}, CurrencyPair, PoolUpdateGuard, PriceValue, TrancheTokenPrice, UpdateState, }; diff --git a/pallets/pool-system/src/lib.rs b/pallets/pool-system/src/lib.rs index aacf7500a1..cc1a55b06b 100644 --- a/pallets/pool-system/src/lib.rs +++ b/pallets/pool-system/src/lib.rs @@ -177,7 +177,7 @@ impl Default for Release { #[frame_support::pallet] pub mod pallet { use cfg_traits::{ - fee::{PoolFeeBucket, PoolFees}, + fee::{PoolFeeBucket, PoolFeesInspect, PoolFeesMutate}, investments::{OrderManager, TrancheCurrency as TrancheCurrencyT}, EpochTransitionHook, PoolUpdateGuard, }; @@ -324,14 +324,14 @@ pub mod pallet { type Time: TimeAsSecs; /// Add pool fees - type PoolFees: PoolFees< - FeeInfo = PoolFeeInfo< - ::AccountId, - Self::Balance, - Self::Rate, - >, - PoolId = Self::PoolId, - >; + type PoolFees: PoolFeesMutate< + FeeInfo = PoolFeeInfo< + ::AccountId, + Self::Balance, + Self::Rate, + >, + PoolId = Self::PoolId, + > + PoolFeesInspect; /// Epoch transition hook required for Pool Fees type OnEpochTransition: EpochTransitionHook< diff --git a/runtime/altair/src/lib.rs b/runtime/altair/src/lib.rs index dd6965b601..ab3e308fa0 100644 --- a/runtime/altair/src/lib.rs +++ b/runtime/altair/src/lib.rs @@ -1505,6 +1505,7 @@ impl pallet_pool_registry::Config for Runtime { type ModifyWriteOffPolicy = pallet_loans::Pallet; type Permission = Permissions; type PoolCreateOrigin = EnsureRoot; + type PoolFeesInspect = PoolFees; type PoolId = PoolId; type RuntimeEvent = RuntimeEvent; type TrancheCurrency = TrancheCurrency; diff --git a/runtime/centrifuge/src/lib.rs b/runtime/centrifuge/src/lib.rs index 2215dab348..43cca58daa 100644 --- a/runtime/centrifuge/src/lib.rs +++ b/runtime/centrifuge/src/lib.rs @@ -1461,6 +1461,7 @@ impl pallet_pool_registry::Config for Runtime { type ModifyWriteOffPolicy = pallet_loans::Pallet; type Permission = Permissions; type PoolCreateOrigin = EnsureRoot; + type PoolFeesInspect = PoolFees; type PoolId = PoolId; type RuntimeEvent = RuntimeEvent; type TrancheCurrency = TrancheCurrency; diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index 9e28eccb9b..820feb386d 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -1092,6 +1092,7 @@ impl pallet_pool_registry::Config for Runtime { type ModifyWriteOffPolicy = pallet_loans::Pallet; type Permission = Permissions; type PoolCreateOrigin = EnsureSigned; + type PoolFeesInspect = PoolFees; type PoolId = PoolId; type RuntimeEvent = RuntimeEvent; type TrancheCurrency = TrancheCurrency;