Skip to content

Remove "SolutionImprovementThreshold" logic #10340

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

Jump to bottom
Merged
merged 2 commits into from
Nov 21, 2025
Merged

Remove "SolutionImprovementThreshold" logic #10340

merged 2 commits into from
Nov 21, 2025

Conversation

@andreitrand
Copy link
Contributor

@andreitrand andreitrand commented Nov 17, 2025 •
edited
Loading

Fixes 9119

The threshold mechanism used by the "election-provider-multi-block" verifier pallet is no longer relevant because there is no more queued solution to compare against during the initial verification and subseuquently, solutions are processed in the order of decreasing score, with the first one being chosen in all cases.

@andreitrand andreitrand requested a review from a team as a code owner November 17, 2025 15:26
@andreitrand andreitrand marked this pull request as draft November 17, 2025 15:26
@andreitrand andreitrand self-assigned this Nov 17, 2025
@andreitrand andreitrand added T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network. T14-system_parachains This PR/Issue is related to system parachains. and removed T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Nov 17, 2025
@andreitrand andreitrand force-pushed the andreitrand-remove-solution-improvement-threshold-logic branch from b6af2a9 to 6f2a7dd Compare November 17, 2025 15:43
@andreitrand
Copy link
Contributor Author

andreitrand commented Nov 17, 2025

/cmd prdoc

@andreitrand andreitrand force-pushed the andreitrand-remove-solution-improvement-threshold-logic branch from 2f1b458 to 43e08d4 Compare November 18, 2025 12:04
@andreitrand
Copy link
Contributor Author

andreitrand commented Nov 18, 2025

/cmd prdoc

@andreitrand andreitrand force-pushed the andreitrand-remove-solution-improvement-threshold-logic branch from 474290a to 8005345 Compare November 18, 2025 13:29
@andreitrand
Copy link
Contributor Author

andreitrand commented Nov 18, 2025

/cmd prdoc

@andreitrand andreitrand marked this pull request as ready for review November 18, 2025 13:31
@andreitrand andreitrand force-pushed the andreitrand-remove-solution-improvement-threshold-logic branch from 08f97ea to 6570639 Compare November 18, 2025 14:03
@paritytech-workflow-stopper
Copy link

paritytech-workflow-stopper bot commented Nov 18, 2025

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/19468891835
Failed job name: cargo-clippy

kianenigma
kianenigma reviewed Nov 19, 2025
View reviewed changes
sigurpol
sigurpol reviewed Nov 19, 2025
View reviewed changes
Copy link
Contributor

@sigurpol sigurpol left a comment

Choose a reason for hiding this comment

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

Good stuff! Some very minor commens (mostly cosmetic) and I agree with @kianenigma in reverting all changes for Cargo.lock that seems not related to this PR

@sigurpol
Copy link
Contributor

sigurpol commented Nov 19, 2025

also:

error: variables can be used directly in the `format!` string

this is a classic coming from CI... Worth trying to run cargo +nightly-2025-06-27 clippy --fix --all-targets --allow-dirty --allow-staged or similar

kianenigma
kianenigma reviewed Nov 19, 2025
View reviewed changes
kianenigma
kianenigma reviewed Nov 19, 2025
View reviewed changes
kianenigma
kianenigma reviewed Nov 19, 2025
View reviewed changes
kianenigma
kianenigma reviewed Nov 19, 2025
View reviewed changes
kianenigma
kianenigma reviewed Nov 19, 2025
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Some tests need to be improved, otherwise looks good!

@andreitrand
Remove "SolutionImprovementThreshold" logic
5dba8eb 
Fixes #9119

The threshold mechanism used by the "election-provider-multi-block" verifier pallet
is no longer relevant because there is no more queued solution to compare against
during the initial verification and subseuquently, solutions are processed in the
order of decreasing score, with the first one being chosen in all cases.

---------

Signed-off-by: Andrei Trandafir <[email protected]>
@andreitrand andreitrand force-pushed the andreitrand-remove-solution-improvement-threshold-logic branch from 6570639 to b51d2e7 Compare November 20, 2025 20:29
sigurpol
sigurpol approved these changes Nov 21, 2025
View reviewed changes
Copy link
Contributor

@sigurpol sigurpol left a comment

Choose a reason for hiding this comment

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

Great stuff!

@sigurpol sigurpol added the A4-backport-stable2512 Pull request must be backported to the stable2512 release branch label Nov 21, 2025
@sigurpol
Copy link
Contributor

sigurpol commented Nov 21, 2025 •
edited
Loading

I've added the backport to 2512 label - which is maybe debatable (cc @kianenigma ) but it would be nice to see this change on the field.
@andreitrand , if we agree to backport on 2512 (so that eventually goes live on westend), I think it makes sense for you to

  • take care of the backported PR (automatically created once this one is merged): probably CI will be happy by itself and you can just self-approve and merge (or solve conflicts if any before, shouldn't be the case with 2512)
  • ask Egor to bump and publish relevant pallet

Once in the more far distant future we will have to update the runtime in the fellowship for polkadot / kusama to include and SDK containing this change, you will have to:

  • make a PR in the fellowship repo where:
  • you bump pallet version
  • you remove from polkadot and kusama asset-hub runtime type SolutionImprovementThreshold = SolutionImprovementThreshold;

What do you think?

@andreitrand
Copy link
Contributor Author

andreitrand commented Nov 21, 2025

I've added the backport to 2512 label - which is maybe debatable (cc @kianenigma ) but it would be nice to see this change on the field. @andreitrand , if we agree to backport on 2512 (so that eventually goes live on westend), I think it makes sense for you to

* take care of the backported PR (automatically created once this one is merged): probably CI will be happy by itself and you can just self-approve and merge (or solve conflicts if any before, shouldn't be the case with 2512)

* ask Egor to bump and publish relevant pallet

* make a PR in the fellowship repo where:
  
  * you bump pallet version
  * you remove from polkadot and kusama asset-hub runtime `type SolutionImprovementThreshold = SolutionImprovementThreshold;`
  
  What do you think?

I fully agree, since this will be my first backporting and it should help me familiarize myself with the process.

Ank4n
Ank4n reviewed Nov 21, 2025
View reviewed changes
substrate/primitives/npos-elections/src/lib.rs
}
}

/// Compares two sets of election scores based on desirability, returning true if `self` is
Copy link
Contributor

@Ank4n Ank4n Nov 21, 2025

Choose a reason for hiding this comment

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

what does strict_better mean here? Does all three elements of self election score need to be better than the other?

Ank4n
Ank4n approved these changes Nov 21, 2025
View reviewed changes
Copy link
Contributor

@Ank4n Ank4n left a comment

Choose a reason for hiding this comment

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

LGTM

@andreitrand andreitrand added this pull request to the merge queue Nov 21, 2025
Merged via the queue into master with commit 8c073c5 Nov 21, 2025
339 of 345 checks passed
@andreitrand andreitrand deleted the andreitrand-remove-solution-improvement-threshold-logic branch November 21, 2025 13:33
paritytech-release-backport-bot bot pushed a commit that referenced this pull request Nov 21, 2025
@andreitrand @github-actions
Remove "SolutionImprovementThreshold" logic (#10340)
a3233b5 
Fixes [9119](#9119)

The threshold mechanism used by the "election-provider-multi-block"
verifier pallet is no longer relevant because there is no more queued
solution to compare against during the initial verification and
subseuquently, solutions are processed in the order of decreasing score,
with the first one being chosen in all cases.

---------

Signed-off-by: Andrei Trandafir <[email protected]>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 8c073c5)
@paritytech-release-backport-bot paritytech-release-backport-bot bot mentioned this pull request Nov 21, 2025
Closed
@paritytech-release-backport-bot
Copy link

paritytech-release-backport-bot bot commented Nov 21, 2025

Successfully created backport PR for stable2512:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sigurpol sigurpol sigurpol approved these changes

@Ank4n Ank4n Ank4n approved these changes

@kianenigma kianenigma Awaiting requested review from kianenigma

Assignees

@andreitrand andreitrand

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove SolutionImprovementThreshold logic from the verifier pallet

5 participants

@andreitrand @sigurpol @kianenigma @Ank4n