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

Use the ER from the canonical consensus chain when construct bundle #1871

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

NingLin-P
Copy link
Member

#1353 have discovered an issue that different consensus blocks may able to derive the exact same domain block, and because the domain_hash is used as the key to index ER, the operator may get an ER from non-canonical consensus chain when using domain_hash to load ER and failed to submit to the consensus chain.

#1363 tried to fix the issue by using consensus_hash to index the ER instead, however the issue still exists because the operator will use the domain_hash => consensus_hash mapping to get the consensus_hash by domain_hash first and then use consensus_hash to load ER, since the same domain block could still map to the consensus block of the non-canonical chain, it will still result in a wrong ER. Domain v2 using the same way to load ER thus this issue still exists

This PR tries to fix the issue completely by allowing a domain block can map to multiple consensus blocks, and only using the consensus block of the canonical chain to load ER. A test is also added in this PR, if pick the test to the main branch and run, it will fail due to BuiltOnUnknownConsensusBlock error.

Code contributor checklist:

Signed-off-by: linning <linningde25@gmail.com>
Signed-off-by: linning <linningde25@gmail.com>
Signed-off-by: linning <linningde25@gmail.com>
vedhavyas
vedhavyas previously approved these changes Aug 24, 2023
@NingLin-P NingLin-P merged commit 95bbf65 into main Aug 24, 2023
9 checks passed
@NingLin-P NingLin-P deleted the operator-er branch August 24, 2023 16:11
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