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 |
marcellorigotti
changed the title
| let attempted_slash: u128 = (*slashing_rate * *bond / | ||
| <mock::Test as pallet::Config>::BlocksPerDay::get() as u128) |
There was a problem hiding this comment.
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.
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.
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.
| /// 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.
| fn slash_balance(account_id: &Self::AccountId, slash_rate: FlipBalance); | |
| fn slash_balance(account_id: &Self::AccountId, slash_amount: FlipBalance); |
marcellorigotti
requested review from
zoheb391 and
GabrielBuragev
and removed request for
a team
|
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.
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. |
dandanlen
force-pushed
the
feature/pro-924
branch
from
9334f1c
to
3d19b3f
Compare
dandanlen
changed the title
|
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: