re-enable randao checks (#4187)

* re-enable randao checks

* use `asSigVerified` consistently

* fix spelling

* document why state_transition.makeBeaconBlock trusting signatures is safe

AllTests-mainnet.md

@@ -591,4 +591,4 @@ OK: 1/1 Fail: 0/1 Skip: 0/1
 OK: 9/9 Fail: 0/9 Skip: 0/9
 
 ---TOTAL---
-OK: 327/332 Fail: 0/332 Skip: 5/332
+OK: 328/333 Fail: 0/333 Skip: 5/333

beacon_chain/spec/datatypes/altair.nim

@@ -787,6 +787,10 @@ template asSigVerified*(
        TrustedSignedBeaconBlock): SigVerifiedSignedBeaconBlock =
   isomorphicCast[SigVerifiedSignedBeaconBlock](x)
 
+template asSigVerified*(
+    x: BeaconBlock | TrustedBeaconBlock): SigVerifiedBeaconBlock =
+  isomorphicCast[SigVerifiedBeaconBlock](x)
+
 template asMsgTrusted*(
     x: SignedBeaconBlock |
        SigVerifiedSignedBeaconBlock |

beacon_chain/spec/datatypes/bellatrix.nim

@@ -412,6 +412,10 @@ template asSigVerified*(
        TrustedSignedBeaconBlock): SigVerifiedSignedBeaconBlock =
   isomorphicCast[SigVerifiedSignedBeaconBlock](x)
 
+template asSigVerified*(
+    x: BeaconBlock | TrustedBeaconBlock): SigVerifiedBeaconBlock =
+  isomorphicCast[SigVerifiedBeaconBlock](x)
+
 template asMsgTrusted*(
     x: SignedBeaconBlock |
        SigVerifiedSignedBeaconBlock |

beacon_chain/spec/datatypes/phase0.nim

@@ -319,6 +319,10 @@ template asSigVerified*(
        TrustedSignedBeaconBlock): SigVerifiedSignedBeaconBlock =
   isomorphicCast[SigVerifiedSignedBeaconBlock](x)
 
+template asSigVerified*(
+    x: BeaconBlock | TrustedBeaconBlock): SigVerifiedBeaconBlock =
+  isomorphicCast[SigVerifiedBeaconBlock](x)
+
 template asMsgTrusted*(
     x: SignedBeaconBlock |
        SigVerifiedSignedBeaconBlock |

beacon_chain/spec/forks.nim

@@ -533,16 +533,19 @@ template withStateAndBlck*(
   of BeaconStateFork.Bellatrix:
     const stateFork {.inject.} = BeaconStateFork.Bellatrix
     template state: untyped {.inject.} = s.bellatrixData
+    template forkyState: untyped {.inject.} = s.bellatrixData
     template blck: untyped {.inject.} = b.bellatrixData
     body
   of BeaconStateFork.Altair:
     const stateFork {.inject.} = BeaconStateFork.Altair
     template state: untyped {.inject.} = s.altairData
+    template forkyState: untyped {.inject.} = s.altairData
     template blck: untyped {.inject.} = b.altairData
     body
   of BeaconStateFork.Phase0:
     const stateFork {.inject.} = BeaconStateFork.Phase0
     template state: untyped {.inject.} = s.phase0Data
+    template forkyState: untyped {.inject.} = s.phase0Data
     template blck: untyped {.inject.} = b.phase0Data
     body
 

beacon_chain/spec/state_transition.nim

@@ -352,7 +352,8 @@ proc makeBeaconBlock*(
                                 randao_reveal, eth1_data, graffiti, attestations, deposits,
                                 exits, sync_aggregate, execution_payload)
 
-  let res = process_block(cfg, state.data, blck, verificationFlags, cache)
+  let res = process_block(
+    cfg, state.data, blck.asSigVerified(), verificationFlags, cache)
 
   if res.isErr:
     rollback(state)
@@ -421,7 +422,9 @@ proc makeBeaconBlock*(
                                 randao_reveal, eth1_data, graffiti, attestations, deposits,
                                 exits, sync_aggregate, execution_payload)
 
-  let res = process_block(cfg, state.data, blck, verificationFlags, cache)
+  # Signatures are verified elsewhere, so don't duplicate inefficiently here
+  let res = process_block(
+    cfg, state.data, blck.asSigVerified(), verificationFlags, cache)
 
   if res.isErr:
     rollback(state)
@@ -491,7 +494,8 @@ proc makeBeaconBlock*(
                                 randao_reveal, eth1_data, graffiti, attestations, deposits,
                                 exits, sync_aggregate, execution_payload)
 
-  let res = process_block(cfg, state.data, blck, verificationFlags, cache)
+  let res = process_block(
+    cfg, state.data, blck.asSigVerified(), verificationFlags, cache)
 
   if res.isErr:
     rollback(state)
@@ -538,8 +542,9 @@ proc makeBeaconBlock*(
                            randao_reveal, eth1_data, graffiti, attestations, deposits,
                            exits, sync_aggregate, executionPayload))
 
-    let res = process_block(cfg, state.`kind Data`.data, blck.`kind Data`,
-                            verificationFlags, cache)
+    let res = process_block(
+      cfg, state.`kind Data`.data, blck.`kind Data`.asSigVerified(),
+      verificationFlags, cache)
     if res.isErr:
       rollback(state)
       return err(res.error())

beacon_chain/spec/state_transition_block.nim

@@ -85,8 +85,7 @@ proc process_randao(
     return err("process_randao: proposer index missing, probably along with any active validators")
 
   # Verify RANDAO reveal
-  let
-    epoch = state.get_current_epoch()
+  let epoch = state.get_current_epoch()
 
   if skipRandaoVerification in flags:
     if body.randao_reveal.toRaw != ValidatorSig.infinity.toRaw:
@@ -94,9 +93,15 @@ proc process_randao(
   elif skipBlsValidation notin flags:
     let proposer_pubkey = state.validators.item(proposer_index.get).pubkey
 
+    # `state_transition.makeBeaconBlock` ensures this is run with a trusted
+    # signature, but unless the full skipBlsValidation is specified, RANDAO
+    # epoch signatures still have to be verified.
     if not verify_epoch_signature(
         state.fork, state.genesis_validators_root, epoch, proposer_pubkey,
-        body.randao_reveal):
+        when body.randao_reveal is ValidatorSig:
+          body.randao_reveal
+        else:
+          isomorphicCast[ValidatorSig](body.randao_reveal)):
 
       return err("process_randao: invalid epoch signature")
 

beacon_chain/validators/validator_duties.nim

@@ -462,8 +462,7 @@ proc makeBeaconBlockForHeadAndSlot*(
     execution_payload_root: Opt[Eth2Digest] = Opt.none(Eth2Digest)):
     Future[ForkedBlockResult] {.async.} =
   # Advance state to the slot that we're proposing for
-  let
-    proposalState = assignClone(node.dag.headState)
+  let proposalState = assignClone(node.dag.headState)
 
   # TODO fails at checkpoint synced head
   node.dag.withUpdatedState(
@@ -532,11 +531,10 @@ proc makeBeaconBlockForHeadAndSlot*(
       effectiveExecutionPayload,
       noRollback, # Temporary state - no need for rollback
       cache,
+      # makeBeaconBlock doesn't verify BLS at all, but does have special case
+      # for skipRandaoVerification separately
       verificationFlags =
-        if skip_randao_verification_bool:
-          {skipBlsValidation, skipRandaoVerification}
-        else:
-          {skipBlsValidation},
+        if skip_randao_verification_bool: {skipRandaoVerification} else: {},
       transactions_root =
         if transactions_root.isSome:
           Opt.some transactions_root.get