election-provider-multi-phase: Add extrinsic to challenge signed submissions #11099
Conversation
|
@Doordashcon please ping us once you are ready for review! |
kianenigma
added
A3-in_progress
|
cc @kianenigma @emostov want to check my logic before adding required test and benchmark |
kianenigma
added
A5-grumble
and removed
A3-in_progress
| @@ -627,6 +627,10 @@ pub mod pallet { | |||
| #[pallet::constant] | |||
| type SignedRewardBase: Get<BalanceOf<Self>>; | |||
|
|
|||
| // Base reward for a challenger | |||
| #[pallet::constant] | |||
| type ChallengeRewardBase: Get<BalanceOf<Self>>; | |||
There was a problem hiding this comment.
The reward of the challenger should be the deposit of the challengee. @emostov wdyt? trying to avoid adding more and more configs.
There was a problem hiding this comment.
The reward needs to be less than the deposit so we don't encourage users to spam the system by submitting a bogus solution, then challenging it and claiming back their own solution and claiming back their deposit.
To achieve this we can have a config that is something like ChallengeDepositDiff, e.g. if the deposit is 100 and ChallengeDepositDiff is 5, then reward base would be 95
There was a problem hiding this comment.
Yeah, good point. The reward can just be a Perbill of the deposit then? I think this is slightly more intuitive than a diff.
| /// The minimum amount a solution challenger must have to execute | ||
| /// `challange_submission` | ||
| #[pallet::constant] | ||
| type MinimumSlashableAmount: Get<BalanceOf<Self>>; |
There was a problem hiding this comment.
Similarly, I think the slashable deposit of the challenger should be dynamic and should be simply the same as the amount of the deposit of the solution that you are going after.
| ElectionCompute::Signed, | ||
| ) { | ||
| Ok(solution) => { | ||
| let _ = T::Currency::slash(&who, T::MinimumSlashableAmount::get()); |
There was a problem hiding this comment.
Like Kian mention, T::MinimumSlashableAmount::get() should be replaced with the deposit for the solution
|
@Doordashcon can you give an update on your agenda around this? Just want to make sure you do plan to continue with this PR. Thanks! |
|
any updates @Doordashcon? |
|
@kianenigma please can you describe a solution that will reduce the number of changes in this PR? |
|
What do you mean by reducing the number of changes? |
|
My iteration of the optimization requires implementing |
|
@Doordashcon there is some good work in this PR but honestly at this point I think it is better to leave and re-try this with a fresh start after you have gained a bit more experience in our codebase. I might try and tackle this myself, or you can retry again later. I will close this, but since it is linked to the PR, it won't be lost and we can re-open later. If you provide your DOT/KSM address in the PR description (e.g. |
|
@kianenigma I appreciate your guidance on this PR and would like to give it another shot later on. I've added my DOT address in the PR description. |
|
cc @kianenigma |
|
\tip small |
|
/tip small |
|
Please fix the following problems before calling the tip bot again:
|
|
/tip small |
|
Please fix the following problems before calling the tip bot again:
|
|
/tip small |
|
Please fix the following problems before calling the tip bot again:
|
|
/tip small |
1 similar comment
|
/tip small |
|
A small tip was successfully submitted for Doordashcon (12zsKEDVcHpKEWb99iFt3xrTCQQXZMu477nJQsTBBrof5k2h on polkadot). https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips |
|
I have no idea what was causing the error before this... :/ |
|
Thanks for the tip! |
louismerlin
removed
the
D9-needsaudit 👮
|
is this feature still needed? I'd like to revisit it |
|
Yes, happy to mentor you for another try. |
This PR attempts to fix paritytech/polkadot-sdk#281
polkadot address: 12zsKEDVcHpKEWb99iFt3xrTCQQXZMu477nJQsTBBrof5k2h