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 export genesis state #1233

Merged
merged 2 commits into from
May 6, 2024
Merged

fix export genesis state #1233

merged 2 commits into from
May 6, 2024

Conversation

ermalkaleci
Copy link
Contributor

@ermalkaleci ermalkaleci commented May 2, 2024

Use of state_version 1 is incorrect since chain was started with state_version 0.
Instead of generation genesis header manually we use substrate cmd

@ermalkaleci ermalkaleci requested a review from Dinonard May 2, 2024 09:10
@ermalkaleci ermalkaleci added the client This PR/Issue is related to the topic “client”. label May 2, 2024
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

LGTM

Really weird we had this custom logic instead of using what's available 🤷‍♂️

Copy link
Member

@ashutoshvarma ashutoshvarma left a comment

Choose a reason for hiding this comment

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

Looks good!

This was introduced in v1.1.0 uplift by me. The new genesis cmd required client (executor) as new argument for pulling state_version, and we had issue with zombienet testing.

  • The correct logic was added in this commit only to be overwritten by upcoming commits.
  • I think I tried to streamlined the cmd by pulling the current state_version instead of creating partials for each spec in order to isolate the issue to genesis mismatch, it was an oversight.

Thanks for fixing it!

@Dinonard
Copy link
Member

Dinonard commented May 2, 2024

@ashutoshvarma oh right, now I remember!
I thought it was very old code 😅

Signed-off-by: Ermal Kaleci <ermalkaleci@gmail.com>
Signed-off-by: Ermal Kaleci <ermalkaleci@gmail.com>
@ermalkaleci ermalkaleci force-pushed the fix/export-genesis-state branch from 6597315 to c739984 Compare May 3, 2024 09:50
Copy link

github-actions bot commented May 3, 2024

Code Coverage

Package Line Rate Branch Rate Health
pallets/xc-asset-config/src 64% 0%
precompiles/xcm/src 73% 0%
pallets/astar-xcm-benchmarks/src/fungible 100% 0%
pallets/inflation/src 83% 0%
pallets/dapp-staking-v3/src/benchmarking 99% 0%
chain-extensions/xvm/src 0% 0%
pallets/dapp-staking-v3/rpc/runtime-api/src 0% 0%
pallets/astar-xcm-benchmarks/src/generic 100% 0%
pallets/oracle-benchmarks/src 0% 0%
primitives/src 61% 0%
pallets/astar-xcm-benchmarks/src 88% 0%
precompiles/sr25519/src 64% 0%
pallets/collator-selection/src 92% 0%
chain-extensions/types/unified-accounts/src 0% 0%
precompiles/dapp-staking-v3/src/test 0% 0%
precompiles/unified-accounts/src 100% 0%
chain-extensions/unified-accounts/src 0% 0%
precompiles/assets-erc20/src 81% 0%
precompiles/dapp-staking-v3/src 90% 0%
primitives/src/xcm 64% 0%
chain-extensions/types/assets/src 0% 0%
pallets/dapp-staking-migration/src 0% 0%
pallets/xvm/src 54% 0%
precompiles/dispatch-lockdrop/src 86% 0%
precompiles/substrate-ecdsa/src 74% 0%
pallets/price-aggregator/src 72% 0%
pallets/dynamic-evm-base-fee/src 92% 0%
pallets/dapp-staking-v3/src 92% 0%
pallets/dapp-staking-v3/src/test 0% 0%
pallets/static-price-provider/src 52% 0%
chain-extensions/pallet-assets/src 56% 0%
pallets/unified-accounts/src 86% 0%
chain-extensions/types/xvm/src 0% 0%
pallets/ethereum-checked/src 79% 0%
precompiles/xvm/src 75% 0%
Summary 78% (3599 / 4632) 0% (0 / 0)

Minimum allowed line rate is 50%

@ermalkaleci ermalkaleci merged commit e2d0097 into master May 6, 2024
8 checks passed
@ermalkaleci ermalkaleci deleted the fix/export-genesis-state branch May 6, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client This PR/Issue is related to the topic “client”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants