Makes rootHash private to Manifest object. (#488)

* Makes rootHash private to Manifest object.

* Locks down all fields of Manifest.

* Review comments by Mark

codex/manifest/coders.nim

@@ -26,7 +26,7 @@ import ../errors
 import ../blocktype
 import ./types
 
-func encode*(_: DagPBCoder, manifest: Manifest): ?!seq[byte] =
+proc encode*(_: DagPBCoder, manifest: Manifest): ?!seq[byte] =
   ## Encode the manifest into a ``ManifestCodec``
   ## multicodec container (Dag-pb) for now
   ##
@@ -60,7 +60,7 @@ func encode*(_: DagPBCoder, manifest: Manifest): ?!seq[byte] =
   # ```
   #
 
-  let cid = !manifest.rootHash
+  let cid = ? manifest.cid
   var header = initProtoBuffer()
   header.write(1, cid.data.buffer)
   header.write(2, manifest.blockSize.uint32)
@@ -145,22 +145,31 @@ func decode*(_: DagPBCoder, data: openArray[byte]): ?!Manifest =
   if blocksLen.int != blocks.len:
     return failure("Total blocks and length of blocks in header don't match!")
 
-  var
+  let
-    self = Manifest(
+    self = if pbErasureInfo.buffer.len > 0:
-      rootHash: rootHashCid.some,
+      Manifest.new(
-      originalBytes: originalBytes.NBytes,
+        rootHash = rootHashCid,
-      blockSize: blockSize.NBytes,
+        originalBytes = originalBytes.NBytes,
-      blocks: blocks,
+        blockSize = blockSize.NBytes,
-      hcodec: (? rootHashCid.mhash.mapFailure).mcodec,
+        blocks = blocks,
-      codec: rootHashCid.mcodec,
+        version = rootHashCid.cidver,
-      version: rootHashCid.cidver,
+        hcodec = (? rootHashCid.mhash.mapFailure).mcodec,
-      protected: pbErasureInfo.buffer.len > 0)
+        codec = rootHashCid.mcodec,
-
+        ecK = ecK.int,
-  if self.protected:
+        ecM = ecM.int,
-    self.ecK = ecK.int
+        originalCid = ? Cid.init(originalCid).mapFailure,
-    self.ecM = ecM.int
+        originalLen = originalLen.int
-    self.originalCid = ? Cid.init(originalCid).mapFailure
+      )
-    self.originalLen = originalLen.int
+      else:
+        Manifest.new(
+          rootHash = rootHashCid,
+          originalBytes = originalBytes.NBytes,
+          blockSize = blockSize.NBytes,
+          blocks = blocks,
+          version = rootHashCid.cidver,
+          hcodec = (? rootHashCid.mhash.mapFailure).mcodec,
+          codec = rootHashCid.mcodec
+        )
 
   ? self.verify()
   self.success
@@ -172,9 +181,6 @@ proc encode*(
   ## Encode a manifest using `encoder`
   ##
 
-  if self.rootHash.isNone:
-    ? self.makeRoot()
-
   encoder.encode(self)
 
 func decode*(

codex/manifest/manifest.nim

@@ -27,6 +27,58 @@ import ./types
 
 export types
 
+type
+  Manifest* = ref object of RootObj
+    rootHash: ?Cid          # Root (tree) hash of the contained data set
+    originalBytes*: NBytes  # Exact size of the original (uploaded) file
+    blockSize: NBytes      # Size of each contained block (might not be needed if blocks are len-prefixed)
+    blocks: seq[Cid]       # Block Cid
+    version: CidVersion    # Cid version
+    hcodec: MultiCodec     # Multihash codec
+    codec: MultiCodec      # Data set codec
+    case protected: bool   # Protected datasets have erasure coded info
+    of true:
+      ecK: int               # Number of blocks to encode
+      ecM: int               # Number of resulting parity blocks
+      originalCid: Cid     # The original Cid of the dataset being erasure coded
+      originalLen: int     # The length of the original manifest
+    else:
+      discard
+
+############################################################
+# Accessors
+############################################################
+
+proc blockSize*(self: Manifest): NBytes =
+  self.blockSize
+
+proc blocks*(self: Manifest): seq[Cid] =
+  self.blocks
+
+proc version*(self: Manifest): CidVersion =
+  self.version
+
+proc hcodec*(self: Manifest): MultiCodec =
+  self.hcodec
+
+proc codec*(self: Manifest): MultiCodec =
+  self.codec
+
+proc protected*(self: Manifest): bool =
+  self.protected
+
+proc ecK*(self: Manifest): int =
+  self.ecK
+
+proc ecM*(self: Manifest): int =
+  self.ecM
+
+proc originalCid*(self: Manifest): Cid =
+  self.originalCid
+
+proc originalLen*(self: Manifest): int =
+  self.originalLen
+
 ############################################################
 # Operations on block list
 ############################################################
@@ -231,5 +283,55 @@ proc new*(
   decoder = ManifestContainers[$DagPBCodec]
 ): ?!Manifest =
   ## Create a manifest instance from given data
-  ## 
+  ##
   Manifest.decode(data, decoder)
+
+proc new*(
+  T: type Manifest,
+  rootHash: Cid,
+  originalBytes: NBytes,
+  blockSize: NBytes,
+  blocks: seq[Cid],
+  version: CidVersion,
+  hcodec: MultiCodec,
+  codec: MultiCodec,
+  ecK: int,
+  ecM: int,
+  originalCid: Cid,
+  originalLen: int
+): Manifest =
+  Manifest(
+    rootHash: rootHash.some,
+    originalBytes: originalBytes,
+    blockSize: blockSize,
+    blocks: blocks,
+    version: version,
+    hcodec: hcodec,
+    codec: codec,
+    protected: true,
+    ecK: ecK,
+    ecM: ecM,
+    originalCid: originalCid,
+    originalLen: originalLen
+  )
+
+proc new*(
+  T: type Manifest,
+  rootHash: Cid,
+  originalBytes: NBytes,
+  blockSize: NBytes,
+  blocks: seq[Cid],
+  version: CidVersion,
+  hcodec: MultiCodec,
+  codec: MultiCodec
+): Manifest =
+  Manifest(
+    rootHash: rootHash.some,
+    originalBytes: originalBytes,
+    blockSize: blockSize,
+    blocks: blocks,
+    version: version,
+    hcodec: hcodec,
+    codec: codec,
+    protected: false,
+  )

codex/manifest/types.nim

@@ -28,21 +28,3 @@ const
   ManifestContainers* = {
     $DagPBCodec: DagPBCoder()
   }.toTable
-
-type
-  Manifest* = ref object of RootObj
-    rootHash*: ?Cid         # Root (tree) hash of the contained data set
-    originalBytes*: NBytes  # Exact size of the original (uploaded) file
-    blockSize*: NBytes      # Size of each contained block (might not be needed if blocks are len-prefixed)
-    blocks*: seq[Cid]       # Block Cid
-    version*: CidVersion    # Cid version
-    hcodec*: MultiCodec     # Multihash codec
-    codec*: MultiCodec      # Data set codec
-    case protected*: bool   # Protected datasets have erasure coded info
-    of true:
-      ecK*: int               # Number of blocks to encode
-      ecM*: int               # Number of resulting parity blocks
-      originalCid*: Cid     # The original Cid of the dataset being erasure coded
-      originalLen*: int     # The length of the original manifest
-    else:
-      discard

codex/node.nim

@@ -63,7 +63,7 @@ proc findPeer*(
   peerId: PeerId
 ): Future[?PeerRecord] {.async.} =
   ## Find peer using the discovery service from the given CodexNode
-  ## 
+  ##
   return await node.discovery.findPeer(peerId)
 
 proc connect*(
@@ -221,7 +221,7 @@ proc store*(
     await stream.close()
 
   # Generate manifest
-  blockManifest.originalBytes = NBytes chunker.offset  # store the exact file size
+  blockManifest.originalBytes = NBytes(chunker.offset)  # store the exact file size
   without data =? blockManifest.encode():
     return failure(
       newException(CodexError, "Could not generate dataset manifest!"))
@@ -332,7 +332,7 @@ proc new*(
     contracts = Contracts.default
 ): CodexNodeRef =
   ## Create new instance of a Codex node, call `start` to run it
-  ## 
+  ##
   CodexNodeRef(
     switch: switch,
     blockStore: store,

tests/codex/testmanifest.nim

@@ -62,6 +62,8 @@ checksuite "Manifest":
         Block.new(("Block " & $it).toBytes).tryGet().cid
       )
       manifest = Manifest.new(blocks).tryGet()
+
+    var
       protected = Manifest.new(manifest, 2, 2).tryGet()
 
     check:
@@ -72,7 +74,7 @@ checksuite "Manifest":
 
     # fill up with empty Cid's
     for i in protected.rounded..<protected.len:
-      protected.blocks[i] = EmptyCid[manifest.version]
+      protected[i] = EmptyCid[manifest.version]
         .catch
         .get()[manifest.hcodec]
         .catch