Blinded block APIs #10331

rkapka · 2022-03-09T15:58:01Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Implements publishBlindedBlock and produceBlindedBlock API endpoints as described in ethereum/beacon-APIs#194

# Conflicts: # beacon-chain/rpc/apimiddleware/custom_hooks.go

terencechain · 2022-03-17T20:46:20Z

beacon-chain/rpc/apimiddleware/custom_hooks.go

+	}
+
+	var actualRespContainer interface{}
+	if strings.EqualFold(respContainer.Version, strings.ToLower(ethpbv2.Version_PHASE0.String())) {


Using a switch statement on respContainer.Version will look much nicer than if else if else if else

terencechain · 2022-03-17T21:04:56Z

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

+
+	root, err := phase0Blk.HashTreeRoot()
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not tree hash block: %v", err)
+	}
+
+	// Do not block proposal critical path with debug logging or block feed updates.
+	defer func() {
+		log.WithField("blockRoot", fmt.Sprintf("%#x", bytesutil.Trunc(root[:]))).Debugf(
+			"Block proposal received via RPC")
+		bs.BlockNotifier.BlockFeed().Send(&feed.Event{
+			Type: blockfeed.ReceivedBlock,
+			Data: &blockfeed.ReceivedBlockData{SignedBlock: wrappedPhase0Blk},
+		})
+	}()
+
+	// Broadcast the new block to the network.
+	if err := bs.Broadcaster.Broadcast(ctx, wrappedPhase0Blk.Proto()); err != nil {
+		return status.Errorf(codes.Internal, "Could not broadcast block: %v", err)
+	}
+
+	if err := bs.BlockReceiver.ReceiveBlock(ctx, wrappedPhase0Blk, root); err != nil {
+		return status.Errorf(codes.Internal, "Could not process beacon block: %v", err)
+	}
+
+	return nil
+}
+
+func (bs *Server) submitAltairBlock(ctx context.Context, altairBlk *ethpbv2.BeaconBlockAltair, sig []byte) error {
+	v1alpha1Blk, err := migration.AltairToV1Alpha1SignedBlock(&ethpbv2.SignedBeaconBlockAltair{Message: altairBlk, Signature: sig})
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not convert block to v1 block")
+	}
+	wrappedAltairBlk, err := wrapper.WrappedAltairSignedBeaconBlock(v1alpha1Blk)
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not prepare Altair block")
+	}
+
+	root, err := altairBlk.HashTreeRoot()
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not tree hash block: %v", err)
+	}
+
+	// Do not block proposal critical path with debug logging or block feed updates.
+	defer func() {
+		log.WithField("blockRoot", fmt.Sprintf("%#x", bytesutil.Trunc(root[:]))).Debugf(
+			"Block proposal received via RPC")
+		bs.BlockNotifier.BlockFeed().Send(&feed.Event{
+			Type: blockfeed.ReceivedBlock,
+			Data: &blockfeed.ReceivedBlockData{SignedBlock: wrappedAltairBlk},
+		})
+	}()
+
+	// Broadcast the new block to the network.
+	if err := bs.Broadcaster.Broadcast(ctx, wrappedAltairBlk.Proto()); err != nil {
+		return status.Errorf(codes.Internal, "Could not broadcast block: %v", err)
+	}
+
+	if err := bs.BlockReceiver.ReceiveBlock(ctx, wrappedAltairBlk, root); err != nil {
+		return status.Errorf(codes.Internal, "Could not process beacon block: %v", err)
+	}
+
+	return nil
+}
+
+func (bs *Server) submitBellatrixBlock(ctx context.Context, bellatrixBlk *ethpbv2.BeaconBlockBellatrix, sig []byte) error {
+	v1alpha1Blk, err := migration.BellatrixToV1Alpha1SignedBlock(&ethpbv2.SignedBeaconBlockBellatrix{Message: bellatrixBlk, Signature: sig})
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not convert block to v1 block")
+	}
+	wrappedBellatrixBlk, err := wrapper.WrappedBellatrixSignedBeaconBlock(v1alpha1Blk)
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not prepare bellatrix block")
+	}
+
+	root, err := bellatrixBlk.HashTreeRoot()
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not tree hash block: %v", err)
+	}
+
+	// Do not block proposal critical path with debug logging or block feed updates.
+	defer func() {
+		log.WithField("blockRoot", fmt.Sprintf("%#x", bytesutil.Trunc(root[:]))).Debugf(
+			"Block proposal received via RPC")
+		bs.BlockNotifier.BlockFeed().Send(&feed.Event{
+			Type: blockfeed.ReceivedBlock,
+			Data: &blockfeed.ReceivedBlockData{SignedBlock: wrappedBellatrixBlk},
+		})
+	}()
+
+	// Broadcast the new block to the network.
+	if err := bs.Broadcaster.Broadcast(ctx, wrappedBellatrixBlk.Proto()); err != nil {
+		return status.Errorf(codes.Internal, "Could not broadcast block: %v", err)
+	}
+
+	if err := bs.BlockReceiver.ReceiveBlock(ctx, wrappedBellatrixBlk, root); err != nil {
+		return status.Errorf(codes.Internal, "Could not process beacon block: %v", err)
+	}
+
+	return nil


Could you dedup it the same way that is similar to #10368 ?

It'll make @prestonvanloon very happy :)

You had similar feedback in #10324 (comment). Can I merge the two into a separate PR?

# Conflicts: # beacon-chain/rpc/apimiddleware/custom_hooks.go # beacon-chain/rpc/eth/validator/validator.go

potuz · 2022-03-29T19:57:36Z

testing/util/block.go

+	}
+	if b.ExecutionPayloadHeader == nil {
+		b.ExecutionPayloadHeader = &enginev1.ExecutionPayloadHeader{
+			ParentHash:       make([]byte, 32),


Can we use named constants for these numbers?

I updated all roots to constants. Although you could argue that hashes should use the same constant, we are not doing this at the moment.

terencechain

Another pass. I am ~33% there

terencechain · 2022-03-31T03:57:35Z

beacon-chain/rpc/apimiddleware/custom_hooks.go

+	if currentEpoch < params.BeaconConfig().AltairForkEpoch {
+		endpoint.PostRequest = &signedBeaconBlockContainerJson{}
+	} else if currentEpoch < params.BeaconConfig().BellatrixForkEpoch {
+		endpoint.PostRequest = &signedBeaconBlockAltairContainerJson{}


Are the blocks before Bellatrix blinded as well?

Nope, we return regular blocks in that case.

terencechain · 2022-03-31T04:15:38Z

beacon-chain/rpc/apimiddleware/structs.go

@@ -270,6 +270,12 @@ type produceBlockResponseV2Json struct {
 	Data    *beaconBlockContainerV2Json `json:"data"`
 }

+// produceBlindedBlockResponseJson is used in /v1/validator/blinded_blocks/{slot} API endpoint.
+type produceBlindedBlockResponseJson struct {


What's the difference between bellatrixProduceBlindedBlockResponseJson?

produceBlindedBlockResponseJson comes back from gRPC and includes a Data field, which itself has 3 fields:

Phase0Block

AltairBlock

BellatrixBlock

This is because we use a oneof in protobuf. The purpose of bellatrixProduceBlindedBlockResponseJson is to contain only one field (apart from version) which gets returned from HTTP - the bellatrix block.

terencechain · 2022-03-31T04:18:34Z

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

+	root, err := phase0Blk.HashTreeRoot()
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "Could not tree hash block: %v", err)
+	}
+
+	// Do not block proposal critical path with debug logging or block feed updates.
+	defer func() {
+		log.WithField("blockRoot", fmt.Sprintf("%#x", bytesutil.Trunc(root[:]))).Debugf(
+			"Block proposal received via RPC")
+		bs.BlockNotifier.BlockFeed().Send(&feed.Event{
+			Type: blockfeed.ReceivedBlock,
+			Data: &blockfeed.ReceivedBlockData{SignedBlock: wrappedPhase0Blk},
+		})
+	}()
+
+	// Broadcast the new block to the network.
+	if err := bs.Broadcaster.Broadcast(ctx, wrappedPhase0Blk.Proto()); err != nil {
+		return status.Errorf(codes.Internal, "Could not broadcast block: %v", err)
+	}
+
+	if err := bs.BlockReceiver.ReceiveBlock(ctx, wrappedPhase0Blk, root); err != nil {
+		return status.Errorf(codes.Internal, "Could not process beacon block: %v", err)
+	}


I wonder if you could share these four calls among phase0, altair and bellatrix (not blinded)

potuz · 2022-04-01T00:09:02Z

beacon-chain/rpc/apimiddleware/custom_hooks.go

+// gRPC expects either an XXX_block field in the JSON object, but we have a message field at this point.
+// We do a simple conversion depending on the type of endpoint.PostRequest
+// (which was filled out previously in setInitialPublishBlockPostRequest).
+func preparePublishedBlindedBlock(endpoint *apimiddleware.Endpoint, _ http.ResponseWriter, _ *http.Request) apimiddleware.ErrorJson {


why do we have checks for phase0 and altair blocks here?

The spec allows publishing regular blocks through this endpoint for these forks. See https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/publishBlindedBlock (you need to click the Schema button and expand the oneOf)

potuz · 2022-04-01T00:14:53Z

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

+// `BeaconBlock`, and a successful response (20X) only indicates that the broadcast has been
+// successful. The beacon node is expected to integrate the new block into its state, and
+// therefore validate the block internally, however blocks which fail the validation are still
+// broadcast but a different status code is returned (202).


Doesn't 20X cover 202?

I copied this description from the spec. I think what this tries to say is that a code different than 200 is returned.

potuz · 2022-04-01T00:18:49Z

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

-	root, err := wsb.Block().HashTreeRoot()
-	if err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "Could not tree hash block: %v", err)
+	phase0BlkContainer, ok := req.Message.(*ethpbv2.SignedBlindedBeaconBlockContainer_Phase0Block)


nitpick: does it make sense to invert the order of these requests? I suppose we are more likely to submit a blinded bellatrix block.

terencechain · 2022-04-01T01:01:39Z

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

+	root, err := blindedBellatrixBlk.HashTreeRoot()
+	if err != nil {
+		return status.Errorf(codes.Internal, "Could not calculate block root: %v", err)
+	}
+	signedBlk, err := bs.BeaconDB.Block(ctx, root)
+	if err != nil {
+		return status.Errorf(codes.Internal, "Could not calculate get non-blinded block from DB: %v", err)
+	}


This is not quite right. We won't have the block in DB.

We have to process the blinded block. I have a PR for this, I can open it next week, so this makes the right ReceiveBlock call

This is not quite right. We won't have the block in DB.

Wow, of course. This would come up while testing on a testnet.

Please use #10473 :)

# Conflicts: # proto/eth/v2/generated.ssz.go

terencechain · 2022-04-18T05:25:59Z

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

+		Signature: sig,
+	})
+	if err != nil {
+		return status.Errorf(codes.Internal, "Could not get non-blinded block: %v", err)


Are these error messages correct? These are blinded blocks no?

# Conflicts: # proto/eth/v2/beacon_state.pb.go # proto/eth/v2/beacon_state.proto # proto/eth/v2/generated.ssz.go # proto/migration/v1alpha1_to_v2.go

rkapka added 10 commits March 9, 2022 15:44

protos

34816ed

ssz

61ff1e5

method stub

4e7c336

method stub v2

1cce354

Merge branch '__develop' into define-blinded-block

774eebb

SubmitBlindedBlock grpc

9805cc3

update proto to accept more types

aa4c333

update protos

5356c87

update implementation for new block types

61abbf9

api middleware

21b2d3f

rkapka changed the title ~~Define blinded blocks~~ Blinded block APIs Mar 11, 2022

rkapka added 6 commits March 11, 2022 16:16

protos for ProduceBlindedBlock

03f3279

Merge branch '__develop' into define-blinded-block

4c36b63

# Conflicts: # beacon-chain/rpc/apimiddleware/custom_hooks.go

ProduceBlindedBlock grpc

3c80c73

remove ExecutionPayloadHeader from v2 protos

e23a32c

ProduceBlindedBlock middleware

8772196

fix endpoint URL in proto service

2f69402

rkapka marked this pull request as ready for review March 16, 2022 12:56

rkapka requested a review from a team as a code owner March 16, 2022 12:56

rkapka requested review from jmozah, rauljordan and james-prysm March 16, 2022 12:56

rkapka added Ready For Review A pull request ready for code review API Api related tasks Merge PRs related to the great milestone the merge labels Mar 16, 2022

field name fix

4b20c93

terencechain requested changes Mar 17, 2022

View reviewed changes

rkapka added 3 commits March 18, 2022 10:13

convert if else into switch

0b77833

Merge branch '__develop' into define-blinded-block

58411cf

# Conflicts: # beacon-chain/rpc/apimiddleware/custom_hooks.go # beacon-chain/rpc/eth/validator/validator.go

Merge branch 'develop' into define-blinded-block

df70264

rkapka added 3 commits March 29, 2022 16:25

Merge branch 'develop' into define-blinded-block

0ecd9fc

fix validator tests

8dd7c29

more switch statements

d4b31e4

potuz reviewed Mar 29, 2022

View reviewed changes

rkapka added 2 commits March 30, 2022 16:55

Merge branch '__develop' into define-blinded-block

206a651

update roots to constants

b458ab5

terencechain reviewed Mar 31, 2022

View reviewed changes

deduplicate submitting block

27504ea

rkapka force-pushed the define-blinded-block branch from cf92f28 to 27504ea Compare March 31, 2022 09:45

Merge branch 'develop' into define-blinded-block

7774e97

potuz reviewed Apr 1, 2022

View reviewed changes

terencechain reviewed Apr 1, 2022

View reviewed changes

revert order of blinded block checks

1b97ea5

terencechain mentioned this pull request Apr 1, 2022

Add blinded beacon block protobufs and wrappers #10473

Merged

rkapka added 6 commits April 14, 2022 13:48

Merge branch '__develop' into define-blinded-block

c528a19

# Conflicts: # proto/eth/v2/generated.ssz.go

remove duplicated functions

8945abe

use new methods

7318f91

fix issues

a788dec

fix validator test

303a887

fix wrapper tests

a0802c9

terencechain reviewed Apr 18, 2022

View reviewed changes

rkapka added 6 commits April 21, 2022 17:38

correct error messages

a7441f2

Merge branch '__develop' into define-blinded-block

783eccd

# Conflicts: # proto/eth/v2/beacon_state.pb.go # proto/eth/v2/beacon_state.proto # proto/eth/v2/generated.ssz.go # proto/migration/v1alpha1_to_v2.go

Merge branch 'develop' into define-blinded-block

97b53b5

Merge branch 'develop' into define-blinded-block

61f4f77

import missing package

acf6d7b

Merge branch 'develop' into define-blinded-block

e3dd847

terencechain approved these changes Apr 25, 2022

View reviewed changes

rkapka merged commit 957d853 into develop Apr 25, 2022

rkapka deleted the define-blinded-block branch April 25, 2022 15:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blinded block APIs #10331

Blinded block APIs #10331

rkapka commented Mar 9, 2022 •

edited

terencechain Mar 17, 2022

terencechain Mar 17, 2022

rkapka Mar 18, 2022

potuz Mar 29, 2022

rkapka Mar 30, 2022

terencechain left a comment

terencechain Mar 31, 2022

rkapka Mar 31, 2022

terencechain Mar 31, 2022

rkapka Mar 31, 2022 •

edited

terencechain Mar 31, 2022

potuz Apr 1, 2022

rkapka Apr 1, 2022

potuz Apr 1, 2022

rkapka Apr 1, 2022

potuz Apr 1, 2022

terencechain Apr 1, 2022

rkapka Apr 1, 2022

terencechain Apr 1, 2022

terencechain Apr 18, 2022

Blinded block APIs #10331

Blinded block APIs #10331

Conversation

rkapka commented Mar 9, 2022 • edited

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

terencechain left a 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

rkapka Mar 31, 2022 • edited

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

rkapka commented Mar 9, 2022 •

edited

rkapka Mar 31, 2022 •

edited