From cec53373b69dad05ff9b069197ef22713ae9bf82 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Tue, 14 Jan 2025 17:32:20 +0000 Subject: [PATCH 1/4] fix: resolve misc bugs handling phases in avm witgen --- .../barretenberg/vm/avm/trace/execution.cpp | 32 ++++++++++++++++--- .../barretenberg/vm/avm/trace/execution.hpp | 3 +- .../src/barretenberg/vm/avm/trace/trace.cpp | 16 ++++++++++ .../src/barretenberg/vm/avm/trace/trace.hpp | 1 + 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index cbd6888107a..bf0362f8cd5 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -403,6 +403,7 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, // Loop over all the public call requests auto const phases = { TxExecutionPhase::SETUP, TxExecutionPhase::APP_LOGIC, TxExecutionPhase::TEARDOWN }; + size_t enqueued_call_hint_index = 0; for (auto phase : phases) { const auto public_call_requests = phase == TxExecutionPhase::SETUP ? setup_call_requests : phase == TxExecutionPhase::APP_LOGIC ? app_logic_call_requests @@ -447,11 +448,11 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, auto public_call_request = public_call_requests.at(i); trace_builder.set_public_call_request(public_call_request); // At the start of each enqueued call, we read the enqueued call hints - auto enqueued_call_hint = execution_hints.enqueued_call_hints.at(i); + auto enqueued_call_hint = execution_hints.enqueued_call_hints.at(enqueued_call_hint_index++); ASSERT(public_call_request.contract_address == enqueued_call_hint.contract_address); // Execute! - phase_error = - Execution::execute_enqueued_call(trace_builder, enqueued_call_hint, returndata, apply_e2e_assertions); + phase_error = Execution::execute_enqueued_call( + phase, trace_builder, enqueued_call_hint, returndata, apply_e2e_assertions); if (!is_ok(phase_error)) { info("Phase ", to_name(phase), " reverted."); @@ -468,6 +469,13 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, throw std::runtime_error("A revert was encountered in the SETUP phase, killing the entire TX"); break; } + vinfo("Ended phase ", + to_name(phase), + " with ", + trace_builder.get_l2_gas_left(), + " L2 gas left and ", + trace_builder.get_da_gas_left(), + " DA gas left"); } if (apply_e2e_assertions) { @@ -493,13 +501,18 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, * @returns the error/result of the enqueued call * */ -AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder, +AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, + AvmTraceBuilder& trace_builder, AvmEnqueuedCallHint& enqueued_call_hint, std::vector& returndata, bool check_bytecode_membership) { AvmError error = AvmError::NO_ERROR; + // save gas before phase for use after teardown (teardown shouldn't affect end gas) + const auto l2_gas_left_before_enqueued_call = trace_builder.get_l2_gas_left(); + const auto da_gas_left_before_enqueued_call = trace_builder.get_da_gas_left(); + // These hints help us to set up first call ctx auto context_id = trace_builder.next_context_id; uint32_t l2_gas_allocated_to_enqueued_call = trace_builder.get_l2_gas_left(); @@ -1124,7 +1137,12 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder, " IC: ", error_ic); - trace_builder.handle_exceptional_halt(); + // For nested calls or non-teardown-enqueued-calls, handle the exceptional halt + // (consume all gas). Don't do it for teardown enqueued call because gas needs + // to be reset after teardown (teardown doesn't count towards end-gas). + if (!is_top_level || phase != TxExecutionPhase::TEARDOWN) { + trace_builder.handle_exceptional_halt(); + } if (is_top_level) { break; @@ -1140,6 +1158,10 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder, error = AvmError::NO_ERROR; } } + if (phase == TxExecutionPhase::TEARDOWN) { + // todo does this work on exceptional halt too? + trace_builder.handle_end_of_teardown(l2_gas_left_before_enqueued_call, da_gas_left_before_enqueued_call); + } return error; } diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp index 288535364fd..3ae4ba0b78e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp @@ -40,7 +40,8 @@ class Execution { ExecutionHints const& execution_hints, bool apply_e2e_assertions = false); - static AvmError execute_enqueued_call(AvmTraceBuilder& trace_builder, + static AvmError execute_enqueued_call(TxExecutionPhase phase, + AvmTraceBuilder& trace_builder, AvmEnqueuedCallHint& enqueued_call_hint, std::vector& returndata, bool check_bytecode_membership); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 346ac3b059d..f8d50d896c9 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -429,6 +429,22 @@ void AvmTraceBuilder::handle_exceptional_halt() } } +void AvmTraceBuilder::handle_end_of_teardown(uint32_t pre_teardown_l2_gas_left, uint32_t pre_teardown_da_gas_left) +{ + vinfo("Handling end of teardown"); + + // modify the last row of the gas trace to reset back to pre-teardown gas + // since gas used by teardown doesn't contribute to end-gas + gas_trace_builder.constrain_gas_for_halt(/*exceptional_halt=*/true, // not really an exceptional halt + pre_teardown_l2_gas_left, + pre_teardown_da_gas_left, + /*l2_gas_allocated_to_nested_call=*/0, + /*da_gas_allocated_to_nested_call=*/0); + + // max out the pc to signify "done" + pc = UINT32_MAX; +} + /** * @brief Loads a value from memory into a given intermediate register at a specified clock cycle. * Handles both direct and indirect memory access. diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp index a2c750d633e..91d417142df 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp @@ -243,6 +243,7 @@ class AvmTraceBuilder { void pad_trees(); void allocate_gas_for_call(uint32_t l2_gas, uint32_t da_gas); void handle_exceptional_halt(); + void handle_end_of_teardown(uint32_t pre_teardown_l2_gas_left, uint32_t pre_teardown_da_gas_left); // These are used for testing only. AvmTraceBuilder& set_range_check_required(bool required) From 5a39dcb0b374479e170a954c280fbd0867133bcb Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Tue, 14 Jan 2025 18:28:24 +0000 Subject: [PATCH 2/4] fix test fixture --- .../simulator/src/public/fixtures/index.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/yarn-project/simulator/src/public/fixtures/index.ts b/yarn-project/simulator/src/public/fixtures/index.ts index e47f37df6de..f87b4fb36b7 100644 --- a/yarn-project/simulator/src/public/fixtures/index.ts +++ b/yarn-project/simulator/src/public/fixtures/index.ts @@ -79,16 +79,14 @@ export async function simulateAvmTestContractGenerateCircuitInputs( /*isStaticCall=*/ false, ); const setupExecutionRequests: PublicExecutionRequest[] = []; - // we reverse order because the simulator expects it to be like a "stack" of calls to pop from - for (let i = setupFunctionNames.length - 1; i >= 0; i--) { + for (let i = 0; i < setupFunctionNames.length; i++) { const functionSelector = getAvmTestContractFunctionSelector(setupFunctionNames[i]); const fnArgs = [functionSelector.toField(), ...setupArgs[i]]; const executionRequest = new PublicExecutionRequest(callContext, fnArgs); setupExecutionRequests.push(executionRequest); } const appExecutionRequests: PublicExecutionRequest[] = []; - // we reverse order because the simulator expects it to be like a "stack" of calls to pop from - for (let i = appFunctionNames.length - 1; i >= 0; i--) { + for (let i = 0; i < appFunctionNames.length; i++) { const functionSelector = getAvmTestContractFunctionSelector(appFunctionNames[i]); const fnArgs = [functionSelector.toField(), ...appArgs[i]]; const executionRequest = new PublicExecutionRequest(callContext, fnArgs); @@ -183,10 +181,11 @@ export function createTxForPublicCalls( // TODO(#9269): Remove this fake nullifier method as we move away from 1st nullifier as hash. forPublic.nonRevertibleAccumulatedData.nullifiers[0] = Fr.random(); // fake tx nullifier - for (let i = 0; i < setupExecutionRequests.length; i++) { + // We reverse order because the simulator expects it to be like a "stack" of calls to pop from + for (let i = setupCallRequests.length - 1; i >= 0; i--) { forPublic.nonRevertibleAccumulatedData.publicCallRequests[i] = setupCallRequests[i]; } - for (let i = 0; i < appCallRequests.length; i++) { + for (let i = appCallRequests.length - 1; i >= 0; i--) { forPublic.revertibleAccumulatedData.publicCallRequests[i] = appCallRequests[i]; } if (teardownExecutionRequest) { @@ -207,12 +206,14 @@ export function createTxForPublicCalls( ); const tx = Tx.newWithTxData(txData, teardownExecutionRequest); - for (let i = 0; i < setupExecutionRequests.length; i++) { - tx.enqueuedPublicFunctionCalls.push(setupExecutionRequests[i]); - } - for (let i = 0; i < appExecutionRequests.length; i++) { + // Reverse order because the simulator expects it to be like a "stack" of calls to pop from. + // Also push app calls before setup calls for this reason. + for (let i = appExecutionRequests.length - 1; i >= 0; i--) { tx.enqueuedPublicFunctionCalls.push(appExecutionRequests[i]); } + for (let i = setupExecutionRequests.length - 1; i >= 0; i--) { + tx.enqueuedPublicFunctionCalls.push(setupExecutionRequests[i]); + } return tx; } From f4d03868b26c3fb1131ad9251ec1e5bc4d219d67 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Tue, 14 Jan 2025 19:07:12 +0000 Subject: [PATCH 3/4] fix --- .../src/barretenberg/vm/avm/trace/execution.cpp | 7 +++++-- .../src/barretenberg/vm/avm/trace/gas_trace.cpp | 4 ++-- yarn-project/bb-prover/src/avm_proving.test.ts | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index bf0362f8cd5..b9e291f351f 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -546,7 +546,11 @@ AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, // This error was encountered before any opcodes were executed, but // we need at least one row in the execution trace to then mutate and say "it halted and consumed all gas!" trace_builder.op_add(0, 0, 0, 0, OpCode::ADD_8); - trace_builder.handle_exceptional_halt(); + if (phase == TxExecutionPhase::TEARDOWN) { + trace_builder.handle_end_of_teardown(l2_gas_left_before_enqueued_call, da_gas_left_before_enqueued_call); + } else { + trace_builder.handle_exceptional_halt(); + } return AvmError::FAILED_BYTECODE_RETRIEVAL; } @@ -1159,7 +1163,6 @@ AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, } } if (phase == TxExecutionPhase::TEARDOWN) { - // todo does this work on exceptional halt too? trace_builder.handle_end_of_teardown(l2_gas_left_before_enqueued_call, da_gas_left_before_enqueued_call); } return error; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gas_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gas_trace.cpp index c6af50ad5d8..c1d1f2e0bdf 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gas_trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gas_trace.cpp @@ -124,8 +124,8 @@ void AvmGasTraceBuilder::constrain_gas_for_halt(bool exceptional_halt, uint32_t l2_gas_allocated_to_nested_call, uint32_t da_gas_allocated_to_nested_call) { - debug("Resetting to parent's L2 gas left (", parent_l2_gas_left, ") before consuming gas allocated to nested call"); - debug("Resetting to parent's DA gas left (", parent_da_gas_left, ") before consuming gas allocated to nested call"); + debug("Resetting to parent's L2 gas left (", parent_l2_gas_left, ") before consuming gas used by nested call"); + debug("Resetting to parent's DA gas left (", parent_da_gas_left, ") before consuming gas used by nested call"); // how much gas did the nested call consume auto l2_gas_consumed = l2_gas_allocated_to_nested_call - remaining_l2_gas; auto da_gas_consumed = da_gas_allocated_to_nested_call - remaining_da_gas; diff --git a/yarn-project/bb-prover/src/avm_proving.test.ts b/yarn-project/bb-prover/src/avm_proving.test.ts index 5119af27f2a..fe025d4ddb6 100644 --- a/yarn-project/bb-prover/src/avm_proving.test.ts +++ b/yarn-project/bb-prover/src/avm_proving.test.ts @@ -261,6 +261,22 @@ describe('AVM WitGen, proof generation and verification', () => { }, TIMEOUT, ); + it( + 'Should prove and verify a TX that reverts in teardown', + async () => { + await proveAndVerifyAvmTestContract( + /*checkCircuitOnly=*/ true, + /*setupFunctionNames=*/ [], + /*setupArgs=*/ [], + /*appFunctionNames=*/ [], + /*appArgs=*/ [], + /*teardownFunctionName=*/ 'read_assert_storage_single', + /*teardownArgs=*/ [new Fr(10)], + /*expectRevert=*/ true, + ); + }, + TIMEOUT, + ); }); /** From f8a0cf325074e311b851be673a1d055eeadc7c3e Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Wed, 15 Jan 2025 22:10:02 +0000 Subject: [PATCH 4/4] fixes --- .../barretenberg/vm/avm/trace/execution.cpp | 33 +++-- .../barretenberg/vm/avm/trace/execution.hpp | 3 +- .../bb-prover/src/avm_proving.test.ts | 123 ++++++++++-------- 3 files changed, 93 insertions(+), 66 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index e74ec0fccab..0564e744390 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -451,8 +451,12 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, auto enqueued_call_hint = execution_hints.enqueued_call_hints.at(enqueued_call_hint_index++); ASSERT(public_call_request.contract_address == enqueued_call_hint.contract_address); // Execute! - phase_error = Execution::execute_enqueued_call( - phase, trace_builder, enqueued_call_hint, returndata, apply_e2e_assertions); + phase_error = Execution::execute_enqueued_call(phase, + trace_builder, + enqueued_call_hint, + public_inputs.gas_settings.teardown_gas_limits, + returndata, + apply_e2e_assertions); if (!is_ok(phase_error)) { info("Phase ", to_name(phase), " reverted."); @@ -501,9 +505,10 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, * @returns the error/result of the enqueued call * */ -AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, +AvmError Execution::execute_enqueued_call(TxExecutionPhase& phase, AvmTraceBuilder& trace_builder, AvmEnqueuedCallHint& enqueued_call_hint, + Gas const teardown_gas_limits, std::vector& returndata, bool check_bytecode_membership) { @@ -513,10 +518,18 @@ AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, const auto l2_gas_left_before_enqueued_call = trace_builder.get_l2_gas_left(); const auto da_gas_left_before_enqueued_call = trace_builder.get_da_gas_left(); + // TODO(dbanks12): use this below for teardown instead of raw limits.l2_gas + // auto const teardown_allocated_l2_gas = std::min( + // teardown_gas_limits.l2_gas, + // static_cast(MAX_L2_GAS_PER_TX_PUBLIC_PORTION)); + // These hints help us to set up first call ctx auto context_id = trace_builder.next_context_id; - uint32_t l2_gas_allocated_to_enqueued_call = trace_builder.get_l2_gas_left(); - uint32_t da_gas_allocated_to_enqueued_call = trace_builder.get_da_gas_left(); + uint32_t l2_gas_allocated_to_enqueued_call = + phase == TxExecutionPhase::TEARDOWN ? teardown_gas_limits.l2_gas : l2_gas_left_before_enqueued_call; + uint32_t da_gas_allocated_to_enqueued_call = + phase == TxExecutionPhase::TEARDOWN ? teardown_gas_limits.da_gas : da_gas_left_before_enqueued_call; + ; trace_builder.current_ext_call_ctx = AvmTraceBuilder::ExtCallCtx{ .context_id = context_id, .parent_id = 0, @@ -526,10 +539,10 @@ AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, .nested_returndata = {}, .last_pc = 0, .success_offset = 0, - .start_l2_gas_left = l2_gas_allocated_to_enqueued_call, - .start_da_gas_left = da_gas_allocated_to_enqueued_call, - .l2_gas_left = l2_gas_allocated_to_enqueued_call, - .da_gas_left = da_gas_allocated_to_enqueued_call, + .start_l2_gas_left = l2_gas_left_before_enqueued_call, + .start_da_gas_left = da_gas_left_before_enqueued_call, + .l2_gas_left = l2_gas_left_before_enqueued_call, + .da_gas_left = da_gas_left_before_enqueued_call, .internal_return_ptr_stack = {}, }; trace_builder.next_context_id++; @@ -1143,7 +1156,7 @@ AvmError Execution::execute_enqueued_call(TxExecutionPhase phase, error_ic); // For nested calls or non-teardown-enqueued-calls, handle the exceptional halt - // (consume all gas). Don't do it for teardown enqueued call because gas needs + // (consume all gas). Don't do it for teardown enqueued call because gas needs // to be reset after teardown (teardown doesn't count towards end-gas). if (!is_top_level || phase != TxExecutionPhase::TEARDOWN) { trace_builder.handle_exceptional_halt(); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp index 3ae4ba0b78e..0c0e0e50a8c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp @@ -40,9 +40,10 @@ class Execution { ExecutionHints const& execution_hints, bool apply_e2e_assertions = false); - static AvmError execute_enqueued_call(TxExecutionPhase phase, + static AvmError execute_enqueued_call(TxExecutionPhase& phase, AvmTraceBuilder& trace_builder, AvmEnqueuedCallHint& enqueued_call_hint, + Gas const teardown_gas_limits, std::vector& returndata, bool check_bytecode_membership); diff --git a/yarn-project/bb-prover/src/avm_proving.test.ts b/yarn-project/bb-prover/src/avm_proving.test.ts index 1fa369bf9e8..feb7581be9e 100644 --- a/yarn-project/bb-prover/src/avm_proving.test.ts +++ b/yarn-project/bb-prover/src/avm_proving.test.ts @@ -30,9 +30,9 @@ import { extractAvmVkData } from './verification_key/verification_key_data.js'; const TIMEOUT = 300_000; -describe('AVM WitGen, proof generation and verification', () => { +describe('AVM WitGen, proof generation and verification tests', () => { it( - 'Should prove and verify bulk_testing v1', + 'bulk_testing v1', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ false, // full proving & verifying @@ -42,8 +42,10 @@ describe('AVM WitGen, proof generation and verification', () => { }, TIMEOUT, ); +}); +describe('AVM WitGen, "check circuit" tests', () => { it( - 'Should prove and verify test that performs too many storage writes and reverts', + 'perform too many storage writes and revert', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick @@ -55,7 +57,7 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should run check circuit for test that creates too many note hashes and reverts', + 'create too many note hashes and revert', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick @@ -67,7 +69,7 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify test that creates too many nullifiers and reverts', + 'create too many nullifiers and revert', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick @@ -79,7 +81,7 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify test that creates too many l2tol1 messages and reverts', + 'create too many l2tol1 messages and revert', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick @@ -91,7 +93,7 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify test that creates too many unencrypted logs and reverts', + 'create too many unencrypted logs and revert', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick @@ -103,7 +105,7 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify test that calls the max number of unique contract classes', + 'call the max number of unique contract classes', async () => { const contractDataSource = new MockedAvmTestContractDataSource(); // args is initialized to MAX_PUBLIC_CALLS_TO_UNIQUE_CONTRACT_CLASS_IDS contract addresses with unique class IDs @@ -126,7 +128,7 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify test that attempts too many calls to unique contract class ids', + 'attempt too many calls to unique contract class ids', async () => { const contractDataSource = new MockedAvmTestContractDataSource(); // args is initialized to MAX_PUBLIC_CALLS_TO_UNIQUE_CONTRACT_CLASS_IDS+1 contract addresses with unique class IDs @@ -148,47 +150,59 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify a top-level exceptional halt', + 'top-level exceptional halts in both app logic and teardown', async () => { - await proveAndVerifyAvmTestContractSimple( - /*checkCircuitOnly=*/ true, // quick - 'divide_by_zero', - /*args=*/ [], + await proveAndVerifyAvmTestContract( + /*checkCircuitOnly=*/ true, + /*setupFunctionNames=*/ [], + /*setupArgs=*/ [], + /*appFunctionNames=*/ ['divide_by_zero'], + /*appArgs=*/ [[]], + /*teardownFunctionName=*/ 'divide_by_zero', + /*teardownArgs=*/ [], /*expectRevert=*/ true, ); }, TIMEOUT, ); it( - 'Should prove and verify a nested exceptional halt that propagates to top-level', + 'top-level exceptional halt in app logic, but teardown succeeds', async () => { - await proveAndVerifyAvmTestContractSimple( - /*checkCircuitOnly=*/ true, // quick - 'external_call_to_divide_by_zero', - /*args=*/ [], + await proveAndVerifyAvmTestContract( + /*checkCircuitOnly=*/ true, + /*setupFunctionNames=*/ [], + /*setupArgs=*/ [], + /*appFunctionNames=*/ ['divide_by_zero'], + /*appArgs=*/ [[]], + /*teardownFunctionName=*/ 'add_args_return', + /*teardownArgs=*/ [new Fr(1), new Fr(2)], /*expectRevert=*/ true, ); }, TIMEOUT, ); it( - 'Should prove and verify a nested exceptional halt that is recovered from in caller', + 'top-level exceptional halt in teardown, but app logic succeeds', async () => { - await proveAndVerifyAvmTestContractSimple( - /*checkCircuitOnly=*/ true, // quick - 'external_call_to_divide_by_zero_recovers', - /*args=*/ [], - /*expectRevert=*/ false, + await proveAndVerifyAvmTestContract( + /*checkCircuitOnly=*/ true, + /*setupFunctionNames=*/ [], + /*setupArgs=*/ [], + /*appFunctionNames=*/ ['add_args_return'], + /*appArgs=*/ [[new Fr(1), new Fr(2)]], + /*teardownFunctionName=*/ 'divide_by_zero', + /*teardownArgs=*/ [], + /*expectRevert=*/ true, ); }, TIMEOUT, ); it( - 'Should prove and verify an exceptional halt due to a nested call to non-existent contract that is propagated to top-level', + 'a nested exceptional halt propagate to top-level', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick - 'nested_call_to_nothing', + 'external_call_to_divide_by_zero', /*args=*/ [], /*expectRevert=*/ true, ); @@ -196,11 +210,11 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify an exceptional halt due to a nested call to non-existent contract that is recovered from in caller', + 'a nested exceptional halt is recovered from in caller', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick - 'nested_call_to_nothing_recovers', + 'external_call_to_divide_by_zero_recovers', /*args=*/ [], /*expectRevert=*/ false, ); @@ -208,46 +222,45 @@ describe('AVM WitGen, proof generation and verification', () => { TIMEOUT, ); it( - 'Should prove and verify a top-level exceptional halt due to a non-existent contract', + 'an exceptional halt due to a nested call to non-existent contract is propagated to top-level', async () => { await proveAndVerifyAvmTestContractSimple( /*checkCircuitOnly=*/ true, // quick - 'add_args_return', - /*args=*/ [new Fr(1), new Fr(2)], + 'nested_call_to_nothing', + /*args=*/ [], /*expectRevert=*/ true, - /*skipContractDeployments=*/ true, ); }, TIMEOUT, ); it( - 'Should prove and verify multiple app logic enqueued calls (set storage in first call, read it in next)', + 'an exceptional halt due to a nested call to non-existent contract is recovered from in caller', async () => { - await proveAndVerifyAvmTestContract( - /*checkCircuitOnly=*/ true, - /*setupFunctionNames=*/ [], - /*setupArgs=*/ [], - /*appFunctionNames=*/ ['set_storage_single', 'read_assert_storage_single'], - /*appArgs=*/ [[new Fr(5)], [new Fr(5)]], + await proveAndVerifyAvmTestContractSimple( + /*checkCircuitOnly=*/ true, // quick + 'nested_call_to_nothing_recovers', + /*args=*/ [], + /*expectRevert=*/ false, ); }, TIMEOUT, ); + // FIXME(dbanks12): fails with "Lookup PERM_MAIN_ALU failed." + it.skip('a top-level exceptional halts due to a non-existent contract in app-logic and teardown', async () => { + await proveAndVerifyAvmTestContract( + /*checkCircuitOnly=*/ true, + /*setupFunctionNames=*/ [], + /*setupArgs=*/ [], + /*appFunctionNames=*/ ['add_args_return'], + /*appArgs=*/ [[new Fr(1), new Fr(2)]], + /*teardownFunctionName=*/ 'add_args_return', + /*teardownArgs=*/ [new Fr(1), new Fr(2)], + /*expectRevert=*/ true, + /*skipContractDeployments=*/ true, + ); + }); it( - 'Should prove and verify multiple app logic enqueued calls (like `enqueue_public_from_private`)', - async () => { - await proveAndVerifyAvmTestContract( - /*checkCircuitOnly=*/ true, - /*setupFunctionNames=*/ [], - /*setupArgs=*/ [], - /*appFunctionNames=*/ ['set_opcode_u8', 'set_read_storage_single'], - /*appArgs=*/ [[], [new Fr(5)]], - ); - }, - TIMEOUT, - ); - it.skip( - 'Should prove and verify enqueued calls in every phase, with enqueued calls that depend on each other', + 'enqueued calls in every phase, with enqueued calls that depend on each other', async () => { await proveAndVerifyAvmTestContract( /*checkCircuitOnly=*/ true, @@ -358,7 +371,7 @@ async function proveAndVerifyAvmTestContract( } describe('AVM WitGen, proof generation and verification', () => { - it('Should prove and verify bulk_testing v2', async () => { + it('bulk_testing v2', async () => { const functionName = 'bulk_testing'; const calldata = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map(x => new Fr(x)); const avmCircuitInputs = await simulateAvmTestContractGenerateCircuitInputs(