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

feat: implement new genesis endpoint #41

Merged
merged 6 commits into from
Feb 13, 2024
Merged

Conversation

joroshiba
Copy link
Member

@joroshiba joroshiba commented Jan 18, 2024

This updates to support the new execution API endpoint GetGenesisInfo and additionally adds some metrics:

  • for each rpc request count + success
  • information about soft & firm commitment heights
  • total number of executed transactions

Copy link
Member

@SuperFluffy SuperFluffy left a 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))
Copy link
Member

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.

Copy link
Member Author

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.

@joroshiba joroshiba changed the title feat: everything needed for genesis feat: implement new genesis endpoint Feb 7, 2024
@joroshiba joroshiba marked this pull request as ready for review February 7, 2024 22:40
@joroshiba joroshiba requested a review from noot February 7, 2024 22:41
@joroshiba joroshiba merged commit af9f7e4 into release/1.13 Feb 13, 2024
1 check passed
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