From 6d2a29f42c782193952eee48aed11be3468be290 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 27 Apr 2023 19:58:45 -0300 Subject: [PATCH] Check that public data tree root follows from left to right rollup (#396) * Check that public data tree root follows from left to right rollup * Fix rollup circuits wasm test --- .../circuits/rollup/components/components.cpp | 3 ++ .../aztec3/circuits/rollup/merge/.test.cpp | 6 ++++ .../src/aztec3/circuits/rollup/root/.test.cpp | 16 +++++++--- .../cpp/src/aztec3/utils/circuit_errors.hpp | 3 +- .../src/rollup/rollup_wasm_wrapper.test.ts | 30 ++++++++++--------- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp b/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp index e4809910f46..5de96075bf1 100644 --- a/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp +++ b/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp @@ -132,6 +132,9 @@ void assert_prev_rollups_follow_on_from_each_other(DummyComposer& composer, composer.do_assert(left.end_contract_tree_snapshot == right.start_contract_tree_snapshot, "input proofs have different contract tree snapshots", utils::CircuitErrorCode::CONTRACT_TREE_SNAPSHOT_MISMATCH); + composer.do_assert(left.end_public_data_tree_snapshot == right.start_public_data_tree_snapshot, + "input proofs have different public data tree snapshots", + utils::CircuitErrorCode::CONTRACT_TREE_SNAPSHOT_MISMATCH); } } // namespace aztec3::circuits::rollup::components \ No newline at end of file diff --git a/circuits/cpp/src/aztec3/circuits/rollup/merge/.test.cpp b/circuits/cpp/src/aztec3/circuits/rollup/merge/.test.cpp index 8a3fa7a4208..21f42262ead 100644 --- a/circuits/cpp/src/aztec3/circuits/rollup/merge/.test.cpp +++ b/circuits/cpp/src/aztec3/circuits/rollup/merge/.test.cpp @@ -209,6 +209,12 @@ TEST_F(merge_rollup_tests, native_start_and_end_snapshots) inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.start_contract_tree_snapshot); ASSERT_EQ(outputs.end_contract_tree_snapshot, inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.end_contract_tree_snapshot); + + ASSERT_EQ(outputs.start_public_data_tree_snapshot, + inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.start_public_data_tree_snapshot); + ASSERT_EQ(outputs.end_public_data_tree_snapshot, + inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.end_public_data_tree_snapshot); + ASSERT_FALSE(composer.failed()); } diff --git a/circuits/cpp/src/aztec3/circuits/rollup/root/.test.cpp b/circuits/cpp/src/aztec3/circuits/rollup/root/.test.cpp index 7f0f3eaaa98..39cdede32e6 100644 --- a/circuits/cpp/src/aztec3/circuits/rollup/root/.test.cpp +++ b/circuits/cpp/src/aztec3/circuits/rollup/root/.test.cpp @@ -275,16 +275,24 @@ TEST_F(root_rollup_tests, native_root_missing_nullifier_logic) RootRollupPublicInputs outputs = aztec3::circuits::rollup::native_root_rollup::root_rollup_circuit(composer, rootRollupInputs); - // Check data trees + // Check private data trees ASSERT_EQ( outputs.start_private_data_tree_snapshot, rootRollupInputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.start_private_data_tree_snapshot); ASSERT_EQ( outputs.end_private_data_tree_snapshot, rootRollupInputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.end_private_data_tree_snapshot); - AppendOnlyTreeSnapshot expected_data_tree_snapshot = { .root = data_tree.root(), - .next_available_leaf_index = 16 }; - ASSERT_EQ(outputs.end_private_data_tree_snapshot, expected_data_tree_snapshot); + AppendOnlyTreeSnapshot expected_private_data_tree_snapshot = { .root = data_tree.root(), + .next_available_leaf_index = 16 }; + ASSERT_EQ(outputs.end_private_data_tree_snapshot, expected_private_data_tree_snapshot); + + // Check public data trees + ASSERT_EQ( + outputs.start_public_data_tree_snapshot, + rootRollupInputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.start_public_data_tree_snapshot); + ASSERT_EQ( + outputs.end_public_data_tree_snapshot, + rootRollupInputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.end_public_data_tree_snapshot); // check contract trees ASSERT_EQ(outputs.start_contract_tree_snapshot, diff --git a/circuits/cpp/src/aztec3/utils/circuit_errors.hpp b/circuits/cpp/src/aztec3/utils/circuit_errors.hpp index 43e8e9b1a25..1d40cad0a95 100644 --- a/circuits/cpp/src/aztec3/utils/circuit_errors.hpp +++ b/circuits/cpp/src/aztec3/utils/circuit_errors.hpp @@ -60,7 +60,8 @@ enum CircuitErrorCode : uint16_t { PRIVATE_DATA_TREE_SNAPSHOT_MISMATCH = 7004, NULLIFIER_TREE_SNAPSHOT_MISMATCH = 7005, CONTRACT_TREE_SNAPSHOT_MISMATCH = 7006, - MEMBERSHIP_CHECK_FAILED = 7007, + PUBLIC_DATA_TREE_SNAPSHOT_MISMATCH = 7007, + MEMBERSHIP_CHECK_FAILED = 7008, ROOT_CIRCUIT_FAILED = 8000, }; diff --git a/yarn-project/circuits.js/src/rollup/rollup_wasm_wrapper.test.ts b/yarn-project/circuits.js/src/rollup/rollup_wasm_wrapper.test.ts index 40688267e95..53d24acee63 100644 --- a/yarn-project/circuits.js/src/rollup/rollup_wasm_wrapper.test.ts +++ b/yarn-project/circuits.js/src/rollup/rollup_wasm_wrapper.test.ts @@ -1,11 +1,8 @@ -import { AggregationObject, CircuitError, VerificationKey } from '../index.js'; +import { AggregationObject, CircuitError, MergeRollupInputs, RootRollupInputs, VerificationKey } from '../index.js'; import { makeBaseRollupInputs, makeMergeRollupInputs, makeRootRollupInputs } from '../tests/factories.js'; import { CircuitsWasm } from '../wasm/circuits_wasm.js'; import { RollupWasmWrapper } from './rollup_wasm_wrapper.js'; -// TODO: All these tests are currently failing with segfaults. -// Note that base and root rollup sim are called ok from the circuit_powered_block_builder, -// so the problem must be with an invalid input we're providing. describe('rollup/rollup_wasm_wrapper', () => { let wasm: CircuitsWasm; let rollupWasm: RollupWasmWrapper; @@ -25,13 +22,7 @@ describe('rollup/rollup_wasm_wrapper', () => { return input; }; - const makeMergeRollupInputsForCircuit = () => { - const input = makeMergeRollupInputs(); - for (const previousData of input.previousRollupData) { - previousData.vk = VerificationKey.makeFake(); - previousData.publicInputs.endAggregationObject = AggregationObject.makeFake(); - } - // fix inputs to make it compatible with the merge circuit requirements + const fixPreviousRollupInputs = (input: MergeRollupInputs | RootRollupInputs) => { input.previousRollupData[1].publicInputs.constants = input.previousRollupData[0].publicInputs.constants; input.previousRollupData[1].publicInputs.startPrivateDataTreeSnapshot = input.previousRollupData[0].publicInputs.endPrivateDataTreeSnapshot; @@ -39,6 +30,17 @@ describe('rollup/rollup_wasm_wrapper', () => { input.previousRollupData[0].publicInputs.endNullifierTreeSnapshot; input.previousRollupData[1].publicInputs.startContractTreeSnapshot = input.previousRollupData[0].publicInputs.endContractTreeSnapshot; + input.previousRollupData[1].publicInputs.startPublicDataTreeSnapshot = + input.previousRollupData[0].publicInputs.endPublicDataTreeSnapshot; + }; + + const makeMergeRollupInputsForCircuit = () => { + const input = makeMergeRollupInputs(); + for (const previousData of input.previousRollupData) { + previousData.vk = VerificationKey.makeFake(); + previousData.publicInputs.endAggregationObject = AggregationObject.makeFake(); + } + fixPreviousRollupInputs(input); return input; }; @@ -85,18 +87,18 @@ describe('rollup/rollup_wasm_wrapper', () => { } }); - it.skip('calls root_rollup__sim', async () => { + it('calls root_rollup__sim', async () => { const input = makeRootRollupInputs(); - for (const rd of input.previousRollupData) { rd.vk = VerificationKey.makeFake(); rd.publicInputs.endAggregationObject = AggregationObject.makeFake(); rd.publicInputs = await rollupWasm.simulateBaseRollup(makeBaseRollupInputsForCircuit()); } + fixPreviousRollupInputs(input); const output = await rollupWasm.simulateRootRollup(input); expect(output.startNullifierTreeSnapshot).toEqual( input.previousRollupData[0].publicInputs.startNullifierTreeSnapshot, ); - }); + }, 15_000); });