Simplify state root api (#2864)

`updateOk` is obsolete and always set to true - callers should not have
to care about this detail

also take the opportunity to clean up storage root naming

nimbus/db/aristo/aristo_api.nim

@@ -93,13 +93,11 @@ type
         {.noRaise.}
       ## Fetch an account record from the database indexed by `accPath`.
 
-  AristoApiFetchAccountStateRootFn* =
+  AristoApiFetchStateRootFn* =
     proc(db: AristoDbRef;
-         updateOk: bool;
         ): Result[Hash32,AristoError]
         {.noRaise.}
-      ## Fetch the Merkle hash of the account root. Force update if the
+      ## Fetch the Merkle hash of the account root.
-      ## argument `updateOK` is set `true`.
 
   AristoApiFetchStorageDataFn* =
     proc(db: AristoDbRef;
@@ -113,11 +111,9 @@ type
   AristoApiFetchStorageRootFn* =
     proc(db: AristoDbRef;
          accPath: Hash32;
-         updateOk: bool;
         ): Result[Hash32,AristoError]
         {.noRaise.}
-      ## Fetch the Merkle hash of the storage root related to `accPath`. Force
+      ## Fetch the Merkle hash of the storage root related to `accPath`.
-      ## update if the argument `updateOK` is set `true`.
 
   AristoApiFindTxFn* =
     proc(db: AristoDbRef;
@@ -425,7 +421,7 @@ type
     fetchLastSavedState*: AristoApiFetchLastSavedStateFn
 
     fetchAccountRecord*: AristoApiFetchAccountRecordFn
-    fetchAccountStateRoot*: AristoApiFetchAccountStateRootFn
+    fetchStateRoot*: AristoApiFetchStateRootFn
     fetchStorageData*: AristoApiFetchStorageDataFn
     fetchStorageRoot*: AristoApiFetchStorageRootFn
 
@@ -472,7 +468,7 @@ type
     AristoApiProfFetchLastSavedStateFn  = "fetchLastSavedState"
 
     AristoApiProfFetchAccountRecordFn   = "fetchAccountRecord"
-    AristoApiProfFetchAccountStateRootFn = "fetchAccountStateRoot"
+    AristoApiProfFetchStateRootFn = "fetchStateRoot"
     AristoApiProfFetchStorageDataFn     = "fetchStorageData"
     AristoApiProfFetchStorageRootFn     = "fetchStorageRoot"
 
@@ -534,7 +530,7 @@ when AutoValidateApiHooks:
     doAssert not api.fetchLastSavedState.isNil
 
     doAssert not api.fetchAccountRecord.isNil
-    doAssert not api.fetchAccountStateRoot.isNil
+    doAssert not api.fetchStateRoot.isNil
     doAssert not api.fetchStorageData.isNil
     doAssert not api.fetchStorageRoot.isNil
 
@@ -601,7 +597,7 @@ func init*(api: var AristoApiObj) =
   api.fetchLastSavedState = fetchLastSavedState
 
   api.fetchAccountRecord = fetchAccountRecord
-  api.fetchAccountStateRoot = fetchAccountStateRoot
+  api.fetchStateRoot = fetchStateRoot
   api.fetchStorageData = fetchStorageData
   api.fetchStorageRoot = fetchStorageRoot
 
@@ -650,7 +646,7 @@ func dup*(api: AristoApiRef): AristoApiRef =
 
     fetchLastSavedState:  api.fetchLastSavedState,
     fetchAccountRecord:   api.fetchAccountRecord,
-    fetchAccountStateRoot: api.fetchAccountStateRoot,
+    fetchStateRoot: api.fetchStateRoot,
     fetchStorageData:     api.fetchStorageData,
     fetchStorageRoot:     api.fetchStorageRoot,
 
@@ -742,10 +738,10 @@ func init*(
       AristoApiProfFetchAccountRecordFn.profileRunner:
         result = api.fetchAccountRecord(a, b)
 
-  profApi.fetchAccountStateRoot =
+  profApi.fetchStateRoot =
     proc(a: AristoDbRef; b: bool): auto =
-      AristoApiProfFetchAccountStateRootFn.profileRunner:
+      AristoApiProfFetchStateRootFn.profileRunner:
-        result = api.fetchAccountStateRoot(a, b)
+        result = api.fetchStateRoot(a, b)
 
   profApi.fetchStorageData =
     proc(a: AristoDbRef; b, stoPath: Hash32): auto =
@@ -753,9 +749,9 @@ func init*(
         result = api.fetchStorageData(a, b, stoPath)
 
   profApi.fetchStorageRoot =
-    proc(a: AristoDbRef; b: Hash32; c: bool): auto =
+    proc(a: AristoDbRef; b: Hash32): auto =
       AristoApiProfFetchStorageRootFn.profileRunner:
-        result = api.fetchStorageRoot(a, b, c)
+        result = api.fetchStorageRoot(a, b)
 
   profApi.findTx =
     proc(a: AristoDbRef; b: RootedVertexID; c: HashKey): auto =

nimbus/db/aristo/aristo_fetch.nim

@@ -80,20 +80,13 @@ proc retrieveAccountLeaf(
 proc retrieveMerkleHash(
     db: AristoDbRef;
     root: VertexID;
-    updateOk: bool;
       ): Result[Hash32,AristoError] =
   let key =
-    if updateOk:
+    db.computeKey((root, root)).valueOr:
-      db.computeKey((root, root)).valueOr:
+      if error == GetVtxNotFound:
-        if error == GetVtxNotFound:
+        return ok(EMPTY_ROOT_HASH)
-          return ok(EMPTY_ROOT_HASH)
+      return err(error)
-        return err(error)
+
-    else:
-      let (key, _) = db.getKeyRc((root, root)).valueOr:
-        if error == GetKeyNotFound:
-          return ok(EMPTY_ROOT_HASH) # empty sub-tree
-        return err(error)
-      key
   ok key.to(Hash32)
 
 proc hasAccountPayload(
@@ -219,12 +212,11 @@ proc fetchAccountRecord*(
 
   ok leafVtx.lData.account
 
-proc fetchAccountStateRoot*(
+proc fetchStateRoot*(
     db: AristoDbRef;
-    updateOk: bool;
       ): Result[Hash32,AristoError] =
   ## Fetch the Merkle hash of the account root.
-  db.retrieveMerkleHash(VertexID(1), updateOk)
+  db.retrieveMerkleHash(VertexID(1))
 
 proc hasPathAccount*(
     db: AristoDbRef;
@@ -248,14 +240,13 @@ proc fetchStorageData*(
 proc fetchStorageRoot*(
     db: AristoDbRef;
     accPath: Hash32;
-    updateOk: bool;
       ): Result[Hash32,AristoError] =
   ## Fetch the Merkle hash of the storage root related to `accPath`.
   let stoID = db.fetchStorageIdImpl(accPath).valueOr:
     if error == FetchPathNotFound:
       return ok(EMPTY_ROOT_HASH) # no sub-tree
     return err(error)
-  db.retrieveMerkleHash(stoID, updateOk)
+  db.retrieveMerkleHash(stoID)
 
 proc hasPathStorage*(
     db: AristoDbRef;

nimbus/db/core_db/backend/aristo_trace.nim

@@ -243,7 +243,7 @@ proc jLogger(
 func to(w: AristoApiProfNames; T: type TracePfx): T =
   case w:
   of AristoApiProfFetchAccountRecordFn,
-     AristoApiProfFetchAccountStateRootFn,
+     AristoApiProfFetchStateRootFn,
      AristoApiProfDeleteAccountRecordFn,
      AristoApiProfMergeAccountRecordFn:
     return TrpAccounts
@@ -466,26 +466,25 @@ proc ariTraceRecorder(tr: TraceRecorderRef) =
         debug logTxt $info, level, accPath, accRec
       ok accRec
 
-  tracerApi.fetchAccountStateRoot =
+  tracerApi.fetchStateRoot =
     proc(mpt: AristoDbRef;
-         updateOk: bool;
         ): Result[Hash32,AristoError] =
-      const info = AristoApiProfFetchAccountStateRootFn
+      const info = AristoApiProfFetchStateRootFn
 
       when CoreDbNoisyCaptJournal:
         let level = tr.topLevel()
 
       # Find entry on DB
-      let state = api.fetchAccountStateRoot(mpt, updateOk).valueOr:
+      let state = api.fetchStateRoot(mpt).valueOr:
         when CoreDbNoisyCaptJournal:
-          debug logTxt $info, level, updateOk, error
+          debug logTxt $info, level, error
         tr.jLogger logRecord(info, TrqFind, error)
         return err(error)
 
       tr.jLogger logRecord(info, TrqFind, state)
 
       when CoreDbNoisyCaptJournal:
-        debug logTxt $info, level, updateOk, state
+        debug logTxt $info, level, state
       ok state
 
   tracerApi.fetchStorageData =
@@ -514,7 +513,6 @@ proc ariTraceRecorder(tr: TraceRecorderRef) =
   tracerApi.fetchStorageRoot =
     proc(mpt: AristoDbRef;
          accPath: Hash32;
-         updateOk: bool;
         ): Result[Hash32,AristoError] =
       const info = AristoApiProfFetchStorageRootFn
 
@@ -522,16 +520,16 @@ proc ariTraceRecorder(tr: TraceRecorderRef) =
         let level = tr.topLevel()
 
       # Find entry on DB
-      let state = api.fetchStorageRoot(mpt, accPath, updateOk).valueOr:
+      let state = api.fetchStorageRoot(mpt, accPath).valueOr:
         when CoreDbNoisyCaptJournal:
-          debug logTxt $info, level, accPath, updateOk, error
+          debug logTxt $info, level, accPath, error
         tr.jLogger(accPath, logRecord(info, TrqFind, error))
         return err(error)
 
       tr.jLogger(accPath, logRecord(info, TrqFind, state))
 
       when CoreDbNoisyCaptJournal:
-        debug logTxt $info, level, accPath, updateOk, state
+        debug logTxt $info, level, accPath, state
       ok state
 
   tracerApi.deleteAccountRecord =

nimbus/db/core_db/base.nim

@@ -530,21 +530,17 @@ proc hasPath*(
   acc.ifTrackNewApi:
     debug logTxt, api, elapsed, accPath=($$accPath), result
 
-proc stateRoot*(acc: CoreDbAccRef; updateOk = false): CoreDbRc[Hash32] =
+proc getStateRoot*(acc: CoreDbAccRef): CoreDbRc[Hash32] =
   ## This function retrieves the Merkle state hash of the accounts
   ## column (if available.)
-  ##
-  ## If the argument `updateOk` is set `true`, the Merkle hashes of the
-  ## database will be updated first (if needed, at all).
-  ##
   acc.setTrackNewApi AccStateFn
   result = block:
-    let rc = acc.call(fetchAccountStateRoot, acc.mpt, updateOk)
+    let rc = acc.call(fetchStateRoot, acc.mpt)
     if rc.isOk:
       ok(rc.value)
     else:
       err(rc.error.toError $api)
-  acc.ifTrackNewApi: debug logTxt, api, elapsed, updateOK, result
+  acc.ifTrackNewApi: debug logTxt, api, elapsed, result
 
 # ------------ storage ---------------
 
@@ -647,36 +643,32 @@ proc slotMerge*(
     debug logTxt, api, elapsed, accPath=($$accPath),
             stoPath=($$stoPath), stoData, result
 
-proc slotState*(
+proc slotStorageRoot*(
     acc: CoreDbAccRef;
     accPath: Hash32;
-    updateOk = false;
       ):  CoreDbRc[Hash32] =
   ## This function retrieves the Merkle state hash of the storage data
   ## column (if available) related to the account  indexed by the key
   ## `accPath`.`.
   ##
-  ## If the argument `updateOk` is set `true`, the Merkle hashes of the
+  acc.setTrackNewApi AccSlotStorageRootFn
-  ## database will be updated first (if needed, at all).
-  ##
-  acc.setTrackNewApi AccSlotStateFn
   result = block:
-    let rc = acc.call(fetchStorageRoot, acc.mpt, accPath, updateOk)
+    let rc = acc.call(fetchStorageRoot, acc.mpt, accPath)
     if rc.isOk:
       ok(rc.value)
     else:
       err(rc.error.toError $api)
   acc.ifTrackNewApi:
-    debug logTxt, api, elapsed, accPath=($$accPath), updateOk, result
+    debug logTxt, api, elapsed, accPath=($$accPath), result
 
-proc slotStateEmpty*(
+proc slotStorageEmpty*(
     acc: CoreDbAccRef;
     accPath: Hash32;
       ):  CoreDbRc[bool] =
   ## This function returns `true` if the storage data column is empty or
   ## missing.
   ##
-  acc.setTrackNewApi AccSlotStateEmptyFn
+  acc.setTrackNewApi AccSlotStorageEmptyFn
   result = block:
     let rc = acc.call(hasStorageData, acc.mpt, accPath)
     if rc.isOk:
@@ -686,12 +678,12 @@ proc slotStateEmpty*(
   acc.ifTrackNewApi:
     debug logTxt, api, elapsed, accPath=($$accPath), result
 
-proc slotStateEmptyOrVoid*(
+proc slotStorageEmptyOrVoid*(
     acc: CoreDbAccRef;
     accPath: Hash32;
       ): bool =
-  ## Convenience wrapper, returns `true` where `slotStateEmpty()` would fail.
+  ## Convenience wrapper, returns `true` where `slotStorageEmpty()` would fail.
-  acc.setTrackNewApi AccSlotStateEmptyOrVoidFn
+  acc.setTrackNewApi AccSlotStorageEmptyOrVoidFn
   result = block:
     let rc = acc.call(hasStorageData, acc.mpt, accPath)
     if rc.isOk:
@@ -707,14 +699,13 @@ proc recast*(
     acc: CoreDbAccRef;
     accPath: Hash32;
     accRec: CoreDbAccount;
-    updateOk = false;
       ): CoreDbRc[Account] =
   ## Complete the argument `accRec` to the portable Ethereum representation
   ## of an account statement. This conversion may fail if the storage colState
-  ## hash (see `slotState()` above) is currently unavailable.
+  ## hash (see `slotStorageRoot()` above) is currently unavailable.
   ##
   acc.setTrackNewApi AccRecastFn
-  let rc = acc.call(fetchStorageRoot, acc.mpt, accPath, updateOk)
+  let rc = acc.call(fetchStorageRoot, acc.mpt, accPath)
   result = block:
     if rc.isOk:
       ok Account(
@@ -725,8 +716,8 @@ proc recast*(
     else:
       err(rc.error.toError $api)
   acc.ifTrackNewApi:
-    let slotState = if rc.isOk: $$(rc.value) else: "n/a"
+    let storageRoot = if rc.isOk: $$(rc.value) else: "n/a"
-    debug logTxt, api, elapsed, accPath=($$accPath), slotState, result
+    debug logTxt, api, elapsed, accPath=($$accPath), storageRoot, result
 
 # ------------------------------------------------------------------------------
 # Public transaction related methods

nimbus/db/core_db/base/api_tracking.nim

@@ -45,9 +45,9 @@ type
     AccSlotHasPathFn    = "slotHasPath"
     AccSlotMergeFn      = "slotMerge"
     AccSlotProofFn      = "slotProof"
-    AccSlotStateFn      = "slotState"
+    AccSlotStorageRootFn = "slotStorageRoot"
-    AccSlotStateEmptyFn = "slotStateEmpty"
+    AccSlotStorageEmptyFn = "slotStorageEmpty"
-    AccSlotStateEmptyOrVoidFn = "slotStateEmptyOrVoid"
+    AccSlotStorageEmptyOrVoidFn = "slotStorageEmptyOrVoid"
     AccSlotPairsIt      = "slotPairs"
 
     BaseFinishFn        = "finish"

nimbus/db/ledger.nim

@@ -371,13 +371,11 @@ proc init*(x: typedesc[LedgerRef], db: CoreDbRef): LedgerRef =
   init(x, db, false)
 
 proc getStateRoot*(ac: LedgerRef): Hash32 =
-  const info = "state(): "
   # make sure all savepoint already committed
   doAssert(ac.savePoint.parentSavepoint.isNil)
   # make sure all cache already committed
   doAssert(ac.isDirty == false)
-  ac.ledger.stateRoot(updateOk=true).valueOr:
+  ac.ledger.getStateRoot().expect("working database")
-    raiseAssert info & $$error
 
 proc isTopLevelClean*(ac: LedgerRef): bool =
   ## Getter, returns `true` if all pending data have been commited.
@@ -514,7 +512,7 @@ proc contractCollision*(ac: LedgerRef, address: Address): bool =
     return
   acc.statement.nonce != 0 or
     acc.statement.codeHash != EMPTY_CODE_HASH or
-      not ac.ledger.slotStateEmptyOrVoid(acc.toAccountKey)
+      not ac.ledger.slotStorageEmptyOrVoid(acc.toAccountKey)
 
 proc accountExists*(ac: LedgerRef, address: Address): bool =
   let acc = ac.getAccount(address, false)
@@ -600,7 +598,7 @@ proc clearStorage*(ac: LedgerRef, address: Address) =
   let acc = ac.getAccount(address)
   acc.flags.incl {Alive, NewlyCreated}
 
-  let empty = ac.ledger.slotStateEmpty(acc.toAccountKey).valueOr: return
+  let empty = ac.ledger.slotStorageEmpty(acc.toAccountKey).valueOr: return
   if not empty:
     # need to clear the storage from the database first
     let acc = ac.makeDirty(address, cloneStorage = false)
@@ -733,14 +731,14 @@ iterator accounts*(ac: LedgerRef): Account =
   doAssert(ac.savePoint.parentSavepoint.isNil)
   for _, acc in ac.savePoint.cache:
     yield ac.ledger.recast(
-      acc.toAccountKey, acc.statement, updateOk=true).value
+      acc.toAccountKey, acc.statement).value
 
 iterator pairs*(ac: LedgerRef): (Address, Account) =
   # make sure all savepoint already committed
   doAssert(ac.savePoint.parentSavepoint.isNil)
   for address, acc in ac.savePoint.cache:
     yield (address, ac.ledger.recast(
-      acc.toAccountKey, acc.statement, updateOk=true).value)
+      acc.toAccountKey, acc.statement).value)
 
 iterator storage*(
     ac: LedgerRef;
@@ -771,7 +769,7 @@ proc getStorageRoot*(ac: LedgerRef, address: Address): Hash32 =
   # the storage root will not be updated
   let acc = ac.getAccount(address, false)
   if acc.isNil: EMPTY_ROOT_HASH
-  else: ac.ledger.slotState(acc.toAccountKey).valueOr: EMPTY_ROOT_HASH
+  else: ac.ledger.slotStorageRoot(acc.toAccountKey).valueOr: EMPTY_ROOT_HASH
 
 proc update(wd: var WitnessData, acc: AccountRef) =
   # once the code is touched make sure it doesn't get reset back to false in another update

nimbus/tracer.nim

@@ -189,14 +189,14 @@ proc traceTransactionImpl(
       before.captureAccount(stateDb, miner, minerName)
       stateDb.persist()
       stateDiff["beforeRoot"] = %(stateDb.getStateRoot().toHex)
-      discard com.db.ctx.getAccounts.stateRoot(updateOk=true) # lazy hashing!
+      discard com.db.ctx.getAccounts.getStateRoot() # lazy hashing!
       stateCtx = CaptCtxRef.init(com, stateDb.getStateRoot())
 
     let rc = vmState.processTransaction(tx, sender, header)
     gasUsed = if rc.isOk: rc.value else: 0
 
     if idx.uint64 == txIndex:
-      discard com.db.ctx.getAccounts.stateRoot(updateOk=true) # lazy hashing!
+      discard com.db.ctx.getAccounts.getStateRoot() # lazy hashing!
       after.captureAccount(stateDb, sender, senderName)
       after.captureAccount(stateDb, recipient, recipientName)
       after.captureAccount(stateDb, miner, minerName)

tests/test_blockchain_json.nim

@@ -59,7 +59,7 @@ proc parseEnv(node: JsonNode): TestEnv =
   result.pre = node["pre"]
 
 proc rootExists(db: CoreDbRef; root: Hash32): bool =
-  let state = db.ctx.getAccounts().stateRoot(updateOk=true).valueOr:
+  let state = db.ctx.getAccounts().getStateRoot().valueOr:
     return false
   state == root