Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit-reveal based solution submission for EPM #13129

Closed
wants to merge 1 commit into from

Conversation

gpestana
Copy link
Contributor

Specs (wip): https://hackmd.io/zE1goNRmSsa8q_LCjHD5Kg

Currently, the EPM signed expects staking miners to submit a full election solution, which is expensive (tx-fees) and potentially stores a lot of redundant data on-chain. This PR splits the solution submission into two phases:

  • Commit: a cheap transaction in which a signed account only claims the score that they wish to submit. Once claimed, they reserve their full hefty deposit on the spot.
  • Reveal: an expensive transaction in which miners, in order, each get a number of blocks to submit their full payload. Upon successful submission, they should get their deposit and their transaction fee back.

Closes paritytech/polkadot-sdk#457

@gpestana gpestana added A3-in_progress Pull request is in progress. No review needed at this stage. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jan 12, 2023
@gpestana gpestana self-assigned this Jan 12, 2023
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jan 12, 2023
@gpestana gpestana marked this pull request as draft January 12, 2023 11:22
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2255781

@the-right-joyce the-right-joyce added B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. and removed B7-runtimenoteworthy labels Feb 13, 2023
Signed,
/// Umbrella phase for all the commit-reveal phases (i.e `Commit`, `Reveal` and
/// `FallbackReveal`).
CommitReveal,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a nested enum be more appropriate?

Then the enums could look like

pub enum Phase {
	Off,
	CommitReveal(CommitReveal),
	Signed,
	Unsigned((bool, Bn)),
	Emergency,
}

pub enum CommitReveal {
     Commit,
     Reveal((AccountId, Bn)),
     FallbackReveal
}


/// Duration of the commit reveal phase.
#[pallet::constant]
type CommitReveal: Get<Self::BlockNumber>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this was how the phase durations were named before. But I feel it would be better if we name these associated types as CommitRevealDuration, CommitPhaseDuration and so on. Wdyt?

///
/// Stores a bounded set of `ElectionScore`, indexed by the score.
#[pallet::storage]
#[pallet::getter(fn commitments)]
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need these getters

@kianenigma kianenigma added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Mar 18, 2023
@gpestana
Copy link
Contributor Author

gpestana commented May 1, 2023

I'm closing this PR since I do not intend to work on it any time soon, but let's keep an eye on paritytech/polkadot-sdk#457 and revisit/re-open this PR when useful.

@gpestana gpestana closed this May 1, 2023
@louismerlin louismerlin removed the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A3-in_progress Pull request is in progress. No review needed at this stage. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

EPM/Staking-miner: commit-reveal based submission
6 participants