-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update espresso-sequencer-go with new LC ABI #230
Conversation
Instead of using the block height from the first snapshot after the height in the justification it uses the latest height in the LC contract. Currently not 100% convinced about correctness but also believe it should work because any newer tree still contains the header of interest.
arbnode/batch_poster.go
Outdated
@@ -506,23 +506,28 @@ func (b *BatchPoster) addEspressoBlockMerkleProof( | |||
return fmt.Errorf("this msg has not been included in hotshot") | |||
} | |||
|
|||
snapshot, err := b.lightClientReader.FetchMerkleRoot(jst.Header.Height, nil) | |||
_, err = b.lightClientReader.FetchMerkleRoot(jst.Header.Height, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to fetch the merkle root now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the only thing we needed from the snapshot was the hotshot height. I think were fine to remove this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we still need this call at all?
This seems correct because the validated height should be equivalent to the latest snapshot height |
We're not comfortable with the previous change because making a proof w.r.t to the newest finalized state may cause issues if the batch poster is lagging behind. This is currently not easy to test so we decided to revert back to the old flow.
Instead of using the block height from the first snapshot after the height in the justification it uses the latest height in the LC contract. Currently not 100% convinced about correctness but also believe it should work because any newer tree still contains the header of interest.
Closes #229