diff --git a/Cargo.lock b/Cargo.lock index f717e6e70..f5c98d385 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5038,6 +5038,7 @@ dependencies = [ "slot_clock", "ssv_types", "subnet_service", + "thiserror 2.0.14", "tokio", "tracing", ] @@ -5066,6 +5067,7 @@ dependencies = [ "thiserror 2.0.14", "tokio", "tracing", + "typenum", "types", ] @@ -6142,6 +6144,7 @@ dependencies = [ "ssv_types", "tracing", "tracing-subscriber", + "typenum", "types", ] @@ -7441,13 +7444,18 @@ dependencies = [ "openssl", "operator_key", "rusqlite", + "serde", + "serde_json", "sha2 0.10.9", "slashing_protection", + "ssz_types", "thiserror 2.0.14", "tracing", "tree_hash", "tree_hash_derive", + "typenum", "types", + "zerocopy", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 8c426451c..c2e3f8efc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -133,6 +133,7 @@ serde = { version = "1.0.208", features = ["derive"] } serde_json = "1.0.140" serde_yaml = "0.9" sha2 = "0.10.8" +ssz_types = "0.11.0" strum = { version = "0.27.0", features = ["derive"] } thiserror = "2.0.11" tokio = { version = "1.39.2", features = [ @@ -149,6 +150,7 @@ tracing-appender = "0.2" tracing-subscriber = { version = "0.3.18", features = ["fmt", "env-filter"] } tree_hash = "0.10" tree_hash_derive = "0.10" +typenum = "1.18" vsss-rs = "5.1.0" zeroize = "1.8.1" diff --git a/anchor/common/qbft/Cargo.toml b/anchor/common/qbft/Cargo.toml index 02dbe021b..70c175f38 100644 --- a/anchor/common/qbft/Cargo.toml +++ b/anchor/common/qbft/Cargo.toml @@ -17,6 +17,7 @@ indexmap = { workspace = true } sha2 = { workspace = true } ssv_types = { workspace = true } tracing = { workspace = true } +typenum = { workspace = true } types = { workspace = true } [dev-dependencies] diff --git a/anchor/common/qbft/src/error.rs b/anchor/common/qbft/src/error.rs index 1cd5825c2..59a2c56fa 100644 --- a/anchor/common/qbft/src/error.rs +++ b/anchor/common/qbft/src/error.rs @@ -1,3 +1,5 @@ +use ssv_types::message::SignedSSVMessageError; + /// Error associated with Config building. #[derive(Debug, Clone)] pub enum ConfigBuilderError { @@ -86,6 +88,8 @@ pub enum QbftError { RoundChangeJustificationNotInCommittee, RoundChangeJustificationNoPrepareQuorum, RoundChangeJustificationMultiSigner, + RoundChangeJustificationTooBig { provided: usize, max: usize }, + RoundChangeJustificationListTooBig { provided: usize, max: usize }, PrepareJustificationWrongRound, PrepareJustificationMultiSigner, PrepareJustificationWrongHeight, @@ -93,8 +97,15 @@ pub enum QbftError { PrepareJustificationDecodeFailed, PrepareJustificationNotPrepare, PrepareJustificationRootMismatch, + PrepareJustificationTooBig { provided: usize, max: usize }, + PrepareJustificationListTooBig { provided: usize, max: usize }, // Misc - FailedToAggregate, + MissingLastPreparedValue, + FailedToAggregate(SignedSSVMessageError), + SignedSSVMessageError(SignedSSVMessageError), InvalidState, + MissingData, + CommitQuorumMismatch, + MissingCommit, } diff --git a/anchor/common/qbft/src/lib.rs b/anchor/common/qbft/src/lib.rs index 0e6e708e8..1c67f879e 100644 --- a/anchor/common/qbft/src/lib.rs +++ b/anchor/common/qbft/src/lib.rs @@ -11,14 +11,19 @@ pub use qbft_types::{ UnsignedWrappedQbftMessage, WrappedQbftMessage, }; use ssv_types::{ - OperatorId, Round, - consensus::{QbftData, QbftDataValidator, QbftMessage, QbftMessageType, UnsignedSSVMessage}, + OperatorId, Round, VariableList, + consensus::{ + PrepareJustificationLength, QbftData, QbftDataValidator, QbftMessage, QbftMessageType, + RoundChangeJustificationLength, UnsignedSSVMessage, + }, message::{MsgType, SSVMessage, SignedSSVMessage}, msgid::MessageId, + try_to_variable_list, }; use ssz::{Decode, Encode}; use tracing::{debug, error, warn}; -use types::Hash256; +use typenum::U13; +use types::{Hash256, Unsigned}; use crate::{error::QbftError, msg_container::MessageContainer}; @@ -277,6 +282,37 @@ where Ok(()) } + // Try to decode a SignedSSVMessage from a VariableList + fn try_decode_signed_ssv_messages( + &self, + messages: &VariableList, U13>, + decode_err: QbftError, + ) -> Result, QbftError> { + messages + .iter() + .map(|bytes| SignedSSVMessage::from_ssz_bytes(bytes)) + .collect::, _>>() + .map_err(|_| decode_err) + } + + // Try to encode a SignedSSVMessage into a VariableList + fn try_encode_signed_ssv_messages( + &self, + messages: Vec, + inner_error_fn: impl Fn(usize, usize) -> QbftError, + outer_error_fn: impl Fn(usize, usize) -> QbftError, + ) -> Result, U13>, QbftError> { + let message_vec: Result, _> = messages + .iter() + .map(|msg| { + let msg_without_data = msg.without_full_data(); + try_to_variable_list(msg_without_data.as_ssz_bytes().to_vec(), &inner_error_fn) + }) + .collect(); + + try_to_variable_list(message_vec?, &outer_error_fn) + } + // Perform base QBFT relevant message verification. This verification is applicable to all QBFT // message types @@ -474,7 +510,9 @@ where return; } }; - self.send_proposal(hash, data); + if let Err(e) = self.send_proposal(hash, data) { + error!(?e, "Failed to send proposal to start round"); + } } } @@ -573,7 +611,7 @@ where debug!(state = ?self.state, "State updated to PREPARE"); // Create and send prepare message - self.send_prepare(wrapped_msg.qbft_message.root); + self.send_prepare(wrapped_msg.qbft_message.root)?; Ok(()) } @@ -595,15 +633,22 @@ where let mut max_prepared_round = 0; let mut max_prepared_msg = None; + // Deserialize round change justifications for validation + let signed_rc_justifications = self + .try_decode_signed_ssv_messages::( + &msg.qbft_message.round_change_justification, + QbftError::RoundChangeJustificationDecodeFailed, + )?; + // Make sure we have a quorum of round change messages - if !self.has_quorum(&msg.qbft_message.round_change_justification) { + if !self.has_quorum(&signed_rc_justifications) { warn!("Did not receive a quorum of round change messages"); return Err(QbftError::ProposalRoundChangeJustificationNoQuorum); } // There was a quorum of round change justifications. We need to go though and verify each // one. Each will be a SignedSSVMessage - for signed_round_change in &msg.qbft_message.round_change_justification { + for signed_round_change in &signed_rc_justifications { // Check for multi-signers - round change messages should only have 1 signer if signed_round_change.operator_ids().len() != 1 || signed_round_change.signatures().len() != 1 @@ -663,16 +708,29 @@ where return Err(QbftError::RoundChangeJustificationInvalidDataRound); } - if !self.has_quorum(&round_change.round_change_justification) { + // Verify that if round change has full data, it matches the root + if msg.qbft_message.root != round_change.root { + warn!("Proposal root doesn't match round change prepared root"); + return Err(QbftError::RoundChangeJustificationInvalidPrepareRoot); + } + + // Deserialize prepare justifications for validation + let signed_inner_rc_justifications = self + .try_decode_signed_ssv_messages::( + &round_change.round_change_justification, + QbftError::RoundChangeJustificationDecodeFailed, + )?; + + if !self.has_quorum(&signed_inner_rc_justifications) { warn!( - num_justifications = round_change.round_change_justification.len(), + num_justifications = signed_inner_rc_justifications.len(), "Not enough prepare messages for quorum" ); return Err(QbftError::RoundChangeJustificationNoPrepareQuorum); } // go through all of the round changes prepare justifications - for signed_prepare in &round_change.round_change_justification { + for signed_prepare in &signed_inner_rc_justifications { self.is_valid_prepare_justification_for_round_and_root( signed_prepare, round_change.data_round.into(), @@ -685,10 +743,17 @@ where // If there was a value that was also previously prepared, we must also verify all of the // prepare justifications if let Some(max_prepared_msg) = max_prepared_msg { + // Deserialize prepare justifications for validation + let signed_prepare_justifications = self + .try_decode_signed_ssv_messages::( + &msg.qbft_message.prepare_justification, + QbftError::PrepareJustificationNoQuorum, + )?; + // Make sure we have a quorum of prepare messages - if !self.has_quorum(&msg.qbft_message.prepare_justification) { + if !self.has_quorum(&signed_prepare_justifications) { warn!( - num_justifications = msg.qbft_message.prepare_justification.len(), + num_justifications = signed_prepare_justifications.len(), "Not enough prepare messages for quorum" ); return Err(QbftError::PrepareJustificationNoQuorum); @@ -704,7 +769,7 @@ where } // Validate each prepare message matches highest prepared round/value - for signed_prepare in &msg.qbft_message.prepare_justification { + for signed_prepare in &signed_prepare_justifications { self.is_valid_prepare_justification_for_round_and_root( signed_prepare, max_prepared_msg.data_round.into(), @@ -847,7 +912,7 @@ where self.last_prepared_round = Some(self.current_round); // Send a commit message for the prepare quorum data - self.send_commit(hash); + self.send_commit(hash)?; } Ok(()) } @@ -928,16 +993,18 @@ where // Aggregate all of the commit messages let commit_quorum = self.commit_container.get_quorum_of_messages(round); - let aggregated_commit = self.aggregate_commit_messages(commit_quorum); - if aggregated_commit.is_some() { - debug!(state = ?self.state, "Reached a COMMIT consensus. Success!"); - self.aggregated_commit = aggregated_commit; - self.state = InstanceState::Complete; - self.completed = Some(Completed::Success(hash)); - } else { - error!("Failed to aggregate commit quorum"); - return Err(QbftError::FailedToAggregate); - } + let aggregated_commit = match self.aggregate_commit_messages(commit_quorum) { + Ok(commit) => commit, + Err(err) => { + error!(?err, "Failed to aggregate commit quorum"); + return Err(err); + } + }; + + debug!(state = ?self.state, "Reached a COMMIT consensus. Success!"); + self.aggregated_commit = Some(aggregated_commit); + self.state = InstanceState::Complete; + self.completed = Some(Completed::Success(hash)); } Ok(()) } @@ -946,7 +1013,7 @@ where fn aggregate_commit_messages( &self, commit_quorum: Vec, - ) -> Option { + ) -> Result { // We know this exists, but in favor of avoiding expect match the first element to Some. // This will be the commit message that we aggregate on top of if let Some(first_commit) = commit_quorum.first() { @@ -957,22 +1024,37 @@ where commit_quorum[1..] .iter() .all(|commit_msg| aggregated_ssv == commit_msg.signed_message.ssv_message()) - .then_some(())?; + .then_some(()) + .ok_or(QbftError::CommitQuorumMismatch)?; // Aggregate all of the commits together let signed_commits = commit_quorum[1..] .iter() .map(|msg| msg.signed_message.clone()); - aggregated_commit.aggregate(signed_commits); + if let Err(e) = aggregated_commit.aggregate(signed_commits) { + error!(?e, "Failed to aggregate commits together"); + return Err(QbftError::FailedToAggregate(e)); + } // Set full data let hash = first_commit.qbft_message.root; - aggregated_commit.set_full_data(self.data.get(&hash)?.as_ssz_bytes()); + match self.data.get(&hash) { + Some(data) => { + if let Err(e) = aggregated_commit.set_full_data(data.as_ssz_bytes()) { + error!(?e, "Failed to set full data"); + return Err(QbftError::SignedSSVMessageError(e)); + } + } + None => { + error!("Missing data for hash: {}", hash); + return Err(QbftError::MissingData); + } + } - return Some(aggregated_commit); + return Ok(aggregated_commit); } - None + Err(QbftError::MissingCommit) } /// We have received a round change message. @@ -991,10 +1073,17 @@ where let qbft_msg = &wrapped_msg.qbft_message; // If this is a "prepared" round change, we have to check the justifications. if qbft_msg.data_round > 0 { - if !self.has_quorum(&qbft_msg.round_change_justification) { + // Deserialize prepare justifications for validation + let signed_rc_justifications = self + .try_decode_signed_ssv_messages::( + &qbft_msg.round_change_justification, + QbftError::RoundChangeJustificationDecodeFailed, + )?; + + if !self.has_quorum(&signed_rc_justifications) { debug!( from = *operator_id, - justifications = qbft_msg.round_change_justification.len(), + justifications = signed_rc_justifications.len(), quorum = self.config.quorum_size(), "prepared ROUNDCHANGE has no quorum" ); @@ -1011,7 +1100,7 @@ where return Err(QbftError::InvalidDataRound); } - for justification in qbft_msg.round_change_justification.iter() { + for justification in &signed_rc_justifications { self.is_valid_prepare_justification_for_round_and_root( justification, qbft_msg.data_round.into(), @@ -1067,7 +1156,7 @@ where self.state = InstanceState::SentRoundChange; self.current_round = round; self.proposal_accepted_for_current_round = false; - self.send_round_change(Hash256::default()); + self.send_round_change(Hash256::default())?; } } Ok(()) @@ -1131,7 +1220,10 @@ where // Set the state so SendRoundChange so we include Round + 1 in message self.state = InstanceState::SentRoundChange; - self.send_round_change(Hash256::default()); + if let Err(e) = self.send_round_change(Hash256::default()) { + error!(?e, "Failed to end round due to error sending round change"); + return; + }; self.start_round(); } @@ -1189,17 +1281,36 @@ where data_hash: D::Hash, mut round_change_justification: Vec, mut prepare_justification: Vec, - ) -> UnsignedWrappedQbftMessage { + ) -> Result { let data = self.get_message_data(&msg_type, data_hash); - // Clear full_data from justifications as these do not store full data. + // Clear the full data for round_change_justification in &mut round_change_justification { - round_change_justification.set_full_data(vec![]); + round_change_justification + .set_full_data(vec![]) + .map_err(|_| QbftError::InvalidFullData)?; } for prepare_justification in &mut prepare_justification { - prepare_justification.set_full_data(vec![]); + prepare_justification + .set_full_data(vec![]) + .map_err(|_| QbftError::InvalidFullData)?; } + // Clear full_data from justifications as these do not store full data. + let round_change_justification = self + .try_encode_signed_ssv_messages::( + round_change_justification, + |provided, max| QbftError::RoundChangeJustificationTooBig { provided, max }, + |provided, max| QbftError::RoundChangeJustificationListTooBig { provided, max }, + )?; + + let prepare_justification = self + .try_encode_signed_ssv_messages::( + prepare_justification, + |provided, max| QbftError::PrepareJustificationTooBig { provided, max }, + |provided, max| QbftError::PrepareJustificationListTooBig { provided, max }, + )?; + // Create the QBFT message let qbft_message = QbftMessage { qbft_message_type: msg_type, @@ -1220,13 +1331,13 @@ where .expect("SSVMessage should be valid."); // TODO revisit this // Wrap in unsigned SSV message - UnsignedWrappedQbftMessage { + Ok(UnsignedWrappedQbftMessage { unsigned_message: UnsignedSSVMessage { ssv_message, full_data: data.full_data, }, qbft_message, - } + }) } // Get all of the round change justification messages @@ -1286,15 +1397,17 @@ where } // Get all of the prepare justifications for proposals - fn get_prepare_justifications(&self) -> (Vec, Option) { + fn get_prepare_justifications( + &self, + ) -> Result<(Vec, Option), QbftError> { // No justifications needed for round 1 if self.current_round == Round::default() { - return (vec![], None); + return Ok((vec![], None)); } // Only needed when we're the proposer if !matches!(self.state, InstanceState::AwaitingProposal) { - return (vec![], None); + return Ok((vec![], None)); } // Check if we have our own prepared value that should be proposed @@ -1303,12 +1416,12 @@ where let potential_prepare_just = self.get_round_change_prepare_justifications(); if !potential_prepare_just.is_empty() { if let Some(last_prepared) = self.last_prepared_value { - return (potential_prepare_just, Some(last_prepared)); + return Ok((potential_prepare_just, Some(last_prepared))); } else { // Invariant violated: potential_prepare_just is not empty but no // last_prepared_value Handle gracefully: return no justification error!("prepare justifications exists but no last prepared value was found"); - return (vec![], None); + return Err(QbftError::MissingLastPreparedValue); } } @@ -1318,7 +1431,7 @@ where .get_messages_for_round(self.current_round); if !self.has_quorum(round_changes) { - return (vec![], None); + return Ok((vec![], None)); } // Find the highest prepared round among all round changes @@ -1340,20 +1453,24 @@ where if let Some((_, prepared_value, highest_rc)) = highest_prepared { // Extract the prepare messages from the round change message's justifications // These are stored in the round_change_justification field of the RoundChange - let prepares = &highest_rc.qbft_message.round_change_justification; + let prepare_msgs = self + .try_decode_signed_ssv_messages::( + &highest_rc.qbft_message.round_change_justification, + QbftError::RoundChangeJustificationDecodeFailed, + )?; // Verify we have quorum of prepares - if self.has_quorum(prepares) { - return (prepares.clone(), Some(prepared_value)); + if self.has_quorum(&prepare_msgs) { + return Ok((prepare_msgs, Some(prepared_value))); } } // No prepared value found, proposer can choose new value - (vec![], None) + Ok((vec![], None)) } // Send a new qbft proposal message - fn send_proposal(&mut self, hash: D::Hash, data: Arc) { + fn send_proposal(&mut self, hash: D::Hash, data: Arc) -> Result<(), QbftError> { // Store the data we're proposing self.data.insert(hash, data.clone()); @@ -1361,7 +1478,7 @@ where // round_change_justification: list of round change messages let round_change_justifications = self.get_round_change_justifications(); // prepare_justification: list of prepare messages - let (prepare_justifications, value_to_propose) = self.get_prepare_justifications(); + let (prepare_justifications, value_to_propose) = self.get_prepare_justifications()?; // Determine the value that should be proposed based off of justification. If we have a // prepare justification, we want to propose that value. Else, just the justified value @@ -1373,37 +1490,37 @@ where value_to_propose, round_change_justifications, prepare_justifications, - ); - + )?; self.message_sender.send(unsigned_msg); + Ok(()) } // Send a new qbft prepare message - fn send_prepare(&mut self, data_hash: D::Hash) { + fn send_prepare(&mut self, data_hash: D::Hash) -> Result<(), QbftError> { // Only send prepare if we've seen this data if !self.data.contains_key(&data_hash) { warn!("Attempted to prepare unknown data"); - return; + return Err(QbftError::MissingData); } // Construct unsigned prepare let unsigned_msg = - self.new_unsigned_message(QbftMessageType::Prepare, data_hash, vec![], vec![]); - + self.new_unsigned_message(QbftMessageType::Prepare, data_hash, vec![], vec![])?; self.message_sender.send(unsigned_msg); + Ok(()) } // Send a new qbft commit message - fn send_commit(&mut self, data_hash: D::Hash) { + fn send_commit(&mut self, data_hash: D::Hash) -> Result<(), QbftError> { // Construct unsigned commit let unsigned_msg = - self.new_unsigned_message(QbftMessageType::Commit, data_hash, vec![], vec![]); - + self.new_unsigned_message(QbftMessageType::Commit, data_hash, vec![], vec![])?; self.message_sender.send(unsigned_msg); + Ok(()) } // Send a new qbft round change message - fn send_round_change(&mut self, data_hash: D::Hash) { + fn send_round_change(&mut self, data_hash: D::Hash) -> Result<(), QbftError> { // For Round Change messages // round_change_justification: list of prepare messages let round_change_justifications = self.get_round_change_prepare_justifications(); @@ -1415,12 +1532,11 @@ where data_hash, round_change_justifications, vec![], - ); - + )?; // forget that we accepted a proposal self.proposal_accepted_for_current_round = false; - self.message_sender.send(unsigned_msg); + Ok(()) } /// Extract the data that the instance has come to consensus on diff --git a/anchor/common/qbft/src/qbft_types.rs b/anchor/common/qbft/src/qbft_types.rs index 0d8f10862..68a926ad6 100644 --- a/anchor/common/qbft/src/qbft_types.rs +++ b/anchor/common/qbft/src/qbft_types.rs @@ -60,7 +60,7 @@ pub struct WrappedQbftMessage { impl Display for WrappedQbftMessage { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let mut f = f.debug_struct("WrappedQbftMessage"); - f.field("operator_ids", self.signed_message.operator_ids()) + f.field("operator_ids", &self.signed_message.operator_ids()) .field("full_data", &!self.signed_message.full_data().is_empty()); self.qbft_message.format_fields(&mut f); f.finish() diff --git a/anchor/common/qbft/src/tests.rs b/anchor/common/qbft/src/tests.rs index 2deb8534a..fdbcffbf2 100644 --- a/anchor/common/qbft/src/tests.rs +++ b/anchor/common/qbft/src/tests.rs @@ -11,9 +11,9 @@ use std::{ use qbft_types::DefaultLeaderFunction; use sha2::{Digest, Sha256}; use ssv_types::{ - OperatorId, + OperatorId, RSA_SIGNATURE_SIZE, consensus::{NoDataValidation, QbftMessageType}, - message::{RSA_SIGNATURE_SIZE, SignedSSVMessage}, + message::SignedSSVMessage, }; use ssz_derive::{Decode, Encode}; use tracing::debug_span; @@ -60,7 +60,7 @@ fn convert_unsigned_to_signed( ) -> WrappedQbftMessage { // Create a signed message containing just this operator let signed_message = SignedSSVMessage::new( - vec![vec![0; RSA_SIGNATURE_SIZE]], + vec![[0; RSA_SIGNATURE_SIZE]], vec![OperatorId(*operator_id)], msg.unsigned_message.ssv_message, msg.unsigned_message.full_data, @@ -251,7 +251,7 @@ fn test_round_change_validation_skips_round_one_prepared_values() { use ssv_types::{ consensus::QbftMessage, - message::{MsgType, RSA_SIGNATURE_SIZE, SSVMessage, SignedSSVMessage}, + message::{MsgType, SSVMessage, SignedSSVMessage}, }; // Create QBFT instance @@ -284,8 +284,8 @@ fn test_round_change_validation_skips_round_one_prepared_values() { identifier: [0; 56].to_vec().into(), root: test_data.hash(), data_round: 1, // Claims preparation in round 1 - this is the bug trigger! - round_change_justification: vec![], - prepare_justification: vec![], // INVALID: No justifications for claimed preparation! + round_change_justification: vec![].into(), + prepare_justification: vec![].into(), // INVALID: No justifications for claimed preparation! }; // Create signed round change messages (need quorum of 3 for 3-node committee) @@ -300,7 +300,7 @@ fn test_round_change_validation_skips_round_one_prepared_values() { .expect("should create SSVMessage"); let signed_rc = SignedSSVMessage::new( - vec![vec![0; RSA_SIGNATURE_SIZE]], + vec![[0; RSA_SIGNATURE_SIZE]], vec![OperatorId::from(operator_id)], ssv_message, vec![], // no full_data for round change @@ -317,8 +317,12 @@ fn test_round_change_validation_skips_round_one_prepared_values() { identifier: [0; 56].to_vec().into(), root: test_data.hash(), data_round: 1, // Proposing the "prepared" value from round 1 - round_change_justification: signed_round_changes, - prepare_justification: vec![], // Proposals don't need prepare justifications + round_change_justification: signed_round_changes + .into_iter() + .map(|msg| msg.as_ssz_bytes().into()) + .collect::>() + .into(), + prepare_justification: vec![].into(), // Proposals don't need prepare justifications }; // Create the SSVMessage for the proposal @@ -330,7 +334,7 @@ fn test_round_change_validation_skips_round_one_prepared_values() { .expect("should create proposal SSVMessage"); let signed_proposal = SignedSSVMessage::new( - vec![vec![0; RSA_SIGNATURE_SIZE]], + vec![[0; RSA_SIGNATURE_SIZE]], vec![OperatorId::from(2)], // From operator 2 (leader for round 2) proposal_ssv_message, test_data.as_ssz_bytes(), // full_data for proposal @@ -382,8 +386,9 @@ fn test_leader_waits_when_highest_prepared_data_missing() { use std::sync::{Arc, Mutex}; use ssv_types::{ + RSA_SIGNATURE_SIZE, consensus::QbftMessage, - message::{MsgType, RSA_SIGNATURE_SIZE, SSVMessage, SignedSSVMessage}, + message::{MsgType, SSVMessage, SignedSSVMessage}, }; // Track messages sent by the QBFT instance @@ -431,11 +436,11 @@ fn test_leader_waits_when_highest_prepared_data_missing() { height: 0, round: 2, // Moving to round 2 identifier: [0; 56].to_vec().into(), - root: prepared_hash, // Claims this hash was prepared - data_round: 1, // Claims preparation happened in round 1 - round_change_justification: vec![], // No RC justifications needed for this test - prepare_justification: vec![], /* Should have prepare messages but we'll skip - * validation */ + root: prepared_hash, // Claims this hash was prepared + data_round: 1, // Claims preparation happened in round 1 + round_change_justification: vec![].into(), // No RC justifications needed for this test + prepare_justification: vec![].into(), /* Should have prepare messages but we'll skip + * validation */ }; let ssv_message = SSVMessage::new( @@ -446,7 +451,7 @@ fn test_leader_waits_when_highest_prepared_data_missing() { .expect("should create SSVMessage"); let signed_rc = SignedSSVMessage::new( - vec![vec![0; RSA_SIGNATURE_SIZE]], + vec![[0; RSA_SIGNATURE_SIZE]], vec![OperatorId::from(operator_id)], ssv_message, vec![], // CRITICAL: No full_data - this simulates the missing data scenario! diff --git a/anchor/common/ssv_types/Cargo.toml b/anchor/common/ssv_types/Cargo.toml index 6a3028e74..3a6a5abd1 100644 --- a/anchor/common/ssv_types/Cargo.toml +++ b/anchor/common/ssv_types/Cargo.toml @@ -19,10 +19,15 @@ indexmap = { workspace = true } openssl = { workspace = true } operator_key = { workspace = true } rusqlite = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } sha2 = { workspace = true } slashing_protection = { workspace = true } +ssz_types = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tree_hash = { workspace = true } tree_hash_derive = { workspace = true } +typenum = { workspace = true } types = { workspace = true } +zerocopy = "0.8.24" diff --git a/anchor/common/ssv_types/src/consensus.rs b/anchor/common/ssv_types/src/consensus.rs index 54ff9ca7d..544570c76 100644 --- a/anchor/common/ssv_types/src/consensus.rs +++ b/anchor/common/ssv_types/src/consensus.rs @@ -21,7 +21,7 @@ use types::{ AggregateAndProofBase, AggregateAndProofElectra, AttestationData, BlindedBeaconBlock, ChainSpec, Checkpoint, CommitteeIndex, Domain, EthSpec, ForkName, Hash256, PublicKeyBytes, Signature, Slot, SyncCommitteeContribution, VariableList, - typenum::{U13, U56}, + typenum::{Pow, Prod, Sum, U2, U3, U5, U13, U23, U56, U700, U852, U1000, U10000}, }; use crate::{ValidatorIndex, message::*}; @@ -56,6 +56,21 @@ impl QbftDataValidator for NoDataValidation { } } +/// ValidatorConsensusData.DataSSZ max size: 8388608 bytes (2^23) +/// This is the maximum size that the validator consensus data may be +/// Calculated as 2^23 = 8,388,608 +pub type ValidatorConsensusDataLen = >::Output; + +// RoundChange max size: 51852 +// This is the maximum size that a round change justification may be +// Calculated as (5 * 10,000) + 1,000 + 852 +pub type RoundChangeJustificationLength = Sum, Sum>; + +// Justification max size: 3700 +// This is the maximum size that a prepare justification may be +// Calculated as (3 * 1000) + 700 +pub type PrepareJustificationLength = Sum, U700>; // 3700 + /// A SSV Message that has not been signed yet. #[derive(Clone, Debug, Encode)] pub struct UnsignedSSVMessage { @@ -68,7 +83,7 @@ pub struct UnsignedSSVMessage { } /// A QBFT specific message -#[derive(Debug, Clone, Encode, Decode)] +#[derive(Debug, Clone, Encode, Decode, TreeHash)] #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] pub struct QbftMessage { pub qbft_message_type: QbftMessageType, @@ -78,8 +93,11 @@ pub struct QbftMessage { * encoding in go-client */ pub root: Hash256, pub data_round: u64, - pub round_change_justification: Vec, // always without full_data - pub prepare_justification: Vec, // always without full_data + // always without full data + pub round_change_justification: + VariableList, U13>, + // always without full data + pub prepare_justification: VariableList, U13>, } impl Display for QbftMessage { @@ -182,11 +200,31 @@ impl Decode for QbftMessageType { } } -#[derive(Clone, Debug, PartialEq, Encode, Decode)] +impl TreeHash for QbftMessageType { + fn tree_hash_type() -> TreeHashType { + TreeHashType::Basic + } + + fn tree_hash_packed_encoding(&self) -> PackedEncoding { + let value = *self as u64; + value.tree_hash_packed_encoding() + } + + fn tree_hash_packing_factor() -> usize { + u64::tree_hash_packing_factor() + } + + fn tree_hash_root(&self) -> tree_hash::Hash256 { + let value = *self as u64; + value.tree_hash_root() + } +} + +#[derive(Clone, Debug, PartialEq, Encode, Decode, TreeHash)] pub struct ValidatorConsensusData { pub duty: ValidatorDuty, pub version: DataVersion, - pub data_ssz: Vec, + pub data_ssz: VariableList, } impl QbftData for ValidatorConsensusData { @@ -282,9 +320,9 @@ impl ValidatorConsensusDataValidator { match value.duty.r#type { BEACON_ROLE_AGGREGATOR => { if value.version < DataVersion(ForkName::Electra) { - AggregateAndProofBase::::from_ssz_bytes(value.data_ssz.as_slice())?; + AggregateAndProofBase::::from_ssz_bytes(&value.data_ssz)?; } else { - AggregateAndProofElectra::::from_ssz_bytes(value.data_ssz.as_slice())?; + AggregateAndProofElectra::::from_ssz_bytes(&value.data_ssz)?; } } BEACON_ROLE_PROPOSER => { @@ -293,7 +331,7 @@ impl ValidatorConsensusDataValidator { BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION => { // There is nothing special to check for sync committee contributions. // We just need to ensure that the data is valid. - Contributions::::from_ssz_bytes(value.data_ssz.as_slice())?; + Contributions::::from_ssz_bytes(&value.data_ssz)?; } other => return Err(DataValidationError::InvalidDutyType(other)), }; @@ -470,6 +508,42 @@ impl Decode for DataVersion { } } +impl TreeHash for DataVersion { + fn tree_hash_type() -> TreeHashType { + TreeHashType::Basic + } + + fn tree_hash_packed_encoding(&self) -> PackedEncoding { + let num: u64 = match self.0 { + ForkName::Base => 1, + ForkName::Altair => 2, + ForkName::Bellatrix => 3, + ForkName::Capella => 4, + ForkName::Deneb => 5, + ForkName::Electra => 6, + ForkName::Fulu => 7, + }; + num.tree_hash_packed_encoding() + } + + fn tree_hash_packing_factor() -> usize { + u64::tree_hash_packing_factor() + } + + fn tree_hash_root(&self) -> tree_hash::Hash256 { + let num: u64 = match self.0 { + ForkName::Base => 1, + ForkName::Altair => 2, + ForkName::Bellatrix => 3, + ForkName::Capella => 4, + ForkName::Deneb => 5, + ForkName::Electra => 6, + ForkName::Fulu => 7, + }; + num.tree_hash_root() + } +} + #[derive(Clone, Debug, TreeHash, Encode, Decode)] pub struct Contribution { pub selection_proof_sig: Signature, diff --git a/anchor/common/ssv_types/src/lib.rs b/anchor/common/ssv_types/src/lib.rs index e871aed03..b905afc14 100644 --- a/anchor/common/ssv_types/src/lib.rs +++ b/anchor/common/ssv_types/src/lib.rs @@ -13,8 +13,30 @@ pub mod partial_sig; mod round; mod share; mod sql_conversions; +pub mod test_utils; pub use indexmap::IndexSet; pub use round::Round; pub use share::ENCRYPTED_KEY_LENGTH; +use ssz_types::typenum::Unsigned; pub use types::{Epoch, Slot, VariableList}; + +// Shared constants used across message types +pub const RSA_SIGNATURE_SIZE: usize = 256; +pub const MAX_SIGNATURES: usize = 13; + +/// Converts a Vec to VariableList, returning a custom error on failure. +pub fn try_to_variable_list(vec: Vec, error_fn: F) -> Result, E> +where + N: Unsigned + Clone, + F: FnOnce(usize, usize) -> E, +{ + let vec_len = vec.len(); + let max_len = N::to_usize(); + + if vec_len <= max_len { + Ok(VariableList::from(vec)) + } else { + Err(error_fn(vec_len, max_len)) + } +} diff --git a/anchor/common/ssv_types/src/message.rs b/anchor/common/ssv_types/src/message.rs index 716752c29..28677bcb1 100644 --- a/anchor/common/ssv_types/src/message.rs +++ b/anchor/common/ssv_types/src/message.rs @@ -5,29 +5,28 @@ use std::{ use ssz::{Decode, DecodeError, Encode}; use ssz_derive::{Decode, Encode}; +use ssz_types::VariableList; use thiserror::Error; +use tree_hash::{PackedEncoding, TreeHash, TreeHashType}; +use tree_hash_derive::TreeHash; +use typenum::Unsigned; +use types::{ + Hash256, + typenum::{Prod, Sum, U8, U13, U256, U388, U412, U722, U836, U1000, U1000000}, +}; use crate::{ - OperatorId, - message::{ - SSVMessageError::{EmptyData, SSVDataTooBig}, - SignedSSVMessageError::{ - DuplicatedSigner, FullDataTooLong, NoSignatures, NoSigners, - SignersAndSignaturesWithDifferentLength, SignersNotSorted, TooManyOperatorIDs, - TooManySignatures, WrongRSASignatureSize, ZeroSigner, - }, - }, + MAX_SIGNATURES, OperatorId, RSA_SIGNATURE_SIZE, + consensus::{PrepareJustificationLength, RoundChangeJustificationLength}, msgid::MessageId, + try_to_variable_list, }; const QBFT_MSG_TYPE_SIZE: usize = 8; const HEIGHT_SIZE: usize = 8; const ROUND_SIZE: usize = 8; -const MAX_NO_JUSTIFICATION_SIZE: usize = 3616; -const MAX1_JUSTIFICATION_SIZE: usize = 50624; const IDENTIFIER_SIZE: usize = 56; // same as MessageId length const ROOT_SIZE: usize = 32; -const MAX_SIGNATURES: usize = 13; // For partial signatures const PARTIAL_SIGNATURE_SIZE: usize = 96; @@ -36,38 +35,34 @@ const VALIDATOR_INDEX_SIZE: usize = 8; const SLOT_SIZE: usize = 8; const PARTIAL_SIG_MSG_TYPE_SIZE: usize = 8; const MAX_PARTIAL_SIGNATURE_MESSAGES: usize = 1000; -const ENCODING_OVERHEAD_DIVISOR: usize = 20; - -// For RSA-based SignedSSVMessage -pub const RSA_SIGNATURE_SIZE: usize = 256; - -// Additional from the Go code -const MAX_FULL_DATA_SIZE: usize = 4_194_532; // from spectypes.SignedSSVMessage const MAX_CONSENSUS_MSG_SIZE: usize = QBFT_MSG_TYPE_SIZE + HEIGHT_SIZE + ROUND_SIZE - + IDENTIFIER_SIZE + + (IDENTIFIER_SIZE + ssz::BYTES_PER_LENGTH_OFFSET) + ROOT_SIZE + ROUND_SIZE - + MAX_SIGNATURES * (MAX_NO_JUSTIFICATION_SIZE + MAX1_JUSTIFICATION_SIZE); - -const MAX_ENCODED_CONSENSUS_MSG_SIZE: usize = - MAX_CONSENSUS_MSG_SIZE + (MAX_CONSENSUS_MSG_SIZE / ENCODING_OVERHEAD_DIVISOR) + 4; + + (MAX_SIGNATURES * (RoundChangeJustificationLength::USIZE + ssz::BYTES_PER_LENGTH_OFFSET) + + ssz::BYTES_PER_LENGTH_OFFSET) + + (MAX_SIGNATURES * (PrepareJustificationLength::USIZE + ssz::BYTES_PER_LENGTH_OFFSET) + + ssz::BYTES_PER_LENGTH_OFFSET); const PARTIAL_SIGNATURE_MSG_SIZE: usize = PARTIAL_SIGNATURE_SIZE + ROOT_SIZE + OPERATOR_ID_SIZE + VALIDATOR_INDEX_SIZE; const MAX_PARTIAL_SIGNATURE_MSGS_SIZE: usize = PARTIAL_SIG_MSG_TYPE_SIZE + SLOT_SIZE - + MAX_PARTIAL_SIGNATURE_MESSAGES * PARTIAL_SIGNATURE_MSG_SIZE; + + MAX_PARTIAL_SIGNATURE_MESSAGES * PARTIAL_SIGNATURE_MSG_SIZE + + ssz::BYTES_PER_LENGTH_OFFSET; -const MAX_ENCODED_PARTIAL_SIGNATURE_SIZE: usize = MAX_PARTIAL_SIGNATURE_MSGS_SIZE - + (MAX_PARTIAL_SIGNATURE_MSGS_SIZE / ENCODING_OVERHEAD_DIVISOR) - + 4; +const MAX_FULL_DATA_SIZE: usize = SSVMessageFullDataLen::USIZE; + +/// SSVMessage.Data max size: 722412 (from Go spec) +/// 722412 = 722 * 1000 + 412 = 722000 + 412 +pub type SSVMessageDataLen = Sum, U412>; /// Defines the types of messages with explicit discriminant values. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Copy)] #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] #[repr(u64)] pub enum MsgType { @@ -75,6 +70,26 @@ pub enum MsgType { SSVPartialSignatureMsgType = 1, } +impl TreeHash for MsgType { + fn tree_hash_type() -> TreeHashType { + TreeHashType::Basic + } + + fn tree_hash_packed_encoding(&self) -> PackedEncoding { + let value = *self as u64; + value.tree_hash_packed_encoding() + } + + fn tree_hash_packing_factor() -> usize { + u64::tree_hash_packing_factor() + } + + fn tree_hash_root(&self) -> Hash256 { + let value = *self as u64; + value.tree_hash_root() + } +} + impl TryFrom for MsgType { type Error = DecodeError; @@ -121,17 +136,7 @@ impl Decode for MsgType { } fn from_ssz_bytes(bytes: &[u8]) -> Result { - if bytes.len() != U64_SIZE { - return Err(DecodeError::InvalidByteLength { - len: bytes.len(), - expected: U64_SIZE, - }); - } - let value = - u64::from_le_bytes(bytes.try_into().map_err(|_| { - DecodeError::BytesInvalid(format!("Invalid length: {}", bytes.len())) - })?); - value.try_into() + u64::from_ssz_bytes(bytes)?.try_into() } } @@ -141,8 +146,8 @@ pub enum SSVMessageError { #[error("SSVMessage data is empty")] EmptyData, - #[error("SSVMessage data too large: got {got}, max {max}")] - SSVDataTooBig { got: usize, max: usize }, + #[error("SSVMessage data too large: got {provided}, max {max}")] + SSVDataTooBig { provided: usize, max: usize }, #[error("Wrong domain: got {got}, expected {want}")] WrongDomain { got: String, want: String }, @@ -152,12 +157,12 @@ pub enum SSVMessageError { } /// Represents a bare SSVMessage with a type, ID, and data. -#[derive(Encode, Decode, Clone, PartialEq, Eq)] +#[derive(Encode, Decode, Clone, PartialEq, Eq, TreeHash)] #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] pub struct SSVMessage { msg_type: MsgType, - msg_id: MessageId, // Fixed-size [u8; 56] - data: Vec, // Variable-length byte array + msg_id: MessageId, + data: VariableList, } impl Debug for SSVMessage { @@ -165,13 +170,13 @@ impl Debug for SSVMessage { f.debug_struct("SSVMessage") .field("msg_type", &self.msg_type) .field("msg_id", &self.msg_id) - .field("data", &hex::encode(&self.data)) + .field("data", &hex::encode(self.data.to_vec())) .finish() } } impl SSVMessage { - /// Creates a new `SSVMessage`. + /// Creates a new `SSVMessage` using a vec instead of a `VariableList`. /// /// # Arguments /// @@ -194,6 +199,10 @@ impl SSVMessage { msg_id: MessageId, data: Vec, ) -> Result { + let data = try_to_variable_list::(data, |provided, max| { + SSVMessageError::SSVDataTooBig { provided, max } + })?; + let ssv_message = SSVMessage { msg_type, msg_id, @@ -203,24 +212,25 @@ impl SSVMessage { Ok(ssv_message) } + /// Validate the SSV Message pub fn validate(&self) -> Result<(), SSVMessageError> { if self.data.is_empty() { - return Err(EmptyData); + return Err(SSVMessageError::EmptyData); } match self.msg_type { MsgType::SSVConsensusMsgType => { - if self.data.len() > MAX_ENCODED_CONSENSUS_MSG_SIZE { - return Err(SSVDataTooBig { - got: self.data.len(), - max: MAX_ENCODED_CONSENSUS_MSG_SIZE, + if self.data.len() > MAX_CONSENSUS_MSG_SIZE { + return Err(SSVMessageError::SSVDataTooBig { + provided: self.data.len(), + max: MAX_CONSENSUS_MSG_SIZE, }); } } MsgType::SSVPartialSignatureMsgType => { - if self.data.len() > MAX_ENCODED_PARTIAL_SIGNATURE_SIZE { - return Err(SSVDataTooBig { - got: self.data.len(), - max: MAX_ENCODED_PARTIAL_SIGNATURE_SIZE, + if self.data.len() > MAX_PARTIAL_SIGNATURE_MSGS_SIZE { + return Err(SSVMessageError::SSVDataTooBig { + provided: self.data.len(), + max: MAX_PARTIAL_SIGNATURE_MSGS_SIZE, }); } } @@ -242,6 +252,20 @@ impl SSVMessage { pub fn data(&self) -> &[u8] { &self.data } + + /// A testing helping function to create invalid messages. + #[cfg(test)] + pub fn new_unvalidated( + msg_type: MsgType, + msg_id: MessageId, + data: VariableList, + ) -> Self { + SSVMessage { + msg_type, + msg_id, + data, + } + } } /// Errors that can occur while creating a `SignedSSVMessage`. @@ -262,8 +286,8 @@ pub enum SignedSSVMessageError { #[error("Too many operator IDs: provided {provided}, maximum allowed is {max}.")] TooManyOperatorIDs { provided: usize, max: usize }, - #[error("Full data is too long: {length} bytes, maximum allowed is {max} bytes.")] - FullDataTooLong { length: usize, max: usize }, + #[error("Full data is too long: {provided} bytes, maximum allowed is {max} bytes.")] + FullDataTooLong { provided: usize, max: usize }, #[error("No signers were provided (must have at least one signer).")] NoSigners, @@ -284,52 +308,69 @@ pub enum SignedSSVMessageError { DuplicatedSigner, #[error("Invalid SSVMessage: {0}")] - SSVMessagError(#[from] SSVMessageError), + SSVMessageError(#[from] SSVMessageError), } +/// SignedSSVMessage.FullData max size: 8388836 (from Go spec) +/// 8388836 = 8000000 + 388836 = 8 * 1000000 + 388836 +/// We need to construct 388836 = 388 * 1000 + 836 = 388000 + 836 +type SSVMessageFullDataLen = Sum, Sum, U836>>; + +/// Maximum of 13 signatures. +pub type SignatureList = VariableList, U13>; + /// Represents a signed SSV Message with signatures, operator IDs, the message itself, and full /// data. -#[derive(Encode, Decode, Clone, PartialEq, Eq)] +#[derive(Encode, Decode, Clone, PartialEq, Eq, TreeHash)] pub struct SignedSSVMessage { - signatures: Vec>, // Vec of Vec, max 13 elements, each with 256 bytes - operator_ids: Vec, // Vec of OperatorID (u64), max 13 elements - ssv_message: SSVMessage, // SSVMessage: Required field - full_data: Vec, // Variable-length byte array, max 4,194,532 bytes + signatures: SignatureList, + operator_ids: VariableList, + ssv_message: SSVMessage, + full_data: VariableList, } #[cfg(feature = "arbitrary-fuzz")] -use arbitrary::{Arbitrary, Result, Unstructured}; +mod arbitrary_impls { + use arbitrary::{Arbitrary, Result, Unstructured}; + use ssz::Encode; -#[cfg(feature = "arbitrary-fuzz")] -use crate::consensus::{BeaconVote, QbftMessage}; - -#[cfg(feature = "arbitrary-fuzz")] -impl<'a> Arbitrary<'a> for SignedSSVMessage { - fn arbitrary(u: &mut Unstructured<'a>) -> Result { - // Generate arbitrary BeaconVote - let beacon_vote = BeaconVote::arbitrary(u)?; - - // Generate arbitrary QbftMessage - let qbft_message = QbftMessage::arbitrary(u)?; - - // Create arbitrary basic fields - let signatures = Vec::>::arbitrary(u)?; - let operator_ids = Vec::::arbitrary(u)?; - - // Create SSV message with serialized QbftMessage - let ssv_message = SSVMessage { - msg_type: MsgType::arbitrary(u)?, - msg_id: MessageId::arbitrary(u)?, - data: qbft_message.as_ssz_bytes(), // Serialize QbftMessage to bytes - }; - - // Create the SignedSSVMessage with serialized BeaconVote - Ok(SignedSSVMessage { - signatures, - operator_ids, - ssv_message, - full_data: beacon_vote.as_ssz_bytes(), // Serialize BeaconVote to bytes - }) + use super::*; + use crate::{ + RSA_SIGNATURE_SIZE, + consensus::{BeaconVote, QbftMessage}, + message::MsgType, + msgid::MessageId, + }; + + impl<'a> Arbitrary<'a> for SignedSSVMessage { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + // Generate arbitrary BeaconVote + let beacon_vote = BeaconVote::arbitrary(u)?; + + // Generate arbitrary QbftMessage + let qbft_message = QbftMessage::arbitrary(u)?; + + // Create arbitrary basic fields + let signatures = Vec::<[u8; RSA_SIGNATURE_SIZE]>::arbitrary(u)?; + let operator_ids = Vec::::arbitrary(u)?; + + // Create SSV message with serialized QbftMessage + let ssv_message = SSVMessage::new( + MsgType::arbitrary(u)?, + MessageId::arbitrary(u)?, + qbft_message.as_ssz_bytes(), // Serialize QbftMessage to bytes + ) + .expect("Valid SSVMessage"); + + // Create the SignedSSVMessage with serialized BeaconVote + Ok(SignedSSVMessage::new( + signatures, + operator_ids, + ssv_message, + beacon_vote.as_ssz_bytes(), // Serialize BeaconVote to bytes + ) + .expect("Valid SignedSSVMessage")) + } } } @@ -349,13 +390,17 @@ impl Display for SignedSSVMessage { impl Debug for SignedSSVMessage { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let signatures = self.signatures.iter().map(hex::encode).collect::>(); + let signatures = (&self.signatures) + .into_iter() + .map(|v| v.to_vec()) + .map(hex::encode) + .collect::>(); f.debug_struct("SignedSSVMessage") .field("signatures", &signatures) .field("operator_ids", &self.operator_ids) .field("ssv_message", &self.ssv_message) - .field("full_data", &hex::encode(&self.full_data)) + .field("full_data", &hex::encode(&*self.full_data)) .finish() } } @@ -388,25 +433,41 @@ impl SignedSSVMessage { /// vec![1, 2, 3], /// ) /// .unwrap(); - /// let signed_msg = SignedSSVMessage::new( - /// vec![vec![0; 256]], - /// vec![OperatorId(1)], - /// ssv_msg, - /// vec![4, 5, 6], - /// ) - /// .unwrap(); + /// let signed_msg = + /// SignedSSVMessage::new(vec![[0; 256]], vec![OperatorId(1)], ssv_msg, vec![4, 5, 6]).unwrap(); /// ``` pub fn new( - signatures: Vec>, + signatures: Vec<[u8; RSA_SIGNATURE_SIZE]>, operator_ids: Vec, ssv_message: SSVMessage, full_data: Vec, ) -> Result { + // Convert Vec<[u8; 256]> to VariableList, U13> + // First convert each [u8; 256] to VariableList + // This will always succeed since sig is [u8; 256] and U256 = 256 + let signature_variable_lists: Vec<_> = signatures + .into_iter() + .map(|sig| VariableList::from(sig.to_vec())) + .collect(); + + // Then convert the Vec of VariableLists to VariableList, U13> + // This can fail if we have more than 13 signatures + let signatures = try_to_variable_list::, U13, _, _>( + signature_variable_lists, + |provided, max| SignedSSVMessageError::TooManySignatures { provided, max }, + )?; + let signed_ssv_message = SignedSSVMessage { signatures, - operator_ids, + operator_ids: try_to_variable_list::( + operator_ids, + |provided, max| SignedSSVMessageError::TooManyOperatorIDs { provided, max }, + )?, ssv_message, - full_data, + full_data: try_to_variable_list::( + full_data, + |provided, max| SignedSSVMessageError::FullDataTooLong { provided, max }, + )?, }; signed_ssv_message.validate()?; @@ -415,12 +476,12 @@ impl SignedSSVMessage { } /// Returns a reference to the signatures. - pub fn signatures(&self) -> &Vec> { + pub fn signatures(&self) -> &SignatureList { &self.signatures } /// Returns a reference to the operator IDs. - pub fn operator_ids(&self) -> &Vec { + pub fn operator_ids(&self) -> &[OperatorId] { &self.operator_ids } @@ -434,19 +495,51 @@ impl SignedSSVMessage { &self.full_data } - pub fn set_full_data(&mut self, data: Vec) { - self.full_data = data; + /// Set the fulldata on the message + pub fn set_full_data(&mut self, data: Vec) -> Result<(), SignedSSVMessageError> { + self.full_data = try_to_variable_list(data, |provided, max| { + SignedSSVMessageError::FullDataTooLong { provided, max } + })?; + Ok(()) + } + + /// Returns a clone of this SignedSSVMessage with empty full_data. + pub fn without_full_data(&self) -> Self { + Self { + signatures: self.signatures.clone(), + operator_ids: self.operator_ids.clone(), + ssv_message: self.ssv_message.clone(), + full_data: VariableList::empty(), + } } /// Aggregate a set of signed ssv messages into Self - pub fn aggregate(&mut self, others: I) + pub fn aggregate(&mut self, others: I) -> Result<(), SignedSSVMessageError> where I: IntoIterator, { for signed_msg in others { + if signed_msg.operator_ids.len() != signed_msg.signatures.len() { + return Err(SignedSSVMessageError::SignersAndSignaturesWithDifferentLength); + } + // These will only all have 1 signature/operator, but we call extend for safety - self.signatures.extend(signed_msg.signatures); - self.operator_ids.extend(signed_msg.operator_ids); + for signature in signed_msg.signatures.into_iter() { + self.signatures.push(signature).map_err(|_| { + SignedSSVMessageError::TooManySignatures { + provided: self.signatures.len() + 1, + max: MAX_SIGNATURES, + } + })?; + } + for operator_id in signed_msg.operator_ids.into_iter() { + self.operator_ids.push(operator_id).map_err(|_| { + SignedSSVMessageError::TooManyOperatorIDs { + provided: self.operator_ids.len() + 1, + max: MAX_SIGNATURES, + } + })?; + } } // Maintain id <-> sig pairing during sorting @@ -459,15 +552,22 @@ impl SignedSSVMessage { sig_pairs.sort_by_key(|&(_, op_id)| *op_id); - let (sorted_signatures, sorted_operator_ids) = sig_pairs.into_iter().unzip(); - self.signatures = sorted_signatures; - self.operator_ids = sorted_operator_ids; + let (sorted_signatures, sorted_operator_ids): (Vec<_>, Vec<_>) = + sig_pairs.iter().cloned().unzip(); + self.signatures = try_to_variable_list::, U13, _, _>( + sorted_signatures, + |provided, max| SignedSSVMessageError::TooManySignatures { provided, max }, + )?; + self.operator_ids = + try_to_variable_list::(sorted_operator_ids, |provided, max| { + SignedSSVMessageError::TooManyOperatorIDs { provided, max } + })?; + Ok(()) } - // Validate the signed message to ensure that it is well formed for qbft processing pub fn validate(&self) -> Result<(), SignedSSVMessageError> { if self.signatures.len() > MAX_SIGNATURES { - return Err(TooManySignatures { + return Err(SignedSSVMessageError::TooManySignatures { provided: self.signatures.len(), max: MAX_SIGNATURES, }); @@ -475,7 +575,7 @@ impl SignedSSVMessage { for (i, sig) in self.signatures.iter().enumerate() { if sig.len() != RSA_SIGNATURE_SIZE { - return Err(WrongRSASignatureSize { + return Err(SignedSSVMessageError::WrongRSASignatureSize { index: i, length: sig.len(), sig_length: RSA_SIGNATURE_SIZE, @@ -484,37 +584,37 @@ impl SignedSSVMessage { } if self.operator_ids.len() > MAX_SIGNATURES { - return Err(TooManyOperatorIDs { + return Err(SignedSSVMessageError::TooManyOperatorIDs { provided: self.operator_ids.len(), max: MAX_SIGNATURES, }); } if self.full_data.len() > MAX_FULL_DATA_SIZE { - return Err(FullDataTooLong { - length: self.full_data.len(), + return Err(SignedSSVMessageError::FullDataTooLong { + provided: self.full_data.len(), max: MAX_FULL_DATA_SIZE, }); } // Rule: Must have at least one signer if self.operator_ids.is_empty() { - return Err(NoSigners); + return Err(SignedSSVMessageError::NoSigners); } if self.signatures.is_empty() { - return Err(NoSignatures); + return Err(SignedSSVMessageError::NoSignatures); } if !self.operator_ids.is_sorted() { - return Err(SignersNotSorted); + return Err(SignedSSVMessageError::SignersNotSorted); } // Note: Len Signers & Operators will only be > 1 after commit aggregation // Rule: Signer can't be zero if self.operator_ids.iter().any(|&id| *id == 0) { - return Err(ZeroSigner); + return Err(SignedSSVMessageError::ZeroSigner); } // Rule: Signers must be unique @@ -523,13 +623,13 @@ impl SignedSSVMessage { let mut seen_ids = HashSet::with_capacity(self.operator_ids.len()); for &id in &self.operator_ids { if !seen_ids.insert(id) { - return Err(DuplicatedSigner); + return Err(SignedSSVMessageError::DuplicatedSigner); } } // Rule: Len(Signers) must be equal to Len(Signatures) if self.operator_ids.len() != self.signatures.len() { - return Err(SignersAndSignaturesWithDifferentLength); + return Err(SignedSSVMessageError::SignersAndSignaturesWithDifferentLength); } self.ssv_message.validate()?; @@ -543,44 +643,16 @@ mod tests { use std::iter; use ssz::{Decode, Encode}; + use types::{Signature, Unsigned}; use super::*; - - // Helper functions for building valid test data - // - - /// Returns a default 56-byte ID array with all zeros. - fn default_msg_id() -> MessageId { - [0u8; IDENTIFIER_SIZE].into() - } - - /// Returns a small, non-empty payload for SSVMessage data. - fn small_data() -> Vec { - vec![0x11, 0x22, 0x33] - } - - /// Returns a valid signature of exactly [`RSA_SIGNATURE_SIZE`] bytes. - fn valid_signature() -> Vec { - vec![0u8; RSA_SIGNATURE_SIZE] - } - - /// Creates a valid, non-empty SSVMessage (ensuring it doesn’t exceed the max size). - fn valid_ssv_message() -> SSVMessage { - SSVMessage::new(MsgType::SSVConsensusMsgType, default_msg_id(), small_data()) - .expect("Creating a valid SSVMessage must succeed") - } - - /// Creates a single-signer, single-signature valid SignedSSVMessage. - fn valid_signed_ssv_message() -> SignedSSVMessage { - let msg = valid_ssv_message(); - SignedSSVMessage::new( - vec![valid_signature()], - vec![OperatorId(1)], - msg, - vec![0xAB, 0xCD], // "full_data" well under max - ) - .expect("Creating a valid SignedSSVMessage must succeed") - } + use crate::{ + consensus::{QbftMessage, QbftMessageType}, + partial_sig::{PartialSignatureKind, PartialSignatureMessage, PartialSignatureMessages}, + test_utils::{ + default_msg_id, valid_signature, valid_signed_ssv_message, valid_ssv_message, + }, + }; // Tests for MessageId // @@ -687,14 +759,14 @@ mod tests { /// Checks that data exceeding `MAX_CONSENSUS_MSG_SIZE` triggers `SSVDataTooBig`. #[test] fn test_consensus_message_too_big() { - let oversized = vec![0u8; MAX_ENCODED_CONSENSUS_MSG_SIZE + 1]; + let oversized = vec![0u8; MAX_CONSENSUS_MSG_SIZE + 1]; let result = SSVMessage::new(MsgType::SSVConsensusMsgType, default_msg_id(), oversized); match result { - Err(SSVDataTooBig { got, max }) => { - assert_eq!(got, MAX_ENCODED_CONSENSUS_MSG_SIZE + 1); - assert_eq!(max, MAX_ENCODED_CONSENSUS_MSG_SIZE); + Err(SSVMessageError::SSVDataTooBig { provided, max }) => { + assert_eq!(provided, MAX_CONSENSUS_MSG_SIZE + 1); + assert_eq!(max, MAX_CONSENSUS_MSG_SIZE); } other => panic!("Expected SSVDataTooBig, got {other:?}"), } @@ -703,7 +775,7 @@ mod tests { /// Checks that data exceeding `MAX_PARTIAL_SIGNATURE_MSGS_SIZE` triggers `SSVDataTooBig`. #[test] fn test_partial_signature_message_too_big() { - let oversized = vec![0u8; MAX_ENCODED_PARTIAL_SIGNATURE_SIZE + 1]; + let oversized = vec![0u8; MAX_PARTIAL_SIGNATURE_MSGS_SIZE + 1]; let result = SSVMessage::new( MsgType::SSVPartialSignatureMsgType, @@ -712,9 +784,9 @@ mod tests { ); match result { - Err(SSVDataTooBig { got, max }) => { - assert_eq!(got, MAX_ENCODED_PARTIAL_SIGNATURE_SIZE + 1); - assert_eq!(max, MAX_ENCODED_PARTIAL_SIGNATURE_SIZE); + Err(SSVMessageError::SSVDataTooBig { provided, max }) => { + assert_eq!(provided, MAX_PARTIAL_SIGNATURE_MSGS_SIZE + 1); + assert_eq!(max, MAX_PARTIAL_SIGNATURE_MSGS_SIZE); } other => panic!("Expected SSVDataTooBig, got {other:?}"), } @@ -781,7 +853,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(TooManySignatures { provided, max }) => { + Err(SignedSSVMessageError::TooManySignatures { provided, max }) => { assert_eq!(provided, MAX_SIGNATURES + 1); assert_eq!(max, MAX_SIGNATURES); } @@ -789,32 +861,6 @@ mod tests { } } - /// Checks that a signature with the wrong size triggers `WrongRSASignatureSize`. - #[test] - fn test_signed_ssv_message_wrong_signature_size() { - let ssv_msg = valid_ssv_message(); - let good = valid_signature(); - let mut bad = valid_signature(); - bad.pop(); // now it’s 255 bytes - let sigs = vec![good, bad]; - let ops = vec![OperatorId(1), OperatorId(2)]; - - let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); - - match result { - Err(WrongRSASignatureSize { - index, - length, - sig_length, - }) => { - assert_eq!(index, 1); - assert_eq!(length, 255); - assert_eq!(sig_length, RSA_SIGNATURE_SIZE); - } - other => panic!("Expected WrongRSASignatureSize, got {other:?}"), - } - } - /// Checks that having too many operator IDs triggers `TooManyOperatorIDs`. #[test] fn test_signed_ssv_message_too_many_operator_ids() { @@ -825,7 +871,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(TooManyOperatorIDs { provided, max }) => { + Err(SignedSSVMessageError::TooManyOperatorIDs { provided, max }) => { assert_eq!(provided, MAX_SIGNATURES + 1); assert_eq!(max, MAX_SIGNATURES); } @@ -863,8 +909,8 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, huge_data); match result { - Err(FullDataTooLong { length, max }) => { - assert_eq!(length, MAX_FULL_DATA_SIZE + 1); + Err(SignedSSVMessageError::FullDataTooLong { provided, max }) => { + assert_eq!(provided, MAX_FULL_DATA_SIZE + 1); assert_eq!(max, MAX_FULL_DATA_SIZE); } other => panic!("Expected FullDataTooLong, got {other:?}"), @@ -896,7 +942,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(NoSigners) => (), + Err(SignedSSVMessageError::NoSigners) => (), other => panic!("Expected NoSigners, got {other:?}"), } } @@ -911,7 +957,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(NoSignatures) => (), + Err(SignedSSVMessageError::NoSignatures) => (), other => panic!("Expected NoSignatures, got {other:?}"), } } @@ -927,7 +973,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(SignersNotSorted) => (), + Err(SignedSSVMessageError::SignersNotSorted) => (), other => panic!("Expected SignersNotSorted, got {other:?}"), } } @@ -942,7 +988,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(ZeroSigner) => (), + Err(SignedSSVMessageError::ZeroSigner) => (), other => panic!("Expected ZeroSigner, got {other:?}"), } } @@ -958,7 +1004,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(DuplicatedSigner) => (), + Err(SignedSSVMessageError::DuplicatedSigner) => (), other => panic!("Expected DuplicatedSigner, got {other:?}"), } } @@ -973,7 +1019,7 @@ mod tests { let result = SignedSSVMessage::new(sigs, ops, ssv_msg, vec![]); match result { - Err(SignersAndSignaturesWithDifferentLength) => (), + Err(SignedSSVMessageError::SignersAndSignaturesWithDifferentLength) => (), other => panic!("Expected SignersAndSignaturesWithDifferentLength, got {other:?}"), } } @@ -999,7 +1045,7 @@ mod tests { } /// If we pass an invalid `SSVMessage` (e.g. empty data) to SignedSSVMessage, - /// we expect a `SignedSSVMessageError::SSVMessagError(SSVMessageError::EmptyData)`. + /// we expect a `SignedSSVMessageError::SSVMessageError(SSVMessageError::EmptyData)`. #[test] fn test_invalid_ssv_message_propagates_error() { let empty_msg = SSVMessage::new(MsgType::SSVConsensusMsgType, default_msg_id(), vec![]); @@ -1012,11 +1058,11 @@ mod tests { // Force the scenario: pretend we got an SSVMessage from somewhere else // that didn't call `new()`, and attempt to use it: - let forcibly_invalid_msg = SSVMessage { - msg_type: MsgType::SSVConsensusMsgType, - msg_id: default_msg_id(), - data: vec![], // still empty - }; + let forcibly_invalid_msg = SSVMessage::new_unvalidated( + MsgType::SSVConsensusMsgType, + default_msg_id(), + VariableList::empty(), // still empty + ); let result = SignedSSVMessage::new( vec![valid_signature()], vec![OperatorId(1)], @@ -1025,8 +1071,8 @@ mod tests { ); match result { - Err(SignedSSVMessageError::SSVMessagError(SSVMessageError::EmptyData)) => (), - other => panic!("Expected SSVMessagError(EmptyData), got {other:?}"), + Err(SignedSSVMessageError::SSVMessageError(SSVMessageError::EmptyData)) => (), + other => panic!("Expected SSVMessageError(EmptyData), got {other:?}"), } } @@ -1045,7 +1091,8 @@ mod tests { ) .expect("Should be valid"); - base.aggregate(iter::once(extra)); + base.aggregate(iter::once(extra)) + .expect("Aggregation should succeed"); let ops = base.operator_ids(); let sigs = base.signatures(); assert_eq!( @@ -1055,4 +1102,108 @@ mod tests { ); assert_eq!(sigs.len(), 2, "Expected 2 signatures total"); } + + // Test for message size constants + /// Test that SSVMessage properly rejects data that's too large for VariableList + #[test] + fn test_ssv_message_variable_list_size_enforcement() { + // Data within the limit should work + let valid_data = vec![0u8; 100]; + let result = SSVMessage::new( + MsgType::SSVConsensusMsgType, + default_msg_id(), + valid_data.clone(), + ); + assert!(result.is_ok(), "Valid size data should succeed"); + + // Data exactly at MAX_CONSENSUS_MSG_SIZE should work + let max_data = vec![0u8; MAX_CONSENSUS_MSG_SIZE]; + let result = SSVMessage::new(MsgType::SSVConsensusMsgType, default_msg_id(), max_data); + assert!(result.is_ok(), "Data at max size should succeed"); + + // Data exceeding MAX_CONSENSUS_MSG_SIZE should fail + let oversized = vec![0u8; MAX_CONSENSUS_MSG_SIZE + 1]; + let result = SSVMessage::new(MsgType::SSVConsensusMsgType, default_msg_id(), oversized); + match result { + Err(SSVMessageError::SSVDataTooBig { provided, max }) => { + assert_eq!(provided, MAX_CONSENSUS_MSG_SIZE + 1); + assert_eq!(max, MAX_CONSENSUS_MSG_SIZE); + } + other => panic!("Expected SSVDataTooBig error, got: {:?}", other), + } + + // Verify the internal VariableList conversion also enforces the limit + // This tests that try_to_variable_list properly converts size errors + let large_vec = vec![0u8; SSVMessageDataLen::to_usize() + 1]; + let result: Result, SSVMessageError> = + try_to_variable_list(large_vec, |provided, max| SSVMessageError::SSVDataTooBig { + provided, + max, + }); + match result { + Err(SSVMessageError::SSVDataTooBig { provided, max }) => { + assert_eq!(provided, SSVMessageDataLen::to_usize() + 1); + assert_eq!(max, SSVMessageDataLen::to_usize()); + } + other => panic!( + "try_to_variable_list should fail with SSVDataTooBig: {:?}", + other + ), + } + } + + #[test] + fn ensure_message_sizes_correct() { + let messages_vec = vec![ + PartialSignatureMessage { + partial_signature: Signature::empty(), + signing_root: Default::default(), + signer: Default::default(), + validator_index: Default::default(), + }; + 1000 + ]; + let partial_signature_messages = PartialSignatureMessages { + kind: PartialSignatureKind::PostConsensus, + slot: Default::default(), + messages: ssz_types::VariableList::new(messages_vec).unwrap(), + }; + + assert_eq!( + partial_signature_messages.ssz_bytes_len(), + MAX_PARTIAL_SIGNATURE_MSGS_SIZE, + ); + + let qbft_message = QbftMessage { + qbft_message_type: QbftMessageType::Proposal, + height: 0, + round: 0, + identifier: vec![0; 56].try_into().unwrap(), + root: Default::default(), + data_round: 0, + round_change_justification: vec![ + vec![0; RoundChangeJustificationLength::USIZE] + .try_into() + .unwrap(); + 13 + ] + .try_into() + .unwrap(), + prepare_justification: vec![ + vec![0; PrepareJustificationLength::USIZE] + .try_into() + .unwrap(); + 13 + ] + .try_into() + .unwrap(), + }; + + assert_eq!(qbft_message.ssz_bytes_len(), MAX_CONSENSUS_MSG_SIZE); + + assert_eq!( + SSVMessageDataLen::to_usize(), + std::cmp::max(MAX_PARTIAL_SIGNATURE_MSGS_SIZE, MAX_CONSENSUS_MSG_SIZE) + ); + } } diff --git a/anchor/common/ssv_types/src/msgid.rs b/anchor/common/ssv_types/src/msgid.rs index bd594d711..490ff8056 100644 --- a/anchor/common/ssv_types/src/msgid.rs +++ b/anchor/common/ssv_types/src/msgid.rs @@ -2,6 +2,7 @@ use std::fmt::{Debug, Formatter}; use derive_more::{Display, From, Into}; use ssz::{Decode, DecodeError, Encode}; +use tree_hash::{PackedEncoding, TreeHash, TreeHashType}; use types::{PublicKeyBytes, VariableList, typenum::U56}; use crate::{committee::CommitteeId, domain_type::DomainType}; @@ -68,6 +69,24 @@ pub enum DutyExecutor { #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] pub struct MessageId([u8; 56]); +impl TreeHash for MessageId { + fn tree_hash_type() -> TreeHashType { + TreeHashType::Vector + } + + fn tree_hash_packed_encoding(&self) -> PackedEncoding { + unreachable!("Vector should never be packed.") + } + + fn tree_hash_packing_factor() -> usize { + unreachable!("Vector should never be packed.") + } + + fn tree_hash_root(&self) -> tree_hash::Hash256 { + self.0.tree_hash_root() + } +} + impl Debug for MessageId { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{}", hex::encode(self.0)) diff --git a/anchor/common/ssv_types/src/operator.rs b/anchor/common/ssv_types/src/operator.rs index 5d4ba9ccb..4a2d00501 100644 --- a/anchor/common/ssv_types/src/operator.rs +++ b/anchor/common/ssv_types/src/operator.rs @@ -3,6 +3,7 @@ use std::{cmp::Eq, fmt::Debug, hash::Hash}; use derive_more::{Deref, Display, From}; use openssl::{pkey::Public, rsa::Rsa}; use ssz_derive::{Decode, Encode}; +use tree_hash::{Hash256, PackedEncoding, TreeHash, TreeHashType}; use types::Address; /// Unique identifier for an Operator. @@ -25,6 +26,25 @@ use types::Address; #[ssz(struct_behaviour = "transparent")] #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] pub struct OperatorId(pub u64); +impl TreeHash for OperatorId { + fn tree_hash_type() -> TreeHashType { + TreeHashType::Basic + } + + fn tree_hash_packed_encoding(&self) -> PackedEncoding { + let value: u64 = self.0; + value.tree_hash_packed_encoding() + } + + fn tree_hash_packing_factor() -> usize { + u64::tree_hash_packing_factor() + } + + fn tree_hash_root(&self) -> Hash256 { + let value: u64 = self.0; + value.tree_hash_root() + } +} /// Client responsible for maintaining the overall health of the network. #[derive(Debug, Clone)] diff --git a/anchor/common/ssv_types/src/partial_sig.rs b/anchor/common/ssv_types/src/partial_sig.rs index 40448cf60..b82a01864 100644 --- a/anchor/common/ssv_types/src/partial_sig.rs +++ b/anchor/common/ssv_types/src/partial_sig.rs @@ -1,9 +1,18 @@ use ssz::{Decode, DecodeError, Encode}; use ssz_derive::{Decode, Encode}; -use types::{Hash256, Signature, Slot}; +use tree_hash::{PackedEncoding, TreeHash, TreeHashType}; +use tree_hash_derive::TreeHash; +use types::{ + Hash256, Signature, Slot, VariableList, + typenum::{Sum, U512, U1000}, +}; use crate::{OperatorId, ValidatorIndex}; +/// Maximum number of partial signature messages: 1512 +/// Calculated as 1000 + 512 = 1512 +pub type PartialSignatureMessagesLen = Sum; + #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] pub enum PartialSignatureKind { @@ -80,15 +89,35 @@ impl Decode for PartialSignatureKind { } } +impl TreeHash for PartialSignatureKind { + fn tree_hash_type() -> TreeHashType { + TreeHashType::Basic + } + + fn tree_hash_packed_encoding(&self) -> PackedEncoding { + let value = *self as u64; + value.tree_hash_packed_encoding() + } + + fn tree_hash_packing_factor() -> usize { + u64::tree_hash_packing_factor() + } + + fn tree_hash_root(&self) -> tree_hash::Hash256 { + let value = *self as u64; + value.tree_hash_root() + } +} + // A partial signature specific message -#[derive(Clone, Debug, Encode, Decode)] +#[derive(Clone, Debug, PartialEq, Encode, Decode, TreeHash)] pub struct PartialSignatureMessages { pub kind: PartialSignatureKind, pub slot: Slot, - pub messages: Vec, + pub messages: VariableList, } -#[derive(Clone, Debug, Encode, Decode)] +#[derive(Clone, Debug, PartialEq, Encode, Decode, TreeHash)] pub struct PartialSignatureMessage { pub partial_signature: Signature, pub signing_root: Hash256, diff --git a/anchor/common/ssv_types/src/test_utils.rs b/anchor/common/ssv_types/src/test_utils.rs new file mode 100644 index 000000000..777627eda --- /dev/null +++ b/anchor/common/ssv_types/src/test_utils.rs @@ -0,0 +1,42 @@ +//! Test utilities shared across the ssv_types crate + +use crate::{ + OperatorId, RSA_SIGNATURE_SIZE, + message::{MsgType, SSVMessage, SignedSSVMessage}, + msgid::MessageId, +}; + +const IDENTIFIER_SIZE: usize = 56; // same as MessageId length + +/// Returns a default 56-byte ID array with all zeros. +pub fn default_msg_id() -> MessageId { + [0u8; IDENTIFIER_SIZE].into() +} + +/// Returns a small, non-empty payload for SSVMessage data. +pub fn small_data() -> Vec { + vec![0x11, 0x22, 0x33] +} + +/// Returns a valid signature of exactly [`RSA_SIGNATURE_SIZE`] bytes. +pub fn valid_signature() -> [u8; RSA_SIGNATURE_SIZE] { + [0u8; RSA_SIGNATURE_SIZE] +} + +/// Creates a valid, non-empty SSVMessage (ensuring it doesn't exceed the max size). +pub fn valid_ssv_message() -> SSVMessage { + SSVMessage::new(MsgType::SSVConsensusMsgType, default_msg_id(), small_data()) + .expect("Creating a valid SSVMessage must succeed") +} + +/// Creates a single-signer, single-signature valid SignedSSVMessage. +pub fn valid_signed_ssv_message() -> SignedSSVMessage { + let msg = valid_ssv_message(); + SignedSSVMessage::new( + vec![valid_signature()], + vec![OperatorId(1)], + msg, + vec![0xAB, 0xCD], // "full_data" well under max + ) + .expect("Creating a valid SignedSSVMessage must succeed") +} diff --git a/anchor/message_sender/Cargo.toml b/anchor/message_sender/Cargo.toml index ea2409dd0..e41102145 100644 --- a/anchor/message_sender/Cargo.toml +++ b/anchor/message_sender/Cargo.toml @@ -16,5 +16,6 @@ processor = { workspace = true } slot_clock = { workspace = true } ssv_types = { workspace = true } subnet_service = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } diff --git a/anchor/message_sender/src/lib.rs b/anchor/message_sender/src/lib.rs index 70cb5e7bc..478f774f0 100644 --- a/anchor/message_sender/src/lib.rs +++ b/anchor/message_sender/src/lib.rs @@ -4,7 +4,9 @@ pub mod impostor; #[cfg(feature = "testing")] pub mod testing; +use openssl::error::ErrorStack; use ssv_types::{CommitteeId, consensus::UnsignedSSVMessage, message::SignedSSVMessage}; +use thiserror::Error as ThisError; pub use crate::network::*; @@ -27,3 +29,11 @@ pub enum Error { OwnOperatorIdUnknown, NotSynced, } + +#[derive(Debug, ThisError)] +enum SigningError { + #[error("Signing error: {0}")] + SignerError(#[from] ErrorStack), + #[error("Ciphertext has {0} bytes, expected 256")] + IncorrectCiphertextLength(usize), +} diff --git a/anchor/message_sender/src/network.rs b/anchor/message_sender/src/network.rs index a3b4c56cd..7603f0d15 100644 --- a/anchor/message_sender/src/network.rs +++ b/anchor/message_sender/src/network.rs @@ -3,20 +3,21 @@ use std::sync::Arc; use database::OwnOperatorId; use message_validator::{DutiesProvider, MessageAcceptance, Validator}; use openssl::{ - error::ErrorStack, hash::MessageDigest, pkey::{PKey, Private}, rsa::Rsa, sign::Signer, }; use slot_clock::SlotClock; -use ssv_types::{CommitteeId, consensus::UnsignedSSVMessage, message::SignedSSVMessage}; +use ssv_types::{ + CommitteeId, RSA_SIGNATURE_SIZE, consensus::UnsignedSSVMessage, message::SignedSSVMessage, +}; use ssz::Encode; use subnet_service::SubnetId; use tokio::sync::{mpsc, mpsc::error::TrySendError, watch}; use tracing::{debug, error, trace, warn}; -use crate::{Error, MessageCallback, MessageSender}; +use crate::{Error, MessageCallback, MessageSender, SigningError}; const SIGNER_NAME: &str = "message_sign_and_send"; const SENDER_NAME: &str = "message_send"; @@ -152,10 +153,15 @@ impl NetworkMessageSender { } } - fn sign(&self, message: &UnsignedSSVMessage) -> Result, ErrorStack> { + fn sign(&self, message: &UnsignedSSVMessage) -> Result<[u8; RSA_SIGNATURE_SIZE], SigningError> { let serialized = message.ssv_message.as_ssz_bytes(); let mut signer = Signer::new(MessageDigest::sha256(), &self.private_key)?; signer.update(&serialized)?; - signer.sign_to_vec() + let mut signature = [0u8; RSA_SIGNATURE_SIZE]; + let len = signer.sign(&mut signature)?; + if len != RSA_SIGNATURE_SIZE { + return Err(SigningError::IncorrectCiphertextLength(len)); + } + Ok(signature) } } diff --git a/anchor/message_sender/src/testing.rs b/anchor/message_sender/src/testing.rs index 1a5291c24..c4dfdd716 100644 --- a/anchor/message_sender/src/testing.rs +++ b/anchor/message_sender/src/testing.rs @@ -1,7 +1,6 @@ use ssv_types::{ - CommitteeId, OperatorId, - consensus::UnsignedSSVMessage, - message::{RSA_SIGNATURE_SIZE, SignedSSVMessage}, + CommitteeId, OperatorId, RSA_SIGNATURE_SIZE, consensus::UnsignedSSVMessage, + message::SignedSSVMessage, }; use tokio::sync::mpsc; @@ -20,7 +19,7 @@ impl MessageSender for MockMessageSender { additional_message_callback: Option>, ) -> Result<(), Error> { let message = SignedSSVMessage::new( - vec![vec![0u8; RSA_SIGNATURE_SIZE]], + vec![[0u8; RSA_SIGNATURE_SIZE]], vec![self.operator_id], message.ssv_message, message.full_data, diff --git a/anchor/message_validator/Cargo.toml b/anchor/message_validator/Cargo.toml index f418d5b9b..0d6b19eed 100644 --- a/anchor/message_validator/Cargo.toml +++ b/anchor/message_validator/Cargo.toml @@ -25,6 +25,7 @@ task_executor = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } +typenum = { workspace = true } types = { workspace = true } [dev-dependencies] diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 229de62d1..de5d8ce2c 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -10,6 +10,7 @@ use ssv_types::{ msgid::Role, }; use ssz::Decode; +use typenum::{U13, Unsigned}; use crate::{ FIRST_ROUND, ValidatedSSVMessage, ValidationContext, ValidationFailure, compute_quorum_size, @@ -169,26 +170,49 @@ pub(crate) fn validate_justifications( return Err(ValidationFailure::UnexpectedRoundChangeJustifications); } - prepare_justifications - .iter() - .chain(round_change_justifications.iter()) - .try_for_each(|signed_message| { - verify_message_signatures(signed_message, operator_pub_keys)?; - // Also check the justifications' justifications - if check_inner_justifications { - validate_justifications( - &QbftMessage::from_ssz_bytes(signed_message.ssv_message().data()) - .map_err(|_| ValidationFailure::MalformedJustifications)?, - operator_pub_keys, - false, - )?; - } - Ok(()) - })?; + // Validate prepare justifications + validate_justification_list( + prepare_justifications, + operator_pub_keys, + check_inner_justifications, + )?; + + // Validate round change justifications + validate_justification_list( + round_change_justifications, + operator_pub_keys, + check_inner_justifications, + )?; Ok(()) } +/// Helper function to validate a list of justifications with generic length parameter +fn validate_justification_list( + justifications: &VariableList, U13>, + operator_pub_keys: &HashMap>, + check_inner_justifications: bool, +) -> Result<(), ValidationFailure> { + justifications.iter().try_for_each(|signed_message_bytes| { + // Parse the SignedSSVMessage from bytes + let signed_message = SignedSSVMessage::from_ssz_bytes(signed_message_bytes) + .map_err(|_| ValidationFailure::MalformedJustifications)?; + + verify_message_signatures(&signed_message, operator_pub_keys)?; + + // Also check the justifications' justifications + if check_inner_justifications { + validate_justifications( + &QbftMessage::from_ssz_bytes(signed_message.ssv_message().data()) + .map_err(|_| ValidationFailure::MalformedJustifications)?, + operator_pub_keys, + false, + )?; + } + Ok(()) + }) +} + #[allow(clippy::comparison_chain)] pub(crate) fn validate_qbft_logic( validation_context: &ValidationContext, @@ -435,10 +459,10 @@ mod tests { use bls::{Hash256, PublicKeyBytes}; use openssl::hash::MessageDigest; use ssv_types::{ - OperatorId, + OperatorId, RSA_SIGNATURE_SIZE, VariableList, consensus::{QbftMessage, QbftMessageType}, domain_type::DomainType, - message::{MsgType, RSA_SIGNATURE_SIZE, SSVMessage, SignedSSVMessage}, + message::{MsgType, SSVMessage, SignedSSVMessage}, msgid::{DutyExecutor, MessageId, Role}, }; use ssz::Encode; @@ -667,7 +691,7 @@ mod tests { let ssv_msg = SSVMessage::new(MsgType::SSVConsensusMsgType, msg_id, invalid_data) .expect("SSVMessage should be created"); let signed_msg = SignedSSVMessage::new( - vec![vec![0xAA; RSA_SIGNATURE_SIZE]], + vec![[0xAA; RSA_SIGNATURE_SIZE]], vec![OperatorId(1)], ssv_msg, vec![], @@ -875,15 +899,15 @@ mod tests { identifier: (&msg_id_b).into(), // Mismatched ID root: Hash256::from([0u8; 32]), data_round: 1, - round_change_justification: vec![], - prepare_justification: vec![], + round_change_justification: VariableList::empty(), + prepare_justification: VariableList::empty(), }; let qbft_bytes = qbft_msg.as_ssz_bytes(); let ssv_msg = SSVMessage::new(MsgType::SSVConsensusMsgType, msg_id_a, qbft_bytes) .expect("SSVMessage should be created"); let signed_msg = SignedSSVMessage::new( - vec![vec![0xAA; RSA_SIGNATURE_SIZE]], + vec![[0xAA; RSA_SIGNATURE_SIZE]], vec![OperatorId(42)], ssv_msg, vec![], @@ -922,7 +946,7 @@ mod tests { let ssv_msg = SSVMessage::new(MsgType::SSVConsensusMsgType, msg_id, qbft_bytes) .expect("SSVMessage should be created"); let signed_msg = SignedSSVMessage::new( - vec![vec![0xAA; RSA_SIGNATURE_SIZE]], + vec![[0xAA; RSA_SIGNATURE_SIZE]], vec![OperatorId(1)], ssv_msg, vec![], @@ -1191,11 +1215,13 @@ mod tests { // Pad signature to RSA_SIGNATURE_SIZE if needed let padded_signature = if signature.len() < RSA_SIGNATURE_SIZE { - let mut padded = vec![0; RSA_SIGNATURE_SIZE]; + let mut padded = [0; RSA_SIGNATURE_SIZE]; padded[..signature.len()].copy_from_slice(&signature); padded } else { signature + .try_into() + .expect("Signature should not be longer than RSA_SIGNATURE_SIZE bytes") }; // Create signed message @@ -1261,7 +1287,7 @@ mod tests { .expect("SSVMessage should be created"); // Create an invalid signature (just random bytes) - let invalid_signature = vec![0xBB; RSA_SIGNATURE_SIZE]; + let invalid_signature = [0xBB; RSA_SIGNATURE_SIZE]; // Create signed message with invalid signature let signed_msg = SignedSSVMessage::new( @@ -1344,7 +1370,7 @@ mod tests { // Create a signed SSV message let signed_msg = SignedSSVMessage::new( - vec![vec![0xAA; RSA_SIGNATURE_SIZE]], + vec![[0xAA; RSA_SIGNATURE_SIZE]], vec![OperatorId(1)], ssv_msg, vec![], diff --git a/anchor/message_validator/src/duty_state.rs b/anchor/message_validator/src/duty_state.rs index 7b793703f..971ea2147 100644 --- a/anchor/message_validator/src/duty_state.rs +++ b/anchor/message_validator/src/duty_state.rs @@ -313,7 +313,7 @@ impl SignerState { if signed_ssv_message.operator_ids().len() > 1 { self.seen_signers - .insert(signed_ssv_message.operator_ids().as_slice().into()); + .insert(signed_ssv_message.operator_ids().into()); } self.message_counts.record_consensus_message( diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index 38f83c42e..362300aa7 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -808,10 +808,11 @@ mod tests { sign::Signer, }; use ssv_types::{ - CommitteeId, CommitteeInfo, IndexSet, OperatorId, ValidatorIndex, + CommitteeId, CommitteeInfo, IndexSet, OperatorId, RSA_SIGNATURE_SIZE, ValidatorIndex, + VariableList, consensus::{QbftMessage, QbftMessageType}, domain_type::DomainType, - message::{MsgType, RSA_SIGNATURE_SIZE, SSVMessage, SignedSSVMessage}, + message::{MsgType, SSVMessage, SignedSSVMessage}, msgid::{DutyExecutor, MessageId, Role}, }; use ssz::Encode; @@ -871,6 +872,31 @@ mod tests { } pub(crate) fn build(self) -> QbftMessage { + // This is a test builder, so using expect() is acceptable here + // Convert Vec to VariableList, U13> + let round_change_justification_vec: Vec<_> = self + .round_change_justification + .into_iter() + .map(|msg| msg.without_full_data()) + .map(|msg| { + let bytes = msg.as_ssz_bytes(); + VariableList::new(bytes).unwrap() // Test data should fit + }) + .collect(); + let round_change_justification = + VariableList::new(round_change_justification_vec).unwrap(); // Test data should fit + + let prepare_justification_vec: Vec<_> = self + .prepare_justification + .into_iter() + .map(|msg| msg.without_full_data()) + .map(|msg| { + let bytes = msg.as_ssz_bytes(); + VariableList::new(bytes).unwrap() // Test data should fit + }) + .collect(); + let prepare_justification = VariableList::new(prepare_justification_vec).unwrap(); // Test data should fit + QbftMessage { qbft_message_type: self.msg_type, height: 1, @@ -878,8 +904,8 @@ mod tests { identifier: (&self.identifier).into(), root: Hash256::from([0u8; 32]), data_round: 1, - round_change_justification: self.round_change_justification, - prepare_justification: self.prepare_justification, + round_change_justification, + prepare_justification, } } } @@ -914,7 +940,7 @@ mod tests { signers .iter() .enumerate() - .map(|(i, _)| vec![0xAA + i as u8; RSA_SIGNATURE_SIZE]) + .map(|(i, _)| [0xAA + i as u8; RSA_SIGNATURE_SIZE]) .collect::>() } else { pks.iter() @@ -922,7 +948,11 @@ mod tests { let p_key = PKey::from_rsa(pk.clone()).unwrap(); let mut signer = Signer::new(MessageDigest::sha256(), &p_key).unwrap(); signer.update(&ssv_msg.as_ssz_bytes()).unwrap(); - signer.sign_to_vec().expect("Failed to sign message") + signer + .sign_to_vec() + .expect("Failed to sign message") + .try_into() + .expect("Signature should be 256 bytes") }) .collect::>() }; diff --git a/anchor/message_validator/src/partial_signature.rs b/anchor/message_validator/src/partial_signature.rs index 2bbff6980..b50b6384a 100644 --- a/anchor/message_validator/src/partial_signature.rs +++ b/anchor/message_validator/src/partial_signature.rs @@ -281,8 +281,8 @@ mod tests { }; use slot_clock::{ManualSlotClock, SlotClock}; use ssv_types::{ - OperatorId, ValidatorIndex, - message::{MsgType, RSA_SIGNATURE_SIZE, SSVMessage, SignedSSVMessage}, + OperatorId, RSA_SIGNATURE_SIZE, ValidatorIndex, + message::{MsgType, SSVMessage, SignedSSVMessage}, partial_sig::PartialSignatureMessage, }; use ssz::Encode; @@ -327,7 +327,7 @@ mod tests { let partial_sig_messages = PartialSignatureMessages { kind, slot: Slot::new(0), - messages, + messages: messages.into(), }; let msg_id = create_message_id_for_test(role); @@ -345,9 +345,15 @@ mod tests { let p_key = PKey::from_rsa(pk.clone()).unwrap(); let mut signer = Signer::new(MessageDigest::sha256(), &p_key).unwrap(); signer.update(&ssv_msg.as_ssz_bytes()).unwrap(); - vec![signer.sign_to_vec().expect("Failed to sign message")] + vec![ + signer + .sign_to_vec() + .expect("Failed to sign message") + .try_into() + .expect("Signature should be 256 bytes"), + ] } else { - vec![vec![0xAA; RSA_SIGNATURE_SIZE]] + vec![[0xAA; RSA_SIGNATURE_SIZE]] }; let signed_msg = SignedSSVMessage::new(signature, vec![signer], ssv_msg, full_data) @@ -444,10 +450,7 @@ mod tests { // Multiple signers - this should fail let signers = vec![OperatorId(1), OperatorId(2)]; - let signatures = vec![ - vec![0xAA; RSA_SIGNATURE_SIZE], - vec![0xBB; RSA_SIGNATURE_SIZE], - ]; + let signatures = vec![[0xAA; RSA_SIGNATURE_SIZE], [0xBB; RSA_SIGNATURE_SIZE]]; let signed_msg = SignedSSVMessage::new(signatures, signers, ssv_msg, vec![]) .expect("SignedSSVMessage should be created"); @@ -735,7 +738,7 @@ mod tests { let partial_sig_messages = PartialSignatureMessages { kind: PartialSignatureKind::PostConsensus, slot: Slot::new(0), - messages, + messages: messages.into(), }; let msg_id = create_message_id_for_test(Role::Proposer); // Not committee role @@ -744,7 +747,7 @@ mod tests { .expect("SSVMessage should be created"); let signed_msg = SignedSSVMessage::new( - vec![vec![0xAA; RSA_SIGNATURE_SIZE]], + vec![[0xAA; RSA_SIGNATURE_SIZE]], vec![OperatorId(1)], ssv_msg, vec![], @@ -787,7 +790,7 @@ mod tests { let partial_sig_messages = PartialSignatureMessages { kind: PartialSignatureKind::PostConsensus, slot: Slot::new(0), - messages, + messages: messages.into(), }; let msg_id = create_message_id_for_test(Role::Committee); @@ -796,7 +799,7 @@ mod tests { .expect("SSVMessage should be created"); let signed_msg = SignedSSVMessage::new( - vec![vec![0xAA; RSA_SIGNATURE_SIZE]], + vec![[0xAA; RSA_SIGNATURE_SIZE]], vec![OperatorId(1)], ssv_msg, vec![], diff --git a/anchor/network/Cargo.toml b/anchor/network/Cargo.toml index 13b32a15d..0a9381c48 100644 --- a/anchor/network/Cargo.toml +++ b/anchor/network/Cargo.toml @@ -36,7 +36,7 @@ rand = { workspace = true } serde = { workspace = true } serde_json = "1.0.137" ssv_types = { workspace = true } -ssz_types = "0.11.0" +ssz_types = { workspace = true } subnet_service = { workspace = true } task_executor = { workspace = true } thiserror = { workspace = true } diff --git a/anchor/signature_collector/src/lib.rs b/anchor/signature_collector/src/lib.rs index d6a5b5c0e..1ea21fa60 100644 --- a/anchor/signature_collector/src/lib.rs +++ b/anchor/signature_collector/src/lib.rs @@ -246,7 +246,7 @@ impl SignatureCollectorManager { let partial_sig_messages = PartialSignatureMessages { kind: metadata.kind, slot: metadata.slot, - messages: signatures, + messages: signatures.into(), }; UnsignedSSVMessage { diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index 6bc829168..e5f839ac1 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -39,6 +39,7 @@ use ssv_types::{ }, msgid::Role, partial_sig::PartialSignatureKind, + try_to_variable_list, }; use ssz::{Decode, DecodeError, Encode}; use tokio::{ @@ -334,7 +335,12 @@ impl AnchorValidatorStore { let consensus_data = ValidatorConsensusData { duty: validator_duty, version: block_version, - data_ssz: signable_block.as_ssz_bytes(), + data_ssz: try_to_variable_list(signable_block.as_ssz_bytes(), |provided, max| { + Error::SpecificError(SpecificError::DataTooLarge(format!( + "Block data too large for consensus: {} > {}", + provided, max + ))) + })?, }; let data_validator = self.create_validator_consensus_data_validator(validator.public_key); @@ -747,6 +753,7 @@ pub enum SpecificError { cluster_id: ClusterId, }, KeyShareDecryptionFailed, + DataTooLarge(String), ClusterLiquidated, } @@ -1175,7 +1182,12 @@ impl ValidatorStore for AnchorValidatorStore { validator_sync_committee_indices: Default::default(), }, version, - data_ssz: message.as_ssz_bytes(), + data_ssz: try_to_variable_list(message.as_ssz_bytes(), |provided, max| { + Error::SpecificError(SpecificError::DataTooLarge(format!( + "Attestation data too large for consensus: {} > {}", + provided, max + ))) + })?, }, self.create_validator_consensus_data_validator(validator_pubkey), start_time, @@ -1476,7 +1488,12 @@ impl ValidatorStore for AnchorValidatorStore { validator_sync_committee_indices: Default::default(), }, version: ForkName::Altair.into(), - data_ssz: data.as_ssz_bytes(), + data_ssz: try_to_variable_list(data.as_ssz_bytes(), |provided, max| { + Error::SpecificError(SpecificError::DataTooLarge(format!( + "Sync committee data too large for consensus: {} > {}", + provided, max + ))) + })?, }, self.create_validator_consensus_data_validator(aggregator_pubkey), start_time,