Skip to content

Comments

Deneb: get-blobs-ssz#12611

Merged
james-prysm merged 1 commit intodeneb-integrationfrom
get-blobs-ssz
Jul 11, 2023
Merged

Deneb: get-blobs-ssz#12611
james-prysm merged 1 commit intodeneb-integrationfrom
get-blobs-ssz

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, nisdas and terencechain and removed request for a team July 11, 2023 00:57

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.

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 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
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants