Implement more checks and wire up the invalid SSZ tests

beacon_chain/ssz.nim

@@ -269,8 +269,17 @@ template fromSszBytes*[T; N](_: type TypeWithMaxLen[T, N],
   mixin fromSszBytes
   fromSszBytes(T, bytes)
 
-proc readValue*(r: var SszReader, val: var auto) =
+proc readValue*[T](r: var SszReader, val: var T) =
-  val = readSszValue(r.stream.readBytes(r.stream.endPos), val.type)
+  const minimalSize = fixedPortionSize(T)
+  when isFixedSize(T):
+    if r.stream[].ensureBytes(minimalSize):
+      val = readSszValue(r.stream.readBytes(minimalSize), T)
+    else:
+      raise newException(MalformedSszError, "SSZ input of insufficient size")
+  else:
+    # TODO Read the fixed portion first and precisely measure the size of
+    # the dynamic portion to consume the right number of bytes.
+    val = readSszValue(r.stream.readBytes(r.stream.endPos), T)
 
 proc readValue*[T](r: var SszReader, val: var SizePrefixed[T]) =
   let length = r.stream.readVarint(uint64)

beacon_chain/ssz/bytes_reader.nim

@@ -75,6 +75,14 @@ template fromSszBytes*(T: type BitSeq, bytes: openarray[byte]): auto =
 func fromSszBytes*[N](T: type BitList[N], bytes: openarray[byte]): auto =
   BitList[N] @bytes
 
+func fromSszBytes*[N](T: type BitArray[N], bytes: openarray[byte]): T =
+  # A bit vector doesn't have a marker bit, but we'll use the helper from
+  # nim-stew to determine the position of the leading (marker) bit.
+  # If it's outside the BitArray size, we have an overflow:
+  if bitsLen(bytes) > N - 1:
+    raise newException(MalformedSszError, "SSZ bit array overflow")
+  copyMem(addr result.bytes[0], unsafeAddr bytes[0], bytes.len)
+
 func readSszValue*(input: openarray[byte], T: type): T =
   mixin fromSszBytes, toSszType
 
@@ -144,6 +152,11 @@ func readSszValue*(input: openarray[byte], T: type): T =
 
       result[resultLen - 1] = readSszValue(input[offset ..< input.len], ElemType)
 
+  elif result is SomeInteger|bool|enum|BitArray:
+    trs "READING BASIC TYPE ", type(result).name, "  input=", input.len
+    result = fromSszBytes(type(result), input)
+    trs "RESULT WAS ", repr(result)
+
   elif result is object|tuple:
     const minimallyExpectedSize = fixedPortionSize(T)
     if input.len < minimallyExpectedSize:
@@ -183,10 +196,5 @@ func readSszValue*(input: openarray[byte], T: type): T =
         trs "READING FOREIGN ", fieldName, ": ", name(SszType)
         field = fromSszBytes(FieldType, input[startOffset ..< endOffset])
 
-  elif result is SomeInteger|bool|enum:
-    trs "READING BASIC TYPE ", type(result).name, "  input=", input.len
-    result = fromSszBytes(type(result), input)
-    trs "RESULT WAS ", repr(result)
-
   else:
     unsupported T

tests/official/test_fixture_ssz_generic_types.nim

@@ -8,9 +8,9 @@
 import
   # Standard library
   os, unittest, strutils, streams, strformat, strscans,
-  macros,
+  macros, typetraits,
   # Status libraries
-  stint, stew/bitseqs, ../testutil,
+  faststreams, stint, stew/bitseqs, ../testutil,
   # Third-party
   yaml,
   # Beacon chain internals
@@ -33,6 +33,9 @@ type
     # Containers have a root (thankfully) and signing_root field
     signing_root: string
 
+  UnconsumedInput* = object of CatchableError
+  TestSizeError* = object of ValueError
+
 # Make signing root optional
 setDefaultValue(SSZHashTreeRoot, signing_root, "")
 
@@ -73,10 +76,26 @@ type
 # Type specific checks
 # ------------------------------------------------------------------------
 
-proc checkBasic(T: typedesc, dir: string, expectedHash: SSZHashTreeRoot) =
+proc checkBasic(T: typedesc,
-  let deserialized = SSZ.loadFile(dir/"serialized.ssz", T)
+                dir: string,
-  check:
+                expectedHash: SSZHashTreeRoot) =
-    expectedHash.root == "0x" & toLowerASCII($deserialized.hashTreeRoot())
+  var fileContents = readFile(dir/"serialized.ssz")
+  var stream = memoryStream(fileContents)
+  var reader = init(SszReader, stream)
+
+  # We are using heap allocation to avoid stack overflow
+  # issues caused by large objects such as `BeaconState`:
+  var deserialized = new T
+  reader.readValue(deserialized[])
+
+  if not stream[].eof:
+    raise newException(UnconsumedInput, "Remaining bytes in the input")
+
+  let
+    expectedHash = expectedHash.root
+    actualHash = "0x" & toLowerASCII($deserialized.hashTreeRoot())
+  check expectedHash == actualHash
+
   # TODO check the value
 
 macro testVector(typeIdent: string, size: int): untyped =
@@ -100,19 +119,13 @@ macro testVector(typeIdent: string, size: int): untyped =
         ident"array", newLit(s), ident(t)
       )
       var testStmt = quote do:
-        # Need heap alloc
+        checkBasic(`T`, dir, expectedHash)
-        var deserialized: ref `T`
-        new deserialized
-        deserialized[] = SSZ.loadFile(dir/"serialized.ssz", `T`)
-        check:
-          expectedHash.root == "0x" & toLowerASCII($deserialized.hashTreeRoot())
-          # TODO check the value
       sizeDispatch.add nnkElifBranch.newTree(
         newCall(ident"==", size, newLit(s)),
         testStmt
       )
     sizeDispatch.add nnkElse.newTree quote do:
-      raise newException(ValueError,
+      raise newException(TestSizeError,
         "Unsupported **size** in type/size combination: array[" &
         $size & "," & typeIdent & ']')
     dispatcher.add nnkElifBranch.newTree(
@@ -136,31 +149,25 @@ proc checkVector(sszSubType, dir: string, expectedHash: SSZHashTreeRoot) =
   doAssert wasMatched
   testVector(typeIdent, size)
 
-type BitContainer[N: static int] = BitList[N] or BitArray[N]
-
-proc testBitContainer(T: typedesc[BitContainer], dir: string, expectedHash: SSZHashTreeRoot) =
-  let deserialized = SSZ.loadFile(dir/"serialized.ssz", T)
-  check:
-    expectedHash.root == "0x" & toLowerASCII($deserialized.hashTreeRoot())
-  # TODO check the value
-
 proc checkBitVector(sszSubType, dir: string, expectedHash: SSZHashTreeRoot) =
   var size: int
   let wasMatched = scanf(sszSubType, "bitvec_$i", size)
   doAssert wasMatched
   case size
-  of 1: testBitContainer(BitArray[1], dir, expectedHash)
+  of 1: checkBasic(BitArray[1], dir, expectedHash)
-  of 2: testBitContainer(BitArray[2], dir, expectedHash)
+  of 2: checkBasic(BitArray[2], dir, expectedHash)
-  of 3: testBitContainer(BitArray[3], dir, expectedHash)
+  of 3: checkBasic(BitArray[3], dir, expectedHash)
-  of 4: testBitContainer(BitArray[4], dir, expectedHash)
+  of 4: checkBasic(BitArray[4], dir, expectedHash)
-  of 5: testBitContainer(BitArray[5], dir, expectedHash)
+  of 5: checkBasic(BitArray[5], dir, expectedHash)
-  of 8: testBitContainer(BitArray[8], dir, expectedHash)
+  of 8: checkBasic(BitArray[8], dir, expectedHash)
-  of 16: testBitContainer(BitArray[16], dir, expectedHash)
+  of 9: checkBasic(BitArray[9], dir, expectedHash)
-  of 31: testBitContainer(BitArray[31], dir, expectedHash)
+  of 16: checkBasic(BitArray[16], dir, expectedHash)
-  of 512: testBitContainer(BitArray[512], dir, expectedHash)
+  of 31: checkBasic(BitArray[31], dir, expectedHash)
-  of 513: testBitContainer(BitArray[513], dir, expectedHash)
+  of 32: checkBasic(BitArray[32], dir, expectedHash)
+  of 512: checkBasic(BitArray[512], dir, expectedHash)
+  of 513: checkBasic(BitArray[513], dir, expectedHash)
   else:
-    raise newException(ValueError, "Unsupported BitVector of size " & $size)
+    raise newException(TestSizeError, "Unsupported BitVector of size " & $size)
 
 # TODO: serialization of "type BitList[maxLen] = distinct BitSeq is not supported"
 #       https://github.com/status-im/nim-beacon-chain/issues/518
@@ -168,30 +175,31 @@ proc checkBitVector(sszSubType, dir: string, expectedHash: SSZHashTreeRoot) =
 #   var maxLen: int
 #   let wasMatched = scanf(sszSubType, "bitlist_$i", maxLen)
 #   case maxLen
-#   of 1: testBitContainer(BitList[1], dir, expectedHash)
+#   of 1: checkBasic(BitList[1], dir, expectedHash)
-#   of 2: testBitContainer(BitList[2], dir, expectedHash)
+#   of 2: checkBasic(BitList[2], dir, expectedHash)
-#   of 3: testBitContainer(BitList[3], dir, expectedHash)
+#   of 3: checkBasic(BitList[3], dir, expectedHash)
-#   of 4: testBitContainer(BitList[4], dir, expectedHash)
+#   of 4: checkBasic(BitList[4], dir, expectedHash)
-#   of 5: testBitContainer(BitList[5], dir, expectedHash)
+#   of 5: checkBasic(BitList[5], dir, expectedHash)
-#   of 8: testBitContainer(BitList[8], dir, expectedHash)
+#   of 8: checkBasic(BitList[8], dir, expectedHash)
-#   of 16: testBitContainer(BitList[16], dir, expectedHash)
+#   of 16: checkBasic(BitList[16], dir, expectedHash)
-#   of 31: testBitContainer(BitList[31], dir, expectedHash)
+#   of 31: checkBasic(BitList[31], dir, expectedHash)
-#   of 512: testBitContainer(BitList[512], dir, expectedHash)
+#   of 512: checkBasic(BitList[512], dir, expectedHash)
-#   of 513: testBitContainer(BitList[513], dir, expectedHash)
+#   of 513: checkBasic(BitList[513], dir, expectedHash)
 #   else:
 #     raise newException(ValueError, "Unsupported Bitlist of max length " & $maxLen)
 
 # Test dispatch for valid inputs
 # ------------------------------------------------------------------------
 
-proc sszCheck(sszType, sszSubType: string) =
+proc sszCheck(baseDir, sszType, sszSubType: string) =
-  let dir = SSZDir/sszType/"valid"/sszSubType
+  let dir = baseDir/sszSubType
 
   # Hash tree root
   var expectedHash: SSZHashTreeRoot
-  var s = openFileStream(dir/"meta.yaml")
+  if fileExists(dir/"meta.yaml"):
-  yaml.load(s, expectedHash)
+    var s = openFileStream(dir/"meta.yaml")
-  s.close()
+    defer: close(s)
+    yaml.load(s, expectedHash)
 
   # Deserialization and checks
   case sszType
@@ -269,8 +277,24 @@ proc runSSZtests() =
     timedTest &"Testing {sszType:12} inputs - valid" & skipped:
       let path = SSZDir/sszType/"valid"
       for pathKind, sszSubType in walkDir(path, relative = true):
-        doAssert pathKind == pcDir
+        if pathKind != pcDir: continue
-        sszCheck(sszType, sszSubType)
+        sszCheck(path, sszType, sszSubType)
+
+    timedTest &"Testing {sszType:12} inputs - invalid" & skipped:
+      let path = SSZDir/sszType/"invalid"
+      for pathKind, sszSubType in walkDir(path, relative = true):
+        if pathKind != pcDir: continue
+        try:
+          sszCheck(path, sszType, sszSubType)
+        except SszError, UnconsumedInput:
+          discard
+        except TestSizeError as err:
+          echo err.msg
+          skip()
+        except:
+          checkpoint getStackTrace(getCurrentException())
+          checkpoint getCurrentExceptionMsg()
+          check false
 
   # TODO: nim-serialization forces us to use exceptions as control flow
   #       as we always have to check user supplied inputs

vendor/nim-stew

@@ -1 +1 @@
-Subproject commit f5be0ab63a64c314e74223891a7651e2eaaa8fec
+Subproject commit 50562b515a771cfc443557ee8e2dceee59207d52