Few fixes to KAH Staking Configs + Preliminary PAH Configs#907
Few fixes to KAH Staking Configs + Preliminary PAH Configs#907kianenigma merged 18 commits intooty-dev-asset-hub-migration-2506from
Conversation
| // | ||
| // After which this limit can be increased again. | ||
| pallet_staking_async::ValidatorCount::<Runtime>::put(2500); | ||
| pallet_staking_async::MaxValidatorsCount::<Runtime>::put(2500); |
| /// warning event in case an era is longer than this amount. | ||
| /// | ||
| /// Under normal conditions, this upper bound is never needed, and eras would be 24h each exactly. Yet, since this is the first deployment of pallet-staking-async, there might be misconfiguration, so we allow up to 12h more in each era. | ||
| pub const MaxEraDuration: u64 = 36 * (1000 * 60 * 60); |
There was a problem hiding this comment.
sanity check plz
There was a problem hiding this comment.
what is the rationale of having 3h extra on Kusama and 12h here? Just a reasonable buffer and no more than that?
There was a problem hiding this comment.
It is 1/2 of the corresponding era.
Actually I had a hard time deciding this, because MaxEraDuration is doing two things:
- Cap the inflation by this amount
- Emit an event
Ideally, I wanted to have two different values:
expected_era_duration+small_epsilonshould emit the event that something is wrongexpected_era_duration+large_buffershould be the cap for inflation.
So for example if we have a bug and an era is 3 days long, we still inflate in accordance to 3 days, but emit events.
There was a problem hiding this comment.
Sanity checked this. Here are two consecutive eras in Polkadot RC:
{
index: 1,927
start: 1,757,518,578,001
}
{
index: 1,928
start: 1,757,604,978,001
}
The diff is 86400000. Which is exactly 24 * 1000 * 60 * 60.
So our value here is fine.
| // https://kusama.subscan.io/account/FyrGiYDGVxg5UUpN3qR5nxKGMxCe5Ddfkb3BXjxybG6j8gX | ||
| let acc = hex_literal::hex!( | ||
| "bea06e6ad606b2a80822a72aaae84a9a80bec27f1beef1880ad4970b72227601" | ||
| "96a6df31a112d610277c818fd9a8443d265fb5ab83cba47c5e89cff16cf9e011" |
There was a problem hiding this comment.
so you have double-checked with our devops friends that this is the one we are going to use, right?
There was a problem hiding this comment.
yup, I checked again the ones in devops, and they have more funds + the old miner keys likely have more compromised seed accounts, as they have been in use for many years, so I reverted back to using the parity-controlled accounts
| /// warning event in case an era is longer than this amount. | ||
| /// | ||
| /// Under normal conditions, this upper bound is never needed, and eras would be 24h each exactly. Yet, since this is the first deployment of pallet-staking-async, there might be misconfiguration, so we allow up to 12h more in each era. | ||
| pub const MaxEraDuration: u64 = 36 * (1000 * 60 * 60); |
There was a problem hiding this comment.
what is the rationale of having 3h extra on Kusama and 12h here? Just a reasonable buffer and no more than that?
| // leave plenty of time for solution mining and submission. | ||
| parameter_types! { | ||
| // alias for the ones backed by parameters-pallet. | ||
| pub MaxSignedSubmissions: u32 = dynamic_params::staking_election::MaxSignedSubmissions::get(); |
There was a problem hiding this comment.
Hm do you think these gets will be re-evaluated every time or it will just use the default value?
I dont think we tested this before?
But you can just do a use dynamic_params::staking_election::MaxSignedSubmissions; to have it handy.
There was a problem hiding this comment.
ah yeah, what I am doing here is kinda stupid code-wise, I am optimizing for being able to read all parameters in one place. I will change it, use makes sense.
system-parachains/asset-hubs/asset-hub-polkadot/src/staking/mod.rs
Outdated
Show resolved
Hide resolved
|
|
||
| impl frame_support::traits::OnRuntimeUpgrade for InitiateStakingAsync { | ||
| fn on_runtime_upgrade() -> Weight { | ||
| if Self::needs_init() { |
There was a problem hiding this comment.
| if Self::needs_init() { | |
| if !Self::needs_init() { | |
| return; |
Nit for less indentation.
system-parachains/asset-hubs/asset-hub-polkadot/src/staking/mod.rs
Outdated
Show resolved
Hide resolved
| let minimum_score = sp_npos_elections::ElectionScore { | ||
| minimal_stake: 8474057820699941, | ||
| sum_stake: 3276970719352749444, | ||
| sum_stake_squared: 244059208045236715654727835467163294, |
There was a problem hiding this comment.
OT: These are basically our POW Parameters? But we have to fix lower bounds still it seems.
There was a problem hiding this comment.
exactly, it is the worst validator set that we accept.
ggwpez
left a comment
ggwpez
left a comment
There was a problem hiding this comment.
Only one concern about the parameter_types wrapping the dynamic_params.
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
…s/runtimes into kiz-polkadot-ahm-configs
…t-fellows/runtimes into kiz-polkadot-ahm-configs
|
Will be merged once CI runs one more time to check nothing major is broken |
kianenigma
merged commit 88f4aaf
into
oty-dev-asset-hub-migration-2506
3 participants
Would be happy to approve the base branch after this.