Verify blobs that arrived from by root request (#13044)

* Verify blobs that arrived from by root request

* Add tests

* Add tests

* Fix test

* Fix lint

---------

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

beacon-chain/sync/BUILD.bazel

@@ -88,6 +88,7 @@ go_library(
         "//beacon-chain/startup:go_default_library",
         "//beacon-chain/state:go_default_library",
         "//beacon-chain/state/stategen:go_default_library",
+        "//beacon-chain/sync/verify:go_default_library",
         "//cache/lru:go_default_library",
         "//cmd/beacon-chain/flags:go_default_library",
         "//config/features:go_default_library",

beacon-chain/sync/initial-sync/BUILD.bazel

@@ -27,6 +27,7 @@ go_library(
         "//beacon-chain/p2p/types:go_default_library",
         "//beacon-chain/startup:go_default_library",
         "//beacon-chain/sync:go_default_library",
+        "//beacon-chain/sync/verify:go_default_library",
         "//cmd/beacon-chain/flags:go_default_library",
         "//config/params:go_default_library",
         "//consensus-types:go_default_library",
@@ -35,7 +36,6 @@ go_library(
         "//consensus-types/primitives:go_default_library",
         "//container/leaky-bucket:go_default_library",
         "//crypto/rand:go_default_library",
-        "//encoding/bytesutil:go_default_library",
         "//math:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
         "//runtime:go_default_library",
@@ -121,6 +121,7 @@ go_test(
         "//beacon-chain/p2p/types:go_default_library",
         "//beacon-chain/startup:go_default_library",
         "//beacon-chain/sync:go_default_library",
+        "//beacon-chain/sync/verify:go_default_library",
         "//cmd/beacon-chain/flags:go_default_library",
         "//config/features:go_default_library",
         "//config/params:go_default_library",

beacon-chain/sync/initial-sync/blocks_fetcher.go

@@ -15,6 +15,7 @@ import (
 	p2pTypes "github.com/prysmaticlabs/prysm/v4/beacon-chain/p2p/types"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/startup"
 	prysmsync "github.com/prysmaticlabs/prysm/v4/beacon-chain/sync"
+	"github.com/prysmaticlabs/prysm/v4/beacon-chain/sync/verify"
 	"github.com/prysmaticlabs/prysm/v4/cmd/beacon-chain/flags"
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	consensus_types "github.com/prysmaticlabs/prysm/v4/consensus-types"
@@ -23,7 +24,6 @@ import (
 	"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
 	leakybucket "github.com/prysmaticlabs/prysm/v4/container/leaky-bucket"
 	"github.com/prysmaticlabs/prysm/v4/crypto/rand"
-	"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/v4/math"
 	p2ppb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/v4/time/slots"
@@ -395,10 +395,6 @@ func sortBlobs(blobs []*p2ppb.BlobSidecar) []*p2ppb.BlobSidecar {
 
 var errBlobVerification = errors.New("peer unable to serve aligned BlobSidecarsByRange and BeaconBlockSidecarsByRange responses")
 var errMissingBlobsForBlockCommitments = errors.Wrap(errBlobVerification, "blobs unavailable for processing block with kzg commitments")
-var errMismatchedBlobBlockRoot = errors.Wrap(errBlobVerification, "BlockRoot in BlobSidecar does not match the expected root")
-var errMissingBlobIndex = errors.Wrap(errBlobVerification, "missing expected blob index")
-var errMismatchedBlobCommitments = errors.Wrap(errBlobVerification, "commitments at given slot, root and index do not match")
-var errMismatchedProposerIndex = errors.Wrap(errBlobVerification, "proposer index does not match")
 
 func verifyAndPopulateBlobs(bwb []blocks2.BlockWithVerifiedBlobs, blobs []*p2ppb.BlobSidecar, blobWindowStart primitives.Slot) ([]blocks2.BlockWithVerifiedBlobs, error) {
 	// Assumes bwb has already been sorted by sortedBlockWithVerifiedBlobSlice.
@@ -431,31 +427,8 @@ func verifyAndPopulateBlobs(bwb []blocks2.BlockWithVerifiedBlobs, blobs []*p2ppb
 				return nil, missingCommitError(bb.Block.Root(), bb.Block.Block().Slot(), commits[ci:])
 			}
 			bl := blobs[blobi]
-			if bl.Slot != block.Slot() {
+			if err := verify.BlobAlignsWithBlock(bl, bb.Block); err != nil {
-				return nil, missingCommitError(bb.Block.Root(), bb.Block.Block().Slot(), commits[ci:])
+				return nil, err
-			}
-			if bytesutil.ToBytes32(bl.BlockRoot) != bb.Block.Root() {
-				return nil, errors.Wrapf(errMismatchedBlobBlockRoot,
-					"block root %#x != BlobSidecar.BlockRoot %#x at slot %d", bb.Block.Root(), bl.BlockRoot, block.Slot())
-			}
-			if bytesutil.ToBytes32(bl.BlockParentRoot) != block.ParentRoot() {
-				return nil, errors.Wrapf(errMismatchedBlobBlockRoot,
-					"block parent root %#x != BlobSidecar.BlockParentRoot %#x at slot %d", block.ParentRoot(), bl.BlockParentRoot, block.Slot())
-			}
-			if bl.ProposerIndex != block.ProposerIndex() {
-				return nil, errors.Wrapf(errMismatchedProposerIndex,
-					"block proposer index %d != BlobSidecar.ProposerIndex %d at slot %d", block.ProposerIndex(), bl.ProposerIndex, block.Slot())
-			}
-			if ci != int(bl.Index) {
-				return nil, errors.Wrapf(errMissingBlobIndex,
-					"did not receive blob index %d for block root %#x at slot %d", ci, bb.Block.Root(), block.Slot())
-			}
-			ec := bytesutil.ToBytes48(commits[ci])
-			ac := bytesutil.ToBytes48(bl.KzgCommitment)
-			if ec != ac {
-				return nil, errors.Wrapf(errMismatchedBlobCommitments,
-					"commitment %#x != block commitment %#x, at index %d for block root %#x at slot %d ",
-					ac, ec, bl.Index, bb.Block.Root(), block.Slot())
 			}
 			bb.Blobs[ci] = bl
 			blobi += 1

beacon-chain/sync/initial-sync/blocks_fetcher_test.go

@@ -17,6 +17,7 @@ import (
 	p2pt "github.com/prysmaticlabs/prysm/v4/beacon-chain/p2p/testing"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/startup"
 	beaconsync "github.com/prysmaticlabs/prysm/v4/beacon-chain/sync"
+	"github.com/prysmaticlabs/prysm/v4/beacon-chain/sync/verify"
 	"github.com/prysmaticlabs/prysm/v4/cmd/beacon-chain/flags"
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	"github.com/prysmaticlabs/prysm/v4/consensus-types/blocks"
@@ -1079,33 +1080,33 @@ func TestVerifyAndPopulateBlobs(t *testing.T) {
 		}
 	}
 	_, err = verifyAndPopulateBlobs(bwb, blobs, firstBlockSlot)
-	require.ErrorIs(t, err, errMissingBlobsForBlockCommitments)
+	require.ErrorContains(t, "BlockSlot in BlobSidecar does not match the expected slot", err)
 
 	bwb, blobs = testSequenceBlockWithBlob(t, 10)
 	blobs[lastBlobIdx].BlockRoot = blobs[0].BlockRoot
 	_, err = verifyAndPopulateBlobs(bwb, blobs, firstBlockSlot)
-	require.ErrorIs(t, err, errMismatchedBlobBlockRoot)
+	require.ErrorIs(t, err, verify.ErrMismatchedBlobBlockRoot)
 
 	bwb, blobs = testSequenceBlockWithBlob(t, 10)
 	blobs[lastBlobIdx].Index = 100
 	_, err = verifyAndPopulateBlobs(bwb, blobs, firstBlockSlot)
-	require.ErrorIs(t, err, errMissingBlobIndex)
+	require.ErrorIs(t, err, verify.ErrIncorrectBlobIndex)
 
 	bwb, blobs = testSequenceBlockWithBlob(t, 10)
 	blobs[lastBlobIdx].ProposerIndex = 100
 	_, err = verifyAndPopulateBlobs(bwb, blobs, firstBlockSlot)
-	require.ErrorIs(t, err, errMismatchedProposerIndex)
+	require.ErrorIs(t, err, verify.ErrMismatchedProposerIndex)
 
 	bwb, blobs = testSequenceBlockWithBlob(t, 10)
 	blobs[lastBlobIdx].BlockParentRoot = blobs[0].BlockParentRoot
 	_, err = verifyAndPopulateBlobs(bwb, blobs, firstBlockSlot)
-	require.ErrorIs(t, err, errMismatchedBlobBlockRoot)
+	require.ErrorIs(t, err, verify.ErrMismatchedBlobBlockRoot)
 
 	var emptyKzg [48]byte
 	bwb, blobs = testSequenceBlockWithBlob(t, 10)
 	blobs[lastBlobIdx].KzgCommitment = emptyKzg[:]
 	_, err = verifyAndPopulateBlobs(bwb, blobs, firstBlockSlot)
-	require.ErrorIs(t, err, errMismatchedBlobCommitments)
+	require.ErrorIs(t, err, verify.ErrMismatchedBlobCommitments)
 
 	// happy path
 	bwb, blobs = testSequenceBlockWithBlob(t, 10)

beacon-chain/sync/rpc_beacon_blocks_by_root.go

@@ -9,9 +9,11 @@ import (
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/db"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/execution"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/p2p/types"
+	"github.com/prysmaticlabs/prysm/v4/beacon-chain/sync/verify"
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	"github.com/prysmaticlabs/prysm/v4/consensus-types/blocks"
 	"github.com/prysmaticlabs/prysm/v4/consensus-types/interfaces"
+	"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
 	eth "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/v4/runtime/version"
 )
@@ -154,7 +156,18 @@ func (s *Service) sendAndSaveBlobSidecars(ctx context.Context, request types.Blo
 		return err
 	}
 
+	block, err := s.cfg.beaconDB.Block(ctx, bytesutil.ToBytes32(request[0].BlockRoot))
+	if err != nil {
+		return err
+	}
+	RoBlock, err := blocks.NewROBlock(block)
+	if err != nil {
+		return err
+	}
 	for _, sidecar := range sidecars {
+		if err := verify.BlobAlignsWithBlock(sidecar, RoBlock); err != nil {
+			return err
+		}
 		log.WithFields(blobFields(sidecar)).Debug("Received blob sidecar gossip RPC")
 	}
 

beacon-chain/sync/verify/BUILD.bazel

@@ -0,0 +1,30 @@
+load("@prysm//tools/go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["blob.go"],
+    importpath = "github.com/prysmaticlabs/prysm/v4/beacon-chain/sync/verify",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//config/fieldparams:go_default_library",
+        "//consensus-types/blocks:go_default_library",
+        "//encoding/bytesutil:go_default_library",
+        "//proto/prysm/v1alpha1:go_default_library",
+        "//runtime/version:go_default_library",
+        "@com_github_pkg_errors//:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["blob_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//config/fieldparams:go_default_library",
+        "//consensus-types/blocks:go_default_library",
+        "//consensus-types/interfaces:go_default_library",
+        "//proto/prysm/v1alpha1:go_default_library",
+        "//testing/require:go_default_library",
+        "//testing/util:go_default_library",
+    ],
+)

beacon-chain/sync/verify/blob.go

@@ -0,0 +1,71 @@
+package verify
+
+import (
+	"github.com/pkg/errors"
+	field_params "github.com/prysmaticlabs/prysm/v4/config/fieldparams"
+	"github.com/prysmaticlabs/prysm/v4/consensus-types/blocks"
+	"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
+	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
+	"github.com/prysmaticlabs/prysm/v4/runtime/version"
+)
+
+var (
+	errBlobVerification          = errors.New("unable to verify blobs")
+	ErrMismatchedBlobBlockRoot   = errors.Wrap(errBlobVerification, "BlockRoot in BlobSidecar does not match the expected root")
+	ErrMismatchedBlobBlockSlot   = errors.Wrap(errBlobVerification, "BlockSlot in BlobSidecar does not match the expected slot")
+	ErrMismatchedBlobCommitments = errors.Wrap(errBlobVerification, "commitments at given slot, root and index do not match")
+	ErrMismatchedProposerIndex   = errors.Wrap(errBlobVerification, "proposer index does not match")
+	ErrIncorrectBlobIndex        = errors.Wrap(errBlobVerification, "incorrect blob index")
+)
+
+// BlobAlignsWithBlock verifies if the blob aligns with the block.
+func BlobAlignsWithBlock(blob *ethpb.BlobSidecar, block blocks.ROBlock) error {
+	if block.Version() < version.Deneb {
+		return nil
+	}
+
+	commits, err := block.Block().Body().BlobKzgCommitments()
+	if err != nil {
+		return err
+	}
+
+	if len(commits) == 0 {
+		return nil
+	}
+
+	if blob.Index >= field_params.MaxBlobsPerBlock {
+		return errors.Wrapf(ErrIncorrectBlobIndex, "blob index %d >= max blobs per block %d", blob.Index, field_params.MaxBlobsPerBlock)
+	}
+
+	// Verify slot
+	blobSlot := blob.Slot
+	blockSlot := block.Block().Slot()
+	if blobSlot != blockSlot {
+		return errors.Wrapf(ErrMismatchedBlobBlockSlot, "slot %d != BlobSidecar.Slot %d", blockSlot, blobSlot)
+	}
+
+	// Verify block and parent roots
+	blockRoot := bytesutil.ToBytes32(blob.BlockRoot)
+	if blockRoot != block.Root() {
+		return errors.Wrapf(ErrMismatchedBlobBlockRoot, "block root %#x != BlobSidecar.BlockRoot %#x at slot %d", block.Root(), blockRoot, blobSlot)
+	}
+
+	blockParentRoot := bytesutil.ToBytes32(blob.BlockParentRoot)
+	if blockParentRoot != block.Block().ParentRoot() {
+		return errors.Wrapf(ErrMismatchedBlobBlockRoot, "block parent root %#x != BlobSidecar.BlockParentRoot %#x at slot %d", block.Block().ParentRoot(), blockParentRoot, blobSlot)
+	}
+
+	// Verify proposer index
+	if blob.ProposerIndex != block.Block().ProposerIndex() {
+		return errors.Wrapf(ErrMismatchedProposerIndex, "proposer index %d != BlobSidecar.ProposerIndex %d at slot %d", block.Block().ProposerIndex(), blob.ProposerIndex, blobSlot)
+	}
+
+	// Verify commitment
+	blockCommitment := bytesutil.ToBytes48(commits[blob.Index])
+	blobCommitment := bytesutil.ToBytes48(blob.KzgCommitment)
+	if blobCommitment != blockCommitment {
+		return errors.Wrapf(ErrMismatchedBlobCommitments, "commitment %#x != block commitment %#x, at index %d for block root %#x at slot %d ", blobCommitment, blockCommitment, blob.Index, blockRoot, blobSlot)
+	}
+
+	return nil
+}

beacon-chain/sync/verify/blob_test.go

@@ -0,0 +1,142 @@
+package verify
+
+import (
+	"testing"
+
+	fieldparams "github.com/prysmaticlabs/prysm/v4/config/fieldparams"
+	"github.com/prysmaticlabs/prysm/v4/consensus-types/blocks"
+	"github.com/prysmaticlabs/prysm/v4/consensus-types/interfaces"
+	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
+	"github.com/prysmaticlabs/prysm/v4/testing/require"
+	"github.com/prysmaticlabs/prysm/v4/testing/util"
+)
+
+func TestBlobAlignsWithBlock(t *testing.T) {
+	tests := []struct {
+		name        string
+		block       interfaces.ReadOnlySignedBeaconBlock
+		blob        *ethpb.BlobSidecar
+		expectedErr string
+	}{
+		{
+			name: "Block version less than Deneb",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlock()
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob: &ethpb.BlobSidecar{},
+		},
+		{
+			name: "No commitments in block",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob: &ethpb.BlobSidecar{},
+		},
+		{
+			name: "Blob index exceeds max blobs per block",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Body.BlobKzgCommitments = make([][]byte, fieldparams.MaxBlobsPerBlock+1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob:        &ethpb.BlobSidecar{Index: fieldparams.MaxBlobsPerBlock},
+			expectedErr: "blob index 6 >= max blobs per block 6: incorrect blob index",
+		},
+		{
+			name: "Blob slot does not match block slot",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Slot = 2
+				b.Block.Body.BlobKzgCommitments = make([][]byte, 1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob:        &ethpb.BlobSidecar{Slot: 1},
+			expectedErr: "slot 2 != BlobSidecar.Slot 1: BlockSlot in BlobSidecar does not match the expected slot",
+		},
+		{
+			name: "Blob block root does not match block root",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Body.BlobKzgCommitments = make([][]byte, 1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob: &ethpb.BlobSidecar{BlockRoot: []byte{1}},
+			expectedErr: "block root 0x0200000000000000000000000000000000000000000000000000000000000000 != " +
+				"BlobSidecar.BlockRoot 0x0100000000000000000000000000000000000000000000000000000000000000 at slot 0",
+		},
+		{
+			name: "Blob parent root does not match block parent root",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Body.BlobKzgCommitments = make([][]byte, 1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob: &ethpb.BlobSidecar{BlockRoot: []byte{2}, BlockParentRoot: []byte{1}},
+			expectedErr: "block parent root 0x0000000000000000000000000000000000000000000000000000000000000000 != " +
+				"BlobSidecar.BlockParentRoot 0x0100000000000000000000000000000000000000000000000000000000000000 at slot 0",
+		},
+		{
+			name: "Blob proposer index does not match block proposer index",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Body.BlobKzgCommitments = make([][]byte, 1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob:        &ethpb.BlobSidecar{BlockRoot: []byte{2}, ProposerIndex: 1},
+			expectedErr: "proposer index 0 != BlobSidecar.ProposerIndex 1 at slot ",
+		},
+		{
+			name: "Blob commitment does not match block commitment",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Body.BlobKzgCommitments = make([][]byte, 1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob: &ethpb.BlobSidecar{BlockRoot: []byte{2}, KzgCommitment: []byte{1}},
+			expectedErr: "commitment 0x010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 != " +
+				"block commitment 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+		},
+		{
+			name: "All fields are correctly aligned",
+			block: func() interfaces.ReadOnlySignedBeaconBlock {
+				b := util.NewBeaconBlockDeneb()
+				b.Block.Body.BlobKzgCommitments = make([][]byte, 1)
+				sb, err := blocks.NewSignedBeaconBlock(b)
+				require.NoError(t, err)
+				return sb
+			}(),
+			blob: &ethpb.BlobSidecar{BlockRoot: []byte{2}},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			block, err := blocks.NewROBlockWithRoot(tt.block, [32]byte{2})
+			require.NoError(t, err)
+			err = BlobAlignsWithBlock(tt.blob, block)
+			if tt.expectedErr == "" {
+				require.NoError(t, err)
+			} else {
+				require.StringContains(t, tt.expectedErr, err.Error())
+			}
+		})
+	}
+}