diff --git a/ledger/block/src/verify.rs b/ledger/block/src/verify.rs index 5fdf77ff9..9a8be98f0 100644 --- a/ledger/block/src/verify.rs +++ b/ledger/block/src/verify.rs @@ -38,6 +38,7 @@ impl Block { ratified_finalize_operations: Vec>, ) -> Result<(Vec>, Vec)> { // Ensure the block hash is correct. + // Block producer: https://github.com/ProvableHQ/snarkVM/blob/mainnet-staging/ledger/block/src/lib.rs#L100C16-L100C77 self.verify_hash(previous_block.height(), previous_block.hash())?; // Ensure the block authority is correct. @@ -55,6 +56,7 @@ impl Block { )?; // Ensure the block solutions are correct. + // NOTE: this function is not named correctly, as it also computes expected rewards. let ( expected_cumulative_weight, expected_cumulative_proof_target, @@ -67,9 +69,11 @@ impl Block { ) = self.verify_solutions(previous_block, current_puzzle, current_epoch_hash)?; // Ensure the block ratifications are correct. + // NOTE: simply the same for the block producer self.verify_ratifications(expected_block_reward, expected_puzzle_reward)?; // Ensure the block transactions are correct. + // NOTE: some of the internal logic can be DRYed up with the block producer. self.verify_transactions()?; // Set the expected previous state root. @@ -178,6 +182,7 @@ impl Block { } }; // Ensure the block round minus the committee lookback range is at least the starting round of the committee lookback. + // NOTE: must be true by design of the subdag. ensure!( expected_round.saturating_sub(Committee::::COMMITTEE_LOOKBACK_RANGE) >= current_committee_lookback.starting_round(), @@ -209,12 +214,14 @@ impl Block { // Compute the expected leader. let expected_leader = current_committee_lookback.get_leader(expected_round)?; // Ensure the block is authored by the expected leader. + // NOTE: correct by design of the subdag. ensure!( subdag.leader_address() == expected_leader, "Quorum block {expected_height} is authored by an unexpected leader (found: {}, expected: {expected_leader})", subdag.leader_address() ); // Ensure the transmission IDs from the subdag correspond to the block. + // NOTE: correct by how a block producer drains transmissions from the subdag. Self::check_subdag_transmissions( subdag, &self.solutions, @@ -236,12 +243,14 @@ impl Block { // Check that the committee IDs are correct. if let Authority::Quorum(subdag) = &self.authority { // Check that the committee ID of the leader certificate is correct. + // NOTE: correct by design of committing the leader certificate: https://github.com/ProvableHQ/snarkOS/blob/profile_check_next_block/node/bft/src/bft.rs#L498 ensure!( subdag.leader_certificate().committee_id() == current_committee_lookback.id(), "Leader certificate has an incorrect committee ID" ); // Check that all all certificates on each round have the same committee ID. + // NOTE: correct by design of the certificates. cfg_iter!(subdag).try_for_each(|(round, certificates)| { // Check that every certificate for a given round shares the same committee ID. let expected_committee_id = certificates @@ -311,6 +320,7 @@ impl Block { let timestamp = self.timestamp(); // Ensure the number of solutions is within the allowed range. + // NOTE: https://github.com/ProvableHQ/snarkVM/blob/mainnet-staging/ledger/src/advance.rs#L198-L200 ensure!( self.solutions.len() <= N::MAX_SOLUTIONS, "Block {height} contains too many prover solutions (found '{}', expected '{}')", @@ -318,6 +328,7 @@ impl Block { N::MAX_SOLUTIONS ); // Ensure the number of aborted solution IDs is within the allowed range. + // NOTE: correct by design, MAX_ABORTED_SOLUTIONS == MAX_TRANSMISSIONS ensure!( self.aborted_solution_ids.len() <= Solutions::::MAX_ABORTED_SOLUTIONS, "Block {height} contains too many aborted solution IDs (found '{}', expected '{}')", @@ -326,6 +337,7 @@ impl Block { ); // Ensure there are no duplicate solution IDs. + // NOTE: already ensured when processing the subdag. if has_duplicates( self.solutions .as_ref() @@ -346,6 +358,7 @@ impl Block { // Verify the solutions. if let Some(coinbase) = self.solutions.deref() { // Ensure the puzzle proof is valid. + // NOTE: extensively unit tested that this performs the same check as the individual solution checks. if let Err(e) = current_puzzle.check_solutions(coinbase, current_epoch_hash, previous_block.proof_target()) { bail!("Block {height} contains an invalid puzzle proof - {e}"); @@ -359,6 +372,8 @@ impl Block { } }; + // NOTE: the calculations below are simply equivalent to the block producer's calculation. + // Calculate the next coinbase targets and timestamps. let ( expected_coinbase_target, diff --git a/ledger/src/check_next_block.rs b/ledger/src/check_next_block.rs index 77752b478..304069a74 100644 --- a/ledger/src/check_next_block.rs +++ b/ledger/src/check_next_block.rs @@ -20,22 +20,37 @@ impl> Ledger { let height = block.height(); // Ensure the block hash does not already exist. + // NOTE: impossible to fail for honest block producer if previous block hash is correct. + // The block hash is computed in https://github.com/ProvableHQ/snarkVM/blob/profile_check_next_block/ledger/block/src/lib.rs#L100 if self.contains_block_hash(&block.hash())? { bail!("Block hash '{}' already exists in the ledger", block.hash()) } // Ensure the block height does not already exist. + // NOTE: impossible to fail for honest block producer if previous block height is correct. + // The block height is set in https://github.com/ProvableHQ/snarkVM/blob/profile_check_next_block/ledger/src/advance.rs#L244 if self.contains_block_height(block.height())? { bail!("Block height '{height}' already exists in the ledger") } // Ensure the solutions do not already exist. + // NOTE: impossible to fail for honest block producer, as this is checked when processing the DAG. + // Only a single DAG is processed at a time: https://github.com/ProvableHQ/snarkOS/blob/profile_check_next_block/node/bft/src/bft.rs#L455 for solution_id in block.solutions().solution_ids() { if self.contains_solution_id(solution_id)? { bail!("Solution ID {solution_id} already exists in the ledger"); } } + // Ensure the transactions do not already exist. + // NOTE: impossible to fail for honest block producer, as this is checked when processing the DAG. + // Only a single DAG is processed at a time: https://github.com/ProvableHQ/snarkOS/blob/profile_check_next_block/node/bft/src/bft.rs#L455 + for transaction_id in block.transactions().transaction_ids() { + if self.contains_transaction_id(transaction_id)? { + bail!("Transaction ID {transaction_id} already exists in the ledger"); + } + } + // TODO (howardwu): Remove this after moving the total supply into credits.aleo. { // // Retrieve the latest total supply. @@ -69,6 +84,7 @@ impl> Ledger { )?; // Ensure speculation over the unconfirmed transactions is correct and ensure each transaction is well-formed and unique. + // NOTE: impossible to fail for honest block producer, as they will also verify transactions and call atomic_speculate are performed. let ratified_finalize_operations = self.vm.check_speculate(state, block.ratifications(), block.solutions(), block.transactions(), rng)?; @@ -106,6 +122,7 @@ impl> Ledger { }; // Ensure the block is correct. + // NOTE: impossible to fail for honest block producer, as they will also verify transactions and call atomic_speculate are performed. let (expected_existing_solution_ids, expected_existing_transaction_ids) = block.verify( &self.latest_block(), self.latest_state_root(),