From 4b43f13e6534d963ff711ad7e4f2a2c5fe646be7 Mon Sep 17 00:00:00 2001
From: Nishant Das <nishdas93@gmail.com>
Date: Tue, 11 Feb 2025 21:44:00 +0800
Subject: [PATCH] Fix Blob Reconstruction (#14909)

* Fix Mutating Blob Mask

* Changelog

* Typo
---
 beacon-chain/execution/engine_client.go      | 28 +++++++++-----------
 beacon-chain/execution/engine_client_test.go | 25 ++++++++++++++++-
 changelog/nisdas_fix_mutating_blob_mask.md   |  4 +++
 3 files changed, 40 insertions(+), 17 deletions(-)
 create mode 100644 changelog/nisdas_fix_mutating_blob_mask.md

diff --git a/beacon-chain/execution/engine_client.go b/beacon-chain/execution/engine_client.go
index e955d10644..ae8baff7eb 100644
--- a/beacon-chain/execution/engine_client.go
+++ b/beacon-chain/execution/engine_client.go
@@ -543,9 +543,11 @@ func (s *Service) ReconstructBlobSidecars(ctx context.Context, block interfaces.
 
 	// Collect KZG hashes for non-existing blobs
 	var kzgHashes []common.Hash
+	var kzgIndexes []int
 	for i, commitment := range kzgCommitments {
 		if !hasIndex(uint64(i)) {
 			kzgHashes = append(kzgHashes, primitives.ConvertKzgCommitmentToVersionedHash(commitment))
+			kzgIndexes = append(kzgIndexes, i)
 		}
 	}
 	if len(kzgHashes) == 0 {
@@ -568,27 +570,21 @@ func (s *Service) ReconstructBlobSidecars(ctx context.Context, block interfaces.
 
 	// Reconstruct verified blob sidecars
 	var verifiedBlobs []blocks.VerifiedROBlob
-	for i, blobIndex := 0, 0; i < len(kzgCommitments); i++ {
-		if hasIndex(uint64(i)) {
+	for i := 0; i < len(kzgHashes); i++ {
+		if blobs[i] == nil {
 			continue
 		}
-
-		if blobIndex >= len(blobs) || blobs[blobIndex] == nil {
-			blobIndex++
-			continue
-		}
-		blob := blobs[blobIndex]
-		blobIndex++
-
-		proof, err := blocks.MerkleProofKZGCommitment(blockBody, i)
+		blob := blobs[i]
+		blobIndex := kzgIndexes[i]
+		proof, err := blocks.MerkleProofKZGCommitment(blockBody, blobIndex)
 		if err != nil {
-			log.WithError(err).WithField("index", i).Error("failed to get Merkle proof for KZG commitment")
+			log.WithError(err).WithField("index", blobIndex).Error("failed to get Merkle proof for KZG commitment")
 			continue
 		}
 		sidecar := &ethpb.BlobSidecar{
-			Index:                    uint64(i),
+			Index:                    uint64(blobIndex),
 			Blob:                     blob.Blob,
-			KzgCommitment:            kzgCommitments[i],
+			KzgCommitment:            kzgCommitments[blobIndex],
 			KzgProof:                 blob.KzgProof,
 			SignedBlockHeader:        header,
 			CommitmentInclusionProof: proof,
@@ -596,14 +592,14 @@ func (s *Service) ReconstructBlobSidecars(ctx context.Context, block interfaces.
 
 		roBlob, err := blocks.NewROBlobWithRoot(sidecar, blockRoot)
 		if err != nil {
-			log.WithError(err).WithField("index", i).Error("failed to create RO blob with root")
+			log.WithError(err).WithField("index", blobIndex).Error("failed to create RO blob with root")
 			continue
 		}
 
 		v := s.blobVerifier(roBlob, verification.ELMemPoolRequirements)
 		verifiedBlob, err := v.VerifiedROBlob()
 		if err != nil {
-			log.WithError(err).WithField("index", i).Error("failed to verify RO blob")
+			log.WithError(err).WithField("index", blobIndex).Error("failed to verify RO blob")
 			continue
 		}
 
diff --git a/beacon-chain/execution/engine_client_test.go b/beacon-chain/execution/engine_client_test.go
index 3ac4ba4448..903c99bb22 100644
--- a/beacon-chain/execution/engine_client_test.go
+++ b/beacon-chain/execution/engine_client_test.go
@@ -2455,6 +2455,25 @@ func TestReconstructBlobSidecars(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, 3, len(verifiedBlobs))
 	})
+
+	t.Run("recovered 3 missing blobs with mutated blob mask", func(t *testing.T) {
+		exists := []bool{true, false, true, false, true, false}
+		hi := mockSummary(t, exists)
+
+		srv := createBlobServer(t, 3, func() {
+			// Mutate blob mask
+			exists[1] = true
+			exists[3] = true
+		})
+		defer srv.Close()
+
+		rpcClient, client := setupRpcClient(t, srv.URL, client)
+		defer rpcClient.Close()
+
+		verifiedBlobs, err := client.ReconstructBlobSidecars(ctx, sb, r, hi)
+		require.NoError(t, err)
+		require.Equal(t, 3, len(verifiedBlobs))
+	})
 }
 
 func createRandomKzgCommitments(t *testing.T, num int) [][]byte {
@@ -2467,12 +2486,16 @@ func createRandomKzgCommitments(t *testing.T, num int) [][]byte {
 	return kzgCommitments
 }
 
-func createBlobServer(t *testing.T, numBlobs int) *httptest.Server {
+func createBlobServer(t *testing.T, numBlobs int, callbackFuncs ...func()) *httptest.Server {
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		defer func() {
 			require.NoError(t, r.Body.Close())
 		}()
+		// Execute callback functions for each request.
+		for _, f := range callbackFuncs {
+			f()
+		}
 
 		blobs := make([]pb.BlobAndProofJson, numBlobs)
 		for i := range blobs {
diff --git a/changelog/nisdas_fix_mutating_blob_mask.md b/changelog/nisdas_fix_mutating_blob_mask.md
new file mode 100644
index 0000000000..bb9e13e15d
--- /dev/null
+++ b/changelog/nisdas_fix_mutating_blob_mask.md
@@ -0,0 +1,4 @@
+### Fixed
+
+- We change how we track blob indexes during their reconstruction from the EL to prevent
+a mutating blob mask from causing invalid sidecars from being created.
\ No newline at end of file