diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index bd17c96b6353..8223aa77bb25 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -499,6 +499,12 @@ pub mod pallet { InvalidConfig, /// The revenue must be claimed for 1 or more timeslices. NoClaimTimeslices, + /// The caller doesn't have the permission to enable or disable auto-renewal. + NoPermission, + /// We reached the limit for auto-renewals. + TooManyAutoRenewals, + /// Only cores which are assigned to a task can be auto-renewed. + NonTaskAutoRenewal, } #[pallet::hooks] @@ -825,15 +831,13 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::notify_core_count())] pub fn enable_auto_renew(origin: OriginFor, core: CoreIndex) -> DispatchResult { let who = ensure_signed(origin)?; - // TODO: ensure origin is the parachain's sovereign account. let sale = SaleInfo::::get().ok_or(Error::::NoSales)?; - // If the core hasn't been renewed yet we will renew it now. let record = if let Some(record) = AllowedRenewals::::get(AllowedRenewalId { core, when: sale.region_begin }) { - Self::do_renew(who, core)?; + Self::do_renew(who.clone(), core)?; record } else { // If we couldn't find the renewal record for the current bulk period we should be @@ -846,22 +850,28 @@ pub mod pallet { let workload = record.completion.drain_complete().ok_or(Error::::IncompleteAssignment)?; + // Given that only non-interlaced cores can be renewed, there should be only one // assignment in the core's workload. ensure!(workload.len() == 1, Error::::IncompleteAssignment); - let schedule_item = &workload[0]; + let Some(schedule_item) = workload.get(0) else { + return Err(Error::::NotAllowed.into()) + }; + let CoreAssignment::Task(task_id) = schedule_item.assignment else { - // return an error. - todo!(); + return Err(Error::::NonTaskAutoRenewal.into()) }; + // Only the sovereign account of the task can enable auto-renewal. + ensure!(who == T::SovereignAccountOf::convert(task_id), Error::::NoPermission); + AutoRenewals::::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)) }) - .map_err(|_| Error::::NotAllowed)?; // TODO: custom error + .map_err(|_| Error::::TooManyAutoRenewals)?; // The caller must pay for the transaction otherwise spamming would be possible by // turning auto-renewal on and off. diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs index f929f0d50dcf..e5766702ee7c 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -1428,3 +1428,8 @@ fn start_sales_sets_correct_core_count() { System::assert_has_event(Event::::CoreCountRequested { core_count: 9 }.into()); }) } + +#[test] +fn enable_auto_renew_works() { + TestExt::new().endow(1, 1000).execute_with(|| {}) +}