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

Add finalized property to HTTP API responses #5693

Closed
nflaig opened this issue Jun 24, 2023 · 6 comments · Fixed by #6645
Closed

Add finalized property to HTTP API responses #5693

nflaig opened this issue Jun 24, 2023 · 6 comments · Fixed by #6645
Assignees
Labels
help wanted The author indicates that additional help is wanted. meta-feature-request Issues to track feature requests. prio-medium Resolve this some time soon (tm).
Milestone

Comments

@nflaig nflaig added the meta-feature-request Issues to track feature requests. label Jun 24, 2023
@nflaig nflaig added the help wanted The author indicates that additional help is wanted. label Jun 26, 2023
@philknows
Copy link
Member

@zilm13
Copy link

zilm13 commented Mar 21, 2024

Any updates on this?
Testing Teku VC with Lodestar BN to fix Consensys/teku#8118
And getting error in parsing https://ethereum.github.io/beacon-APIs/#/Beacon/getStateValidators because of missing finalized field which is required by the spec https://github.com/ethereum/beacon-APIs/blob/master/apis/beacon/states/validators.yaml#L48

@nflaig
Copy link
Member Author

nflaig commented Mar 21, 2024

I think @ensi321 wanted to look into this, right now Lodestar is pretty limited in serving finalized data derived from the state as we only keep the unfinalized state in-memory and only persist very few states to load from disk (will be addressed by #6033). We could just hard code those to finalized=false for now to fix the compatibility issue and be spec compliant.

On the note of cross-client compatibliy, we currently also only support json bodies for all requests but as per spec there are a few APIs that should also support ssz bodies. How does Teku handle this case, do you send the ssz body and if you receive a 415 (unsupported media type) do you resend the request with a json body?

I am working on a overhaul of the API to add first class support for ssz res/resp to (almost) all APIs so this will be addressed soon as well.

@zilm13
Copy link

zilm13 commented Mar 21, 2024

@nflaig
Hmm if it's really not finalized it would be fair as a quick fix.
Yeah, we send it by default in ssz (could be overridden with command line flag), if we receive 415, we fallback to json storing that ssz is not supported, so on next requests we will use json until VC is restarted.

@nflaig
Copy link
Member Author

nflaig commented Mar 21, 2024

if we receive 415, we fallback to json storing that ssz is not supported, so on next requests we will use json until VC is restarted

ah perfekt, this is the ideal behavior I also had in mind because we want to send ssz bodies for almost all APIs even those that don't have it (yet) defined in the spec

@nflaig nflaig added prio-medium Resolve this some time soon (tm). and removed prio-low This is nice to have. labels Mar 22, 2024
@nflaig nflaig assigned nflaig and unassigned wemeetagain Apr 5, 2024
@nflaig
Copy link
Member Author

nflaig commented Apr 5, 2024

We can implement this already without the historical state regen, we already can serve some finalized data without it, e.g. finalized blocks or finalized checkpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted The author indicates that additional help is wanted. meta-feature-request Issues to track feature requests. prio-medium Resolve this some time soon (tm).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants