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

Deneb: get-blobs-ssz #12611

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Deneb: get-blobs-ssz #12611

merged 1 commit into from
Jul 11, 2023

Conversation

james-prysm
Copy link
Contributor

@james-prysm james-prysm commented Jul 11, 2023

What type of PR is this?
Feature

What does this PR do? Why is it needed?

Adds ssz to the get blob sidecars beacon API endpoint
https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getBlobSidecars

Which issues(s) does this PR fix?

part of #12248

Other notes for review

@james-prysm james-prysm requested a review from a team as a code owner July 11, 2023 00:57
@james-prysm james-prysm requested review from kasey, terencechain and nisdas and removed request for a team July 11, 2023 00:57
@@ -213,7 +213,8 @@ func handlePostSSZ(m *apimiddleware.ApiProxyMiddleware, endpoint apimiddleware.E
return true
}

func sszRequested(req *http.Request) (bool, error) {
// SszRequested returns a bool value if the ssz type is requested.
func SszRequested(req *http.Request) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's maybe move this function to /network (how about a reader.go file?) because it doesn't make sense to import the middleware package in the HTTP handler. We shouldn't need the middleware at all when writing HTTP handlers.

beacon-chain/rpc/eth/blob/handlers.go Outdated Show resolved Hide resolved
network.WriteError(w, errJson)
return
}
network.WriteSsz(w, sszResp, "sidecar_response.ssz")
Copy link
Contributor

Choose a reason for hiding this comment

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

The spec defines the return object as "blob sidecars", so blob_sidecars.ssz could be more appropriate

network.WriteError(w, errJson)
return
}
sidecarResp := &ethpb.SidecarsResponse{
Copy link
Contributor

@rkapka rkapka Jul 11, 2023

Choose a reason for hiding this comment

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

You don't need to define SidecarsResponse. You can use BlobSidecars because SSZ does not preserve field names. It only stores offsets and binary data.

@james-prysm james-prysm changed the title initial implementation and tests Deneb: get-blobs-ssz Jul 11, 2023
@james-prysm james-prysm added the Deneb PRs or issues for the Deneb upgrade label Jul 11, 2023
@james-prysm james-prysm force-pushed the get-blobs-ssz branch 2 times, most recently from 1115fee to 460833e Compare July 11, 2023 21:04
@james-prysm james-prysm merged commit 9bb60c9 into deneb-integration Jul 11, 2023
13 of 15 checks passed
@james-prysm james-prysm deleted the get-blobs-ssz branch July 11, 2023 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb PRs or issues for the Deneb upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants