chore(bb): reinstate "chore: uncomment asserts in oink rec verifier"" (…

…#8356) Fixes the base rollup test by making the input proof have the same circuit size, number of public inputs, and pub inputs offset. --------- Co-authored-by: lucasxia01 <[email protected]>
AztecProtocol · Sep 4, 2024 · 4dbad01 · 4dbad01
1 parent 4ee69ac
commit 4dbad01
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 28 deletions.
diff --git a/barretenberg/cpp/src/barretenberg/aztec_ivc/aztec_ivc.test.cpp b/barretenberg/cpp/src/barretenberg/aztec_ivc/aztec_ivc.test.cpp
@@ -151,10 +151,10 @@ TEST_F(AztecIVCTests, BasicFour)
 };
 
 /**
- * @brief Check that the IVC fails to verify if an intermediate fold proof is invalid
+ * @brief Check that the IVC fails if an intermediate fold proof is invalid
  * @details When accumulating 4 circuits, there are 3 fold proofs to verify (the first two are recursively verfied and
- * the 3rd is verified as part of the IVC proof). Check that if any of one of these proofs is invalid, the IVC will fail
- * to verify.
+ * the 3rd is verified as part of the IVC proof). Check that if any of one of these proofs is invalid, the IVC will
+ * fail.
  *
  */
 TEST_F(AztecIVCTests, BadProofFailure)
@@ -175,7 +175,7 @@ TEST_F(AztecIVCTests, BadProofFailure)
         EXPECT_TRUE(ivc.prove_and_verify());
     }
 
-    // The IVC fails to verify if the FIRST fold proof is tampered with
+    // The IVC throws an exception if the FIRST fold proof is tampered with
     {
         AztecIVC ivc;
         ivc.trace_structure = TraceStructure::SMALL_TEST;
@@ -185,6 +185,11 @@ TEST_F(AztecIVCTests, BadProofFailure)
         // Construct and accumulate a set of mocked private function execution circuits
         size_t NUM_CIRCUITS = 4;
         for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
+            if (idx == 3) { // At idx = 3, we've tampered with the one of the folding proofs so create the recursive
+                            // folding verifier will throw an error.
+                EXPECT_ANY_THROW(circuit_producer.create_next_circuit(ivc, /*log2_num_gates=*/5));
+                break;
+            }
             auto circuit = circuit_producer.create_next_circuit(ivc, /*log2_num_gates=*/5);
             ivc.accumulate(circuit);
 
@@ -193,11 +198,9 @@ TEST_F(AztecIVCTests, BadProofFailure)
                 tamper_with_proof(ivc.verification_queue[0].proof); // tamper with first proof
             }
         }
-
-        EXPECT_FALSE(ivc.prove_and_verify());
     }
 
-    // The IVC fails to verify if the SECOND fold proof is tampered with
+    // The IVC fails if the SECOND fold proof is tampered with
     {
         AztecIVC ivc;
         ivc.trace_structure = TraceStructure::SMALL_TEST;
@@ -207,6 +210,11 @@ TEST_F(AztecIVCTests, BadProofFailure)
         // Construct and accumulate a set of mocked private function execution circuits
         size_t NUM_CIRCUITS = 4;
         for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
+            if (idx == 3) { // At idx = 3, we've tampered with the one of the folding proofs so create the recursive
+                            // folding verifier will throw an error.
+                EXPECT_ANY_THROW(circuit_producer.create_next_circuit(ivc, /*log2_num_gates=*/5));
+                break;
+            }
             auto circuit = circuit_producer.create_next_circuit(ivc, /*log2_num_gates=*/5);
             ivc.accumulate(circuit);
 
@@ -215,11 +223,9 @@ TEST_F(AztecIVCTests, BadProofFailure)
                 tamper_with_proof(ivc.verification_queue[1].proof); // tamper with second proof
             }
         }
-
-        EXPECT_FALSE(ivc.prove_and_verify());
     }
 
-    // The IVC fails to verify if the 3rd/FINAL fold proof is tampered with
+    // The IVC fails if the 3rd/FINAL fold proof is tampered with
     {
         AztecIVC ivc;
         ivc.trace_structure = TraceStructure::SMALL_TEST;

diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp
@@ -106,7 +106,7 @@ TEST_F(ClientIVCTests, BasicThree)
 };
 
 /**
- * @brief Check that the IVC fails to verify if an intermediate fold proof is invalid
+ * @brief Check that the IVC fails if an intermediate fold proof is invalid
  *
  */
 TEST_F(ClientIVCTests, BasicFailure)
@@ -128,13 +128,10 @@ TEST_F(ClientIVCTests, BasicFailure)
             break;
         }
     }
-
-    // Accumulate another circuit; this involves recursive folding verification of the bad proof
+    // Accumulate another circuit; this involves recursive folding verification of the bad proof which throws an error
+    // because of circuit sizes don't match.
     Builder circuit_2 = create_mock_circuit(ivc);
-    ivc.accumulate(circuit_2);
-
-    // The bad fold proof should result in an invalid witness in the final circuit and the IVC should fail to verify
-    EXPECT_FALSE(prove_and_verify(ivc));
+    EXPECT_ANY_THROW(ivc.accumulate(circuit_2));
 };
 
 /**

diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/oink_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/oink_recursive_verifier.cpp
@@ -41,14 +41,18 @@ template <typename Flavor> void OinkRecursiveVerifier_<Flavor>::verify()
     CommitmentLabels labels;
 
     FF circuit_size = transcript->template receive_from_prover<FF>(domain_separator + "circuit_size");
-    transcript->template receive_from_prover<FF>(domain_separator + "public_input_size");
-    transcript->template receive_from_prover<FF>(domain_separator + "pub_inputs_offset");
-
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1032): Uncomment these once it doesn't cause issues
-    // with the flows
-    // ASSERT(static_cast<uint32_t>(circuit_size.get_value()) == key->circuit_size);
-    // ASSERT(static_cast<uint32_t>(public_input_size.get_value()) == key->num_public_inputs);
-    // ASSERT(static_cast<uint32_t>(pub_inputs_offset.get_value()) == key->pub_inputs_offset);
+    FF public_input_size = transcript->template receive_from_prover<FF>(domain_separator + "public_input_size");
+    FF pub_inputs_offset = transcript->template receive_from_prover<FF>(domain_separator + "pub_inputs_offset");
+
+    if (static_cast<uint32_t>(circuit_size.get_value()) != instance->verification_key->circuit_size) {
+        throw_or_abort("OinkRecursiveVerifier::verify: proof circuit size does not match verification key");
+    }
+    if (static_cast<uint32_t>(public_input_size.get_value()) != instance->verification_key->num_public_inputs) {
+        throw_or_abort("OinkRecursiveVerifier::verify: proof public input size does not match verification key");
+    }
+    if (static_cast<uint32_t>(pub_inputs_offset.get_value()) != instance->verification_key->pub_inputs_offset) {
+        throw_or_abort("OinkRecursiveVerifier::verify: proof public input offset does not match verification key");
+    }
 
     std::vector<FF> public_inputs;
     for (size_t i = 0; i < instance->verification_key->num_public_inputs; ++i) {

diff --git a/yarn-project/README.md b/yarn-project/README.md
@@ -86,6 +86,6 @@ COMMIT_TAG=<RELEASE_TAG_NUMBER_YOU_WANT e.g. aztec-packages-v0.8.8>
     - Extract `VERSION` as the script shows (in the eg it should be 0.8.8)
     - Skip the version existing checks like `if [ "$VERSION" == "$PUBLISHED_VERSION" ]` and `if [ "$VERSION" != "$HIGHER_VERSION" ]`. Since this is our first time deploying the package, `PUBLISHED_VERSION` and `HIGHER_VERSION` will be empty and hence these checks would fail. These checks are necessary in the CI for continual releases.
     - Locally update the package version in package.json using `jq` as shown in the script.
-    - Do a dry-run
+    - Do a dry-run.
     - If dry run succeeds, publish the package!
 5. Create a PR by adding your package into the `deploy-npm` script so next release onwards, CI can cut releases for your package.
diff --git a/yarn-project/circuits.js/src/structs/verification_key.ts b/yarn-project/circuits.js/src/structs/verification_key.ts
@@ -79,7 +79,7 @@ export class CommitmentMap {
 // TODO: find better home for these constants
 export const CIRCUIT_SIZE_INDEX = 0;
 export const CIRCUIT_PUBLIC_INPUTS_INDEX = 1;
-export const CIRCUIT_RECURSIVE_INDEX = 0;
+export const CIRCUIT_RECURSIVE_INDEX = 3;
 
 /**
  * Provides a 'fields' representation of a circuit's verification key

diff --git a/yarn-project/prover-client/src/test/bb_prover_base_rollup.test.ts b/yarn-project/prover-client/src/test/bb_prover_base_rollup.test.ts
@@ -41,9 +41,13 @@ describe('prover/bb_prover/base-rollup', () => {
     const tx = makePaddingProcessedTxFromTubeProof(paddingTxPublicInputsAndProof);
 
     logger.verbose('Building base rollup inputs');
+    const baseRollupInputProof = makeEmptyRecursiveProof(NESTED_RECURSIVE_PROOF_LENGTH);
+    baseRollupInputProof.proof[0] = paddingTxPublicInputsAndProof.verificationKey.keyAsFields.key[0];
+    baseRollupInputProof.proof[1] = paddingTxPublicInputsAndProof.verificationKey.keyAsFields.key[1];
+    baseRollupInputProof.proof[2] = paddingTxPublicInputsAndProof.verificationKey.keyAsFields.key[2];
     const baseRollupInputs = await buildBaseRollupInput(
       tx,
-      makeEmptyRecursiveProof(NESTED_RECURSIVE_PROOF_LENGTH),
+      baseRollupInputProof,
       context.globalVariables,
       context.actualDb,
       paddingTxPublicInputsAndProof.verificationKey,