Skip to content

support for lazy leader deletion #9025

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.

Sign up for GitHub

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

Open
dharjeezy wants to merge 2 commits into
base: master
Choose a base branch
from
Open

support for lazy leader deletion #9025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15a446c
support for lazy leader deletion
dharjeezy Jun 28, 2025
1e77e0a
pr doc
dharjeezy Jun 28, 2025
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
8 changes: 8 additions & 0 deletions prdoc/pr_9025.prdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: EPMB / signed pallet: Add support for lazy deletion of the leader from the storage
Copy link
Contributor

@sigurpol sigurpol Jun 30, 2025

Choose a reason for hiding this comment

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

Suggested change
title: EPMB / signed pallet: Add support for lazy deletion of the leader from the storage
title: "EPMB / signed pallet: Add support for lazy deletion of the leader from the storage"

doc:
- audience: Runtime Dev
description: |-
Lazy deletion of leader from storage
crates:
- name: pallet-election-provider-multi-block
bump: minor
Copy link
Contributor

@sigurpol sigurpol Jun 30, 2025

Choose a reason for hiding this comment

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

probably major since we have changed a pub(crate) API but CI will tell us

31 changes: 17 additions & 14 deletions substrate/frame/election-provider-multi-block/src/signed/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<T: Config> SolutionDataProvider for Pallet<T> {
// defensive: if there is a result to be reported, then we must have had some
// leader.
if let Some((winner, metadata)) =
Submissions::<T>::take_leader_with_data(Self::current_round()).defensive()
Submissions::<T>::take_leader(Self::current_round(), true).defensive()
{
// first, let's give them their reward.
let reward = metadata.reward.saturating_add(metadata.fee);
Expand Down Expand Up @@ -380,24 +380,27 @@ pub mod pallet {
result
}

/// *Fully* **TAKE** (i.e. get and remove) the leader from storage, with all of its
/// associated data.
/// Takes the leader from the sorted scores for a given round.
///
/// This function always removes the leader's score and their submission metadata from storage.
///
/// This removes all associated data of the leader from storage, discarding the submission
/// data and score, returning the rest.
pub(crate) fn take_leader_with_data(
/// If `with_data` is `true`, it will also remove all associated submission pages.
/// If `false`, the submission pages are left in storage for later.
pub(crate) fn take_leader(
Copy link
Contributor

@sigurpol sigurpol Jun 30, 2025

Choose a reason for hiding this comment

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

it would be probably nice to have tests covering both cases, verifying that in one case submission pages are deleted and in the other cases are left where they are.

Copy link
Contributor

@sigurpol sigurpol Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

[nitpick] please rebase , after #7997 has been merged this fn is now called take_leader_with_data. You might also want to do a round of fmt to make CI happy (I would suggest you to use something like cargo +nightly-2025-01-24 fmt --all locally or the the /cmd fmt bot)

round: u32,
with_data: bool,
) -> Option<(T::AccountId, SubmissionMetadata<T>)> {
Self::mutate_checked(round, || {
SortedScores::<T>::mutate(round, |sorted| sorted.pop()).and_then(
|(submitter, _score)| {
// NOTE: safe to remove unbounded, as at most `Pages` pages are stored.
let r: MultiRemovalResults = SubmissionStorage::<T>::clear_prefix(
(round, &submitter),
u32::MAX,
None,
);
debug_assert!(r.unique <= T::Pages::get());
if with_data {
let r: MultiRemovalResults = SubmissionStorage::<T>::clear_prefix(
(round, &submitter),
u32::MAX,
None,
);
debug_assert!(r.unique <= T::Pages::get());
}

SubmissionMetadataStorage::<T>::take(round, &submitter)
.map(|metadata| (submitter, metadata))
Expand Down Expand Up @@ -1010,7 +1013,7 @@ impl<T: Config> Pallet<T> {
/// Common logic for handling solution rejection - slash the submitter and try next solution
fn handle_solution_rejection(current_round: u32) {
if let Some((loser, metadata)) =
Submissions::<T>::take_leader_with_data(current_round).defensive()
Submissions::<T>::take_leader(current_round, true).defensive()
{
// Slash the deposit
let slash = metadata.deposit;
Expand Down
Loading