cleanups

* move stuff out of beaconnode
* fix use-after-free in query

beacon_chain/beacon_node.nim

@@ -1,9 +1,9 @@
 import
   # Standard library
-  os, net, tables, random, strutils, times, memfiles,
+  os, net, tables, random, strutils, times,
 
   # Nimble packages
-  stew/[objects, bitseqs], stew/ranges/ptr_arith,
+  stew/[objects, bitseqs, byteutils], stew/ranges/ptr_arith,
   chronos, chronicles, confutils, metrics,
   json_serialization/std/[options, sets], serialization/errors,
   eth/trie/db, eth/trie/backends/rocksdb_backend, eth/async_utils,
@@ -16,7 +16,6 @@ import
   sync_protocol, request_manager, validator_keygen, interop, statusbar
 
 const
-  dataDirValidators = "validators"
   genesisFile = "genesis.ssz"
   hasPrompt = not defined(withoutPrompt)
   maxEmptySlotCount = uint64(24*60*60) div SECONDS_PER_SLOT
@@ -71,14 +70,8 @@ type
 
 proc onBeaconBlock*(node: BeaconNode, blck: BeaconBlock) {.gcsafe.}
 
-func localValidatorsDir(conf: BeaconNodeConf): string =
-  conf.dataDir / "validators"
-
-func databaseDir(conf: BeaconNodeConf): string =
-  conf.dataDir / "db"
-
 proc saveValidatorKey(keyName, key: string, conf: BeaconNodeConf) =
-  let validatorsDir = conf.dataDir / dataDirValidators
+  let validatorsDir = conf.localValidatorsDir
   let outputFile = validatorsDir / keyName
   createDir validatorsDir
   writeFile(outputFile, key)
@@ -283,17 +276,8 @@ proc addLocalValidator(
   node.attachedValidators.addLocalValidator(pubKey, privKey)
 
 proc addLocalValidators(node: BeaconNode, state: BeaconState) =
-  for validatorKeyFile in node.config.validators:
-    node.addLocalValidator state, validatorKeyFile.load
-
-  for kind, file in walkDir(node.config.localValidatorsDir):
-    if kind in {pcFile, pcLinkToFile}:
-      if cmpIgnoreCase(".privkey", splitFile(file).ext) == 0:
-        try:
-          let keyText = ValidatorPrivKey.init(readFile(file).string)
-          node.addLocalValidator state, keyText
-        except CatchableError:
-          warn "Failed to load a validator private key", file
+  for validatorKey in node.config.validatorKeys:
+    node.addLocalValidator state, validatorKey
 
   info "Local validators attached ", count = node.attachedValidators.count
 
@@ -1058,10 +1042,6 @@ when hasPrompt:
       # var t: Thread[ptr Prompt]
       # createThread(t, processPromptCommands, addr p)
 
-template bytes(memFile: MemFile): untyped =
-  let f = memFile
-  makeOpenArray(f.mem, byte, f.size)
-
 when isMainModule:
   randomize()
 
@@ -1191,14 +1171,14 @@ when isMainModule:
 
     of QueryCmd.get:
       let pathFragments = config.getQueryPath.split('/', maxsplit = 1)
-      var navigator: DynamicSszNavigator
+      let bytes =
+        case pathFragments[0]
+        of "genesis_state":
+          readFile(config.dataDir/genesisFile).string.toBytes()
+        else:
+          stderr.write config.getQueryPath & " is not a valid path"
+          quit 1
 
-      case pathFragments[0]
-      of "genesis_state":
-        var genesisMapFile = memfiles.open(config.dataDir/genesisFile)
-        navigator = DynamicSszNavigator.init(genesisMapFile.bytes, BeaconState)
-      else:
-        stderr.write config.getQueryPath & " is not a valid path"
-        quit 1
+      let navigator = DynamicSszNavigator.init(bytes, BeaconState)
 
       echo navigator.navigatePath(pathFragments[1 .. ^1]).toJson

beacon_chain/conf.nim

@@ -1,5 +1,6 @@
 import
-  os, options, strformat,
+  os, options, strformat, strutils,
+  chronicles, confutils,
   confutils/defs, chronicles/options as chroniclesOptions,
   spec/[crypto]
 
@@ -251,3 +252,25 @@ proc validatorFileBaseName*(validatorIdx: int): string =
 
 func dumpDir*(conf: BeaconNodeConf): string =
   conf.dataDir / "dump"
+
+func localValidatorsDir*(conf: BeaconNodeConf): string =
+  conf.dataDir / "validators"
+
+func databaseDir*(conf: BeaconNodeConf): string =
+  conf.dataDir / "db"
+
+iterator validatorKeys*(conf: BeaconNodeConf): ValidatorPrivKey =
+  for validatorKeyFile in conf.validators:
+    try:
+      yield validatorKeyFile.load
+    except CatchableError as err:
+      warn "Failed to load validator private key",
+        file = validatorKeyFile.string, err = err.msg
+
+  for kind, file in walkDir(conf.localValidatorsDir):
+    if kind in {pcFile, pcLinkToFile} and
+        cmpIgnoreCase(".privkey", splitFile(file).ext) == 0:
+      try:
+        yield ValidatorPrivKey.init(readFile(file).string)
+      except CatchableError as err:
+        warn "Failed to load a validator private key", file, err = err.msg

beacon_chain/spec/beaconstate.nim

@@ -308,7 +308,7 @@ proc process_registry_updates*(state: var BeaconState) =
   # Make visible, e.g.,
   # https://github.com/status-im/nim-beacon-chain/pull/608
   # https://github.com/sigp/lighthouse/pull/657
-  let epoch = get_current_epoch(state)
+  let epoch {.used.} = get_current_epoch(state)
   trace "process_registry_updates validator balances",
     balances=state.balances,
     active_validator_indices=get_active_validator_indices(state, epoch),

beacon_chain/spec/crypto.nim

@@ -9,40 +9,17 @@
 # cryptography in the spec is in flux, with sizes and test vectors still being
 # hashed out. This layer helps isolate those chagnes.
 
-# Useful conversation about BLS signatures (TODO: condense this)
+# BLS signatures can be combined such that multiple signatures are aggregated.
+# Each time a new signature is added, the corresponding public key must be
+# added to the verification key as well - if a key signs twice, it must be added
+# twice to the verification key. Aggregated signatures can be combined
+# arbitrarily (like addition) as long as public keys are aggregated in the same
+# way.
 #
-#   I can probably google this somehow, but bls signatures, anyone knows off the
-#   top of their head if they have to be combined one by one, or can two group
-#   signatures be combined? what happens to overlap then?
-#   Danny Ryan
-#   @djrtwo
-#   Dec 21 12:00
-#   Yeah, you can do any linear combination of signatures. but you have to
-#   remember the linear combination of pubkeys that constructed
-#   if you have two instances of a signature from pubkey p, then you need 2*p in
-#   the group pubkey
-#   because the attestation bitfield is only 1 bit per pubkey right now,
-#   attestations do not support this
-#   it could be extended to support N overlaps up to N times per pubkey if we
-#   had N bits per validator instead of 1
-#   We are shying away from this for the time being. If there end up being
-#   substantial difficulties in network layer aggregation, then adding bits
-#   to aid in supporting overlaps is one potential solution
-#   Jacek Sieka
-#   @arnetheduck
-#   Dec 21 12:02
-#   ah nice, you anticipated my followup question there :) so it's not a
-#   straight-off set union operation
-#   Danny Ryan
-#   @djrtwo
-#   Dec 21 12:02
-#   depending on the particular network level troubles we run into
-#   right
-#   aggregatng sigs and pubkeys are both just ec adds
-#   https://github.com/ethereum/py-evm/blob/d82b10ae361cde6abbac62f171fcea7809c4e3cf/eth/_utils/bls.py#L191-L202
-#   subtractions work too (i suppose this is obvious). You can linearly combine
-#   sigs or pubs in any way
-
+# In eth2, we use a single bit to record which keys have signed, thus we cannot
+# combined overlapping aggregates - ie if we have an aggregate of signatures of
+# A, B and C, and another with B, C and D, we cannot practically combine them
+# even if in theory it is possible to allow this in BLS.
 
 import
   stew/[endians2, objects, byteutils], hashes, nimcrypto/utils,

vendor/nim-stew

@@ -1 +1 @@
-Subproject commit 1c4293b3e754b5ea68a188b60b192801162cd44e
+Subproject commit cc416a2e6054a99fef5e3b4bb292a2fb87cc459e