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

Keep domain compatible with gemini-3h #2638

Merged
merged 4 commits into from
Mar 25, 2024
Merged

Keep domain compatible with gemini-3h #2638

merged 4 commits into from
Mar 25, 2024

Conversation

NingLin-P
Copy link
Member

The first commit fixes the recent domain incompatible issue with gemini-3h. The issue is due to the change of the order of the domain inherent extrinsic, the change is used to correct the order to keep consistency with the order on the fraud proof verification side, but because the change is on the client side, the domain node run with different release will produce different ER and trigger fraud proof.

The PR fixes the issue on gemini-3h by using the old order for consensus block <= #168430 and switching to the new order for consensus block > #168430, the block #168430 is where the order changed in the ER chain on the consensus state.

The second commit is some minor changes to add more detailed log for fraud proof.

Code contributor checklist:

Signed-off-by: linning <linningde25@gmail.com>
Signed-off-by: linning <linningde25@gmail.com>
// from the consensus block #168431, we have to follow this change in the client side to ensure
// every domain node that sync from genesis will produce the same ER and hence can successfully
// submit ER to exend the previous ER.
let maintain_runtime_inherent_extrinsic_order = parent_number >= 168430u32.into();
Copy link
Member

Choose a reason for hiding this comment

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

You should also check that the chain is in fact Gemini 3h using genesis hash or something equivalent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added code to determine if the node is running in gemini-3h by checking the genesis hash, and tested in gemini-3h.

Signed-off-by: linning <linningde25@gmail.com>
Comment on lines +424 to +430
let is_gemini_3h = self.consensus_client.info().genesis_hash
== FromStr::from_str(
// The genesis hash of gemini-3h
"0c121c75f4ef450f40619e1fca9d1e8e7fbabc42c895bc4790801e85d5a91c34",
)
.map_err(|_| ())
.expect("parsing consensus block hash should success");
Copy link
Member

Choose a reason for hiding this comment

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

Assuming parsing works, this should do the job, but even better way would be to define genesis has as [u8; 32] constant and store boolean result as a property of the data structure to not query info all the time since genesis hash never changes.

nazar-pc
nazar-pc previously approved these changes Mar 25, 2024
Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Non-blocking

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P NingLin-P added this pull request to the merge queue Mar 25, 2024
Merged via the queue into main with commit 360fb7e Mar 25, 2024
9 checks passed
@NingLin-P NingLin-P deleted the domain-compatible-3h branch March 25, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants