Better error-handling for the slashingdb import/export feature

* Error when specifying an invalid --data-dir (or --validator-dir)
* Error when entering an invalid validator public key (e.g. invalid hex value)
* Warning when attempting to export a validator not present in the local database

Some unnecessary remains of the v1 mode has been removed as well

Author: zah

beacon_chain/conf.nim

@@ -9,15 +9,18 @@
 
 import
   strutils, os, options, unicode, uri,
+
   chronicles, chronicles/options as chroniclesOptions,
   confutils, confutils/defs, confutils/std/net, stew/shims/net as stewNet,
-  stew/io2, unicodedb/properties, normalize,
+  stew/[io2, byteutils], unicodedb/properties, normalize,
   eth/common/eth_types as commonEthTypes, eth/net/nat,
   eth/p2p/discoveryv5/enr,
   json_serialization, web3/[ethtypes, confutils_defs],
-  spec/[crypto, keystore, digest, datatypes, network],
+
+  ./spec/[crypto, keystore, digest, datatypes, network],
   ./networking/network_metadata,
-  filepath
+  ./validators/slashing_protection_common,
+  ./filepath
 
 export
   uri,
@@ -505,7 +508,7 @@ type
           desc: "Limit the export to specific validators " &
                 "(specified as numeric indices or public keys)"
           abbr: "v"
-          name: "validator" }: seq[string]
+          name: "validator" }: seq[PubKey0x]
         exportedInterchangeFile* {.
           desc: "EIP-3076 slashing protection interchange file to export"
           argument }: OutFile
@@ -559,8 +562,9 @@ type
         name: "rpc-port" }: Port
 
       rpcAddress* {.
-        defaultValue: defaultAdminListenAddress
-        desc: "Address of the server to connect to for RPC [=127.0.0.1]"
+        desc: "Address of the server to connect to for RPC"
+        defaultValue: init(ValidIpAddress, "127.0.0.1")
+        defaultValueDesc: "127.0.0.1"
         name: "rpc-address" }: ValidIpAddress
 
       retryDelay* {.
@@ -623,6 +627,13 @@ func parseCmdArg*(T: type Uri, input: TaintedString): T
 func completeCmdArg*(T: type Uri, input: TaintedString): seq[string] =
   return @[]
 
+func parseCmdArg*(T: type PubKey0x, input: TaintedString): T
+                 {.raises: [ValueError, Defect].} =
+  PubKey0x(hexToPaddedByteArray[RawPubKeySize](input.string))
+
+func completeCmdArg*(T: type PubKey0x, input: TaintedString): seq[string] =
+  return @[]
+
 func parseCmdArg*(T: type Checkpoint, input: TaintedString): T
                  {.raises: [ValueError, Defect].} =
   let sepIdx = find(input.string, ':')

beacon_chain/nimbus_beacon_node.nim

@@ -1962,8 +1962,7 @@ proc doSlashingImport(conf: BeaconNodeConf) {.raises: [SerializationError, IOErr
     genesis_validators_root = Eth2Digest spdir.metadata.genesis_validators_root,
     basePath = dir,
     dbname = filetrunc,
-    modes = {kCompleteArchiveV2},
-    disagreementBehavior = kChooseV2
+    modes = {kCompleteArchive}
   )
 
   # Now import the slashing interchange file

beacon_chain/validators/slashing_protection.nim

@@ -45,23 +45,15 @@ export chronicles
 
 type
   SlashProtDBMode* = enum
-    kCompleteArchiveV1 # Complete Format V1 backend (saves all attestations)
-    kCompleteArchiveV2 # Complete Format V2 backend (saves all attestations)
-    kLowWatermarkV2    # Low-Watermark Format V2 backend (prunes attestations)
+    kCompleteArchive # Complete Format V2 backend (saves all attestations)
+    kLowWatermark    # Low-Watermark Format V2 backend (prunes attestations)
 
   SlashingProtectionDB* = ref object
     ## Database storing the blocks attested
     ## by validators attached to a beacon node
     ## or validator client.
     db_v2*: SlashingProtectionDB_v2
     modes: set[SlashProtDBMode]
-    disagreementBehavior: DisagreementBehavior
-
-  DisagreementBehavior* = enum
-    ## How to handle disagreement between DB versions
-    kCrash
-    kChooseV1
-    kChooseV2
 
 # DB Multiversioning
 # -------------------------------------------------------------
@@ -78,20 +70,18 @@ proc init*(
        genesis_validators_root: Eth2Digest,
        basePath, dbname: string,
        modes: set[SlashProtDBMode],
-       disagreementBehavior: DisagreementBehavior
      ): T =
   ## Initialize or load a slashing protection DB
   ## This is for Beacon Node usage
   ## Handles DB version migration
 
   doAssert modes.card >= 1, "No slashing protection mode chosen. Choose a v1, a v2 or v1 and v2 slashing DB mode."
   doAssert not(
-    kCompleteArchiveV2 in modes and
-    kLowWatermarkV2 in modes), "Mode(s): " & $modes & ". Choose only one of V2 DB modes."
+    kCompleteArchive in modes and
+    kLowWatermark in modes), "Mode(s): " & $modes & ". Choose only one of V2 DB modes."
 
   new result
   result.modes = modes
-  result.disagreementBehavior = disagreementBehavior
 
   let (db, requiresMigration) = SlashingProtectionDB_v2.initCompatV1(
     genesis_validators_root,
@@ -142,8 +132,7 @@ proc init*(
   ## Does not handle migration
   init(
     T, genesis_validators_root, basePath, dbname,
-    modes = {kLowWatermarkV2},
-    disagreementBehavior = kChooseV2
+    modes = {kLowWatermark}
   )
 
 proc loadUnchecked*(
@@ -158,15 +147,15 @@ proc loadUnchecked*(
   new result
 
   result.modes = {}
-  result.disagreementBehavior = kCrash
 
   try:
     result.db_v2 = SlashingProtectionDB_v2.loadUnchecked(
       basePath, dbname, readOnly
     )
-    result.modes.incl(kCompleteArchiveV2)
-  except:
-    result.disagreementBehavior = kChooseV1
+    result.modes.incl(kCompleteArchive)
+  except CatchableError as err:
+    error "Failed to load the Slashing protection database", err = err.msg
+    quit 1
 
 proc close*(db: SlashingProtectionDB) =
   ## Close a slashing protection database
@@ -280,10 +269,10 @@ proc pruneAfterFinalization*(
   ## This ensures that even if pruning is called with an incorrect epoch
   ## slashing protection can fallback to the minimal / high-watermark protection mode.
   ##
-  ## Pruning is only done if pruning is enabled (DB in kLowWatermarkV2 mode)
+  ## Pruning is only done if pruning is enabled (DB in kLowWatermark mode)
   ## Pruning is only triggered on v2 database.
 
-  if kLowWatermarkV2 in db.modes:
+  if kLowWatermark in db.modes:
     db.db_v2.pruneAfterFinalization(finalizedEpoch)
 
 # The high-level import/export functions are
@@ -305,7 +294,7 @@ proc toSPDIR*(db: SlashingProtectionDB): SPDIR
 proc exportSlashingInterchange*(
        db: SlashingProtectionDB,
        path: string,
-       validatorsWhiteList: seq[string] = @[],
+       validatorsWhiteList: seq[PubKey0x] = @[],
        prettify = true) {.raises: [Defect, IOError].} =
   ## Export a database to the Slashing Protection Database Interchange Format
   # We could modify toSPDIR to do the filtering directly
@@ -317,7 +306,14 @@ proc exportSlashingInterchange*(
     # O(a log b) with b the number of validators to keep
     #        and a the total number of validators in DB
     let validators = validatorsWhiteList.sorted()
-    spdir.data.keepItIf(validators.binarySearch("0x" & it.pubkey.PubKeyBytes.toHex()) != -1)
+    spdir.data.keepItIf(validators.binarySearch(it.pubkey) != -1)
+
+    if spdir.data.len != validatorsWhiteList.len:
+      let exportedKeys = spdir.data.mapIt(it.pubkey).sorted()
+      for v in validators:
+        if exportedKeys.binarySearch(v) == -1:
+          warn "Specified validator key not found in the slashing database",
+                key = v.PubKeyBytes.toHex
 
   Json.saveFile(path, spdir, prettify)
   echo "Exported slashing protection DB to '", path, "'"

beacon_chain/validators/slashing_protection_common.nim

@@ -9,7 +9,7 @@
 
 import
   # Stdlib
-  std/[typetraits, strutils, algorithm, sequtils],
+  std/[typetraits, strutils, algorithm],
   # Status
   eth/db/[kvstore, kvstore_sqlite3],
   stew/results,
@@ -170,6 +170,15 @@ func `==`*(a, b: BadVote): bool =
     of BadVoteKind.DatabaseError:
       true
 
+template `==`*(a, b: PubKey0x): bool =
+  PubKeyBytes(a) == PubKeyBytes(b)
+
+template `<`*(a, b: PubKey0x): bool =
+  PubKeyBytes(a) < PubKeyBytes(b)
+
+template cmp*(a, b: PubKey0x): bool =
+  cmp(PubKeyBytes(a), PubKeyBytes(b))
+
 func `==`*(a, b: BadProposal): bool =
   ## Comparison operator.
   ## Used implictily by Result when comparing the

beacon_chain/validators/slashing_protection_v1.nim

@@ -398,7 +398,7 @@ proc loadUnchecked*(
     subkey(kGenesisValidatorsRoot)
   ).get(), "The Slashing DB is missing genesis information"
 
-  result = T(backend: backend)
+  T(backend: backend)
 
 proc close*(db: SlashingProtectionDB_v1) =
   discard db.backend.close()