diff --git a/substrate/frame/broker/src/dispatchable_impls.rs b/substrate/frame/broker/src/dispatchable_impls.rs index 45a0a514c307..8eaeade34280 100644 --- a/substrate/frame/broker/src/dispatchable_impls.rs +++ b/substrate/frame/broker/src/dispatchable_impls.rs @@ -469,6 +469,62 @@ impl Pallet { Ok(()) } + pub(crate) fn do_enable_auto_renew( + who: T::AccountId, + core: CoreIndex, + workload_end_hint: Option, + ) -> DispatchResult { + let sale = SaleInfo::::get().ok_or(Error::::NoSales)?; + + let record = if let Some(workload_end) = workload_end_hint { + AllowedRenewals::::get(AllowedRenewalId { core, when: workload_end }) + .ok_or(Error::::NotAllowed)? + } else { + // If the core hasn't been renewed yet we will renew it now. + if let Some(record) = + AllowedRenewals::::get(AllowedRenewalId { core, when: sale.region_begin }) + { + Self::do_renew(who.clone(), core)?; + record + } else { + // If we couldn't find the renewal record for the current bulk period we should + // be able to find it for the upcoming bulk period. + // + // If not the core is not eligable for renewal. + AllowedRenewals::::get(AllowedRenewalId { core, when: sale.region_end }) + .ok_or(Error::::NotAllowed)? + } + }; + + 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 Some(schedule_item) = workload.get(0) else { + return Err(Error::::NotAllowed.into()) + }; + + let CoreAssignment::Task(task_id) = schedule_item.assignment else { + 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::::TooManyAutoRenewals)?; + + // TODO: event + Ok(()) + } + pub(crate) fn ensure_cores_for_sale( status: &StatusRecord, sale: &SaleInfoRecordOf, diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index 936d539186ae..7b46c67fd320 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -101,8 +101,6 @@ pub mod pallet { /// Type used for getting the associated account of a task. This account is controlled by /// the task itself. - // - // TODO: maybe rename this? type SovereignAccountOf: Convert; /// Identifier from which the internal Pot is generated. @@ -838,53 +836,7 @@ pub mod pallet { workload_end_hint: Option, ) -> DispatchResult { let who = ensure_signed(origin)?; - - let sale = SaleInfo::::get().ok_or(Error::::NoSales)?; - - let record = if let Some(workload_end) = workload_end_hint { - AllowedRenewals::::get(AllowedRenewalId { core, when: workload_end }) - .ok_or(Error::::NotAllowed)? - } else { - // If the core hasn't been renewed yet we will renew it now. - if let Some(record) = - AllowedRenewals::::get(AllowedRenewalId { core, when: sale.region_begin }) - { - Self::do_renew(who.clone(), core)?; - record - } else { - // If we couldn't find the renewal record for the current bulk period we should - // be able to find it for the upcoming bulk period. - // - // If not the core is not eligable for renewal. - AllowedRenewals::::get(AllowedRenewalId { core, when: sale.region_end }) - .ok_or(Error::::NotAllowed)? - } - }; - - 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 Some(schedule_item) = workload.get(0) else { - return Err(Error::::NotAllowed.into()) - }; - - let CoreAssignment::Task(task_id) = schedule_item.assignment else { - 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::::TooManyAutoRenewals)?; + Self::do_enable_auto_renew(who, core, workload_end_hint)?; // 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/mock.rs b/substrate/frame/broker/src/mock.rs index e0698682d7a9..451a2b784434 100644 --- a/substrate/frame/broker/src/mock.rs +++ b/substrate/frame/broker/src/mock.rs @@ -188,10 +188,10 @@ ord_parameter_types! { type EnsureOneOrRoot = EitherOfDiverse, EnsureSignedBy>; pub struct TaskToAccountId; -// TODO +// Dummy implementation which converts `TaskId` to `AccountId`. impl Convert for TaskToAccountId { - fn convert(_task: TaskId) -> u64 { - 0 + fn convert(task: TaskId) -> u64 { + task.into() } } @@ -209,6 +209,7 @@ impl crate::Config for Test { type AdminOrigin = EnsureOneOrRoot; type PriceAdapter = Linear; type SovereignAccountOf = TaskToAccountId; + type MaxAutoRenewals = ConstU32<5>; } pub fn advance_to(b: u64) { diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs index e5766702ee7c..fac6e381db16 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -1409,6 +1409,19 @@ fn renewal_works_leases_ended_before_start_sales() { }); } +#[test] +fn enable_auto_renew_works() { + TestExt::new().endow(1, 1000).execute_with(|| { + assert_ok!(Broker::do_start_sales(100, 1)); + advance_to(2); + let region = Broker::do_purchase(1, u64::max_value()).unwrap(); + assert_ok!(Broker::do_assign(region, Some(1), 1001, Final)); + + assert_ok!(Broker::do_enable_auto_renew(1001, region.core, None)); + assert_eq!(AutoRenewals::::get().to_vec(), vec![(0, 1001)]); + }); +} + #[test] fn start_sales_sets_correct_core_count() { TestExt::new().endow(1, 1000).execute_with(|| { @@ -1428,8 +1441,3 @@ 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(|| {}) -}