Consolidate modules by areas [part 1] (#2365)

* Move sync in subfolder

* move validator related thingies in validators

* fix binary builds

* update bounds comment [skip ci]

.gitignore

@@ -43,9 +43,8 @@ build/
 /docker/*.json
 
 # generated during Nim compilation
-beacon_chain/sync_protocol.nim.generated.nim
+*.nim.generated.nim
 
 /dist
 /benchmark_results
 /.update.timestamp
-

beacon_chain/beacon_node_common.nim

@@ -15,10 +15,11 @@ import
 
   # Local modules
   ./conf, ./time, ./beacon_chain_db, ./attestation_pool, ./eth2_network,
-  ./beacon_node_types, ./eth1_monitor, ./request_manager,
+  ./beacon_node_types, ./eth1_monitor,
-  ./sync_manager, ./eth2_processor,
+  ./eth2_processor,
   ./block_pools/[chain_dag, quarantine],
-  ./spec/datatypes
+  ./spec/datatypes,
+  ./sync/[sync_manager, request_manager]
 
 export
   osproc, chronos, rpcserver, conf, time, beacon_chain_db,
@@ -73,4 +74,3 @@ proc currentSlot*(node: BeaconNode): Slot =
 
 template runtimePreset*(node: BeaconNode): RuntimePreset =
   node.db.preset
-

beacon_chain/beacon_node_types.nim

@@ -6,7 +6,7 @@ import
   spec/[datatypes, digest, crypto],
   block_pools/block_pools_types,
   fork_choice/fork_choice_types,
-  validator_protection/slashing_protection
+  validators/slashing_protection
 
 from libp2p/protocols/pubsub/pubsub import ValidationResult
 

beacon_chain/deposit_contract.nim

@@ -3,7 +3,8 @@ import
   chronos, chronicles, confutils, stint, json_serialization,
   ../beacon_chain/network_metadata,
   web3, web3/confutils_defs, eth/keys, stew/io2, filepath,
-  spec/[datatypes, crypto, presets], ssz/merkleization, keystore_management
+  spec/[datatypes, crypto, presets], ssz/merkleization,
+  ./validators/keystore_management
 
 # Compiled version of /scripts/depositContract.v.py in this repo
 # The contract was compiled in Remix (https://remix.ethereum.org/) with vyper (remote) compiler.

beacon_chain/eth2_network.nim

@@ -23,8 +23,9 @@ import
   eth/net/nat, eth/p2p/discoveryv5/[enr, node],
   "."/[
     version, conf, eth2_discovery, libp2p_json_serialization,
-    ssz/ssz_serialization, peer_pool, time, keystore_management],
+    ssz/ssz_serialization, peer_pool, time],
-  ./spec/[datatypes, digest, helpers, network]
+  ./spec/[datatypes, digest, helpers, network],
+  ./validators/keystore_management
 
 import libp2p/protocols/pubsub/gossipsub
 

beacon_chain/eth2_processor.nim

@@ -13,7 +13,8 @@ import
   chronicles, chronos, metrics,
   ./spec/[crypto, datatypes, digest],
   ./block_pools/[clearance, chain_dag],
-  ./attestation_aggregation, ./exit_pool, ./validator_pool,
+  ./attestation_aggregation, ./exit_pool,
+  ./validators/validator_pool,
   ./beacon_node_types, ./attestation_pool,
   ./time, ./conf, ./sszdump
 

beacon_chain/nimbus_beacon_node.nim

@@ -26,10 +26,11 @@ import
     attestation_aggregation, attestation_pool, beacon_chain_db,
     beacon_node_common, beacon_node_status, beacon_node_types, conf,
     eth1_monitor, eth2_discovery, eth2_network, eth2_processor, exit_pool,
-    extras, filepath, interop, keystore_management, network_metadata,
+    extras, filepath, interop, network_metadata,
-    nimbus_binary_common, request_manager, ssz/merkleization, statusbar,
+    nimbus_binary_common, ssz/merkleization, statusbar,
-    sync_manager, sync_protocol, time, validator_duties, validator_pool,
+    time, version],
-    validator_protection/slashing_protection, version,],
+  ./validators/[validator_duties, validator_pool, slashing_protection, keystore_management],
+  ./sync/[sync_manager, sync_protocol, request_manager],
   ./rpc/[beacon_api, config_api, debug_api, event_api, nimbus_api, node_api,
     validator_api],
   ./spec/[

beacon_chain/nimbus_signing_process.nim

@@ -11,7 +11,7 @@ import
 
   # Local modules
   spec/[digest, crypto],
-  keystore_management
+  validators/keystore_management
 
 programMain:
   var validators: Table[ValidatorPubKey, ValidatorPrivKey]

beacon_chain/nimbus_validator_client.nim

@@ -18,15 +18,15 @@ import
   # Local modules
   spec/[datatypes, digest, crypto, helpers, network, signatures],
   spec/eth2_apis/beacon_rpc_client,
+  sync/sync_manager,
   conf, time, version,
-  eth2_network, eth2_discovery, validator_pool, beacon_node_types,
+  eth2_network, eth2_discovery, beacon_node_types,
   attestation_aggregation,
   nimbus_binary_common,
   version, ssz/merkleization,
-  sync_manager, keystore_management,
   spec/eth2_apis/callsigs_types,
   eth2_json_rpc_serialization,
-  validator_protection/slashing_protection,
+  validators/[keystore_management, validator_pool, slashing_protection],
   eth/db/[kvstore, kvstore_sqlite3]
 
 logScope: topics = "vc"

beacon_chain/rpc/beacon_api.nim

@@ -11,7 +11,7 @@ import
   chronicles,
   nimcrypto/utils as ncrutils,
   ../beacon_node_common, ../eth2_json_rpc_serialization, ../eth2_network,
-  ../validator_duties,
+  ../validators/validator_duties,
   ../block_pools/chain_dag, ../exit_pool,
   ../spec/[crypto, digest, datatypes, validator, network],
   ../spec/eth2_apis/callsigs_types,

beacon_chain/rpc/nimbus_api.nim

@@ -13,7 +13,7 @@ import
 
   rpc_utils,
   ../beacon_node_common, ../nimbus_binary_common, ../eth2_network,
-  ../eth1_monitor, ../validator_duties,
+  ../eth1_monitor, ../validators/validator_duties,
   ../spec/[digest, datatypes, presets],
 
   libp2p/protocols/pubsub/pubsubpeer

beacon_chain/rpc/node_api.nim

@@ -4,7 +4,7 @@ import std/options,
   eth/p2p/discoveryv5/enr,
   libp2p/[multiaddress, multicodec],
   nimcrypto/utils as ncrutils,
-  ../beacon_node_common, ../eth2_network, ../sync_manager,
+  ../beacon_node_common, ../eth2_network, ../sync/sync_manager,
   ../peer_pool, ../version,
   ../spec/[datatypes, digest, presets],
   ../spec/eth2_apis/callsigs_types

beacon_chain/rpc/rpc_utils.nim

@@ -1,7 +1,7 @@
 import
   std/[strutils, parseutils],
   stew/byteutils,
-  ../beacon_node_common, ../validator_duties,
+  ../beacon_node_common, ../validators/validator_duties,
   ../block_pools/[block_pools_types, chain_dag],
   ../spec/[datatypes, digest, helpers]
 

beacon_chain/rpc/validator_api.nim

@@ -19,7 +19,7 @@ import
   ../spec/eth2_apis/callsigs_types,
   ../block_pools/[chain_dag, spec_cache], ../ssz/merkleization,
   ../beacon_node_common, ../beacon_node_types, ../attestation_pool,
-  ../validator_duties, ../eth2_network,
+  ../validators/validator_duties, ../eth2_network,
   ../eth2_json_rpc_serialization,
   ./rpc_utils
 

beacon_chain/sync/README.md

@@ -0,0 +1,69 @@
+# Block syncing
+
+This folder holds all modules related to block syncing
+
+Block syncing uses ETH2 RPC protocol.
+
+Reference diagram
+
+![Block flow](../../docs/block_flow.png)
+
+## Eth2 RPC in
+
+Blocks are requested during sync by the SyncManager.
+
+Blocks are received by batch:
+- `syncStep(SyncManager, index, peer)`
+- in case of success:
+  - `push(SyncQueue, SyncRequest, seq[SignedBeaconBlock]) is called to handle a successful sync step.
+    It calls `validate(SyncQueue, SignedBeaconBlock)` on each block retrieved one-by-one
+  - `validate` only enqueues the block in the SharedBlockQueue `AsyncQueue[BlockEntry]` but does no extra validation only the GossipSub case
+- in case of failure:
+  - `push(SyncQueue, SyncRequest)` is called to reschedule the sync request.
+
+Every second when sync is not in progress, the beacon node will ask the RequestManager to download all missing blocks currently in quarantaine.
+- via `handleMissingBlocks`
+- which calls `fetchAncestorBlocks`
+- which asynchronously enqueue the request in the SharedBlockQueue `AsyncQueue[BlockEntry]`.
+
+The RequestManager runs an event loop:
+- that calls `fetchAncestorBlocksFromNetwork`
+- which RPC calls peers with `beaconBlocksByRoot`
+- and calls `validate(RequestManager, SignedBeaconBlock)` on each block retrieved one-by-one
+- `validate` only enqueues the block in the `AsyncQueue[BlockEntry]` but does no extra validation only the GossipSub case
+
+## Weak subjectivity sync
+
+Not implemented!
+
+## Comments
+
+The `validate` procedure name for `SyncManager` and `RequestManager`
+as no P2P validation actually occurs.
+
+## Sync vs Steady State
+
+During sync:
+- The RequestManager is deactivated
+- The syncManager is working full speed ahead
+- Gossip is deactivated
+
+## Bottlenecks during sync
+
+During sync:
+- The bottleneck is clearing the SharedBlockQueue `AsyncQueue[BlockEntry]` via `storeBlock`
+  which requires full verification (state transition + cryptography)
+
+## Backpressure
+
+The SyncManager handles backpressure by ensuring that
+`current_queue_slot <= request.slot <= current_queue_slot + sq.queueSize * sq.chunkSize`.
+- queueSize is -1, unbounded, by default according to comment but all init paths uses 1 (?)
+- chunkSize is SLOTS_PER_EPOCH = 32
+
+However the shared `AsyncQueue[BlockEntry]` itself is unbounded.
+Concretely:
+- The shared `AsyncQueue[BlockEntry]` is bounded for sync
+- The shared `AsyncQueue[BlockEntry]` is unbounded for validated gossip blocks
+
+RequestManager and Gossip are deactivated during sync and so do not contribute to pressure.

beacon_chain/sync/request_manager.nim

@@ -1,7 +1,9 @@
 import options, sequtils, strutils
 import chronos, chronicles
-import spec/[datatypes, digest], eth2_network, beacon_node_types, sync_protocol,
+import ../spec/[datatypes, digest],
-       sync_manager, ssz/merkleization, ./eth2_processor
+       ".."/[eth2_network, beacon_node_types,
+           ssz/merkleization, eth2_processor],
+       ./sync_protocol, ./sync_manager
 export sync_manager
 
 logScope:

beacon_chain/sync/sync_manager.nim

@@ -1,10 +1,11 @@
 import chronicles
 import options, deques, heapqueue, tables, strutils, sequtils, math, algorithm
 import stew/results, chronos, chronicles
-import spec/[datatypes, digest, helpers, eth2_apis/callsigs_types], peer_pool, eth2_network
+import ../spec/[datatypes, digest, helpers, eth2_apis/callsigs_types],
+       ../peer_pool, ../eth2_network
 
-import ./eth2_processor
+import ../eth2_processor
-import block_pools/block_pools_types
+import ../block_pools/block_pools_types
 export datatypes, digest, chronos, chronicles, results, block_pools_types
 
 logScope:

beacon_chain/sync/sync_protocol.nim

@@ -1,9 +1,9 @@
 import
   options, tables, sets, macros,
   chronicles, chronos, stew/ranges/bitranges, libp2p/switch,
-  spec/[datatypes, network, crypto, digest],
+  ../spec/[datatypes, network, crypto, digest],
-  beacon_node_types, eth2_network,
+  ".."/[beacon_node_types, eth2_network,
-  block_pools/chain_dag
+      block_pools/chain_dag]
 
 logScope:
   topics = "sync"

beacon_chain/validators/README.md

@@ -0,0 +1,7 @@
+# Validators
+
+This folder holds all modules related to a Beacon Chain Validator besides the binaries they interact directly with (nimbus_validator_cliant and nimbus_signing_process):
+- Validator keystore
+- Validator slashing protection
+- Validator duties
+- Validator pool

beacon_chain/validators/keystore_management.nim

@@ -2,10 +2,10 @@ import
   std/[os, strutils, terminal, wordwrap, unicode],
   chronicles, chronos, web3, stint, json_serialization, zxcvbn,
   serialization, blscurve, eth/common/eth_types, eth/keys, confutils, bearssl,
-  spec/[datatypes, digest, crypto, keystore],
+  ../spec/[datatypes, digest, crypto, keystore],
   stew/io2, libp2p/crypto/crypto as lcrypto,
   nimcrypto/utils as ncrutils,
-  "."/[conf, ssz/merkleization, network_metadata, filepath]
+  ".."/[conf, ssz/merkleization, network_metadata, filepath]
 
 export
   keystore
@@ -35,7 +35,7 @@ const
 
   mostCommonPasswords = wordListArray(
     currentSourcePath.parentDir /
-      "../vendor/nimbus-security-resources/passwords/10-million-password-list-top-100000.txt",
+      "../../vendor/nimbus-security-resources/passwords/10-million-password-list-top-100000.txt",
     minWordLen = minPasswordLen)
 
 proc echoP*(msg: string) =

beacon_chain/validators/validator_duties.nim

@@ -18,16 +18,17 @@ import
   eth/[keys, async_utils], eth/p2p/discoveryv5/[protocol, enr],
 
   # Local modules
-  ./spec/[
+  ../spec/[
     datatypes, digest, crypto, helpers, network, signatures, state_transition,
     validator],
-  ./conf, ./time, ./validator_pool,
+  ../conf, ../time,
-  ./attestation_pool, ./exit_pool,
+  ../attestation_pool, ../exit_pool,
-  ./block_pools/[spec_cache, chain_dag, clearance],
+  ../block_pools/[spec_cache, chain_dag, clearance],
-  ./eth2_network, ./keystore_management, ./beacon_node_common,
+  ../eth2_network, ../beacon_node_common,
-  ./beacon_node_types, ./nimbus_binary_common, ./eth1_monitor, ./version,
+  ../beacon_node_types, ../nimbus_binary_common, ../eth1_monitor, ../version,
-  ./ssz/merkleization, ./attestation_aggregation, ./sync_manager, ./sszdump,
+  ../ssz/merkleization, ../attestation_aggregation, ../sync/sync_manager, ../sszdump,
-  ./validator_protection/slashing_protection
+  ./slashing_protection,
+  ./validator_pool, ./keystore_management
 
 # Metrics for tracking attestation and beacon block loss
 const delayBuckets = [-Inf, -4.0, -2.0, -1.0, -0.5, -0.1, -0.05,

beacon_chain/validators/validator_pool.nim

@@ -3,8 +3,9 @@ import
   chronos, chronicles, metrics,
   json_serialization/std/[sets, net],
   eth/db/[kvstore, kvstore_sqlite3],
-  ./spec/[datatypes, crypto, digest, signatures, helpers],
+  ../spec/[datatypes, crypto, digest, signatures, helpers],
-  ./beacon_node_types, validator_protection/slashing_protection
+  ../beacon_node_types,
+  ./slashing_protection
 
 declareGauge validators,
   "Number of validators attached to the beacon node"

docs/block_flow.md

@@ -120,33 +120,10 @@ Logs:
 - We jump into libp2p/protocols/pubsub/pubsub.nim in the method `validate(PubSub, message)`
 - which was called by `rpcHandler(GossipSub, PubSubPeer, RPCMsg)`
 
-### Eth2 RPC in
+### Block syncing
 
-Blocks are requested during sync by the SyncManager.
+See [Block syncing](../beacon_chain/sync/README.md)
-
+for flow explanation and performance characteristics.
-Blocks are received by batch:
-- `syncStep(SyncManager, index, peer)`
-- in case of success:
-  - `push(SyncQueue, SyncRequest, seq[SignedBeaconBlock]) is called to handle a successful sync step.
-    It calls `validate(SyncQueue, SignedBeaconBlock)` on each block retrieved one-by-one
-  - `validate` only enqueues the block in the `AsyncQueue[BlockEntry]` but does no extra validation only the GossipSub case
-- in case of failure:
-  - `push(SyncQueue, SyncRequest)` is called to reschedule the sync request.
-
-Every second when sync is not in progress, the beacon node will ask the RequestManager to download all missing blocks currently in quarantaine.
-- via `handleMissingBlocks`
-- which calls `fetchAncestorBlocks`
-- which asynchronously enqueue the request
-
-The RequestManager runs an event loop:
-- that calls `fetchAncestorBlocksFromNetwork`
-- which RPC calls peers with `beaconBlocksByRoot`
-- and calls `validate(RequestManager, SignedBeaconBlock)` on each block retrieved one-by-one
-- `validate` only enqueues the block in the `AsyncQueue[BlockEntry]` but does no extra validation only the GossipSub case
-
-### Weak subjectivity sync
-
-Not implemented!
 
 ### Consuming the shared block queue
 
@@ -159,45 +136,7 @@ This in turn calls:
 - `addRawBlock(ChainDagRef, QuarantineRef, SignedBeaconBlock, forkChoiceCallback)`
 - trigger sending attestation if relevant
 
-### Comments
+### Steady state (synced to head)
-
-The `validate` procedure name for `SyncManager` and `RequestManager`
-as no P2P validation actually occurs.
-
-Furthermore it seems like we are not gossiping blocks as soon as they are validated. TODO: When do we gossip them? Are we waiting for full verification.
-
-### Sync vs Steady State
-
-During sync:
-- The RequestManager is deactivated
-- The syncManager is working full speed ahead
-- Gossip is deactivated
-
-#### Bottlenecks during sync
-
-During sync:
-- The bottleneck is clearing the SharedBlockQueue via `storeBlock`
-  which requires full verification (state transition + cryptography)
-
-##### Backpressure
-
-The SyncManager handles backpressure by ensuring that
-`current_queue_slot <= request.slot <= current_queue_slot + sq.queueSize * sq.chunkSize`.
-- queueSize is -1 unbounded by default according to comment but it seems like all init path uses 1 (?)
-- chunkSize is SLOTS_PER_EPOCH = 32
-
-However the shared `AsyncQueue[BlockEntry]` is unbounded (?) with.
-
-RequestManager and Gossip are deactivated during sync and so do not contribute to pressure
-
-##### Latency & Throughput sensitiveness
-
-Latency TODO
-
-Throughput:
-- storeBlock is bottlenecked by verification (state transition and cryptography) of a block.
-  It might also be bottlenecked by the BeaconChainDB database speed.
-
 #### Bottlenecks when synced
 
 The SyncManager is deactivated

ncli/ncli_slashing.nim

@@ -11,7 +11,7 @@ import
   std/[os, strutils],
   confutils,
   eth/db/[kvstore, kvstore_sqlite3],
-  ../beacon_chain/validator_protection/slashing_protection,
+  ../beacon_chain/validators/slashing_protection,
   ../beacon_chain/spec/digest
 
 type

research/block_sim.nim

@@ -24,7 +24,8 @@ import
                         helpers, validator, signatures, state_transition],
   ../beacon_chain/[
     attestation_pool, beacon_node_types, beacon_chain_db,
-    validator_pool, eth1_monitor, extras],
+    eth1_monitor, extras],
+  ../beacon_chain/validators/validator_pool,
   ../beacon_chain/block_pools/[chain_dag, quarantine, clearance],
   ../beacon_chain/ssz/[merkleization, ssz_serialization],
   ./simutils

tests/slashing_protection/test_migration.nim

@@ -17,7 +17,7 @@ import
   serialization,
   json_serialization,
   # Internal
-  ../../beacon_chain/validator_protection/[
+  ../../beacon_chain/validators/[
     slashing_protection,
     slashing_protection_v1,
     slashing_protection_v2

tests/slashing_protection/test_official_interchange_vectors.nim

@@ -13,7 +13,7 @@ import
   nimcrypto/utils,
   chronicles,
   # Internal
-  ../../beacon_chain/validator_protection/slashing_protection,
+  ../../beacon_chain/validators/slashing_protection,
   ../../beacon_chain/spec/[datatypes, digest, crypto, presets],
   # Test utilies
   ../testutil,

tests/slashing_protection/test_slashing_interchange.nim

@@ -15,7 +15,7 @@ import
   stew/results,
   nimcrypto/utils,
   # Internal
-  ../../beacon_chain/validator_protection/slashing_protection,
+  ../../beacon_chain/validators/slashing_protection,
   ../../beacon_chain/spec/[datatypes, digest, crypto, presets],
   # Test utilies
   ../testutil

tests/slashing_protection/test_slashing_protection_db.nim

@@ -14,7 +14,7 @@ import
   eth/db/kvstore,
   stew/results,
   # Internal
-  ../../beacon_chain/validator_protection/slashing_protection,
+  ../../beacon_chain/validators/slashing_protection,
   ../../beacon_chain/spec/[datatypes, digest, crypto, presets],
   # Test utilies
   ../testutil

tests/test_sync_manager.nim

@@ -2,7 +2,8 @@
 
 import unittest
 import chronos
-import ../beacon_chain/[eth2_processor, sync_manager]
+import ../beacon_chain/eth2_processor,
+       ../beacon_chain/sync/sync_manager
 
 type
   SomeTPeer = ref object

tests/test_sync_protocol.nim

@@ -10,7 +10,7 @@
 import unittest, ./testutil
 
 when false:
-  import ../beacon_chain/sync_protocol
+  import ../beacon_chain/sync/sync_protocol
 
 suiteReport "Sync protocol":
   # Compile test

tests/testblockutil.nim

@@ -7,7 +7,8 @@
 
 import
   options, stew/endians2,
-  ../beacon_chain/[extras, validator_pool],
+  ../beacon_chain/extras,
+  ../beacon_chain/validators/validator_pool,
   ../beacon_chain/ssz/merkleization,
   ../beacon_chain/spec/[beaconstate, crypto, datatypes, digest, presets,
                         helpers, validator, signatures, state_transition]