diff --git a/Cargo.lock b/Cargo.lock index 1f289e9d17..4ba8707ebb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3122,7 +3122,6 @@ dependencies = [ "snarkvm-ledger-narwhal-batch-certificate", "snarkvm-ledger-narwhal-batch-header", "snarkvm-ledger-narwhal-transmission-id", - "time", ] [[package]] diff --git a/ledger/narwhal/batch-certificate/Cargo.toml b/ledger/narwhal/batch-certificate/Cargo.toml index 56c613d919..7d1650cfb6 100644 --- a/ledger/narwhal/batch-certificate/Cargo.toml +++ b/ledger/narwhal/batch-certificate/Cargo.toml @@ -27,7 +27,7 @@ edition = "2021" default = [ ] serial = [ "console/serial" ] wasm = [ "console/wasm" ] -test-helpers = [ "narwhal-batch-header/test-helpers", "time" ] +test-helpers = [ "narwhal-batch-header/test-helpers" ] [dependencies.console] package = "snarkvm-console" @@ -52,10 +52,6 @@ features = [ "serde" ] version = "1.0" features = [ "preserve_order" ] -[dependencies.time] -version = "0.3" -optional = true - [dev-dependencies.bincode] version = "1.3" diff --git a/ledger/narwhal/batch-certificate/src/bytes.rs b/ledger/narwhal/batch-certificate/src/bytes.rs index d7dbd25dc2..b2638fa557 100644 --- a/ledger/narwhal/batch-certificate/src/bytes.rs +++ b/ledger/narwhal/batch-certificate/src/bytes.rs @@ -20,48 +20,84 @@ impl FromBytes for BatchCertificate { // Read the version. let version = u8::read_le(&mut reader)?; // Ensure the version is valid. - if version != 1 { - return Err(error("Invalid batch version")); + if version != 1 && version != 2 { + return Err(error("Invalid batch certificate version")); } - // Read the certificate ID. - let certificate_id = Field::read_le(&mut reader)?; - // Read the batch header. - let batch_header = BatchHeader::read_le(&mut reader)?; - // Read the number of signatures. - let num_signatures = u32::read_le(&mut reader)?; - // Read the signatures. - let mut signatures = IndexMap::with_capacity(num_signatures as usize); - for _ in 0..num_signatures { - // Read the signature. - let signature = Signature::read_le(&mut reader)?; - // Read the timestamp. - let timestamp = i64::read_le(&mut reader)?; - // Insert the signature and timestamp. - signatures.insert(signature, timestamp); + if version == 1 { + // Read the certificate ID. + let certificate_id = Field::read_le(&mut reader)?; + // Read the batch header. + let batch_header = BatchHeader::read_le(&mut reader)?; + // Read the number of signatures. + let num_signatures = u32::read_le(&mut reader)?; + // Read the signatures. + let mut signatures = IndexMap::with_capacity(num_signatures as usize); + for _ in 0..num_signatures { + // Read the signature. + let signature = Signature::read_le(&mut reader)?; + // Read the timestamp. + let timestamp = i64::read_le(&mut reader)?; + // Insert the signature and timestamp. + signatures.insert(signature, timestamp); + } + // Return the batch certificate. + Self::from_v1_deprecated(certificate_id, batch_header, signatures).map_err(error) + } else if version == 2 { + // Read the batch header. + let batch_header = BatchHeader::read_le(&mut reader)?; + // Read the number of signatures. + let num_signatures = u16::read_le(&mut reader)?; + // Read the signatures. + let mut signatures = IndexSet::with_capacity(num_signatures as usize); + for _ in 0..num_signatures { + // Read the signature. + let signature = Signature::read_le(&mut reader)?; + // Insert the signature. + signatures.insert(signature); + } + // Return the batch certificate. + Self::from(batch_header, signatures).map_err(error) + } else { + unreachable!("Invalid batch certificate version") } - // Return the batch certificate. - Self::from(certificate_id, batch_header, signatures).map_err(|e| error(e.to_string())) } } impl ToBytes for BatchCertificate { /// Writes the batch certificate to the buffer. fn write_le(&self, mut writer: W) -> IoResult<()> { - // Write the version. - 1u8.write_le(&mut writer)?; - // Write the certificate ID. - self.certificate_id.write_le(&mut writer)?; - // Write the batch header. - self.batch_header.write_le(&mut writer)?; - // Write the number of signatures. - u32::try_from(self.signatures.len()).map_err(|e| error(e.to_string()))?.write_le(&mut writer)?; - // Write the signatures. - for (signature, timestamp) in &self.signatures { - // Write the signature. - signature.write_le(&mut writer)?; - // Write the timestamp. - timestamp.write_le(&mut writer)?; + match self { + Self::V1 { certificate_id, batch_header, signatures } => { + // Write the version. + 1u8.write_le(&mut writer)?; + // Write the certificate ID. + certificate_id.write_le(&mut writer)?; + // Write the batch header. + batch_header.write_le(&mut writer)?; + // Write the number of signatures. + u32::try_from(signatures.len()).map_err(error)?.write_le(&mut writer)?; + // Write the signatures. + for (signature, timestamp) in signatures.iter() { + // Write the signature. + signature.write_le(&mut writer)?; + // Write the timestamp. + timestamp.write_le(&mut writer)?; + } + } + Self::V2 { batch_header, signatures } => { + // Write the version. + 2u8.write_le(&mut writer)?; + // Write the batch header. + batch_header.write_le(&mut writer)?; + // Write the number of signatures. + u16::try_from(signatures.len()).map_err(error)?.write_le(&mut writer)?; + // Write the signatures. + for signature in signatures.iter() { + // Write the signature. + signature.write_le(&mut writer)?; + } + } } Ok(()) } diff --git a/ledger/narwhal/batch-certificate/src/lib.rs b/ledger/narwhal/batch-certificate/src/lib.rs index a71f640ce0..bfdc385016 100644 --- a/ledger/narwhal/batch-certificate/src/lib.rs +++ b/ledger/narwhal/batch-certificate/src/lib.rs @@ -30,33 +30,53 @@ use narwhal_transmission_id::TransmissionID; use core::hash::{Hash, Hasher}; use indexmap::{IndexMap, IndexSet}; -#[derive(Clone, PartialEq, Eq)] -pub struct BatchCertificate { - /// The certificate ID. - certificate_id: Field, - /// The batch header. - batch_header: BatchHeader, - /// The `(signature, timestamp)` pairs for the batch ID from the committee. - signatures: IndexMap, i64>, +#[derive(Clone)] +pub enum BatchCertificate { + // TODO (howardwu): For mainnet - Delete V1 and switch everyone to V2 as the default. + V1 { + /// The certificate ID. + certificate_id: Field, + /// The batch header. + batch_header: BatchHeader, + /// The `(signature, timestamp)` pairs for the batch ID from the committee. + signatures: IndexMap, i64>, + }, + V2 { + /// The batch header. + batch_header: BatchHeader, + /// The signatures for the batch ID from the committee. + signatures: IndexSet>, + }, } impl BatchCertificate { - /// Initializes a new batch certificate. - pub fn new(batch_header: BatchHeader, signatures: IndexMap, i64>) -> Result { - // Compute the certificate ID. - let certificate_id = Self::compute_certificate_id(batch_header.batch_id(), &signatures)?; - // Return the batch certificate. - Self::from(certificate_id, batch_header, signatures) - } - - /// Initializes a new batch certificate. - pub fn from( + // TODO (howardwu): For mainnet - Delete V1 and switch everyone to V2 as the default. + /// Initializes a (deprecated) V1 batch certificate. + pub fn from_v1_deprecated( certificate_id: Field, batch_header: BatchHeader, signatures: IndexMap, i64>, ) -> Result { + /// Returns the certificate ID. + fn compute_certificate_id( + batch_id: Field, + signatures: &IndexMap, i64>, + ) -> Result> { + let mut preimage = Vec::new(); + // Insert the batch ID. + batch_id.write_le(&mut preimage)?; + // Insert the signatures. + for (signature, timestamp) in signatures { + // Insert the signature. + signature.write_le(&mut preimage)?; + // Insert the timestamp. + timestamp.write_le(&mut preimage)?; + } + // Hash the preimage. + N::hash_bhp1024(&preimage.to_bits_le()) + } // Compute the certificate ID. - if certificate_id != Self::compute_certificate_id(batch_header.batch_id(), &signatures)? { + if certificate_id != compute_certificate_id(batch_header.batch_id(), &signatures)? { bail!("Invalid batch certificate ID") } // Verify the signatures are valid. @@ -66,102 +86,118 @@ impl BatchCertificate { bail!("Invalid batch certificate signature") } } + // Return the V1 batch certificate. + Ok(Self::V1 { certificate_id, batch_header, signatures }) + } + + /// Initializes a new batch certificate. + pub fn from(batch_header: BatchHeader, signatures: IndexSet>) -> Result { + // Verify the signatures are valid. + for signature in &signatures { + if !signature.verify(&signature.to_address(), &[batch_header.batch_id()]) { + bail!("Invalid batch certificate signature") + } + } // Return the batch certificate. - Self::from_unchecked(certificate_id, batch_header, signatures) + Self::from_unchecked(batch_header, signatures) } /// Initializes a new batch certificate. - pub fn from_unchecked( - certificate_id: Field, - batch_header: BatchHeader, - signatures: IndexMap, i64>, - ) -> Result { + pub fn from_unchecked(batch_header: BatchHeader, signatures: IndexSet>) -> Result { // Ensure the signatures are not empty. ensure!(!signatures.is_empty(), "Batch certificate must contain signatures"); // Return the batch certificate. - Ok(Self { certificate_id, batch_header, signatures }) + Ok(Self::V2 { batch_header, signatures }) + } +} + +impl PartialEq for BatchCertificate { + fn eq(&self, other: &Self) -> bool { + self.batch_id() == other.batch_id() + } +} + +impl Eq for BatchCertificate {} + +impl Hash for BatchCertificate { + fn hash(&self, state: &mut H) { + match self { + Self::V1 { batch_header, signatures, .. } => { + batch_header.batch_id().hash(state); + (signatures.len() as u64).hash(state); + for signature in signatures.iter() { + signature.hash(state); + } + } + Self::V2 { batch_header, .. } => { + batch_header.batch_id().hash(state); + } + } } } impl BatchCertificate { /// Returns the certificate ID. - pub const fn certificate_id(&self) -> Field { - self.certificate_id + pub const fn id(&self) -> Field { + match self { + Self::V1 { certificate_id, .. } => *certificate_id, + Self::V2 { batch_header, .. } => batch_header.batch_id(), + } } /// Returns the batch header. pub const fn batch_header(&self) -> &BatchHeader { - &self.batch_header + match self { + Self::V1 { batch_header, .. } => batch_header, + Self::V2 { batch_header, .. } => batch_header, + } } /// Returns the batch ID. pub const fn batch_id(&self) -> Field { - self.batch_header.batch_id() + self.batch_header().batch_id() } /// Returns the author. pub const fn author(&self) -> Address { - self.batch_header.author() + self.batch_header().author() } /// Returns the round. pub const fn round(&self) -> u64 { - self.batch_header.round() + self.batch_header().round() } /// Returns the transmission IDs. pub const fn transmission_ids(&self) -> &IndexSet> { - self.batch_header.transmission_ids() + self.batch_header().transmission_ids() } /// Returns the batch certificate IDs for the previous round. pub const fn previous_certificate_ids(&self) -> &IndexSet> { - self.batch_header.previous_certificate_ids() - } - - /// Returns the median timestamp of the batch ID from the committee. - pub fn median_timestamp(&self) -> i64 { - let mut timestamps = self.timestamps().chain([self.batch_header.timestamp()].into_iter()).collect::>(); - timestamps.sort_unstable(); - timestamps[timestamps.len() / 2] + self.batch_header().previous_certificate_ids() } - /// Returns the timestamps of the batch ID from the committee. - pub fn timestamps(&self) -> impl '_ + ExactSizeIterator { - self.signatures.values().copied() - } - - /// Returns the signatures of the batch ID from the committee. - pub fn signatures(&self) -> impl ExactSizeIterator> { - self.signatures.keys() - } -} - -impl Hash for BatchCertificate { - fn hash(&self, state: &mut H) { - self.batch_header.batch_id().hash(state); - (self.signatures.len() as u64).hash(state); - for signature in &self.signatures { - signature.hash(state); + /// Returns the timestamp of the batch header. + pub fn timestamp(&self) -> i64 { + match self { + Self::V1 { batch_header, signatures, .. } => { + // Return the median timestamp. + let mut timestamps = + signatures.values().copied().chain([batch_header.timestamp()].into_iter()).collect::>(); + timestamps.sort_unstable(); + timestamps[timestamps.len() / 2] + } + Self::V2 { batch_header, .. } => batch_header.timestamp(), } } -} -impl BatchCertificate { - /// Returns the certificate ID. - pub fn compute_certificate_id(batch_id: Field, signatures: &IndexMap, i64>) -> Result> { - let mut preimage = Vec::new(); - // Insert the batch ID. - batch_id.write_le(&mut preimage)?; - // Insert the signatures. - for (signature, timestamp) in signatures { - // Insert the signature. - signature.write_le(&mut preimage)?; - // Insert the timestamp. - timestamp.write_le(&mut preimage)?; + /// Returns the signatures of the batch ID from the committee. + pub fn signatures(&self) -> Box>> { + match self { + Self::V1 { signatures, .. } => Box::new(signatures.keys()), + Self::V2 { signatures, .. } => Box::new(signatures.iter()), } - // Hash the preimage. - N::hash_bhp1024(&preimage.to_bits_le()) } } @@ -201,15 +237,13 @@ pub mod test_helpers { rng, ); // Sample a list of signatures. - let mut signatures = IndexMap::with_capacity(5); + let mut signatures = IndexSet::with_capacity(5); for _ in 0..5 { let private_key = PrivateKey::new(rng).unwrap(); - let timestamp = time::OffsetDateTime::now_utc().unix_timestamp(); - let timestamp_field = Field::from_u64(timestamp as u64); - signatures.insert(private_key.sign(&[batch_header.batch_id(), timestamp_field], rng).unwrap(), timestamp); + signatures.insert(private_key.sign(&[batch_header.batch_id()], rng).unwrap()); } // Return the batch certificate. - BatchCertificate::new(batch_header, signatures).unwrap() + BatchCertificate::from(batch_header, signatures).unwrap() } /// Returns a list of sample batch certificates, sampled at random. @@ -245,7 +279,7 @@ pub mod test_helpers { sample_batch_certificate_for_round(previous_round, rng), ]; // Construct the previous certificate IDs. - let previous_certificate_ids: IndexSet<_> = previous_certificates.iter().map(|c| c.certificate_id()).collect(); + let previous_certificate_ids: IndexSet<_> = previous_certificates.iter().map(|c| c.id()).collect(); // Sample the leader certificate. let certificate = sample_batch_certificate_for_round_with_previous_certificate_ids( current_round, diff --git a/ledger/narwhal/batch-certificate/src/serialize.rs b/ledger/narwhal/batch-certificate/src/serialize.rs index 9035b4330b..1e06e50823 100644 --- a/ledger/narwhal/batch-certificate/src/serialize.rs +++ b/ledger/narwhal/batch-certificate/src/serialize.rs @@ -18,13 +18,21 @@ impl Serialize for BatchCertificate { /// Serializes the batch certificate to a JSON-string or buffer. fn serialize(&self, serializer: S) -> Result { match serializer.is_human_readable() { - true => { - let mut certificate = serializer.serialize_struct("BatchCertificate", 3)?; - certificate.serialize_field("certificate_id", &self.certificate_id)?; - certificate.serialize_field("batch_header", &self.batch_header)?; - certificate.serialize_field("signatures", &self.signatures)?; - certificate.end() - } + true => match self { + Self::V1 { certificate_id, batch_header, signatures } => { + let mut state = serializer.serialize_struct("BatchCertificate", 3)?; + state.serialize_field("certificate_id", certificate_id)?; + state.serialize_field("batch_header", batch_header)?; + state.serialize_field("signatures", signatures)?; + state.end() + } + Self::V2 { batch_header, signatures } => { + let mut state = serializer.serialize_struct("BatchCertificate", 2)?; + state.serialize_field("batch_header", batch_header)?; + state.serialize_field("signatures", signatures)?; + state.end() + } + }, false => ToBytesSerializer::serialize_with_size_encoding(self, serializer), } } @@ -36,12 +44,28 @@ impl<'de, N: Network> Deserialize<'de> for BatchCertificate { match deserializer.is_human_readable() { true => { let mut value = serde_json::Value::deserialize(deserializer)?; - Ok(Self::from( - DeserializeExt::take_from_value::(&mut value, "certificate_id")?, - DeserializeExt::take_from_value::(&mut value, "batch_header")?, - DeserializeExt::take_from_value::(&mut value, "signatures")?, - ) - .map_err(de::Error::custom)?) + + // Check if a certificate ID field is present. + let certificate_id = match value.get("certificate_id") { + Some(..) => Some(DeserializeExt::take_from_value::(&mut value, "certificate_id")?), + None => None, + }; + + // Parse for V1 and V2 batch certificates. + if let Some(certificate_id) = certificate_id { + Self::from_v1_deprecated( + certificate_id, + DeserializeExt::take_from_value::(&mut value, "batch_header")?, + DeserializeExt::take_from_value::(&mut value, "signatures")?, + ) + .map_err(de::Error::custom) + } else { + Self::from( + DeserializeExt::take_from_value::(&mut value, "batch_header")?, + DeserializeExt::take_from_value::(&mut value, "signatures")?, + ) + .map_err(de::Error::custom) + } } false => FromBytesDeserializer::::deserialize_with_size_encoding(deserializer, "batch certificate"), } diff --git a/ledger/narwhal/subdag/src/lib.rs b/ledger/narwhal/subdag/src/lib.rs index a5f46e2914..beafc19639 100644 --- a/ledger/narwhal/subdag/src/lib.rs +++ b/ledger/narwhal/subdag/src/lib.rs @@ -57,14 +57,15 @@ fn sanity_check_subdag_with_dfs(subdag: &BTreeMap Subdag { /// Returns the certificate IDs of the subdag (from earliest round to latest round). pub fn certificate_ids(&self) -> impl Iterator> + '_ { - self.values().flatten().map(BatchCertificate::certificate_id) + self.values().flatten().map(BatchCertificate::id) } /// Returns the leader certificate. @@ -134,10 +135,22 @@ impl Subdag { self.values().flatten().flat_map(BatchCertificate::transmission_ids) } - /// Returns the timestamp of the anchor round, defined as the median timestamp of the leader certificate. + /// Returns the timestamp of the anchor round, defined as the median timestamp of the subdag. pub fn timestamp(&self) -> i64 { - // Retrieve the median timestamp from the leader certificate. - self.leader_certificate().median_timestamp() + match self.leader_certificate() { + BatchCertificate::V1 { .. } => self.leader_certificate().timestamp(), + BatchCertificate::V2 { .. } => { + // Retrieve the timestamps of the certificates. + let mut timestamps = self.values().flatten().map(BatchCertificate::timestamp).collect::>(); + // Sort the timestamps. + #[cfg(not(feature = "serial"))] + timestamps.par_sort_unstable(); + #[cfg(feature = "serial")] + timestamps.sort_unstable(); + // Return the median timestamp. + timestamps[timestamps.len() / 2] + } + } } /// Returns the subdag root of the transactions. @@ -145,10 +158,7 @@ impl Subdag { // Prepare the leaves. let leaves = cfg_iter!(self.subdag) .map(|(_, certificates)| { - certificates - .iter() - .flat_map(|certificate| certificate.certificate_id().to_bits_le()) - .collect::>() + certificates.iter().flat_map(|certificate| certificate.id().to_bits_le()).collect::>() }) .collect::>(); @@ -201,7 +211,7 @@ pub mod test_helpers { for _ in 0..AVAILABILITY_THRESHOLD { let certificate = narwhal_batch_certificate::test_helpers::sample_batch_certificate_for_round(starting_round, rng); - previous_certificate_ids.insert(certificate.certificate_id()); + previous_certificate_ids.insert(certificate.id()); subdag.entry(starting_round).or_default().insert(certificate); } @@ -210,7 +220,7 @@ pub mod test_helpers { for _ in 0..QUORUM_THRESHOLD { let certificate = narwhal_batch_certificate::test_helpers::sample_batch_certificate_for_round_with_previous_certificate_ids(starting_round + 1, previous_certificate_ids.clone(), rng); - previous_certificate_ids_2.insert(certificate.certificate_id()); + previous_certificate_ids_2.insert(certificate.id()); subdag.entry(starting_round + 1).or_default().insert(certificate); } diff --git a/ledger/store/src/block/mod.rs b/ledger/store/src/block/mod.rs index eeee7c684d..2f6fcc1e9d 100644 --- a/ledger/store/src/block/mod.rs +++ b/ledger/store/src/block/mod.rs @@ -405,10 +405,9 @@ pub trait BlockStorage: 'static + Clone + Send + Sync { // Retrieve the certificate IDs to store. let certificates_to_store = match block.authority() { Authority::Beacon(_) => Vec::new(), - Authority::Quorum(subdag) => subdag - .iter() - .flat_map(|(round, certificates)| certificates.iter().map(|c| (c.certificate_id(), *round))) - .collect(), + Authority::Quorum(subdag) => { + subdag.iter().flat_map(|(round, certificates)| certificates.iter().map(|c| (c.id(), *round))).collect() + } }; // Prepare the rejected transaction IDs and their corresponding unconfirmed transaction IDs. @@ -529,7 +528,7 @@ pub trait BlockStorage: 'static + Clone + Send + Sync { Some(authority) => match authority { Cow::Owned(Authority::Beacon(_)) | Cow::Borrowed(Authority::Beacon(_)) => Vec::new(), Cow::Owned(Authority::Quorum(ref subdag)) | Cow::Borrowed(Authority::Quorum(ref subdag)) => { - subdag.values().flatten().map(|c| c.certificate_id()).collect() + subdag.values().flatten().map(|c| c.id()).collect() } }, None => bail!("Failed to remove block: missing authority for block '{block_height}' ('{block_hash}')"), @@ -798,7 +797,7 @@ pub trait BlockStorage: 'static + Clone + Send + Sync { match subdag.get(&round) { Some(certificates) => { // Retrieve the certificate for the given certificate ID. - match certificates.iter().find(|certificate| &certificate.certificate_id() == certificate_id) { + match certificates.iter().find(|certificate| &certificate.id() == certificate_id) { Some(certificate) => Ok(Some(certificate.clone())), None => bail!("The certificate '{certificate_id}' is missing in block storage"), }