Coredb and sync maintenance update (#2583)

* bump metrics

* Remove cruft

* Cosmetics, update some logging, noise control

* Renamed `CoreDb` function `hasKey` => `hasKeyRc` and provided `hasKey`

why:
  Currently, `hasKey` returns a `Result[]` rather than a `bool` which
  is what one would expect from a function prototype of this name.

  This was a bit of an annoyance and cost unnecessary attention.

README.md

@@ -219,12 +219,6 @@ available.)
 For these variables, using <variable>=0 is ignored and <variable>=2
 has the same effect as <variable>=1 (ditto for other numbers.)
 
-Other settings where the non-zero value matters:
-
- * ENABLE_ETH_VERSION=66<br>
-   Enable legacy protocol `eth66` (or another available protocol version.)
-
-
 ### <a name="devel-tips"></a>Development tips
 
 Interesting Make variables and targets are documented in the [nimbus-build-system](https://github.com/status-im/nimbus-build-system) repo.

nimbus/common/common.nim

@@ -122,7 +122,7 @@ func daoCheck(conf: ChainConfig) =
 proc initializeDb(com: CommonRef) =
   let kvt = com.db.ctx.getKvt()
   proc contains(kvt: CoreDbKvtRef; key: openArray[byte]): bool =
-    kvt.hasKey(key).expect "valid bool"
+    kvt.hasKeyRc(key).expect "valid bool"
   if canonicalHeadHashKey().toOpenArray notin kvt:
     info "Writing genesis to DB"
     doAssert(com.genesisHeader.number == 0.BlockNumber,

nimbus/config.nim

@@ -183,14 +183,6 @@ type
       abbr: "y"
       name: "sync-mode" .}: SyncMode
 
-    syncCtrlFile* {.
-      desc: "Specify a file that is regularly checked for updates. If it " &
-            "exists it is checked for whether it contains extra information " &
-            "specific to the type of sync process. This option is primarily " &
-            "intended only for sync testing and debugging."
-      abbr: "z"
-      name: "sync-ctrl-file" }: Option[string]
-
     importKey* {.
       desc: "Import unencrypted 32 bytes hex private key from a file"
       defaultValue: ""
@@ -858,11 +850,6 @@ proc makeConfig*(cmdLine = commandLineParams()): NimbusConf
      result.dataDir.string != defaultDataDir():
     result.keyStore = OutDir(result.dataDir.string / "keystore")
 
-  # For consistency
-  if result.syncCtrlFile.isSome and result.syncCtrlFile.unsafeGet == "":
-    error "Argument missing", option="sync-ctrl-file"
-    quit QuitFailure
-
 when isMainModule:
   # for testing purpose
   discard makeConfig()

nimbus/db/core_db/backend/aristo_trace.nim

@@ -433,7 +433,7 @@ proc kvtTraceRecorder(tr: TraceRecorderRef) =
 
       # Find entry on DB
       let
-        hasKey = api.hasKey(kvt, key).valueOr:
+        hasKey = api.hasKeyRc(kvt, key).valueOr:
           when CoreDbNoisyCaptJournal:
             debug logTxt $info, level, key=($$key), error
           tr.jLogger(key, logRecord(info, TrqAdd, error))
@@ -455,7 +455,7 @@ proc kvtTraceRecorder(tr: TraceRecorderRef) =
 
   assert tr.kvtSave != tr.db.kvtApi
   assert tr.kvtSave.del != tr.db.kvtApi.del
-  assert tr.kvtSave.hasKey == tr.db.kvtApi.hasKey
+  assert tr.kvtSave.hasKeyRc == tr.db.kvtApi.hasKeyRc
 
 
 proc ariTraceRecorder(tr: TraceRecorderRef) =

nimbus/db/core_db/base.nim

@@ -391,18 +391,31 @@ proc put*(
   kvt.ifTrackNewApi:
     debug logTxt, api, elapsed, key=key.toStr, val=val.toLenStr, result
 
-proc hasKey*(kvt: CoreDbKvtRef; key: openArray[byte]): CoreDbRc[bool] =
-  ## Would be named `contains` if it returned `bool` rather than `Result[]`.
+proc hasKeyRc*(kvt: CoreDbKvtRef; key: openArray[byte]): CoreDbRc[bool] =
+  ## For the argument `key` return `true` if `get()` returned a value on
+  ## that argument, `false` if it returned `GetNotFound`, and an error
+  ## otherwise.
   ##
-  kvt.setTrackNewApi KvtHasKeyFn
+  kvt.setTrackNewApi KvtHasKeyRcFn
   result = block:
-    let rc = kvt.call(hasKey, kvt.kvt, key)
+    let rc = kvt.call(hasKeyRc, kvt.kvt, key)
     if rc.isOk:
       ok(rc.value)
     else:
       err(rc.error.toError $api)
   kvt.ifTrackNewApi: debug logTxt, api, elapsed, key=key.toStr, result
 
+proc hasKey*(kvt: CoreDbKvtRef; key: openArray[byte]): bool =
+  ## Simplified version of `hasKeyRc` where `false` is returned instead of
+  ## an error.
+  ##
+  ## This function prototype is in line with the `hasKey` function for
+  ## `Tables`.
+  ##
+  kvt.setTrackNewApi KvtHasKeyFn
+  result = kvt.call(hasKeyRc, kvt.kvt, key).valueOr: false
+  kvt.ifTrackNewApi: debug logTxt, api, elapsed, key=key.toStr, result
+
 # ------------------------------------------------------------------------------
 # Public functions for generic columns
 # ------------------------------------------------------------------------------

nimbus/db/core_db/base/api_tracking.nim

@@ -73,6 +73,7 @@ type
     KvtDelFn            = "del"
     KvtGetFn            = "get"
     KvtGetOrEmptyFn     = "getOrEmpty"
+    KvtHasKeyRcFn       = "hasKeyRc"
     KvtHasKeyFn         = "hasKey"
     KvtLenFn            = "len"
     KvtPairsIt          = "pairs"