From 191c1bcca8821e7faa1dff210eadc802af6db157 Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 12 Sep 2025 02:16:22 +1000 Subject: [PATCH 1/7] Bump ssz_types to v0.12.1 --- Cargo.lock | 7 +- Cargo.toml | 2 +- .../src/attestation_verification.rs | 54 ++++-- beacon_node/beacon_chain/src/beacon_chain.rs | 180 +++++++++++++----- .../src/data_column_verification.rs | 8 +- beacon_node/beacon_chain/src/errors.rs | 1 + .../beacon_chain/src/fetch_blobs/tests.rs | 2 +- beacon_node/beacon_chain/src/kzg_utils.rs | 31 +-- beacon_node/beacon_chain/src/test_utils.rs | 4 +- .../execution_layer/src/engine_api/http.rs | 24 +-- .../src/engine_api/json_structures.rs | 107 ++++------- .../src/engine_api/new_payload_request.rs | 2 +- beacon_node/execution_layer/src/lib.rs | 10 +- .../test_utils/execution_block_generator.rs | 59 +++--- .../src/test_utils/mock_builder.rs | 2 +- beacon_node/genesis/src/common.rs | 15 +- .../lighthouse_network/src/rpc/codec.rs | 12 +- .../lighthouse_network/src/rpc/methods.rs | 12 +- .../lighthouse_network/src/rpc/protocol.rs | 6 +- .../lighthouse_network/tests/rpc_tests.rs | 30 +-- common/eth2/src/types.rs | 4 +- .../src/per_block_processing.rs | 7 +- .../src/per_block_processing/tests.rs | 24 ++- consensus/types/src/attestation.rs | 21 +- consensus/types/src/chain_spec.rs | 3 +- consensus/types/src/light_client_bootstrap.rs | 48 +++-- .../types/src/light_client_finality_update.rs | 12 +- consensus/types/src/light_client_update.rs | 36 ++-- consensus/types/src/test_utils/test_random.rs | 2 +- slasher/src/test_utils.rs | 4 +- 30 files changed, 453 insertions(+), 276 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88b5b7b57d2..c92446204f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8923,14 +8923,15 @@ dependencies = [ [[package]] name = "ssz_types" -version = "0.11.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75b55bedc9a18ed2860a46d6beb4f4082416ee1d60be0cc364cebdcdddc7afd4" +checksum = "6026eef4cace4a29933c507de27cd7ef3e3a382c5e7ca78115f5b73519bc5d97" dependencies = [ "arbitrary", + "derivative", "ethereum_serde_utils", "ethereum_ssz", - "itertools 0.13.0", + "itertools 0.14.0", "serde", "serde_derive", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 0b930b605d6..5baa499b3a5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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"] } diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 470664d4429..f360e2fb16a 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -63,6 +63,7 @@ use types::{ Attestation, AttestationData, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, SingleAttestation, Slot, SubnetId, + attestation::Error as AttestationError, }; pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations}; @@ -107,14 +108,18 @@ pub enum Error { /// ## Peer scoring /// /// The peer has sent an invalid message. - InvalidSelectionProof { aggregator_index: u64 }, + InvalidSelectionProof { + aggregator_index: u64, + }, /// The `selection_proof` on the aggregate attestation selects it as a validator, however the /// aggregator index is not in the committee for that attestation. /// /// ## Peer scoring /// /// The peer has sent an invalid message. - AggregatorNotInCommittee { aggregator_index: u64 }, + AggregatorNotInCommittee { + aggregator_index: u64, + }, /// The `attester_index` for a `SingleAttestation` is not a member of the committee defined /// by its `beacon_block_root`, `committee_index` and `slot`. /// @@ -166,7 +171,9 @@ pub enum Error { /// /// The attestation points to a block we have not yet imported. It's unclear if the attestation /// is valid or not. - UnknownHeadBlock { beacon_block_root: Hash256 }, + UnknownHeadBlock { + beacon_block_root: Hash256, + }, /// The `attestation.data.beacon_block_root` block is from before the finalized checkpoint. /// /// ## Peer scoring @@ -174,7 +181,9 @@ pub enum Error { /// The attestation is not descended from the finalized checkpoint, which is a REJECT according /// to the spec. We downscore lightly because this could also happen if we are processing /// attestations extremely slowly. - HeadBlockFinalized { beacon_block_root: Hash256 }, + HeadBlockFinalized { + beacon_block_root: Hash256, + }, /// The `attestation.data.slot` is not from the same epoch as `data.target.epoch`. /// /// ## Peer scoring @@ -201,7 +210,10 @@ pub enum Error { /// ## Peer scoring /// /// The peer has sent an invalid message. - NoCommitteeForSlotAndIndex { slot: Slot, index: CommitteeIndex }, + NoCommitteeForSlotAndIndex { + slot: Slot, + index: CommitteeIndex, + }, /// The attestation doesn't have only one aggregation bit set. /// /// ## Peer scoring @@ -216,14 +228,20 @@ pub enum Error { /// It's unclear if this attestation is valid, however we have already observed a /// single-participant attestation from this validator for this epoch and should not observe /// another. - PriorAttestationKnown { validator_index: u64, epoch: Epoch }, + PriorAttestationKnown { + validator_index: u64, + epoch: Epoch, + }, /// The attestation is attesting to a state that is later than itself. (Viz., attesting to the /// future). /// /// ## Peer scoring /// /// The peer has sent an invalid message. - AttestsToFutureBlock { block: Slot, attestation: Slot }, + AttestsToFutureBlock { + block: Slot, + attestation: Slot, + }, /// The attestation was received on an invalid attestation subnet. /// /// ## Peer scoring @@ -250,7 +268,10 @@ pub enum Error { /// ## Peer scoring /// /// The peer has sent an invalid message. - InvalidTargetEpoch { slot: Slot, epoch: Epoch }, + InvalidTargetEpoch { + slot: Slot, + epoch: Epoch, + }, /// The attestation references an invalid target block. /// /// ## Peer scoring @@ -267,6 +288,7 @@ 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), + AttestationError(Box), } impl From for Error { @@ -442,8 +464,10 @@ fn process_slash_info( .spec .fork_name_at_slot::(attestation.data.slot); - let indexed_attestation = attestation.to_indexed(fork_name); - (indexed_attestation, true, err) + match attestation.to_indexed(fork_name) { + Ok(indexed_attestation) => (indexed_attestation, true, err), + Err(e) => return Error::AttestationError(Box::new(e)), + } } SignatureNotCheckedIndexed(indexed, err) => (indexed, true, err), SignatureInvalid(e) => return e, @@ -932,7 +956,15 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { .spec .fork_name_at_slot::(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::AttestationError(Box::new(e)), + )); + } + }; let validator_index = match Self::verify_middle_checks(attestation, chain) { Ok(t) => t, diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6e11b666102..dc247cf90a5 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5515,11 +5515,21 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_base.into(), - attestations: attestations_base.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, _phantom: PhantomData, }, }), @@ -5536,11 +5546,21 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_base.into(), - attestations: attestations_base.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, _phantom: PhantomData, @@ -5563,11 +5583,21 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_base.into(), - attestations: attestations_base.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: block_proposal_contents @@ -5595,18 +5625,30 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_base.into(), - attestations: attestations_base.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: block_proposal_contents .to_payload() .try_into() .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - bls_to_execution_changes: bls_to_execution_changes.into(), + bls_to_execution_changes: bls_to_execution_changes + .try_into() + .map_err(BlockProductionError::SszTypesError)?, }, }), None, @@ -5634,17 +5676,29 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_base.into(), - attestations: attestations_base.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_base + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: payload .try_into() .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - bls_to_execution_changes: bls_to_execution_changes.into(), + bls_to_execution_changes: bls_to_execution_changes + .try_into() + .map_err(BlockProductionError::SszTypesError)?, blob_kzg_commitments: kzg_commitments.ok_or( BlockProductionError::MissingKzgCommitment( "Kzg commitments missing from block contents".to_string(), @@ -5677,17 +5731,29 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_electra.into(), - attestations: attestations_electra.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_electra + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_electra + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: payload .try_into() .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - bls_to_execution_changes: bls_to_execution_changes.into(), + bls_to_execution_changes: bls_to_execution_changes + .try_into() + .map_err(BlockProductionError::SszTypesError)?, blob_kzg_commitments: kzg_commitments .ok_or(BlockProductionError::InvalidPayloadFork)?, execution_requests: maybe_requests @@ -5719,17 +5785,29 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_electra.into(), - attestations: attestations_electra.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_electra + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_electra + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: payload .try_into() .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - bls_to_execution_changes: bls_to_execution_changes.into(), + bls_to_execution_changes: bls_to_execution_changes + .try_into() + .map_err(BlockProductionError::SszTypesError)?, blob_kzg_commitments: kzg_commitments .ok_or(BlockProductionError::InvalidPayloadFork)?, execution_requests: maybe_requests @@ -5761,17 +5839,29 @@ impl BeaconChain { randao_reveal, eth1_data, graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_electra.into(), - attestations: attestations_electra.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), + proposer_slashings: proposer_slashings + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attester_slashings: attester_slashings_electra + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + attestations: attestations_electra + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + deposits: deposits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, + voluntary_exits: voluntary_exits + .try_into() + .map_err(BlockProductionError::SszTypesError)?, sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: payload .try_into() .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - bls_to_execution_changes: bls_to_execution_changes.into(), + bls_to_execution_changes: bls_to_execution_changes + .try_into() + .map_err(BlockProductionError::SszTypesError)?, blob_kzg_commitments: kzg_commitments .ok_or(BlockProductionError::InvalidPayloadFork)?, execution_requests: maybe_requests diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 608e003a228..ccc2fe24fab 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -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:: { 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() diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index a1a0ec74f66..a3415851869 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -300,6 +300,7 @@ pub enum BlockProductionError { KzgError(kzg::Error), FailedToBuildBlobSidecars(String), MissingExecutionRequests, + SszTypesError(ssz_types::Error), } easy_from_to!(BlockProcessingError, BlockProductionError); diff --git a/beacon_node/beacon_chain/src/fetch_blobs/tests.rs b/beacon_node/beacon_chain/src/fetch_blobs/tests.rs index e4855dd5598..cbe2f78fbda 100644 --- a/beacon_node/beacon_chain/src/fetch_blobs/tests.rs +++ b/beacon_node/beacon_chain/src/fetch_blobs/tests.rs @@ -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() diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 2147ed59663..815e62f0b2f 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -251,7 +251,8 @@ pub(crate) fn build_data_column_sidecars( .get(col) .ok_or(format!("Missing blob cell at index {col}"))?; let cell: Vec = cell.to_vec(); - let cell = Cell::::from(cell); + let cell = + Cell::::try_from(cell).map_err(|e| format!("BytesPerCell exceeded: {e:?}"))?; let proof = blob_cell_proofs .get(col) @@ -269,23 +270,27 @@ pub(crate) fn build_data_column_sidecars( } } - let sidecars: Vec>> = columns + let sidecars: Result>>, String> = columns .into_iter() .zip(column_kzg_proofs) .enumerate() - .map(|(index, (col, proofs))| { - Arc::new(DataColumnSidecar { - index: index as u64, - column: DataColumn::::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>, String> { + Ok(Arc::new(DataColumnSidecar { + index: index as u64, + column: DataColumn::::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%). diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index c2230ba0572..73bde6acae3 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2323,7 +2323,7 @@ where .collect::>(); // 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(); @@ -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::>(); diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index bc927e19b41..d2f7250aa8f 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -1814,16 +1814,16 @@ mod test { fee_recipient: Address::repeat_byte(1), state_root: Hash256::repeat_byte(1), receipts_root: Hash256::repeat_byte(0), - logs_bloom: vec![1; 256].into(), + logs_bloom: vec![1; 256].try_into().unwrap(), prev_randao: Hash256::repeat_byte(1), block_number: 0, gas_limit: 1, gas_used: 2, timestamp: 42, - extra_data: vec![].into(), + extra_data: vec![].try_into().unwrap(), base_fee_per_gas: Uint256::from(1), block_hash: ExecutionBlockHash::repeat_byte(1), - transactions: vec![].into(), + transactions: vec![].try_into().unwrap(), }, )) .await; @@ -1861,16 +1861,16 @@ mod test { fee_recipient: Address::repeat_byte(1), state_root: Hash256::repeat_byte(1), receipts_root: Hash256::repeat_byte(0), - logs_bloom: vec![1; 256].into(), + logs_bloom: vec![1; 256].try_into().unwrap(), prev_randao: Hash256::repeat_byte(1), block_number: 0, gas_limit: 1, gas_used: 2, timestamp: 42, - extra_data: vec![].into(), + extra_data: vec![].try_into().unwrap(), base_fee_per_gas: Uint256::from(1), block_hash: ExecutionBlockHash::repeat_byte(1), - transactions: vec![].into(), + transactions: vec![].try_into().unwrap(), }, )) .await @@ -2071,16 +2071,16 @@ mod test { fee_recipient: Address::from_str("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b").unwrap(), state_root: Hash256::from_str("0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45").unwrap(), receipts_root: Hash256::from_str("0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421").unwrap(), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: Hash256::zero(), block_number: 1, gas_limit: u64::from_str_radix("1c95111",16).unwrap(), gas_used: 0, timestamp: 5, - extra_data: vec![].into(), + extra_data: vec![].try_into().unwrap(), base_fee_per_gas: Uint256::from(7), block_hash: ExecutionBlockHash::from_str("0x6359b8381a370e2f54072a5784ddd78b6ed024991558c511d4452eb4f6ac898c").unwrap(), - transactions: vec![].into(), + transactions: vec![].try_into().unwrap(), }); assert_eq!(payload, expected); @@ -2096,16 +2096,16 @@ mod test { fee_recipient: Address::from_str("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b").unwrap(), state_root: Hash256::from_str("0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45").unwrap(), receipts_root: Hash256::from_str("0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421").unwrap(), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: Hash256::zero(), block_number: 1, gas_limit: u64::from_str_radix("1c9c380",16).unwrap(), gas_used: 0, timestamp: 5, - extra_data: vec![].into(), + extra_data: vec![].try_into().unwrap(), base_fee_per_gas: Uint256::from(7), block_hash: ExecutionBlockHash::from_str("0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858").unwrap(), - transactions: vec![].into(), + transactions: vec![].try_into().unwrap(), })) .await; }, diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index 33decd4ec86..3cd54c4f797 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -1,7 +1,8 @@ use super::*; use alloy_rlp::RlpEncodable; use serde::{Deserialize, Serialize}; -use ssz::Decode; +use ssz::{Decode, TryFromIter}; +use ssz_types::{FixedVector, VariableList, typenum::Unsigned}; use strum::EnumString; use superstruct::superstruct; use types::beacon_block_body::KzgCommitments; @@ -9,7 +10,7 @@ use types::blob_sidecar::BlobsList; use types::execution_requests::{ ConsolidationRequests, DepositRequests, RequestType, WithdrawalRequests, }; -use types::{Blob, FixedVector, KzgProof, Unsigned}; +use types::{Blob, KzgProof}; #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -147,12 +148,7 @@ impl From> for JsonExecutionPayloadCapell base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_to_json(payload.withdrawals), } } } @@ -173,12 +169,7 @@ impl From> for JsonExecutionPayloadDeneb base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_to_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -202,12 +193,7 @@ impl From> for JsonExecutionPayloadElectr base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_to_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -231,12 +217,7 @@ impl From> for JsonExecutionPayloadFulu { base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_to_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -260,12 +241,7 @@ impl From> for JsonExecutionPayloadGloas base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_to_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -322,12 +298,7 @@ impl From> for ExecutionPayloadCapell base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_from_json(payload.withdrawals), } } } @@ -349,12 +320,7 @@ impl From> for ExecutionPayloadDeneb base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_from_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -378,12 +344,7 @@ impl From> for ExecutionPayloadElectr base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_from_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -407,12 +368,7 @@ impl From> for ExecutionPayloadFulu { base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_from_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -436,12 +392,7 @@ impl From> for ExecutionPayloadGloas base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: payload - .withdrawals - .into_iter() - .map(Into::into) - .collect::>() - .into(), + withdrawals: withdrawals_from_json(payload.withdrawals), blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, } @@ -673,6 +624,25 @@ impl From for Withdrawal { } } } + +// Helper functions to convert between `VariableList` and `VariableList`. +// The `expect` is required to enable `from` impls between `ExecutionPayload` and `JsonExecutionPayload`. +fn withdrawals_to_json(list: VariableList) -> VariableList +where + N: Unsigned, +{ + VariableList::try_from_iter(list.into_iter().map(Into::into)) + .expect("conversion to JSON should preserve withdrawals length") +} + +fn withdrawals_from_json(list: VariableList) -> VariableList +where + N: Unsigned, +{ + VariableList::try_from_iter(list.into_iter().map(Into::into)) + .expect("conversion from JSON should preserve withdrawals length") +} + #[derive(Debug, PartialEq, Clone, RlpEncodable)] pub struct EncodableJsonWithdrawal<'a> { pub index: u64, @@ -980,14 +950,7 @@ impl From> for ExecutionPayloadBodyV1< fn from(value: JsonExecutionPayloadBodyV1) -> Self { Self { transactions: value.transactions, - withdrawals: value.withdrawals.map(|json_withdrawals| { - Withdrawals::::from( - json_withdrawals - .into_iter() - .map(Into::into) - .collect::>(), - ) - }), + withdrawals: value.withdrawals.map(withdrawals_from_json), } } } @@ -996,9 +959,7 @@ impl From> for JsonExecutionPayloadBodyV1< fn from(value: ExecutionPayloadBodyV1) -> Self { Self { transactions: value.transactions, - withdrawals: value.withdrawals.map(|withdrawals| { - VariableList::from(withdrawals.into_iter().map(Into::into).collect::>()) - }), + withdrawals: value.withdrawals.map(withdrawals_to_json), } } } diff --git a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs index aa5261c80b0..617d2e01129 100644 --- a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs +++ b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs @@ -337,7 +337,7 @@ mod test { *beacon_block .body_mut() .blob_kzg_commitments_mut() - .expect("should get commitments") = commitments.into(); + .expect("should get commitments") = commitments.try_into().unwrap(); let new_payload_request = NewPayloadRequest::try_from(beacon_block.to_ref()) .expect("should create new payload request"); diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 401646f3670..05d8b9f922c 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -2092,6 +2092,7 @@ enum InvalidBuilderPayload { payload: u64, expected: u64, }, + SszTypesError(ssz_types::Error), } impl fmt::Display for InvalidBuilderPayload { @@ -2133,6 +2134,7 @@ impl fmt::Display for InvalidBuilderPayload { InvalidBuilderPayload::GasLimitMismatch { payload, expected } => { write!(f, "payload gas limit was {} not {}", payload, expected) } + Self::SszTypesError(e) => write!(f, "{:?}", e), } } } @@ -2188,7 +2190,13 @@ fn verify_builder_bid( .withdrawals() .ok() .cloned() - .map(|withdrawals| Withdrawals::::from(withdrawals).tree_hash_root()); + .map(|withdrawals| { + Withdrawals::::try_from(withdrawals) + .map_err(InvalidBuilderPayload::SszTypesError) + .map(|w| w.tree_hash_root()) + }) + .transpose()?; + let payload_withdrawals_root = header.withdrawals_root().ok(); let expected_gas_limit = proposer_gas_limit .and_then(|target_gas_limit| expected_gas_limit(parent_gas_limit, target_gas_limit, spec)); diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index 4836f9307c8..29c764ee305 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -40,7 +40,7 @@ pub enum Block { } pub fn mock_el_extra_data() -> types::VariableList { - "block gen was here".as_bytes().to_vec().into() + "block gen was here".as_bytes().to_vec().try_into().unwrap() } impl Block { @@ -602,7 +602,7 @@ impl ExecutionBlockGenerator { fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, @@ -611,7 +611,7 @@ impl ExecutionBlockGenerator { extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), + transactions: vec![].try_into().unwrap(), }), PayloadAttributes::V2(pa) => match self.get_fork_at_timestamp(pa.timestamp) { ForkName::Bellatrix => ExecutionPayload::Bellatrix(ExecutionPayloadBellatrix { @@ -619,7 +619,7 @@ impl ExecutionBlockGenerator { fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, @@ -628,14 +628,14 @@ impl ExecutionBlockGenerator { extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), + transactions: vec![].try_into().unwrap(), }), ForkName::Capella => ExecutionPayload::Capella(ExecutionPayloadCapella { parent_hash: head_block_hash, fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, @@ -644,8 +644,8 @@ impl ExecutionBlockGenerator { extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), - withdrawals: pa.withdrawals.clone().into(), + transactions: vec![].try_into().unwrap(), + withdrawals: pa.withdrawals.clone().try_into().unwrap(), }), _ => unreachable!(), }, @@ -655,7 +655,7 @@ impl ExecutionBlockGenerator { fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, @@ -664,8 +664,8 @@ impl ExecutionBlockGenerator { extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), - withdrawals: pa.withdrawals.clone().into(), + transactions: vec![].try_into().unwrap(), + withdrawals: pa.withdrawals.clone().try_into().unwrap(), blob_gas_used: 0, excess_blob_gas: 0, }), @@ -674,7 +674,7 @@ impl ExecutionBlockGenerator { fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, @@ -683,8 +683,8 @@ impl ExecutionBlockGenerator { extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), - withdrawals: pa.withdrawals.clone().into(), + transactions: vec![].try_into().unwrap(), + withdrawals: pa.withdrawals.clone().try_into().unwrap(), blob_gas_used: 0, excess_blob_gas: 0, }), @@ -693,17 +693,17 @@ impl ExecutionBlockGenerator { fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: "block gen was here".as_bytes().to_vec().try_into().unwrap(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), - withdrawals: pa.withdrawals.clone().into(), + transactions: vec![].try_into().unwrap(), + withdrawals: pa.withdrawals.clone().try_into().unwrap(), blob_gas_used: 0, excess_blob_gas: 0, }), @@ -712,17 +712,17 @@ impl ExecutionBlockGenerator { fee_recipient: pa.suggested_fee_recipient, receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), - logs_bloom: vec![0; 256].into(), + logs_bloom: vec![0; 256].try_into().unwrap(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: "block gen was here".as_bytes().to_vec().try_into().unwrap(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), - transactions: vec![].into(), - withdrawals: pa.withdrawals.clone().into(), + transactions: vec![].try_into().unwrap(), + withdrawals: pa.withdrawals.clone().try_into().unwrap(), blob_gas_used: 0, excess_blob_gas: 0, }), @@ -813,24 +813,25 @@ pub fn generate_blobs( let bundle = if fork_name.fulu_enabled() { let (kzg_commitment, kzg_proofs, blob) = load_test_blobs_bundle_v2::()?; BlobsBundle { - commitments: vec![kzg_commitment; n_blobs].into(), + commitments: vec![kzg_commitment; n_blobs].try_into().unwrap(), proofs: vec![kzg_proofs.to_vec(); n_blobs] .into_iter() .flatten() .collect::>() - .into(), - blobs: vec![blob; n_blobs].into(), + .try_into() + .unwrap(), + blobs: vec![blob; n_blobs].try_into().unwrap(), } } else { let (kzg_commitment, kzg_proof, blob) = load_test_blobs_bundle_v1::()?; BlobsBundle { - commitments: vec![kzg_commitment; n_blobs].into(), - proofs: vec![kzg_proof; n_blobs].into(), - blobs: vec![blob; n_blobs].into(), + commitments: vec![kzg_commitment; n_blobs].try_into().unwrap(), + proofs: vec![kzg_proof; n_blobs].try_into().unwrap(), + blobs: vec![blob; n_blobs].try_into().unwrap(), } }; - Ok((bundle, transactions.into())) + Ok((bundle, transactions.try_into().unwrap())) } pub fn static_valid_tx() -> Result, String> { diff --git a/beacon_node/execution_layer/src/test_utils/mock_builder.rs b/beacon_node/execution_layer/src/test_utils/mock_builder.rs index 6b63881d856..f0991f1733f 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -72,7 +72,7 @@ impl Operation { } pub fn mock_builder_extra_data() -> types::VariableList { - "mock_builder".as_bytes().to_vec().into() + "mock_builder".as_bytes().to_vec().try_into().unwrap() } #[derive(Debug)] diff --git a/beacon_node/genesis/src/common.rs b/beacon_node/genesis/src/common.rs index e48fa362046..88a88810d8a 100644 --- a/beacon_node/genesis/src/common.rs +++ b/beacon_node/genesis/src/common.rs @@ -37,10 +37,17 @@ pub fn genesis_deposits( proofs.push(proof); } - Ok(deposit_data + deposit_data .into_iter() .zip(proofs) - .map(|(data, proof)| (data, proof.into())) - .map(|(data, proof)| Deposit { proof, data }) - .collect()) + .map(|(data, proof)| { + let converted_proof = proof + .try_into() + .map_err(|e| format!("Error converting proof: {:?}", e))?; + Ok(Deposit { + proof: converted_proof, + data, + }) + }) + .collect() } diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index acb01884564..77d2a34e16e 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -1002,8 +1002,9 @@ mod tests { let mut block: BeaconBlockBellatrix<_, FullPayload> = BeaconBlockBellatrix::empty(spec); - let tx = VariableList::from(vec![0; 1024]); - let txs = VariableList::from(std::iter::repeat_n(tx, 5000).collect::>()); + let tx = VariableList::try_from(vec![0; 1024]).unwrap(); + let txs = + VariableList::try_from(std::iter::repeat_n(tx, 5000).collect::>()).unwrap(); block.body.execution_payload.execution_payload.transactions = txs; @@ -1021,8 +1022,9 @@ mod tests { let mut block: BeaconBlockBellatrix<_, FullPayload> = BeaconBlockBellatrix::empty(spec); - let tx = VariableList::from(vec![0; 1024]); - let txs = VariableList::from(std::iter::repeat_n(tx, 100000).collect::>()); + let tx = VariableList::try_from(vec![0; 1024]).unwrap(); + let txs = + VariableList::try_from(std::iter::repeat_n(tx, 100000).collect::>()).unwrap(); block.body.execution_payload.execution_payload.transactions = txs; @@ -1080,7 +1082,7 @@ mod tests { data_column_ids: RuntimeVariableList::new( vec![DataColumnsByRootIdentifier { block_root: Hash256::zero(), - columns: VariableList::from(vec![0, 1, 2]), + columns: VariableList::try_from(vec![0, 1, 2]).unwrap(), }], spec.max_request_blocks(fork_name), ) diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 9319973e597..5439fa468c2 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -30,14 +30,22 @@ pub const MAX_ERROR_LEN: u64 = 256; pub struct ErrorType(pub VariableList); impl From for ErrorType { + // This will panic if `string.as_bytes()` exceeds `MaxErrorLen`. fn from(s: String) -> Self { - Self(VariableList::from(s.as_bytes().to_vec())) + Self( + VariableList::try_from(s.as_bytes().to_vec()) + .expect("length should not exceed MaxErrorLen"), + ) } } impl From<&str> for ErrorType { + // This will panic if `string.as_bytes()` exceeds `MaxErrorLen`. fn from(s: &str) -> Self { - Self(VariableList::from(s.as_bytes().to_vec())) + Self( + VariableList::try_from(s.as_bytes().to_vec()) + .expect("length should not exceed MaxErrorLen"), + ) } } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 228a74f08cc..08085f3c271 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -70,13 +70,15 @@ pub static BLOB_SIDECAR_SIZE_MINIMAL: LazyLock = LazyLock::new(BlobSidecar::::max_size); pub static ERROR_TYPE_MIN: LazyLock = LazyLock::new(|| { - VariableList::::from(Vec::::new()) + VariableList::::try_from(Vec::::new()) + .expect("MaxErrorLen should not exceed MAX_ERROR_LEN") .as_ssz_bytes() .len() }); pub static ERROR_TYPE_MAX: LazyLock = LazyLock::new(|| { - VariableList::::from(vec![0u8; MAX_ERROR_LEN as usize]) + VariableList::::try_from(vec![0u8; MAX_ERROR_LEN as usize]) + .expect("MaxErrorLen should not exceed MAX_ERROR_LEN") .as_ssz_bytes() .len() }); diff --git a/beacon_node/lighthouse_network/tests/rpc_tests.rs b/beacon_node/lighthouse_network/tests/rpc_tests.rs index e37f4131a76..25367608864 100644 --- a/beacon_node/lighthouse_network/tests/rpc_tests.rs +++ b/beacon_node/lighthouse_network/tests/rpc_tests.rs @@ -26,8 +26,8 @@ type E = MinimalEthSpec; /// Bellatrix block with length < max_rpc_size. fn bellatrix_block_small(spec: &ChainSpec) -> BeaconBlock { let mut block = BeaconBlockBellatrix::::empty(spec); - let tx = VariableList::from(vec![0; 1024]); - let txs = VariableList::from(std::iter::repeat_n(tx, 5000).collect::>()); + let tx = VariableList::try_from(vec![0; 1024]).unwrap(); + let txs = VariableList::try_from(std::iter::repeat_n(tx, 5000).collect::>()).unwrap(); block.body.execution_payload.execution_payload.transactions = txs; @@ -41,8 +41,8 @@ fn bellatrix_block_small(spec: &ChainSpec) -> BeaconBlock { /// Hence, we generate a bellatrix block just greater than `MAX_RPC_SIZE` to test rejection on the rpc layer. fn bellatrix_block_large(spec: &ChainSpec) -> BeaconBlock { let mut block = BeaconBlockBellatrix::::empty(spec); - let tx = VariableList::from(vec![0; 1024]); - let txs = VariableList::from(std::iter::repeat_n(tx, 100000).collect::>()); + let tx = VariableList::try_from(vec![0; 1024]).unwrap(); + let txs = VariableList::try_from(std::iter::repeat_n(tx, 100000).collect::>()).unwrap(); block.body.execution_payload.execution_payload.transactions = txs; @@ -1018,14 +1018,17 @@ fn test_tcp_columns_by_root_chunked_rpc() { }, signature: Signature::empty(), }, - column: vec![vec![0; E::bytes_per_blob()].into()].into(), - kzg_commitments: vec![KzgCommitment::empty_for_testing()].into(), - kzg_proofs: vec![KzgProof::empty()].into(), + column: vec![vec![0; E::bytes_per_blob()].try_into().unwrap()] + .try_into() + .unwrap(), + kzg_commitments: vec![KzgCommitment::empty_for_testing()].try_into().unwrap(), + kzg_proofs: vec![KzgProof::empty()].try_into().unwrap(), kzg_commitments_inclusion_proof: vec![ Hash256::zero(); E::kzg_commitments_inclusion_proof_depth() ] - .into(), + .try_into() + .unwrap(), }); let rpc_response = Response::DataColumnsByRoot(Some(data_column.clone())); @@ -1160,14 +1163,17 @@ fn test_tcp_columns_by_range_chunked_rpc() { }, signature: Signature::empty(), }, - column: vec![vec![0; E::bytes_per_blob()].into()].into(), - kzg_commitments: vec![KzgCommitment::empty_for_testing()].into(), - kzg_proofs: vec![KzgProof::empty()].into(), + column: vec![vec![0; E::bytes_per_blob()].try_into().unwrap()] + .try_into() + .unwrap(), + kzg_commitments: vec![KzgCommitment::empty_for_testing()].try_into().unwrap(), + kzg_proofs: vec![KzgProof::empty()].try_into().unwrap(), kzg_commitments_inclusion_proof: vec![ Hash256::zero(); E::kzg_commitments_inclusion_proof_depth() ] - .into(), + .try_into() + .unwrap(), }); let rpc_response = Response::DataColumnsByRange(Some(data_column.clone())); diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index b72ab293801..36dfc3ab4b5 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1683,8 +1683,8 @@ mod tests { BeaconBlock::::Deneb(BeaconBlockDeneb::empty(&spec)), Signature::empty(), ); - let blobs = BlobsList::::from(vec![Blob::::default()]); - let kzg_proofs = KzgProofs::::from(vec![KzgProof::empty()]); + let blobs = BlobsList::::try_from(vec![Blob::::default()]).unwrap(); + let kzg_proofs = KzgProofs::::try_from(vec![KzgProof::empty()]).unwrap(); let signed_block_contents = PublishBlockRequest::new(Arc::new(block), Some((kzg_proofs, blobs))); diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 99abbef9c1e..9e7a20040e8 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -628,7 +628,12 @@ pub fn get_expected_withdrawals( .safe_rem(state.validators().len() as u64)?; } - Ok((withdrawals.into(), processed_partial_withdrawals_count)) + Ok(( + withdrawals + .try_into() + .map_err(BlockProcessingError::SszTypesError)?, + processed_partial_withdrawals_count, + )) } /// Apply withdrawals to the state. diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 183063ac762..c32797f77f3 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -213,7 +213,7 @@ async fn valid_4_deposits() { let mut state = harness.get_current_state(); let (deposits, state) = harness.make_deposits(&mut state, 4, None, None); - let deposits = VariableList::from(deposits); + let deposits = VariableList::try_from(deposits).unwrap(); let mut head_block = harness .chain @@ -237,7 +237,7 @@ async fn invalid_deposit_deposit_count_too_big() { let mut state = harness.get_current_state(); let (deposits, state) = harness.make_deposits(&mut state, 1, None, None); - let deposits = VariableList::from(deposits); + let deposits = VariableList::try_from(deposits).unwrap(); let mut head_block = harness .chain @@ -269,7 +269,7 @@ async fn invalid_deposit_count_too_small() { let mut state = harness.get_current_state(); let (deposits, state) = harness.make_deposits(&mut state, 1, None, None); - let deposits = VariableList::from(deposits); + let deposits = VariableList::try_from(deposits).unwrap(); let mut head_block = harness .chain @@ -301,7 +301,7 @@ async fn invalid_deposit_bad_merkle_proof() { let mut state = harness.get_current_state(); let (deposits, state) = harness.make_deposits(&mut state, 1, None, None); - let deposits = VariableList::from(deposits); + let deposits = VariableList::try_from(deposits).unwrap(); let mut head_block = harness .chain @@ -336,7 +336,7 @@ async fn invalid_deposit_wrong_sig() { let (deposits, state) = harness.make_deposits(&mut state, 1, None, Some(SignatureBytes::empty())); - let deposits = VariableList::from(deposits); + let deposits = VariableList::try_from(deposits).unwrap(); let mut head_block = harness .chain @@ -360,7 +360,7 @@ async fn invalid_deposit_invalid_pub_key() { let (deposits, state) = harness.make_deposits(&mut state, 1, Some(PublicKeyBytes::empty()), None); - let deposits = VariableList::from(deposits); + let deposits = VariableList::try_from(deposits).unwrap(); let mut head_block = harness .chain @@ -753,10 +753,12 @@ async fn invalid_attester_slashing_1_invalid() { let mut attester_slashing = harness.make_attester_slashing(vec![1, 2]); match &mut attester_slashing { AttesterSlashing::Base(attester_slashing) => { - attester_slashing.attestation_1.attesting_indices = VariableList::from(vec![2, 1]); + attester_slashing.attestation_1.attesting_indices = + VariableList::try_from(vec![2, 1]).unwrap(); } AttesterSlashing::Electra(attester_slashing) => { - attester_slashing.attestation_1.attesting_indices = VariableList::from(vec![2, 1]); + attester_slashing.attestation_1.attesting_indices = + VariableList::try_from(vec![2, 1]).unwrap(); } } @@ -791,10 +793,12 @@ async fn invalid_attester_slashing_2_invalid() { let mut attester_slashing = harness.make_attester_slashing(vec![1, 2]); match &mut attester_slashing { AttesterSlashing::Base(attester_slashing) => { - attester_slashing.attestation_2.attesting_indices = VariableList::from(vec![2, 1]); + attester_slashing.attestation_2.attesting_indices = + VariableList::try_from(vec![2, 1]).unwrap(); } AttesterSlashing::Electra(attester_slashing) => { - attester_slashing.attestation_2.attesting_indices = VariableList::from(vec![2, 1]); + attester_slashing.attestation_2.attesting_indices = + VariableList::try_from(vec![2, 1]).unwrap(); } } diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 860f0d0a2d3..14d25425a24 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -622,19 +622,26 @@ pub struct SingleAttestation { } impl SingleAttestation { - pub fn to_indexed(&self, fork_name: ForkName) -> IndexedAttestation { + pub fn to_indexed( + &self, + fork_name: ForkName, + ) -> Result, Error> { if fork_name.electra_enabled() { - IndexedAttestation::Electra(IndexedAttestationElectra { - attesting_indices: vec![self.attester_index].into(), + Ok(IndexedAttestation::Electra(IndexedAttestationElectra { + attesting_indices: vec![self.attester_index] + .try_into() + .map_err(Error::SszTypesError)?, data: self.data.clone(), signature: self.signature.clone(), - }) + })) } else { - IndexedAttestation::Base(IndexedAttestationBase { - attesting_indices: vec![self.attester_index].into(), + Ok(IndexedAttestation::Base(IndexedAttestationBase { + attesting_indices: vec![self.attester_index] + .try_into() + .map_err(Error::SszTypesError)?, data: self.data.clone(), signature: self.signature.clone(), - }) + })) } } } diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index a1005d904ae..6238b582671 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -2031,7 +2031,8 @@ fn max_data_columns_by_root_request_common(max_request_blocks: u64) let empty_data_columns_by_root_id = DataColumnsByRootIdentifier { block_root: Hash256::zero(), - columns: VariableList::from(vec![0; E::number_of_columns()]), + columns: VariableList::try_from(vec![0; E::number_of_columns()]) + .expect("VariableList::try_from should succeed"), }; RuntimeVariableList::>::new( diff --git a/consensus/types/src/light_client_bootstrap.rs b/consensus/types/src/light_client_bootstrap.rs index 5850db876c2..530ccd88dee 100644 --- a/consensus/types/src/light_client_bootstrap.rs +++ b/consensus/types/src/light_client_bootstrap.rs @@ -151,32 +151,44 @@ impl LightClientBootstrap { ForkName::Altair | ForkName::Bellatrix => Self::Altair(LightClientBootstrapAltair { header: LightClientHeaderAltair::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Capella => Self::Capella(LightClientBootstrapCapella { header: LightClientHeaderCapella::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Deneb => Self::Deneb(LightClientBootstrapDeneb { header: LightClientHeaderDeneb::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Electra => Self::Electra(LightClientBootstrapElectra { header: LightClientHeaderElectra::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Fulu => Self::Fulu(LightClientBootstrapFulu { header: LightClientHeaderFulu::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Gloas => Self::Gloas(LightClientBootstrapGloas { header: LightClientHeaderGloas::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), }; @@ -201,32 +213,44 @@ impl LightClientBootstrap { ForkName::Altair | ForkName::Bellatrix => Self::Altair(LightClientBootstrapAltair { header: LightClientHeaderAltair::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Capella => Self::Capella(LightClientBootstrapCapella { header: LightClientHeaderCapella::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Deneb => Self::Deneb(LightClientBootstrapDeneb { header: LightClientHeaderDeneb::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Electra => Self::Electra(LightClientBootstrapElectra { header: LightClientHeaderElectra::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Fulu => Self::Fulu(LightClientBootstrapFulu { header: LightClientHeaderFulu::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), ForkName::Gloas => Self::Gloas(LightClientBootstrapGloas { header: LightClientHeaderGloas::block_to_light_client_header(block)?, current_sync_committee, - current_sync_committee_branch: current_sync_committee_branch.into(), + current_sync_committee_branch: current_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, }), }; diff --git a/consensus/types/src/light_client_finality_update.rs b/consensus/types/src/light_client_finality_update.rs index 4fa98de40be..644824f12c2 100644 --- a/consensus/types/src/light_client_finality_update.rs +++ b/consensus/types/src/light_client_finality_update.rs @@ -116,7 +116,7 @@ impl LightClientFinalityUpdate { finalized_header: LightClientHeaderAltair::block_to_light_client_header( finalized_block, )?, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate, signature_slot, }) @@ -128,7 +128,7 @@ impl LightClientFinalityUpdate { finalized_header: LightClientHeaderCapella::block_to_light_client_header( finalized_block, )?, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate, signature_slot, }), @@ -139,7 +139,7 @@ impl LightClientFinalityUpdate { finalized_header: LightClientHeaderDeneb::block_to_light_client_header( finalized_block, )?, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate, signature_slot, }), @@ -150,7 +150,7 @@ impl LightClientFinalityUpdate { finalized_header: LightClientHeaderElectra::block_to_light_client_header( finalized_block, )?, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate, signature_slot, }), @@ -161,7 +161,7 @@ impl LightClientFinalityUpdate { finalized_header: LightClientHeaderFulu::block_to_light_client_header( finalized_block, )?, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate, signature_slot, }), @@ -172,7 +172,7 @@ impl LightClientFinalityUpdate { finalized_header: LightClientHeaderGloas::block_to_light_client_header( finalized_block, )?, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate, signature_slot, }), diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index bf1a8c614a7..afb7ebc96dc 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -261,9 +261,11 @@ impl LightClientUpdate { Self::Altair(LightClientUpdateAltair { attested_header, next_sync_committee, - next_sync_committee_branch: next_sync_committee_branch.into(), + next_sync_committee_branch: next_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, finalized_header, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate: sync_aggregate.clone(), signature_slot: block_slot, }) @@ -285,9 +287,11 @@ impl LightClientUpdate { Self::Capella(LightClientUpdateCapella { attested_header, next_sync_committee, - next_sync_committee_branch: next_sync_committee_branch.into(), + next_sync_committee_branch: next_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, finalized_header, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate: sync_aggregate.clone(), signature_slot: block_slot, }) @@ -309,9 +313,11 @@ impl LightClientUpdate { Self::Deneb(LightClientUpdateDeneb { attested_header, next_sync_committee, - next_sync_committee_branch: next_sync_committee_branch.into(), + next_sync_committee_branch: next_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, finalized_header, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate: sync_aggregate.clone(), signature_slot: block_slot, }) @@ -333,9 +339,11 @@ impl LightClientUpdate { Self::Electra(LightClientUpdateElectra { attested_header, next_sync_committee, - next_sync_committee_branch: next_sync_committee_branch.into(), + next_sync_committee_branch: next_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, finalized_header, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate: sync_aggregate.clone(), signature_slot: block_slot, }) @@ -357,9 +365,11 @@ impl LightClientUpdate { Self::Fulu(LightClientUpdateFulu { attested_header, next_sync_committee, - next_sync_committee_branch: next_sync_committee_branch.into(), + next_sync_committee_branch: next_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, finalized_header, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate: sync_aggregate.clone(), signature_slot: block_slot, }) @@ -381,9 +391,11 @@ impl LightClientUpdate { Self::Gloas(LightClientUpdateGloas { attested_header, next_sync_committee, - next_sync_committee_branch: next_sync_committee_branch.into(), + next_sync_committee_branch: next_sync_committee_branch + .try_into() + .map_err(Error::SszTypesError)?, finalized_header, - finality_branch: finality_branch.into(), + finality_branch: finality_branch.try_into().map_err(Error::SszTypesError)?, sync_aggregate: sync_aggregate.clone(), signature_slot: block_slot, }) diff --git a/consensus/types/src/test_utils/test_random.rs b/consensus/types/src/test_utils/test_random.rs index 98bb8565dd6..7c8f86e14df 100644 --- a/consensus/types/src/test_utils/test_random.rs +++ b/consensus/types/src/test_utils/test_random.rs @@ -115,7 +115,7 @@ where } } - output.into() + output.try_into().unwrap() } } diff --git a/slasher/src/test_utils.rs b/slasher/src/test_utils.rs index 26338a019a2..bbbadac7618 100644 --- a/slasher/src/test_utils.rs +++ b/slasher/src/test_utils.rs @@ -17,7 +17,7 @@ pub fn indexed_att_electra( target_root: u64, ) -> IndexedAttestation { IndexedAttestation::Electra(IndexedAttestationElectra { - attesting_indices: attesting_indices.as_ref().to_vec().into(), + attesting_indices: attesting_indices.as_ref().to_vec().try_into().unwrap(), data: AttestationData { slot: Slot::new(0), index: 0, @@ -42,7 +42,7 @@ pub fn indexed_att( target_root: u64, ) -> IndexedAttestation { IndexedAttestation::Base(IndexedAttestationBase { - attesting_indices: attesting_indices.as_ref().to_vec().into(), + attesting_indices: attesting_indices.as_ref().to_vec().try_into().unwrap(), data: AttestationData { slot: Slot::new(0), index: 0, From 9cafd21e153e097860d2ce89d872bc99e061253a Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 15 Sep 2025 01:57:28 +1000 Subject: [PATCH 2/7] Fixes --- beacon_node/beacon_chain/benches/benches.rs | 7 ++- .../src/attestation_verification.rs | 54 ++++--------------- .../beacon_chain/tests/block_verification.rs | 12 +++-- consensus/fork_choice/tests/tests.rs | 4 +- consensus/types/src/attestation.rs | 17 +++--- lcli/src/http_sync.rs | 4 +- testing/ef_tests/src/cases/ssz_generic.rs | 2 +- 7 files changed, 36 insertions(+), 64 deletions(-) diff --git a/beacon_node/beacon_chain/benches/benches.rs b/beacon_node/beacon_chain/benches/benches.rs index d090fc35f74..de3ced3be11 100644 --- a/beacon_node/beacon_chain/benches/benches.rs +++ b/beacon_node/beacon_chain/benches/benches.rs @@ -26,8 +26,11 @@ fn create_test_block_and_blobs( let blobs = (0..num_of_blobs) .map(|_| Blob::::default()) .collect::>() - .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) } diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index f360e2fb16a..470664d4429 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -63,7 +63,6 @@ use types::{ Attestation, AttestationData, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, SingleAttestation, Slot, SubnetId, - attestation::Error as AttestationError, }; pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations}; @@ -108,18 +107,14 @@ pub enum Error { /// ## Peer scoring /// /// The peer has sent an invalid message. - InvalidSelectionProof { - aggregator_index: u64, - }, + InvalidSelectionProof { aggregator_index: u64 }, /// The `selection_proof` on the aggregate attestation selects it as a validator, however the /// aggregator index is not in the committee for that attestation. /// /// ## Peer scoring /// /// The peer has sent an invalid message. - AggregatorNotInCommittee { - aggregator_index: u64, - }, + AggregatorNotInCommittee { aggregator_index: u64 }, /// The `attester_index` for a `SingleAttestation` is not a member of the committee defined /// by its `beacon_block_root`, `committee_index` and `slot`. /// @@ -171,9 +166,7 @@ pub enum Error { /// /// The attestation points to a block we have not yet imported. It's unclear if the attestation /// is valid or not. - UnknownHeadBlock { - beacon_block_root: Hash256, - }, + UnknownHeadBlock { beacon_block_root: Hash256 }, /// The `attestation.data.beacon_block_root` block is from before the finalized checkpoint. /// /// ## Peer scoring @@ -181,9 +174,7 @@ pub enum Error { /// The attestation is not descended from the finalized checkpoint, which is a REJECT according /// to the spec. We downscore lightly because this could also happen if we are processing /// attestations extremely slowly. - HeadBlockFinalized { - beacon_block_root: Hash256, - }, + HeadBlockFinalized { beacon_block_root: Hash256 }, /// The `attestation.data.slot` is not from the same epoch as `data.target.epoch`. /// /// ## Peer scoring @@ -210,10 +201,7 @@ pub enum Error { /// ## Peer scoring /// /// The peer has sent an invalid message. - NoCommitteeForSlotAndIndex { - slot: Slot, - index: CommitteeIndex, - }, + NoCommitteeForSlotAndIndex { slot: Slot, index: CommitteeIndex }, /// The attestation doesn't have only one aggregation bit set. /// /// ## Peer scoring @@ -228,20 +216,14 @@ pub enum Error { /// It's unclear if this attestation is valid, however we have already observed a /// single-participant attestation from this validator for this epoch and should not observe /// another. - PriorAttestationKnown { - validator_index: u64, - epoch: Epoch, - }, + PriorAttestationKnown { validator_index: u64, epoch: Epoch }, /// The attestation is attesting to a state that is later than itself. (Viz., attesting to the /// future). /// /// ## Peer scoring /// /// The peer has sent an invalid message. - AttestsToFutureBlock { - block: Slot, - attestation: Slot, - }, + AttestsToFutureBlock { block: Slot, attestation: Slot }, /// The attestation was received on an invalid attestation subnet. /// /// ## Peer scoring @@ -268,10 +250,7 @@ pub enum Error { /// ## Peer scoring /// /// The peer has sent an invalid message. - InvalidTargetEpoch { - slot: Slot, - epoch: Epoch, - }, + InvalidTargetEpoch { slot: Slot, epoch: Epoch }, /// The attestation references an invalid target block. /// /// ## Peer scoring @@ -288,7 +267,6 @@ 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), - AttestationError(Box), } impl From for Error { @@ -464,10 +442,8 @@ fn process_slash_info( .spec .fork_name_at_slot::(attestation.data.slot); - match attestation.to_indexed(fork_name) { - Ok(indexed_attestation) => (indexed_attestation, true, err), - Err(e) => return Error::AttestationError(Box::new(e)), - } + let indexed_attestation = attestation.to_indexed(fork_name); + (indexed_attestation, true, err) } SignatureNotCheckedIndexed(indexed, err) => (indexed, true, err), SignatureInvalid(e) => return e, @@ -956,15 +932,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { .spec .fork_name_at_slot::(attestation.data.slot); - let indexed_attestation = match attestation.to_indexed(fork_name) { - Ok(indexed) => indexed, - Err(e) => { - return Err(SignatureNotCheckedSingle( - attestation, - Error::AttestationError(Box::new(e)), - )); - } - }; + let indexed_attestation = attestation.to_indexed(fork_name); let validator_index = match Self::verify_middle_checks(attestation, chain) { Ok(t) => t, diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 58ca4a032ed..a3ed73e334a 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -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, @@ -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, @@ -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(), @@ -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), diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 25c3f03d3b9..67b792ef0d8 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -753,10 +753,10 @@ async fn invalid_attestation_empty_bitfield() { MutationDelay::NoDelay, |attestation, _| match attestation { IndexedAttestation::Base(att) => { - att.attesting_indices = vec![].into(); + att.attesting_indices = vec![].try_into().unwrap(); } IndexedAttestation::Electra(att) => { - att.attesting_indices = vec![].into(); + att.attesting_indices = vec![].try_into().unwrap(); } }, |result| { diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 14d25425a24..6d661ffb66b 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -622,26 +622,23 @@ pub struct SingleAttestation { } impl SingleAttestation { - pub fn to_indexed( - &self, - fork_name: ForkName, - ) -> Result, Error> { + pub fn to_indexed(&self, fork_name: ForkName) -> IndexedAttestation { if fork_name.electra_enabled() { - Ok(IndexedAttestation::Electra(IndexedAttestationElectra { + IndexedAttestation::Electra(IndexedAttestationElectra { attesting_indices: vec![self.attester_index] .try_into() - .map_err(Error::SszTypesError)?, + .expect("Single index should not exceed MaxValidatorsPerSlot"), data: self.data.clone(), signature: self.signature.clone(), - })) + }) } else { - Ok(IndexedAttestation::Base(IndexedAttestationBase { + IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: vec![self.attester_index] .try_into() - .map_err(Error::SszTypesError)?, + .expect("Single index should not exceed MaxValidatorsPerCommittee"), data: self.data.clone(), signature: self.signature.clone(), - })) + }) } } } diff --git a/lcli/src/http_sync.rs b/lcli/src/http_sync.rs index 2e36eadf235..5a35750f288 100644 --- a/lcli/src/http_sync.rs +++ b/lcli/src/http_sync.rs @@ -139,8 +139,8 @@ async fn get_block_from_source( let block_root = block_from_source.canonical_root(); let block_contents = SignedBlockContents { signed_block: Arc::new(block_from_source), - kzg_proofs: kzg_proofs.into(), - blobs: blobs.into(), + kzg_proofs: kzg_proofs.try_into().unwrap(), + blobs: blobs.try_into().unwrap(), }; let publish_block_req = PublishBlockRequest::BlockContents(block_contents); diff --git a/testing/ef_tests/src/cases/ssz_generic.rs b/testing/ef_tests/src/cases/ssz_generic.rs index 4152711aee7..399c10316a6 100644 --- a/testing/ef_tests/src/cases/ssz_generic.rs +++ b/testing/ef_tests/src/cases/ssz_generic.rs @@ -326,6 +326,6 @@ where N::to_usize() ))) } else { - Ok(decoded.into()) + Ok(decoded.try_into().unwrap()) } } From 9565bacaa9ca040b4cac9a596092e71337dadb5b Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 15 Sep 2025 05:54:41 +1000 Subject: [PATCH 3/7] Fix discovered bug in rpc tests --- beacon_node/lighthouse_network/tests/rpc_tests.rs | 4 ++-- consensus/types/src/eth_spec.rs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/beacon_node/lighthouse_network/tests/rpc_tests.rs b/beacon_node/lighthouse_network/tests/rpc_tests.rs index 25367608864..81d08764a5f 100644 --- a/beacon_node/lighthouse_network/tests/rpc_tests.rs +++ b/beacon_node/lighthouse_network/tests/rpc_tests.rs @@ -1018,7 +1018,7 @@ fn test_tcp_columns_by_root_chunked_rpc() { }, signature: Signature::empty(), }, - column: vec![vec![0; E::bytes_per_blob()].try_into().unwrap()] + column: vec![vec![0; E::bytes_per_cell()].try_into().unwrap()] .try_into() .unwrap(), kzg_commitments: vec![KzgCommitment::empty_for_testing()].try_into().unwrap(), @@ -1163,7 +1163,7 @@ fn test_tcp_columns_by_range_chunked_rpc() { }, signature: Signature::empty(), }, - column: vec![vec![0; E::bytes_per_blob()].try_into().unwrap()] + column: vec![vec![0; E::bytes_per_cell()].try_into().unwrap()] .try_into() .unwrap(), kzg_commitments: vec![KzgCommitment::empty_for_testing()].try_into().unwrap(), diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index e001cf0e4e9..47d32ad9e4d 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -310,6 +310,11 @@ pub trait EthSpec: 'static + Default + Sync + Send + Clone + Debug + PartialEq + Self::BytesPerBlob::to_usize() } + /// Returns the `BYTES_PER_CELL` constant for this specification. + fn bytes_per_cell() -> usize { + Self::BytesPerCell::to_usize() + } + /// Returns the `KZG_COMMITMENT_INCLUSION_PROOF_DEPTH` preset for this specification. fn kzg_proof_inclusion_proof_depth() -> usize { Self::KzgCommitmentInclusionProofDepth::to_usize() From bd9dc392dfa9fcc0d26fd9e221bf0eb7c5734338 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 15 Sep 2025 19:32:35 +1000 Subject: [PATCH 4/7] Use truncating ErrorType --- .../lighthouse_network/src/rpc/methods.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 5439fa468c2..9aab0799521 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -29,23 +29,21 @@ pub const MAX_ERROR_LEN: u64 = 256; #[derive(Debug, Clone)] pub struct ErrorType(pub VariableList); -impl From for ErrorType { - // This will panic if `string.as_bytes()` exceeds `MaxErrorLen`. - fn from(s: String) -> Self { +impl From<&str> for ErrorType { + // This will truncate the error if `string.as_bytes()` exceeds `MaxErrorLen`. + fn from(s: &str) -> Self { + let mut bytes = s.as_bytes().to_vec(); + bytes.truncate(MAX_ERROR_LEN as usize); Self( - VariableList::try_from(s.as_bytes().to_vec()) - .expect("length should not exceed MaxErrorLen"), + VariableList::try_from(bytes) + .expect("length should not exceed MaxErrorLen after truncation"), ) } } -impl From<&str> for ErrorType { - // This will panic if `string.as_bytes()` exceeds `MaxErrorLen`. - fn from(s: &str) -> Self { - Self( - VariableList::try_from(s.as_bytes().to_vec()) - .expect("length should not exceed MaxErrorLen"), - ) +impl From for ErrorType { + fn from(s: String) -> Self { + Self::from(s.as_str()) } } From 8dd6cadf22a27df08cc734aadb7dad4a338f95b0 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 15 Sep 2025 19:51:47 +1000 Subject: [PATCH 5/7] Remove manual check --- testing/ef_tests/src/cases/ssz_generic.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/testing/ef_tests/src/cases/ssz_generic.rs b/testing/ef_tests/src/cases/ssz_generic.rs index 399c10316a6..8742f8a1409 100644 --- a/testing/ef_tests/src/cases/ssz_generic.rs +++ b/testing/ef_tests/src/cases/ssz_generic.rs @@ -318,14 +318,13 @@ where { let s: String = serde::de::Deserialize::deserialize(deserializer)?; let decoded: Vec = hex::decode(&s.as_str()[2..]).map_err(D::Error::custom)?; + let decoded_len = decoded.len(); - if decoded.len() > N::to_usize() { - Err(D::Error::custom(format!( + decoded.try_into().map_err(|_| { + D::Error::custom(format!( "Too many values for list, got: {}, limit: {}", - decoded.len(), + decoded_len, N::to_usize() - ))) - } else { - Ok(decoded.try_into().unwrap()) - } + )) + }) } From 6aa826bbb7c058f1ee2ecee1e220142efaa9f239 Mon Sep 17 00:00:00 2001 From: Mac L Date: Sun, 21 Sep 2025 01:18:05 +1000 Subject: [PATCH 6/7] Remove expects on json structures --- .../execution_layer/src/engine_api/http.rs | 50 +++- .../src/engine_api/json_structures.rs | 237 +++++++++++------- beacon_node/execution_layer/src/lib.rs | 7 + .../src/test_utils/handle_rpc.rs | 190 +++++++------- 4 files changed, 293 insertions(+), 191 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index d2f7250aa8f..74fb078510e 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -768,7 +768,7 @@ impl HttpJsonRpc { &self, execution_payload: ExecutionPayload, ) -> Result { - let params = json!([JsonExecutionPayload::from(execution_payload)]); + let params = json!([JsonExecutionPayload::try_from(execution_payload)?]); let response: JsonPayloadStatusV1 = self .rpc_request( @@ -785,7 +785,7 @@ impl HttpJsonRpc { &self, execution_payload: ExecutionPayload, ) -> Result { - let params = json!([JsonExecutionPayload::from(execution_payload)]); + let params = json!([JsonExecutionPayload::try_from(execution_payload)?]); let response: JsonPayloadStatusV1 = self .rpc_request( @@ -803,7 +803,12 @@ impl HttpJsonRpc { new_payload_request_deneb: NewPayloadRequestDeneb<'_, E>, ) -> Result { let params = json!([ - JsonExecutionPayload::Deneb(new_payload_request_deneb.execution_payload.clone().into()), + JsonExecutionPayload::Deneb( + new_payload_request_deneb + .execution_payload + .clone() + .try_into()? + ), new_payload_request_deneb.versioned_hashes, new_payload_request_deneb.parent_beacon_block_root, ]); @@ -825,7 +830,10 @@ impl HttpJsonRpc { ) -> Result { let params = json!([ JsonExecutionPayload::Electra( - new_payload_request_electra.execution_payload.clone().into() + new_payload_request_electra + .execution_payload + .clone() + .try_into()? ), new_payload_request_electra.versioned_hashes, new_payload_request_electra.parent_beacon_block_root, @@ -850,7 +858,12 @@ impl HttpJsonRpc { new_payload_request_fulu: NewPayloadRequestFulu<'_, E>, ) -> Result { let params = json!([ - JsonExecutionPayload::Fulu(new_payload_request_fulu.execution_payload.clone().into()), + JsonExecutionPayload::Fulu( + new_payload_request_fulu + .execution_payload + .clone() + .try_into()? + ), new_payload_request_fulu.versioned_hashes, new_payload_request_fulu.parent_beacon_block_root, new_payload_request_fulu @@ -874,7 +887,12 @@ impl HttpJsonRpc { new_payload_request_gloas: NewPayloadRequestGloas<'_, E>, ) -> Result { let params = json!([ - JsonExecutionPayload::Gloas(new_payload_request_gloas.execution_payload.clone().into()), + JsonExecutionPayload::Gloas( + new_payload_request_gloas + .execution_payload + .clone() + .try_into()? + ), new_payload_request_gloas.versioned_hashes, new_payload_request_gloas.parent_beacon_block_root, new_payload_request_gloas @@ -1125,10 +1143,14 @@ impl HttpJsonRpc { ) .await?; - Ok(response + response .into_iter() - .map(|opt_json| opt_json.map(From::from)) - .collect()) + .map(|opt_json| { + opt_json + .map(|json| json.try_into().map_err(Error::from)) + .transpose() + }) + .collect::, _>>() } pub async fn get_payload_bodies_by_range_v1( @@ -1149,10 +1171,14 @@ impl HttpJsonRpc { ) .await?; - Ok(response + response .into_iter() - .map(|opt_json| opt_json.map(From::from)) - .collect()) + .map(|opt_json| { + opt_json + .map(|json| json.try_into().map_err(Error::from)) + .transpose() + }) + .collect::, _>>() } pub async fn exchange_capabilities(&self) -> Result { diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index 3cd54c4f797..cc46070325d 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -131,9 +131,11 @@ impl From> for JsonExecutionPayloadBell } } } -impl From> for JsonExecutionPayloadCapella { - fn from(payload: ExecutionPayloadCapella) -> Self { - JsonExecutionPayloadCapella { +impl TryFrom> for JsonExecutionPayloadCapella { + type Error = ssz_types::Error; + + fn try_from(payload: ExecutionPayloadCapella) -> Result { + Ok(JsonExecutionPayloadCapella { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -148,13 +150,15 @@ impl From> for JsonExecutionPayloadCapell base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_to_json(payload.withdrawals), - } + withdrawals: withdrawals_to_json(payload.withdrawals)?, + }) } } -impl From> for JsonExecutionPayloadDeneb { - fn from(payload: ExecutionPayloadDeneb) -> Self { - JsonExecutionPayloadDeneb { +impl TryFrom> for JsonExecutionPayloadDeneb { + type Error = ssz_types::Error; + + fn try_from(payload: ExecutionPayloadDeneb) -> Result { + Ok(JsonExecutionPayloadDeneb { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -169,16 +173,18 @@ impl From> for JsonExecutionPayloadDeneb base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_to_json(payload.withdrawals), + withdrawals: withdrawals_to_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for JsonExecutionPayloadElectra { - fn from(payload: ExecutionPayloadElectra) -> Self { - JsonExecutionPayloadElectra { +impl TryFrom> for JsonExecutionPayloadElectra { + type Error = ssz_types::Error; + + fn try_from(payload: ExecutionPayloadElectra) -> Result { + Ok(JsonExecutionPayloadElectra { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -193,16 +199,18 @@ impl From> for JsonExecutionPayloadElectr base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_to_json(payload.withdrawals), + withdrawals: withdrawals_to_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for JsonExecutionPayloadFulu { - fn from(payload: ExecutionPayloadFulu) -> Self { - JsonExecutionPayloadFulu { +impl TryFrom> for JsonExecutionPayloadFulu { + type Error = ssz_types::Error; + + fn try_from(payload: ExecutionPayloadFulu) -> Result { + Ok(JsonExecutionPayloadFulu { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -217,16 +225,18 @@ impl From> for JsonExecutionPayloadFulu { base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_to_json(payload.withdrawals), + withdrawals: withdrawals_to_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for JsonExecutionPayloadGloas { - fn from(payload: ExecutionPayloadGloas) -> Self { - JsonExecutionPayloadGloas { +impl TryFrom> for JsonExecutionPayloadGloas { + type Error = ssz_types::Error; + + fn try_from(payload: ExecutionPayloadGloas) -> Result { + Ok(JsonExecutionPayloadGloas { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -241,22 +251,34 @@ impl From> for JsonExecutionPayloadGloas base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_to_json(payload.withdrawals), + withdrawals: withdrawals_to_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for JsonExecutionPayload { - fn from(execution_payload: ExecutionPayload) -> Self { +impl TryFrom> for JsonExecutionPayload { + type Error = ssz_types::Error; + + fn try_from(execution_payload: ExecutionPayload) -> Result { match execution_payload { - ExecutionPayload::Bellatrix(payload) => JsonExecutionPayload::Bellatrix(payload.into()), - ExecutionPayload::Capella(payload) => JsonExecutionPayload::Capella(payload.into()), - ExecutionPayload::Deneb(payload) => JsonExecutionPayload::Deneb(payload.into()), - ExecutionPayload::Electra(payload) => JsonExecutionPayload::Electra(payload.into()), - ExecutionPayload::Fulu(payload) => JsonExecutionPayload::Fulu(payload.into()), - ExecutionPayload::Gloas(payload) => JsonExecutionPayload::Gloas(payload.into()), + ExecutionPayload::Bellatrix(payload) => { + Ok(JsonExecutionPayload::Bellatrix(payload.into())) + } + ExecutionPayload::Capella(payload) => { + Ok(JsonExecutionPayload::Capella(payload.try_into()?)) + } + ExecutionPayload::Deneb(payload) => { + Ok(JsonExecutionPayload::Deneb(payload.try_into()?)) + } + ExecutionPayload::Electra(payload) => { + Ok(JsonExecutionPayload::Electra(payload.try_into()?)) + } + ExecutionPayload::Fulu(payload) => Ok(JsonExecutionPayload::Fulu(payload.try_into()?)), + ExecutionPayload::Gloas(payload) => { + Ok(JsonExecutionPayload::Gloas(payload.try_into()?)) + } } } } @@ -281,9 +303,11 @@ impl From> for ExecutionPayloadBell } } } -impl From> for ExecutionPayloadCapella { - fn from(payload: JsonExecutionPayloadCapella) -> Self { - ExecutionPayloadCapella { +impl TryFrom> for ExecutionPayloadCapella { + type Error = ssz_types::Error; + + fn try_from(payload: JsonExecutionPayloadCapella) -> Result { + Ok(ExecutionPayloadCapella { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -298,14 +322,16 @@ impl From> for ExecutionPayloadCapell base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_from_json(payload.withdrawals), - } + withdrawals: withdrawals_from_json(payload.withdrawals)?, + }) } } -impl From> for ExecutionPayloadDeneb { - fn from(payload: JsonExecutionPayloadDeneb) -> Self { - ExecutionPayloadDeneb { +impl TryFrom> for ExecutionPayloadDeneb { + type Error = ssz_types::Error; + + fn try_from(payload: JsonExecutionPayloadDeneb) -> Result { + Ok(ExecutionPayloadDeneb { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -320,16 +346,18 @@ impl From> for ExecutionPayloadDeneb base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_from_json(payload.withdrawals), + withdrawals: withdrawals_from_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for ExecutionPayloadElectra { - fn from(payload: JsonExecutionPayloadElectra) -> Self { - ExecutionPayloadElectra { +impl TryFrom> for ExecutionPayloadElectra { + type Error = ssz_types::Error; + + fn try_from(payload: JsonExecutionPayloadElectra) -> Result { + Ok(ExecutionPayloadElectra { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -344,16 +372,18 @@ impl From> for ExecutionPayloadElectr base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_from_json(payload.withdrawals), + withdrawals: withdrawals_from_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for ExecutionPayloadFulu { - fn from(payload: JsonExecutionPayloadFulu) -> Self { - ExecutionPayloadFulu { +impl TryFrom> for ExecutionPayloadFulu { + type Error = ssz_types::Error; + + fn try_from(payload: JsonExecutionPayloadFulu) -> Result { + Ok(ExecutionPayloadFulu { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -368,16 +398,18 @@ impl From> for ExecutionPayloadFulu { base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_from_json(payload.withdrawals), + withdrawals: withdrawals_from_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for ExecutionPayloadGloas { - fn from(payload: JsonExecutionPayloadGloas) -> Self { - ExecutionPayloadGloas { +impl TryFrom> for ExecutionPayloadGloas { + type Error = ssz_types::Error; + + fn try_from(payload: JsonExecutionPayloadGloas) -> Result { + Ok(ExecutionPayloadGloas { parent_hash: payload.parent_hash, fee_recipient: payload.fee_recipient, state_root: payload.state_root, @@ -392,22 +424,34 @@ impl From> for ExecutionPayloadGloas base_fee_per_gas: payload.base_fee_per_gas, block_hash: payload.block_hash, transactions: payload.transactions, - withdrawals: withdrawals_from_json(payload.withdrawals), + withdrawals: withdrawals_from_json(payload.withdrawals)?, blob_gas_used: payload.blob_gas_used, excess_blob_gas: payload.excess_blob_gas, - } + }) } } -impl From> for ExecutionPayload { - fn from(json_execution_payload: JsonExecutionPayload) -> Self { +impl TryFrom> for ExecutionPayload { + type Error = ssz_types::Error; + + fn try_from(json_execution_payload: JsonExecutionPayload) -> Result { match json_execution_payload { - JsonExecutionPayload::Bellatrix(payload) => ExecutionPayload::Bellatrix(payload.into()), - JsonExecutionPayload::Capella(payload) => ExecutionPayload::Capella(payload.into()), - JsonExecutionPayload::Deneb(payload) => ExecutionPayload::Deneb(payload.into()), - JsonExecutionPayload::Electra(payload) => ExecutionPayload::Electra(payload.into()), - JsonExecutionPayload::Fulu(payload) => ExecutionPayload::Fulu(payload.into()), - JsonExecutionPayload::Gloas(payload) => ExecutionPayload::Gloas(payload.into()), + JsonExecutionPayload::Bellatrix(payload) => { + Ok(ExecutionPayload::Bellatrix(payload.into())) + } + JsonExecutionPayload::Capella(payload) => { + Ok(ExecutionPayload::Capella(payload.try_into()?)) + } + JsonExecutionPayload::Deneb(payload) => { + Ok(ExecutionPayload::Deneb(payload.try_into()?)) + } + JsonExecutionPayload::Electra(payload) => { + Ok(ExecutionPayload::Electra(payload.try_into()?)) + } + JsonExecutionPayload::Fulu(payload) => Ok(ExecutionPayload::Fulu(payload.try_into()?)), + JsonExecutionPayload::Gloas(payload) => { + Ok(ExecutionPayload::Gloas(payload.try_into()?)) + } } } } @@ -541,13 +585,17 @@ impl TryFrom> for GetPayloadResponse { } JsonGetPayloadResponse::Capella(response) => { Ok(GetPayloadResponse::Capella(GetPayloadResponseCapella { - execution_payload: response.execution_payload.into(), + execution_payload: response.execution_payload.try_into().map_err(|e| { + format!("Failed to convert json to execution payload: {:?}", e) + })?, block_value: response.block_value, })) } JsonGetPayloadResponse::Deneb(response) => { Ok(GetPayloadResponse::Deneb(GetPayloadResponseDeneb { - execution_payload: response.execution_payload.into(), + execution_payload: response.execution_payload.try_into().map_err(|e| { + format!("Failed to convert json to execution payload: {:?}", e) + })?, block_value: response.block_value, blobs_bundle: response.blobs_bundle.into(), should_override_builder: response.should_override_builder, @@ -555,34 +603,40 @@ impl TryFrom> for GetPayloadResponse { } JsonGetPayloadResponse::Electra(response) => { Ok(GetPayloadResponse::Electra(GetPayloadResponseElectra { - execution_payload: response.execution_payload.into(), + execution_payload: response.execution_payload.try_into().map_err(|e| { + format!("Failed to convert json to execution payload: {:?}", e) + })?, block_value: response.block_value, blobs_bundle: response.blobs_bundle.into(), should_override_builder: response.should_override_builder, requests: response.execution_requests.try_into().map_err(|e| { - format!("Failed to convert json to execution requests : {:?}", e) + format!("Failed to convert json to execution requests: {:?}", e) })?, })) } JsonGetPayloadResponse::Fulu(response) => { Ok(GetPayloadResponse::Fulu(GetPayloadResponseFulu { - execution_payload: response.execution_payload.into(), + execution_payload: response.execution_payload.try_into().map_err(|e| { + format!("Failed to convert json to execution payload: {:?}", e) + })?, block_value: response.block_value, blobs_bundle: response.blobs_bundle.into(), should_override_builder: response.should_override_builder, requests: response.execution_requests.try_into().map_err(|e| { - format!("Failed to convert json to execution requests {:?}", e) + format!("Failed to convert json to execution requests: {:?}", e) })?, })) } JsonGetPayloadResponse::Gloas(response) => { Ok(GetPayloadResponse::Gloas(GetPayloadResponseGloas { - execution_payload: response.execution_payload.into(), + execution_payload: response.execution_payload.try_into().map_err(|e| { + format!("Failed to convert json to execution payload: {:?}", e) + })?, block_value: response.block_value, blobs_bundle: response.blobs_bundle.into(), should_override_builder: response.should_override_builder, requests: response.execution_requests.try_into().map_err(|e| { - format!("Failed to convert json to execution requests {:?}", e) + format!("Failed to convert json to execution requests: {:?}", e) })?, })) } @@ -626,21 +680,22 @@ impl From for Withdrawal { } // Helper functions to convert between `VariableList` and `VariableList`. -// The `expect` is required to enable `from` impls between `ExecutionPayload` and `JsonExecutionPayload`. -fn withdrawals_to_json(list: VariableList) -> VariableList +fn withdrawals_to_json( + list: VariableList, +) -> Result, ssz_types::Error> where N: Unsigned, { VariableList::try_from_iter(list.into_iter().map(Into::into)) - .expect("conversion to JSON should preserve withdrawals length") } -fn withdrawals_from_json(list: VariableList) -> VariableList +fn withdrawals_from_json( + list: VariableList, +) -> Result, ssz_types::Error> where N: Unsigned, { VariableList::try_from_iter(list.into_iter().map(Into::into)) - .expect("conversion from JSON should preserve withdrawals length") } #[derive(Debug, PartialEq, Clone, RlpEncodable)] @@ -946,21 +1001,25 @@ pub struct JsonExecutionPayloadBodyV1 { pub withdrawals: Option>, } -impl From> for ExecutionPayloadBodyV1 { - fn from(value: JsonExecutionPayloadBodyV1) -> Self { - Self { +impl TryFrom> for ExecutionPayloadBodyV1 { + type Error = ssz_types::Error; + + fn try_from(value: JsonExecutionPayloadBodyV1) -> Result { + Ok(Self { transactions: value.transactions, - withdrawals: value.withdrawals.map(withdrawals_from_json), - } + withdrawals: value.withdrawals.map(withdrawals_from_json).transpose()?, + }) } } -impl From> for JsonExecutionPayloadBodyV1 { - fn from(value: ExecutionPayloadBodyV1) -> Self { - Self { +impl TryFrom> for JsonExecutionPayloadBodyV1 { + type Error = ssz_types::Error; + + fn try_from(value: ExecutionPayloadBodyV1) -> Result { + Ok(Self { transactions: value.transactions, - withdrawals: value.withdrawals.map(withdrawals_to_json), - } + withdrawals: value.withdrawals.map(withdrawals_to_json).transpose()?, + }) } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 05d8b9f922c..3d751c7f326 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -171,11 +171,18 @@ pub enum Error { InvalidPayloadBody(String), InvalidPayloadConversion, InvalidBlobConversion(String), + SszTypesError(ssz_types::Error), BeaconStateError(BeaconStateError), PayloadTypeMismatch, VerifyingVersionedHashes(versioned_hashes::Error), } +impl From for Error { + fn from(e: ssz_types::Error) -> Self { + Error::SszTypesError(e) + } +} + impl From for Error { fn from(e: BeaconStateError) -> Self { Error::BeaconStateError(e) diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index 7a451beddb3..2168ed8961e 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -252,7 +252,7 @@ pub async fn handle_rpc( Some( ctx.execution_block_generator .write() - .new_payload(request.into()), + .new_payload(request.try_into().unwrap()), ) } else { None @@ -361,98 +361,107 @@ pub async fn handle_rpc( } match method { - ENGINE_GET_PAYLOAD_V1 => { - Ok(serde_json::to_value(JsonExecutionPayload::from(response)).unwrap()) + ENGINE_GET_PAYLOAD_V1 => Ok(serde_json::to_value( + JsonExecutionPayload::try_from(response).unwrap(), + ) + .unwrap()), + ENGINE_GET_PAYLOAD_V2 => { + Ok(match JsonExecutionPayload::try_from(response).unwrap() { + JsonExecutionPayload::Bellatrix(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseBellatrix { + execution_payload, + block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), + }) + .unwrap() + } + JsonExecutionPayload::Capella(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseCapella { + execution_payload, + block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), + }) + .unwrap() + } + _ => unreachable!(), + }) } - ENGINE_GET_PAYLOAD_V2 => Ok(match JsonExecutionPayload::from(response) { - JsonExecutionPayload::Bellatrix(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseBellatrix { - execution_payload, - block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), - }) - .unwrap() - } - JsonExecutionPayload::Capella(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseCapella { - execution_payload, - block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), - }) - .unwrap() - } - _ => unreachable!(), - }), // From v3 onwards, we use the getPayload version only for the corresponding // ExecutionPayload version. So we return an error if the ExecutionPayload version // we get does not correspond to the getPayload version. - ENGINE_GET_PAYLOAD_V3 => Ok(match JsonExecutionPayload::from(response) { - JsonExecutionPayload::Deneb(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseDeneb { - execution_payload, - block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), - blobs_bundle: maybe_blobs - .ok_or(( - "No blobs returned despite V3 Payload".to_string(), - GENERIC_ERROR_CODE, - ))? - .into(), - should_override_builder: false, - }) - .unwrap() - } - _ => unreachable!(), - }), - ENGINE_GET_PAYLOAD_V4 => Ok(match JsonExecutionPayload::from(response) { - JsonExecutionPayload::Electra(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseElectra { - execution_payload, - block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), - blobs_bundle: maybe_blobs - .ok_or(( - "No blobs returned despite V4 Payload".to_string(), - GENERIC_ERROR_CODE, - ))? - .into(), - should_override_builder: false, - // TODO(electra): add EL requests in mock el - execution_requests: Default::default(), - }) - .unwrap() - } - _ => unreachable!(), - }), - ENGINE_GET_PAYLOAD_V5 => Ok(match JsonExecutionPayload::from(response) { - JsonExecutionPayload::Fulu(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseFulu { - execution_payload, - block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), - blobs_bundle: maybe_blobs - .ok_or(( - "No blobs returned despite V5 Payload".to_string(), - GENERIC_ERROR_CODE, - ))? - .into(), - should_override_builder: false, - execution_requests: Default::default(), - }) - .unwrap() - } - JsonExecutionPayload::Gloas(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseGloas { - execution_payload, - block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), - blobs_bundle: maybe_blobs - .ok_or(( - "No blobs returned despite V5 Payload".to_string(), - GENERIC_ERROR_CODE, - ))? - .into(), - should_override_builder: false, - execution_requests: Default::default(), - }) - .unwrap() - } - _ => unreachable!(), - }), + ENGINE_GET_PAYLOAD_V3 => { + Ok(match JsonExecutionPayload::try_from(response).unwrap() { + JsonExecutionPayload::Deneb(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseDeneb { + execution_payload, + block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), + blobs_bundle: maybe_blobs + .ok_or(( + "No blobs returned despite V3 Payload".to_string(), + GENERIC_ERROR_CODE, + ))? + .into(), + should_override_builder: false, + }) + .unwrap() + } + _ => unreachable!(), + }) + } + ENGINE_GET_PAYLOAD_V4 => { + Ok(match JsonExecutionPayload::try_from(response).unwrap() { + JsonExecutionPayload::Electra(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseElectra { + execution_payload, + block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), + blobs_bundle: maybe_blobs + .ok_or(( + "No blobs returned despite V4 Payload".to_string(), + GENERIC_ERROR_CODE, + ))? + .into(), + should_override_builder: false, + // TODO(electra): add EL requests in mock el + execution_requests: Default::default(), + }) + .unwrap() + } + _ => unreachable!(), + }) + } + ENGINE_GET_PAYLOAD_V5 => { + Ok(match JsonExecutionPayload::try_from(response).unwrap() { + JsonExecutionPayload::Fulu(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseFulu { + execution_payload, + block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), + blobs_bundle: maybe_blobs + .ok_or(( + "No blobs returned despite V5 Payload".to_string(), + GENERIC_ERROR_CODE, + ))? + .into(), + should_override_builder: false, + execution_requests: Default::default(), + }) + .unwrap() + } + JsonExecutionPayload::Gloas(execution_payload) => { + serde_json::to_value(JsonGetPayloadResponseGloas { + execution_payload, + block_value: Uint256::from(DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI), + blobs_bundle: maybe_blobs + .ok_or(( + "No blobs returned despite V5 Payload".to_string(), + GENERIC_ERROR_CODE, + ))? + .into(), + should_override_builder: false, + execution_requests: Default::default(), + }) + .unwrap() + } + _ => unreachable!(), + }) + } _ => unreachable!(), } } @@ -644,7 +653,8 @@ pub async fn handle_rpc( transactions: payload.transactions().clone(), withdrawals: payload.withdrawals().ok().cloned(), }; - let json_payload_body = JsonExecutionPayloadBodyV1::from(payload_body); + let json_payload_body: JsonExecutionPayloadBodyV1 = + payload_body.try_into().unwrap(); response.push(Some(json_payload_body)); } None => response.push(None), From 12c260a630e31d5dc67a140e9981118b48ce2450 Mon Sep 17 00:00:00 2001 From: Mac L Date: Sun, 21 Sep 2025 01:55:35 +1000 Subject: [PATCH 7/7] Remove expects on IndexedAttestation --- .../src/attestation_verification.rs | 39 +++++++++++++++++-- .../gossip_methods.rs | 14 +++++++ consensus/types/src/attestation.rs | 21 +++++----- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 470664d4429..d7f27108db7 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -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, @@ -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), + /// 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 for Error { @@ -275,6 +283,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: ssz_types::Error) -> Self { + Self::SszTypesError(e) + } +} + /// Used to avoid double-checking signatures. #[derive(Copy, Clone)] enum CheckAttestationSignature { @@ -442,7 +456,18 @@ fn process_slash_info( .spec .fork_name_at_slot::(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), @@ -932,7 +957,15 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { .spec .fork_name_at_slot::(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, diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 1f1a3427e78..dc1c4c89191 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -2734,6 +2734,20 @@ impl NetworkBeaconProcessor { } } } + AttnError::SszTypesError(_) => { + error!( + %peer_id, + block = ?beacon_block_root, + ?attestation_type, + "Rejecting attestation due to a critical SSZ types error" + ); + self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject); + self.gossip_penalize_peer( + peer_id, + PeerAction::MidToleranceError, + "attn_ssz_types_error", + ); + } } debug!( diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 6d661ffb66b..52646867925 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -622,23 +622,22 @@ pub struct SingleAttestation { } impl SingleAttestation { - pub fn to_indexed(&self, fork_name: ForkName) -> IndexedAttestation { + pub fn to_indexed( + &self, + fork_name: ForkName, + ) -> Result, ssz_types::Error> { if fork_name.electra_enabled() { - IndexedAttestation::Electra(IndexedAttestationElectra { - attesting_indices: vec![self.attester_index] - .try_into() - .expect("Single index should not exceed MaxValidatorsPerSlot"), + Ok(IndexedAttestation::Electra(IndexedAttestationElectra { + attesting_indices: vec![self.attester_index].try_into()?, data: self.data.clone(), signature: self.signature.clone(), - }) + })) } else { - IndexedAttestation::Base(IndexedAttestationBase { - attesting_indices: vec![self.attester_index] - .try_into() - .expect("Single index should not exceed MaxValidatorsPerCommittee"), + Ok(IndexedAttestation::Base(IndexedAttestationBase { + attesting_indices: vec![self.attester_index].try_into()?, data: self.data.clone(), signature: self.signature.clone(), - }) + })) } } }