From 2a9f2484beee936bb4c381d768df47d5a3d2c07a Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:01:46 -0800 Subject: [PATCH 1/5] Use the penultimate round for committee selection for timestamp --- ledger/src/advance.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index 2c15ae0dc4..eebdb62b74 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -240,9 +240,14 @@ impl> Ledger { // Determine the timestamp for the next block. let next_timestamp = match subdag { Some(subdag) => { - // Get the committee lookback round. - let committee_lookback_round = - subdag.anchor_round().saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); + // Calculate the penultimate round, which is the round before the anchor round. + let penultimate_round = subdag.anchor_round().saturating_sub(1); + // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, + // because committees are updated in even rounds. + let committee_lookback_round = match penultimate_round % 2 == 0 { + true => penultimate_round.saturating_sub(1), + false => penultimate_round.saturating_sub(2), + }; // Retrieve the committee lookback. let committee_lookback = self .get_committee_for_round(committee_lookback_round)? From b822aa8f78d8409d2f79e094c53eaf2fa636cad5 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:02:40 -0800 Subject: [PATCH 2/5] Use the previous committee lookback for timestamp verification --- ledger/block/src/verify.rs | 11 +++++++++-- ledger/src/check_next_block.rs | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ledger/block/src/verify.rs b/ledger/block/src/verify.rs index 29bd8cb979..b7afba54df 100644 --- a/ledger/block/src/verify.rs +++ b/ledger/block/src/verify.rs @@ -30,6 +30,7 @@ impl Block { &self, previous_block: &Block, current_state_root: N::StateRoot, + previous_committee_lookback: &Committee, current_committee_lookback: &Committee, current_puzzle: &CoinbasePuzzle, current_epoch_challenge: &EpochChallenge, @@ -46,7 +47,12 @@ impl Block { expected_timestamp, expected_existing_solution_ids, expected_existing_transaction_ids, - ) = self.verify_authority(previous_block.round(), previous_block.height(), current_committee_lookback)?; + ) = self.verify_authority( + previous_block.round(), + previous_block.height(), + previous_committee_lookback, + current_committee_lookback, + )?; // Ensure the block solutions are correct. let ( @@ -143,6 +149,7 @@ impl Block { &self, previous_round: u64, previous_height: u32, + previous_committee_lookback: &Committee, current_committee_lookback: &Committee, ) -> Result<(u64, u32, i64, Vec>, Vec)> { // Note: Do not remove this. This ensures that all blocks after genesis are quorum blocks. @@ -223,7 +230,7 @@ impl Block { // Beacon blocks do not have a timestamp check. Authority::Beacon(..) => self.timestamp(), // Quorum blocks use the weighted median timestamp from the subdag. - Authority::Quorum(subdag) => subdag.timestamp(current_committee_lookback), + Authority::Quorum(subdag) => subdag.timestamp(previous_committee_lookback), }; // Return success. diff --git a/ledger/src/check_next_block.rs b/ledger/src/check_next_block.rs index 3f282f660b..78e33c262d 100644 --- a/ledger/src/check_next_block.rs +++ b/ledger/src/check_next_block.rs @@ -95,10 +95,24 @@ impl> Ledger { .get_committee_for_round(committee_lookback_round)? .ok_or(anyhow!("Failed to fetch committee for round {committee_lookback_round}"))?; + // Calculate the penultimate round, which is the round before the anchor round. + let penultimate_round = block.round().saturating_sub(1); + // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, + // because committees are updated in even rounds. + let penultimate_committee_lookback_round = match penultimate_round % 2 == 0 { + true => penultimate_round.saturating_sub(1), + false => penultimate_round.saturating_sub(2), + }; + // Retrieve the previous committee lookback. + let previous_committee_lookback = self + .get_committee_for_round(penultimate_committee_lookback_round)? + .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))?; + // Ensure the block is correct. let (expected_existing_solution_ids, expected_existing_transaction_ids) = block.verify( &self.latest_block(), self.latest_state_root(), + &previous_committee_lookback, &committee_lookback, self.coinbase_puzzle(), &self.latest_epoch_challenge()?, From c78aaf5b83e9a95873b2fd4bbdda7970696e8b7f Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:59:36 -0800 Subject: [PATCH 3/5] Properly subtract the committee lookback range --- ledger/src/advance.rs | 12 +++++++----- ledger/src/check_next_block.rs | 5 ++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index eebdb62b74..2dcd1d7ca9 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -244,16 +244,18 @@ impl> Ledger { let penultimate_round = subdag.anchor_round().saturating_sub(1); // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, // because committees are updated in even rounds. - let committee_lookback_round = match penultimate_round % 2 == 0 { + let previous_penultimate_round = match penultimate_round % 2 == 0 { true => penultimate_round.saturating_sub(1), false => penultimate_round.saturating_sub(2), }; + let penultimate_committee_lookback_round = + previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); // Retrieve the committee lookback. - let committee_lookback = self - .get_committee_for_round(committee_lookback_round)? - .ok_or(anyhow!("Failed to fetch committee for round {committee_lookback_round}"))?; + let previous_committee_lookback = + self.get_committee_for_round(penultimate_committee_lookback_round)? + .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))?; // Return the timestamp for the given committee lookback. - subdag.timestamp(&committee_lookback) + subdag.timestamp(&previous_committee_lookback) } None => OffsetDateTime::now_utc().unix_timestamp(), }; diff --git a/ledger/src/check_next_block.rs b/ledger/src/check_next_block.rs index 78e33c262d..3a22a21129 100644 --- a/ledger/src/check_next_block.rs +++ b/ledger/src/check_next_block.rs @@ -99,10 +99,13 @@ impl> Ledger { let penultimate_round = block.round().saturating_sub(1); // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, // because committees are updated in even rounds. - let penultimate_committee_lookback_round = match penultimate_round % 2 == 0 { + let previous_penultimate_round = match penultimate_round % 2 == 0 { true => penultimate_round.saturating_sub(1), false => penultimate_round.saturating_sub(2), }; + // Get the previous committee lookback round. + let penultimate_committee_lookback_round = + previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); // Retrieve the previous committee lookback. let previous_committee_lookback = self .get_committee_for_round(penultimate_committee_lookback_round)? From bbd466f881eb3d3291fd4b894b73cd92d171c5b4 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Thu, 15 Feb 2024 16:01:12 -0800 Subject: [PATCH 4/5] fix comment --- ledger/src/advance.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index 2dcd1d7ca9..1b2a7b9140 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -248,9 +248,10 @@ impl> Ledger { true => penultimate_round.saturating_sub(1), false => penultimate_round.saturating_sub(2), }; + // Get the previous committee lookback round. let penultimate_committee_lookback_round = previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); - // Retrieve the committee lookback. + // Retrieve the previous committee lookback. let previous_committee_lookback = self.get_committee_for_round(penultimate_committee_lookback_round)? .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))?; From 870cd70f1d5da48722d767ec0415361aba1ea2f2 Mon Sep 17 00:00:00 2001 From: Howard Wu <9260812+howardwu@users.noreply.github.com> Date: Fri, 16 Feb 2024 10:36:50 -0800 Subject: [PATCH 5/5] nit: scope the committee lookback logic --- ledger/src/advance.rs | 28 ++++++++++-------- ledger/src/check_next_block.rs | 54 ++++++++++++++++++---------------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index 1b2a7b9140..62a2775832 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -240,21 +240,23 @@ impl> Ledger { // Determine the timestamp for the next block. let next_timestamp = match subdag { Some(subdag) => { - // Calculate the penultimate round, which is the round before the anchor round. - let penultimate_round = subdag.anchor_round().saturating_sub(1); - // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, - // because committees are updated in even rounds. - let previous_penultimate_round = match penultimate_round % 2 == 0 { - true => penultimate_round.saturating_sub(1), - false => penultimate_round.saturating_sub(2), - }; - // Get the previous committee lookback round. - let penultimate_committee_lookback_round = - previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); // Retrieve the previous committee lookback. - let previous_committee_lookback = + let previous_committee_lookback = { + // Calculate the penultimate round, which is the round before the anchor round. + let penultimate_round = subdag.anchor_round().saturating_sub(1); + // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, + // because committees are updated in even rounds. + let previous_penultimate_round = match penultimate_round % 2 == 0 { + true => penultimate_round.saturating_sub(1), + false => penultimate_round.saturating_sub(2), + }; + // Get the previous committee lookback round. + let penultimate_committee_lookback_round = + previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); + // Output the previous committee lookback. self.get_committee_for_round(penultimate_committee_lookback_round)? - .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))?; + .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))? + }; // Return the timestamp for the given committee lookback. subdag.timestamp(&previous_committee_lookback) } diff --git a/ledger/src/check_next_block.rs b/ledger/src/check_next_block.rs index 3a22a21129..c23797f8d4 100644 --- a/ledger/src/check_next_block.rs +++ b/ledger/src/check_next_block.rs @@ -82,34 +82,38 @@ impl> Ledger { let ratified_finalize_operations = self.vm.check_speculate(state, block.ratifications(), block.solutions(), block.transactions())?; - // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, - // because committees are updated in even rounds. - let previous_round = match block.round() % 2 == 0 { - true => block.round().saturating_sub(1), - false => block.round().saturating_sub(2), - }; - // Get the committee lookback round. - let committee_lookback_round = previous_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); // Retrieve the committee lookback. - let committee_lookback = self - .get_committee_for_round(committee_lookback_round)? - .ok_or(anyhow!("Failed to fetch committee for round {committee_lookback_round}"))?; - - // Calculate the penultimate round, which is the round before the anchor round. - let penultimate_round = block.round().saturating_sub(1); - // Get the round number for the previous committee. Note, we subtract 2 from odd rounds, - // because committees are updated in even rounds. - let previous_penultimate_round = match penultimate_round % 2 == 0 { - true => penultimate_round.saturating_sub(1), - false => penultimate_round.saturating_sub(2), + let committee_lookback = { + // Determine the round number for the previous committee. Note, we subtract 2 from odd rounds, + // because committees are updated in even rounds. + let previous_round = match block.round() % 2 == 0 { + true => block.round().saturating_sub(1), + false => block.round().saturating_sub(2), + }; + // Determine the committee lookback round. + let committee_lookback_round = previous_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); + // Output the committee lookback. + self.get_committee_for_round(committee_lookback_round)? + .ok_or(anyhow!("Failed to fetch committee for round {committee_lookback_round}"))? }; - // Get the previous committee lookback round. - let penultimate_committee_lookback_round = - previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); + // Retrieve the previous committee lookback. - let previous_committee_lookback = self - .get_committee_for_round(penultimate_committee_lookback_round)? - .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))?; + let previous_committee_lookback = { + // Calculate the penultimate round, which is the round before the anchor round. + let penultimate_round = block.round().saturating_sub(1); + // Determine the round number for the previous committee. Note, we subtract 2 from odd rounds, + // because committees are updated in even rounds. + let previous_penultimate_round = match penultimate_round % 2 == 0 { + true => penultimate_round.saturating_sub(1), + false => penultimate_round.saturating_sub(2), + }; + // Determine the previous committee lookback round. + let penultimate_committee_lookback_round = + previous_penultimate_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE); + // Output the previous committee lookback. + self.get_committee_for_round(penultimate_committee_lookback_round)? + .ok_or(anyhow!("Failed to fetch committee for round {penultimate_committee_lookback_round}"))? + }; // Ensure the block is correct. let (expected_existing_solution_ids, expected_existing_transaction_ids) = block.verify(