-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: Update slashing values for mainnet #4148
Conversation
PRO-924 Constant value slashing
We currently slash validators based on a percentage of either the current bond, or their current total stake. For mainnet release we want every slash to be a fixed notional 1FLIP penalty. |
Codecov Report
@@ Coverage Diff @@
## main #4148 +/- ##
=====================================
- Coverage 72% 72% -0%
=====================================
Files 379 380 +1
Lines 61792 61841 +49
Branches 61792 61841 +49
=====================================
+ Hits 44353 44382 +29
- Misses 15140 15153 +13
- Partials 2299 2306 +7
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
let attempted_slash: u128 = (*slashing_rate * *bond / | ||
<mock::Test as pallet::Config>::BlocksPerDay::get() as u128) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be a function. We do this in the slash()
function.
/// (2/3 must agree the node was an offender) on keygen failure. | ||
#[pallet::storage] | ||
pub(super) type KeygenSlashRate<T, I = ()> = StorageValue<_, Percent, ValueQuery>; | ||
pub(super) type KeygenSlashAmount<T, I = ()> = StorageValue<_, FlipBalance, ValueQuery>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will require a storage migration. (LMK and I can show you how to do this)
origin: OriginFor<T>, | ||
percent_of_total_funds: Percent, | ||
amount_to_slash: FlipBalance, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW this is a breaking change. In some cases we would want to change the call_index
here - but since it's a governance extrinsic it's probably ok.
state-chain/traits/src/lib.rs
Outdated
@@ -326,8 +327,8 @@ pub trait Slashing { | |||
/// Slashes a validator for the equivalent of some number of blocks offline. | |||
fn slash(validator_id: &Self::AccountId, blocks_offline: Self::BlockNumber); | |||
|
|||
/// Slahes a percentage of a validator's total balance. | |||
fn slash_balance(account_id: &Self::AccountId, amount: Percent); | |||
/// Slashes a validator for a fixed amount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Slashes a validator for a fixed amount | |
/// Slashes a validator by some fixed amount. |
(nit)
state-chain/traits/src/lib.rs
Outdated
/// Slahes a percentage of a validator's total balance. | ||
fn slash_balance(account_id: &Self::AccountId, amount: Percent); | ||
/// Slashes a validator for a fixed amount | ||
fn slash_balance(account_id: &Self::AccountId, slash_rate: FlipBalance); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fn slash_balance(account_id: &Self::AccountId, slash_rate: FlipBalance); | |
fn slash_balance(account_id: &Self::AccountId, slash_amount: FlipBalance); |
Regarding the breaking change and the storage migration we can check tomorrow in person @dandanlen, thanks! |
#[frame_support::storage_alias] | ||
pub type KeygenSlashAmount<T: Config<I>, I: 'static> = | ||
StorageValue<Pallet<T, I>, FlipBalance, ValueQuery>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need this - you can just use the crate::KeygenSlashAmount that is defined in the new version of the pallet.
0.1% bond for a day of being offline (with negative reputation) 1 FLIP for failed Keygen
I'll resolve the conflicts and merge. |
9334f1c
to
3d19b3f
Compare
This waiting for anything? |
AFAIK it is ready to be merged, @dandanlen can you confirm? |
No, sorry, I must have forgotten. |
Pull Request
Closes: PRO-924
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
Update slashing: