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

Fetch the hotshot liveness instead of inferring it #171

Open
wants to merge 15 commits into
base: integration
Choose a base branch
from

Conversation

ImJeremyHe
Copy link
Member

This PR:

Validators will not infer the hotshot liveness from the messages. It would fetch the hotshot liveness from the hotshot light client contract.
This provides the STF the correct information to check the hotshot liveness.

var isHotShotLive bool
isHotShotLive, err := v.lightClientReader.IsHotShotLive(msg.Message.Header.BlockNumber)
if err != nil {
return false, fmt.Errorf("error fetching the hotshot liveness")

Choose a reason for hiding this comment

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

nit: I think we should also attach the error here like - fmt.Errorf("error fetching the hotshot liveness: %v", err)

Copy link

Choose a reason for hiding this comment

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

also attach the block number

Copy link

@Sneh1999 Sneh1999 left a comment

Choose a reason for hiding this comment

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

Also what happens if we cant fetch the liveness - should we just assume that Hotshot is down? and still proceed with transactions

@ImJeremyHe
Copy link
Member Author

ImJeremyHe commented Jul 31, 2024

Also what happens if we cant fetch the liveness - should we just assume that Hotshot is down? and still proceed with transactions

For now, the validators will just retry it again and again since there is a periodic task. The hotshot livenesses are stored in the L1 so I think there should not be a large probability that we failed to get it.

@ImJeremyHe ImJeremyHe marked this pull request as draft August 5, 2024 01:51
@ImJeremyHe ImJeremyHe marked this pull request as ready for review August 6, 2024 05:46
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.

3 participants