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

Explicitly specify a data directory when exporting parachain genesis state #1519

Closed
JoshOrndorff opened this issue Nov 16, 2023 · 4 comments · Fixed by #1638
Closed

Explicitly specify a data directory when exporting parachain genesis state #1519

JoshOrndorff opened this issue Nov 16, 2023 · 4 comments · Fixed by #1638

Comments

@JoshOrndorff
Copy link

JoshOrndorff commented Nov 16, 2023

When zombienet exports the parachain genesis state it uses a command like the following:

parachain-template-node export-genesis-state --chain /tmp/zombie-91732cca35cc89d9bc40d256bff11459_-279173-rjyW4YgPKWMA/2000-rococo-local.json > /tmp/zombie-91732cca35cc89d9bc40d256bff11459_-279173-rjyW4YgPKWMA/cfg/genesis-state-2000

This has worked reliably so far, but only because of a bug in cumulus that I am now fixing. (See paritytech/polkadot-sdk#2326 and paritytech/polkadot-sdk#2331 for context.) Previously, Cumulus made some assumptions about the genesis block, and created a new one on the fly whenever you asked to export the genesis state. With this bug fixed, cumulus will now read the genesis block from the database instead of re-generating it 🚀

But that means that you will now need to make sure you are not implicitly using some pre-existing un-purged data directory.

@pepoviola
Copy link
Collaborator

Hi @JoshOrndorff, thanks for the heads up :)

We can fix in zombienet by ensuring we just pass a unique tmp base path, but I think we can also add --tmp flag (is not available in the `export-genesis-* commands) to make more easy to use to others. Wdyt? I can add the code on top of your pr if you are agree.

Thx!

@JoshOrndorff
Copy link
Author

I also considered using --tmp and found it doesn't exist on the export-genesis-state subcommand.

If you want to add --tmp I'm fine with it, but that would be a change in cumulus obviously.

I'm also fine if you just want to pass -d /tmp/some/collision/resistant/place.

Another option is to just consider that the first collator generate's the genesis state. You already created a dedicated data directory for that collator anyway. I guess that's not a great idea actually because maybe you want to support having zero nodes on a parachain just to test the registration process.

bkchr added a commit to paritytech/polkadot-sdk that referenced this issue Dec 13, 2023
…make it respect custom genesis block builders (#2331)

Closes #2326.

This PR both fixes a logic bug and replaces an incorrect name.

## Bug Fix: Respecting custom genesis builder

Prior to this PR the standard logic for creating a genesis block was
repeated inside of cumulus. This PR removes that duplicated logic, and
calls into the proper `BuildGenesisBlock` implementation.

One consequence is that if the genesis block has already been
initialized, it will not be re-created, but rather read from the
database like it is for other node invocations. So you need to watch out
for old unpurged data during the development process. Offchain tools may
need to be updated accordingly. I've already filed
paritytech/zombienet#1519

## Rename: It doesn't export state. It exports head data.

The name export-genesis-state was always wrong, nad it's never too late
to right a wrong. I've changed the name of the struct to
`ExportGenesisHeadCommand`.

There is still the question of what to do with individual nodes' public
CLIs. I have updated the parachain template to a reasonable default that
preserves compatibility with tools that will expect
`export-genesis-state` to still work. And I've chosen not to modify the
public CLIs of any other nodes in the repo. I'll leave it up to their
individual owners/maintains to decide whether that is appropriate.

---------

Co-authored-by: Joshy Orndorff <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
@JoshOrndorff
Copy link
Author

FYI, paritytech/polkadot-sdk#2331 is merged now, so this issue will be important soon.

@pepoviola
Copy link
Collaborator

FYI, paritytech/polkadot-sdk#2331 is merged now, so this issue will be important soon.

Thanks for the heads up @JoshOrndorff! I will fix asap and ping you for testing.
Thx!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants