clean up dump feature

* don't write blocks that get added to database
* don't write states
* write to folders
* add state dumping feature to `ncli_db` to get any known state from the
database

beacon_chain/beacon_node.nim

@@ -287,6 +287,22 @@ proc onAttestation(node: BeaconNode, attestation: Attestation) =
 
   node.attestationPool.add(attestation)
 
+proc dumpBlock[T](
+    node: BeaconNode, signedBlock: SignedBeaconBlock,
+    res: Result[T, BlockError]) =
+  if node.config.dumpEnabled and res.isErr:
+    case res.error
+    of Invalid:
+      dump(
+        node.config.dumpDirInvalid, signedBlock,
+        hash_tree_root(signedBlock.message))
+    of MissingParent:
+      dump(
+        node.config.dumpDirIncoming, signedBlock,
+        hash_tree_root(signedBlock.message))
+    else:
+      discard
+
 proc storeBlock(
     node: BeaconNode, signedBlock: SignedBeaconBlock): Result[void, BlockError] =
   let blockRoot = hash_tree_root(signedBlock.message)
@@ -296,28 +312,16 @@ proc storeBlock(
     cat = "block_listener",
     pcs = "receive_block"
 
-  if node.config.dumpEnabled:
-    dump(node.config.dumpDir / "incoming", signedBlock, blockRoot)
-
   beacon_blocks_received.inc()
   let blck = node.blockPool.add(blockRoot, signedBlock)
+
+  node.dumpBlock(signedBlock, blck)
+
   if blck.isErr:
-    if blck.error == Invalid and node.config.dumpEnabled:
-      dump(node.config.dumpDir / "invalid", signedBlock, blockRoot)
-
-      let parent = node.blockPool.getRef(signedBlock.message.parent_root)
-      if parent != nil:
-        let parentBs = parent.atSlot(signedBlock.message.slot - 1)
-        node.blockPool.withState(node.blockPool.tmpState, parentBs):
-            dump(node.config.dumpDir / "invalid", hashedState, parent)
-
     return err(blck.error)
 
   # The block we received contains attestations, and we might not yet know about
-  # all of them. Let's add them to the attestation pool - in case the block
+  # all of them. Let's add them to the attestation pool.
-  # is not yet resolved, neither will the attestations be!
-  # But please note that we only care about recent attestations.
-  # TODO shouldn't add attestations if the block turns out to be invalid..
   let currentSlot = node.beaconClock.now.toSlot
   if currentSlot.afterGenesis and
     signedBlock.message.slot.epoch + 1 >= currentSlot.slot.epoch:
@@ -770,7 +774,11 @@ proc run*(node: BeaconNode) =
       let (afterGenesis, slot) = node.beaconClock.now.toSlot()
       if not afterGenesis:
         return false
-      node.blockPool.isValidBeaconBlock(signedBlock, slot, {})
+
+      let blck = node.blockPool.isValidBeaconBlock(signedBlock, slot, {})
+      node.dumpBlock(signedBlock, blck)
+
+      blck.isOk
 
     installAttestationHandlers(node)
 
@@ -1080,10 +1088,7 @@ programMain:
 
     createPidFile(config.dataDir.string / "beacon_node.pid")
 
-    if config.dumpEnabled:
+    config.createDumpDirs()
-      createDir(config.dumpDir)
-      createDir(config.dumpDir / "incoming")
-      createDir(config.dumpDir / "invalid")
 
     var node = waitFor BeaconNode.init(config)
 

beacon_chain/block_pool.nim

@@ -171,7 +171,8 @@ proc updateStateData*(pool: BlockPool, state: var StateData, bs: BlockSlot) =
 proc loadTailState*(pool: BlockPool): StateData =
   loadTailState(pool.dag)
 
-proc isValidBeaconBlock*(pool: var BlockPool,
+proc isValidBeaconBlock*(
-                         signed_beacon_block: SignedBeaconBlock,
+    pool: var BlockPool, signed_beacon_block: SignedBeaconBlock,
-                         current_slot: Slot, flags: UpdateFlags): bool =
+    current_slot: Slot, flags: UpdateFlags): Result[void, BlockError] =
-  isValidBeaconBlock(pool.dag, pool.quarantine, signed_beacon_block, current_slot, flags)
+  isValidBeaconBlock(
+    pool.dag, pool.quarantine, signed_beacon_block, current_slot, flags)

beacon_chain/block_pools/clearance.nim

@@ -259,7 +259,7 @@ proc add*(
 proc isValidBeaconBlock*(
        dag: CandidateChains, quarantine: var Quarantine,
        signed_beacon_block: SignedBeaconBlock, current_slot: Slot,
-       flags: UpdateFlags): bool =
+       flags: UpdateFlags): Result[void, BlockError] =
   logScope:
     topics = "clearance valid_blck"
     received_block = shortLog(signed_beacon_block.message)
@@ -276,7 +276,7 @@ proc isValidBeaconBlock*(
   if not (signed_beacon_block.message.slot <= current_slot + 1):
     debug "block is from a future slot",
       current_slot
-    return false
+    return err(Invalid)
 
   # The block is from a slot greater than the latest finalized slot (with a
   # MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance) -- i.e. validate that
@@ -284,7 +284,7 @@ proc isValidBeaconBlock*(
   # compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)
   if not (signed_beacon_block.message.slot > dag.finalizedHead.slot):
     debug "block is not from a slot greater than the latest finalized slot"
-    return false
+    return err(Invalid)
 
   # The block is the first block with valid signature received for the proposer
   # for the slot, signed_beacon_block.message.slot.
@@ -324,7 +324,7 @@ proc isValidBeaconBlock*(
       debug "block isn't first block with valid signature received for the proposer",
         blckRef = slotBlockRef,
         existing_block = shortLog(blck.message)
-      return false
+      return err(Invalid)
 
   # If this block doesn't have a parent we know about, we can't/don't really
   # trace it back to a known-good state/checkpoint to verify its prevenance;
@@ -347,7 +347,7 @@ proc isValidBeaconBlock*(
     debug "parent unknown, putting block in quarantine"
     quarantine.pending[hash_tree_root(signed_beacon_block.message)] =
       signed_beacon_block
-    return false
+    return err(MissingParent)
 
   # The proposer signature, signed_beacon_block.signature, is valid with
   # respect to the proposer_index pubkey.
@@ -356,13 +356,13 @@ proc isValidBeaconBlock*(
 
   if proposer.isNone:
     notice "cannot compute proposer for message"
-    return false
+    return err(Invalid)
 
   if proposer.get()[0] !=
       ValidatorIndex(signed_beacon_block.message.proposer_index):
     debug "block had unexpected proposer",
       expected_proposer = proposer.get()[0]
-    return false
+    return err(Invalid)
 
   if not verify_block_signature(
       dag.headState.data.data.fork,
@@ -374,6 +374,6 @@ proc isValidBeaconBlock*(
     debug "block failed signature verification",
       signature = shortLog(signed_beacon_block.signature)
 
-    return false
+    return err(Invalid)
 
-  true
+  ok()

beacon_chain/conf.nim

@@ -324,6 +324,25 @@ proc defaultDataDir*(conf: BeaconNodeConf|ValidatorClientConf): string =
 func dumpDir*(conf: BeaconNodeConf|ValidatorClientConf): string =
   conf.dataDir / "dump"
 
+func dumpDirInvalid*(conf: BeaconNodeConf|ValidatorClientConf): string =
+  conf.dumpDir / "invalid" # things that failed validation
+
+func dumpDirIncoming*(conf: BeaconNodeConf|ValidatorClientConf): string =
+  conf.dumpDir / "incoming" # things that couldn't be validated (missingparent etc)
+
+func dumpDirOutgoing*(conf: BeaconNodeConf|ValidatorClientConf): string =
+  conf.dumpDir / "outgoing" # things we produced
+
+proc createDumpDirs*(conf: BeaconNodeConf) =
+  if conf.dumpEnabled:
+    try:
+      createDir(conf.dumpDirInvalid)
+      createDir(conf.dumpDirIncoming)
+      createDir(conf.dumpDirOutgoing)
+    except CatchableError as err:
+      # Dumping is mainly a debugging feature, so ignore these..
+      warn "Cannot create dump directories", msg = err.msg
+
 func validatorsDir*(conf: BeaconNodeConf|ValidatorClientConf): string =
   string conf.validatorsDirFlag.get(InputDir(conf.dataDir / "validators"))
 

beacon_chain/validator_duties.nim

@@ -122,7 +122,7 @@ proc createAndSendAttestation(node: BeaconNode,
   node.sendAttestation(attestation)
 
   if node.config.dumpEnabled:
-    dump(node.config.dumpDir, attestation.data, validator.pubKey)
+    dump(node.config.dumpDirOutgoing, attestation.data, validator.pubKey)
 
   info "Attestation sent",
     attestation = shortLog(attestation),
@@ -221,10 +221,7 @@ proc proposeSignedBlock*(node: BeaconNode,
     cat = "consensus"
 
   if node.config.dumpEnabled:
-    dump(node.config.dumpDir, newBlock, newBlockRef[])
+    dump(node.config.dumpDirOutgoing, newBlock, newBlockRef[])
-    node.blockPool.withState(
-        node.blockPool.tmpState, newBlockRef[].atSlot(newBlockRef[].slot)):
-      dump(node.config.dumpDir, hashedState, newBlockRef[])
 
   node.network.broadcast(node.topicBeaconBlocks, newBlock)
 

ncli/ncli_db.nim

@@ -18,6 +18,7 @@ type
   DbCmd* = enum
     bench
     dumpState
+    rewindState
 
   DbConf = object
     databaseDir* {.
@@ -40,6 +41,15 @@ type
         argument
         desc: "State roots to save".}: seq[string]
 
+    of rewindState:
+      blockRoot* {.
+        argument
+        desc: "Block root".}: string
+
+      slot* {.
+        argument
+        desc: "Slot".}: uint64
+
 proc cmdBench(conf: DbConf) =
   var timers: array[Timers, RunningStat]
 
@@ -104,6 +114,28 @@ proc cmdDumpState(conf: DbConf) =
     except CatchableError as e:
       echo "Couldn't load ", stateRoot, ": ", e.msg
 
+proc cmdRewindState(conf: DbConf) =
+  echo "Opening database..."
+  let
+    db = BeaconChainDB.init(
+      kvStore SqStoreRef.init(conf.databaseDir.string, "nbc").tryGet())
+
+  if not BlockPool.isInitialized(db):
+    echo "Database not initialized"
+    quit 1
+
+  echo "Initializing block pool..."
+  let pool = BlockPool.init(db, {})
+
+  let blckRef = pool.getRef(fromHex(Eth2Digest, conf.blockRoot))
+  if blckRef == nil:
+    echo "Block not found in database"
+    return
+
+  pool.withState(pool.tmpState, blckRef.atSlot(Slot(conf.slot))):
+    echo "Writing state..."
+    dump("./", hashedState, blck)
+
 when isMainModule:
   let
     conf = DbConf.load()
@@ -113,3 +145,5 @@ when isMainModule:
     cmdBench(conf)
   of dumpState:
     cmdDumpState(conf)
+  of rewindState:
+    cmdRewindState(conf)