Skip to content

[AHM-Kusama] EPMB: invulnerables and offchain storage#8877

Merged
sigurpol merged 7 commits intomasterfrom
kiz-epmb-invulnerable-offchain-storage
Jun 18, 2025
Merged

[AHM-Kusama] EPMB: invulnerables and offchain storage#8877
sigurpol merged 7 commits intomasterfrom
kiz-epmb-invulnerable-offchain-storage

Conversation

@kianenigma
Copy link
Copy Markdown
Contributor

@kianenigma kianenigma commented Jun 17, 2025

This PR bring two new features to the election-provider-multi-block pallets, based on audit reviews and our empirical experience in westend:

  1. While I have not found the actual root cause, the offchain-worker miner in westend collators is sometimes unreliable. In this PR, I have added a configuration that would disable the call cache for this offchain worker. I have run this new code in my personal WAH collator, and it is indeed more reliable. This just gives us more lever to easily disable the cache. Investigation should still continue as to what is the root cause.
  2. the signed submission system now supports a governance set of invulnerables. See the code comments for what this implies.

Marking this as silent for now as these crates are unreleased for faster pace, but can add prdoc if needed.

This will be quite useful to lower the chance of backports after #8764 deadline, but is not mandatory.

It partially closes #8879

@kianenigma kianenigma requested a review from a team as a code owner June 17, 2025 11:55
@kianenigma kianenigma added the R0-no-crate-publish-required The change does not require any crates to be re-published. label Jun 17, 2025
@kianenigma kianenigma assigned sigurpol and unassigned sigurpol Jun 17, 2025
@kianenigma kianenigma requested review from seadanda and sigurpol June 17, 2025 11:56
@sigurpol
Copy link
Copy Markdown
Contributor

sigurpol commented Jun 17, 2025

Investigation should still continue as to what is the root cause.

Have you already created an issue for that so that we can track it and we (to be read as : I...) don't forget ?

@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/15706725409
Failed job name: build-rustdoc

@kianenigma
Copy link
Copy Markdown
Contributor Author

Investigation should still continue as to what is the root cause.

Have you already created an issue for that so that we can track it and we (to be read as : I...) don't forget ?

great point! created #8880

Comment thread substrate/frame/election-provider-multi-block/src/unsigned/miner.rs
@sigurpol
Copy link
Copy Markdown
Contributor

sigurpol commented Jun 17, 2025

A general comment: I’m not sure if we are too late for this and I am fine with reviewing the PR as it is, but I would have appreciated two separate PRs, as they address two completely unrelated topics. E.g. I am done with the review of OCW part and beside a comment on a test to extend, I am pretty sure it can be merged immediately. I haven't started on the invulnerable part but that might require further discussion etc.
It's also true that I can review the first two commits independently and achieve more or less the same 😄

@kianenigma
Copy link
Copy Markdown
Contributor Author

A general comment: I’m not sure if we are too late for this and I am fine with reviewing the PR as it is, but I would have appreciated two separate PRs, as they address two completely unrelated topics. E.g. I am done with the review of OCW part and beside a comment on a test to extend, I am pretty sure it can be merged immediately. I haven't started on the invulnerable part but that might require further discussion etc. It's also true that I can review the first two commits independently and achieve more or less the same 😄

Yeah let's see -- if your review of the second commit turns out complicated, I can split the PRs.

Comment thread substrate/frame/election-provider-multi-block/src/signed/tests.rs
Comment thread substrate/frame/election-provider-multi-block/src/signed/tests.rs
Copy link
Copy Markdown
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.

LGTM, some minor comments mostly about adding tests.
Not a blocker on my side

@sigurpol
Copy link
Copy Markdown
Contributor

A general comment: I’m not sure if we are too late for this and I am fine with reviewing the PR as it is, but I would have appreciated two separate PRs, as they address two completely unrelated topics. E.g. I am done with the review of OCW part and beside a comment on a test to extend, I am pretty sure it can be merged immediately. I haven't started on the invulnerable part but that might require further discussion etc. It's also true that I can review the first two commits independently and achieve more or less the same 😄

Yeah let's see -- if your review of the second commit turns out complicated, I can split the PRs.

It turned out to be unnecessary in the end; it was quite small. I complained for nothing. 😆

Comment thread substrate/frame/election-provider-multi-block/src/signed/mod.rs Outdated
Comment thread substrate/frame/election-provider-multi-block/src/signed/tests.rs
Comment thread substrate/frame/election-provider-multi-block/src/signed/tests.rs
Comment thread substrate/frame/election-provider-multi-block/src/signed/mod.rs Outdated
Comment thread substrate/frame/election-provider-multi-block/src/unsigned/miner.rs
Copy link
Copy Markdown
Contributor Author

@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.

All new changes make sense to me, big thanks @sigurpol!

@seadanda PTAL and merge once you are good with this :)

@sigurpol sigurpol added this pull request to the merge queue Jun 18, 2025
Merged via the queue into master with commit 617cbc7 Jun 18, 2025
246 checks passed
@sigurpol sigurpol deleted the kiz-epmb-invulnerable-offchain-storage branch June 18, 2025 17:45
github-merge-queue Bot pushed a commit that referenced this pull request Aug 21, 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>
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
This PR bring two new features to the `election-provider-multi-block`
pallets, based on audit reviews and our empirical experience in westend:

1. While I have not found the actual root cause, the offchain-worker
miner in westend collators is sometimes unreliable. In this PR, I have
added a configuration that would disable the call cache for this
offchain worker. I have run this new code in my personal WAH collator,
and it is indeed more reliable. This just gives us more lever to easily
disable the cache. Investigation should still continue as to what is the
root cause.
2. the signed submission system now supports a governance set of
`invulnerables`. See the code comments for what this implies.
 
Marking this as silent for now as these crates are unreleased for faster
pace, but can add prdoc if needed.

This will be quite useful to lower the chance of backports after #8764
deadline, but is not mandatory.

It partially closes
https://github.com/orgs/paritytech/projects/189?pane=issue&itemId=114585501&issue=paritytech%7Cpolkadot-sdk%7C8879

---------

Co-authored-by: Paolo La Camera <paolo@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

R0-no-crate-publish-required The change does not require any crates to be re-published.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants