Skip to content

Conversation

@shamil-gadelshin
Copy link
Contributor

Changes:

  • SlashDeferDuration changed to the BONDING_DURATION - 1
  • Staking history_depth was increased to 336

Changes:
- SlashDeferDuration changed to the BONDING_DURATION - 1
- Staking history_depth was increased to 336
.collect(),
invulnerables: initial_authorities.iter().map(|x| x.0.clone()).collect(),
slash_reward_fraction: Perbill::from_percent(10),
history_depth: 336,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where did 84 come from? Was it just some default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

pub const BondingDuration: pallet_staking::EraIndex = 24;
pub const SlashDeferDuration: pallet_staking::EraIndex = 6; // 1/4 the bonding duration.
pub const BondingDuration: pallet_staking::EraIndex = BONDING_DURATION;
pub const SlashDeferDuration: pallet_staking::EraIndex = BONDING_DURATION - 1; // 'slightly less' than the bonding duration.
Copy link
Member

Choose a reason for hiding this comment

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

If we ever (mistakenly) set BONDING_DURATION to 0 we might get problems (compiler would catch attempting to set a negative number).
, or even setting it to 1 then SlashDeferDuration = 0.

So maybe setting pub const Slash_Defer_Duration = 23 then BondingDuration = Slash_Defer_Duration + 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The compiler prevents it with the arithmetic operation will overflow message.

@mnaamani
Copy link
Member

So I only had a small comment, but otherwise the numbers look good.

@mnaamani mnaamani merged commit 0f73dc2 into Joystream:iznik Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants