Core db maintenance update (#2087)

* CoreDb+Aristo: Fix handler code

* Aristo+Kvt: Remove cruft

* Aristo+Kvt: The function `forkTop()` always provides a single transaction

why:
  Previously it provided a single squashed tx only if there were any. Now
  it will provide a blind one if there were none.

* Fix Copyright header

nimbus/db/aristo/aristo_api.nim

@@ -118,53 +118,13 @@ type
       ## A non centre descriptor should always be destructed after use (see
       ## also# comments on `fork()`.)
 
-  AristoApiForkFn* =
-    proc(db: AristoDbRef;
-         rawTopLayer = false;
-        ): Result[AristoDbRef,AristoError]
-        {.noRaise.}
-      ## This function creates a new empty descriptor accessing the same
-      ## backend (if any) database as the argument `db`. This new descriptor
-      ## joins the list of descriptors accessing the same backend database.
-      ##
-      ## After use, any unused non centre descriptor should be destructed
-      ## via `forget()`. Not doing so will not only hold memory ressources
-      ## but might also cost computing ressources for maintaining and
-      ## updating backend filters when writing to the backend database .
-      ##
-      ## If the argument `rawTopLayer` is set `true` the function will
-      ## provide an uninitalised and inconsistent (!) top layer. This
-      ## setting avoids some database lookup for cases where the top layer
-      ## is redefined anyway.
-
   AristoApiForkTopFn* =
     proc(db: AristoDbRef;
          dontHashify = false;
         ): Result[AristoDbRef,AristoError]
         {.noRaise.}
-      ## Clone a top transaction into a new DB descriptor accessing the same
+      ## Clone a descriptor in a way so that there is exactly one active
-      ## backend database (if any) as the argument `db`. The new descriptor
+      ## transaction.
-      ## is linked to the transaction parent and is fully functional as a
-      ## forked instance (see comments on `aristo_desc.reCentre()` for
-      ## details.) If there is no active transaction, the top layer state
-      ## is cloned.
-      ##
-      ## Input situation:
-      ## ::
-      ##   tx -> db0   with tx is top transaction, tx.level > 0
-      ##
-      ## Output situation:
-      ## ::
-      ##   tx  -> db0 \
-      ##               >  share the same backend
-      ##   tx1 -> db1 /
-      ##
-      ## where `tx.level > 0`, `db1.level == 1` and `db1` is returned. The
-      ## transaction `tx1` can be retrieved via `db1.txTop()`.
-      ##
-      ## The new DB descriptor will contain a copy of the argument transaction
-      ## `tx` as top layer of level 1 (i.e. this is he only transaction.)
-      ## Rolling back will end up at the backend layer (incl. backend filter.)
       ##
       ## If the arguent flag `dontHashify` is passed `true`, the clone
       ## descriptor will *NOT* be hashified right after construction.
@@ -365,7 +325,6 @@ type
     fetchPayload*: AristoApiFetchPayloadFn
     finish*: AristoApiFinishFn
     forget*: AristoApiForgetFn
-    fork*: AristoApiForkFn
     forkTop*: AristoApiForkTopFn
     getKey*: AristoApiGetKeyFn
     getKeyRc*: AristoApiGetKeyRcFn
@@ -397,7 +356,6 @@ type
     AristoApiProfFetchPayloadFn = "fetchPayload"
     AristoApiProfFinishFn       = "finish"
     AristoApiProfForgetFn       = "forget"
-    AristoApiProfForkFn         = "fork"
     AristoApiProfForkTopFn      = "forkTop"
     AristoApiProfGetKeyFn       = "getKey"
     AristoApiProfGetKeyRcFn     = "getKeyRc"
@@ -439,7 +397,6 @@ when AutoValidateApiHooks:
     doAssert not api.fetchPayload.isNil
     doAssert not api.finish.isNil
     doAssert not api.forget.isNil
-    doAssert not api.fork.isNil
     doAssert not api.forkTop.isNil
     doAssert not api.getKey.isNil
     doAssert not api.getKeyRc.isNil
@@ -491,7 +448,6 @@ func init*(api: var AristoApiObj) =
   api.fetchPayload = fetchPayload
   api.finish = finish
   api.forget = forget
-  api.fork = fork
   api.forkTop = forkTop
   api.getKey = getKey
   api.getKeyRc = getKeyRc
@@ -526,7 +482,6 @@ func dup*(api: AristoApiRef): AristoApiRef =
     fetchPayload: api.fetchPayload,
     finish:       api.finish,
     forget:       api.forget,
-    fork:         api.fork,
     forkTop:      api.forkTop,
     getKey:       api.getKey,
     getKeyRc:     api.getKeyRc,
@@ -606,11 +561,6 @@ func init*(
       AristoApiProfForgetFn.profileRunner:
         result = api.forget(a)
 
-  profApi.fork =
-    proc(a: AristoDbRef; b = false): auto =
-      AristoApiProfForkFn.profileRunner:
-        result = api.fork(a, b)
-
   profApi.forkTop =
     proc(a: AristoDbRef; b = false): auto =
       AristoApiProfForkTopFn.profileRunner:

nimbus/db/aristo/aristo_desc.nim

@@ -176,6 +176,7 @@ proc reCentre*(db: AristoDbRef) =
 proc fork*(
     db: AristoDbRef;
     noTopLayer = false;
+    noFilter = false;
       ): Result[AristoDbRef,AristoError] =
   ## This function creates a new empty descriptor accessing the same backend
   ## (if any) database as the argument `db`. This new descriptor joins the
@@ -186,6 +187,9 @@ proc fork*(
   ## also cost computing ressources for maintaining and updating backend
   ## filters when writing to the backend database .
   ##
+  ## If the argument `noFilter` is set `true` the function will fork directly
+  ## off the backend database and ignore any filter.
+  ##
   ## If the argument `noTopLayer` is set `true` the function will provide an
   ## uninitalised and inconsistent (!) descriptor object without top layer.
   ## This setting avoids some database lookup for cases where the top layer
@@ -199,8 +203,14 @@ proc fork*(
     dudes:   db.dudes,
     backend: db.backend)
 
+  if not noFilter:
+    clone.roFilter = db.roFilter # Ref is ok here (filters are immutable)
+
   if not noTopLayer:
     clone.top = LayerRef.init()
+    if not db.roFilter.isNil:
+      clone.top.final.vGen = db.roFilter.vGen
+    else:
       let rc = clone.backend.getIdgFn()
       if rc.isOk:
         clone.top.final.vGen = rc.value

nimbus/db/aristo/aristo_tx.nim

@@ -18,8 +18,9 @@ import
   results,
   "."/[aristo_desc, aristo_filter, aristo_get, aristo_layers, aristo_hashify]
 
-func isTop*(tx: AristoTxRef): bool
+func isTop*(tx: AristoTxRef): bool {.gcsafe.}
-func level*(db: AristoDbRef): int
+func level*(db: AristoDbRef): int {.gcsafe.}
+proc txBegin*(db: AristoDbRef): Result[AristoTxRef,AristoError] {.gcsafe.}
 
 # ------------------------------------------------------------------------------
 # Private helpers
@@ -72,6 +73,7 @@ func to*(tx: AristoTxRef; T: type[AristoDbRef]): T =
   ## Getter, retrieves the parent database descriptor from argument `tx`
   tx.db
 
+
 proc forkTx*(
     tx: AristoTxRef;                  # Transaction descriptor
     dontHashify = false;              # Process/fix MPT hashes
@@ -127,11 +129,9 @@ proc forkTx*(
       return err(rc.error)
 
   # Set up clone associated to `db`
-  let txClone = ? db.fork(noToplayer = true)
+  let txClone = ? db.fork(noToplayer = true, noFilter = false)
   txClone.top = db.layersCc tx.level  # Provide tx level 1 stack
   txClone.stack = @[stackLayer]       # Zero level stack
-  txClone.roFilter = db.roFilter          # No need to copy (done when updated)
-  txClone.backend = db.backend
   txClone.top.txUid = 1
   txClone.txUidGen = 1
 
@@ -154,43 +154,26 @@ proc forkTop*(
     dontHashify = false;              # Process/fix MPT hashes
       ): Result[AristoDbRef,AristoError] =
   ## Variant of `forkTx()` for the top transaction if there is any. Otherwise
-  ## the top layer is cloned, only.
+  ## the top layer is cloned, and an empty transaction is set up. After
+  ## successful fork the returned descriptor has transaction level 1.
   ##
   ## Use `aristo_desc.forget()` to clean up this descriptor.
   ##
   if db.txRef.isNil:
-    let dbClone = ? db.fork(noToplayer = true)
+    let dbClone = ? db.fork(noToplayer = true, noFilter = false)
-
     dbClone.top = db.layersCc         # Is a deep copy
-    dbClone.roFilter = db.roFilter # No need to copy contents when updated
-    dbClone.backend = db.backend
 
     if not dontHashify:
       dbClone.hashify().isOkOr:
         discard dbClone.forget()
         return err(error[1])
+
+    discard dbClone.txBegin
     return ok(dbClone)
+    # End if()
 
   db.txRef.forkTx dontHashify
 
-
-proc exec*(
-    tx: AristoTxRef;
-    action: AristoDbAction;
-    dontHashify = false;              # Process/fix MPT hashes
-      ): Result[void,AristoError] =
-  ## Execute function argument `action()` on a temporary `tx.forkTx()`
-  ## transaction clone database. After return, the temporary database gets
-  ## destroyed.
-  ##
-  ## If the arguent flag `dontHashify` is passed `true`, the clone database
-  ## will *NOT* be hashified right after construction.
-  ##
-  let db = ? tx.forkTx dontHashify
-  db.action()
-  ? db.forget()
-  ok()
-
 # ------------------------------------------------------------------------------
 # Public functions: Transaction frame
 # ------------------------------------------------------------------------------
@@ -272,7 +255,6 @@ proc commit*(
   if 0 < db.stack.len:
     db.txRef.txUid = db.getTxUid
     db.top.txUid = db.txRef.txUid
-
   ok()
 
 
@@ -364,7 +346,6 @@ proc stow*(
     delta: LayerDeltaRef(),
     final: LayerFinalRef(vGen: db.vGen),
     txUid: db.top.txUid)
-
   ok()
 
 # ------------------------------------------------------------------------------

nimbus/db/core_db/backend/aristo_db/handlers_aristo.nim

@@ -479,7 +479,7 @@ proc ctxMethods(cCtx: AristoCoreDbCtxRef): CoreDbCtxFns =
     base = cCtx.base   # Will not change and can be captured
     db = base.parent   # Ditto
     api = base.api     # Ditto
-    mpt = base.ctx.mpt # Ditto
+    mpt = cCtx.mpt     # Ditto
 
   proc ctxNewTrie(
       kind: CoreDbSubTrie;
@@ -724,10 +724,11 @@ func init*(T: type AristoBaseRef; db: CoreDbRef; adb: AristoDbRef): T =
     api:    AristoApiRef.init())
 
   # Create initial context
-  result.ctx = db.bless AristoCoreDbCtxRef(
+  let ctx = AristoCoreDbCtxRef(
     base: result,
     mpt:  adb)
-  result.ctx.methods = result.ctx.ctxMethods
+  ctx.methods = ctx.ctxMethods
+  result.ctx = db.bless ctx
 
   when CoreDbEnableApiProfiling:
     let profApi = AristoApiProfRef.init(result.api, adb.backend)

nimbus/db/core_db/base/validate.nim

@@ -71,7 +71,6 @@ proc validateMethodsDesc(fns: CoreDbMptFns) =
 
 proc validateMethodsDesc(fns: CoreDbAccFns) =
   doAssert not fns.backendFn.isNil
-  doAssert not fns.newMptFn.isNil
   doAssert not fns.fetchFn.isNil
   doAssert not fns.deleteFn.isNil
   doAssert not fns.stoFlushFn.isNil

nimbus/db/kvt/kvt_tx.nim

@@ -1,5 +1,5 @@
 # nimbus-eth1
-# Copyright (c) 2023 Status Research & Development GmbH
+# Copyright (c) 2023-2024 Status Research & Development GmbH
 # Licensed under either of
 #  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
 #    http://www.apache.org/licenses/LICENSE-2.0)
@@ -20,7 +20,8 @@ import
   ./kvt_desc/desc_backend,
   "."/[kvt_desc, kvt_layers]
 
-func isTop*(tx: KvtTxRef): bool
+func isTop*(tx: KvtTxRef): bool {.gcsafe.}
+proc txBegin*(db: KvtDbRef): Result[KvtTxRef,KvtError] {.gcsafe.}
 
 # ------------------------------------------------------------------------------
 # Private helpers
@@ -113,31 +114,20 @@ proc forkTx*(tx: KvtTxRef): Result[KvtDbRef,KvtError] =
 
 proc forkTop*(db: KvtDbRef): Result[KvtDbRef,KvtError] =
   ## Variant of `forkTx()` for the top transaction if there is any. Otherwise
-  ## the top layer is cloned, only.
+  ## the top layer is cloned, and an empty transaction is set up. After
+  ## successful fork the returned descriptor has transaction level 1.
   ##
   ## Use `kvt_desc.forget()` to clean up this descriptor.
   ##
   if db.txRef.isNil:
     let dbClone = ? db.fork()
     dbClone.top = db.layersCc
+
+    discard dbClone.txBegin
     return ok(dbClone)
 
   db.txRef.forkTx()
 
-
-proc exec*(
-    tx: KvtTxRef;
-    action: KvtDbAction;
-      ): Result[void,KvtError] =
-  ## Execute function argument `action()` on a temporary `tx.forkTx()`
-  ## transaction database. After return, the temporary database gets
-  ## destroyed.
-  ##
-  let db = ? tx.forkTx()
-  db.action()
-  ? db.forget()
-  ok()
-
 # ------------------------------------------------------------------------------
 # Public functions: Transaction frame
 # ------------------------------------------------------------------------------

tests/test_aristo/test_filter.nim

@@ -191,6 +191,11 @@ proc dbTriplet(w: LeafQuartet; rdbPath: string): Result[DbTriplet,AristoError] =
   let dx = [db, db.forkTop.value, db.forkTop.value]
   xCheck dx[0].nForked == 2
 
+  # Reduce unwanted tx layers
+  for n in 1 ..< dx.len:
+    check dx[n].level == 1
+    check dx[n].txTop.value.commit.isOk
+
   # Clause (9) from `aristo/README.md` example
   for n in 0 ..< dx.len:
     let report = dx[n].mergeList w[n+1]

tests/test_aristo/test_helpers.nim

@@ -217,7 +217,7 @@ proc hashify*(
     noisy: bool;
       ): Result[void,(VertexID,AristoError)] =
   when declared(aristo_hashify.noisy):
-    aristo_hashify.exec(aristo_hashify.hashify(db), noisy)
+    aristo_hashify.exec(noisy, aristo_hashify.hashify(db))
   else:
     aristo_hashify.hashify(db)
 
@@ -230,7 +230,7 @@ proc delete*(
     noisy: bool;
       ): Result[bool,(VertexID,AristoError)] =
   when declared(aristo_delete.noisy):
-    aristo_delete.exec(aristo_delete.delete(db, root, path, accPath), noisy)
+    aristo_delete.exec(noisy, aristo_delete.delete(db, root, path, accPath))
   else:
     aristo_delete.delete(db, root, path, accPath)
 
@@ -241,7 +241,7 @@ proc delete*(
     noisy: bool;
       ): Result[bool,(VertexID,AristoError)] =
   when declared(aristo_delete.noisy):
-    aristo_delete.exec(aristo_delete.delete(db, lty, accPath), noisy)
+    aristo_delete.exec(noisy, aristo_delete.delete(db, lty, accPath))
   else:
     aristo_delete.delete(db, lty, accPath)
 
@@ -252,7 +252,7 @@ proc delTree*(
     noisy: bool;
       ): Result[void,(VertexID,AristoError)] =
   when declared(aristo_delete.noisy):
-    aristo_delete.exec(aristo_delete.delTree(db, root, accPath), noisy)
+    aristo_delete.exec(noisy, aristo_delete.delTree(db, root, accPath))
   else:
     aristo_delete.delTree(db, root, accPath)
 
@@ -266,7 +266,7 @@ proc merge*(
     noisy: bool;
       ): Result[bool, AristoError] =
   when declared(aristo_merge.noisy):
-    aristo_merge.exec(aristo_merge.merge(db, root, path, data, accPath), noisy)
+    aristo_merge.exec(noisy, aristo_merge.merge(db, root, path, data, accPath))
   else:
     aristo_merge.merge(db, root, path, data, accPath)
 
@@ -278,7 +278,7 @@ proc mergePayload*(
     noisy: bool;
       ): Result[Hike,AristoError] =
   when declared(aristo_merge.noisy):
-    aristo_merge.exec(aristo_merge.mergePayload(db, lty, pyl, accPath), noisy)
+    aristo_merge.exec(noisy, aristo_merge.mergePayload(db, lty, pyl, accPath))
   else:
     aristo_merge.mergePayload(db, lty, pyl, accPath)