reenable block bls verification (#875)

* finalizing state_transition

* cleanup

Co-authored-by: Dustin Brody <[email protected]>

beacon_chain/block_pool.nim

@@ -373,7 +373,6 @@ proc add*(
     # TODO if the block is from the future, we should not be resolving it (yet),
     #      but maybe we should use it as a hint that our clock is wrong?
     updateStateData(pool, pool.tmpState, BlockSlot(blck: parent, slot: blck.slot - 1))
-
     if not state_transition(pool.tmpState.data, signedBlock, {}):
       # TODO find a better way to log all this block data
       notice "Invalid block",
@@ -382,7 +381,6 @@ proc add*(
         cat = "filtering"
 
       return
-
     # Careful, tmpState.data has been updated but not blck - we need to create
     # the BlockRef first!
     pool.tmpState.blck = pool.addResolvedBlock(

beacon_chain/spec/datatypes.nim

@@ -601,6 +601,12 @@ func shortLog*(v: BeaconBlock): auto =
     voluntary_exits_len: v.body.voluntary_exits.len(),
   )
 
+func shortLog*(v: SignedBeaconBlock): auto =
+  (
+    blck: shortLog(v.message),
+    signature: shortLog(v.signature)
+  )
+
 func shortLog*(v: AttestationData): auto =
   (
     slot: shortLog(v.slot),

beacon_chain/spec/state_transition_block.nim

@@ -454,7 +454,9 @@ proc makeBeaconBlock*(
   ## the slot for which a block is to be created.
   var cache = get_empty_per_epoch_cache()
   let proposer_index = get_beacon_proposer_index(state, cache)
-  doAssert proposer_index.isSome, "Unable to get proposer index when proposing!"
+  if proposer_index.isNone:
+    warn "Unable to get proposer index when proposing!"
+    return
 
   # To create a block, we'll first apply a partial block to the state, skipping
   # some validations.

beacon_chain/state_transition.nim

@@ -31,9 +31,9 @@
 # now.
 
 import
-  collections/sets, chronicles, sets,
+  collections/sets, chronicles, sets, options,
   ./extras, ./ssz, metrics,
-  ./spec/[datatypes, digest, helpers, validator],
+  ./spec/[datatypes, crypto, digest, helpers, validator],
   ./spec/[state_transition_block, state_transition_epoch],
   ../nbench/bench_lab
 
@@ -101,6 +101,29 @@ proc process_slots*(state: var BeaconState, slot: Slot) {.nbench.}=
     if is_epoch_transition:
       beacon_current_validators.set(get_epoch_validator_count(state))
 
+# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/beacon-chain.md#verify_block_signature
+proc verify_block_signature*(
+    state: BeaconState, signedBlock: SignedBeaconBlock): bool {.nbench.} =
+  if signedBlock.message.proposer_index >= state.validators.len.uint64:
+    notice "Invalid proposer index in block",
+      blck = shortLog(signedBlock.message)
+    return false
+
+  let
+    proposer = state.validators[signedBlock.message.proposer_index]
+    domain = get_domain(
+      state, DOMAIN_BEACON_PROPOSER,
+      compute_epoch_at_slot(signedBlock.message.slot))
+    signing_root = compute_signing_root(signedBlock.message, domain)
+
+  if not bls_verify(proposer.pubKey, signing_root.data, signedBlock.signature):
+    notice "Block: signature verification failed",
+      blck = shortLog(signedBlock),
+      signingRoot = shortLog(signing_root)
+    return false
+
+  true
+
 # https://github.com/ethereum/eth2.0-specs/blob/v0.9.4/specs/core/0_beacon-chain.md#beacon-chain-state-transition-function
 proc verifyStateRoot(state: BeaconState, blck: BeaconBlock): bool =
   # This is inlined in state_transition(...) in spec.
@@ -155,15 +178,17 @@ proc state_transition*(
   # that the block is sane.
   # TODO what should happen if block processing fails?
   #      https://github.com/ethereum/eth2.0-specs/issues/293
-  var per_epoch_cache = get_empty_per_epoch_cache()
+  if skipBLSValidation in flags or
+      verify_block_signature(state, signedBlock):
+    var per_epoch_cache = get_empty_per_epoch_cache()
 
-  if process_block(state, signedBlock.message, flags, per_epoch_cache):
-    # This is a bit awkward - at the end of processing we verify that the
-    # state we arrive at is what the block producer thought it would be -
-    # meaning that potentially, it could fail verification
-    if skipStateRootValidation in flags or verifyStateRoot(state, signedBlock.message):
-      # State root is what it should be - we're done!
-      return true
+    if processBlock(state, signedBlock.message, flags, per_epoch_cache):
+      # This is a bit awkward - at the end of processing we verify that the
+      # state we arrive at is what the block producer thought it would be -
+      # meaning that potentially, it could fail verification
+      if skipStateRootValidation in flags or verifyStateRoot(state, signedBlock.message):
+        # State root is what it should be - we're done!
+        return true
 
   # Block processing failed, roll back changes
   state = old_state
@@ -222,20 +247,23 @@ proc state_transition*(
   var old_state = state
 
   process_slots(state, signedBlock.message.slot)
-  var per_epoch_cache = get_empty_per_epoch_cache()
 
-  if process_block(state.data, signedBlock.message, flags, per_epoch_cache):
-    if skipStateRootValidation in flags or verifyStateRoot(state.data, signedBlock.message):
-      # State root is what it should be - we're done!
+  if skipBLSValidation in flags or
+      verify_block_signature(state.data, signedBlock):
+
+    var per_epoch_cache = get_empty_per_epoch_cache()
+    if processBlock(state.data, signedBlock.message, flags, per_epoch_cache):
+      if skipStateRootValidation in flags or verifyStateRoot(state.data, signedBlock.message):
+        # State root is what it should be - we're done!
 
-      # TODO when creating a new block, state_root is not yet set.. comparing
-      #      with zero hash here is a bit fragile however, but this whole thing
-      #      should go away with proper hash caching
-      state.root =
-        if signedBlock.message.state_root == Eth2Digest(): hash_tree_root(state.data)
-        else: signedBlock.message.state_root
+        # TODO when creating a new block, state_root is not yet set.. comparing
+        #      with zero hash here is a bit fragile however, but this whole thing
+        #      should go away with proper hash caching
+        state.root =
+          if signedBlock.message.state_root == Eth2Digest(): hash_tree_root(state.data)
+          else: signedBlock.message.state_root
 
-      return true
+        return true
 
   # Block processing failed, roll back changes
   state = old_state

tests/mocking/mock_blocks.nim

@@ -8,7 +8,7 @@
 import
   options,
   # Specs
-  ../../beacon_chain/spec/[datatypes, crypto, validator, state_transition_block],
+  ../../beacon_chain/spec/[datatypes, validator, state_transition_block],
   # Internals
   ../../beacon_chain/[ssz, extras, state_transition],
   # Mock helpers
@@ -19,40 +19,21 @@ import
 
 proc signMockBlockImpl(
       state: BeaconState,
-      signedBlock: var SignedBeaconBlock,
-      proposer_index: ValidatorIndex
+      signedBlock: var SignedBeaconBlock
     ) =
   let block_slot = signedBlock.message.slot
   doAssert state.slot <= block_slot
 
-  let privkey = MockPrivKeys[proposer_index]
+  let privkey = MockPrivKeys[signedBlock.message.proposer_index]
 
   signedBlock.message.body.randao_reveal = get_epoch_signature(
     state.fork, state.genesis_validators_root, block_slot, privkey)
   signedBlock.signature = get_block_signature(
     state.fork, state.genesis_validators_root, block_slot,
     hash_tree_root(signedBlock.message), privkey)
 
-proc signMockBlock*(
-  state: BeaconState,
-  signedBlock: var SignedBeaconBlock
-  ) =
-
-  var emptyCache = get_empty_per_epoch_cache()
-  let proposer_index =
-    if signedBlock.message.slot == state.slot:
-      get_beacon_proposer_index(state, emptyCache)
-    else:
-      # Stub to get proposer index of future slot
-      # Note: this relies on ``let`` deep-copying the state
-      #       i.e. BeaconState should have value semantics
-      #            and not contain ref objects or pointers
-      var stubState = state
-      process_slots(stub_state, signedBlock.message.slot)
-      get_beacon_proposer_index(stub_state, emptyCache)
-
-  # In tests, just let this throw if appropriate
-  signMockBlockImpl(state, signedBlock, proposer_index.get)
+proc signMockBlock*(state: BeaconState, signedBlock: var SignedBeaconBlock) =
+  signMockBlockImpl(state, signedBlock)
 
 proc mockBlock(
     state: BeaconState,

tests/testblockutil.nim

@@ -62,6 +62,20 @@ func makeInitialDeposits*(
   for i in 0..<n.int:
     result.add makeDeposit(i, flags)
 
+func signBlock*(
+    fork: Fork, genesis_validators_root: Eth2Digest, blck: BeaconBlock,
+    privKey: ValidatorPrivKey, flags: UpdateFlags = {}): SignedBeaconBlock =
+  SignedBeaconBlock(
+    message: blck,
+    signature:
+      if skipBlsValidation notin flags:
+        get_block_signature(
+          fork, genesis_validators_root, blck.slot,
+          hash_tree_root(blck), privKey)
+      else:
+        ValidatorSig()
+  )
+
 proc addTestBlock*(
     state: var BeaconState,
     parent_root: Eth2Digest,
@@ -101,13 +115,10 @@ proc addTestBlock*(
 
   doAssert message.isSome(), "Should have created a valid block!"
 
-  var
-    new_block = SignedBeaconBlock(
-      message: message.get()
-    )
-
-
-  let ok = process_block(state, new_block.message, flags, cache)
+  let
+    new_block = signBlock(
+      state.fork, state.genesis_validators_root, message.get(), privKey, flags)
+    ok = process_block(state, new_block.message, flags, cache)
 
   doAssert ok, "adding block after producing it should work"
   new_block