Skip to content

EPMB/Signed: Make invulnerables non-eject-able#9511

Merged
kianenigma merged 11 commits intomasterfrom
kiz-invulnerable-signed-no-eject
Aug 21, 2025
Merged

EPMB/Signed: Make invulnerables non-eject-able#9511
kianenigma merged 11 commits intomasterfrom
kiz-invulnerable-signed-no-eject

Conversation

@kianenigma
Copy link
Copy Markdown
Contributor

Follow-up to #8877 and audits: Make it such that invulnerable accounts cannot be ejected from the election signed queue altogether.

@kianenigma kianenigma requested a review from a team as a code owner August 19, 2025 08:50
@kianenigma kianenigma added A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch T2-pallets This PR/Issue is related to a particular pallet. labels Aug 19, 2025
@kianenigma
Copy link
Copy Markdown
Contributor Author

/cmd prdoc --bump patch

Ok(Some((discarded, _score))) => {
if Pallet::<T>::is_invulnerable(&discarded) {
// invulnerable accounts are never ejected.
return Err(Error::<T>::QueueFull.into());
Copy link
Copy Markdown
Contributor

@seadanda seadanda Aug 19, 2025

Choose a reason for hiding this comment

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

Since we're force-inserting and keeping right, and if the one to be ejected is an invulnerable we return an Error, the entire queue becomes locked if the lowest score belongs to an invulnerable account even if I'm submitting a higher score.

Could we instead eject the lowest non-invulnerable?

To demonstrate, if we imagine the array is a tuple of score and I/N for invulnerable/not:

[(20, I), (50, N), (70, N)]

And assume the limit is 3, then if I submit a result with score 80, we hit this logic and it can't eject the invulnerable at the bottom, so the queue is declared full, when really we should end up with a queue
[(20, I), (70, N), (80, N)]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is a good idea, I'll explore doing it as such.

The only scenario that it would be useful for will be:

If the queue is full minus an invulnerable who has happen to submit a suboptimal solution. The cost of filling the queue for N-1 is still quite large.

I think a much better approach would be to have two separate queues here altogether, but that might take more time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@paritytech-workflow-stopper
Copy link
Copy Markdown

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/17069536548
Failed job name: run-frame-omni-bencher

@kianenigma kianenigma requested a review from seadanda August 19, 2025 14:26
Copy link
Copy Markdown
Contributor

@seadanda seadanda left a comment

Choose a reason for hiding this comment

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

Nice

Comment thread substrate/frame/election-provider-multi-block/src/signed/tests.rs Outdated
@kianenigma kianenigma enabled auto-merge August 20, 2025 09:31
@kianenigma kianenigma added this pull request to the merge queue Aug 21, 2025
Merged via the queue into master with commit 56d3c42 Aug 21, 2025
249 of 252 checks passed
@kianenigma kianenigma deleted the kiz-invulnerable-signed-no-eject branch August 21, 2025 09:02
@paritytech-release-backport-bot
Copy link
Copy Markdown

Created backport PR for unstable2507:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-9511-to-unstable2507
git worktree add --checkout .worktree/backport-9511-to-unstable2507 backport-9511-to-unstable2507
cd .worktree/backport-9511-to-unstable2507
git reset --hard HEAD^
git cherry-pick -x 56d3c42cf4b8b650ae416db0482ad56eb64938c9
git push --force-with-lease

pepoviola pushed a commit that referenced this pull request Aug 26, 2025
Follow-up to #8877 and
audits: Make it such that invulnerable accounts cannot be ejected from
the election signed queue altogether.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
Follow-up to #8877 and
audits: Make it such that invulnerable accounts cannot be ejected from
the election signed queue altogether.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch T2-pallets This PR/Issue is related to a particular pallet.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants