From 150a763408297624a869191b5454408cd6c2f5a1 Mon Sep 17 00:00:00 2001 From: Howard Wu <9260812+howardwu@users.noreply.github.com> Date: Fri, 9 Feb 2024 19:31:10 -0800 Subject: [PATCH] Reverts election certificates --- ledger/narwhal/batch-header/src/bytes.rs | 55 +----------- ledger/narwhal/batch-header/src/lib.rs | 92 ++------------------ ledger/narwhal/batch-header/src/serialize.rs | 23 +---- ledger/narwhal/batch-header/src/to_id.rs | 15 ---- ledger/narwhal/subdag/src/bytes.rs | 34 +------- ledger/narwhal/subdag/src/lib.rs | 27 +----- ledger/narwhal/subdag/src/serialize.rs | 9 +- 7 files changed, 18 insertions(+), 237 deletions(-) diff --git a/ledger/narwhal/batch-header/src/bytes.rs b/ledger/narwhal/batch-header/src/bytes.rs index a59e9cd89f..37474eb118 100644 --- a/ledger/narwhal/batch-header/src/bytes.rs +++ b/ledger/narwhal/batch-header/src/bytes.rs @@ -20,8 +20,7 @@ impl FromBytes for BatchHeader { // Read the version. let version = u8::read_le(&mut reader)?; // Ensure the version is valid. - // TODO (howardwu): For mainnet - Change the version back to 1. - if version != 1 && version != 2 { + if version != 1 { return Err(error("Invalid batch header version")); } @@ -66,44 +65,12 @@ impl FromBytes for BatchHeader { previous_certificate_ids.insert(Field::read_le(&mut reader)?); } - // TODO (howardwu): For mainnet - Change this to always encode the number of committed certificate IDs. - // We currently only encode the size and certificates in the new version, for backwards compatibility. - let num_last_election_certificate_ids = if version == 2 { - // Read the number of last election certificate IDs. - u16::read_le(&mut reader)? - } else { - // Set the number of last election certificate IDs to zero. - 0 - }; - // Ensure the number of last election certificate IDs is within bounds. - if num_last_election_certificate_ids > Self::MAX_CERTIFICATES { - return Err(error(format!( - "Number of last election certificate IDs ({num_last_election_certificate_ids}) exceeds the maximum ({})", - Self::MAX_CERTIFICATES - ))); - } - // Read the last election certificate IDs. - let mut last_election_certificate_ids = IndexSet::new(); - for _ in 0..num_last_election_certificate_ids { - // Read the certificate ID. - last_election_certificate_ids.insert(Field::read_le(&mut reader)?); - } - // Read the signature. let signature = Signature::read_le(&mut reader)?; // Construct the batch. - let batch = Self::from( - version, - author, - round, - timestamp, - transmission_ids, - previous_certificate_ids, - last_election_certificate_ids, - signature, - ) - .map_err(|e| error(e.to_string()))?; + let batch = Self::from(author, round, timestamp, transmission_ids, previous_certificate_ids, signature) + .map_err(error)?; // Return the batch. match batch.batch_id == batch_id { @@ -117,8 +84,7 @@ impl ToBytes for BatchHeader { /// Writes the batch header to the buffer. fn write_le(&self, mut writer: W) -> IoResult<()> { // Write the version. - // TODO (howardwu): For mainnet - Change this back to '1u8.write_le(&mut writer)?'; - self.version.write_le(&mut writer)?; + 1u8.write_le(&mut writer)?; // Write the batch ID. self.batch_id.write_le(&mut writer)?; // Write the author. @@ -141,19 +107,6 @@ impl ToBytes for BatchHeader { // Write the certificate ID. certificate_id.write_le(&mut writer)?; } - // TODO (howardwu): For mainnet - Change this to always encode the number of committed certificate IDs. - // We currently only encode the size and certificates in the new version, for backwards compatibility. - if self.version != 1 { - // Write the number of last election certificate IDs. - u16::try_from(self.last_election_certificate_ids.len()) - .map_err(|e| error(e.to_string()))? - .write_le(&mut writer)?; - // Write the last election certificate IDs. - for certificate_id in &self.last_election_certificate_ids { - // Write the certificate ID. - certificate_id.write_le(&mut writer)?; - } - } // Write the signature. self.signature.write_le(&mut writer) } diff --git a/ledger/narwhal/batch-header/src/lib.rs b/ledger/narwhal/batch-header/src/lib.rs index e7a8de8c7a..4389f696dc 100644 --- a/ledger/narwhal/batch-header/src/lib.rs +++ b/ledger/narwhal/batch-header/src/lib.rs @@ -31,10 +31,6 @@ use narwhal_transmission_id::TransmissionID; #[derive(Clone, PartialEq, Eq)] pub struct BatchHeader { - /// The version of the batch header. - /// TODO (howardwu): For mainnet - Remove this version from the struct, we only use it here for backwards compatibility. - /// NOTE: You must keep the version encoding in the byte serialization, just remove it from the struct in memory. - version: u8, /// The batch ID, defined as the hash of the author, round number, timestamp, transmission IDs, /// previous batch certificate IDs, and last election certificate IDs. batch_id: Field, @@ -48,8 +44,6 @@ pub struct BatchHeader { transmission_ids: IndexSet>, /// The batch certificate IDs of the previous round. previous_certificate_ids: IndexSet>, - /// The last election batch certificate IDs. - last_election_certificate_ids: IndexSet>, /// The signature of the batch ID from the creator. signature: Signature, } @@ -74,20 +68,12 @@ impl BatchHeader { timestamp: i64, transmission_ids: IndexSet>, previous_certificate_ids: IndexSet>, - last_election_certificate_ids: IndexSet>, rng: &mut R, ) -> Result { - // Set the version. - // TODO (howardwu): For mainnet - Remove this version from the struct, we only use it here for backwards compatibility. - // NOTE: You must keep the version encoding in the byte serialization, just remove it from the struct in memory. - let version = 2u8; - match round { 0 | 1 => { // If the round is zero or one, then there should be no previous certificate IDs. ensure!(previous_certificate_ids.is_empty(), "Invalid round number, must not have certificates"); - // If the round is zero or one, then there should be no last election certificate IDs. - ensure!(last_election_certificate_ids.is_empty(), "Invalid batch, contains election certificates"); } // If the round is not zero and not one, then there should be at least one previous certificate ID. _ => ensure!(!previous_certificate_ids.is_empty(), "Invalid round number, must have certificates"), @@ -105,58 +91,30 @@ impl BatchHeader { "Invalid number of previous certificate IDs ({})", previous_certificate_ids.len() ); - // Ensure the number of last election certificate IDs is within bounds. - ensure!( - last_election_certificate_ids.len() <= Self::MAX_CERTIFICATES as usize, - "Invalid number of last election certificate IDs ({})", - last_election_certificate_ids.len() - ); // Retrieve the address. let author = Address::try_from(private_key)?; // Compute the batch ID. - let batch_id = Self::compute_batch_id( - version, - author, - round, - timestamp, - &transmission_ids, - &previous_certificate_ids, - &last_election_certificate_ids, - )?; + let batch_id = Self::compute_batch_id(author, round, timestamp, &transmission_ids, &previous_certificate_ids)?; // Sign the preimage. let signature = private_key.sign(&[batch_id], rng)?; // Return the batch header. - Ok(Self { - version, - author, - batch_id, - round, - timestamp, - transmission_ids, - previous_certificate_ids, - last_election_certificate_ids, - signature, - }) + Ok(Self { author, batch_id, round, timestamp, transmission_ids, previous_certificate_ids, signature }) } /// Initializes a new batch header. pub fn from( - version: u8, author: Address, round: u64, timestamp: i64, transmission_ids: IndexSet>, previous_certificate_ids: IndexSet>, - last_election_certificate_ids: IndexSet>, signature: Signature, ) -> Result { match round { 0 | 1 => { // If the round is zero or one, then there should be no previous certificate IDs. ensure!(previous_certificate_ids.is_empty(), "Invalid round number, must not have certificates"); - // If the round is zero or one, then there should be no last election certificate IDs. - ensure!(last_election_certificate_ids.is_empty(), "Invalid batch, contains election certificates"); } // If the round is not zero and not one, then there should be at least one previous certificate ID. _ => ensure!(!previous_certificate_ids.is_empty(), "Invalid round number, must have certificates"), @@ -174,39 +132,15 @@ impl BatchHeader { "Invalid number of previous certificate IDs ({})", previous_certificate_ids.len() ); - // Ensure the number of last election certificate IDs is within bounds. - ensure!( - last_election_certificate_ids.len() <= Self::MAX_CERTIFICATES as usize, - "Invalid number of last election certificate IDs ({})", - last_election_certificate_ids.len() - ); // Compute the batch ID. - let batch_id = Self::compute_batch_id( - version, - author, - round, - timestamp, - &transmission_ids, - &previous_certificate_ids, - &last_election_certificate_ids, - )?; + let batch_id = Self::compute_batch_id(author, round, timestamp, &transmission_ids, &previous_certificate_ids)?; // Verify the signature. if !signature.verify(&author, &[batch_id]) { bail!("Invalid signature for the batch header"); } // Return the batch header. - Ok(Self { - version, - author, - batch_id, - round, - timestamp, - transmission_ids, - previous_certificate_ids, - last_election_certificate_ids, - signature, - }) + Ok(Self { author, batch_id, round, timestamp, transmission_ids, previous_certificate_ids, signature }) } } @@ -241,11 +175,6 @@ impl BatchHeader { &self.previous_certificate_ids } - /// Returns the last election batch certificate IDs. - pub const fn last_election_certificate_ids(&self) -> &IndexSet> { - &self.last_election_certificate_ids - } - /// Returns the signature. pub const fn signature(&self) -> &Signature { &self.signature @@ -304,19 +233,8 @@ pub mod test_helpers { narwhal_transmission_id::test_helpers::sample_transmission_ids(rng).into_iter().collect::>(); // Checkpoint the timestamp for the batch. let timestamp = OffsetDateTime::now_utc().unix_timestamp(); - // Sample the last election certificate IDs. - let last_election_certificate_ids = (0..5).map(|_| Field::::rand(rng)).collect::>(); // Return the batch header. - BatchHeader::new( - &private_key, - round, - timestamp, - transmission_ids, - previous_certificate_ids, - last_election_certificate_ids, - rng, - ) - .unwrap() + BatchHeader::new(&private_key, round, timestamp, transmission_ids, previous_certificate_ids, rng).unwrap() } /// Returns a list of sample batch headers, sampled at random. diff --git a/ledger/narwhal/batch-header/src/serialize.rs b/ledger/narwhal/batch-header/src/serialize.rs index 15cefd3a70..6acddaead2 100644 --- a/ledger/narwhal/batch-header/src/serialize.rs +++ b/ledger/narwhal/batch-header/src/serialize.rs @@ -19,16 +19,13 @@ impl Serialize for BatchHeader { fn serialize(&self, serializer: S) -> Result { match serializer.is_human_readable() { true => { - let mut header = serializer.serialize_struct("BatchHeader", 9)?; - // TODO (howardwu): For mainnet - Remove the version field, and update the 'len' above to 8. - header.serialize_field("version", &self.version)?; + let mut header = serializer.serialize_struct("BatchHeader", 7)?; header.serialize_field("batch_id", &self.batch_id)?; header.serialize_field("author", &self.author)?; header.serialize_field("round", &self.round)?; header.serialize_field("timestamp", &self.timestamp)?; header.serialize_field("transmission_ids", &self.transmission_ids)?; header.serialize_field("previous_certificate_ids", &self.previous_certificate_ids)?; - header.serialize_field("last_election_certificate_ids", &self.last_election_certificate_ids)?; header.serialize_field("signature", &self.signature)?; header.end() } @@ -45,31 +42,13 @@ impl<'de, N: Network> Deserialize<'de> for BatchHeader { let mut header = serde_json::Value::deserialize(deserializer)?; let batch_id: Field = DeserializeExt::take_from_value::(&mut header, "batch_id")?; - // TODO (howardwu): For mainnet - Remove the version parsing. - // If the version field is present, then parse the version. - let version = DeserializeExt::take_from_value::(&mut header, "version").unwrap_or(1); - // TODO (howardwu): For mainnet - Remove the version checking. - // Ensure the version is valid. - if version != 1 && version != 2 { - return Err(error("Invalid batch header version")).map_err(de::Error::custom); - } - // TODO (howardwu): For mainnet - Always take from the 'header', no need to use this match case anymore. - // If the version is not 1, then parse the last election certificate IDs. - let last_election_certificate_ids = match version { - 1 => IndexSet::new(), - 2 => DeserializeExt::take_from_value::(&mut header, "last_election_certificate_ids")?, - _ => unreachable!(), - }; - // Recover the header. let batch_header = Self::from( - version, DeserializeExt::take_from_value::(&mut header, "author")?, DeserializeExt::take_from_value::(&mut header, "round")?, DeserializeExt::take_from_value::(&mut header, "timestamp")?, DeserializeExt::take_from_value::(&mut header, "transmission_ids")?, DeserializeExt::take_from_value::(&mut header, "previous_certificate_ids")?, - last_election_certificate_ids, DeserializeExt::take_from_value::(&mut header, "signature")?, ) .map_err(de::Error::custom)?; diff --git a/ledger/narwhal/batch-header/src/to_id.rs b/ledger/narwhal/batch-header/src/to_id.rs index 1d3eea042b..1d99cbda77 100644 --- a/ledger/narwhal/batch-header/src/to_id.rs +++ b/ledger/narwhal/batch-header/src/to_id.rs @@ -18,13 +18,11 @@ impl BatchHeader { /// Returns the batch ID. pub fn to_id(&self) -> Result> { Self::compute_batch_id( - self.version, self.author, self.round, self.timestamp, &self.transmission_ids, &self.previous_certificate_ids, - &self.last_election_certificate_ids, ) } } @@ -32,13 +30,11 @@ impl BatchHeader { impl BatchHeader { /// Returns the batch ID. pub fn compute_batch_id( - version: u8, author: Address, round: u64, timestamp: i64, transmission_ids: &IndexSet>, previous_certificate_ids: &IndexSet>, - last_election_certificate_ids: &IndexSet>, ) -> Result> { let mut preimage = Vec::new(); // Insert the author. @@ -60,17 +56,6 @@ impl BatchHeader { // Insert the certificate ID. certificate_id.write_le(&mut preimage)?; } - // TODO (howardwu): For mainnet - Change this to always encode the number of committed certificate IDs. - // We currently only encode the size and certificates only in the new version, for backwards compatibility. - if version != 1 { - // Insert the number of last election certificate IDs. - u32::try_from(last_election_certificate_ids.len())?.write_le(&mut preimage)?; - // Insert the last election certificate IDs. - for certificate_id in last_election_certificate_ids { - // Insert the certificate ID. - certificate_id.write_le(&mut preimage)?; - } - } // Hash the preimage. N::hash_bhp1024(&preimage.to_bits_le()) } diff --git a/ledger/narwhal/subdag/src/bytes.rs b/ledger/narwhal/subdag/src/bytes.rs index 703f1c5ead..5b7aef35a9 100644 --- a/ledger/narwhal/subdag/src/bytes.rs +++ b/ledger/narwhal/subdag/src/bytes.rs @@ -20,8 +20,7 @@ impl FromBytes for Subdag { // Read the version. let version = u8::read_le(&mut reader)?; // Ensure the version is valid. - // TODO (howardwu): For mainnet - Change the version back to 1. - if version != 1 && version != 2 { + if version != 1 { return Err(error(format!("Invalid subdag version ({version})"))); } @@ -55,27 +54,8 @@ impl FromBytes for Subdag { subdag.insert(round, certificates); } - // Read the election certificate IDs. - let mut election_certificate_ids = IndexSet::new(); - // TODO (howardwu): For mainnet - Always attempt to deserialize the election certificate IDs. - if version != 1 { - // Read the number of election certificate IDs. - let num_election_certificate_ids = u16::read_le(&mut reader)?; - // Ensure the number of election certificate IDs is within bounds. - if num_election_certificate_ids > BatchHeader::::MAX_CERTIFICATES { - return Err(error(format!( - "Number of election certificate IDs ({num_election_certificate_ids}) exceeds the maximum ({})", - BatchHeader::::MAX_CERTIFICATES - ))); - } - for _ in 0..num_election_certificate_ids { - // Read the election certificate ID. - election_certificate_ids.insert(Field::read_le(&mut reader)?); - } - } - // Return the subdag. - Self::from(subdag, election_certificate_ids).map_err(error) + Self::from(subdag).map_err(error) } } @@ -83,8 +63,7 @@ impl ToBytes for Subdag { /// Writes the subdag to the buffer. fn write_le(&self, mut writer: W) -> IoResult<()> { // Write the version. - // TODO (howardwu): For mainnet - Change the version back to 1. - 2u8.write_le(&mut writer)?; + 1u8.write_le(&mut writer)?; // Write the number of rounds. u32::try_from(self.subdag.len()).map_err(error)?.write_le(&mut writer)?; // Write the round certificates. @@ -99,13 +78,6 @@ impl ToBytes for Subdag { certificate.write_le(&mut writer)?; } } - // Write the number of election certificate IDs. - u16::try_from(self.election_certificate_ids.len()).map_err(error)?.write_le(&mut writer)?; - // Write the election certificate IDs. - for election_certificate_id in &self.election_certificate_ids { - // Write the election certificate ID. - election_certificate_id.write_le(&mut writer)?; - } Ok(()) } } diff --git a/ledger/narwhal/subdag/src/lib.rs b/ledger/narwhal/subdag/src/lib.rs index f6858bfbdc..d699d07ecf 100644 --- a/ledger/narwhal/subdag/src/lib.rs +++ b/ledger/narwhal/subdag/src/lib.rs @@ -113,8 +113,6 @@ fn weighted_median(timestamps_and_stake: Vec<(i64, u64)>) -> i64 { pub struct Subdag { /// The subdag of round certificates. subdag: BTreeMap>>, - /// The election certificate IDs. - election_certificate_ids: IndexSet>, } impl PartialEq for Subdag { @@ -128,10 +126,7 @@ impl Eq for Subdag {} impl Subdag { /// Initializes a new subdag. - pub fn from( - subdag: BTreeMap>>, - election_certificate_ids: IndexSet>, - ) -> Result { + pub fn from(subdag: BTreeMap>>) -> Result { // Ensure the subdag is not empty. ensure!(!subdag.is_empty(), "Subdag cannot be empty"); // Ensure the subdag does not exceed the maximum number of rounds. @@ -143,17 +138,12 @@ impl Subdag { ensure!(subdag.iter().next_back().map_or(0, |(r, _)| *r) % 2 == 0, "Anchor round must be even"); // Ensure there is only one leader certificate. ensure!(subdag.iter().next_back().map_or(0, |(_, c)| c.len()) == 1, "Subdag cannot have multiple leaders"); - // Ensure the number of election certificate IDs is within bounds. - ensure!( - election_certificate_ids.len() <= usize::try_from(BatchHeader::::MAX_CERTIFICATES)?, - "Number of election certificate IDs exceeds the maximum" - ); // Ensure the rounds are sequential. ensure!(is_sequential(&subdag), "Subdag rounds must be sequential"); // Ensure the subdag structure matches the commit. ensure!(sanity_check_subdag_with_dfs(&subdag), "Subdag structure does not match commit"); // Ensure the leader certificate is an even round. - Ok(Self { subdag, election_certificate_ids }) + Ok(Self { subdag }) } } @@ -217,11 +207,6 @@ impl Subdag { } } - /// Returns the election certificate IDs. - pub fn election_certificate_ids(&self) -> &IndexSet> { - &self.election_certificate_ids - } - /// Returns the subdag root of the certificates. pub fn to_subdag_root(&self) -> Result> { // Prepare the leaves. @@ -300,14 +285,8 @@ pub mod test_helpers { ); subdag.insert(starting_round + 2, indexset![certificate]); - // Initialize the election certificate IDs. - let mut election_certificate_ids = IndexSet::new(); - for _ in 0..AVAILABILITY_THRESHOLD { - election_certificate_ids.insert(rng.gen()); - } - // Return the subdag. - Subdag::from(subdag, election_certificate_ids).unwrap() + Subdag::from(subdag).unwrap() } /// Returns a list of sample subdags, sampled at random. diff --git a/ledger/narwhal/subdag/src/serialize.rs b/ledger/narwhal/subdag/src/serialize.rs index fb73011d91..f02389871e 100644 --- a/ledger/narwhal/subdag/src/serialize.rs +++ b/ledger/narwhal/subdag/src/serialize.rs @@ -19,9 +19,8 @@ impl Serialize for Subdag { fn serialize(&self, serializer: S) -> Result { match serializer.is_human_readable() { true => { - let mut certificate = serializer.serialize_struct("Subdag", 2)?; + let mut certificate = serializer.serialize_struct("Subdag", 1)?; certificate.serialize_field("subdag", &self.subdag)?; - certificate.serialize_field("election_certificate_ids", &self.election_certificate_ids)?; certificate.end() } false => ToBytesSerializer::serialize_with_size_encoding(self, serializer), @@ -36,11 +35,7 @@ impl<'de, N: Network> Deserialize<'de> for Subdag { true => { let mut value = serde_json::Value::deserialize(deserializer)?; - // TODO (howardwu): For mainnet - Directly take the value, do not check if its missing. - let election_certificate_ids = - DeserializeExt::take_from_value::(&mut value, "election_certificate_ids").unwrap_or_default(); - - Ok(Self::from(DeserializeExt::take_from_value::(&mut value, "subdag")?, election_certificate_ids) + Ok(Self::from(DeserializeExt::take_from_value::(&mut value, "subdag")?) .map_err(de::Error::custom)?) } false => FromBytesDeserializer::::deserialize_with_size_encoding(deserializer, "subdag"),