Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Large diffs are not rendered by default.

17 changes: 17 additions & 0 deletions prdoc/pr_7080.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "[pallet-broker] add extrinsic to remove an assignment"

doc:
- audience: Runtime User
description: |
A new `remove_assignment` extrinsic is introduced to the broker pallet to allow an assignment to be removed by the root origin.

crates:
- name: pallet-broker
bump: major
- name: coretime-rococo-runtime
bump: patch
- name: coretime-westend-runtime
bump: patch
27 changes: 27 additions & 0 deletions substrate/frame/broker/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,33 @@ mod benches {
Ok(())
}

#[benchmark]
fn remove_assignment() -> Result<(), BenchmarkError> {
Copy link
Contributor

@seadanda seadanda Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to also add this to the weights files for coretime-westend and coretime-rococo. Check the output of clippy (or the clippy CI job)

setup_and_start_sale::<T>()?;

advance_to::<T>(2);

let caller: T::AccountId = whitelisted_caller();
T::Currency::set_balance(
&caller.clone(),
T::Currency::minimum_balance().saturating_add(10_000_000u32.into()),
);

let region = Broker::<T>::do_purchase(caller.clone(), 10_000_000u32.into())
.expect("Offer not high enough for configuration.");

Broker::<T>::do_assign(region, None, 1000, Provisional)
.map_err(|_| BenchmarkError::Weightless)?;

let origin =
T::AdminOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;

#[extrinsic_call]
_(origin as T::RuntimeOrigin, region);

Ok(())
}

// Implements a test for each benchmark. Execute with:
// `cargo test -p pallet-broker --features runtime-benchmarks`.
impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);
Expand Down
8 changes: 8 additions & 0 deletions substrate/frame/broker/src/dispatchable_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,14 @@ impl<T: Config> Pallet<T> {
Ok(())
}

pub(crate) fn do_remove_assignment(region_id: RegionId) -> DispatchResult {
let workplan_key = (region_id.begin, region_id.core);
ensure!(Workplan::<T>::contains_key(&workplan_key), Error::<T>::AssignmentNotFound);
Workplan::<T>::remove(&workplan_key);
Self::deposit_event(Event::<T>::AssignmentRemoved { region_id });
Ok(())
}

pub(crate) fn do_pool(
region_id: RegionId,
maybe_check_owner: Option<T::AccountId>,
Expand Down
17 changes: 17 additions & 0 deletions substrate/frame/broker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@ pub mod pallet {
/// The task to which the Region was assigned.
task: TaskId,
},
/// An assignment has been removed from the workplan.
AssignmentRemoved {
/// The Region which was removed from the workplan.
region_id: RegionId,
},
/// A Region has been added to the Instantaneous Coretime Pool.
Pooled {
/// The Region which was added to the Instantaneous Coretime Pool.
Expand Down Expand Up @@ -558,6 +563,8 @@ pub mod pallet {
SovereignAccountNotFound,
/// Attempted to disable auto-renewal for a core that didn't have it enabled.
AutoRenewalNotEnabled,
/// Attempted to force remove an assignment that doesn't exist.
AssignmentNotFound,
/// Needed to prevent spam attacks.The amount of credits the user attempted to purchase is
/// below `T::MinimumCreditPurchase`.
CreditPurchaseTooSmall,
Expand Down Expand Up @@ -996,6 +1003,16 @@ pub mod pallet {
Self::do_remove_lease(task)
}

/// Remove an assignment from the Workplan.
///
/// - `origin`: Must be Root or pass `AdminOrigin`.
/// - `region_id`: The Region to be removed from the workplan.
#[pallet::call_index(26)]
pub fn remove_assignment(origin: OriginFor<T>, region_id: RegionId) -> DispatchResult {
T::AdminOrigin::ensure_origin_or_root(origin)?;
Self::do_remove_assignment(region_id)
}

#[pallet::call_index(99)]
#[pallet::weight(T::WeightInfo::swap_leases())]
pub fn swap_leases(origin: OriginFor<T>, id: TaskId, other: TaskId) -> DispatchResult {
Expand Down
24 changes: 23 additions & 1 deletion substrate/frame/broker/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ use frame_support::{
};
use frame_system::RawOrigin::Root;
use pretty_assertions::assert_eq;
use sp_runtime::{traits::Get, Perbill, TokenError};
use sp_runtime::{
traits::{BadOrigin, Get},
Perbill, TokenError,
};
use CoreAssignment::*;
use CoretimeTraceItem::*;
use Finality::*;
Expand Down Expand Up @@ -1848,6 +1851,25 @@ fn disable_auto_renew_works() {
});
}

#[test]
fn remove_assignment_works() {
TestExt::new().endow(1, 1000).execute_with(|| {
assert_ok!(Broker::do_start_sales(100, 1));
advance_to(2);
let region_id = Broker::do_purchase(1, u64::max_value()).unwrap();
assert_ok!(Broker::do_assign(region_id, Some(1), 1001, Final));
let workplan_key = (region_id.begin, region_id.core);
assert_ne!(Workplan::<Test>::get(workplan_key), None);
assert_noop!(Broker::remove_assignment(RuntimeOrigin::signed(2), region_id), BadOrigin);
assert_ok!(Broker::remove_assignment(RuntimeOrigin::root(), region_id));
assert_eq!(Workplan::<Test>::get(workplan_key), None);
assert_noop!(
Broker::remove_assignment(RuntimeOrigin::root(), region_id),
Error::<Test>::AssignmentNotFound
);
});
}

#[test]
fn start_sales_sets_correct_core_count() {
TestExt::new().endow(1, 1000).execute_with(|| {
Expand Down
Loading
Loading