From 1395860630fb396ab71b80295509d94d1c3be630 Mon Sep 17 00:00:00 2001
From: Ian Shim <100327837+ian-shim@users.noreply.github.com>
Date: Mon, 19 Feb 2024 13:26:14 -0800
Subject: [PATCH] fix: Don't mark blob as confirmed if already confirmed (#262)

---
 disperser/batcher/batcher_test.go            | 10 ++++++++++
 disperser/common/blobstore/shared_storage.go | 11 +++++++++++
 disperser/common/inmem/store.go              |  9 +++++++++
 3 files changed, 30 insertions(+)

diff --git a/disperser/batcher/batcher_test.go b/disperser/batcher/batcher_test.go
index 669fc7147e..55f3f75e94 100644
--- a/disperser/batcher/batcher_test.go
+++ b/disperser/batcher/batcher_test.go
@@ -214,6 +214,16 @@ func TestBatcherIterations(t *testing.T) {
 	count, size = components.encodingStreamer.EncodedBlobstore.GetEncodedResultSize()
 	assert.Equal(t, 0, count)
 	assert.Equal(t, uint64(0), size)
+
+	// confirmed metadata should be immutable and not be updated
+	existingBlobIndex := meta1.ConfirmationInfo.BlobIndex
+	meta1, err = blobStore.MarkBlobConfirmed(ctx, meta1, &disperser.ConfirmationInfo{
+		BlobIndex: existingBlobIndex + 1,
+	})
+	assert.NoError(t, err)
+	// check confirmation info isn't updated
+	assert.Equal(t, existingBlobIndex, meta1.ConfirmationInfo.BlobIndex)
+	assert.Equal(t, disperser.Confirmed, meta1.BlobStatus)
 }
 
 func TestBlobFailures(t *testing.T) {
diff --git a/disperser/common/blobstore/shared_storage.go b/disperser/common/blobstore/shared_storage.go
index 8995b583e7..c3bf4632e2 100644
--- a/disperser/common/blobstore/shared_storage.go
+++ b/disperser/common/blobstore/shared_storage.go
@@ -134,6 +134,17 @@ func (s *SharedBlobStore) getBlobContentParallel(ctx context.Context, blobKey di
 }
 
 func (s *SharedBlobStore) MarkBlobConfirmed(ctx context.Context, existingMetadata *disperser.BlobMetadata, confirmationInfo *disperser.ConfirmationInfo) (*disperser.BlobMetadata, error) {
+	// TODO (ian-shim): remove this check once we are sure that the metadata is never overwritten
+	refreshedMetadata, err := s.GetBlobMetadata(ctx, existingMetadata.GetBlobKey())
+	if err != nil {
+		s.logger.Error("[MarkBlobConfirmed] error getting blob metadata", "err", err)
+		return nil, err
+	}
+	alreadyConfirmed, _ := refreshedMetadata.IsConfirmed()
+	if alreadyConfirmed {
+		s.logger.Warn("[MarkBlobConfirmed] trying to confirm blob already marked as confirmed", "blobKey", existingMetadata.GetBlobKey().String())
+		return refreshedMetadata, nil
+	}
 	newMetadata := *existingMetadata
 	// Update the TTL if needed
 	ttlFromNow := time.Now().Add(s.blobMetadataStore.ttl)
diff --git a/disperser/common/inmem/store.go b/disperser/common/inmem/store.go
index 1482be99ac..880f85c00b 100644
--- a/disperser/common/inmem/store.go
+++ b/disperser/common/inmem/store.go
@@ -71,6 +71,15 @@ func (q *BlobStore) GetBlobContent(ctx context.Context, blobHash disperser.BlobH
 }
 
 func (q *BlobStore) MarkBlobConfirmed(ctx context.Context, existingMetadata *disperser.BlobMetadata, confirmationInfo *disperser.ConfirmationInfo) (*disperser.BlobMetadata, error) {
+	// TODO (ian-shim): remove this check once we are sure that the metadata is never overwritten
+	refreshedMetadata, err := q.GetBlobMetadata(ctx, existingMetadata.GetBlobKey())
+	if err != nil {
+		return nil, err
+	}
+	alreadyConfirmed, _ := refreshedMetadata.IsConfirmed()
+	if alreadyConfirmed {
+		return refreshedMetadata, nil
+	}
 	blobKey := existingMetadata.GetBlobKey()
 	if _, ok := q.Metadata[blobKey]; !ok {
 		return nil, disperser.ErrBlobNotFound