chore: PR comments

ethereum-consensus/src/capella/block_processing.rs

@@ -4,11 +4,12 @@ use crate::{
         get_randao_mix, process_attestation, process_attester_slashing, process_block_header,
         process_deposit, process_eth1_data, process_proposer_slashing, process_randao,
         process_sync_aggregate, process_voluntary_exit, BeaconBlock, BeaconBlockBody, BeaconState,
-        BlsPublicKey, DomainType::BlsToExecutionChange, ExecutionEngine, ExecutionPayload,
-        ExecutionPayloadHeader, NewPayloadRequest, SignedBlsToExecutionChange,
+        BlsPublicKey, DomainType, ExecutionEngine, ExecutionPayload, ExecutionPayloadHeader,
+        NewPayloadRequest, SignedBlsToExecutionChange,
     },
     crypto::{hash, verify_signature},
     primitives::{BLS_WITHDRAWAL_PREFIX, ETH1_ADDRESS_WITHDRAWAL_PREFIX},
+    signing::verify_signed_data,
     ssz::ByteVector,
     state_transition::{
         invalid_operation_error, Context, InvalidDeposit, InvalidExecutionPayload,
@@ -44,29 +45,50 @@ pub fn process_bls_to_execution_change<
     signed_address_change: &mut SignedBlsToExecutionChange,
     context: &Context,
 ) -> Result<()> {
-    let address_change = signed_address_change;
+    let address_change = &signed_address_change.message;
+    let signature = &signed_address_change.signature;
 
-    assert!(address_change.message.validator_index < state.validators.len());
+    if address_change.validator_index >= state.validators.len() {
+        return Err(invalid_operation_error(InvalidOperation::ValidatorIndex(
+            address_change.validator_index,
+        )))
+    }
 
-    let validator = &mut state.validators[address_change.message.validator_index];
+    let withdrawal_credentials_prefix =
+        state.validators[address_change.validator_index].withdrawal_credentials[0];
 
-    assert!(validator.withdrawal_credentials.starts_with(&[BLS_WITHDRAWAL_PREFIX]));
-    assert!(
-        validator.withdrawal_credentials[1..]
-            == hash(address_change.message.from_bls_public_key.as_ref())[1..]
-    );
+    if withdrawal_credentials_prefix != BLS_WITHDRAWAL_PREFIX {
+        return Err(invalid_operation_error(InvalidOperation::WithdrawalCredentialsPrefix(
+            state.validators[address_change.validator_index].withdrawal_credentials[0],
+        )))
+    }
+
+    let public_key: &BlsPublicKey = &address_change.from_bls_public_key;
+
+    let domain = compute_domain(
+        DomainType::BlsToExecutionChange,
+        None,
+        Some(state.genesis_validators_root),
+        context,
+    )?;
+    let signed_data =
+        verify_signed_data(&mut address_change.clone(), signature, public_key, domain);
+    if signed_data.is_err() {
+        return Err(invalid_operation_error(InvalidOperation::ExecutionChange(
+            signed_address_change.signature.clone(),
+        )))
+    }
+
+    let validator = &mut state.validators[address_change.validator_index];
 
-    let domain =
-        compute_domain(BlsToExecutionChange, None, Some(state.genesis_validators_root), context)?;
-    let signing_root = compute_signing_root(address_change, domain)?;
-    let pk: &BlsPublicKey = &address_change.message.from_bls_public_key;
-    assert!(verify_signature(pk, signing_root.as_ref(), &address_change.signature,).is_ok());
-    let withdrawal_credentials = vec![ETH1_ADDRESS_WITHDRAWAL_PREFIX];
+    validator.withdrawal_credentials[0] = ETH1_ADDRESS_WITHDRAWAL_PREFIX;
+    let mut withdrawal_credentials: Vec<u8> = validator.withdrawal_credentials.as_ref().to_vec();
+    withdrawal_credentials[12..].copy_from_slice(address_change.to_execution_address.as_ref());
+    let withdrawal_credentials: &[u8] = &withdrawal_credentials;
+    let withdrawal_credentials =
+        ByteVector::<32>::try_from(withdrawal_credentials).expect("Wrong size");
+    validator.withdrawal_credentials = withdrawal_credentials;
 
-    let withdrawal_credentials_array: [u8; 32] =
-        withdrawal_credentials.try_into().expect("Wrong size");
-    validator.withdrawal_credentials = ByteVector::try_from(withdrawal_credentials_array.as_ref())
-        .expect("Failed to convert array to ByteVector");
     Ok(())
 }
 
@@ -130,7 +152,7 @@ pub fn process_operations<
                 expected: expected_deposit_count,
                 count: body.deposits.len(),
             },
-        )));
+        )))
     }
     body.proposer_slashings
         .iter_mut()
@@ -202,7 +224,7 @@ pub fn process_execution_payload<
                 expected: state.latest_execution_payload_header.block_hash.clone(),
             }
             .into(),
-        ));
+        ))
     }
 
     let current_epoch = get_current_epoch(state, context);
@@ -214,7 +236,7 @@ pub fn process_execution_payload<
                 expected: randao_mix.clone(),
             }
             .into(),
-        ));
+        ))
     }
 
     let timestamp = compute_timestamp_at_slot(state, state.slot, context)?;
@@ -225,7 +247,7 @@ pub fn process_execution_payload<
                 expected: timestamp,
             }
             .into(),
-        ));
+        ))
     }
 
     let new_payload_request = NewPayloadRequest(payload);

ethereum-consensus/src/state_transition/error.rs

@@ -84,6 +84,14 @@ pub enum InvalidOperation {
     SyncAggregate(#[from] InvalidSyncAggregate),
     #[error("invalid execution payload: {0}")]
     ExecutionPayload(#[from] InvalidExecutionPayload),
+    #[error("invalid bls signature for execution chang {0:?}")]
+    ExecutionChange(BlsSignature),
+    #[error("validator index is out of bounds {0}")]
+    ValidatorIndex(usize),
+    #[error("invalid withdrawal credentials prefix{0}")]
+    WithdrawalCredentialsPrefix(u8),
+    #[error("invalid withdrawal credentials public key{0:?}")]
+    WithdrawalCredentialsPublicKey(&'static [u8]),
 }
 
 #[derive(Debug, Error)]