fix: PR comments

ethereum-consensus/src/capella/block_processing.rs

@@ -4,16 +4,17 @@ use crate::{
         get_randao_mix, is_fully_withdrawable_validator, is_partially_withdrawable_validator,
         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, ExecutionAddress, ExecutionEngine, ExecutionPayload, ExecutionPayloadHeader,
+        process_voluntary_exit, BeaconBlock, BeaconBlockBody, BeaconState, DomainType,
+        ExecutionAddress, ExecutionEngine, ExecutionPayload, ExecutionPayloadHeader,
         NewPayloadRequest, SignedBlsToExecutionChange, Withdrawal,
     },
+    crypto::hash,
     primitives::{BLS_WITHDRAWAL_PREFIX, ETH1_ADDRESS_WITHDRAWAL_PREFIX},
     signing::verify_signed_data,
     ssz::prelude::*,
     state_transition::{
-        invalid_operation_error, Context, InvalidDeposit, InvalidExecutionPayload,
-        InvalidOperation, InvalidWithdrawals, Result,
+        invalid_operation_error, Context, InvalidBlsToExecutionChange, InvalidDeposit,
+        InvalidExecutionPayload, InvalidOperation, InvalidWithdrawals, Result,
     },
 };
 
@@ -44,22 +45,34 @@ pub fn process_bls_to_execution_change<
     signed_address_change: &SignedBlsToExecutionChange,
     context: &Context,
 ) -> Result<()> {
-    let address_change = &signed_address_change.message;
+    let mut address_change = signed_address_change.message.clone();
     let signature = &signed_address_change.signature;
 
     if address_change.validator_index >= state.validators.len() {
-        return Err(invalid_operation_error(InvalidOperation::ValidatorIndex(
-            address_change.validator_index,
-        )));
+        return Err(invalid_operation_error(InvalidOperation::BlsToExecutionChange(
+            InvalidBlsToExecutionChange::BadValidatorIndex(address_change.validator_index),
+        )))
     }
 
     let withdrawal_credentials_prefix =
         state.validators[address_change.validator_index].withdrawal_credentials[0];
 
     if withdrawal_credentials_prefix != BLS_WITHDRAWAL_PREFIX {
-        return Err(invalid_operation_error(InvalidOperation::WithdrawalCredentialsPrefix(
-            state.validators[address_change.validator_index].withdrawal_credentials[0],
-        )));
+        return Err(invalid_operation_error(InvalidOperation::BlsToExecutionChange(
+            InvalidBlsToExecutionChange::WithdrawalCredentialsPrefix(
+                state.validators[address_change.validator_index].withdrawal_credentials[0],
+            ),
+        )))
+    }
+
+    let public_key = address_change.from_bls_public_key.clone();
+
+    if state.validators[address_change.validator_index].withdrawal_credentials[1..] !=
+        hash(public_key.as_slice())[1..]
+    {
+        return Err(invalid_operation_error(InvalidOperation::BlsToExecutionChange(
+            InvalidBlsToExecutionChange::WithdrawalCredentialsPublicKey(public_key),
+        )))
     }
 
     let domain = compute_domain(
@@ -68,24 +81,15 @@ pub fn process_bls_to_execution_change<
         Some(state.genesis_validators_root),
         context,
     )?;
-    let public_key = &address_change.from_bls_public_key;
-    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(),
-        )));
-    }
+
+    verify_signed_data(&mut address_change, signature, &public_key, domain)?;
 
     let validator = &mut state.validators[address_change.validator_index];
 
     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;
+    validator.withdrawal_credentials[1..12].fill(0);
+    validator.withdrawal_credentials[12..]
+        .copy_from_slice(address_change.to_execution_address.as_ref());
 
     Ok(())
 }
@@ -150,7 +154,7 @@ pub fn process_operations<
                 expected: expected_deposit_count,
                 count: body.deposits.len(),
             },
-        )));
+        )))
     }
     body.proposer_slashings
         .iter_mut()
@@ -222,7 +226,7 @@ pub fn process_execution_payload<
                 expected: state.latest_execution_payload_header.block_hash.clone(),
             }
             .into(),
-        ));
+        ))
     }
 
     let current_epoch = get_current_epoch(state, context);
@@ -234,7 +238,7 @@ pub fn process_execution_payload<
                 expected: randao_mix.clone(),
             }
             .into(),
-        ));
+        ))
     }
 
     let timestamp = compute_timestamp_at_slot(state, state.slot, context)?;
@@ -245,7 +249,7 @@ pub fn process_execution_payload<
                 expected: timestamp,
             }
             .into(),
-        ));
+        ))
     }
 
     let new_payload_request = NewPayloadRequest(payload);
@@ -316,7 +320,7 @@ pub fn process_withdrawals<
                 provided: execution_payload.withdrawals.to_vec(),
                 expected: expected_withdrawals,
             },
-        )));
+        )))
     }
 
     for withdrawal in &expected_withdrawals {
@@ -401,7 +405,7 @@ pub fn get_expected_withdrawals<
             withdrawal_index += 1;
         }
         if withdrawals.len() == context.max_withdrawals_per_payload {
-            break;
+            break
         }
         validator_index = (validator_index + 1) % state.validators.len();
     }

ethereum-consensus/src/state_transition/error.rs

@@ -1,5 +1,5 @@
 use crate::{
-    capella::Withdrawal,
+    capella::{BlsPublicKey, Withdrawal},
     crypto::Error as CryptoError,
     phase0::{AttestationData, BeaconBlockHeader, Checkpoint},
     primitives::{BlsSignature, Bytes32, Epoch, Hash32, Root, Slot, ValidatorIndex},
@@ -85,14 +85,10 @@ 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]),
+    #[error("invalid withdrawals: {0}")]
+    Withdrawal(#[from] InvalidWithdrawals),
+    #[error("invalid bls signature to execution change: {0}")]
+    BlsToExecutionChange(#[from] InvalidBlsToExecutionChange),
 }
 
 #[derive(Debug, Error)]
@@ -195,6 +191,18 @@ pub enum InvalidWithdrawals {
     IncorrectWithdrawals { provided: Vec<Withdrawal>, expected: Vec<Withdrawal> },
 }
 
+#[derive(Debug, Error)]
+pub enum InvalidBlsToExecutionChange {
+    #[error("invalid bls signature for execution change {0:?}")]
+    ExecutionChange(BlsSignature),
+    #[error("validator index is out of bounds {0}")]
+    BadValidatorIndex(usize),
+    #[error("invalid withdrawal credentials prefix{0}")]
+    WithdrawalCredentialsPrefix(u8),
+    #[error("invalid withdrawal credentials public key{0:?}")]
+    WithdrawalCredentialsPublicKey(BlsPublicKey),
+}
+
 #[derive(Debug, Error)]
 pub enum InvalidSyncAggregate {
     #[error("invalid sync committee aggregate signature {signature} signing over previous slot block root {root}")]