Invalidate boundary when converting HashList to HashArray in HashSeq

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)