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

Add consensus_block_info digest to domain block header and use it to load ER #1885

Merged
merged 7 commits into from
Aug 29, 2023

Conversation

NingLin-P
Copy link
Member

@NingLin-P NingLin-P commented Aug 28, 2023

#1871 introduced changes that when there are multiple consensus blocks mapped to a domain block, always use the ER that derives from the canonical consensus block. While this is correct for loading ER to submit with bundle, it is problematic for another use case, namely loading the parent ER when constructing an ER. In this case, it is expected to load the parent ER that is in the same fork as the under-constructing ER instead of always loading the ER from the canonical chain.

This PR adds a digest to the domain block header, which contains the hash of the domain consensus block that derives the domain block, and uses this digest to load ER, with this approach we can:

  • Ensure different consensus blocks must derive different domain blocks, a domain block can uniquely index and load an ER thus there should be no more similar ER-related issues
  • Remove the aux storage for the domain block => consensus block mapping, since the domain block header already contains this mapping inherently

Code contributor checklist:

This degist contains the consensus block hash that derive the domain block, it is
used in later commits to load ER. This commit also revise the sp-domain-digests
to replace the deprecated primary block with consensus block and remove the
consensus block number from the digest since it is useless

Signed-off-by: linning <[email protected]>
…d ER

This commit change to the way to load ER from using the domain_hash => consensus_hash
mapping to using the consensus hash of the consensus_block_info digest

Signed-off-by: linning <[email protected]>
Copy link
Contributor

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

a small nit lgtm

domains/client/domain-operator/src/lib.rs Outdated Show resolved Hide resolved
@NingLin-P NingLin-P merged commit b436abd into main Aug 29, 2023
9 checks passed
@NingLin-P NingLin-P deleted the domain-digest branch August 29, 2023 11:58
@vedhavyas
Copy link
Contributor

@NingLin-P we should also push a companion pr for gemini-3f maintenance branch

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