Address review comments

beacon_chain/beacon_node.nim

@@ -25,7 +25,7 @@ import
   beacon_node_common, beacon_node_types, block_pools/block_pools_types,
   nimbus_binary_common,
   mainchain_monitor, version, ssz/[merkleization], sszdump,
-  sync_protocol, request_manager, keystore_directories, interop, statusbar,
+  sync_protocol, request_manager, keystore_management, interop, statusbar,
   sync_manager, state_transition,
   validator_duties, validator_api, attestation_aggregation
 

beacon_chain/merkle_minimal.nim

@@ -18,6 +18,14 @@ import
   ../../beacon_chain/spec/[beaconstate, datatypes, digest, helpers],
   ../../beacon_chain/ssz/merkleization
 
+# TODO
+#
+# This module currently represents a direct translation of the Python
+# code, appearing in the spec. We need to review it to ensure that it
+# doesn't duplicate any code defined in ssz.nim already.
+#
+# All tests need to be moved to the test suite.
+
 func round_step_down*(x: Natural, step: static Natural): int {.inline.} =
   ## Round the input to the previous multiple of "step"
   when (step and (step - 1)) == 0:
@@ -60,10 +68,8 @@ proc merkleTreeFromLeaves*(
         h.update zeroHashes[depth-1]
       result.nnznodes[depth].add nodeHash
 
-proc getMerkleProof*[Depth: static int](
-        tree: SparseMerkleTree[Depth],
-        index: int,
-      ): array[Depth, Eth2Digest] =
+proc getMerkleProof*[Depth: static int](tree: SparseMerkleTree[Depth],
+                                        index: int): array[Depth, Eth2Digest] =
 
   # Descend down the tree according to the bit representation
   # of the index:

beacon_chain/spec/crypto.nim

@@ -84,9 +84,6 @@ template `==`*[N, T](a: BlsValue[N, T], b: T): bool =
 template `==`*[N, T](a: T, b: BlsValue[N, T]): bool =
   a == b.blsValue
 
-template `==`*(a, b: ValidatorPrivKey): bool =
-  blscurve.SecretKey(a) == blscurve.SecretKey(b)
-
 # API
 # ----------------------------------------------------------------------
 # https://github.com/ethereum/eth2.0-specs/blob/v0.11.3/specs/phase0/beacon-chain.md#bls-signatures

beacon_chain/spec/keystore.nim

@@ -187,13 +187,21 @@ proc generateMnemonic*(words: openarray[cstring],
 
 proc deriveChildKey*(parentKey: ValidatorPrivKey,
                      index: Natural): ValidatorPrivKey =
-  doAssert derive_child_secretKey(SecretKey result,
-                                  SecretKey parentKey,
-                                  uint32 index)
+  let success = derive_child_secretKey(SecretKey result,
+                                       SecretKey parentKey,
+                                       uint32 index)
+  # TODO `derive_child_secretKey` is reporting pre-condition
+  #       failures with return values. We should turn the checks
+  #       into asserts inside the function.
+  doAssert success
 
 proc deriveMasterKey*(seed: KeySeed): ValidatorPrivKey =
-  doAssert derive_master_secretKey(SecretKey result,
-                                   seq[byte] seed)
+  let success = derive_master_secretKey(SecretKey result,
+                                        seq[byte] seed)
+  # TODO `derive_master_secretKey` is reporting pre-condition
+  #       failures with return values. We should turn the checks
+  #       into asserts inside the function.
+  doAssert success
 
 proc deriveMasterKey*(mnemonic: Mnemonic,
                       password: KeyStorePass): ValidatorPrivKey =

beacon_chain/validator_client.nim

@@ -21,7 +21,7 @@ import
   eth2_network, eth2_discovery, validator_pool, beacon_node_types,
   nimbus_binary_common,
   version, ssz/merkleization,
-  sync_manager, keystore_directories,
+  sync_manager, keystore_management,
   spec/eth2_apis/validator_callsigs_types,
   eth2_json_rpc_serialization
 

beacon_chain/validator_duties.nim

@@ -20,7 +20,7 @@ import
   # Local modules
   spec/[datatypes, digest, crypto, beaconstate, helpers, validator, network],
   conf, time, validator_pool, state_transition,
-  attestation_pool, block_pool, eth2_network, keystore_directories,
+  attestation_pool, block_pool, eth2_network, keystore_management,
   beacon_node_common, beacon_node_types, nimbus_binary_common,
   mainchain_monitor, version, ssz/merkleization, interop,
   attestation_aggregation, sync_manager, sszdump

docker/manage_testnet_hosts.nims

@@ -1,5 +1,5 @@
 import
-  strformat, os, confutils, algorightm
+  strformat, os, confutils, algorithm
 
 type
   Command = enum

tests/test_keystore.nim

@@ -9,11 +9,14 @@
 
 import
   unittest, ./testutil, json,
-  stew/byteutils,
+  stew/byteutils, blscurve,
   ../beacon_chain/spec/[crypto, keystore]
 
 from strutils import replace
 
+template `==`*(a, b: ValidatorPrivKey): bool =
+  blscurve.SecretKey(a) == blscurve.SecretKey(b)
+
 const
   scryptVector = """{
     "crypto": {