Invalidate boundary when converting HashList to HashArray in HashSeq (#150)

* Invalidate boundary when converting HashList to HashArray in HashSeq

Cache layout is equivalent, but when using readSszBytes the boundary
has to be cleared first to cover the case where the resize does not
incrementally fill up the list to the full capacity before converting.
Also move the boundary cache invalidation from types.nim to codec.nim.

* i386

* We actually only need to clear the boundary between retained and new

Author: etan-status

ssz_serialization/codec.nim

@@ -185,6 +185,8 @@ proc readSszValue*[T](
     type E = typeof toSszType(declval ElemType(typeof val))
     when val is HashList|HashSeq:
       template v: untyped = val.data
+      when not supportsBulkCopy(type v[0]):
+        let oldDataLen = v.len
     else:
       template v: untyped = val
 
@@ -225,9 +227,8 @@ proc readSszValue*[T](
 
         when val is HashList|HashSeq:
           # Unconditionally trigger small, O(1) updates to handle when the list
-          # shrinks without otherwise changing.
-          val.clearCaches(0)
-          val.clearCaches(max(val.len - 1, 0))
+          # resizes without otherwise changing.
+          val.clearCaches(max(min(val.len, oldDataLen) - 1, 0))
 
     else:
       if input.len == 0:
@@ -275,9 +276,8 @@ proc readSszValue*[T](
 
       when val is HashList|HashSeq:
         # Unconditionally trigger small, O(1) updates to handle when the list
-        # shrinks without otherwise changing.
-        val.clearCaches(0)
-        val.clearCaches(max(val.len - 1, 0))
+        # resizes without otherwise changing.
+        val.clearCaches(max(min(val.len, oldDataLen) - 1, 0))
 
   elif val is BitArray:
     if sizeof(val) != input.len:

ssz_serialization/types.nim

@@ -533,8 +533,7 @@ func resizeHashes[T, I](
   ## Ensure the hash cache is the correct size for the data in the list - must
   ## be called whenever `data` shrinks or grows.
   let
-    leaves = int(
-      chunkIdx(T, dataLen + dataPerChunk(T) - 1))
+    leaves = int(chunkIdx(T, dataLen + dataPerChunk(T) - 1))
     newSize = 1 + max(cacheNodes(depth, leaves), 1)
 
   # Growing might be because of add(), addDefault(), or in-place reading of a
@@ -557,22 +556,14 @@ func resizeHashes[T, I](
 
   newIndices[0] = nodesAtLayer(0, depth, leaves)
   for i in 1 .. max(depth, 1):
-    newIndices[i] =
-      newIndices[i - 1] + nodesAtLayer(i - 1, depth, leaves)
+    newIndices[i] = newIndices[i - 1] + nodesAtLayer(i - 1, depth, leaves)
 
-  # When resizing, copy each layer (truncating when shrinking)
+  # When resizing, copy each layer (truncating when shrinking).
   for i in 1 ..< max(depth, 1):
     let copyLen = min(
       indices[i] - indices[i-1], newIndices[i] - newIndices[i - 1])
     for j in 0 ..< copyLen:
-      newHashes[newIndices[i - 1] + j] = hashes[indices[i - 1] + j]
-
-    # When shrinking or growing, the last entry at each layer may cover a
-    # subtree that straddles the boundary between retained and changed elements
-    # (removed when shrinking, newly added when growing), making its cached
-    # hash stale - reset it to force recomputation
-    if copyLen > 0:
-      newHashes[newIndices[i - 1] + copyLen - 1] = uninitSentinel
+      assign(newHashes[newIndices[i - 1] + j], hashes[indices[i - 1] + j])
 
   swap(hashes, newHashes)
   indices = newIndices
@@ -653,7 +644,8 @@ func resizeHashes*(a: var HashSeq) =
   let
     totalChunkCount = a.data.totalChunkCount
     (firstIdx, maxDepth) = totalChunkCount.progressiveBottom
-  if a.hashes.len != maxDepth:
+    oldMaxDepth = a.hashes.len
+  if maxDepth != oldMaxDepth:
     clearCache(a.root)
     a.hashes.setLen(maxDepth)
     a.indices.reset()
@@ -667,6 +659,11 @@ func resizeHashes*(a: var HashSeq) =
           a.hashes[depth].setLen(hashesLen)
         else:
           clearCache(a.hashes[depth][0])
+          if maxDepth > oldMaxDepth and
+              oldMaxDepth > 0 and depth == oldMaxDepth - 1:
+            # Convert bottom `HashList` to intermediate `HashArray`
+            a.T.clearCachesArray(
+              a.hashes[depth], max(depth.int shl 1, 1), maxLen - 1)
       a.hashes[^1].reset()
       let maxLen = a.T.valuesPerChunk.Limit shl ((maxDepth - 1) shl 1)
       a.indices.setLen(a.T.hashListIndicesLen(maxLen))
@@ -788,9 +785,8 @@ template swap*(a, b: var HashList) =
 template swap*(a, b: var HashSeq) =
   swap(a.data, b.data)
   swap(a.root, b.root)
-  swap(a.stemHashes, b.stemHashes)
-  swap(a.bottomHashes, b.bottomHashes)
-  swap(a.bottomIndices, b.bottomIndices)
+  swap(a.hashes, b.hashes)
+  swap(a.indices, b.indices)
 
 template clear*(a: var HashList) =
   if not a.data.setLen(0):

tests/test_hashcache.nim

@@ -9,11 +9,11 @@
 {.used.}
 
 import
-  std/sequtils,
+  std/[random, sequtils],
   stew/byteutils,
   unittest2,
   ../ssz_serialization,
-  ../ssz_serialization/[merkleization, types]
+  ../ssz_serialization/merkleization
 
 type Foo = object
   x: Digest
@@ -24,8 +24,11 @@ let foo = Foo(
     "0x4175371111cef0d13cb836c17dba708f026f2ddbf057b91384bb78b1ba42343c")),
   y: 42)
 
-proc checkResize(items: var HashList, counts: varargs[int]) =
+proc checkResize[T](items: var T, counts: varargs[int]) =
   for count in counts:
+    when T is HashList:
+      if count + 4 > int(T.maxLen):
+        continue
     for data in [
         SSZ.encode((0 ..< count).mapIt(foo)),
         SSZ.encode((0 ..< count).mapIt(foo) & (0 ..< 4).mapIt(default(Foo)))]:
@@ -35,9 +38,10 @@ proc checkResize(items: var HashList, counts: varargs[int]) =
         raiseAssert "Valid SSZ"
       check items.hash_tree_root() == items.data.hash_tree_root()
 
-suite "HashList":
+template runHashCacheTests[T](_: typedesc[T]): untyped =
   setup:
-    var items: HashList[Foo, 8192]
+    randomize(42)
+    var items: T
 
   test "Shrink to smaller cache depth":
     items.checkResize(1074, 1018)
@@ -58,21 +62,86 @@ suite "HashList":
     items.checkResize(100, 0)
 
   test "Multiple resizes in sequence":
-    items.checkResize(100, 500, 1074, 1018, 200, 2000, 50, 0, 300)
+    items.checkResize(
+      100, 500, 1074, 1018, 200, 2000, 50, 0, 300, 304, 309, 314)
 
   test "Incremental add":
     for i in 0 ..< 100:
-      check:
+      when items is HashList:
+        check items.add(foo)
+      else:
         items.add(foo)
-        items.hash_tree_root() == items.data.hash_tree_root()
+      check items.hash_tree_root() == items.data.hash_tree_root()
 
   test "Incremental add across cache depth boundary":
     items.checkResize(1020)
     for i in 1020 ..< 1080:
-      check:
+      when items is HashList:
+        check items.add(foo)
+      else:
         items.add(foo)
-        items.hash_tree_root() == items.data.hash_tree_root()
+      check items.hash_tree_root() == items.data.hash_tree_root()
 
   test "Incremental decrease":
     for i in countdown(1050, 0):
       items.checkResize(i)
+
+  test "Progressive depth boundaries":
+    items.checkResize(21844, 340, 20, 84, 1, 340)
+
+  test "Random resize sequence":
+    for _ in 0 ..< 50:
+      let count =
+        when items is HashList:
+          rand(int(items.maxLen) - 4)
+        else:
+          rand(4000)
+      items.checkResize(count)
+
+  test "Random add/resize mix":
+    for _ in 0 ..< 100:
+      let canAdd =
+        when items is HashList:
+          items.data.len < int(items.maxLen)
+        else:
+          true
+      if canAdd and rand(1) == 0:
+        when items is HashList:
+          check items.add(foo)
+        else:
+          items.add(foo)
+        check items.hash_tree_root() == items.data.hash_tree_root()
+      else:
+        let count =
+          when items is HashList:
+            rand(int(items.maxLen) - 4)
+          else:
+            rand(4000)
+        items.checkResize(count)
+
+suite "HashList":
+  runHashCacheTests(HashList[Foo, 8192])
+
+suite "HashSeq":
+  runHashCacheTests(HashSeq[Foo])
+
+suite "Cache layout equivalence (for HashSeq)":
+  template checkEquivalence(maxLen: static Limit) =
+    test $maxLen:
+      var
+        ha: HashArray[maxLen, Foo]
+        hl: HashList[Foo, maxLen]
+      for i in 0 ..< int(maxLen):
+        ha.data[i] = foo
+        check hl.add(foo)
+      discard ha.hash_tree_root()
+      discard hl.hash_tree_root()
+      check hl.hashes.len == ha.hashes.len
+      for i in 1 ..< hl.hashes.len:
+        check hl.hashes[i] == ha.hashes[i]
+
+  checkEquivalence(1)
+  checkEquivalence(4)
+  checkEquivalence(16)
+  checkEquivalence(64)
+  checkEquivalence(256)