Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coretime auto-renew #4424

Merged
merged 53 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
afb5e75
start
Szegoo May 4, 2024
2716e0f
setup
Szegoo May 8, 2024
e443c9e
outlining the main logic
Szegoo May 9, 2024
2a8a68b
bounded vec
Szegoo May 10, 2024
8df2f81
resolvinig some todos
Szegoo May 11, 2024
82859ad
auto-renew for legacy leases
Szegoo May 11, 2024
e2a4dd5
init test
Szegoo May 11, 2024
9c1c7e3
more tests
Szegoo May 12, 2024
c7ccf5a
enable_auto_renew_renews
Szegoo May 12, 2024
c3eaa21
different approach
Szegoo May 12, 2024
edffbfc
docs
Szegoo May 13, 2024
68f244c
enable_auto_renewal_with_end_hint_works
Szegoo May 13, 2024
893fa9d
auto_renewal_works
Szegoo May 15, 2024
0b50a58
progress
Szegoo May 15, 2024
9dc9b6a
store account instead of task
Szegoo May 15, 2024
9baa77a
fix builds
Szegoo May 15, 2024
ee7e3c4
disable auto-renewal
Szegoo May 17, 2024
e0f1c71
disable auto renew test
Szegoo May 17, 2024
1a46abc
docs
Szegoo May 17, 2024
2f17544
account for changing core indices
Szegoo May 23, 2024
63b6f66
fix
Szegoo May 23, 2024
aaba50d
bench enable_auto_renew
Szegoo May 23, 2024
7d682e6
bench disable & auto_renew
Szegoo May 23, 2024
af8f109
weights
Szegoo May 23, 2024
f22c451
fix
Szegoo May 23, 2024
a6c0676
dummy weights
Szegoo May 23, 2024
a4f3b6c
don't auto renew when not needed
Szegoo May 24, 2024
4f18949
fix auto renewal
Szegoo May 24, 2024
5d29cd3
more checks
Szegoo May 24, 2024
0d9e91d
test fixes/improvements
Szegoo May 27, 2024
dad2b91
merge
Szegoo May 30, 2024
88c9ce1
merge fix
Szegoo May 30, 2024
17e0ee5
cleanup | benchmarks: TODO
Szegoo May 30, 2024
8be2d02
Update substrate/frame/broker/src/dispatchable_impls.rs
Szegoo May 31, 2024
c8fa5f8
Update substrate/frame/broker/src/types.rs
Szegoo May 31, 2024
7d887ab
Update substrate/frame/broker/src/lib.rs
Szegoo May 31, 2024
78a203f
resolve comments
Szegoo May 31, 2024
5f02e32
cleanup
Szegoo May 31, 2024
b0aa972
rename
Szegoo May 31, 2024
de89f30
comment
Szegoo May 31, 2024
7259f41
fix benchmarks
Szegoo May 31, 2024
9a23b2f
cleanup
Szegoo Jun 3, 2024
0c4e651
simplify logic
Szegoo Jun 3, 2024
ef9c4fb
fix benchmarks
Szegoo Jul 1, 2024
4d35d2b
Merge branch 'master' into auto-renew
Szegoo Jul 1, 2024
15d9e1b
merge fixes
Szegoo Jul 1, 2024
eff4ee7
benchmark fix
Szegoo Jul 1, 2024
61f466d
resolve conflicts
Szegoo Jul 30, 2024
d2041f1
prdoc
Szegoo Jul 30, 2024
7155233
improve docs
Szegoo Jul 30, 2024
9a90fbd
improve comments
Szegoo Aug 1, 2024
0cf0dad
Merge branch 'master' into auto-renew
Szegoo Aug 1, 2024
c1fc125
Merge branch 'master' into auto-renew
Szegoo Aug 3, 2024
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
65 changes: 65 additions & 0 deletions substrate/frame/broker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ pub mod pallet {
type ConvertBalance: Convert<BalanceOf<Self>, RelayBalanceOf<Self>>
+ ConvertBack<BalanceOf<Self>, RelayBalanceOf<Self>>;

/// 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<TaskId, Self::AccountId>;

/// Identifier from which the internal Pot is generated.
#[pallet::constant]
type PalletId: Get<PalletId>;
Expand All @@ -114,6 +120,9 @@ pub mod pallet {
/// Maximum number of system cores.
#[pallet::constant]
type MaxReservedCores: Get<u32>;

#[pallet::constant]
type MaxAutoRenewals: Get<u32>;
}

/// The current configuration of this pallet.
Expand Down Expand Up @@ -172,6 +181,11 @@ pub mod pallet {
#[pallet::storage]
pub type CoreCountInbox<T> = StorageValue<_, CoreIndex, OptionQuery>;

/// Keeping track of cores which have auto-renewal enabled.
#[pallet::storage]
Copy link
Member

Choose a reason for hiding this comment

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

You have an invariant that this list is supposed to be sorted. This should the very least be documented here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, keep in mind that this PR is still very much WIP.

Copy link
Contributor

Choose a reason for hiding this comment

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

PR is merged without this being really addressed, should be a 1 liner fn try_state

pub type AutoRenewals<T: Config> =
StorageValue<_, BoundedVec<(CoreIndex, TaskId), T::MaxAutoRenewals>, ValueQuery>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
Expand Down Expand Up @@ -803,6 +817,57 @@ pub mod pallet {
Ok(())
}

/// Extrinsic for enabling auto renewal.
///
/// 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.
#[pallet::call_index(20)]
#[pallet::weight(T::WeightInfo::notify_core_count())]
pub fn enable_auto_renew(origin: OriginFor<T>, core: CoreIndex) -> DispatchResult {
let who = ensure_signed(origin)?;
// TODO: ensure origin is the parachain's sovereign account.

let sale = SaleInfo::<T>::get().ok_or(Error::<T>::NoSales)?;

// If the core hasn't been renewed yet we will renew it now.
let record = if let Some(record) =
AllowedRenewals::<T>::get(AllowedRenewalId { core, when: sale.region_begin })
{
Self::do_renew(who, 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.
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
//
// If not the core is not eligable for renewal.
AllowedRenewals::<T>::get(AllowedRenewalId { core, when: sale.region_end })
.ok_or(Error::<T>::NotAllowed)?
};

let workload =
record.completion.drain_complete().ok_or(Error::<T>::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::<T>::IncompleteAssignment);
let schedule_item = &workload[0];
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
let CoreAssignment::Task(task_id) = schedule_item.assignment else {
// return an error.
todo!();
};

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))
})
.map_err(|_| Error::<T>::NotAllowed)?; // TODO: custom error

// The caller must pay for the transaction otherwise spamming would be possible by
// turning auto-renewal on and off.
Ok(())
}

#[pallet::call_index(99)]
#[pallet::weight(T::WeightInfo::swap_leases())]
pub fn swap_leases(origin: OriginFor<T>, id: TaskId, other: TaskId) -> DispatchResult {
Expand Down
11 changes: 10 additions & 1 deletion substrate/frame/broker/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use frame_system::{EnsureRoot, EnsureSignedBy};
use sp_arithmetic::Perbill;
use sp_core::{ConstU32, ConstU64, Get};
use sp_runtime::{
traits::{BlockNumberProvider, Identity},
traits::{BlockNumberProvider, Convert, Identity},
BuildStorage, Saturating,
};
use sp_std::collections::btree_map::BTreeMap;
Expand Down Expand Up @@ -187,6 +187,14 @@ ord_parameter_types! {
}
type EnsureOneOrRoot = EitherOfDiverse<EnsureRoot<u64>, EnsureSignedBy<One, u64>>;

pub struct TaskToAccountId;
// TODO
impl Convert<TaskId, u64> for TaskToAccountId {
fn convert(_task: TaskId) -> u64 {
0
}
}

impl crate::Config for Test {
type RuntimeEvent = RuntimeEvent;
type Currency = ItemOf<TestFungibles<(), u64, (), ConstU64<0>, ()>, (), u64>;
Expand All @@ -200,6 +208,7 @@ impl crate::Config for Test {
type PalletId = TestBrokerId;
type AdminOrigin = EnsureOneOrRoot;
type PriceAdapter = Linear;
type SovereignAccountOf = TaskToAccountId;
}

pub fn advance_to(b: u64) {
Expand Down
15 changes: 14 additions & 1 deletion substrate/frame/broker/src/tick_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use sp_arithmetic::{
traits::{One, SaturatedConversion, Saturating, Zero},
FixedPointNumber,
};
use sp_runtime::traits::ConvertBack;
use sp_runtime::traits::{Convert, ConvertBack};
use sp_std::{vec, vec::Vec};
use CompletionStatus::Complete;

Expand Down Expand Up @@ -238,6 +238,8 @@ impl<T: Config> Pallet<T> {
});
Leases::<T>::put(&leases);

Self::renew_cores();

let max_possible_sales = status.core_count.saturating_sub(first_core);
let limit_cores_offered = config.limit_cores_offered.unwrap_or(CoreIndex::max_value());
let cores_offered = limit_cores_offered.min(max_possible_sales);
Expand Down Expand Up @@ -329,4 +331,15 @@ impl<T: Config> Pallet<T> {
T::Coretime::assign_core(core, rc_begin, assignment.clone(), None);
Self::deposit_event(Event::<T>::CoreAssigned { core, when: rc_begin, assignment });
}

/// Renews all the cores which have auto-renewal enabled.
pub(crate) fn renew_cores() {
let renewals = AutoRenewals::<T>::get();
renewals.iter().for_each(|(core, task_id)| {
let payer = T::SovereignAccountOf::convert(*task_id);
if let Err(_) = Self::do_renew(payer, *core) {
// TODO: emit event
}
});
}
}
Loading