allow users to publish blobs #14442

0w3n-d · 2024-09-11T18:06:08Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Allowing users to publish blobs before publishing blocks, gives the blobs a head start. They can begin to propagate around the network while the block is being validated.

Which issues(s) does this PR fix?

Fixes #

Other notes for review

Acknowledgements

I have read CONTRIBUTING.md.
I have made an appropriate entry to CHANGELOG.md.
I have added a description to this PR with sufficient context for reviewers to understand this PR.

CLAassistant · 2024-09-11T18:06:16Z

All committers have signed the CLA.

beacon-chain/rpc/eth/beacon/handlers_test.go

james-prysm · 2024-09-11T19:08:12Z

beacon-chain/rpc/endpoints.go

@@ -558,6 +559,16 @@ func (s *Service) beaconEndpoints(
 			handler: server.PublishBlockV2,
 			methods: []string{http.MethodPost},
 		},
+		{


since this isn't a beacon-api https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/ this should probably go under prysmBeaconEndpoints ( line 930 and below in the same file) and be under that namespace with the prysm subpath

james-prysm · 2024-09-11T19:08:43Z

beacon-chain/rpc/endpoints_test.go

@@ -31,6 +31,7 @@ func Test_endpoints(t *testing.T) {
 		"/eth/v2/beacon/blinded_blocks":                              {http.MethodPost},
 		"/eth/v1/beacon/blocks":                                      {http.MethodPost},
 		"/eth/v2/beacon/blocks":                                      {http.MethodPost},
+		"/eth/v2/beacon/blobs":                                       {http.MethodPost},


path would be affected based on my previous comment

james-prysm · 2024-09-11T19:12:54Z

beacon-chain/rpc/eth/beacon/handlers.go

+			return
+		}
+
+		readOnlySc, err := blocks.NewROBlobWithRoot(sc, beaconRoot)


we have a helper function bytesutil.ToBytes32 maybe don't need lines 981/982

rkapka · 2024-09-11T20:34:35Z

api/server/structs/endpoints_blob.go

+
+type PublishBlobsRequest struct {
+	BlobSidecars *BlobSidecars `json:"blob_sidecars"`
+	BeaconRoot   string        `json:"beacon_root"`


This should be called either block_root or beacon_block_root since there is no such thing as a beacon root

rkapka · 2024-09-11T20:36:06Z

beacon-chain/rpc/eth/beacon/handlers.go

@@ -959,6 +959,54 @@ func (s *Server) publishBlock(ctx context.Context, w http.ResponseWriter, r *htt
 	httputil.HandleError(w, "Body does not represent a valid block type", http.StatusBadRequest)
 }

+func (s *Server) PublishBlobs(w http.ResponseWriter, r *http.Request) {


Echoing what @james-prysm said, this handler should be placed in /home/radek/go/prysm/beacon-chain/rpc/prysm/beacon/handlers.go since it's not an official Beacon API endpoint

rkapka · 2024-09-11T20:38:26Z

beacon-chain/rpc/endpoints.go

@@ -558,6 +559,16 @@ func (s *Service) beaconEndpoints(
 			handler: server.PublishBlockV2,
 			methods: []string{http.MethodPost},
 		},
+		{


rkapka · 2024-09-11T20:38:31Z

beacon-chain/rpc/endpoints_test.go

@@ -31,6 +31,7 @@ func Test_endpoints(t *testing.T) {
 		"/eth/v2/beacon/blinded_blocks":                              {http.MethodPost},
 		"/eth/v1/beacon/blocks":                                      {http.MethodPost},
 		"/eth/v2/beacon/blocks":                                      {http.MethodPost},
+		"/eth/v2/beacon/blobs":                                       {http.MethodPost},


rkapka · 2024-09-11T20:39:04Z

beacon-chain/rpc/eth/beacon/handlers.go

+	copy(beaconRoot[:], root)
+
+	for _, blobSidecar := range req.BlobSidecars.Sidecars {
+


Suggested change

rkapka · 2024-09-11T20:39:42Z

beacon-chain/rpc/eth/beacon/handlers.go

+
+	root, err := bytesutil.DecodeHexWithLength(req.BeaconRoot, 32)
+	if err != nil {
+		httputil.HandleError(w, "Could not decode beacon root", http.StatusBadRequest)


Suggested change

httputil.HandleError(w, "Could not decode beacon root", http.StatusBadRequest)

httputil.HandleError(w, "Could not decode block root", http.StatusBadRequest)

rkapka · 2024-09-11T20:40:44Z

beacon-chain/rpc/eth/beacon/handlers.go

+
+		sc, err := blobSidecar.ToConsensus()
+		if err != nil {
+			http.Error(w, "Could not decode blob sidecar", http.StatusBadRequest)


please use httputil.HandleError

rkapka · 2024-09-11T20:40:49Z

beacon-chain/rpc/eth/beacon/handlers.go

+
+		readOnlySc, err := blocks.NewROBlobWithRoot(sc, beaconRoot)
+		if err != nil {
+			http.Error(w, "Could not create read-only blob", http.StatusInternalServerError)


please use httputil.HandleError

rkapka · 2024-09-11T20:40:53Z

beacon-chain/rpc/eth/beacon/handlers.go

+
+		verifiedBlob := blocks.NewVerifiedROBlob(readOnlySc)
+		if err := s.BlobReceiver.ReceiveBlob(ctx, verifiedBlob); err != nil {
+			http.Error(w, "Could not receive blob", http.StatusInternalServerError)


please use httputil.HandleError

beacon-chain/rpc/prysm/beacon/handlers.go

prestonvanloon · 2024-09-13T01:58:02Z

beacon-chain/rpc/prysm/testing/BUILD.bazel

+
+go_library(
+    name = "go_default_library",
+    testonly = True,


Nice. Thanks for marking this as testonly = True.

prestonvanloon · 2024-09-13T02:04:50Z

beacon-chain/rpc/prysm/beacon/BUILD.bazel

@@ -18,12 +18,15 @@ go_library(
        "//beacon-chain/rpc/eth/helpers:go_default_library",
        "//beacon-chain/rpc/eth/shared:go_default_library",
        "//beacon-chain/rpc/lookup:go_default_library",


This file is missing somethings. Please run bazel run //:gazelle

beacon-chain/rpc/prysm/beacon/handlers.go

james-prysm · 2024-09-16T21:14:01Z

waiting for conflicts to resolve before next review

0w3n-d · 2024-09-17T18:27:18Z

waiting for conflicts to resolve before next review

Conflicts resolved now

james-prysm · 2024-09-18T14:46:27Z

beacon-chain/rpc/prysm/testing/json.go

+	}
+ }`, Blob)
+
+ var PublishBlobsRequestEmptySidecarsList = fmt.Sprintf(`{


linter complaining here

james-prysm · 2024-09-18T14:46:49Z

There are linting errors

0w3n-d · 2024-09-30T17:05:10Z

There are linting errors

linting should be fixed now

james-prysm · 2024-09-30T22:25:46Z

seems like stuff is still broken

Allowing users to publish blobs before publishing blocks, gives the blobs a head start. They can begin to propagate around the network while the block is being validated.

0w3n-d · 2024-10-01T09:12:45Z

seems like stuff is still broken

OK hopefully It's fixed now. I've also squashed the commits as it was getting a bit long.

prestonvanloon · 2024-10-01T15:45:41Z

Please run gazelle

0w3n-d · 2024-10-01T19:01:26Z

Please run gazelle

Done

beacon-chain/rpc/prysm/beacon/handlers.go

0w3n-d requested a review from a team as a code owner September 11, 2024 18:06

0w3n-d requested review from potuz, terencechain and rkapka September 11, 2024 18:06

prestonvanloon reviewed Sep 11, 2024

View reviewed changes

beacon-chain/rpc/eth/beacon/handlers_test.go Outdated Show resolved Hide resolved

james-prysm reviewed Sep 11, 2024

View reviewed changes

rkapka reviewed Sep 11, 2024

View reviewed changes

0w3n-d force-pushed the publish_blobs branch 3 times, most recently from abb5e6d to e7017f0 Compare September 12, 2024 15:35

james-prysm reviewed Sep 12, 2024

View reviewed changes

beacon-chain/rpc/prysm/beacon/handlers.go Show resolved Hide resolved

prestonvanloon reviewed Sep 13, 2024

View reviewed changes

james-prysm reviewed Sep 18, 2024

View reviewed changes

0w3n-d force-pushed the publish_blobs branch from 642badf to 9398025 Compare September 30, 2024 17:01

allow users to publish blobs

1898ac0

Allowing users to publish blobs before publishing blocks, gives the blobs a head start. They can begin to propagate around the network while the block is being validated.

0w3n-d force-pushed the publish_blobs branch from 9398025 to 1898ac0 Compare October 1, 2024 09:10

Merge branch 'develop' into publish_blobs

9dd4d31

0w3n-d force-pushed the publish_blobs branch from c02ea58 to 9dd4d31 Compare October 1, 2024 18:59

prestonvanloon previously approved these changes Oct 1, 2024

View reviewed changes

rkapka reviewed Oct 1, 2024

View reviewed changes

Update beacon-chain/rpc/prysm/beacon/handlers.go

68b566f

rkapka dismissed prestonvanloon’s stale review via 68b566f October 1, 2024 19:45

rkapka added 4 commits October 1, 2024 21:45

Update beacon-chain/rpc/prysm/beacon/handlers.go

33c9594

Update beacon-chain/rpc/prysm/beacon/handlers.go

47a8197

Update beacon-chain/rpc/prysm/beacon/handlers.go

c1ae5bf

Update beacon-chain/rpc/prysm/beacon/handlers.go

4759073

rkapka previously approved these changes Oct 1, 2024

View reviewed changes

rkapka reviewed Oct 1, 2024

View reviewed changes

Update beacon-chain/rpc/prysm/beacon/handlers.go

5566e59

rkapka dismissed their stale review via 5566e59 October 1, 2024 19:51

rkapka added 4 commits October 1, 2024 21:51

Update beacon-chain/rpc/prysm/beacon/handlers.go

23951dd

Update beacon-chain/rpc/prysm/beacon/handlers.go

f7be0af

Update beacon-chain/rpc/prysm/beacon/handlers.go

e9d2f19

Update beacon-chain/rpc/prysm/beacon/handlers.go

137de33

rkapka approved these changes Oct 1, 2024

View reviewed changes

rkapka enabled auto-merge October 1, 2024 20:01

rkapka added this pull request to the merge queue Oct 1, 2024

Merged via the queue into prysmaticlabs:develop with commit 2e29164 Oct 1, 2024
18 checks passed

		copy(beaconRoot[:], root)

		for _, blobSidecar := range req.BlobSidecars.Sidecars {

	httputil.HandleError(w, "Could not decode beacon root", http.StatusBadRequest)
	httputil.HandleError(w, "Could not decode block root", http.StatusBadRequest)

allow users to publish blobs #14442

allow users to publish blobs #14442

Conversation

0w3n-d commented Sep 11, 2024 • edited Loading

CLAassistant commented Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

james-prysm commented Sep 16, 2024

0w3n-d commented Sep 17, 2024

Choose a reason for hiding this comment

james-prysm commented Sep 18, 2024

0w3n-d commented Sep 30, 2024

james-prysm commented Sep 30, 2024

0w3n-d commented Oct 1, 2024

prestonvanloon commented Oct 1, 2024

0w3n-d commented Oct 1, 2024

0w3n-d commented Sep 11, 2024 •

edited

Loading

CLAassistant commented Sep 11, 2024 •

edited

Loading