different approach

substrate/frame/broker/src/coretime_interface.rs

@@ -22,7 +22,7 @@ use frame_support::Parameter;
 use scale_info::TypeInfo;
 use sp_arithmetic::traits::AtLeast32BitUnsigned;
 use sp_core::RuntimeDebug;
-use sp_runtime::traits::BlockNumberProvider;
+use sp_runtime::traits::{BadOrigin, BlockNumberProvider};
 use sp_std::vec::Vec;
 
 /// Index of a Polkadot Core.
@@ -146,3 +146,18 @@ impl CoretimeInterface for () {
 	#[cfg(feature = "runtime-benchmarks")]
 	fn ensure_notify_revenue_info(_when: RCBlockNumberOf<Self>, _revenue: Self::Balance) {}
 }
+
+/// TODO
+pub trait TaskAccountInterface {
+	/// TODO
+	type AccountId;
+	/// TODO
+	type OuterOrigin: Into<Result<Self::TaskOrigin, Self::OuterOrigin>>;
+	/// TODO
+	type TaskOrigin;
+
+	/// TODO
+	fn ensure_task_sovereign_account(o: Self::OuterOrigin) -> Result<TaskId, BadOrigin>;
+	/// TODO
+	fn sovereign_account(task: TaskId) -> Self::AccountId;
+}

substrate/frame/broker/src/dispatchable_impls.rs

@@ -470,7 +470,7 @@ impl<T: Config> Pallet<T> {
 	}
 
 	pub(crate) fn do_enable_auto_renew(
-		who: T::AccountId,
+		task: TaskId,
 		core: CoreIndex,
 		workload_end_hint: Option<Timeslice>,
 	) -> DispatchResult {
@@ -484,7 +484,7 @@ impl<T: Config> Pallet<T> {
 			if let Some(record) =
 				AllowedRenewals::<T>::get(AllowedRenewalId { core, when: sale.region_begin })
 			{
-				Self::do_renew(who.clone(), core)?;
+				Self::do_renew(T::SovereignAccountOf::sovereign_account(task), core)?;
 				record
 			} else {
 				// If we couldn't find the renewal record for the current bulk period we should
@@ -506,22 +506,22 @@ impl<T: Config> Pallet<T> {
 			return Err(Error::<T>::NotAllowed.into())
 		};
 
-		let CoreAssignment::Task(task_id) = schedule_item.assignment else {
+		if let CoreAssignment::Task(core_task) = schedule_item.assignment {
+			// Sovereign account of a task can only enable auto renewal for its own core.
+			ensure!(task == core_task, Error::<T>::NoPermission);
+		} else {
 			return Err(Error::<T>::NonTaskAutoRenewal.into())
 		};
 
-		// Only the sovereign account of the task can enable auto-renewal.
-		ensure!(who == T::SovereignAccountOf::convert(task_id), Error::<T>::NoPermission);
-
 		AutoRenewals::<T>::try_mutate(|renewals| {
 			let pos = renewals
 				.binary_search_by(|r: &(CoreIndex, TaskId)| r.0.cmp(&core))
 				.unwrap_or_else(|e| e);
-			renewals.try_insert(pos, (core, task_id))
+			renewals.try_insert(pos, (core, task))
 		})
 		.map_err(|_| Error::<T>::TooManyAutoRenewals)?;
 
-		Self::deposit_event(Event::AutoRenewalEnabled { core, task: task_id });
+		Self::deposit_event(Event::AutoRenewalEnabled { core, task });
 		Ok(())
 	}
 

substrate/frame/broker/src/lib.rs

@@ -101,7 +101,10 @@ pub mod pallet {
 
 		/// Type used for getting the associated account of a task. This account is controlled by
 		/// the task itself.
-		type SovereignAccountOf: Convert<TaskId, Self::AccountId>;
+		type SovereignAccountOf: TaskAccountInterface<
+			AccountId = Self::AccountId,
+			OuterOrigin = Self::RuntimeOrigin,
+		>;
 
 		/// Identifier from which the internal Pot is generated.
 		#[pallet::constant]
@@ -842,6 +845,8 @@ pub mod pallet {
 		/// Callable by the account associated with the task on the specified core. This account
 		/// will be charged at the start of every bulk period for renewing core time.
 		///
+		/// - `origin`: Must be the sovereign account of the task
+		/// - `core`: The core for which we want to enable auto renewal.
 		/// The optional `workload_end_hint` parameter should be used when enabling auto-renewal for
 		/// the core which holds a legacy lease, as it would be inefficient to look it up otherwise.
 		#[pallet::call_index(20)]
@@ -851,8 +856,9 @@ pub mod pallet {
 			core: CoreIndex,
 			workload_end_hint: Option<Timeslice>,
 		) -> DispatchResult {
-			let who = ensure_signed(origin)?;
-			Self::do_enable_auto_renew(who, core, workload_end_hint)?;
+			// Only the sovereign account of the task can enable auto-renewal.
+			let task = T::SovereignAccountOf::ensure_task_sovereign_account(origin)?;
+			Self::do_enable_auto_renew(task, core, workload_end_hint)?;
 
 			// The caller must pay for the transaction otherwise spamming would be possible by
 			// turning auto-renewal on and off.

substrate/frame/broker/src/mock.rs

@@ -27,11 +27,11 @@ use frame_support::{
 	},
 	PalletId,
 };
-use frame_system::{EnsureRoot, EnsureSignedBy};
+use frame_system::{EnsureRoot, EnsureSignedBy, RawOrigin};
 use sp_arithmetic::Perbill;
 use sp_core::{ConstU32, ConstU64, Get};
 use sp_runtime::{
-	traits::{BlockNumberProvider, Convert, Identity},
+	traits::{BadOrigin, BlockNumberProvider, Identity},
 	BuildStorage, Saturating,
 };
 use sp_std::collections::btree_map::BTreeMap;
@@ -187,10 +187,21 @@ ord_parameter_types! {
 }
 type EnsureOneOrRoot = EitherOfDiverse<EnsureRoot<u64>, EnsureSignedBy<One, u64>>;
 
-pub struct TaskToAccountId;
+pub struct TaskSovereignAccount;
 // Dummy implementation which converts `TaskId` to `AccountId`.
-impl Convert<TaskId, u64> for TaskToAccountId {
-	fn convert(task: TaskId) -> u64 {
+impl TaskAccountInterface for TaskSovereignAccount {
+	type AccountId = u64;
+	type OuterOrigin = RuntimeOrigin;
+	type TaskOrigin = RawOrigin<u64>;
+
+	fn ensure_task_sovereign_account(o: RuntimeOrigin) -> Result<TaskId, BadOrigin> {
+		match o.into() {
+			Ok(RawOrigin::Signed(account)) => Ok(account as TaskId),
+			_ => Err(BadOrigin),
+		}
+	}
+
+	fn sovereign_account(task: TaskId) -> u64 {
 		task.into()
 	}
 }
@@ -208,7 +219,7 @@ impl crate::Config for Test {
 	type PalletId = TestBrokerId;
 	type AdminOrigin = EnsureOneOrRoot;
 	type PriceAdapter = Linear;
-	type SovereignAccountOf = TaskToAccountId;
+	type SovereignAccountOf = TaskSovereignAccount;
 	type MaxAutoRenewals = ConstU32<5>;
 }
 

substrate/frame/broker/src/tests.rs

@@ -1457,6 +1457,9 @@ fn enable_auto_renew_works() {
 	});
 }
 
+#[test]
+fn enable_auto_renewal_with_end_hint_works() {}
+
 #[test]
 fn enable_auto_renew_renews() {
 	TestExt::new().endow(1, 1000).execute_with(|| {

substrate/frame/broker/src/tick_impls.rs

@@ -21,7 +21,7 @@ use sp_arithmetic::{
 	traits::{One, SaturatedConversion, Saturating, Zero},
 	FixedPointNumber,
 };
-use sp_runtime::traits::{Convert, ConvertBack};
+use sp_runtime::traits::ConvertBack;
 use sp_std::{vec, vec::Vec};
 use CompletionStatus::Complete;
 
@@ -336,7 +336,7 @@ impl<T: Config> Pallet<T> {
 	pub(crate) fn renew_cores() {
 		let renewals = AutoRenewals::<T>::get();
 		renewals.into_iter().for_each(|(core, task)| {
-			let payer = T::SovereignAccountOf::convert(task);
+			let payer = T::SovereignAccountOf::sovereign_account(task);
 			if let Err(_) = Self::do_renew(payer.clone(), core) {
 				Self::deposit_event(Event::<T>::AutoRenewalFailed { core, task, payer });
 			}