add blockchain_dag altair database reading; add rollback tests (#2683)

* add blockchain_dag altair database reading; add rollback tests; fix some unnecessary type conversions

* remove debugging scaffolding

* proposeSignedBlock() will need to be async for merge; introduce altair types to VC

AllTests-mainnet.md

@@ -82,16 +82,18 @@ OK: 11/11 Fail: 0/11 Skip: 0/11
 ```diff
 + empty database [Preset: mainnet]                                                           OK
 + find ancestors [Preset: mainnet]                                                           OK
++ sanity check Altair and cross-fork getState rollback [Preset: mainnet]                     OK
 + sanity check Altair blocks [Preset: mainnet]                                               OK
 + sanity check Altair states [Preset: mainnet]                                               OK
 + sanity check Altair states, reusing buffers [Preset: mainnet]                              OK
 + sanity check genesis roundtrip [Preset: mainnet]                                           OK
 + sanity check phase 0 blocks [Preset: mainnet]                                              OK
++ sanity check phase 0 getState rollback [Preset: mainnet]                                   OK
 + sanity check phase 0 states [Preset: mainnet]                                              OK
 + sanity check phase 0 states, reusing buffers [Preset: mainnet]                             OK
 + sanity check state diff roundtrip [Preset: mainnet]                                        OK
 ```
-OK: 10/10 Fail: 0/10 Skip: 0/10
+OK: 12/12 Fail: 0/12 Skip: 0/12
 ## Beacon state [Preset: mainnet]
 ```diff
 + Smoke test initialize_beacon_state_from_eth1 [Preset: mainnet]                             OK
@@ -311,4 +313,4 @@ OK: 3/3 Fail: 0/3 Skip: 0/3
 OK: 1/1 Fail: 0/1 Skip: 0/1
 
 ---TOTAL---
-OK: 177/185 Fail: 0/185 Skip: 8/185
+OK: 179/187 Fail: 0/187 Skip: 8/187

beacon_chain/beacon_chain_db.nim

@@ -344,7 +344,7 @@ proc new*(T: type BeaconChainDB,
     altair_blocks: altair_blocks,
     stateRoots: stateRoots,
     statesNoVal: statesNoVal,
-    altairStatesNoVal: statesNoVal,
+    altairStatesNoVal: altairStatesNoVal,
     stateDiffs: stateDiffs,
     summaries: summaries,
   )
@@ -514,6 +514,13 @@ proc putState*(
     db: BeaconChainDB, value: phase0.BeaconState | altair.BeaconState) =
   db.putState(hash_tree_root(value), value)
 
+# For testing rollback
+proc putCorruptPhase0State*(db: BeaconChainDB, key: Eth2Digest) =
+  db.statesNoVal.putSnappySSZ(key.data, Validator())
+
+proc putCorruptAltairState*(db: BeaconChainDB, key: Eth2Digest) =
+  db.altairStatesNoVal.putSnappySSZ(key.data, Validator())
+
 func stateRootKey(root: Eth2Digest, slot: Slot): array[40, byte] =
   var ret: array[40, byte]
   # big endian to get a naturally ascending order on slots in sorted indices
@@ -626,13 +633,13 @@ proc getStateOnlyMutableValidators(
   of GetResult.notFound:
     false
   of GetResult.corrupted:
-    rollback(output)
+    rollback()
     false
 
 proc getAltairStateOnlyMutableValidators(
     immutableValidators: openArray[ImmutableValidatorData2],
     store: KvStoreRef, key: openArray[byte], output: var altair.BeaconState,
-    rollback: AltairRollbackProc): bool =
+    rollback: RollbackProc): bool =
   ## Load state into `output` - BeaconState is large so we want to avoid
   ## re-allocating it if possible
   ## Return `true` iff the entry was found in the database and `output` was
@@ -668,7 +675,7 @@ proc getAltairStateOnlyMutableValidators(
   of GetResult.notFound:
     false
   of GetResult.corrupted:
-    rollback(output)
+    rollback()
     false
 
 proc getState(
@@ -697,7 +704,7 @@ proc getState(
   of GetResult.notFound:
     false
   of GetResult.corrupted:
-    rollback(output)
+    rollback()
     false
 
 proc getState*(
@@ -721,7 +728,7 @@ proc getState*(
 
 proc getAltairState*(
     db: BeaconChainDB, key: Eth2Digest, output: var altair.BeaconState,
-    rollback: AltairRollbackProc): bool =
+    rollback: RollbackProc): bool =
   ## Load state into `output` - BeaconState is large so we want to avoid
   ## re-allocating it if possible
   ## Return `true` iff the entry was found in the database and `output` was

beacon_chain/consensus_object_pools/blockchain_dag.nim

@@ -510,11 +510,23 @@ proc getState(
     else:
       unsafeAddr dag.headState
 
-  func restore(v: var phase0.BeaconState) =
+  let v = addr state.data
-    assign(v, restoreAddr[].data.hbsPhase0.data)
 
-  if not dag.db.getState(stateRoot, state.data.hbsPhase0.data, restore):
+  func restore() =
-    return false
+    assign(v[], restoreAddr[].data)
+
+  if blck.slot < dag.altairTransitionSlot:
+    if state.data.beaconStateFork != forkPhase0:
+      state.data = (ref ForkedHashedBeaconState)(beaconStateFork: forkPhase0)[]
+
+    if not dag.db.getState(stateRoot, state.data.hbsPhase0.data, restore):
+      return false
+  else:
+    if state.data.beaconStateFork != forkAltair:
+      state.data = (ref ForkedHashedBeaconState)(beaconStateFork: forkAltair)[]
+
+    if not dag.db.getAltairState(stateRoot, state.data.hbsAltair.data, restore):
+      return false
 
   state.blck = blck
   setStateRoot(state.data, stateRoot)

beacon_chain/fork_choice/fork_choice.nim

@@ -157,7 +157,7 @@ proc process_attestation_queue(self: var ForkChoice) =
     if it.slot < self.checkpoints.time:
       for validator_index in it.attesting_indices:
         self.backend.process_attestation(
-          validator_index.ValidatorIndex, it.block_root, it.slot.epoch())
+          validator_index, it.block_root, it.slot.epoch())
       false
     else:
       true

beacon_chain/nimbus_validator_client.nim

@@ -18,7 +18,8 @@ import
   json_serialization/std/[options, net],
 
   # Local modules
-  ./spec/[datatypes, digest, crypto, helpers, network, signatures],
+  ./spec/datatypes/[phase0, altair],
+  ./spec/[digest, crypto, helpers, network, signatures],
   ./spec/eth2_apis/beacon_rpc_client,
   ./sync/sync_manager,
   "."/[conf, beacon_clock, version],
@@ -140,7 +141,7 @@ proc onSlotStart(vc: ValidatorClient, lastSlot, scheduledSlot: Slot) {.gcsafe, a
       let validator = vc.attachedValidators.validators[public_key]
       let randao_reveal = await validator.genRandaoReveal(
         vc.fork, vc.beaconGenesis.genesis_validators_root, slot)
-      var newBlock = SignedBeaconBlock(
+      var newBlock = phase0.SignedBeaconBlock(
           message: await vc.client.get_v1_validator_block(slot, vc.graffitiBytes, randao_reveal)
         )
       newBlock.root = hash_tree_root(newBlock.message)

beacon_chain/rpc/beacon_api.nim

@@ -409,7 +409,7 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {.
       # It was not integrated into the beacon node's database.
       return 202
     else:
-      let res = proposeSignedBlock(node, head, AttachedValidator(), blck)
+      let res = await proposeSignedBlock(node, head, AttachedValidator(), blck)
       if res == head:
         node.network.broadcast(getBeaconBlocksTopic(node.forkDigest), blck)
         # The block failed validation, but was successfully broadcast anyway.

beacon_chain/rpc/beacon_rest_api.nim

@@ -502,7 +502,7 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
         else:
           let idx = cindex.get()
           if uint64(idx) < committees_per_slot:
-            res.add(getCommittee(slot, CommitteeIndex(idx)))
+            res.add(getCommittee(slot, idx))
 
       var res: seq[RestBeaconStatesCommittees]
       let qepoch =
@@ -629,7 +629,7 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
       node.network.broadcast(getBeaconBlocksTopic(node.forkDigest), blck)
       return RestApiResponse.jsonError(Http202, BlockValidationError)
     else:
-      let res = proposeSignedBlock(node, head, AttachedValidator(), blck)
+      let res = await proposeSignedBlock(node, head, AttachedValidator(), blck)
       if res == head:
         node.network.broadcast(getBeaconBlocksTopic(node.forkDigest), blck)
         return RestApiResponse.jsonError(Http202, BlockValidationError)

beacon_chain/rpc/eth2_json_rest_serialization.nim

@@ -336,9 +336,9 @@ proc decodeBody*[T](t: typedesc[T],
   let data =
     try:
       RestJson.decode(cast[string](body.data), T)
-    except SerializationError as exc:
+    except SerializationError:
       return err("Unable to deserialize data")
-    except CatchableError as exc:
+    except CatchableError:
       return err("Unexpected deserialization error")
   ok(data)
 

beacon_chain/rpc/rpc_utils.nim

@@ -26,7 +26,7 @@ template withStateForStateId*(stateId: string, body: untyped): untyped =
 
   if isState(node.dag.headState):
     withStateVars(node.dag.headState):
-      var cache {.inject.}: StateCache
+      var cache {.inject, used.}: StateCache
       body
   else:
     let rpcState = assignClone(node.dag.headState)

beacon_chain/rpc/validator_api.nim

@@ -56,7 +56,7 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {.
     if head.slot >= body.message.slot:
       raise newException(CatchableError,
         "Proposal is for a past slot: " & $body.message.slot)
-    if head == proposeSignedBlock(node, head, AttachedValidator(), body):
+    if head == await proposeSignedBlock(node, head, AttachedValidator(), body):
       raise newException(CatchableError, "Could not propose block")
     return true
 

beacon_chain/spec/state_transition.nim

@@ -104,14 +104,10 @@ func verifyStateRoot(state: phase0.BeaconState, blck: altair.TrustedBeaconBlock)
   true
 
 type
-  RollbackProc* = proc(v: var phase0.BeaconState) {.gcsafe, raises: [Defect].}
+  RollbackProc* = proc() {.gcsafe, raises: [Defect].}
-  AltairRollbackProc* = proc(v: var altair.BeaconState) {.gcsafe, raises: [Defect].}
 
-func noRollback*(state: var phase0.BeaconState) =
+func noRollback*() =
-  trace "Skipping rollback of broken phase 0 state"
+  trace "Skipping rollback of broken state"
-
-func noRollback*(state: var altair.BeaconState) =
-  trace "Skipping rollback of broken Altair state"
 
 type
   RollbackHashedProc* = proc(state: var phase0.HashedBeaconState) {.gcsafe, raises: [Defect].}

beacon_chain/spec/state_transition_epoch.nim

@@ -874,7 +874,7 @@ proc process_epoch*(
   process_historical_roots_update(state)
   process_participation_record_updates(state)
 
-# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-alpha.6/specs/altair/beacon-chain.md#epoch-processing
+# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-alpha.7/specs/altair/beacon-chain.md#epoch-processing
 proc process_epoch*(
     state: var altair.BeaconState, flags: UpdateFlags, cache: var StateCache,
     rewards: var RewardInfo) {.nbench.} =

beacon_chain/validators/validator_duties.nim

@@ -110,10 +110,10 @@ proc getAttachedValidator*(node: BeaconNode,
                            idx: ValidatorIndex): AttachedValidator =
   if idx < state_validators.len.ValidatorIndex:
     let validator = node.getAttachedValidator(state_validators[idx].pubkey)
-    if validator != nil and validator.index != some(idx.ValidatorIndex):
+    if validator != nil and validator.index != some(idx):
       # Update index, in case the validator was activated!
       notice "Validator activated", pubkey = validator.pubkey, index = idx
-      validator.index  = some(idx.ValidatorIndex)
+      validator.index  = some(idx)
     validator
   else:
     warn "Validator index out of bounds",
@@ -340,7 +340,8 @@ proc makeBeaconBlockForHeadAndSlot*(node: BeaconNode,
 proc proposeSignedBlock*(node: BeaconNode,
                          head: BlockRef,
                          validator: AttachedValidator,
-                         newBlock: SignedBeaconBlock): BlockRef =
+                         newBlock: SignedBeaconBlock):
+                         Future[BlockRef] {.async.} =
   let newBlockRef = node.dag.addRawBlock(node.quarantine, newBlock) do (
       blckRef: BlockRef, trustedBlock: TrustedSignedBeaconBlock,
       epochRef: EpochRef):
@@ -420,7 +421,7 @@ proc proposeBlock(node: BeaconNode,
   newBlock.signature = await validator.signBlockProposal(
     fork, genesis_validators_root, slot, newBlock.root)
 
-  return node.proposeSignedBlock(head, validator, newBlock)
+  return await node.proposeSignedBlock(head, validator, newBlock)
 
 proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) =
   ## Perform all attestations that the validators attached to this node should

tests/test_beacon_chain_db.nim

@@ -60,8 +60,7 @@ suite "Beacon chain DB" & preset():
       db.getBlock(Eth2Digest()).isNone
 
   test "sanity check phase 0 blocks" & preset():
-    var
+    var db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true)
-      db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true)
 
     let
       signedBlock = withDigest((phase0.TrustedBeaconBlock)())
@@ -90,8 +89,7 @@ suite "Beacon chain DB" & preset():
     db.close()
 
   test "sanity check Altair blocks" & preset():
-    var
+    var db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true)
-      db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true)
 
     let
       signedBlock = withDigest((altair.TrustedBeaconBlock)())
@@ -225,6 +223,53 @@ suite "Beacon chain DB" & preset():
 
     db.close()
 
+  test "sanity check phase 0 getState rollback" & preset():
+    var
+      db = makeTestDB(SLOTS_PER_EPOCH)
+      dag = init(ChainDAGRef, defaultRuntimePreset, db)
+      state = (ref ForkedHashedBeaconState)(
+        beaconStateFork: forkPhase0,
+        hbsPhase0: phase0.HashedBeaconState(data: phase0.BeaconState(
+          slot: 10.Slot)))
+      root = Eth2Digest()
+
+    db.putCorruptPhase0State(root)
+
+    let restoreAddr = addr dag.headState
+
+    func restore() =
+      assign(state[], restoreAddr[].data)
+
+    check:
+      state[].hbsPhase0.data.slot == 10.Slot
+      not db.getState(root, state[].hbsPhase0.data, restore)
+      state[].hbsPhase0.data.slot != 10.Slot
+
+  test "sanity check Altair and cross-fork getState rollback" & preset():
+    var
+      db = makeTestDB(SLOTS_PER_EPOCH)
+      dag = init(ChainDAGRef, defaultRuntimePreset, db)
+      state = (ref ForkedHashedBeaconState)(
+        beaconStateFork: forkAltair,
+        hbsAltair: altair.HashedBeaconState(data: altair.BeaconState(
+          slot: 10.Slot)))
+      root = Eth2Digest()
+
+    db.putCorruptAltairState(root)
+
+    let restoreAddr = addr dag.headState
+
+    func restore() =
+      assign(state[], restoreAddr[].data)
+
+    check:
+      state[].hbsAltair.data.slot == 10.Slot
+      not db.getAltairState(root, state[].hbsAltair.data, restore)
+
+      # assign() has switched the case object fork
+      state[].beaconStateFork == forkPhase0
+      state[].hbsPhase0.data.slot != 10.Slot
+
   test "find ancestors" & preset():
     var
       db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true)