-
Notifications
You must be signed in to change notification settings - Fork 335
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
Increases staking delays #1030
Increases staking delays #1030
Conversation
Correct, it is safe. We will need to use
Yes, it is. This would not change the scheduled time for already made requests, only requests made after the delay is changed. |
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.
lgtm
There are rust tests in each of the runtimes that should be failing with these changes. If CI doesn't catch that, it must not be running them... I'll fix them locally when I get a chance and verify whether or not CI is running the tests. |
AFAICT, they are failing no? |
@4meta5 are those changes safe to be included in runtime 1000 (staking) migrations ? |
Yes, it is safe. It will not change anything in the migration. |
I was worried by the fact the round length increased, but rewards per round did not changed in the integration tests. It seems the mock runtime uses a constant inflation per round genesis config https://github.com/PureStake/moonbeam/blob/master/runtime/moonbase/tests/common/mod.rs#L109-L127 We should update this to use the actual numbers. Because inflation rewards are proportional to total supply, we should mint more balance in the runtime mock genesis config. I'll make a jira ticket for this.
|
for x in 2..599 { | ||
for x in 2..1199 { |
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.
I'd prefer to express these in terms of the configuration values like 5 * LeaveDelegatorsDelay
or whatever value this is. But that can be for a followup.
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.
In my deferred staking
PR (#1035 ) I added a roll_to_round_begin
which takes a round number rather than a block number. That's only loosely related to this...
/// Reward payments delay (number of rounds) | ||
/// Blocks per round | ||
pub const DefaultBlocksPerRound: u32 = 6 * HOURS; | ||
/// Rounds before the collator leaving the candidates request can be executed |
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.
/// Rounds before the collator leaving the candidates request can be executed | |
/// Rounds before the candidate's request to leave can be executed |
My rewording makes it clear that this applies to all candidates, not just the selected collators. But after writing it, I wasn't sure that was actually true.
* fix genesis chain_specs so round inflation is set based on default blocks per round configured * fix build * use gorka suggestion to use associated type instead of storage item * fix units * export ParachainStakingConfigTrait from runtime to use in chainspecs * fix * revert export use direct const getter * unused import * try TS inflation genesis config * fix arithmetic * fix annual to round inflation in genesis * fix * fix * pub mod inflation * compiles still TS tests off * fix ts
FYI the alphanet round was already set to 2 hours (600) on chain |
What does it do?
@4meta5 is it safe to change the block per round while the network is already live ? Is it safe also to change the delays while the network is live ?
My understanding is that the default staking round is only used for genesis, so we will need to use
set_blocks_per_round
. Is it safe to use it in the middle of a round ?