fix(referenda-tracks): implement requested changes

substrate/frame/referenda-tracks/src/lib.rs

@@ -64,12 +64,17 @@ type TracksIter<T, I> = 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<I: 'static = ()>: frame_system::Config + pallet_referenda::Config<I> {
-		type UpdateOrigin: EnsureOrigin<Self::RuntimeOrigin>;
+		type AdminOrigin: EnsureOrigin<Self::RuntimeOrigin>;
+
+		type UpdateOrigin: EnsureOriginWithArg<Self::RuntimeOrigin, TrackIdOf<Self, I>>;
 
 		type RuntimeEvent: From<Event<Self, I>>
 			+ IsType<<Self as frame_system::Config>::RuntimeEvent>;
@@ -102,7 +107,7 @@ pub mod pallet {
 		/// A new track has been inserted
 		Created { id: TrackIdOf<T, I> },
 		/// The information for a track has been updated
-		Updated { id: TrackIdOf<T, I>, info: TrackInfoOf<T, I> },
+		Updated { id: TrackIdOf<T, I> },
 		/// A track has been removed
 		Removed { id: TrackIdOf<T, I> },
 	}
@@ -138,7 +143,7 @@ pub mod pallet {
 			info: TrackInfoOf<T, I>,
 			pallet_origin: PalletsOriginOf<T>,
 		) -> DispatchResultWithPostInfo {
-			T::UpdateOrigin::ensure_origin(origin)?;
+			T::AdminOrigin::ensure_origin(origin)?;
 
 			ensure!(Tracks::<T, I>::get(id) == None, Error::<T, I>::TrackIdAlreadyExisting);
 
@@ -165,19 +170,19 @@ pub mod pallet {
 			id: TrackIdOf<T, I>,
 			info: TrackInfoOf<T, I>,
 		) -> DispatchResultWithPostInfo {
-			T::UpdateOrigin::ensure_origin(origin)?;
+			T::UpdateOrigin::ensure_origin(origin, &id)?;
 
 			Tracks::<T, I>::try_mutate(id, |track| {
 				if track.is_none() {
 					return Err(Error::<T, I>::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<T>, id: TrackIdOf<T, I>) -> DispatchResultWithPostInfo {
-			T::UpdateOrigin::ensure_origin(origin)?;
+		pub fn remove(
+			origin: OriginFor<T>,
+			id: TrackIdOf<T, I>,
+			pallet_origin: PalletsOriginOf<T>,
+		) -> DispatchResultWithPostInfo {
+			T::AdminOrigin::ensure_origin(origin)?;
 
 			ensure!(Tracks::<T, I>::contains_key(id), Error::<T, I>::TrackIdNotFound);
+			ensure!(
+				OriginToTrackId::<T, I>::get(&pallet_origin) == Some(id),
+				DispatchError::BadOrigin
+			);
 
-			OriginToTrackId::<T, I>::iter_keys().for_each(|origin| {
-				let track_id = OriginToTrackId::<T, I>::get(origin.clone())
-					.expect("The given key is provided by StorageMap::iter_keys; qed");
-				if track_id == id {
-					OriginToTrackId::<T, I>::remove(origin);
-				}
-			});
 			Tracks::<T, I>::remove(id);
+			OriginToTrackId::<T, I>::remove(pallet_origin);
+
 			TracksIds::<T, I>::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);

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,14 +113,31 @@ impl pallet_balances::Config for Test {
 	type MaxHolds = ();
 }
 
+pub struct EnsureOriginToTrack;
+impl EnsureOriginWithArg<RuntimeOrigin, TrackIdOf<Test, ()>> for EnsureOriginToTrack {
+	type Success = ();
+
+	fn try_origin(
+		o: RuntimeOrigin,
+		id: &TrackIdOf<Test, ()>,
+	) -> Result<Self::Success, RuntimeOrigin> {
+		let track_id_for_origin: TrackIdOf<Test, ()> =
+			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;
 }
 impl pallet_referenda_tracks::Config for Test {
 	type TrackId = u8;
 	type RuntimeEvent = RuntimeEvent;
 	type MaxTracks = MaxTracks;
-	type UpdateOrigin = EnsureRoot<u64>;
+	type AdminOrigin = EnsureRoot<u64>;
+	type UpdateOrigin = EnsureOriginToTrack;
 	type WeightInfo = ();
 }
 

substrate/frame/referenda-tracks/src/tests.rs

@@ -156,24 +156,14 @@ mod update {
 		});
 	}
 
-	#[test]
-	fn fails_if_non_existing() {
-		new_test_ext(None).execute_with(|| {
-			assert_noop!(
-				ReferendaTracks::<Test, ()>::update(RuntimeOrigin::root(), 1, TRACK),
-				Error::<Test, ()>::TrackIdNotFound,
-			);
-		});
-	}
-
 	#[test]
 	fn it_works() {
 		new_test_ext(Some(vec![(1, TRACK, ORIGIN_SIGNED_1)])).execute_with(|| {
 			let mut track = TRACK.clone();
 			track.max_deciding = 2;
 
 			assert_ok!(ReferendaTracks::<Test, ()>::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::<Test, ()>::remove(RuntimeOrigin::signed(1), 1),
+				ReferendaTracks::<Test, ()>::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::<Test, ()>::remove(RuntimeOrigin::root(), 1),
+				ReferendaTracks::<Test, ()>::remove(RuntimeOrigin::root(), 1, ORIGIN_SIGNED_1),
 				Error::<Test, ()>::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::<Test, ()>::remove(RuntimeOrigin::root(), 1));
+			assert_ok!(ReferendaTracks::<Test, ()>::remove(RuntimeOrigin::root(), 1, ORIGIN_SIGNED_1));
 
 			assert_eq!(
 				System::events(),