diff --git a/substrate/frame/referenda-tracks/src/lib.rs b/substrate/frame/referenda-tracks/src/lib.rs index 648bdde7a00e..02ff882d7e11 100644 --- a/substrate/frame/referenda-tracks/src/lib.rs +++ b/substrate/frame/referenda-tracks/src/lib.rs @@ -64,12 +64,17 @@ type TracksIter = Map< #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{pallet_prelude::*, traits::EnsureOrigin}; + use frame_support::{ + pallet_prelude::*, + traits::{EnsureOrigin, EnsureOriginWithArg}, + }; use frame_system::pallet_prelude::*; #[pallet::config] pub trait Config: frame_system::Config + pallet_referenda::Config { - type UpdateOrigin: EnsureOrigin; + type AdminOrigin: EnsureOrigin; + + type UpdateOrigin: EnsureOriginWithArg>; type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -102,7 +107,7 @@ pub mod pallet { /// A new track has been inserted Created { id: TrackIdOf }, /// The information for a track has been updated - Updated { id: TrackIdOf, info: TrackInfoOf }, + Updated { id: TrackIdOf }, /// A track has been removed Removed { id: TrackIdOf }, } @@ -138,7 +143,7 @@ pub mod pallet { info: TrackInfoOf, pallet_origin: PalletsOriginOf, ) -> DispatchResultWithPostInfo { - T::UpdateOrigin::ensure_origin(origin)?; + T::AdminOrigin::ensure_origin(origin)?; ensure!(Tracks::::get(id) == None, Error::::TrackIdAlreadyExisting); @@ -165,19 +170,19 @@ pub mod pallet { id: TrackIdOf, info: TrackInfoOf, ) -> DispatchResultWithPostInfo { - T::UpdateOrigin::ensure_origin(origin)?; + T::UpdateOrigin::ensure_origin(origin, &id)?; Tracks::::try_mutate(id, |track| { if track.is_none() { return Err(Error::::TrackIdNotFound); }; - *track = Some(info.clone()); + *track = Some(info); Ok(()) })?; - Self::deposit_event(Event::Updated { id, info }); + Self::deposit_event(Event::Updated { id }); Ok(().into()) } @@ -190,19 +195,22 @@ pub mod pallet { /// /// Weight: `O(n)` #[pallet::call_index(2)] - pub fn remove(origin: OriginFor, id: TrackIdOf) -> DispatchResultWithPostInfo { - T::UpdateOrigin::ensure_origin(origin)?; + pub fn remove( + origin: OriginFor, + id: TrackIdOf, + pallet_origin: PalletsOriginOf, + ) -> DispatchResultWithPostInfo { + T::AdminOrigin::ensure_origin(origin)?; ensure!(Tracks::::contains_key(id), Error::::TrackIdNotFound); + ensure!( + OriginToTrackId::::get(&pallet_origin) == Some(id), + DispatchError::BadOrigin + ); - OriginToTrackId::::iter_keys().for_each(|origin| { - let track_id = OriginToTrackId::::get(origin.clone()) - .expect("The given key is provided by StorageMap::iter_keys; qed"); - if track_id == id { - OriginToTrackId::::remove(origin); - } - }); Tracks::::remove(id); + OriginToTrackId::::remove(pallet_origin); + TracksIds::::try_mutate(|tracks_ids| { let new_tracks_ids = tracks_ids.clone().into_iter().filter(|i| i != &id).collect(); *tracks_ids = BoundedVec::<_, _>::truncate_from(new_tracks_ids); diff --git a/substrate/frame/referenda-tracks/src/mock.rs b/substrate/frame/referenda-tracks/src/mock.rs index 8dae217235f3..672c058a885b 100644 --- a/substrate/frame/referenda-tracks/src/mock.rs +++ b/substrate/frame/referenda-tracks/src/mock.rs @@ -21,11 +21,11 @@ use crate as pallet_referenda_tracks; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ ord_parameter_types, parameter_types, - traits::{ConstU16, ConstU32, ConstU64, EqualPrivilegeOnly, VoteTally}, + traits::{ConstU16, ConstU32, ConstU64, EnsureOriginWithArg, EqualPrivilegeOnly, VoteTally}, weights::Weight, }; use frame_system::{EnsureRoot, EnsureSignedBy}; -use pallet_referenda::{PalletsOriginOf, TrackIdOf, TrackInfoOf}; +use pallet_referenda::{PalletsOriginOf, TrackIdOf, TrackInfoOf, TracksInfo}; use scale_info::TypeInfo; use sp_core::H256; use sp_io::TestExternalities; @@ -113,6 +113,22 @@ impl pallet_balances::Config for Test { type MaxHolds = (); } +pub struct EnsureOriginToTrack; +impl EnsureOriginWithArg> for EnsureOriginToTrack { + type Success = (); + + fn try_origin( + o: RuntimeOrigin, + id: &TrackIdOf, + ) -> Result { + let track_id_for_origin: TrackIdOf = + Tracks::track_for(&o.clone().caller).map_err(|_| o.clone())?; + frame_support::ensure!(&track_id_for_origin == id, o); + + Ok(()) + } +} + parameter_types! { pub const MaxTracks: u32 = 2; } @@ -120,7 +136,8 @@ impl pallet_referenda_tracks::Config for Test { type TrackId = u8; type RuntimeEvent = RuntimeEvent; type MaxTracks = MaxTracks; - type UpdateOrigin = EnsureRoot; + type AdminOrigin = EnsureRoot; + type UpdateOrigin = EnsureOriginToTrack; type WeightInfo = (); } diff --git a/substrate/frame/referenda-tracks/src/tests.rs b/substrate/frame/referenda-tracks/src/tests.rs index 1d85210df573..fecf5f7661bc 100644 --- a/substrate/frame/referenda-tracks/src/tests.rs +++ b/substrate/frame/referenda-tracks/src/tests.rs @@ -156,16 +156,6 @@ mod update { }); } - #[test] - fn fails_if_non_existing() { - new_test_ext(None).execute_with(|| { - assert_noop!( - ReferendaTracks::::update(RuntimeOrigin::root(), 1, TRACK), - Error::::TrackIdNotFound, - ); - }); - } - #[test] fn it_works() { new_test_ext(Some(vec![(1, TRACK, ORIGIN_SIGNED_1)])).execute_with(|| { @@ -173,7 +163,7 @@ mod update { track.max_deciding = 2; assert_ok!(ReferendaTracks::::update( - RuntimeOrigin::root(), + RuntimeOrigin::signed(1), 1, track.clone() )); @@ -182,7 +172,7 @@ mod update { System::events(), vec![EventRecord { phase: Phase::Initialization, - event: RuntimeEvent::Tracks(Event::Updated { id: 1, info: track.clone() }), + event: RuntimeEvent::Tracks(Event::Updated { id: 1 }), topics: vec![], }], ); @@ -199,7 +189,7 @@ mod remove { fn fails_if_incorrect_origin() { new_test_ext(None).execute_with(|| { assert_noop!( - ReferendaTracks::::remove(RuntimeOrigin::signed(1), 1), + ReferendaTracks::::remove(RuntimeOrigin::signed(1), 1, ORIGIN_SIGNED_1), BadOrigin ); }); @@ -209,7 +199,7 @@ mod remove { fn fails_if_non_existing() { new_test_ext(None).execute_with(|| { assert_noop!( - ReferendaTracks::::remove(RuntimeOrigin::root(), 1), + ReferendaTracks::::remove(RuntimeOrigin::root(), 1, ORIGIN_SIGNED_1), Error::::TrackIdNotFound, ); }); @@ -218,7 +208,7 @@ mod remove { #[test] fn it_works() { new_test_ext(Some(vec![(1, TRACK, ORIGIN_SIGNED_1)])).execute_with(|| { - assert_ok!(ReferendaTracks::::remove(RuntimeOrigin::root(), 1)); + assert_ok!(ReferendaTracks::::remove(RuntimeOrigin::root(), 1, ORIGIN_SIGNED_1)); assert_eq!( System::events(),