fix: allow root to propose and remove fees (#1855)

pallets/pool-fees/src/lib.rs

@@ -327,16 +327,18 @@ pub mod pallet {
 			bucket: PoolFeeBucket,
 			fee: PoolFeeInfoOf<T>,
 		) -> DispatchResult {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed_or_root(origin)?;
 
 			ensure!(
 				T::PoolReserve::pool_exists(pool_id),
 				Error::<T>::PoolNotFound
 			);
-			ensure!(
-				T::IsPoolAdmin::check((who, pool_id)),
-				Error::<T>::NotPoolAdmin
-			);
+			if let Some(signer) = who {
+				ensure!(
+					T::IsPoolAdmin::check((signer, pool_id)),
+					Error::<T>::NotPoolAdmin
+				);
+			}
 
 			let fee_id = Self::generate_fee_id()?;
 			T::ChangeGuard::note(
@@ -385,11 +387,16 @@ pub mod pallet {
 		#[pallet::call_index(2)]
 		#[pallet::weight(T::WeightInfo::remove_fee(T::MaxPoolFeesPerBucket::get()))]
 		pub fn remove_fee(origin: OriginFor<T>, fee_id: T::FeeId) -> DispatchResult {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed_or_root(origin)?;
 
 			let fee = Self::get_active_fee(fee_id)?;
+			//
 			ensure!(
-				matches!(fee.editor, PoolFeeEditor::Account(account) if account == who),
+				match (fee.editor, who) {
+					(PoolFeeEditor::Account(editor), Some(signer)) => editor == signer,
+					(PoolFeeEditor::Root, None) => true,
+					_ => false,
+				},
 				Error::<T>::UnauthorizedEdit
 			);
 			Self::do_remove_fee(fee_id)?;

pallets/pool-fees/src/mock.rs

@@ -219,6 +219,16 @@ pub fn default_fixed_fee() -> PoolFeeInfoOf<Runtime> {
 	})
 }
 
+pub fn root_editor_fee() -> PoolFeeInfoOf<Runtime> {
+	PoolFeeInfoOf::<Runtime> {
+		destination: DESTINATION,
+		editor: PoolFeeEditor::Root,
+		fee_type: PoolFeeType::Fixed {
+			limit: PoolFeeAmount::ShareOfPortfolioValuation(Rate::saturating_from_rational(1, 10)),
+		},
+	}
+}
+
 pub fn default_chargeable_fee() -> PoolFeeInfoOf<Runtime> {
 	new_fee(PoolFeeType::ChargedUpTo {
 		limit: PoolFeeAmount::AmountPerSecond(1),

pallets/pool-fees/src/tests.rs

@@ -16,9 +16,10 @@ mod extrinsics {
 
 	mod should_work {
 		use super::*;
+		use crate::mock::{default_chargeable_fee, root_editor_fee};
 
 		#[test]
-		fn propose_new_fee_works() {
+		fn propose_new_fee_works_signed() {
 			ExtBuilder::default().build().execute_with(|| {
 				let fees = default_fees();
 
@@ -49,6 +50,29 @@ mod extrinsics {
 			})
 		}
 
+		#[test]
+		fn propose_new_fee_works_root() {
+			ExtBuilder::default().build().execute_with(|| {
+				let fee = default_chargeable_fee();
+
+				assert_ok!(PoolFees::propose_new_fee(
+					RuntimeOrigin::root(),
+					POOL,
+					BUCKET,
+					fee.clone()
+				));
+				System::assert_last_event(
+					Event::<Runtime>::Proposed {
+						fee_id: 1u64,
+						pool_id: POOL,
+						bucket: BUCKET,
+						fee,
+					}
+					.into(),
+				);
+			})
+		}
+
 		#[test]
 		fn apply_new_fee_works() {
 			ExtBuilder::default().build().execute_with(|| {
@@ -73,7 +97,7 @@ mod extrinsics {
 		}
 
 		#[test]
-		fn remove_only_fee_works() {
+		fn remove_single_account_editor_fee_works() {
 			ExtBuilder::default().build().execute_with(|| {
 				add_fees(vec![default_fixed_fee()]);
 
@@ -90,6 +114,24 @@ mod extrinsics {
 			})
 		}
 
+		#[test]
+		fn remove_single_root_editor_fee_works() {
+			ExtBuilder::default().build().execute_with(|| {
+				add_fees(vec![root_editor_fee()]);
+
+				assert_ok!(PoolFees::remove_fee(RuntimeOrigin::root(), 1));
+
+				System::assert_last_event(
+					Event::<Runtime>::Removed {
+						pool_id: POOL,
+						bucket: BUCKET,
+						fee_id: 1,
+					}
+					.into(),
+				);
+			})
+		}
+
 		#[test]
 		fn remove_fee_works() {
 			ExtBuilder::default().build().execute_with(|| {
@@ -257,6 +299,19 @@ mod extrinsics {
 			})
 		}
 
+		#[test]
+		fn apply_new_fee_from_root() {
+			ExtBuilder::default().build().execute_with(|| {
+				config_change_mocks(&default_fixed_fee());
+
+				// Cannot be called from root
+				assert_noop!(
+					PoolFees::apply_new_fee(RuntimeOrigin::root(), POOL, CHANGE_ID),
+					DispatchError::BadOrigin
+				);
+			})
+		}
+
 		#[test]
 		fn apply_new_fee_missing_pool() {
 			ExtBuilder::default().build().execute_with(|| {
@@ -280,6 +335,10 @@ mod extrinsics {
 						Error::<Runtime>::UnauthorizedEdit
 					);
 				}
+				assert_noop!(
+					PoolFees::remove_fee(RuntimeOrigin::root(), 1),
+					Error::<Runtime>::UnauthorizedEdit
+				);
 			})
 		}
 
@@ -313,6 +372,10 @@ mod extrinsics {
 						PoolFees::charge_fee(RuntimeOrigin::signed(account), 1, 1000),
 						Error::<Runtime>::UnauthorizedCharge
 					);
+					assert_noop!(
+						PoolFees::charge_fee(RuntimeOrigin::root(), 1, 1000),
+						DispatchError::BadOrigin
+					);
 				}
 			})
 		}
@@ -364,6 +427,10 @@ mod extrinsics {
 						PoolFees::uncharge_fee(RuntimeOrigin::signed(account), 1, 1000),
 						Error::<Runtime>::UnauthorizedCharge
 					);
+					assert_noop!(
+						PoolFees::uncharge_fee(RuntimeOrigin::root(), 1, 1000),
+						DispatchError::BadOrigin
+					);
 				}
 			})
 		}