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

Reconstruct full Capella block #11732

Merged
merged 21 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions beacon-chain/blockchain/pow_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/prysmaticlabs/prysm/v3/consensus-types/interfaces"
types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v3/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v3/runtime/version"
"github.com/prysmaticlabs/prysm/v3/time/slots"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -92,6 +93,7 @@ func (s *Service) getBlkParentHashAndTD(ctx context.Context, blkHash []byte) ([]
if blk == nil {
return nil, nil, errors.New("pow block is nil")
}
blk.Version = version.Bellatrix
Copy link
Member

Choose a reason for hiding this comment

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

Why set this ? Shouldn't it already have its version set in the ExecutionBlockByHash method ? Or are you not able to infer that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not able to infer from how ExecutionBlockByHash is implemented. I can add a version argument, but this seems cleaner to me.

blkTDBig, err := hexutil.DecodeBig(blk.TotalDifficulty)
if err != nil {
return nil, nil, errors.Wrap(err, "could not decode merge block total difficulty")
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/execution/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ go_test(
"//network/authorization:go_default_library",
"//proto/engine/v1:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
"//runtime/version:go_default_library",
"//testing/assert:go_default_library",
"//testing/require:go_default_library",
"//testing/util:go_default_library",
Expand Down
49 changes: 36 additions & 13 deletions beacon-chain/execution/engine_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type ForkchoiceUpdatedResponse struct {
// block with an execution payload from a signed beacon block and a connection
// to an execution client's engine API.
type ExecutionPayloadReconstructor interface {
ReconstructFullBellatrixBlock(
ReconstructFullBlock(
ctx context.Context, blindedBlock interfaces.SignedBeaconBlock,
) (interfaces.SignedBeaconBlock, error)
ReconstructFullBellatrixBlockBatch(
Expand Down Expand Up @@ -403,9 +403,9 @@ func (s *Service) HeaderByNumber(ctx context.Context, number *big.Int) (*types.H
return hdr, err
}

// ReconstructFullBellatrixBlock takes in a blinded beacon block and reconstructs
// ReconstructFullBlock takes in a blinded beacon block and reconstructs
// a beacon block with a full execution payload via the engine API.
func (s *Service) ReconstructFullBellatrixBlock(
func (s *Service) ReconstructFullBlock(
ctx context.Context, blindedBlock interfaces.SignedBeaconBlock,
) (interfaces.SignedBeaconBlock, error) {
if err := blocks.BeaconBlockIsNil(blindedBlock); err != nil {
Expand Down Expand Up @@ -437,11 +437,12 @@ func (s *Service) ReconstructFullBellatrixBlock(
if executionBlock == nil {
return nil, fmt.Errorf("received nil execution block for request by hash %#x", executionBlockHash)
}
executionBlock.Version = blindedBlock.Version()
Copy link
Member

Choose a reason for hiding this comment

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

Same question here

payload, err := fullPayloadFromExecutionBlock(header, executionBlock)
if err != nil {
return nil, err
}
fullBlock, err := blocks.BuildSignedBeaconBlockFromExecutionPayload(blindedBlock, payload)
fullBlock, err := blocks.BuildSignedBeaconBlockFromExecutionPayload(blindedBlock, payload.Proto())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -504,7 +505,7 @@ func (s *Service) ReconstructFullBellatrixBlockBatch(
if err != nil {
return nil, err
}
fullBlock, err := blocks.BuildSignedBeaconBlockFromExecutionPayload(blindedBlocks[realIdx], payload)
fullBlock, err := blocks.BuildSignedBeaconBlockFromExecutionPayload(blindedBlocks[realIdx], payload.Proto())
if err != nil {
return nil, err
}
Expand All @@ -526,26 +527,47 @@ func (s *Service) ReconstructFullBellatrixBlockBatch(

func fullPayloadFromExecutionBlock(
header interfaces.ExecutionData, block *pb.ExecutionBlock,
) (*pb.ExecutionPayload, error) {
) (interfaces.ExecutionData, error) {
if header.IsNil() || block == nil {
return nil, errors.New("execution block and header cannot be nil")
}
if !bytes.Equal(header.BlockHash(), block.Hash[:]) {
blockHash := block.Hash
if !bytes.Equal(header.BlockHash(), blockHash[:]) {
return nil, fmt.Errorf(
"block hash field in execution header %#x does not match execution block hash %#x",
header.BlockHash(),
block.Hash,
blockHash,
)
}
txs := make([][]byte, len(block.Transactions))
for i, tx := range block.Transactions {
blockTransactions := block.Transactions
txs := make([][]byte, len(blockTransactions))
for i, tx := range blockTransactions {
txBin, err := tx.MarshalBinary()
if err != nil {
return nil, err
}
txs[i] = txBin
}
return &pb.ExecutionPayload{

if block.Version == version.Bellatrix {
return blocks.WrappedExecutionPayload(&pb.ExecutionPayload{
ParentHash: header.ParentHash(),
FeeRecipient: header.FeeRecipient(),
StateRoot: header.StateRoot(),
ReceiptsRoot: header.ReceiptsRoot(),
LogsBloom: header.LogsBloom(),
PrevRandao: header.PrevRandao(),
BlockNumber: header.BlockNumber(),
GasLimit: header.GasLimit(),
GasUsed: header.GasUsed(),
Timestamp: header.Timestamp(),
ExtraData: header.ExtraData(),
BaseFeePerGas: header.BaseFeePerGas(),
BlockHash: blockHash[:],
Transactions: txs,
})
}
return blocks.WrappedExecutionPayloadCapella(&pb.ExecutionPayloadCapella{
ParentHash: header.ParentHash(),
FeeRecipient: header.FeeRecipient(),
StateRoot: header.StateRoot(),
Expand All @@ -558,9 +580,10 @@ func fullPayloadFromExecutionBlock(
Timestamp: header.Timestamp(),
ExtraData: header.ExtraData(),
BaseFeePerGas: header.BaseFeePerGas(),
BlockHash: block.Hash[:],
BlockHash: blockHash[:],
Transactions: txs,
}, nil
Withdrawals: block.Withdrawals,
})
}

// Handles errors received from the RPC server according to the specification.
Expand Down
35 changes: 23 additions & 12 deletions beacon-chain/execution/engine_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
payloadattribute "github.com/prysmaticlabs/prysm/v3/consensus-types/payload-attribute"
"github.com/prysmaticlabs/prysm/v3/encoding/bytesutil"
pb "github.com/prysmaticlabs/prysm/v3/proto/engine/v1"
"github.com/prysmaticlabs/prysm/v3/runtime/version"
"github.com/prysmaticlabs/prysm/v3/testing/assert"
"github.com/prysmaticlabs/prysm/v3/testing/require"
"github.com/prysmaticlabs/prysm/v3/testing/util"
Expand Down Expand Up @@ -493,7 +494,7 @@ func TestReconstructFullBellatrixBlock(t *testing.T) {
t.Run("nil block", func(t *testing.T) {
service := &Service{}

_, err := service.ReconstructFullBellatrixBlock(ctx, nil)
_, err := service.ReconstructFullBlock(ctx, nil)
require.ErrorContains(t, "nil data", err)
})
t.Run("only blinded block", func(t *testing.T) {
Expand All @@ -502,7 +503,7 @@ func TestReconstructFullBellatrixBlock(t *testing.T) {
bellatrixBlock := util.NewBeaconBlockBellatrix()
wrapped, err := blocks.NewSignedBeaconBlock(bellatrixBlock)
require.NoError(t, err)
_, err = service.ReconstructFullBellatrixBlock(ctx, wrapped)
_, err = service.ReconstructFullBlock(ctx, wrapped)
require.ErrorContains(t, want, err)
})
t.Run("pre-merge execution payload", func(t *testing.T) {
Expand All @@ -517,7 +518,7 @@ func TestReconstructFullBellatrixBlock(t *testing.T) {
require.NoError(t, err)
wantedWrapped, err := blocks.NewSignedBeaconBlock(wanted)
require.NoError(t, err)
reconstructed, err := service.ReconstructFullBellatrixBlock(ctx, wrapped)
reconstructed, err := service.ReconstructFullBlock(ctx, wrapped)
require.NoError(t, err)
require.DeepEqual(t, wantedWrapped, reconstructed)
})
Expand Down Expand Up @@ -590,7 +591,7 @@ func TestReconstructFullBellatrixBlock(t *testing.T) {
blindedBlock.Block.Body.ExecutionPayloadHeader = header
wrapped, err := blocks.NewSignedBeaconBlock(blindedBlock)
require.NoError(t, err)
reconstructed, err := service.ReconstructFullBellatrixBlock(ctx, wrapped)
reconstructed, err := service.ReconstructFullBlock(ctx, wrapped)
require.NoError(t, err)

got, err := reconstructed.Block().Body().Execution()
Expand Down Expand Up @@ -1149,6 +1150,7 @@ func fixtures() map[string]interface{} {
receiptsRoot := bytesutil.PadTo([]byte("receiptsRoot"), fieldparams.RootLength)
logsBloom := bytesutil.PadTo([]byte("logs"), fieldparams.LogsBloomLength)
executionBlock := &pb.ExecutionBlock{
Version: version.Bellatrix,
Header: gethtypes.Header{
ParentHash: common.BytesToHash(parent),
UncleHash: common.BytesToHash(sha3Uncles),
Expand Down Expand Up @@ -1257,7 +1259,7 @@ func Test_fullPayloadFromExecutionBlock(t *testing.T) {
tests := []struct {
name string
args args
want *pb.ExecutionPayload
want func() interfaces.ExecutionData
err string
}{
{
Expand All @@ -1267,7 +1269,8 @@ func Test_fullPayloadFromExecutionBlock(t *testing.T) {
BlockHash: []byte("foo"),
},
block: &pb.ExecutionBlock{
Hash: common.BytesToHash([]byte("bar")),
Version: version.Bellatrix,
Hash: common.BytesToHash([]byte("bar")),
},
},
err: "does not match execution block hash",
Expand All @@ -1279,22 +1282,30 @@ func Test_fullPayloadFromExecutionBlock(t *testing.T) {
BlockHash: wantedHash[:],
},
block: &pb.ExecutionBlock{
Hash: wantedHash,
Version: version.Bellatrix,
Hash: wantedHash,
},
},
want: &pb.ExecutionPayload{
BlockHash: wantedHash[:],
want: func() interfaces.ExecutionData {
p, err := blocks.WrappedExecutionPayload(&pb.ExecutionPayload{
BlockHash: wantedHash[:],
Transactions: [][]byte{},
})
require.NoError(t, err)
return p
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
wrapped, err := blocks.WrappedExecutionPayloadHeader(tt.args.header)
require.NoError(t, err)
got, err := fullPayloadFromExecutionBlock(wrapped, tt.args.block)
if (err != nil) && !strings.Contains(err.Error(), tt.err) {
t.Fatalf("Wanted err %s got %v", tt.err, err)
if err != nil {
assert.ErrorContains(t, tt.err, err)
} else {
assert.DeepEqual(t, tt.want(), got)
}
require.DeepEqual(t, tt.want, got)
})
}
}
Expand Down
6 changes: 4 additions & 2 deletions beacon-chain/execution/testing/mock_engine_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ func (e *EngineClient) ExecutionBlockByHash(_ context.Context, h common.Hash, _
return b, e.ErrExecBlockByHash
}

func (e *EngineClient) ReconstructFullBellatrixBlock(
// ReconstructFullBlock --
func (e *EngineClient) ReconstructFullBlock(
_ context.Context, blindedBlock interfaces.SignedBeaconBlock,
) (interfaces.SignedBeaconBlock, error) {
if !blindedBlock.Block().IsBlinded() {
Expand All @@ -94,12 +95,13 @@ func (e *EngineClient) ReconstructFullBellatrixBlock(
return blocks.BuildSignedBeaconBlockFromExecutionPayload(blindedBlock, payload)
}

// ReconstructFullBellatrixBlockBatch --
func (e *EngineClient) ReconstructFullBellatrixBlockBatch(
ctx context.Context, blindedBlocks []interfaces.SignedBeaconBlock,
) ([]interfaces.SignedBeaconBlock, error) {
fullBlocks := make([]interfaces.SignedBeaconBlock, 0, len(blindedBlocks))
for _, b := range blindedBlocks {
newBlock, err := e.ReconstructFullBellatrixBlock(ctx, b)
newBlock, err := e.ReconstructFullBlock(ctx, b)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/rpc/eth/beacon/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ func (bs *Server) GetBlockV2(ctx context.Context, req *ethpbv2.BlockRequestV2) (
if blindedBellatrixBlk == nil {
return nil, status.Error(codes.Internal, "Nil block")
}
signedFullBlock, err := bs.ExecutionPayloadReconstructor.ReconstructFullBellatrixBlock(ctx, blk)
signedFullBlock, err := bs.ExecutionPayloadReconstructor.ReconstructFullBlock(ctx, blk)
if err != nil {
return nil, status.Errorf(
codes.Internal,
Expand Down Expand Up @@ -641,7 +641,7 @@ func (bs *Server) GetBlockSSZV2(ctx context.Context, req *ethpbv2.BlockRequestV2
if blindedBellatrixBlk == nil {
return nil, status.Error(codes.Internal, "Nil block")
}
signedFullBlock, err := bs.ExecutionPayloadReconstructor.ReconstructFullBellatrixBlock(ctx, blk)
signedFullBlock, err := bs.ExecutionPayloadReconstructor.ReconstructFullBlock(ctx, blk)
if err != nil {
return nil, status.Errorf(
codes.Internal,
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/sync/rpc_beacon_blocks_by_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (s *Service) beaconBlocksRootRPCHandler(ctx context.Context, msg interface{
}

if blk.Block().IsBlinded() {
blk, err = s.cfg.executionPayloadReconstructor.ReconstructFullBellatrixBlock(ctx, blk)
blk, err = s.cfg.executionPayloadReconstructor.ReconstructFullBlock(ctx, blk)
if err != nil {
log.WithError(err).Error("Could not get reconstruct full bellatrix block from blinded body")
s.writeErrorResponseToStream(responseCodeServerError, types.ErrGeneric.Error(), stream)
Expand Down
12 changes: 6 additions & 6 deletions consensus-types/blocks/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ var (
// ErrNilObject is returned in a constructor when the underlying object is nil.
ErrNilObject = errors.New("received nil object")
// ErrNilSignedBeaconBlock is returned when a nil signed beacon block is received.
ErrNilSignedBeaconBlock = errors.New("signed beacon block can't be nil")
ErrNilSignedBeaconBlock = errors.New("signed beacon block can't be nil")
errNonBlindedSignedBeaconBlock = errors.New("can only build signed beacon block from blinded format")
)

// NewSignedBeaconBlock creates a signed beacon block from a protobuf signed beacon block.
Expand Down Expand Up @@ -177,14 +178,13 @@ func BuildSignedBeaconBlockFromExecutionPayload(
if err := BeaconBlockIsNil(blk); err != nil {
return nil, err
}
if !blk.IsBlinded() {
return nil, errNonBlindedSignedBeaconBlock
}
b := blk.Block()
payloadHeader, err := b.Body().Execution()
switch {
case errors.Is(err, ErrUnsupportedGetter):
return nil, errors.Wrap(err, "can only build signed beacon block from blinded format")
case err != nil:
if err != nil {
return nil, errors.Wrap(err, "could not get execution payload header")
default:
}

var wrappedPayload interfaces.ExecutionData
Expand Down
4 changes: 2 additions & 2 deletions consensus-types/blocks/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,14 @@ func TestBuildSignedBeaconBlockFromExecutionPayload(t *testing.T) {
_, err := BuildSignedBeaconBlockFromExecutionPayload(nil, nil)
require.ErrorIs(t, ErrNilSignedBeaconBlock, err)
})
t.Run("unsupported field payload header", func(t *testing.T) {
t.Run("not blinded payload", func(t *testing.T) {
altairBlock := &eth.SignedBeaconBlockAltair{
Block: &eth.BeaconBlockAltair{
Body: &eth.BeaconBlockBodyAltair{}}}
blk, err := NewSignedBeaconBlock(altairBlock)
require.NoError(t, err)
_, err = BuildSignedBeaconBlockFromExecutionPayload(blk, nil)
require.Equal(t, true, errors.Is(err, ErrUnsupportedGetter))
require.Equal(t, true, errors.Is(err, errNonBlindedSignedBeaconBlock))
})
t.Run("payload header root and payload root mismatch", func(t *testing.T) {
blockHash := bytesutil.Bytes32(1)
Expand Down
9 changes: 9 additions & 0 deletions encoding/bytesutil/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ func ToBytes4(x []byte) [4]byte {
return y
}

// ToBytes20 is a convenience method for converting a byte slice to a fix
// sized 20 byte array. This method will truncate the input if it is larger
// than 20 bytes.
func ToBytes20(x []byte) [20]byte {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add some unit tests for this ?

var y [20]byte
copy(y[:], x)
return y
}

// ToBytes32 is a convenience method for converting a byte slice to a fix
// sized 32 byte array. This method will truncate the input if it is larger
// than 32 bytes.
Expand Down
4 changes: 4 additions & 0 deletions proto/engine/v1/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,17 @@ go_library(
deps = [
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//consensus-types/primitives:go_default_library",
"//encoding/bytesutil:go_default_library",
"//proto/eth/ext:go_default_library",
"//runtime/version:go_default_library",
"@com_github_ethereum_go_ethereum//common:go_default_library",
"@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
"@com_github_ethereum_go_ethereum//core/types:go_default_library",
"@com_github_golang_protobuf//proto:go_default_library",
"@com_github_pkg_errors//:go_default_library",
"@com_github_prysmaticlabs_fastssz//:go_default_library",
"@com_github_sirupsen_logrus//:go_default_library",
"@go_googleapis//google/api:annotations_go_proto",
"@io_bazel_rules_go//proto/wkt:descriptor_go_proto",
"@io_bazel_rules_go//proto/wkt:timestamp_go_proto",
Expand Down Expand Up @@ -114,6 +117,7 @@ go_test(
":go_default_library",
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//consensus-types/primitives:go_default_library",
"//encoding/bytesutil:go_default_library",
"//testing/require:go_default_library",
"@com_github_ethereum_go_ethereum//common:go_default_library",
Expand Down
Loading