-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: implement new genesis endpoint #41
Conversation
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.
In general this looks good, except that this change carries a typo forward ("dequencer") and that I think the omitempty
has on the base heights has the wrong semantics. Also I think I would like to emphasize celestia here (over data availability) similar to what we did in conductor.
func (s *ExecutionServiceServerV1Alpha2) GetGenesisInfo(ctx context.Context, req *astriaPb.GetGenesisInfoRequest) (*astriaPb.GenesisInfo, error) { | ||
log.Info("GetGenesisInfo called", "request", req) | ||
|
||
rollupId := sha256.Sum256([]byte(s.bc.Config().AstriaRollupName)) |
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.
nit: Inject the ID into the config as part of startup instead of calculating it ad hoc? No hard opinion on that tho.
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.
I think this is the best way to do this without messing with how configs get parsed and such in geth. As many changes kept to this file is better. Especially since this is called once on startup, not a high computation cost.
This updates to support the new execution API endpoint
GetGenesisInfo
and additionally adds some metrics: