From 543f7286fef92cdc974f5f6daa4c2d168eb65cbb Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Fri, 20 Oct 2023 13:49:18 -0400 Subject: [PATCH 1/5] Filter out the failed solutions --- ledger/src/advance.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index f7b7c205af..cf95d3b3be 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -123,6 +123,17 @@ impl> Ledger { let (solutions, solutions_root, combined_proof_target) = match candidate_solutions.is_empty() { true => (None, Field::::zero(), 0u128), false => { + // Only select the candidate solutions that are valid. + let latest_epoch_challenge = self.latest_epoch_challenge()?; + let coinbase_verifying_key = self.coinbase_puzzle.coinbase_verifying_key(); + let candidate_solutions = cfg_into_iter!(candidate_solutions) + .filter(|solution| { + solution + .verify(coinbase_verifying_key, &latest_epoch_challenge, self.latest_proof_target()) + .unwrap_or(false) + }) + .collect::>(); + // TODO (raychu86): Optimize this. We are currently double verifying the solutions. // Accumulate the prover solutions. let solutions = self.coinbase_puzzle.accumulate( candidate_solutions, From d18a7676b4cc0999245a2da495d6655cb2839532 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:10:14 -0400 Subject: [PATCH 2/5] Get the aborted solution ids --- ledger/coinbase/src/lib.rs | 2 +- ledger/src/advance.rs | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ledger/coinbase/src/lib.rs b/ledger/coinbase/src/lib.rs index b691eac746..fd4c5715b6 100644 --- a/ledger/coinbase/src/lib.rs +++ b/ledger/coinbase/src/lib.rs @@ -168,7 +168,7 @@ impl CoinbasePuzzle { // Initialize the coinbase solution. let solutions = CoinbaseSolution::new(prover_solutions); // Verify the solutions. - self.verify(&solutions, epoch_challenge, proof_target)?; + ensure!(self.verify(&solutions, epoch_challenge, proof_target)?, "The prover solutions must be valid"); // Return the solutions. Ok(solutions) } diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index cf95d3b3be..f72c0a5299 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -123,20 +123,22 @@ impl> Ledger { let (solutions, solutions_root, combined_proof_target) = match candidate_solutions.is_empty() { true => (None, Field::::zero(), 0u128), false => { - // Only select the candidate solutions that are valid. + // Separate the valid and aborted candidate solutions. let latest_epoch_challenge = self.latest_epoch_challenge()?; - let coinbase_verifying_key = self.coinbase_puzzle.coinbase_verifying_key(); - let candidate_solutions = cfg_into_iter!(candidate_solutions) - .filter(|solution| { + let (valid_candidate_solutions, _aborted_candidate_solutions): (Vec<_>, Vec<_>) = + cfg_into_iter!(candidate_solutions).partition(|solution| { solution - .verify(coinbase_verifying_key, &latest_epoch_challenge, self.latest_proof_target()) + .verify( + &self.coinbase_puzzle.coinbase_verifying_key(), + &latest_epoch_challenge, + self.latest_proof_target(), + ) .unwrap_or(false) - }) - .collect::>(); - // TODO (raychu86): Optimize this. We are currently double verifying the solutions. + }); + // TODO (raychu86): Optimize this. We are currently double verifying the valid solutions. // Accumulate the prover solutions. let solutions = self.coinbase_puzzle.accumulate( - candidate_solutions, + valid_candidate_solutions, &self.latest_epoch_challenge()?, self.latest_proof_target(), )?; From e3ff4726980f6a7fba8da7e1dd84e114f3b61bf1 Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:10:42 -0400 Subject: [PATCH 3/5] nit --- ledger/src/advance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index f72c0a5299..f8c7743080 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -129,7 +129,7 @@ impl> Ledger { cfg_into_iter!(candidate_solutions).partition(|solution| { solution .verify( - &self.coinbase_puzzle.coinbase_verifying_key(), + self.coinbase_puzzle.coinbase_verifying_key(), &latest_epoch_challenge, self.latest_proof_target(), ) @@ -139,7 +139,7 @@ impl> Ledger { // Accumulate the prover solutions. let solutions = self.coinbase_puzzle.accumulate( valid_candidate_solutions, - &self.latest_epoch_challenge()?, + &latest_epoch_challenge, self.latest_proof_target(), )?; // Compute the solutions root. From 49e11f2ab49cc2c4633b5b47d4ec1ba6834da8b9 Mon Sep 17 00:00:00 2001 From: Howard Wu <9260812+howardwu@users.noreply.github.com> Date: Fri, 20 Oct 2023 13:21:22 -0700 Subject: [PATCH 4/5] Remove accumulation method --- ledger/block/src/verify.rs | 8 ++-- ledger/coinbase/benches/coinbase_puzzle.rs | 37 ++----------------- .../src/helpers/coinbase_solution/bytes.rs | 2 +- .../src/helpers/coinbase_solution/mod.rs | 19 +++++++++- .../helpers/coinbase_solution/serialize.rs | 4 +- ledger/coinbase/src/lib.rs | 24 ++---------- ledger/coinbase/src/tests.rs | 19 +++++----- ledger/src/advance.rs | 20 ++++------ 8 files changed, 49 insertions(+), 84 deletions(-) diff --git a/ledger/block/src/verify.rs b/ledger/block/src/verify.rs index 63d23e5bb1..0b875a6d91 100644 --- a/ledger/block/src/verify.rs +++ b/ledger/block/src/verify.rs @@ -291,9 +291,11 @@ impl Block { } // Ensure the puzzle proof is valid. - let is_valid = - current_puzzle.verify(coinbase, current_epoch_challenge, previous_block.proof_target())?; - ensure!(is_valid, "Block {height} contains invalid puzzle proof"); + if let Err(e) = + current_puzzle.check_solutions(coinbase, current_epoch_challenge, previous_block.proof_target()) + { + bail!("Block {height} contains an invalid puzzle proof - {e}"); + } // Compute the combined proof target. let combined_proof_target = coinbase.to_combined_proof_target()?; diff --git a/ledger/coinbase/benches/coinbase_puzzle.rs b/ledger/coinbase/benches/coinbase_puzzle.rs index 64ee0d45dd..418466b67f 100644 --- a/ledger/coinbase/benches/coinbase_puzzle.rs +++ b/ledger/coinbase/benches/coinbase_puzzle.rs @@ -21,7 +21,7 @@ use console::{ account::*, network::{Network, Testnet3}, }; -use snarkvm_ledger_coinbase::{CoinbasePuzzle, EpochChallenge, PuzzleConfig}; +use snarkvm_ledger_coinbase::{CoinbasePuzzle, CoinbaseSolution, EpochChallenge, PuzzleConfig}; use criterion::Criterion; use rand::{self, thread_rng, CryptoRng, RngCore}; @@ -82,35 +82,6 @@ fn coinbase_puzzle_prove(c: &mut Criterion) { } } -#[cfg(feature = "setup")] -fn coinbase_puzzle_accumulate(c: &mut Criterion) { - let rng = &mut thread_rng(); - - let max_degree = 1 << 15; - let max_config = PuzzleConfig { degree: max_degree }; - let universal_srs = CoinbasePuzzle::::setup(max_config).unwrap(); - - for degree in [(1 << 13) - 1] { - let config = PuzzleConfig { degree }; - let puzzle = CoinbasePuzzleInst::trim(&universal_srs, config).unwrap(); - let epoch_challenge = sample_epoch_challenge(degree, rng); - - for batch_size in [10, 100, ::MAX_PROVER_SOLUTIONS] { - let solutions = (0..batch_size) - .map(|_| { - let (address, nonce) = sample_address_and_nonce(rng); - puzzle.prove(&epoch_challenge, address, nonce, None).unwrap() - }) - .collect::>(); - - c.bench_function( - &format!("CoinbasePuzzle::Accumulate {batch_size} of 2^{}", ((degree + 1) as f64).log2()), - |b| b.iter(|| puzzle.accumulate(solutions.clone(), &epoch_challenge, 0).unwrap()), - ); - } - } -} - #[cfg(feature = "setup")] fn coinbase_puzzle_verify(c: &mut Criterion) { let rng = &mut thread_rng(); @@ -131,11 +102,11 @@ fn coinbase_puzzle_verify(c: &mut Criterion) { puzzle.prove(&epoch_challenge, address, nonce, None).unwrap() }) .collect::>(); - let solutions = puzzle.accumulate(solutions, &epoch_challenge, 0).unwrap(); + let solutions = CoinbaseSolution::new(solutions).unwrap(); c.bench_function( &format!("CoinbasePuzzle::Verify {batch_size} of 2^{}", ((degree + 1) as f64).log2()), - |b| b.iter(|| assert!(puzzle.verify(&solutions, &epoch_challenge, 0u64).unwrap())), + |b| b.iter(|| puzzle.check_solutions(&solutions, &epoch_challenge, 0u64).unwrap()), ); } } @@ -144,7 +115,7 @@ fn coinbase_puzzle_verify(c: &mut Criterion) { criterion_group! { name = coinbase_puzzle; config = Criterion::default().sample_size(10); - targets = coinbase_puzzle_trim, coinbase_puzzle_prove, coinbase_puzzle_accumulate, coinbase_puzzle_verify, + targets = coinbase_puzzle_trim, coinbase_puzzle_prove, coinbase_puzzle_verify, } criterion_main!(coinbase_puzzle); diff --git a/ledger/coinbase/src/helpers/coinbase_solution/bytes.rs b/ledger/coinbase/src/helpers/coinbase_solution/bytes.rs index 90c26b769f..66d541a3fb 100644 --- a/ledger/coinbase/src/helpers/coinbase_solution/bytes.rs +++ b/ledger/coinbase/src/helpers/coinbase_solution/bytes.rs @@ -25,7 +25,7 @@ impl FromBytes for CoinbaseSolution { prover_solutions.push(ProverSolution::read_le(&mut reader)?); } // Return the solutions. - Ok(Self::new(prover_solutions)) + Self::new(prover_solutions).map_err(error) } } diff --git a/ledger/coinbase/src/helpers/coinbase_solution/mod.rs b/ledger/coinbase/src/helpers/coinbase_solution/mod.rs index d1dcad5bb2..bfdacbace4 100644 --- a/ledger/coinbase/src/helpers/coinbase_solution/mod.rs +++ b/ledger/coinbase/src/helpers/coinbase_solution/mod.rs @@ -29,8 +29,23 @@ pub struct CoinbaseSolution { impl CoinbaseSolution { /// Initializes a new instance of the solutions. - pub fn new(solutions: Vec>) -> Self { - Self { solutions: solutions.into_iter().map(|solution| (solution.commitment(), solution)).collect() } + pub fn new(solutions: Vec>) -> Result { + // Ensure the solutions are not empty. + ensure!(!solutions.is_empty(), "There are no solutions to verify for the coinbase puzzle"); + // Ensure the number of partial solutions does not exceed `MAX_PROVER_SOLUTIONS`. + if solutions.len() > N::MAX_PROVER_SOLUTIONS { + bail!( + "The solutions exceed the allowed number of partial solutions. ({} > {})", + solutions.len(), + N::MAX_PROVER_SOLUTIONS + ); + } + // Ensure the puzzle commitments are unique. + if has_duplicates(solutions.iter().map(|s| s.commitment())) { + bail!("The solutions contain duplicate puzzle commitments"); + } + // Return the solutions. + Ok(Self { solutions: solutions.into_iter().map(|solution| (solution.commitment(), solution)).collect() }) } /// Returns the puzzle commitments. diff --git a/ledger/coinbase/src/helpers/coinbase_solution/serialize.rs b/ledger/coinbase/src/helpers/coinbase_solution/serialize.rs index d6d5ca8330..ac973aee78 100644 --- a/ledger/coinbase/src/helpers/coinbase_solution/serialize.rs +++ b/ledger/coinbase/src/helpers/coinbase_solution/serialize.rs @@ -36,7 +36,7 @@ impl<'de, N: Network> Deserialize<'de> for CoinbaseSolution { match deserializer.is_human_readable() { true => { let mut solutions = serde_json::Value::deserialize(deserializer)?; - Ok(Self::new(DeserializeExt::take_from_value::(&mut solutions, "solutions")?)) + Self::new(DeserializeExt::take_from_value::(&mut solutions, "solutions")?).map_err(de::Error::custom) } false => FromBytesDeserializer::::deserialize_with_size_encoding(deserializer, "solutions"), } @@ -60,7 +60,7 @@ pub(super) mod tests { let partial_solution = PartialSolution::new(address, u64::rand(rng), KZGCommitment(rng.gen())); prover_solutions.push(ProverSolution::new(partial_solution, KZGProof { w: rng.gen(), random_v: None })); } - CoinbaseSolution::new(prover_solutions) + CoinbaseSolution::new(prover_solutions).unwrap() } #[test] diff --git a/ledger/coinbase/src/lib.rs b/ledger/coinbase/src/lib.rs index fd4c5715b6..d07e0109e7 100644 --- a/ledger/coinbase/src/lib.rs +++ b/ledger/coinbase/src/lib.rs @@ -158,28 +158,13 @@ impl CoinbasePuzzle { Ok(ProverSolution::new(partial_solution, proof)) } - /// Returns the coinbase solution for the given prover solutions. - pub fn accumulate( - &self, - prover_solutions: Vec>, - epoch_challenge: &EpochChallenge, - proof_target: u64, - ) -> Result> { - // Initialize the coinbase solution. - let solutions = CoinbaseSolution::new(prover_solutions); - // Verify the solutions. - ensure!(self.verify(&solutions, epoch_challenge, proof_target)?, "The prover solutions must be valid"); - // Return the solutions. - Ok(solutions) - } - /// Returns `true` if the solutions are valid. - pub fn verify( + pub fn check_solutions( &self, solutions: &CoinbaseSolution, epoch_challenge: &EpochChallenge, proof_target: u64, - ) -> Result { + ) -> Result<()> { let timer = timer!("CoinbasePuzzle::verify"); // Ensure the solutions are not empty. @@ -204,12 +189,11 @@ impl CoinbasePuzzle { if !cfg_iter!(solutions).all(|(_, solution)| { solution.verify(self.coinbase_verifying_key(), epoch_challenge, proof_target).unwrap_or(false) }) { - return Ok(false); + bail!("The solutions contain an invalid prover solution"); } finish!(timer, "Verify each solution"); - // Return the verification result. - Ok(true) + Ok(()) } /// Returns the coinbase proving key. diff --git a/ledger/coinbase/src/tests.rs b/ledger/coinbase/src/tests.rs index 2df7e508d0..5c5546ce26 100644 --- a/ledger/coinbase/src/tests.rs +++ b/ledger/coinbase/src/tests.rs @@ -43,11 +43,11 @@ fn test_coinbase_puzzle() { puzzle.prove(&epoch_challenge, address, nonce, None).unwrap() }) .collect::>(); - let full_solution = puzzle.accumulate(solutions, &epoch_challenge, 0).unwrap(); - assert!(puzzle.verify(&full_solution, &epoch_challenge, 0u64).unwrap()); + let full_solution = CoinbaseSolution::new(solutions).unwrap(); + assert!(puzzle.check_solutions(&full_solution, &epoch_challenge, 0u64).is_ok()); let bad_epoch_challenge = EpochChallenge::new(rng.next_u32(), Default::default(), degree).unwrap(); - assert!(!puzzle.verify(&full_solution, &bad_epoch_challenge, 0u64).unwrap()); + assert!(puzzle.check_solutions(&full_solution, &bad_epoch_challenge, 0u64).is_err()); } } } @@ -103,8 +103,8 @@ fn test_edge_case_for_degree() { // Generate a prover solution. let prover_solution = puzzle.prove(&epoch_challenge, address, rng.gen(), None).unwrap(); - let coinbase_solution = puzzle.accumulate(vec![prover_solution], &epoch_challenge, 0).unwrap(); - assert!(puzzle.verify(&coinbase_solution, &epoch_challenge, 0u64).unwrap()); + let coinbase_solution = CoinbaseSolution::new(vec![prover_solution]).unwrap(); + assert!(puzzle.check_solutions(&coinbase_solution, &epoch_challenge, 0u64).is_ok()); } /// Use `cargo test profiler --features timer` to run this test. @@ -141,11 +141,10 @@ fn test_profiler() -> Result<()> { puzzle.prove(&epoch_challenge, address, nonce, None).unwrap() }) .collect::>(); - // Accumulate the solutions. - let solution = puzzle.accumulate(solutions, &epoch_challenge, 0).unwrap(); - - // Verify the solution. - puzzle.verify(&solution, &epoch_challenge, 0u64).unwrap(); + // Construct the solutions. + let solutions = CoinbaseSolution::new(solutions).unwrap(); + // Verify the solutions. + puzzle.check_solutions(&solutions, &epoch_challenge, 0u64).unwrap(); } bail!("\n\nRemember to #[ignore] this test!\n\n") diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index f8c7743080..51cf57e14d 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -123,25 +123,19 @@ impl> Ledger { let (solutions, solutions_root, combined_proof_target) = match candidate_solutions.is_empty() { true => (None, Field::::zero(), 0u128), false => { - // Separate the valid and aborted candidate solutions. + // Retrieve the coinbase verifying key. + let coinbase_verifying_key = self.coinbase_puzzle.coinbase_verifying_key(); + // Retrieve the latest epoch challenge. let latest_epoch_challenge = self.latest_epoch_challenge()?; + // Separate the candidate solutions into valid and aborted solutions. let (valid_candidate_solutions, _aborted_candidate_solutions): (Vec<_>, Vec<_>) = cfg_into_iter!(candidate_solutions).partition(|solution| { solution - .verify( - self.coinbase_puzzle.coinbase_verifying_key(), - &latest_epoch_challenge, - self.latest_proof_target(), - ) + .verify(coinbase_verifying_key, &latest_epoch_challenge, self.latest_proof_target()) .unwrap_or(false) }); - // TODO (raychu86): Optimize this. We are currently double verifying the valid solutions. - // Accumulate the prover solutions. - let solutions = self.coinbase_puzzle.accumulate( - valid_candidate_solutions, - &latest_epoch_challenge, - self.latest_proof_target(), - )?; + // Construct the solutions. + let solutions = CoinbaseSolution::new(valid_candidate_solutions)?; // Compute the solutions root. let solutions_root = solutions.to_accumulator_point()?; // Compute the combined proof target. From 01733bb9bf8bc697e4c47dc8cce001ad3faf222e Mon Sep 17 00:00:00 2001 From: Howard Wu <9260812+howardwu@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:24:26 -0700 Subject: [PATCH 5/5] Add todo --- ledger/src/advance.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ledger/src/advance.rs b/ledger/src/advance.rs index 861a9256cb..de002dd214 100644 --- a/ledger/src/advance.rs +++ b/ledger/src/advance.rs @@ -128,6 +128,7 @@ impl> Ledger { // Retrieve the latest epoch challenge. let latest_epoch_challenge = self.latest_epoch_challenge()?; // Separate the candidate solutions into valid and aborted solutions. + // TODO: Add `aborted_solution_ids` to the block. let (valid_candidate_solutions, _aborted_candidate_solutions): (Vec<_>, Vec<_>) = cfg_into_iter!(candidate_solutions).partition(|solution| { solution