properly (re)factor consolidation signature checking (#6334)

beacon_chain/el/el_manager.nim

@@ -1956,7 +1956,6 @@ proc startExchangeTransitionConfigurationLoop(
 
   while true:
     # https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.3/src/engine/paris.md#specification-3
-    debug "Exchange transition configuration tick"
     await m.exchangeTransitionConfiguration()
     await sleepAsync(60.seconds)
 

beacon_chain/spec/signatures.nim

@@ -421,3 +421,23 @@ proc verify_bls_to_execution_change_signature*(
   let signing_root = compute_bls_to_execution_change_signing_root(
     genesisFork, genesis_validators_root, msg.message)
   blsVerify(pubkey, signing_root.data, signature)
+
+func compute_consolidation_signing_root(
+    genesisFork: Fork, genesis_validators_root: Eth2Digest,
+    msg: Consolidation): Eth2Digest =
+  # Uses genesis fork version regardless
+  doAssert genesisFork.current_version == genesisFork.previous_version
+
+  let domain = compute_domain(
+    DOMAIN_CONSOLIDATION, genesisFork.current_version,
+    genesis_validators_root=genesis_validators_root)
+  compute_signing_root(msg, domain)
+
+proc verify_consolidation_signature*(
+    genesisFork: Fork, genesis_validators_root: Eth2Digest,
+    msg: SignedConsolidation | TrustedSignedConsolidation,
+    pubkeys: openArray[ValidatorPubKey]): bool =
+  withTrust(msg.signature):
+    let signing_root = compute_consolidation_signing_root(
+      genesisFork, genesis_validators_root, msg.message)
+    blsFastAggregateVerify(pubkeys, signing_root.data, msg.signature)

beacon_chain/spec/state_transition_block.nim

@@ -644,20 +644,11 @@ proc process_consolidation*(
       target_validator.withdrawal_credentials.data.toOpenArray(12, 31)):
     return err("Consolidation: source and target don't have same withdrawal address")
 
-  debugComment "this is per spec, near-verbatim, but Nimbus generally factors this out into spec/signatures.nim. so, create verify_consolidation_signature infra there, call here"
   # Verify consolidation is signed by the source and the target
-  let
+  if not verify_consolidation_signature(
-    domain = compute_domain(
+      cfg.genesisFork, state.genesis_validators_root, signed_consolidation,
-      DOMAIN_CONSOLIDATION, cfg.GENESIS_FORK_VERSION,
+      [source_validator[].pubkey, target_validator.pubkey]):
-      genesis_validators_root=state.genesis_validators_root)
+    return err("Consolidation: invalid signature")
-    signing_root = compute_signing_root(consolidation, domain)
-    pubkeys = [source_validator[].pubkey, target_validator.pubkey]
-
-  debugComment "as a good example, this trustedsig hack typically/should live in spec/signatures.nim"
-  when not (signed_consolidation.signature is TrustedSig):
-    if not blsFastAggregateVerify(
-        pubkeys, signing_root.data, signed_consolidation.signature):
-      return err("Consolidation: invalid signature")
 
   # Initiate source validator exit and append pending consolidation
   source_validator[].exit_epoch = compute_consolidation_epoch_and_update_churn(
@@ -667,8 +658,7 @@ proc process_consolidation*(
   debugComment "check HashList add return value"
   discard state.pending_consolidations.add(PendingConsolidation(
     source_index: consolidation.source_index,
-    target_index: consolidation.target_index
+    target_index: consolidation.target_index))
-  ))
 
   ok()
 

tests/testblockutil.nim

@@ -208,8 +208,6 @@ proc addTestBlock*(
       else:
         default(bellatrix.ExecutionPayloadForSigning)
 
-    debugComment "addTestBlock Electra attestation support"
-
     makeBeaconBlock(
       cfg,
       state,