diff --git a/pallets/pool-fees/src/lib.rs b/pallets/pool-fees/src/lib.rs index 5e1f4f2c0f..f7a167ef85 100644 --- a/pallets/pool-fees/src/lib.rs +++ b/pallets/pool-fees/src/lib.rs @@ -327,16 +327,18 @@ pub mod pallet { bucket: PoolFeeBucket, fee: PoolFeeInfoOf, ) -> DispatchResult { - let who = ensure_signed(origin)?; + let who = ensure_signed_or_root(origin)?; ensure!( T::PoolReserve::pool_exists(pool_id), Error::::PoolNotFound ); - ensure!( - T::IsPoolAdmin::check((who, pool_id)), - Error::::NotPoolAdmin - ); + if let Some(signer) = who { + ensure!( + T::IsPoolAdmin::check((signer, pool_id)), + Error::::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, 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::::UnauthorizedEdit ); Self::do_remove_fee(fee_id)?; diff --git a/pallets/pool-fees/src/mock.rs b/pallets/pool-fees/src/mock.rs index 28da32d9b2..780f46668b 100644 --- a/pallets/pool-fees/src/mock.rs +++ b/pallets/pool-fees/src/mock.rs @@ -219,6 +219,16 @@ pub fn default_fixed_fee() -> PoolFeeInfoOf { }) } +pub fn root_editor_fee() -> PoolFeeInfoOf { + PoolFeeInfoOf:: { + destination: DESTINATION, + editor: PoolFeeEditor::Root, + fee_type: PoolFeeType::Fixed { + limit: PoolFeeAmount::ShareOfPortfolioValuation(Rate::saturating_from_rational(1, 10)), + }, + } +} + pub fn default_chargeable_fee() -> PoolFeeInfoOf { new_fee(PoolFeeType::ChargedUpTo { limit: PoolFeeAmount::AmountPerSecond(1), diff --git a/pallets/pool-fees/src/tests.rs b/pallets/pool-fees/src/tests.rs index 06285e0a4e..7a4ea0fbd6 100644 --- a/pallets/pool-fees/src/tests.rs +++ b/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::::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::::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::::UnauthorizedEdit ); } + assert_noop!( + PoolFees::remove_fee(RuntimeOrigin::root(), 1), + Error::::UnauthorizedEdit + ); }) } @@ -313,6 +372,10 @@ mod extrinsics { PoolFees::charge_fee(RuntimeOrigin::signed(account), 1, 1000), Error::::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::::UnauthorizedCharge ); + assert_noop!( + PoolFees::uncharge_fee(RuntimeOrigin::root(), 1, 1000), + DispatchError::BadOrigin + ); } }) }