Aristo db update foreground caching (status-im#1605)

* Fix vertex ID generator state handling for rocksdb backend

why:
 * Key error in walk iterator
 * Needs to be loaded when opening the database

* Use non-zero sub-table prefixes for rocksdb

why:
  Handy for debugging

* Fix error code for missing key on rocksdb backend

why:
  Previously returned `VOID_HASH_KEY` rather than `GetKeyNotFound`

* Explicitly copy vertex data between internal table and function/result argument

why:
  Function argument or return reference may still refer to the same data
  object.

* Updated error symbols

why:
  Error symbol names for the hike module now start with the prefix `Hike`.

* Write back modified branch node into local top layer cache

why:
  With the backend available, the source of the branch node references
  might not be the top layer cache. So any change must be explicitely
  recorded.

nimbus/db/aristo/aristo_constants.nim

@@ -21,6 +21,9 @@ const
   EmptyNibbleSeq* = EmptyBlob.initNibbleRange
     ## Useful shortcut (borrowed from `sync/snap/constants.nim`)
 
+  EmptyVidSeq* = seq[VertexID].default
+    ## Useful shortcut
+
   VOID_CODE_KEY* = EMPTY_CODE_HASH.to(HashKey)
     ## Equivalent of `nil` for `Account` object code hash
 

nimbus/db/aristo/aristo_debug.nim

@@ -251,17 +251,20 @@ proc ppXMap*(
 
 proc ppBe[T](be: T; db: AristoDb; indent: int): string =
   ## Walk over backend tables
-  let pfx = indent.toPfx
+  let
+    pfx = indent.toPfx
+    pfx1 = indent.toPfx(1)
+    pfx2 = indent.toPfx(2)
   result = "<" & $be.kind & ">"
-  result &= pfx & "vGen" & pfx & " [" & be.walkIdg.toSeq.mapIt(
-      it[2].pp
+  result &= pfx & "vGen" & pfx1 & "[" & be.walkIdg.toSeq.mapIt(
+      it[2].mapIt(it.ppVid).join(",")
     ).join(",") & "]"
-  result &= pfx & "sTab" & pfx & " {" & be.walkVtx.toSeq.mapIt(
+  result &= pfx & "sTab" & pfx1 & "{" & be.walkVtx.toSeq.mapIt(
       $(1+it[0]) & "(" & it[1].ppVid & "," & it[2].ppVtx(db,it[1]) & ")"
-    ).join("," & pfx & "  ") & "}"
-  result &= pfx & "kMap" & pfx & " {" & be.walkKey.toSeq.mapIt(
+    ).join(pfx2) & "}"
+  result &= pfx & "kMap" & pfx1 & "{" & be.walkKey.toSeq.mapIt(
       $(1+it[0]) & "(" & it[1].ppVid & "," & it[2].ppKey & ")"
-    ).join("," & pfx & "  ") & "}"
+    ).join(pfx2) & "}"
 
 # ------------------------------------------------------------------------------
 # Public functions
@@ -421,6 +424,7 @@ proc pp*(
 
 proc pp*(
     db: AristoDb;
+    vGenOk = true;
     sTabOk = true;
     lTabOk = true;
     kMapOk = true;
@@ -430,7 +434,7 @@ proc pp*(
   let
     pfx1 = indent.toPfx
     pfx2 = indent.toPfx(1)
-    tagOk = 1 < sTabOk.ord + lTabOk.ord + kMapOk.ord + pPrfOk.ord
+    tagOk = 1 < sTabOk.ord + lTabOk.ord + kMapOk.ord + pPrfOk.ord + vGenOk.ord
   var
     pfy = ""
 
@@ -445,6 +449,9 @@ proc pp*(
     rc
 
   if not db.top.isNil:
+    if vGenOk:
+      let info = "vGen(" & $db.top.vGen.len & ")"
+      result &= info.doPrefix & db.top.vGen.pp
     if sTabOk:
       let info = "sTab(" & $db.top.sTab.len & ")"
       result &= info.doPrefix & db.top.sTab.pp(db,indent+1)

nimbus/db/aristo/aristo_delete.nim

@@ -34,24 +34,27 @@ proc branchStillNeeded(vtx: VertexRef): bool =
     if vtx.bVid[n].isValid:
       return true
 
-proc clearKey(db: AristoDb; vid: VertexID) =
-  let key = db.top.kMap.getOrVoid vid
-  if key.isValid:
+proc clearKey(
+    db: AristoDb;                      # Database, top layer
+    vid: VertexID;                     # Vertex IDs to clear
+      ) =
+  let lbl = db.top.kMap.getOrVoid vid
+  if lbl.isValid:
     db.top.kMap.del vid
-    db.top.pAmk.del key
+    db.top.pAmk.del lbl
   elif db.getKeyBackend(vid).isOK:
     # Register for deleting on backend
     db.top.kMap[vid] = VOID_HASH_LABEL
-    db.top.pAmk.del key
+    db.top.pAmk.del lbl
 
-proc doneWith(db: AristoDb; vid: VertexID) =
+proc doneWith(
+    db: AristoDb;                      # Database, top layer
+    vid: VertexID;                     # Vertex IDs to clear
+      ) =
   # Remove entry
-  db.vidDispose vid    # Will be propagated to backend
+  db.vidDispose vid                    # Will be propagated to backend
   db.top.sTab.del vid
-  let key = db.top.kMap.getOrVoid vid
-  if key.isValid:
-    db.top.kMap.del vid
-    db.top.pAmk.del key
+  db.clearKey vid
 
 
 proc deleteImpl(
@@ -66,45 +69,53 @@ proc deleteImpl(
     return err((VertexID(0),hike.error))
 
   # doAssert 0 < hike.legs.len and hike.tail.len == 0 # as assured by `hikeUp()`
-  var inx = hike.legs.len - 1
-
-  # Remove leaf entry on the top
-  let lf =  hike.legs[inx].wp
-  if lf.vtx.vType != Leaf:
-    return err((lf.vid,DelLeafExpexted))
-  if lf.vid in db.top.pPrf:
-    return err((lf.vid, DelLeafLocked))
-  db.doneWith lf.vid
-  inx.dec
-
-  while 0 <= inx:
-    # Unlink child vertex
-    let br = hike.legs[inx].wp
-    if br.vtx.vType != Branch:
-      return err((br.vid,DelBranchExpexted))
-    if br.vid in db.top.pPrf:
-      return err((br.vid, DelBranchLocked))
-    br.vtx.bVid[hike.legs[inx].nibble] = VertexID(0)
-
-    if br.vtx.branchStillNeeded:
-      db.clearKey br.vid
-      break
-
-    # Remove this `Branch` entry
-    db.doneWith br.vid
-    inx.dec
 
-    if inx < 0:
-      break
+  var lf: VidVtxPair
+  block:
+    var inx = hike.legs.len - 1
+
+    # Remove leaf entry on the top
+    lf =  hike.legs[inx].wp
+    if lf.vtx.vType != Leaf:
+      return err((lf.vid,DelLeafExpexted))
+    if lf.vid in db.top.pPrf:
+      return err((lf.vid, DelLeafLocked))
+    db.doneWith(lf.vid)
+    inx.dec
 
-    # There might be an optional `Extension` to remove
-    let ext = hike.legs[inx].wp
-    if ext.vtx.vType == Extension:
+    while 0 <= inx:
+      # Unlink child vertex
+      let br = hike.legs[inx].wp
+      if br.vtx.vType != Branch:
+        return err((br.vid,DelBranchExpexted))
       if br.vid in db.top.pPrf:
-        return err((ext.vid, DelExtLocked))
-      db.doneWith ext.vid
+        return err((br.vid, DelBranchLocked))
+      br.vtx.bVid[hike.legs[inx].nibble] = VertexID(0)
+      db.top.sTab[br.vid] = br.vtx
+
+      if br.vtx.branchStillNeeded:
+        # Clear all keys up to the toot key
+        db.clearKey(br.vid)
+        while 0 < inx:
+          inx.dec
+          db.clearKey(hike.legs[inx].wp.vid)
+        break
+
+      # Remove this `Branch` entry
+      db.doneWith(br.vid)
       inx.dec
 
+      if inx < 0:
+        break
+
+      # There might be an optional `Extension` to remove
+      let ext = hike.legs[inx].wp
+      if ext.vtx.vType == Extension:
+        if br.vid in db.top.pPrf:
+          return err((ext.vid, DelExtLocked))
+        db.doneWith(ext.vid)
+        inx.dec
+
   # Delete leaf entry
   let rc = db.getVtxBackend lf.vid
   if rc.isErr and rc.error == GetVtxNotFound:

nimbus/db/aristo/aristo_desc.nim

@@ -41,14 +41,14 @@ type
     sTab*: Table[VertexID,VertexRef] ## Structural vertex table
     lTab*: Table[LeafTie,VertexID]   ## Direct access, path to leaf vertex
     kMap*: Table[VertexID,HashLabel] ## Merkle hash key mapping
-    pAmk*: Table[HashLabel,VertexID] ## Reverse mapper for data import
+    pAmk*: Table[HashLabel,VertexID] ## Reverse `kMap` entries, hash key lookup
     pPrf*: HashSet[VertexID]         ## Locked vertices (proof nodes)
     vGen*: seq[VertexID]             ## Unique vertex ID generator
 
   AristoDb* = object
     ## Set of database layers, supporting transaction frames
-    top*: AristoLayerRef             ## Database working layer
-    stack*: seq[AristoLayerRef]      ## Stashed parent layers
+    top*: AristoLayerRef             ## Database working layer, mutable
+    stack*: seq[AristoLayerRef]      ## Stashed immutable parent layers
     backend*: AristoBackendRef       ## Backend database (may well be `nil`)
 
     # Debugging data below, might go away in future
@@ -58,13 +58,13 @@ type
 # Public helpers
 # ------------------------------------------------------------------------------
 
-proc getOrVoid*[W](tab: Table[W,VertexRef]; w: W): VertexRef =
+func getOrVoid*[W](tab: Table[W,VertexRef]; w: W): VertexRef =
   tab.getOrDefault(w, VertexRef(nil))
 
-proc getOrVoid*[W](tab: Table[W,HashLabel]; w: W): HashLabel =
+func getOrVoid*[W](tab: Table[W,HashLabel]; w: W): HashLabel =
   tab.getOrDefault(w, VOID_HASH_LABEL)
 
-proc getOrVoid*[W](tab: Table[W,VertexID]; w: W): VertexID =
+func getOrVoid*[W](tab: Table[W,VertexID]; w: W): VertexID =
   tab.getOrDefault(w, VertexID(0))
 
 # --------

nimbus/db/aristo/aristo_desc/aristo_error.nim

@@ -45,12 +45,12 @@ type
     CacheMissingNodekeys
 
     # Path function `hikeUp()`
-    PathRootMissing
-    PathLeafTooEarly
-    PathBranchTailEmpty
-    PathBranchBlindEdge
-    PathExtTailEmpty
-    PathExtTailMismatch
+    HikeRootMissing
+    HikeLeafTooEarly
+    HikeBranchTailEmpty
+    HikeBranchBlindEdge
+    HikeExtTailEmpty
+    HikeExtTailMismatch
 
     # Path/nibble/key conversions in `aisto_path.nim`
     PathExpected64Nibbles
@@ -69,6 +69,7 @@ type
     MergeLeafPathCachedAlready
     MergeNonBranchProofModeLock
     MergeRootBranchLinkBusy
+    MergeAssemblyFailed # Ooops, internal error
 
     MergeHashKeyInvalid
     MergeRootVidInvalid

nimbus/db/aristo/aristo_hashify.nim

@@ -42,13 +42,12 @@
 {.push raises: [].}
 
 import
-  std/[algorithm, sequtils, strutils],
-  std/[sets, tables],
+  std/[sequtils, sets, strutils, tables],
   chronicles,
   eth/common,
   stew/results,
-  "."/[aristo_constants, aristo_debug, aristo_desc, aristo_get,
-       aristo_hike, aristo_transcode, aristo_vid]
+  "."/[aristo_constants, aristo_desc, aristo_get, aristo_hike,
+       aristo_transcode, aristo_vid]
 
 type
   BackVidValRef = ref object
@@ -75,25 +74,6 @@ func getOrVoid(tab: BackVidTab; vid: VertexID): BackVidValRef =
 func isValid(brv: BackVidValRef): bool =
   brv != BackVidValRef(nil)
 
-# ------------------------------------------------------------------------------
-# Private helper, debugging
-# ------------------------------------------------------------------------------
-
-proc pp(w: BackVidValRef): string =
-  if w.isNil:
-    return "n/a"
-  result = "(" & w.root.pp & ","
-  if w.onBe:
-    result &= "*"
-  result &= "," & w.toVid.pp & ")"
-
-proc pp(t: BackVidTab): string =
-  proc pp(b: bool): string =
-    if b: "*" else: ""
-  "{" & t.keys.toSeq.mapIt(it.uint64).sorted.mapIt(it.VertexID)
-              .mapIt("(" & it.pp & "," & t.getOrVoid(it).pp & ")")
-              .join(",") & "}"
-
 # ------------------------------------------------------------------------------
 # Private functions
 # ------------------------------------------------------------------------------
@@ -233,7 +213,7 @@ proc hashify*(
   for (lky,vid) in db.top.lTab.pairs:
     let hike = lky.hikeUp(db)
     if hike.error != AristoError(0):
-      return err((hike.root,hike.error))
+      return err((vid,hike.error))
 
     roots.incl hike.root
 
@@ -367,20 +347,24 @@ proc hashifyCheck*(
 
   else:
     for (vid,lbl) in db.top.kMap.pairs:
-      let vtx = db.getVtx vid
-      if vtx.isValid:
-        let rc = vtx.toNode(db)
-        if rc.isOk:
-          if lbl.key != rc.value.encode.digestTo(HashKey):
-            return err((vid,HashifyCheckVtxHashMismatch))
-
-          let revVid = db.top.pAmk.getOrVoid lbl
-          if not revVid.isValid:
-            return err((vid,HashifyCheckRevHashMissing))
-          if revVid != vid:
-            return err((vid,HashifyCheckRevHashMismatch))
-
-  if db.top.pAmk.len != db.top.kMap.len:
+      if lbl.isValid:                              # Otherwise to be deleted
+        let vtx = db.getVtx vid
+        if vtx.isValid:
+          let rc = vtx.toNode(db)
+          if rc.isOk:
+            if lbl.key != rc.value.encode.digestTo(HashKey):
+              return err((vid,HashifyCheckVtxHashMismatch))
+
+            let revVid = db.top.pAmk.getOrVoid lbl
+            if not revVid.isValid:
+              return err((vid,HashifyCheckRevHashMissing))
+            if revVid != vid:
+              return err((vid,HashifyCheckRevHashMismatch))
+
+  # Some `kMap[]` entries may ne void indicating backend deletion
+  let kMapCount = db.top.kMap.values.toSeq.filterIt(it.isValid).len
+
+  if db.top.pAmk.len != kMapCount:
     var knownKeys: HashSet[VertexID]
     for (key,vid) in db.top.pAmk.pairs:
       if not db.top.kMap.hasKey(vid):
@@ -390,7 +374,8 @@ proc hashifyCheck*(
       knownKeys.incl vid
     return err((VertexID(0),HashifyCheckRevCountMismatch)) # should not apply(!)
 
-  if 0 < db.top.pAmk.len and not relax and db.top.pAmk.len != db.top.sTab.len:
+  if 0 < db.top.pAmk.len and not relax and db.top.pAmk.len < db.top.sTab.len:
+    # Cannot have less changes than cached entries
     return err((VertexID(0),HashifyCheckVtxCountMismatch))
 
   for vid in db.top.pPrf: