Skip to content
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.

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

election-provider-multi-phase: Add extrinsic to challenge signed submissions #281

Open
emostov opened this issue Mar 16, 2022 · 5 comments
Assignees
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@emostov
Copy link

emostov commented Mar 16, 2022

The election provider multi phase pallet allows "staking miners" to submit elections solutions during the signed phase. We only store a limited number of solutions. There are a few areas here where a malicious actor can grieve the system:

  • Due to the current storage layout, for any given solution score, only one solution can exist, which is problematic if miners converge on the same score since an attacker can block virtually all honest miners simply pre-crafting that score and submitting it with a bogus solution. This means the cost of ensuring no signed submissions works is just the deposit of a single solution as opposed to the cost of a queues worth of deposits.
  • Currently we check the best solution, and if its ok we don't check the remaining solutions, meaning an attacker could spam the queue with no repercussion.

To address this we should add a challenge extrinsic. This extrinsic would take a solution id, check if the challenger has some min slash-able amount and then perform the costly operation of checking if the solution matches its claimed score.

  • If the claimed score is correct, the challenger would be slashed. One optimization here would be to mark the solution as checked, so when we call elect we don't need to check the solution again.
  • If the claimed score is not correct, the malicious submitter will lose the deposit, the solution will be ejected and the challenger will receive some reward. Importantly, the reward for the challenger must be less than the solutions deposit in order to ensure there is no incentive to submit a bogus solution and then challenge yourself.

Also note: https://github.com/paritytech/srlabs_findings/issues/97#issuecomment-1069218413

@Doordashcon
Copy link
Contributor

Hi @emostov, i've been going through the code and would like to take on this issue :)

@emostov
Copy link
Author

emostov commented Mar 18, 2022

Sounds good - let me know if you have any questions

@Doordashcon
Copy link
Contributor

Doordashcon commented Mar 23, 2022

"solution id" is synonymous to submission index and this index can be used to retrieve a signed submission?

@emostov
Copy link
Author

emostov commented Mar 23, 2022

Yes, it should be the submission index that is used as the key to this map:
pub type SignedSubmissionsMap<T: Config> = StorageMap<_, Twox64Concat, u32, SignedSubmissionOf<T>, OptionQuery>;.

This is guaranteed to be unique during the signed phase.

@kianenigma
Copy link
Contributor

Due to the current storage layout, for any given solution score, only one solution can exist, which is problematic if miners converge on the same score since an attacker can block virtually all honest miners simply pre-crafting that score and submitting it with a bogus solution. This means the cost of ensuring no signed submissions works is just the deposit of a single solution as opposed to the cost of a queues worth of deposits.

FYI, this is no longer the case, as of paritytech/substrate#12237

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. T1-FRAME This PR/Issue is related to core FRAME, the framework. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed Z6-mentor labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: ⌛️ Sometime-soon
Status: Backlog
Development

Successfully merging a pull request may close this issue.

5 participants