From b82f4913ec1be05659f651a9d073ce31c03fe581 Mon Sep 17 00:00:00 2001 From: samtvlabs Date: Wed, 20 Sep 2023 05:39:23 +0000 Subject: [PATCH] chore: PR comments --- .../src/capella/block_processing.rs | 70 ++++++++++++------- .../src/state_transition/error.rs | 8 +++ 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/ethereum-consensus/src/capella/block_processing.rs b/ethereum-consensus/src/capella/block_processing.rs index 2c7c153a0..479085d56 100644 --- a/ethereum-consensus/src/capella/block_processing.rs +++ b/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 = 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); diff --git a/ethereum-consensus/src/state_transition/error.rs b/ethereum-consensus/src/state_transition/error.rs index 139e9ff61..33cb9bbb9 100644 --- a/ethereum-consensus/src/state_transition/error.rs +++ b/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)]