Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ slashing_protection = { path = "validator_client/slashing_protection" }
slot_clock = { path = "common/slot_clock" }
smallvec = { version = "1.11.2", features = ["arbitrary"] }
snap = "1"
ssz_types = "0.11.0"
ssz_types = "0.12.1"
state_processing = { path = "consensus/state_processing" }
store = { path = "beacon_node/store" }
strum = { version = "0.24", features = ["derive"] }
Expand Down
7 changes: 5 additions & 2 deletions beacon_node/beacon_chain/benches/benches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ fn create_test_block_and_blobs<E: EthSpec>(
let blobs = (0..num_of_blobs)
.map(|_| Blob::<E>::default())
.collect::<Vec<_>>()
.into();
let proofs = vec![KzgProof::empty(); num_of_blobs * E::number_of_columns()].into();
.try_into()
.unwrap();
let proofs = vec![KzgProof::empty(); num_of_blobs * E::number_of_columns()]
.try_into()
.unwrap();

(signed_block, blobs, proofs)
}
Expand Down
39 changes: 36 additions & 3 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use state_processing::{
};
use std::borrow::Cow;
use strum::AsRefStr;
use tracing::debug;
use tracing::{debug, error};
use tree_hash::TreeHash;
use types::{
Attestation, AttestationData, AttestationRef, BeaconCommittee,
Expand Down Expand Up @@ -267,6 +267,14 @@ pub enum Error {
/// We were unable to process this attestation due to an internal error. It's unclear if the
/// attestation is valid.
BeaconChainError(Box<BeaconChainError>),
/// A critical error occurred while converting SSZ types.
/// This can only occur when a VariableList was not able to be constructed from a single
/// attestation.
///
/// ## Peer scoring
///
/// The peer has sent an invalid message.
SszTypesError(ssz_types::Error),
}

impl From<BeaconChainError> for Error {
Expand All @@ -275,6 +283,12 @@ impl From<BeaconChainError> for Error {
}
}

impl From<ssz_types::Error> for Error {
fn from(e: ssz_types::Error) -> Self {
Self::SszTypesError(e)
}
}

/// Used to avoid double-checking signatures.
#[derive(Copy, Clone)]
enum CheckAttestationSignature {
Expand Down Expand Up @@ -442,7 +456,18 @@ fn process_slash_info<T: BeaconChainTypes>(
.spec
.fork_name_at_slot::<T::EthSpec>(attestation.data.slot);

let indexed_attestation = attestation.to_indexed(fork_name);
let indexed_attestation = match attestation.to_indexed(fork_name) {
Ok(indexed) => indexed,
Err(e) => {
error!(
attestation_root = ?attestation.data.tree_hash_root(),
error = ?e,
"Unable to construct VariableList from a single attestation. \
This indicates a serious bug in SSZ handling"
);
return Error::SszTypesError(e);
}
};
(indexed_attestation, true, err)
}
SignatureNotCheckedIndexed(indexed, err) => (indexed, true, err),
Expand Down Expand Up @@ -932,7 +957,15 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
.spec
.fork_name_at_slot::<T::EthSpec>(attestation.data.slot);

let indexed_attestation = attestation.to_indexed(fork_name);
let indexed_attestation = match attestation.to_indexed(fork_name) {
Ok(indexed) => indexed,
Err(e) => {
return Err(SignatureNotCheckedSingle(
attestation,
Error::SszTypesError(e),
));
}
};

let validator_index = match Self::verify_middle_checks(attestation, chain) {
Ok(t) => t,
Expand Down
180 changes: 135 additions & 45 deletions beacon_node/beacon_chain/src/beacon_chain.rs

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions beacon_node/beacon_chain/src/data_column_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -839,16 +839,16 @@ mod test {
let state = harness.get_current_state();
let ((block, _blobs_opt), _state) = harness
.make_block_with_modifier(state, slot, |block| {
*block.body_mut().blob_kzg_commitments_mut().unwrap() = vec![].into();
*block.body_mut().blob_kzg_commitments_mut().unwrap() = vec![].try_into().unwrap();
})
.await;

let index = 0;
let column_sidecar = DataColumnSidecar::<E> {
index,
column: vec![].into(),
kzg_commitments: vec![].into(),
kzg_proofs: vec![].into(),
column: vec![].try_into().unwrap(),
kzg_commitments: vec![].try_into().unwrap(),
kzg_proofs: vec![].try_into().unwrap(),
signed_block_header: block.signed_block_header(),
kzg_commitments_inclusion_proof: block
.message()
Expand Down
1 change: 1 addition & 0 deletions beacon_node/beacon_chain/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ pub enum BlockProductionError {
KzgError(kzg::Error),
FailedToBuildBlobSidecars(String),
MissingExecutionRequests,
SszTypesError(ssz_types::Error),
}

easy_from_to!(BlockProcessingError, BlockProductionError);
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/fetch_blobs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ fn create_test_block_and_blobs(
.map(|(blob, proofs)| {
BlobAndProof::V2(BlobAndProofV2 {
blob,
proofs: proofs.to_vec().into(),
proofs: proofs.to_vec().try_into().unwrap(),
})
})
.collect()
Expand Down
31 changes: 18 additions & 13 deletions beacon_node/beacon_chain/src/kzg_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ pub(crate) fn build_data_column_sidecars<E: EthSpec>(
.get(col)
.ok_or(format!("Missing blob cell at index {col}"))?;
let cell: Vec<u8> = cell.to_vec();
let cell = Cell::<E>::from(cell);
let cell =
Cell::<E>::try_from(cell).map_err(|e| format!("BytesPerCell exceeded: {e:?}"))?;

let proof = blob_cell_proofs
.get(col)
Expand All @@ -269,23 +270,27 @@ pub(crate) fn build_data_column_sidecars<E: EthSpec>(
}
}

let sidecars: Vec<Arc<DataColumnSidecar<E>>> = columns
let sidecars: Result<Vec<Arc<DataColumnSidecar<E>>>, String> = columns
.into_iter()
.zip(column_kzg_proofs)
.enumerate()
.map(|(index, (col, proofs))| {
Arc::new(DataColumnSidecar {
index: index as u64,
column: DataColumn::<E>::from(col),
kzg_commitments: kzg_commitments.clone(),
kzg_proofs: VariableList::from(proofs),
signed_block_header: signed_block_header.clone(),
kzg_commitments_inclusion_proof: kzg_commitments_inclusion_proof.clone(),
})
})
.map(
|(index, (col, proofs))| -> Result<Arc<DataColumnSidecar<E>>, String> {
Ok(Arc::new(DataColumnSidecar {
index: index as u64,
column: DataColumn::<E>::try_from(col)
.map_err(|e| format!("MaxBlobCommitmentsPerBlock exceeded: {e:?}"))?,
kzg_commitments: kzg_commitments.clone(),
kzg_proofs: VariableList::try_from(proofs)
.map_err(|e| format!("MaxBlobCommitmentsPerBlock exceeded: {e:?}"))?,
signed_block_header: signed_block_header.clone(),
kzg_commitments_inclusion_proof: kzg_commitments_inclusion_proof.clone(),
}))
},
)
.collect();

Ok(sidecars)
sidecars
}

/// Reconstruct blobs from a subset of data column sidecars (requires at least 50%).
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2323,7 +2323,7 @@ where
.collect::<Vec<_>>();

// Building a VarList from leaves
let deposit_data_list = VariableList::<_, U4294967296>::from(leaves.clone());
let deposit_data_list = VariableList::<_, U4294967296>::try_from(leaves.clone()).unwrap();

// Setting the deposit_root to be the tree_hash_root of the VarList
state.eth1_data_mut().deposit_root = deposit_data_list.tree_hash_root();
Expand All @@ -2347,7 +2347,7 @@ where
let deposits = datas
.into_par_iter()
.zip(proofs.into_par_iter())
.map(|(data, proof)| (data, proof.into()))
.map(|(data, proof)| (data, proof.try_into().unwrap()))
.map(|(data, proof)| Deposit { proof, data })
.collect::<Vec<_>>();

Expand Down
12 changes: 8 additions & 4 deletions beacon_node/beacon_chain/tests/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ async fn invalid_signature_attester_slashing() {

let attester_slashing = if fork_name.electra_enabled() {
let indexed_attestation = IndexedAttestationElectra {
attesting_indices: vec![0].into(),
attesting_indices: vec![0].try_into().unwrap(),
data: AttestationData {
slot: Slot::new(0),
index: 0,
Expand All @@ -723,7 +723,7 @@ async fn invalid_signature_attester_slashing() {
AttesterSlashing::Electra(attester_slashing)
} else {
let indexed_attestation = IndexedAttestationBase {
attesting_indices: vec![0].into(),
attesting_indices: vec![0].try_into().unwrap(),
data: AttestationData {
slot: Slot::new(0),
index: 0,
Expand Down Expand Up @@ -890,7 +890,9 @@ async fn invalid_signature_deposit() {
let harness = get_invalid_sigs_harness(&chain_segment).await;
let mut snapshots = chain_segment.clone();
let deposit = Deposit {
proof: vec![Hash256::zero(); DEPOSIT_TREE_DEPTH + 1].into(),
proof: vec![Hash256::zero(); DEPOSIT_TREE_DEPTH + 1]
.try_into()
.unwrap(),
data: DepositData {
pubkey: Keypair::random().pk.into(),
withdrawal_credentials: Hash256::zero(),
Expand Down Expand Up @@ -1262,7 +1264,9 @@ async fn block_gossip_verification() {
as usize;

if let Ok(kzg_commitments) = block.body_mut().blob_kzg_commitments_mut() {
*kzg_commitments = vec![KzgCommitment::empty_for_testing(); kzg_commitments_len + 1].into();
*kzg_commitments = vec![KzgCommitment::empty_for_testing(); kzg_commitments_len + 1]
.try_into()
.unwrap();
assert!(
matches!(
unwrap_err(harness.chain.verify_block_for_gossip(Arc::new(SignedBeaconBlock::from_block(block, signature))).await),
Expand Down
Loading
Loading