Fix builder exit signature batch verification logic and small refactor (#9173)

We had a bug when performing batch builder exit signature verification. The EF spec tests cover this case, but the EF tests only calls  individual signature verification (which is a separate code path). This PR unifies the two code paths. We should probably spend some time reviewing EF test code coverage and make sure we don't have separate code paths that do similar things.


  


Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>

consensus/state_processing/src/per_block_processing/process_operations.rs

@@ -8,6 +8,7 @@ use crate::per_block_processing::builder::{
     convert_validator_index_to_builder_index, is_builder_index,
 };
 use crate::per_block_processing::errors::{BlockProcessingError, ExitInvalid, IntoWithIndex};
+use crate::per_block_processing::signature_sets::{exit_signature_set, get_pubkey_from_state};
 use crate::per_block_processing::verify_payload_attestation::verify_payload_attestation;
 use bls::{PublicKeyBytes, SignatureBytes};
 use ssz_types::FixedVector;
@@ -547,7 +548,8 @@ fn process_builder_voluntary_exit<E: EthSpec>(
     let builder_index =
         convert_validator_index_to_builder_index(signed_exit.message.validator_index);
 
-    let builder = state
+    // Verify builder is known
+    state
         .builders()?
         .get(builder_index as usize)
         .cloned()
@@ -570,22 +572,17 @@ fn process_builder_voluntary_exit<E: EthSpec>(
         ));
     }
 
-    // Verify signature (using EIP-7044 domain: capella_fork_version for Deneb+)
     if verify_signatures.is_true() {
-        let pubkey = builder.pubkey;
-        let domain = spec.compute_domain(
-            Domain::VoluntaryExit,
-            spec.capella_fork_version,
-            state.genesis_validators_root(),
+        verify!(
+            exit_signature_set(
+                state,
+                |i| get_pubkey_from_state(state, i),
+                signed_exit,
+                spec
+            )?
+            .verify(),
+            ExitInvalid::BadSignature
         );
-        let message = signed_exit.message.signing_root(domain);
-        // TODO(gloas): use builder pubkey cache once available
-        let bls_pubkey = pubkey
-            .decompress()
-            .map_err(|_| BlockOperationError::invalid(ExitInvalid::BadSignature))?;
-        if !signed_exit.signature.verify(&bls_pubkey, message) {
-            return Err(BlockOperationError::invalid(ExitInvalid::BadSignature));
-        }
     }
 
     // Initiate builder exit

consensus/state_processing/src/per_block_processing/signature_sets.rs

@@ -2,6 +2,7 @@
 //! validated individually, or alongside in others in a potentially cheaper bulk operation.
 //!
 //! This module exposes one function to extract each type of `SignatureSet` from a `BeaconBlock`.
+use super::builder::{convert_validator_index_to_builder_index, is_builder_index};
 use bls::{AggregateSignature, PublicKey, PublicKeyBytes, Signature, SignatureSet};
 use ssz::DecodeError;
 use std::borrow::Cow;
@@ -503,7 +504,7 @@ pub fn deposit_pubkey_signature_message(
 }
 
 /// Returns a signature set that is valid if the `SignedVoluntaryExit` was signed by the indicated
-/// validator.
+/// validator (or builder, in the case of a builder exit).
 pub fn exit_signature_set<'a, E, F>(
     state: &'a BeaconState<E>,
     get_pubkey: F,
@@ -515,7 +516,18 @@ where
     F: Fn(usize) -> Option<Cow<'a, PublicKey>>,
 {
     let exit = &signed_exit.message;
-    let proposer_index = exit.validator_index as usize;
+    let validator_index = exit.validator_index;
+
+    let is_builder_exit =
+        state.fork_name_unchecked().gloas_enabled() && is_builder_index(validator_index);
+
+    let pubkey = if is_builder_exit {
+        let builder_index = convert_validator_index_to_builder_index(validator_index);
+        get_builder_pubkey_from_state(state, builder_index)
+            .ok_or(Error::ValidatorUnknown(validator_index))?
+    } else {
+        get_pubkey(validator_index as usize).ok_or(Error::ValidatorUnknown(validator_index))?
+    };
 
     let domain = if state.fork_name_unchecked().deneb_enabled() {
         // EIP-7044
@@ -537,7 +549,7 @@ where
 
     Ok(SignatureSet::single_pubkey(
         &signed_exit.signature,
-        get_pubkey(proposer_index).ok_or(Error::ValidatorUnknown(proposer_index as u64))?,
+        pubkey,
         message,
     ))
 }