From 9cecb68520aeb24bc90c79177ddf3c5e6a1b3806 Mon Sep 17 00:00:00 2001
From: Eric <5089238+emizzle@users.noreply.github.com>
Date: Mon, 21 Aug 2023 12:51:04 +1000
Subject: [PATCH] [repostore] Retrieve empty blocks (#513)

Add handling of empty blocks in the RepoStore.

* Add empty block handling to repostore for put, del, has
Also added tests for all empty block handling blockstore operations. This showed there was an ambiguous identifier present for `hasBlock`, so one of the two `hasBlock` definitions was removed in `repostore`.

* Change CacheStore to RepoStore in testerasure
As CacheStore is not used in the node, update the Datastore used in the erasure coding tests to be a RepoStore. This ensures that the K > 1 cases are being tested, where they will produce empty padding blocks in the erasure-coded manifests.
---
 codex/stores/repostore.nim           | 27 ++++++++++++++++-----------
 tests/codex/stores/testrepostore.nim | 23 +++++++++++++++++++++++
 tests/codex/testerasure.nim          |  7 ++++++-
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/codex/stores/repostore.nim b/codex/stores/repostore.nim
index 75400296..9bf3b010 100644
--- a/codex/stores/repostore.nim
+++ b/codex/stores/repostore.nim
@@ -85,6 +85,10 @@ method getBlock*(self: RepoStore, cid: Cid): Future[?!Block] {.async.} =
   ## Get a block from the blockstore
   ##
 
+  if cid.isEmpty:
+    trace "Empty block, ignoring"
+    return success cid.emptyBlock
+
   without key =? makePrefixKey(self.postFixLen, cid), err:
     trace "Error getting key from provider", err = err.msg
     return failure(err)
@@ -132,6 +136,10 @@ method putBlock*(
   ## Put a block to the blockstore
   ##
 
+  if blk.isEmpty:
+    trace "Empty block, ignoring"
+    return success()
+
   without key =? makePrefixKey(self.postFixLen, blk.cid), err:
     trace "Error getting key from provider", err = err.msg
     return failure(err)
@@ -205,6 +213,10 @@ method delBlock*(self: RepoStore, cid: Cid): Future[?!void] {.async.} =
 
   trace "Deleting block", cid
 
+  if cid.isEmpty:
+    trace "Empty block, ignoring"
+    return success()
+
   if blk =? (await self.getBlock(cid)):
     if key =? makePrefixKey(self.postFixLen, cid) and
       err =? (await self.repoDs.delete(key)).errorOption:
@@ -233,6 +245,10 @@ method hasBlock*(self: RepoStore, cid: Cid): Future[?!bool] {.async.} =
   ## Check if the block exists in the blockstore
   ##
 
+  if cid.isEmpty:
+    trace "Empty block, ignoring"
+    return true.success
+
   without key =? makePrefixKey(self.postFixLen, cid), err:
     trace "Error getting key from provider", err = err.msg
     return failure(err)
@@ -320,17 +336,6 @@ method close*(self: RepoStore): Future[void] {.async.} =
 
   (await self.repoDs.close()).expect("Should close datastore")
 
-proc hasBlock*(self: RepoStore, cid: Cid): Future[?!bool] {.async.} =
-  ## Check if the block exists in the blockstore.
-  ## Return false if error encountered
-  ##
-
-  without key =? makePrefixKey(self.postFixLen, cid), err:
-    trace "Error getting key from provider", err = err.msg
-    return failure(err.msg)
-
-  return await self.repoDs.has(key)
-
 proc reserve*(self: RepoStore, bytes: uint): Future[?!void] {.async.} =
   ## Reserve bytes
   ##
diff --git a/tests/codex/stores/testrepostore.nim b/tests/codex/stores/testrepostore.nim
index 523f65f0..3124c650 100644
--- a/tests/codex/stores/testrepostore.nim
+++ b/tests/codex/stores/testrepostore.nim
@@ -19,6 +19,7 @@ import pkg/codex/clock
 
 import ../helpers
 import ../helpers/mockclock
+import ../examples
 import ./commonstoretests
 
 checksuite "Test RepoStore start/stop":
@@ -283,6 +284,28 @@ asyncchecksuite "RepoStore":
     check blockExpirations2.len == 1
     assertExpiration(blockExpirations2[0], blk3)
 
+  test "should put empty blocks":
+    let blk = Cid.example.emptyBlock
+    check (await repo.putBlock(blk)).isOk
+
+  test "should get empty blocks":
+    let blk = Cid.example.emptyBlock
+
+    let got = await repo.getBlock(blk.cid)
+    check got.isOk
+    check got.get.cid == blk.cid
+
+  test "should delete empty blocks":
+    let blk = Cid.example.emptyBlock
+    check (await repo.delBlock(blk.cid)).isOk
+
+  test "should have empty block":
+    let blk = Cid.example.emptyBlock
+
+    let has = await repo.hasBlock(blk.cid)
+    check has.isOk
+    check has.get
+
 commonBlockStoreTests(
   "RepoStore Sql backend", proc: BlockStore =
     BlockStore(
diff --git a/tests/codex/testerasure.nim b/tests/codex/testerasure.nim
index 94f7c0e8..6f8a9612 100644
--- a/tests/codex/testerasure.nim
+++ b/tests/codex/testerasure.nim
@@ -2,6 +2,7 @@ import std/sequtils
 
 import pkg/asynctest
 import pkg/chronos
+import pkg/datastore
 import pkg/questionable/results
 
 import pkg/codex/erasure
@@ -21,12 +22,16 @@ asyncchecksuite "Erasure encode/decode":
   var manifest: Manifest
   var store: BlockStore
   var erasure: Erasure
+  var repoDs: Datastore
+  var metaDs: SQLiteDatastore
 
   setup:
     rng = Rng.instance()
     chunker = RandomChunker.new(rng, size = dataSetSize, chunkSize = BlockSize)
     manifest = !Manifest.new(blockSize = BlockSize)
-    store = CacheStore.new(cacheSize = (dataSetSize * 2), chunkSize = BlockSize)
+    repoDs = SQLiteDatastore.new(Memory).tryGet()
+    metaDs = SQLiteDatastore.new(Memory).tryGet()
+    store = RepoStore.new(repoDs, metaDs)
     erasure = Erasure.new(store, leoEncoderProvider, leoDecoderProvider)
 
     while (