Fixes to make EF Capella tests pass (#3719)

* Fixes to make EF Capella tests pass

* Clippy for state_processing
This commit is contained in:
Michael Sproul
2022-11-15 06:14:31 +11:00
committed by GitHub
parent 276e1845fd
commit 0cdd049da9
37 changed files with 434 additions and 196 deletions

View File

@@ -1,7 +1,7 @@
use std::marker::PhantomData;
use tree_hash::TreeHash;
use types::{
AbstractExecPayload, BeaconState, BeaconStateError, ChainSpec, EthSpec, ExecPayload, Hash256,
AbstractExecPayload, BeaconState, BeaconStateError, ChainSpec, EthSpec, Hash256,
SignedBeaconBlock, Slot,
};

View File

@@ -42,7 +42,9 @@ mod verify_deposit;
mod verify_exit;
mod verify_proposer_slashing;
#[cfg(feature = "withdrawals-processing")]
use crate::common::decrease_balance;
#[cfg(feature = "arbitrary-fuzz")]
use arbitrary::Arbitrary;
@@ -466,21 +468,22 @@ pub fn get_expected_withdrawals<T: EthSpec>(
spec: &ChainSpec,
) -> Result<Withdrawals<T>, BlockProcessingError> {
let epoch = state.current_epoch();
let mut withdrawal_index = *state.next_withdrawal_index()?;
let mut validator_index = *state.next_withdrawal_validator_index()?;
let mut withdrawal_index = state.next_withdrawal_index()?;
let mut validator_index = state.next_withdrawal_validator_index()?;
let mut withdrawals = vec![];
for _ in 0..state.validators().len() {
let validator = state.get_validator(validator_index as usize)?;
let balance = *state
.balances()
.get(validator_index as usize)
.ok_or_else(|| BeaconStateError::BalancesOutOfBounds(validator_index as usize))?;
let balance = *state.balances().get(validator_index as usize).ok_or(
BeaconStateError::BalancesOutOfBounds(validator_index as usize),
)?;
if validator.is_fully_withdrawable_at(balance, epoch, spec) {
withdrawals.push(Withdrawal {
index: withdrawal_index,
validator_index,
address: Address::from_slice(&validator.withdrawal_credentials[12..]),
address: validator
.get_eth1_withdrawal_address(spec)
.ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?,
amount: balance,
});
withdrawal_index.safe_add_assign(1)?;
@@ -488,7 +491,9 @@ pub fn get_expected_withdrawals<T: EthSpec>(
withdrawals.push(Withdrawal {
index: withdrawal_index,
validator_index,
address: Address::from_slice(&validator.withdrawal_credentials[12..]),
address: validator
.get_eth1_withdrawal_address(spec)
.ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?,
amount: balance.safe_sub(spec.max_effective_balance)?,
});
withdrawal_index.safe_add_assign(1)?;
@@ -496,7 +501,9 @@ pub fn get_expected_withdrawals<T: EthSpec>(
if withdrawals.len() == T::max_withdrawals_per_payload() {
break;
}
validator_index = validator_index.safe_add(1)? % state.validators().len() as u64;
validator_index = validator_index
.safe_add(1)?
.safe_rem(state.validators().len() as u64)?;
}
Ok(withdrawals.into())
@@ -513,12 +520,13 @@ pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload<T>
BeaconState::Merge(_) => Ok(()),
BeaconState::Capella(_) | BeaconState::Eip4844(_) => {
let expected_withdrawals = get_expected_withdrawals(state, spec)?;
let expected_root = expected_withdrawals.tree_hash_root();
let withdrawals_root = payload.withdrawals_root()?;
if expected_withdrawals.tree_hash_root() != payload.withdrawals_root()? {
if expected_root != withdrawals_root {
return Err(BlockProcessingError::WithdrawalsRootMismatch {
expected: expected_withdrawals.tree_hash_root(),
found: payload.withdrawals_root()?,
expected: expected_root,
found: withdrawals_root,
});
}
@@ -531,9 +539,11 @@ pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload<T>
}
if let Some(latest_withdrawal) = expected_withdrawals.last() {
*state.next_withdrawal_index_mut()? = latest_withdrawal.index + 1;
let next_validator_index =
(latest_withdrawal.validator_index + 1) % state.validators().len() as u64;
*state.next_withdrawal_index_mut()? = latest_withdrawal.index.safe_add(1)?;
let next_validator_index = latest_withdrawal
.validator_index
.safe_add(1)?
.safe_rem(state.validators().len() as u64)?;
*state.next_withdrawal_validator_index_mut()? = next_validator_index;
}

View File

@@ -1 +1,2 @@
#[allow(clippy::module_inception)]
pub mod eip4844;

View File

@@ -6,8 +6,8 @@ use ssz::Decode;
use ssz_types::VariableList;
use types::consts::eip4844::{BLOB_TX_TYPE, VERSIONED_HASH_VERSION_KZG};
use types::{
AbstractExecPayload, BeaconBlockBodyRef, EthSpec, ExecPayload, FullPayload, FullPayloadRef,
KzgCommitment, Transaction, Transactions, VersionedHash,
AbstractExecPayload, BeaconBlockBodyRef, EthSpec, ExecPayload, KzgCommitment, Transaction,
Transactions, VersionedHash,
};
pub fn process_blob_kzg_commitments<T: EthSpec, Payload: AbstractExecPayload<T>>(
@@ -34,7 +34,7 @@ pub fn verify_kzg_commitments_against_transactions<T: EthSpec>(
let nested_iter = transactions
.into_iter()
.filter(|tx| {
tx.get(0)
tx.first()
.map(|tx_type| *tx_type == BLOB_TX_TYPE)
.unwrap_or(false)
})

View File

@@ -94,6 +94,7 @@ pub enum BlockProcessingError {
index: usize,
length: usize,
},
WithdrawalCredentialsInvalid,
}
impl From<BeaconStateError> for BlockProcessingError {

View File

@@ -33,8 +33,11 @@ pub fn process_operations<'a, T: EthSpec, Payload: AbstractExecPayload<T>>(
process_attestations(state, block_body, verify_signatures, ctxt, spec)?;
process_deposits(state, block_body.deposits(), spec)?;
process_exits(state, block_body.voluntary_exits(), verify_signatures, spec)?;
#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))]
process_bls_to_execution_changes(state, block_body, verify_signatures, spec)?;
if let Ok(bls_to_execution_changes) = block_body.bls_to_execution_changes() {
process_bls_to_execution_changes(state, bls_to_execution_changes, verify_signatures, spec)?;
}
Ok(())
}
@@ -287,39 +290,25 @@ pub fn process_exits<T: EthSpec>(
/// Returns `Ok(())` if the validation and state updates completed successfully. Otherwise returs
/// an `Err` describing the invalid object or cause of failure.
#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))]
pub fn process_bls_to_execution_changes<'a, T: EthSpec, Payload: AbstractExecPayload<T>>(
pub fn process_bls_to_execution_changes<T: EthSpec>(
state: &mut BeaconState<T>,
block_body: BeaconBlockBodyRef<'a, T, Payload>,
bls_to_execution_changes: &[SignedBlsToExecutionChange],
verify_signatures: VerifySignatures,
spec: &ChainSpec,
) -> Result<(), BlockProcessingError> {
match block_body {
BeaconBlockBodyRef::Base(_)
| BeaconBlockBodyRef::Altair(_)
| BeaconBlockBodyRef::Merge(_) => Ok(()),
BeaconBlockBodyRef::Capella(_) | BeaconBlockBodyRef::Eip4844(_) => {
for (i, signed_address_change) in
block_body.bls_to_execution_changes()?.iter().enumerate()
{
verify_bls_to_execution_change(
state,
&signed_address_change,
verify_signatures,
spec,
)
.map_err(|e| e.into_with_index(i))?;
for (i, signed_address_change) in bls_to_execution_changes.iter().enumerate() {
verify_bls_to_execution_change(state, signed_address_change, verify_signatures, spec)
.map_err(|e| e.into_with_index(i))?;
state
.get_validator_mut(signed_address_change.message.validator_index as usize)?
.change_withdrawal_credentials(
&signed_address_change.message.to_execution_address,
spec,
);
}
Ok(())
}
state
.get_validator_mut(signed_address_change.message.validator_index as usize)?
.change_withdrawal_credentials(
&signed_address_change.message.to_execution_address,
spec,
);
}
Ok(())
}
/// Validates each `Deposit` and updates the state, short-circuiting on an invalid object.

View File

@@ -42,13 +42,13 @@ pub fn verify_bls_to_execution_change<T: EthSpec>(
// FIXME: Should this check be put inside the verify_signatures.is_true() condition?
// I believe that's used for fuzzing so this is a Mehdi question..
verify!(
validator.withdrawal_credentials.as_bytes()[1..] == pubkey_hash[1..],
validator.withdrawal_credentials.as_bytes().get(1..) == pubkey_hash.get(1..),
Invalid::WithdrawalCredentialsMismatch
);
if verify_signatures.is_true() {
verify!(
bls_execution_change_signature_set(state, signed_address_change, spec,)?.verify(),
bls_execution_change_signature_set(state, signed_address_change, spec)?.verify(),
Invalid::BadSignature
);
}

View File

@@ -1,4 +1,6 @@
use crate::upgrade::{upgrade_to_altair, upgrade_to_bellatrix};
use crate::upgrade::{
upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_eip4844,
};
use crate::{per_epoch_processing::EpochProcessingSummary, *};
use safe_arith::{ArithError, SafeArith};
use types::*;
@@ -55,6 +57,14 @@ pub fn per_slot_processing<T: EthSpec>(
if spec.bellatrix_fork_epoch == Some(state.current_epoch()) {
upgrade_to_bellatrix(state, spec)?;
}
// Capella.
if spec.capella_fork_epoch == Some(state.current_epoch()) {
upgrade_to_capella(state, spec)?;
}
// Eip4844
if spec.eip4844_fork_epoch == Some(state.current_epoch()) {
upgrade_to_eip4844(state, spec)?;
}
}
Ok(summary)

View File

@@ -1,4 +1,3 @@
use ssz_types::VariableList;
use std::mem;
use types::{BeaconState, BeaconStateCapella, BeaconStateError as Error, ChainSpec, EthSpec, Fork};