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

Cumulus's ExportGenesisState command does not respect custom GenesisBuilder implementations #2326

Closed
JoshOrndorff opened this issue Nov 14, 2023 · 5 comments · Fixed by #2331

Comments

@JoshOrndorff
Copy link
Contributor

Substrate is flexible enough that chain developers can provide their own methods for creating a genesis block. For details see the GenesisBlockBuilder trait docs and my Stack Exchange Question.

Although Substrate has this feature, it is not widely used in the Ecosystem. As far as I know, the Tuxedo template is the first to use a Custom Genesis Block Builder. It is working great as a standalone chain. The problem comes when I try to create a parachain.

Specifically, cumulus's ExportGenesisState command assumes that the genesis block will not contain any extrinsics. Indeed, the generate_genesis_block helper function assumes no genesis extrinsics.

@JoshOrndorff
Copy link
Contributor Author

Also, why is it called "export genesis state"? Shouldn't it be "export genesis header" or head data or something?

@bkchr
Copy link
Member

bkchr commented Nov 14, 2023

I think it was called genesis state at some point at time in the register function.

@JoshOrndorff
Copy link
Contributor Author

JoshOrndorff commented Nov 15, 2023

I'm really stuck on this issue about the custom genesis builder. My problem is when the parachain collator tries to author block number 1 (so the first "normal" block after genesis). It cannot find the parent (the genesis) in the database. And the reason is that it is looking for the wrong genesis block hash.

First let's export the genesis head data manually including my fix from #2331. My debug line shows the hash of the genesis block.
Repeating this command a few times, even after purging, always gives the same deterministic values.

$ ./target/release/parachain-template-node  export-genesis-state -d /tmp/6
🤵🤵🤵🤵🤵🤵🤵 In Command. Genesis Hash is 0x968497d030f04a8f62923761448874d477c71308e49f8e1cff041b40d51430c3

0x00000000000000000000000000000000000000000000000000000000000000000094c2f53461d61eda16379d206c41794cccf45061db810eb5f0a445ac830807d59bcdb7a1ee3d5f1fa1ca1ddd2a534c2097034b7b16264b9ace8e7f61c0fe16bc00

Now let's use zombienet to start a network. Here are some excerpt's from the collator's logs.

# First some output from our custom genesis block builder showing the same genesis hash as before. Followed immediately by a standard parachain log confirming the expected genesis block in the db.
About to finish build_genesis_block. The hash at this point is 0x968497d030f04a8f62923761448874d477c71308e49f8e1cff041b40d51430c3
2023-11-15 16:11:29 [Parachain] 🔨 Initializing Genesis block/state (state: 0x94c2…07d5, header-hash: 0x9684…30c3)    

# As we wait for our turn to start collating, we get further confirmation of the expected genesis block hash
2023-11-15 16:12:01 [Parachain] 💤 Idle (1 peers), best: #0 (0x9684…30c3), finalized #0 (0x9684…30c3), ⬇ 85 B/s ⬆ 53 B/s 

# Finally, we try to author a block, but we fail to find the parent.
🤵🤵🤵🤵🤵🤵🤵 In Aura about to call check block status
🤵🤵🤵🤵🤵🤵🤵 Last header is Header { parent_hash: 0x0000000000000000000000000000000000000000000000000000000000000000, number: 0, state_root: 0x8337ac1ad1ee3adb85987ab4460232b5fe4fe452e99f715ea9d8bf4f5b53e003, extrinsics_root: 0x03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314, digest: Digest { logs: [] } }
🤵🤵🤵🤵🤵🤵🤵 and its hash is 0x633ae7ab12476b9af06994356a7b2fbb7f08c566ae07795fe7a92b9e00908852
🤵🤵🤵🤵🤵🤵🤵 In Aura continuing.
2023-11-15 16:12:30 [Parachain] Could not find the header of the genesis block in the database! block_hash=0x633ae7ab12476b9af06994356a7b2fbb7f08c566ae07795fe7a92b9e00908852

My debugging logs show that the parent that we are looking for, to collate on, is not the genesis block we expected. You can't see it from these logs, but I've also confirmed that the state root and extrinsics root are both unexpected as well.

This is exactly what I would expect if the collator process or relay chain was somewhere "assuming" that I used the standard genesis block builder. But I can't figure out where. The only place I could find was in the ExportGenesisState command, and I fixed that in #2331, but I still have this problem.

And this custom genesis builder is working fine in a stand-alone node.

@bkchr, if you could help me track this down, I would be very grateful.

@bkchr
Copy link
Member

bkchr commented Nov 15, 2023

Looks like you may register the wrong header on the relay chain. Best is that you check the head data in the relay chain state.

@muraca
Copy link
Contributor

muraca commented Nov 16, 2023

I've been playing with this issue as well.
When launching the network, the output is:
[Parachain] 🔨 Initializing Genesis block/state (state: 0xdb2f…d87b, header-hash: 0xc43b…254f)

Once the parachain is started, the output is the following:

tracing::error!(
target: LOG_TARGET,
block_hash = ?hash,
"Could not find the header of the genesis block in the database!",
);

[Parachain] Could not find the header of the genesis block in the database! block_hash=0xafd7f546d317aa37b61562a9721505a5f5ae9ce57a2368e89e25f40b446eedab

I added the following lines to retrieve the expected hash:

tracing::error!(
    target: LOG_TARGET,
    expected_hash = ?self.block_status.block_hash(Zero::zero()),
    "We were expecting another hash.",
);

[Parachain] We were expecting another hash. expected_hash=Ok(Some(0xc43be0484c645277473d541ce98d664cbdf32cf258078abc31c964d21c16254f))
It matches the one printed at the beginning.

bkchr added a commit 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants