refactor: split PoolFees trait into *Mutate and *Inspect

libs/traits/src/fee.rs

@@ -29,19 +29,30 @@ pub enum PoolFeeBucket {
 }
 
 /// Trait to add fees to a pool
-pub trait PoolFees {
+pub trait PoolFeesMutate {
 	type PoolId;
 	type FeeInfo;
 
 	/// Add a new fee to the pool and bucket.
 	///
 	/// 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;

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::*;

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<T: Config> PoolFees for Pallet<T> {
+	impl<T: Config> PoolFeesMutate for Pallet<T> {
 		type FeeInfo = PoolFeeInfoOf<T>;
 		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<T: Config> PoolFeesInspect for Pallet<T> {
+		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::<T>::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<T: Config> EpochTransitionHook for Pallet<T> {

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(),
+				<Runtime as Config>::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(),
+				<Runtime as Config>::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);
+		})
+	}
+}

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<PoolId = Self::PoolId>;
+
 		/// 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<T>,
@@ -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<T>,
@@ -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<T>,

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,
-		<<T as pallet_pool_system::Config>::PoolFees as cfg_traits::fee::PoolFees>::FeeInfo,
+		<<T as pallet_pool_system::Config>::PoolFees as cfg_traits::fee::PoolFeesMutate>::FeeInfo,
 	);
 	type TrancheInput = TrancheInput<
 		<T as pallet_pool_system::Config>::Rate,
@@ -297,6 +299,28 @@ impl<T> PreConditions<T> 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<u64>;
+	type PoolFeesInspect = MockPoolFeesInspect;
 	type PoolId = u64;
 	type RuntimeEvent = RuntimeEvent;
 	type TrancheCurrency = TrancheCurrency;

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,
 };

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,
 };

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<
-				<Self as frame_system::Config>::AccountId,
-				Self::Balance,
-				Self::Rate,
-			>,
-			PoolId = Self::PoolId,
-		>;
+		type PoolFees: PoolFeesMutate<
+				FeeInfo = PoolFeeInfo<
+					<Self as frame_system::Config>::AccountId,
+					Self::Balance,
+					Self::Rate,
+				>,
+				PoolId = Self::PoolId,
+			> + PoolFeesInspect<PoolId = Self::PoolId>;
 
 		/// Epoch transition hook required for Pool Fees
 		type OnEpochTransition: EpochTransitionHook<

runtime/altair/src/lib.rs

@@ -1505,6 +1505,7 @@ impl pallet_pool_registry::Config for Runtime {
 	type ModifyWriteOffPolicy = pallet_loans::Pallet<Self>;
 	type Permission = Permissions;
 	type PoolCreateOrigin = EnsureRoot<AccountId>;
+	type PoolFeesInspect = PoolFees;
 	type PoolId = PoolId;
 	type RuntimeEvent = RuntimeEvent;
 	type TrancheCurrency = TrancheCurrency;

runtime/centrifuge/src/lib.rs

@@ -1461,6 +1461,7 @@ impl pallet_pool_registry::Config for Runtime {
 	type ModifyWriteOffPolicy = pallet_loans::Pallet<Self>;
 	type Permission = Permissions;
 	type PoolCreateOrigin = EnsureRoot<AccountId>;
+	type PoolFeesInspect = PoolFees;
 	type PoolId = PoolId;
 	type RuntimeEvent = RuntimeEvent;
 	type TrancheCurrency = TrancheCurrency;

runtime/development/src/lib.rs

@@ -1092,6 +1092,7 @@ impl pallet_pool_registry::Config for Runtime {
 	type ModifyWriteOffPolicy = pallet_loans::Pallet<Self>;
 	type Permission = Permissions;
 	type PoolCreateOrigin = EnsureSigned<AccountId>;
+	type PoolFeesInspect = PoolFees;
 	type PoolId = PoolId;
 	type RuntimeEvent = RuntimeEvent;
 	type TrancheCurrency = TrancheCurrency;