Skip to content
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

Fix genesis chain specs #1046

Merged
merged 16 commits into from
Dec 2, 2021
Merged

Conversation

4meta5
Copy link
Contributor

@4meta5 4meta5 commented Dec 2, 2021

Fixes genesis chain specs so they reflect runtime configured blocks per round.

Also fixes a problem discovered wherein the per-round inflation genesis config for all networks wasn't set correctly. It assumed that inflation per round = annuual inflation / number of rounds per year. That isn't correct, @girazoki corrected this assumption a long time ago in parachain_staking::inflation.

On top of #1030

@4meta5
Copy link
Contributor Author

4meta5 commented Dec 2, 2021

No description provided.

@crystalin crystalin added A8-mergeoncegreen Pull request is reviewed well. B0-silent Changes should not be mentioned in any release notes labels Dec 2, 2021
@girazoki
Copy link
Collaborator

girazoki commented Dec 2, 2021

It looks to me as if constructing the InflationInfo requires reading the Round storage, but it is not set yet since that happens on build_genesis. Either hardcode them or import parachain_staking and mark build it as

min: Perbill::from_parts(Perbill::from_percent(4).deconstruct() / ::DefaultBlocksPerRound::get()),

@girazoki
Copy link
Collaborator

girazoki commented Dec 2, 2021

So essentially, if you dont wanna change the way InflationInfo is consctructed, I suggest something like:

InflationInfo {
		expect: Range {
			min: 100_000 * GLMR,
			ideal: 200_000 * GLMR,
			max: 500_000 * GLMR,
		},
		annual: Range {
			min: Perbill::from_percent(4),
			ideal: Perbill::from_percent(5),
			max: Perbill::from_percent(5),
		},
		round: Range {
			min: Perbill::from_parts(Perbill::from_percent(4).deconstruct() / moonbase_runtime::DefaultBlocksPerRound::get()),
			ideal: Perbill::from_parts(Perbill::from_percent(5).deconstruct() / moonbase_runtime::DefaultBlocksPerRound::get()),
			max:  Perbill::from_parts(Perbill::from_percent(5).deconstruct() / moonbase_runtime::DefaultBlocksPerRound::get()),
		},
	}

@JoshOrndorff
Copy link
Contributor

JoshOrndorff commented Dec 2, 2021

The build is failing. Seems like we could use parachainstaking's Config trait. But I wonder why the compiler says it is ambiguous... Is there some other meaning of DefaultBlocksPerRound?

@4meta5
Copy link
Contributor Author

4meta5 commented Dec 2, 2021

The build is failing. Seems like we could use parachainstaking's Config trait. But I wonder why the compiler says it is ambiguous... Is there some other meaning of DefaultBlocksPerRound?

I think the trait has to be in scope. I'm going to export it from the runtime.

@girazoki
Copy link
Collaborator

girazoki commented Dec 2, 2021

Just use moonbase_runtime::DefaultBlocksPerRound::get()

@librelois librelois mentioned this pull request Dec 2, 2021
15 tasks
@4meta5
Copy link
Contributor Author

4meta5 commented Dec 2, 2021

One more TS test failing, it should've failed in the base PR, but the genesis inflation wasn't updated.

@librelois
Copy link
Collaborator

librelois commented Dec 2, 2021

@4meta5 It still doesn't compile, think about compiling locally before pushing, because the CI takes too long to give you the compilation errors:

error[E0603]: module `inflation` is private
   --> node/service/src/chain_spec/moonbase.rs:165:26
    |
165 |         use parachain_staking::inflation::{perbill_annual_to_perbill_round, BLOCKS_PER_YEAR};
    |                                ^^^^^^^^^ private module
    |
note: the module `inflation` is defined here
   --> /home/elois/dev/purestake/moonbeam/pallets/parachain-staking/src/lib.rs:50:1
    |
50  | mod inflation;
    | ^^^^^^^^^^^^^^

error[E0603]: module `inflation` is private
   --> node/service/src/chain_spec/moonbeam.rs:148:26
    |
148 |         use parachain_staking::inflation::{perbill_annual_to_perbill_round, BLOCKS_PER_YEAR};
    |                                ^^^^^^^^^ private module
    |
note: the module `inflation` is defined here
   --> /home/elois/dev/purestake/moonbeam/pallets/parachain-staking/src/lib.rs:50:1
    |
50  | mod inflation;
    | ^^^^^^^^^^^^^^

error[E0603]: module `inflation` is private
   --> node/service/src/chain_spec/moonriver.rs:144:26
    |
144 |         use parachain_staking::inflation::{perbill_annual_to_perbill_round, BLOCKS_PER_YEAR};
    |                                ^^^^^^^^^ private module
    |
note: the module `inflation` is defined here
   --> /home/elois/dev/purestake/moonbeam/pallets/parachain-staking/src/lib.rs:50:1
    |
50  | mod inflation;
    | ^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0603`.
error: could not compile `moonbeam-service` due to 3 previous errors

@4meta5
Copy link
Contributor Author

4meta5 commented Dec 2, 2021

It still doesn't compile, think about compiling locally before pushing, because the CI takes too long to give you the compilation errors

Yeah, I usually don't compile the node locally because it takes a long time. I'll start compiling locally to speed this up.

@librelois
Copy link
Collaborator

@4meta5 there are still other compilation errors, the CI is not made to serve you as a compiler, but to ensure that there are no omissions, you must compile locally otherwise you risk having too many round trips with the CI

@4meta5
Copy link
Contributor Author

4meta5 commented Dec 2, 2021

Understood, compiling locally now. Usually doesn't take as many tries as today...

you must compile locally otherwise you risk having too many round trips with the CI

I don't really get this risk, but I'm compiling the node locally now and will debug locally before the next commit.

@4meta5 4meta5 merged commit e385285 into crystalin-staking-period Dec 2, 2021
@4meta5 4meta5 deleted the amar-fix-inflation-config branch December 2, 2021 19:11
crystalin added a commit that referenced this pull request Dec 3, 2021
* Increases staking delays

* Fix comments

* Update runtime rust tests to reflect MinCollatorStk changes

* fix rust tests (2x longer rounds)

* fix moonbeam and moonriver tests

* Fix genesis chain specs (#1046)

* 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

Co-authored-by: Stephen Shelton <steve@brewcraft.org>
Co-authored-by: nanocryk <6422796+nanocryk@users.noreply.github.com>
Co-authored-by: librelois <c@elo.tf>
Co-authored-by: Amar Singh <asinghchrony@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A8-mergeoncegreen Pull request is reviewed well. B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants