diff --git a/barretenberg/cpp/src/barretenberg/api/api_chonk.cpp b/barretenberg/cpp/src/barretenberg/api/api_chonk.cpp index 1dbe99828a5..29fc2b5ea21 100644 --- a/barretenberg/cpp/src/barretenberg/api/api_chonk.cpp +++ b/barretenberg/cpp/src/barretenberg/api/api_chonk.cpp @@ -4,8 +4,6 @@ #include "barretenberg/api/log.hpp" #include "barretenberg/bbapi/bbapi.hpp" #include "barretenberg/chonk/chonk.hpp" -#include "barretenberg/chonk/chonk_verifier.hpp" -#include "barretenberg/chonk/mock_circuit_producer.hpp" #include "barretenberg/chonk/private_execution_steps.hpp" #include "barretenberg/chonk/proof_compression.hpp" #include "barretenberg/common/get_bytecode.hpp" @@ -68,7 +66,8 @@ void ChonkAPI::prove(const Flags& flags, BB_BENCH_NAME("ChonkAPI::prove"); bbapi::BBApiRequest request; request.vk_policy = bbapi::parse_vk_policy(flags.vk_policy); - std::vector raw_steps = PrivateExecutionStepRaw::load_and_decompress(input_path); + // Steps stay compressed until accumulated, so only one decompressed step is resident at a time. + std::vector raw_steps = PrivateExecutionStepRaw::load(input_path); std::vector kinds; kinds.reserve(raw_steps.size()); @@ -77,48 +76,38 @@ void ChonkAPI::prove(const Flags& flags, } bbapi::ChonkStart{ .kinds = std::move(kinds) }.execute(request); info("Chonk: starting with ", raw_steps.size(), " circuits"); - for (size_t i = 0; i < raw_steps.size(); ++i) { - const auto& step = raw_steps[i]; + + for (auto& step : raw_steps) { + step.self_decompress(); bbapi::ChonkLoad{ - .circuit = { .name = step.function_name, .bytecode = step.bytecode, .verification_key = step.vk }, + .circuit = { .name = step.function_name, + .bytecode = std::move(step.bytecode), + .verification_key = std::move(step.vk) }, .kind = step.kind, } .execute(request); - // NOLINTNEXTLINE(bugprone-unchecked-optional-access): we know the optional has been set here. info("Chonk: accumulating " + step.function_name); - bbapi::ChonkAccumulate{ .witness = step.witness }.execute(request); + bbapi::ChonkAccumulate{ .witness = std::move(step.witness) }.execute(request); } auto proof = bbapi::ChonkProve{}.execute(request).proof; const bool output_to_stdout = output_dir == "-"; - const auto write_proof = [&]() { - const auto proof_fields = proof.to_field_elements(); - if (output_to_stdout) { - vinfo("writing Chonk proof to stdout"); - write_bytes_to_stdout(to_buffer(proof_fields)); - } else if (flags.output_format == "json") { - vinfo("writing Chonk proof (JSON) in directory ", output_dir); - // Note: Chonk proof doesn't have a vk_hash, so we pass an empty string - std::string json_content = ProofJson::build(proof_fields, "", flags.scheme); - write_file(output_dir / "proof.json", std::vector(json_content.begin(), json_content.end())); - info("Proof (JSON) saved to ", output_dir / "proof.json"); - } else { - vinfo("writing Chonk proof in directory ", output_dir); - write_file(output_dir / "proof", to_buffer(proof_fields)); - } - }; - - write_proof(); - - if (flags.write_vk) { - vinfo("writing Chonk vk in directory ", output_dir); - // Write CHONK vk for the hiding kernel (last step) — proven as MegaZK. - Flags hiding_flags = flags; - hiding_flags.circuit_kind = "hiding"; - write_chonk_vk(raw_steps[raw_steps.size() - 1].bytecode, output_dir, hiding_flags); + const auto proof_fields = proof.to_field_elements(); + if (output_to_stdout) { + vinfo("writing Chonk proof to stdout"); + write_bytes_to_stdout(to_buffer(proof_fields)); + } else if (flags.output_format == "json") { + vinfo("writing Chonk proof (JSON) in directory ", output_dir); + // Note: Chonk proof doesn't have a vk_hash, so we pass an empty string + std::string json_content = ProofJson::build(proof_fields, "", flags.scheme); + write_file(output_dir / "proof.json", std::vector(json_content.begin(), json_content.end())); + info("Proof (JSON) saved to ", output_dir / "proof.json"); + } else { + vinfo("writing Chonk proof in directory ", output_dir); + write_file(output_dir / "proof", to_buffer(proof_fields)); } } @@ -192,22 +181,6 @@ void ChonkAPI::proof_stats(const std::filesystem::path& proof_path, const std::f } } -// WORKTODO(bbapi) remove this -bool ChonkAPI::prove_and_verify(const std::filesystem::path& input_path) -{ - PrivateExecutionSteps steps; - steps.parse(PrivateExecutionStepRaw::load_and_decompress(input_path)); - - std::shared_ptr ivc = steps.accumulate(); - // Construct the hiding kernel as the final step of the IVC - - auto proof = ivc->prove(); - auto vk_and_hash = ivc->get_hiding_kernel_vk_and_hash(); - ChonkNativeVerifier verifier(vk_and_hash); - const bool verified = verifier.verify(proof); - return verified; -} - void ChonkAPI::gates(const Flags& flags, const std::filesystem::path& bytecode_path) { BB_BENCH_NAME("ChonkAPI::gates"); @@ -225,7 +198,6 @@ void ChonkAPI::write_solidity_verifier([[maybe_unused]] const Flags& flags, bool ChonkAPI::check_precomputed_vks(const Flags& flags, const std::filesystem::path& input_path) { BB_BENCH_NAME("ChonkAPI::check_precomputed_vks"); - bbapi::BBApiRequest request; std::vector raw_steps = PrivateExecutionStepRaw::load_and_decompress(input_path); bbapi::VkPolicy vk_policy = bbapi::parse_vk_policy(flags.vk_policy); diff --git a/barretenberg/cpp/src/barretenberg/api/api_chonk.hpp b/barretenberg/cpp/src/barretenberg/api/api_chonk.hpp index 4ed7081c5f7..bbfe9cf09cc 100644 --- a/barretenberg/cpp/src/barretenberg/api/api_chonk.hpp +++ b/barretenberg/cpp/src/barretenberg/api/api_chonk.hpp @@ -18,7 +18,7 @@ namespace bb { * TypeScript (private function calls) * → msgpack encode to ivc-inputs.msgpack * → ChonkAPI::prove() loads and accumulates all steps - * → Chonk proof + VK written to output directory + * → Chonk proof written to output directory * ``` * * **Key entry points:** @@ -27,7 +27,7 @@ namespace bb { * - `write_vk()`: Computes verification key for standalone or IVC circuits * - `check_precomputed_vks()`: Validates that precomputed VKs match computed ones * - * @see PrivateExecutionSteps for the input format + * @see PrivateExecutionStepRaw for the input format * @see Chonk for the underlying IVC implementation */ class ChonkAPI : public API { @@ -37,11 +37,11 @@ class ChonkAPI : public API { * @brief Main production entry point: generate a Chonk proof from private execution steps. * * @details Loads execution steps from ivc-inputs.msgpack, accumulates each circuit via Chonk, - * and writes the final proof (and optionally VK) to the output directory. + * and writes the final proof to the output directory. * - * @param flags Runtime flags (vk_policy, write_vk, etc.) + * @param flags Runtime flags (vk_policy, output_format, etc.) * @param input_path Path to ivc-inputs.msgpack containing PrivateExecutionStepRaw entries - * @param output_dir Directory to write proof (and vk if flags.write_vk is set) + * @param output_dir Directory to write the proof */ void prove(const Flags& flags, const std::filesystem::path& input_path, const std::filesystem::path& output_dir); @@ -57,11 +57,6 @@ class ChonkAPI : public API { const std::filesystem::path& proof_path, const std::filesystem::path& vk_path) override; - /** - * @brief Test/debug function: prove and verify in one call (bypasses serialization). - */ - bool prove_and_verify(const std::filesystem::path& input_path); - /** * @brief Output gate count statistics for a circuit. */ diff --git a/barretenberg/cpp/src/barretenberg/api/file_io.hpp b/barretenberg/cpp/src/barretenberg/api/file_io.hpp index 3bf84b67ef4..b3e1dc8edba 100644 --- a/barretenberg/cpp/src/barretenberg/api/file_io.hpp +++ b/barretenberg/cpp/src/barretenberg/api/file_io.hpp @@ -156,8 +156,8 @@ inline std::vector read_vk_file(const std::filesystem::path& vk_path) return read_file(vk_path.string()); } catch (const std::runtime_error&) { THROW std::runtime_error("Unable to open file: " + vk_path.string() + - "\nGenerate a vk during proving by running `bb prove` with an additional `--write_vk` " - "flag, or run `bb write_vk` to generate a standalone vk." + "\nRun `bb write_vk` to generate a vk. For UltraHonk, you can also generate it during " + "proving with `bb prove --write_vk`." "\nIf you already have a vk file, specify its path with `--vk_path `."); } } diff --git a/barretenberg/cpp/src/barretenberg/bb/cli.cpp b/barretenberg/cpp/src/barretenberg/bb/cli.cpp index afe3529ad65..db4c71a7a58 100644 --- a/barretenberg/cpp/src/barretenberg/bb/cli.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/cli.cpp @@ -258,7 +258,8 @@ int parse_and_run_cli_command(int argc, char* argv[]) }; const auto add_write_vk_flag = [&](CLI::App* subcommand) { - return subcommand->add_flag("--write_vk", flags.write_vk, "Write the provided circuit's verification key"); + return subcommand->add_flag( + "--write_vk", flags.write_vk, "Write the provided circuit's verification key (UltraHonk only)"); }; const auto remove_zk_option = [&](CLI::App* subcommand) { @@ -1007,6 +1008,10 @@ int parse_and_run_cli_command(int argc, char* argv[]) } else if (flags.scheme == "chonk") { ChonkAPI api; if (prove->parsed()) { + if (flags.write_vk) { + throw_or_abort("prove --write_vk is not supported for Chonk. Use bb write_vk --scheme chonk " + "--circuit_kind hiding -b -o ."); + } if (!std::filesystem::exists(ivc_inputs_path)) { throw_or_abort("The prove command for Chonk expect a valid file passed with --ivc_inputs_path " " (default ./ivc-inputs.msgpack)"); diff --git a/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp b/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp index 65c74d59636..c2cfe88b7f6 100644 --- a/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp +++ b/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp @@ -217,6 +217,51 @@ TEST(BBApiInputValidation, MsgpackLoadRejectsTrailingData) std::filesystem::remove(tmp); } +TEST(PrivateExecutionStepRaw, SelfDecompressIsIdempotent) +{ + const PrivateExecutionStepRaw step{ .bytecode = { 1, 2, 3 }, .witness = { 4, 5 }, .vk = {}, .function_name = "f" }; + auto tmp = std::filesystem::temp_directory_path() / "bb_test_step_idempotent.msgpack"; + PrivateExecutionStepRaw::compress_and_save({ step }, tmp); + + auto loaded = PrivateExecutionStepRaw::load(tmp); + std::filesystem::remove(tmp); + ASSERT_EQ(loaded.size(), 1); + loaded[0].self_decompress(); + loaded[0].self_decompress(); + + EXPECT_EQ(loaded[0].bytecode, step.bytecode); + EXPECT_EQ(loaded[0].witness, step.witness); +} + +TEST(PrivateExecutionStepRaw, SelfDecompressLeavesUncompressedStepsUnchanged) +{ + const PrivateExecutionStepRaw step{ .bytecode = { 1, 2, 3 }, .witness = { 4, 5 }, .vk = {}, .function_name = "f" }; + auto parsed = PrivateExecutionStepRaw::parse_uncompressed(pack_steps({ step })); + ASSERT_EQ(parsed.size(), 1); + + parsed[0].self_decompress(); + + EXPECT_EQ(parsed[0].bytecode, step.bytecode); + EXPECT_EQ(parsed[0].witness, step.witness); +} + +TEST(PrivateExecutionStepRaw, CompressAndSaveDoesNotRecompressLoadedSteps) +{ + const PrivateExecutionStepRaw step{ .bytecode = { 1, 2, 3 }, .witness = { 4, 5 }, .vk = {}, .function_name = "f" }; + auto first = std::filesystem::temp_directory_path() / "bb_test_step_first.msgpack"; + auto second = std::filesystem::temp_directory_path() / "bb_test_step_second.msgpack"; + PrivateExecutionStepRaw::compress_and_save({ step }, first); + + PrivateExecutionStepRaw::compress_and_save(PrivateExecutionStepRaw::load(first), second); + auto reloaded = PrivateExecutionStepRaw::load_and_decompress(second); + std::filesystem::remove(first); + std::filesystem::remove(second); + + ASSERT_EQ(reloaded.size(), 1); + EXPECT_EQ(reloaded[0].bytecode, step.bytecode); + EXPECT_EQ(reloaded[0].witness, step.witness); +} + // Regression tests for AesEncrypt/AesDecrypt input validation. // Without these guards, a socket client could force a ~4 GB allocation via the // uint32_t `length` field, or pass `length != plaintext.size()` and silently diff --git a/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk.cpp b/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk.cpp index a9f407469bb..d59c8119156 100644 --- a/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk.cpp +++ b/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk.cpp @@ -317,9 +317,8 @@ ChonkComputeVk::Response ChonkComputeVk::execute([[maybe_unused]] const BBApiReq static_cast(kind), ")"); - auto constraint_system = acir_format::circuit_buf_to_mega_acir_format(std::move(circuit.bytecode)); - - acir_format::AcirProgram program{ constraint_system, /*witness=*/{} }; + acir_format::AcirProgram program{ acir_format::circuit_buf_to_mega_acir_format(std::move(circuit.bytecode)), + /*witness=*/{} }; auto verification_key = compute_chonk_vk(program, kind); info("ChonkComputeVk - VK derived, size: ", verification_key.bytes.size(), " bytes"); return { .bytes = std::move(verification_key.bytes), .fields = std::move(verification_key.fields) }; diff --git a/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk_pinned_inputs.test.cpp b/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk_pinned_inputs.test.cpp index bbb485b586a..9fb113317a3 100644 --- a/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk_pinned_inputs.test.cpp +++ b/barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk_pinned_inputs.test.cpp @@ -82,8 +82,13 @@ class ChonkPinnedIvcInputsTest : public ::testing::Test { auto raw_steps = bb::PrivateExecutionStepRaw::load_and_decompress(inputs_path); ASSERT_FALSE(raw_steps.empty()) << "no execution steps in " << inputs_path; - const auto hiding_bytecode = raw_steps.back().bytecode; + ASSERT_EQ(raw_steps.back().kind, bb::CircuitKind::HidingKernel); + auto hiding_bytecode = raw_steps.back().bytecode; const auto hiding_vk = raw_steps.back().vk; + const auto derived_vk = bb::bbapi::ChonkComputeVk{ .circuit = { .bytecode = std::move(hiding_bytecode) }, + .kind = bb::CircuitKind::HidingKernel } + .execute(); + ASSERT_EQ(hiding_vk, derived_vk.bytes) << "embedded hiding VK differs from bytecode-derived VK"; bb::bbapi::BBApiRequest request; request.vk_policy = bb::bbapi::VkPolicy::DEFAULT; @@ -108,7 +113,7 @@ class ChonkPinnedIvcInputsTest : public ::testing::Test { auto prove_response = bb::bbapi::ChonkProve{}.execute(request); auto verify_response = - bb::bbapi::ChonkVerify{ .proof = std::move(prove_response.proof), .vk = hiding_vk }.execute(); + bb::bbapi::ChonkVerify{ .proof = std::move(prove_response.proof), .vk = derived_vk.bytes }.execute(); EXPECT_TRUE(verify_response.valid) << "ChonkVerify rejected " << flow_dir.filename(); } }; diff --git a/barretenberg/cpp/src/barretenberg/chonk/chonk.cpp b/barretenberg/cpp/src/barretenberg/chonk/chonk.cpp index f50ec08eb37..0db375392ba 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/chonk.cpp +++ b/barretenberg/cpp/src/barretenberg/chonk/chonk.cpp @@ -499,10 +499,8 @@ void Chonk::accumulate_hiding_kernel(ClientCircuit& circuit, vinfo("Constructing hiding kernel instance (proving deferred to prove())"); hiding_prover_inst = std::make_shared(circuit); - // Free circuit block memory now that trace data has been copied to prover polynomials - for (auto& block : circuit.blocks.get()) { - block.free_data(); - } + // The circuit has been copied into the prover polynomials; free its construction data. + circuit.free_construction_data(); if (precomputed_vk) { #ifndef NDEBUG @@ -531,10 +529,8 @@ HonkProof Chonk::instance_to_accumulator(ClientCircuit& circuit, #ifndef NDEBUG debug_incoming_circuit(circuit, prover_instance, vk); #endif - // Free circuit block memory (wires and selectors) now that they've been copied to prover polynomials. - for (auto& block : circuit.blocks.get()) { - block.free_data(); - } + // The circuit has been copied into the prover polynomials; free its construction data. + circuit.free_construction_data(); // Run sumcheck on the incoming instance; the folding prover caches the resulting claim. The claims of the whole // group are batched together once, when the group's last circuit is accumulated (see prove_multilinear_batching). @@ -662,6 +658,9 @@ void Chonk::accumulate(ClientCircuit& circuit, const CircuitVerificationKey& vk) #ifndef NDEBUG verify_decider_natively(); #endif + // The decider proof is the last use of the final accumulator's polynomials. + prover_accumulator.non_shifted_polynomial = ProverAccumulator::Polynomial{}; + prover_accumulator.shifted_polynomial = ProverAccumulator::Polynomial{}; goblin.prove_batch_merge(); } break; @@ -749,10 +748,13 @@ ChonkProof Chonk::prove() goblin.prove_eccvm(); vinfo("finished eccvm proving."); - // Phase 4: Build translator proving key from ECCVM-derived challenges. - TranslatorCircuitBuilder translator_builder( - goblin.translation_batching_challenge_v, goblin.evaluation_challenge_x, goblin.op_queue); - auto translator_key = std::make_shared(translator_builder); + // Phase 4: Build translator proving key from ECCVM-derived challenges. The key copies the circuit into its + // polynomials, so the circuit builder is released before proving. + auto translator_key = [&] { + TranslatorCircuitBuilder translator_builder( + goblin.translation_batching_challenge_v, goblin.evaluation_challenge_x, goblin.op_queue); + return std::make_shared(translator_builder); + }(); // Phase 5: Translator Oink + Joint Sumcheck + Joint PCS on the shared transcript. vinfo("prove translator and joint..."); diff --git a/barretenberg/cpp/src/barretenberg/chonk/chonk.hpp b/barretenberg/cpp/src/barretenberg/chonk/chonk.hpp index b50ed97d704..d145dff0bc4 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/chonk.hpp +++ b/barretenberg/cpp/src/barretenberg/chonk/chonk.hpp @@ -160,20 +160,32 @@ class Chonk { size_t num_circuits; // total number of circuits to be accumulated in the IVC size_t num_circuits_accumulated = 0; // number of circuits accumulated so far - public: - ProverAccumulator prover_accumulator; // previous accumulator (output of the previous kernel's batch) + std::shared_ptr folding_prover; // folds the current group; (re)created at each group start +#ifndef NDEBUG + VerifierAccumulator native_verifier_accum; + std::shared_ptr native_folding_verifier; // native cross-check of the current group + std::shared_ptr native_verifier_accumulation_transcript = std::make_shared(); +#endif + + // Management of linking databus commitments between circuits in the IVC + DataBusDepot bus_depot; + + // Hiding kernel prover instance: built during accumulate_hiding_kernel(), consumed by prove(). + std::shared_ptr hiding_prover_inst; + + protected: + // Previous accumulator (output of the previous kernel's batch). Its polynomials are released once the hiding + // kernel's decider proof is constructed. + ProverAccumulator prover_accumulator; + + public: HonkProof multilinear_batch_proof; // current kernel's multilinear batching proof (consumed in its recursive verifier) HonkProof decider_proof; // decider proof to be verified in the Hiding kernel VerifierAccumulator recursive_verifier_native_accum; // native value of the previous accumulator -#ifndef NDEBUG - VerifierAccumulator native_verifier_accum; - std::shared_ptr native_folding_verifier; // native cross-check of the current group - std::shared_ptr native_verifier_accumulation_transcript = std::make_shared(); -#endif // PARALLEL QUEUES: These two queues must stay synchronized. // - verification_queue: Native proofs created by accumulate() (prover side) @@ -182,13 +194,9 @@ class Chonk { VerificationQueue verification_queue; StdlibVerificationQueue stdlib_verification_queue; - // Management of linking databus commitments between circuits in the IVC - DataBusDepot bus_depot; - Goblin goblin; - // Hiding kernel prover state: built during accumulate_hiding_kernel(), consumed by prove(). - std::shared_ptr hiding_prover_inst; + // Hiding kernel VK: set during accumulate_hiding_kernel(). std::shared_ptr hiding_vk; /** diff --git a/barretenberg/cpp/src/barretenberg/chonk/chonk.test.cpp b/barretenberg/cpp/src/barretenberg/chonk/chonk.test.cpp index 41fcefa3a29..705c5a9ee3d 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/chonk.test.cpp +++ b/barretenberg/cpp/src/barretenberg/chonk/chonk.test.cpp @@ -1,6 +1,8 @@ +#include #include #include #include +#include #include "barretenberg/chonk/chonk.hpp" #include "barretenberg/chonk/chonk_verifier.hpp" @@ -38,6 +40,15 @@ enum class KernelIOField : uint8_t { ECC_OP_HASH }; +/** + * @brief Chonk with its prover accumulator exposed, for tests that read or substitute it. + */ +class ChonkWithAccumulatorAccess : public Chonk { + public: + using Chonk::Chonk; + using Chonk::prover_accumulator; +}; + } // namespace class ChonkTests : public ::testing::Test { @@ -378,9 +389,11 @@ TEST_F(ChonkTests, MsgpackProofFromFileOrBuffer) auto [proof, vk] = accumulate_and_prove_ivc(/*num_app_circuits=*/1, settings); { // Serialize/deserialize the proof to/from a file, check that it verifies - const std::string filename = "proof.msgpack"; + const std::string filename = + std::filesystem::temp_directory_path() / ("bb_chonk_proof_" + std::to_string(getpid()) + ".msgpack"); proof.to_file_msgpack(filename); auto proof_deserialized = ChonkProof::from_file_msgpack(filename); + std::filesystem::remove(filename); EXPECT_TRUE(verify_chonk(proof_deserialized, vk)); } @@ -471,7 +484,7 @@ TEST_F(ChonkTests, AccumulatorBinding) const size_t num_app_circuits = MAX_APPS_PER_KERNEL + 1; CircuitProducer producer_one(num_app_circuits); const size_t num_circuits = producer_one.total_num_circuits; - Chonk chonk_one{ producer_one.circuit_kinds() }; + ChonkWithAccumulatorAccess chonk_one{ producer_one.circuit_kinds() }; // Accumulate the first MAX_APPS_PER_KERNEL apps and the kernel for (size_t idx = 0; idx < MAX_APPS_PER_KERNEL + 1; idx++) { @@ -482,7 +495,7 @@ TEST_F(ChonkTests, AccumulatorBinding) // ── Step 2: Run the IVC with an INVALID first app + accumulator substitution ─ MockDatabusProducer mock_databus; - Chonk invalid_chonk{ producer_one.circuit_kinds() }; + ChonkWithAccumulatorAccess invalid_chonk{ producer_one.circuit_kinds() }; CircuitProducer producer_two(num_app_circuits); TestSettings settings_two{ .log2_num_gates = SMALL_LOG_2_NUM_GATES + 1 }; diff --git a/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.cpp b/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.cpp index 195c1e79a75..a122b8a052c 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.cpp +++ b/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.cpp @@ -91,6 +91,9 @@ void ChonkStepProcessor::process_step(ChonkExecutionStep&& step, ChonkPrecompute { const acir_format::ProgramMetadata metadata{ .ivc = ivc }; auto circuit = acir_format::create_circuit(step.program, metadata); + // The circuit holds everything accumulation needs: free the constraint system and witness before accumulating, + // which is when memory use peaks. + step.program = acir_format::AcirProgram{}; info("Chonk: accumulating ", step.name); if (detail::use_memory_profile) { diff --git a/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.hpp b/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.hpp index 8d313edff3a..3c6e3d9cd3f 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.hpp +++ b/barretenberg/cpp/src/barretenberg/chonk/chonk_step_processor.hpp @@ -32,7 +32,6 @@ class ChonkStepProcessor { public: explicit ChonkStepProcessor(std::vector circuit_kinds); - std::shared_ptr get_ivc() const { return ivc; } size_t get_num_circuits_accumulated() const { return ivc->get_num_circuits_accumulated(); } void process_step(ChonkExecutionStep&& step, ChonkPrecomputedVkPolicy policy = ChonkPrecomputedVkPolicy::DEFAULT); diff --git a/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp b/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp index f60e72efb38..962b1c4ee0c 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp +++ b/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp @@ -5,12 +5,15 @@ // ===================== #include "private_execution_steps.hpp" -#include "barretenberg/chonk/chonk.hpp" -#include "barretenberg/chonk/chonk_step_processor.hpp" +#include "barretenberg/common/bb_bench.hpp" #include "barretenberg/common/serialize.hpp" #include "barretenberg/common/thread.hpp" -#include "barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp" +#include "barretenberg/serialize/msgpack_impl.hpp" +#include #include +#include +#include +#include namespace bb { @@ -100,14 +103,22 @@ template T unpack_from_file(const std::filesystem::path& filename) std::vector PrivateExecutionStepRaw::load(const std::filesystem::path& input_path) { BB_BENCH(); - return unpack_from_file>(input_path); + auto raw_steps = unpack_from_file>(input_path); + for (auto& step : raw_steps) { + step.compressed = true; + } + return raw_steps; } // TODO(#7371) we should not have so many levels of serialization here. void PrivateExecutionStepRaw::self_decompress() { + if (!compressed) { + return; + } bytecode = decompress(bytecode.data(), bytecode.size()); witness = decompress(witness.data(), witness.size()); + compressed = false; } // TODO(#7371) we should not have so many levels of serialization here. @@ -116,10 +127,7 @@ std::vector PrivateExecutionStepRaw::load_and_decompres { BB_BENCH(); auto raw_steps = load(input_path); - parallel_for(raw_steps.size(), [&](size_t i) { - raw_steps[i].bytecode = decompress(raw_steps[i].bytecode.data(), raw_steps[i].bytecode.size()); - raw_steps[i].witness = decompress(raw_steps[i].witness.data(), raw_steps[i].witness.size()); - }); + parallel_for(raw_steps.size(), [&](size_t i) { raw_steps[i].self_decompress(); }); return raw_steps; } @@ -137,62 +145,17 @@ std::vector PrivateExecutionStepRaw::parse_uncompressed return raw_steps; } -void PrivateExecutionSteps::parse(std::vector&& steps) -{ - BB_BENCH(); - - // Preallocate space to write into diretly as push_back would not be thread safe - folding_stack.resize(steps.size()); - precomputed_vks.resize(steps.size()); - function_names.resize(steps.size()); - kinds.resize(steps.size()); - - // Parse each step's bytecode/witness in parallel (thread-safe with msgpack format) - parallel_for(steps.size(), [&](size_t i) { - PrivateExecutionStepRaw step = std::move(steps[i]); - - acir_format::AcirFormat constraints = acir_format::circuit_buf_to_mega_acir_format(std::move(step.bytecode)); - acir_format::WitnessVector witness = acir_format::witness_buf_to_witness_vector(std::move(step.witness)); - - folding_stack[i] = { std::move(constraints), std::move(witness) }; - if (step.vk.empty()) { - // For backwards compatibility, but it affects performance and correctness. - precomputed_vks[i] = {}; - } else { - precomputed_vks[i] = std::move(step.vk); - } - function_names[i] = std::move(step.function_name); - kinds[i] = step.kind; - }); -} - -std::shared_ptr PrivateExecutionSteps::accumulate() -{ - auto step_processor = ChonkStepProcessor(kinds); - - for (auto& vk : precomputed_vks) { - if (vk.empty()) { - info("DEPRECATED: Precomputed VKs expected for the given circuits."); - break; - } - } - for (size_t i = 0; i < folding_stack.size(); ++i) { - step_processor.process_step({ .name = std::move(function_names[i]), - .program = std::move(folding_stack[i]), - .precomputed_vk = std::move(precomputed_vks[i]), - .kind = kinds[i] }); - } - - return step_processor.get_ivc(); -} - void PrivateExecutionStepRaw::compress_and_save(std::vector&& steps, const std::filesystem::path& output_path) { - // First, compress the bytecode and witness fields of each step + // First, compress the bytecode and witness fields of each step that is not already compressed for (PrivateExecutionStepRaw& step : steps) { + if (step.compressed) { + continue; + } step.bytecode = compress(step.bytecode); step.witness = compress(step.witness); + step.compressed = true; } // Serialize to msgpack diff --git a/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.hpp b/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.hpp index a9c4dc316ac..58922dc76e9 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.hpp +++ b/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.hpp @@ -5,18 +5,14 @@ // ===================== #include "barretenberg/chonk/circuit_input.hpp" -#include "barretenberg/dsl/acir_format/acir_format.hpp" #include "barretenberg/serialize/msgpack.hpp" #include #include -#include #include #include namespace bb { -class Chonk; - /** * @brief This is the msgpack encoding of the objects returned by the following typescript: * const stepToStruct = (step: PrivateExecutionStep) => { @@ -40,9 +36,15 @@ struct PrivateExecutionStepRaw { // Represents the function name. std::string function_name; CircuitKind kind = CircuitKind::None; + // Whether bytecode and witness hold gzip data. Not serialized: load() sets it, since the file stores them + // compressed. + bool compressed = false; // Unrolled from SERIALIZATION_FIELDS for custom name for function_name. void msgpack(auto pack_fn) { pack_fn(NVP(bytecode, witness, vk), "functionName", function_name, NVP(kind)); }; + /** + * @brief Decompress bytecode and witness in place; does nothing if they are not compressed. + */ void self_decompress(); static std::vector load_and_decompress(const std::filesystem::path& input_path); static std::vector load(const std::filesystem::path& input_path); @@ -51,43 +53,4 @@ struct PrivateExecutionStepRaw { const std::filesystem::path& output_path); }; -/** - * @brief Parsed private execution steps ready for Chonk accumulation. - * - * @details After deserializing from msgpack, this struct holds the decoded ACIR programs, - * their witnesses, precomputed VKs, and function names. The accumulate() method runs the - * complete IVC accumulation over all steps. - * - * Data flow: - * TypeScript (Aztec client) → msgpack encode → ivc-inputs.msgpack - * → PrivateExecutionStepRaw::load_and_decompress() - * → PrivateExecutionSteps::parse() - * → PrivateExecutionSteps::accumulate() - * → Chonk IVC proof - * - * - */ -struct PrivateExecutionSteps { - std::vector folding_stack; ///< ACIR programs with witnesses - std::vector function_names; ///< Function names for logging - std::vector> precomputed_vks; ///< Serialized precomputed VKs (performance) - std::vector kinds; ///< Per-step CircuitKind - - /** - * @brief Creates a Chonk instance and accumulates each circuit in the folding stack. - * Uses precomputed VKs when available for performance. The returned Chonk instance - * is ready to call prove() to generate the final IVC proof. - * - * @return Shared pointer to Chonk instance with accumulated state - */ - std::shared_ptr accumulate(); - - /** - * @brief Converts PrivateExecutionStepRaw entries (which contain raw bytecode/witness bytes) - * into structured AcirProgram objects ready for circuit construction. - * - * @param steps Raw execution steps (will be moved from) - */ - void parse(std::vector&& steps); -}; } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp index f26826fc966..ac02916c13e 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp @@ -250,4 +250,19 @@ TEST(MegaCircuitBuilder, DatabusOutOfBoundsReadFails) EXPECT_THROW(builder.read_calldata(BusId::KERNEL_CALLDATA, bad_idx), std::runtime_error); } +/** + * @brief free_construction_data releases the databus also when called through the Ultra base class + */ +TEST(MegaCircuitBuilder, FreeConstructionDataThroughBaseReleasesDatabus) +{ + auto builder = MegaCircuitBuilder(); + builder.add_public_calldata(BusId::KERNEL_CALLDATA, builder.add_variable(fr(42))); + EXPECT_EQ(builder.get_calldata(BusId::KERNEL_CALLDATA).size(), 1); + + UltraCircuitBuilder_& base = builder; + base.free_construction_data(); + + EXPECT_EQ(builder.get_calldata(BusId::KERNEL_CALLDATA).size(), 0); +} + } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/gemini/gemini.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/gemini/gemini.hpp index 42ad0855413..267c608513e 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/gemini/gemini.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/gemini/gemini.hpp @@ -219,7 +219,7 @@ template class GeminiProver_ { A_0_neg.add_scaled(batched_to_be_shifted_by_one, -r_inv); } - return { A_0_pos, A_0_neg }; + return { std::move(A_0_pos), std::move(A_0_neg) }; }; }; diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp index ac17124f447..2b3cb388c0a 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp @@ -932,9 +932,10 @@ template class IPA requires Curve::is_stdlib_type { // Check SRS size up front before any circuit construction - if (vk.get_monomial_points().size() < poly_length) { + const auto& srs_elements = vk.get_monomial_points(); + if (srs_elements.size() < poly_length) { throw_or_abort("IPA recursive verification: not enough SRS points (need " + std::to_string(poly_length) + - ", have " + std::to_string(vk.get_monomial_points().size()) + ")"); + ", have " + std::to_string(srs_elements.size()) + ")"); } add_claim_to_hash_buffer(opening_claim, transcript); @@ -977,9 +978,7 @@ template class IPA // and 8-bit tables, the total table rows = 32768 × 256 = 2^23 exactly. The 5 mandatory overhead rows // (NUM_DISABLED_ROWS_IN_SUMCHECK=4, NUM_ZERO_ROWS=1) push the total to 2^23+5, forcing dyadic_size = 2^24. // To stay within 2^23 we handle the first SRS point separately using operator*. - std::vector srs_elements = vk.get_monomial_points(); - srs_elements.resize(poly_length); - std::vector remaining_srs(srs_elements.begin() + 1, srs_elements.end()); + std::vector remaining_srs(srs_elements.begin() + 1, srs_elements.begin() + poly_length); std::vector remaining_s(s_vec.begin() + 1, s_vec.end()); Commitment first_term = srs_elements[0] * s_vec[0]; Commitment remaining_term = Commitment::fixed_batch_mul(remaining_srs, remaining_s, /*table_bits=*/8); diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp index 1533aba380e..a45a1e6e3f7 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp @@ -228,7 +228,7 @@ template class ShplonkProver_ { current_nu *= nu_challenge; } // Return opening pair (z, 0) and polynomial G(X) = Q(X) - Q_z(X) - return { .polynomial = G, .opening_pair = { .challenge = z_challenge, .evaluation = Fr::zero() } }; + return { .polynomial = std::move(G), .opening_pair = { .challenge = z_challenge, .evaluation = Fr::zero() } }; }; /** * @brief Compute evaluations of fold polynomials Fold_i at r^{2^i} for i>0. diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp index 1d38bc03f5e..68851fa1e8d 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp @@ -283,7 +283,7 @@ std::pair ECCVMPro using ShplonkProver = ShplonkProver_; using ShplonkVerifier = ShplonkVerifier_; - const auto shplonk_output = ShplonkProver::compute_partially_evaluated_quotient( + auto shplonk_output = ShplonkProver::compute_partially_evaluated_quotient( key->commitment_key, univariate_claims.prover_claims, transcript); const auto verifier_opening = ShplonkVerifier::compute_partially_evaluated_quotient_claim( key->commitment_key.get_monomial_points()[0], @@ -293,7 +293,7 @@ std::pair ECCVMPro shplonk_output.opening_claim.opening_pair.challenge); BB_ASSERT(verifier_opening.opening_pair == shplonk_output.opening_claim.opening_pair); - return { shplonk_output.opening_claim, verifier_opening }; + return { std::move(shplonk_output.opening_claim), verifier_opening }; } typename ECCVMProver::Proof ECCVMProver::export_proof() diff --git a/barretenberg/cpp/src/barretenberg/eccvm/msm_builder.hpp b/barretenberg/cpp/src/barretenberg/eccvm/msm_builder.hpp index b4d723e4eaf..bf4094b360b 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/msm_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/msm_builder.hpp @@ -543,7 +543,7 @@ class ECCVMMSMMBuilder { typename MSMRow::AddState{ false, 0, AffineElement{ 0, 0 }, 0, 0 }, typename MSMRow::AddState{ false, 0, AffineElement{ 0, 0 }, 0, 0 } }; - return { msm_rows, point_table_read_counts }; + return { std::move(msm_rows), std::move(point_table_read_counts) }; } }; } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp index cf1c7b315b2..d7586dea99d 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp @@ -51,8 +51,12 @@ void Goblin::prove_eccvm() void Goblin::prove_translator() { BB_BENCH_NAME("Goblin::prove_translator"); - TranslatorBuilder translator_builder(translation_batching_challenge_v, evaluation_challenge_x, op_queue, avm_mode); - auto translator_key = std::make_shared(translator_builder); + // The key copies the circuit into its polynomials, so the circuit builder is released before proving. + auto translator_key = [&] { + TranslatorBuilder translator_builder( + translation_batching_challenge_v, evaluation_challenge_x, op_queue, avm_mode); + return std::make_shared(translator_builder); + }(); TranslatorProver translator_prover(translator_key, transcript); goblin_proof.translator_proof = translator_prover.construct_proof(); } diff --git a/barretenberg/cpp/src/barretenberg/hypernova/hypernova_prover.hpp b/barretenberg/cpp/src/barretenberg/hypernova/hypernova_prover.hpp index af05c7c0509..d04b6d6bc10 100644 --- a/barretenberg/cpp/src/barretenberg/hypernova/hypernova_prover.hpp +++ b/barretenberg/cpp/src/barretenberg/hypernova/hypernova_prover.hpp @@ -66,6 +66,7 @@ class HypernovaFoldingProver { /** * @brief Turn an instance into an accumulator by running Sumcheck. + * @details Consumes the instance's polynomials; callers must not reuse them after this call. */ template Accumulator instance_to_accumulator( @@ -109,6 +110,7 @@ class HypernovaFoldingProver { /** * @brief Turn an instance into an accumulator and cache the resulting claim for the final batching. + * @details Consumes the instance's polynomials; callers must not reuse them after this call. * @return This instance's proof slice (Oink + Sumcheck), exported from the shared transcript. */ template @@ -174,10 +176,17 @@ class HypernovaFoldingProver { auto [unshifted_challenges, shifted_challenges] = get_hypernova_batching_challenges( transcript, InstanceFlavor::NUM_UNSHIFTED_ENTITIES, InstanceFlavor::NUM_SHIFTED_ENTITIES); + auto unshifted = instance->polynomials.get_unshifted(); + auto to_be_shifted = instance->polynomials.get_to_be_shifted(); + // Unshifted batching may modify and move its first polynomial, so shifted batching must not depend on it. + for (const auto& polynomial : to_be_shifted) { + BB_ASSERT(&unshifted[0] != &polynomial, + "The first unshifted polynomial must not be among the to-be-shifted entities"); + } Polynomial batched_unshifted_polynomial = batch_polynomials( - instance->polynomials.get_unshifted(), instance->dyadic_size(), unshifted_challenges); + unshifted, instance->dyadic_size(), unshifted_challenges); Polynomial batched_shifted_polynomial = batch_polynomials( - instance->polynomials.get_to_be_shifted(), instance->dyadic_size(), shifted_challenges); + to_be_shifted, instance->dyadic_size(), shifted_challenges); FF batched_unshifted_evaluation(0); FF batched_shifted_evaluation(0); @@ -208,11 +217,13 @@ class HypernovaFoldingProver { } /** - * @brief Batch prover polynomials. Batching happens in place into the first polynomial in the RefArray supplied. + * @brief Compute a weighted sum of prover polynomials. + * @details If the first polynomial's range covers the others, accumulate into it and move it into the result. + * Otherwise, allocate the result without modifying the inputs. Only the first input may be modified or moved from. */ template static Polynomial batch_polynomials(RefArray, N> polynomials_to_batch, - const size_t& full_batched_size, + const size_t full_batched_size, const std::vector& challenges) { BB_BENCH_NAME("HypernovaFoldingProver::batch_polynomials"); @@ -244,7 +255,7 @@ class HypernovaFoldingProver { return result; } add_scaled_batch(polynomials_to_batch[0], sources_span, tail_scalars); - return polynomials_to_batch[0]; + return std::move(polynomials_to_batch[0]); } /** diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.cpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.cpp index ab712c7fa2b..1fe58d9303c 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.cpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.cpp @@ -43,6 +43,11 @@ void LMDBEnvironment::wait_for_reader() _readGuard.wait(); } +bool LMDBEnvironment::try_wait_for_reader(uint32_t keepFree) +{ + return _readGuard.try_wait(keepFree); +} + void LMDBEnvironment::release_reader() { _readGuard.release(); diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.hpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.hpp index c463ca74675..56c5947959a 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.hpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_environment.hpp @@ -49,6 +49,13 @@ class LMDBEnvironment { void wait_for_reader(); + /** + * @brief Takes a reader permit without blocking. + * @param keepFree How many permits must remain available after this one is taken + * @return false, taking nothing, if fewer than `keepFree + 1` permits are available + */ + bool try_wait_for_reader(uint32_t keepFree = 0); + void release_reader(); void wait_for_writer(); @@ -86,6 +93,16 @@ class LMDBEnvironment { ++_current; } + bool try_wait(uint32_t keepFree) + { + std::unique_lock lock(_lock); + if (static_cast(_current) + keepFree >= _maxAllowed) { + return false; + } + ++_current; + return true; + } + void release() { std::unique_lock lock(_lock); diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.cpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.cpp index 15165a93d91..690369d4c25 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.cpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.cpp @@ -113,7 +113,16 @@ void LMDBStore::put(std::vector& data) void LMDBStore::get(KeysVector& keys, OptionalValuesVector& values, const std::string& name) { - get(keys, values, get_database(name)); + LMDBDatabase::SharedPtr db = get_database(name); + get(keys, values, db, create_shared_read_transaction()); +} + +void LMDBStore::get(KeysVector& keys, + OptionalValuesVector& values, + const std::string& name, + ReadTransaction::SharedPtr tx) +{ + get(keys, values, get_database(name), std::move(tx)); } void LMDBStore::put(KeyDupValuesVector& toWrite, @@ -136,10 +145,12 @@ void LMDBStore::put(KeyDupValuesVector& toWrite, } } } -void LMDBStore::get(KeysVector& keys, OptionalValuesVector& values, LMDBDatabase::SharedPtr db) +void LMDBStore::get(KeysVector& keys, + OptionalValuesVector& values, + LMDBDatabase::SharedPtr db, + ReadTransaction::SharedPtr tx) { values.reserve(keys.size()); - ReadTransaction::SharedPtr tx = create_read_transaction(); if (!db->duplicate_keys_permitted()) { const LMDBDatabase& dbRef = *db; for (auto& k : keys) { @@ -179,6 +190,14 @@ void LMDBStore::get(KeysVector& keys, OptionalValuesVector& values, LMDBDatabase } void LMDBStore::has(const KeyOptionalValuesVector& entries, std::vector& results, const std::string& name) +{ + has(entries, results, name, create_shared_read_transaction()); +} + +void LMDBStore::has(const KeyOptionalValuesVector& entries, + std::vector& results, + const std::string& name, + ReadTransaction::SharedPtr tx) { auto string_cmp = [](const Key& a, const Key& b) { return std::lexicographical_compare(a.begin(), a.end(), b.begin(), b.end()); @@ -191,7 +210,7 @@ void LMDBStore::has(const KeyOptionalValuesVector& entries, std::vector& r KeysVector keys(key_set.begin(), key_set.end()); OptionalValuesVector vals; - get(keys, vals, name); + get(keys, vals, name, std::move(tx)); results.reserve(entries.size()); diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.hpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.hpp index 299dc9f65c1..e7b91f96a14 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.hpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.hpp @@ -48,7 +48,22 @@ class LMDBStore : public LMDBStoreBase { void put(std::vector& data); void get(KeysVector& keys, OptionalValuesVector& values, const std::string& name); + /** + * @brief Reads the given keys through an already open read transaction, so the values observed belong to that + * transaction's snapshot rather than to whatever is committed at call time. + * @note LMDB read transactions are not thread safe. The caller must ensure that nothing else uses `tx` + * (including opening or advancing cursors against it) while this call runs. + */ + void get(KeysVector& keys, OptionalValuesVector& values, const std::string& name, ReadTransaction::SharedPtr tx); void has(const KeyOptionalValuesVector& entries, std::vector& results, const std::string& name); + /** + * @brief Checks the given entries through an already open read transaction, like the `get` overload above. + * @note The caller must ensure that nothing else uses `tx` while this call runs. + */ + void has(const KeyOptionalValuesVector& entries, + std::vector& results, + const std::string& name, + ReadTransaction::SharedPtr tx); Cursor::Ptr create_cursor(ReadTransaction::SharedPtr tx, const std::string& dbName); @@ -63,7 +78,7 @@ class LMDBStore : public LMDBStoreBase { KeyOptionalValuesVector& toDelete, const LMDBDatabase& db, LMDBWriteTransaction& tx); - void get(KeysVector& keys, OptionalValuesVector& values, LMDBDatabase::SharedPtr db); + void get(KeysVector& keys, OptionalValuesVector& values, LMDBDatabase::SharedPtr db, ReadTransaction::SharedPtr tx); // Returns the database of the given name Database::SharedPtr get_database(const std::string& name); // Returns all databases diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.test.cpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.test.cpp index 75c677adc14..89e0add65da 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.test.cpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store.test.cpp @@ -9,7 +9,9 @@ #include #include #include +#include #include +#include #include #include "barretenberg/common/serialize.hpp" @@ -232,6 +234,191 @@ TEST_F(LMDBStoreTest, can_read_from_database) EXPECT_EQ(data[0].value(), ValuesVector{ expected }); } +TEST_F(LMDBStoreTest, reads_against_a_shared_read_transaction_see_a_stable_snapshot) +{ + LMDBStore::Ptr store = create_store(); + const std::string dbName = "Test Database"; + store->open_database(dbName); + + auto key = get_key(0); + auto original = get_value(0, 1); + auto updated = get_value(0, 2); + auto addedKey = get_key(1); + auto addedValue = get_value(1, 1); + + KeyOptionalValuesVector toDelete; + KeyDupValuesVector toWrite = { { { key, { original } } } }; + std::vector putDatas = { { toWrite, toDelete, dbName } }; + store->put(putDatas); + + LMDBStore::ReadTransaction::SharedPtr tx = store->create_shared_read_transaction(); + + KeysVector keys = { { key }, { addedKey } }; + OptionalValuesVector snapshot; + store->get(keys, snapshot, dbName, tx); + ASSERT_EQ(snapshot.size(), 2); + EXPECT_EQ(snapshot[0].value(), ValuesVector{ original }); + EXPECT_FALSE(snapshot[1].has_value()); + + // commits while the read transaction is still open + toWrite = { KeyValuesPair{ key, { updated } }, KeyValuesPair{ addedKey, { addedValue } } }; + putDatas = { { toWrite, toDelete, dbName } }; + store->put(putDatas); + + OptionalValuesVector afterWrite; + store->get(keys, afterWrite, dbName, tx); + ASSERT_EQ(afterWrite.size(), 2); + EXPECT_EQ(afterWrite[0].value(), ValuesVector{ original }); + EXPECT_FALSE(afterWrite[1].has_value()); + + OptionalValuesVector latest; + store->get(keys, latest, dbName); + ASSERT_EQ(latest.size(), 2); + EXPECT_EQ(latest[0].value(), ValuesVector{ updated }); + EXPECT_EQ(latest[1].value(), ValuesVector{ addedValue }); +} + +TEST_F(LMDBStoreTest, can_read_duplicates_against_a_shared_read_transaction) +{ + LMDBStore::Ptr store = create_store(); + const std::string dbName = "Test Database"; + store->open_database(dbName, true); + + int64_t numKeys = 5; + int64_t numValues = 3; + write_test_data({ dbName }, numKeys, numValues, *store); + + LMDBStore::ReadTransaction::SharedPtr tx = store->create_shared_read_transaction(); + + // adds a further duplicate to the key once the read transaction is open + KeyOptionalValuesVector toDelete; + KeyDupValuesVector toWrite = { { { get_key(2), { get_value(2, numValues) } } } }; + std::vector putDatas = { { toWrite, toDelete, dbName } }; + store->put(putDatas); + + KeysVector keys = { { get_key(2) } }; + OptionalValuesVector values; + store->get(keys, values, dbName, tx); + + ValuesVector expected; + for (int64_t i = 0; i < numValues; i++) { + expected.emplace_back(get_value(2, i)); + } + ASSERT_EQ(values.size(), 1); + ASSERT_TRUE(values[0].has_value()); + EXPECT_EQ(values[0].value(), expected); + + OptionalValuesVector latest; + store->get(keys, latest, dbName); + expected.emplace_back(get_value(2, numValues)); + ASSERT_TRUE(latest[0].has_value()); + EXPECT_EQ(latest[0].value(), expected); +} + +TEST_F(LMDBStoreTest, can_share_a_read_transaction_between_gets_and_cursors) +{ + LMDBStore::Ptr store = create_store(); + const std::string dbName = "Test Database"; + store->open_database(dbName); + + int64_t numKeys = 5; + write_test_data({ dbName }, numKeys, 1, *store); + + LMDBStore::ReadTransaction::SharedPtr tx = store->create_shared_read_transaction(); + LMDBStore::Cursor::Ptr cursor = store->create_cursor(tx, dbName); + + KeyOptionalValuesVector toDelete = { { get_key(0), std::nullopt } }; + KeyDupValuesVector toWrite; + std::vector putDatas = { { toWrite, toDelete, dbName } }; + store->put(putDatas); + + KeysVector keys = { { get_key(0) } }; + OptionalValuesVector values; + store->get(keys, values, dbName, tx); + ASSERT_TRUE(values[0].has_value()); + EXPECT_EQ(values[0].value(), ValuesVector{ get_value(0, 0) }); + + EXPECT_TRUE(cursor->set_at_start()); + KeyDupValuesVector entries; + cursor->read_next(static_cast(numKeys), entries); + ASSERT_EQ(entries.size(), static_cast(numKeys)); + EXPECT_EQ(entries[0].first, get_key(0)); +} + +TEST_F(LMDBStoreTest, try_create_shared_read_transaction_does_not_wait_and_keeps_permits_free) +{ + const uint32_t maxReaders = 3; + LMDBStore::Ptr store = std::make_unique(_directory, _mapSize, maxReaders, 1); + + std::vector held; + for (uint32_t i = 0; i < maxReaders - 1; i++) { + held.push_back(store->try_create_shared_read_transaction(1)); + ASSERT_NE(held.back(), nullptr); + } + EXPECT_EQ(store->try_create_shared_read_transaction(1), nullptr); + + held.push_back(store->try_create_shared_read_transaction()); + ASSERT_NE(held.back(), nullptr); + EXPECT_EQ(store->try_create_shared_read_transaction(), nullptr); + + held.pop_back(); + held.pop_back(); + EXPECT_NE(store->try_create_shared_read_transaction(1), nullptr); +} + +TEST_F(LMDBStoreTest, reader_permit_is_returned_when_a_read_transaction_fails_to_begin) +{ + const uint32_t maxReaders = 2; + int toParent[2]; + int toChild[2]; + ASSERT_EQ(pipe(toParent), 0); + ASSERT_EQ(pipe(toChild), 0); + + // Another process occupies every slot in the environment's reader table, so mdb_txn_begin fails here + pid_t pid = fork(); + ASSERT_GE(pid, 0); + if (pid == 0) { + { + auto env = std::make_shared(_directory, _mapSize, 1, maxReaders); + std::vector> txs; + for (uint32_t i = 0; i < maxReaders; i++) { + env->wait_for_reader(); + txs.push_back(std::make_unique(env)); + } + char c = 'r'; + if (write(toParent[1], &c, 1) != 1 || read(toChild[0], &c, 1) != 1) { + _exit(1); + } + } + _exit(0); + } + + char c = 0; + ASSERT_EQ(read(toParent[0], &c, 1), 1); + + LMDBStore::Ptr store = std::make_unique(_directory, _mapSize, maxReaders, 1); + // more failed attempts than there are permits, so a leaked permit would surface as a nullptr here + EXPECT_THROW(store->create_shared_read_transaction(), std::runtime_error); + for (uint32_t i = 0; i < maxReaders; i++) { + EXPECT_THROW(store->try_create_shared_read_transaction(), std::runtime_error); + } + + ASSERT_EQ(write(toChild[1], &c, 1), 1); + int status = 0; + ASSERT_EQ(waitpid(pid, &status, 0), pid); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(WEXITSTATUS(status), 0); + for (int fd : { toParent[0], toParent[1], toChild[0], toChild[1] }) { + close(fd); + } + + std::vector held; + for (uint32_t i = 0; i < maxReaders; i++) { + held.push_back(store->try_create_shared_read_transaction()); + EXPECT_NE(held.back(), nullptr); + } +} + TEST_F(LMDBStoreTest, can_not_read_from_non_existent_database) { LMDBStore::Ptr store = create_store(); diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.cpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.cpp index 459510da23b..5d436a6f53b 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.cpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.cpp @@ -7,28 +7,60 @@ LMDBStoreBase::LMDBStoreBase( , _environment(std::make_shared(_dbDirectory, mapSizeKb, maxDbs, maxNumReaders, ephemeral)) {} LMDBStoreBase::~LMDBStoreBase() = default; +namespace { +// A transaction returns its permit from its destructor, which never runs if its constructor (mdb_txn_begin) throws, so +// a permit taken before construction has to be given back here +template auto with_permit(LMDBEnvironment& env, void (LMDBEnvironment::*release)(), Make make) +{ + try { + return make(); + } catch (...) { + (env.*release)(); + throw; + } +} +} // namespace + LMDBStoreBase::ReadTransaction::Ptr LMDBStoreBase::create_read_transaction() const { _environment->wait_for_reader(); - return std::make_unique(_environment); + return with_permit(*_environment, &LMDBEnvironment::release_reader, [&] { + return std::make_unique(_environment); + }); } LMDBStoreBase::ReadTransaction::SharedPtr LMDBStoreBase::create_shared_read_transaction() const { _environment->wait_for_reader(); - return std::make_shared(_environment); + return with_permit(*_environment, &LMDBEnvironment::release_reader, [&] { + return std::make_shared(_environment); + }); +} + +LMDBStoreBase::ReadTransaction::SharedPtr LMDBStoreBase::try_create_shared_read_transaction(uint32_t keepFree) const +{ + if (!_environment->try_wait_for_reader(keepFree)) { + return nullptr; + } + return with_permit(*_environment, &LMDBEnvironment::release_reader, [&] { + return std::make_shared(_environment); + }); } LMDBStoreBase::DBCreationTransaction::Ptr LMDBStoreBase::create_db_transaction() const { _environment->wait_for_writer(); - return std::make_unique(_environment); + return with_permit(*_environment, &LMDBEnvironment::release_writer, [&] { + return std::make_unique(_environment); + }); } LMDBStoreBase::WriteTransaction::Ptr LMDBStoreBase::create_write_transaction() const { _environment->wait_for_writer(); - return std::make_unique(_environment); + return with_permit(*_environment, &LMDBEnvironment::release_writer, [&] { + return std::make_unique(_environment); + }); } void LMDBStoreBase::copy_store(const std::string& dstPath, bool compact) diff --git a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.hpp b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.hpp index fd03e29c4ea..237100d8dce 100644 --- a/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.hpp +++ b/barretenberg/cpp/src/barretenberg/lmdblib/lmdb_store_base.hpp @@ -20,6 +20,12 @@ class LMDBStoreBase { virtual ~LMDBStoreBase() = 0; ReadTransaction::Ptr create_read_transaction() const; ReadTransaction::SharedPtr create_shared_read_transaction() const; + /** + * @brief Like create_shared_read_transaction, but never waits for a reader permit. + * @param keepFree How many reader permits must remain available once this transaction holds one + * @return nullptr if no permit could be taken + */ + ReadTransaction::SharedPtr try_create_shared_read_transaction(uint32_t keepFree = 0) const; WriteTransaction::Ptr create_write_transaction() const; LMDBDatabaseCreationTransaction::Ptr create_db_transaction() const; void copy_store(const std::string& dstPath, bool compact); diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_message.hpp b/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_message.hpp index b63bd1849f7..96ac48456a3 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_message.hpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_message.hpp @@ -2,10 +2,14 @@ #include "barretenberg/lmdblib/types.hpp" #include "barretenberg/messaging/header.hpp" #include "barretenberg/serialize/msgpack.hpp" +#include "barretenberg/serialize/msgpack_impl/concepts.hpp" #include "msgpack/adaptor/define_decl.hpp" +#include +#include #include #include #include +#include namespace bb::nodejs::lmdb_store { @@ -28,6 +32,9 @@ enum LMDBStoreMessageType { CLOSE, COPY_STORE, + + START_READ_TX, + CLOSE_READ_TX, }; struct OpenDatabaseRequest { @@ -39,7 +46,9 @@ struct OpenDatabaseRequest { struct GetRequest { lmdblib::KeysVector keys; std::string db; - SERIALIZATION_FIELDS(keys, db); + // When set, read through the snapshot of this open read transaction instead of a fresh one + std::optional txId; + SERIALIZATION_FIELDS(keys, db, txId); }; struct GetResponse { @@ -51,7 +60,9 @@ struct HasRequest { // std::map> entries; lmdblib::KeyOptionalValuesVector entries; std::string db; - SERIALIZATION_FIELDS(entries, db); + // When set, check against the snapshot of this open read transaction instead of a fresh one + std::optional txId; + SERIALIZATION_FIELDS(entries, db, txId); }; struct HasResponse { @@ -78,7 +89,9 @@ struct StartCursorRequest { std::optional count; std::optional onePage; std::string db; - SERIALIZATION_FIELDS(key, reverse, count, onePage, db); + // When set, iterate over the snapshot of this open read transaction instead of a fresh one + std::optional txId; + SERIALIZATION_FIELDS(key, reverse, count, onePage, db, txId); }; struct StartCursorResponse { @@ -139,6 +152,68 @@ struct CopyStoreRequest { SERIALIZATION_FIELDS(dstPath, compact); }; +struct StartReadTxResponse { + uint64_t txId; + SERIALIZATION_FIELDS(txId); +}; + +struct CloseReadTxRequest { + // Optional only so that a request without it is rejected rather than read as an indeterminate id + std::optional txId; + SERIALIZATION_FIELDS(txId); +}; + } // namespace bb::nodejs::lmdb_store MSGPACK_ADD_ENUM(bb::nodejs::lmdb_store::LMDBStoreMessageType) + +namespace bb::nodejs::lmdb_store { + +/** + * @brief Requests with an optional `txId` field. Clients that do not use read transactions omit `txId` from the map + * entirely, but barretenberg's msgpack `define_map` rejects any missing key, so these requests get the dedicated + * adaptor below that reads a missing `txId` as nil. + */ +template +concept OptionalTxIdRequest = msgpack_concepts::HasMsgPack && requires(T t) { + { t.txId } -> std::same_as&>; +}; + +} // namespace bb::nodejs::lmdb_store + +namespace msgpack::adaptor { + +template struct convert { + msgpack::object const& operator()(msgpack::object const& o, T& v) const + { + constexpr std::string_view tx_id_key = "txId"; + msgpack::zone zone; + msgpack::object source = o; + if (o.type == msgpack::type::MAP) { + const msgpack::object_map& map = o.via.map; + bool has_tx_id = false; + for (uint32_t i = 0; i < map.size; ++i) { + const msgpack::object& key = map.ptr[i].key; + if (key.type == msgpack::type::STR && + std::string_view(key.via.str.ptr, key.via.str.size) == tx_id_key) { + has_tx_id = true; + break; + } + } + if (!has_tx_id) { + auto* entries = static_cast(zone.allocate_align( + sizeof(msgpack::object_kv) * (map.size + 1), MSGPACK_ZONE_ALIGNOF(msgpack::object_kv))); + std::copy(map.ptr, map.ptr + map.size, entries); + entries[map.size].key = msgpack::object(tx_id_key.data(), zone); + entries[map.size].val = msgpack::object(); + source.via.map.ptr = entries; + source.via.map.size = map.size + 1; + } + } + v.msgpack( + [&](auto&... args) { msgpack::type::define_map{ args... }.msgpack_unpack(source); }); + return o; + } +}; + +} // namespace msgpack::adaptor diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.cpp b/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.cpp index b891f0e3f11..48bd061fb67 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.cpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.cpp @@ -6,9 +6,11 @@ #include #include #include +#include #include #include #include +#include #include using namespace bb::nodejs; @@ -17,6 +19,9 @@ using namespace bb::nodejs::lmdb_store; const uint64_t DEFAULT_MAP_SIZE = 1024UL * 1024; const uint64_t DEFAULT_MAX_READERS = 16; const uint64_t DEFAULT_CURSOR_PAGE_SIZE = 10; +// Reader permits a new read transaction must leave free, so that requests which open their own short-lived read +// transaction can always make progress while long-lived ones are held open +const uint32_t READ_TX_RESERVED_READERS = 1; LMDBStoreWrapper::LMDBStoreWrapper(const Napi::CallbackInfo& info) : ObjectWrap(info) @@ -91,6 +96,9 @@ LMDBStoreWrapper::LMDBStoreWrapper(const Napi::CallbackInfo& info) _msg_processor.register_handler(LMDBStoreMessageType::CLOSE, this, &LMDBStoreWrapper::close, true); _msg_processor.register_handler(LMDBStoreMessageType::COPY_STORE, this, &LMDBStoreWrapper::copy_store, true); + + _msg_processor.register_handler(LMDBStoreMessageType::START_READ_TX, this, &LMDBStoreWrapper::start_read_tx); + _msg_processor.register_handler(LMDBStoreMessageType::CLOSE_READ_TX, this, &LMDBStoreWrapper::close_read_tx); } Napi::Value LMDBStoreWrapper::call(const Napi::CallbackInfo& info) @@ -116,6 +124,16 @@ void LMDBStoreWrapper::verify_store() const throw std::runtime_error(format("LMDB store unavailable, was close already called?")); } +ReadTxData LMDBStoreWrapper::get_read_tx(uint64_t id) +{ + std::lock_guard lock(_read_tx_mutex); + auto it = _read_txs.find(id); + if (it == _read_txs.end()) { + throw std::runtime_error(format("Read transaction ", id, " not found, was it already closed?")); + } + return it->second; +} + BoolResponse LMDBStoreWrapper::open_database(const OpenDatabaseRequest& req) { verify_store(); @@ -123,12 +141,55 @@ BoolResponse LMDBStoreWrapper::open_database(const OpenDatabaseRequest& req) return { true }; } +StartReadTxResponse LMDBStoreWrapper::start_read_tx() +{ + verify_store(); + // Waiting for a permit here could park every libuv worker, starving the CLOSE_READ_TX that would free one + auto tx = _store->try_create_shared_read_transaction(READ_TX_RESERVED_READERS); + if (!tx) { + throw std::runtime_error("No reader slot available to start a read transaction, close an open read " + "transaction or cursor and retry"); + } + ReadTxData data{ std::move(tx), std::make_shared() }; + uint64_t id = data.tx->id(); + { + std::lock_guard lock(_read_tx_mutex); + _read_txs.emplace(id, std::move(data)); + } + return { id }; +} + +BoolResponse LMDBStoreWrapper::close_read_tx(const CloseReadTxRequest& req) +{ + if (!req.txId.has_value()) { + throw std::runtime_error("CLOSE_READ_TX request is missing the read transaction id"); + } + // Moved out so that, if this was the last reference, the transaction is aborted after the registry lock is released + ReadTxData closed; + { + std::lock_guard lock(_read_tx_mutex); + auto it = _read_txs.find(req.txId.value()); + if (it == _read_txs.end()) { + return { false }; + } + closed = std::move(it->second); + _read_txs.erase(it); + } + return { true }; +} + GetResponse LMDBStoreWrapper::get(const GetRequest& req) { verify_store(); lmdblib::OptionalValuesVector vals; lmdblib::KeysVector keys = req.keys; - _store->get(keys, vals, req.db); + if (req.txId.has_value()) { + ReadTxData data = get_read_tx(req.txId.value()); + std::lock_guard tx_lock(*data.mtx); + _store->get(keys, vals, req.db, data.tx); + } else { + _store->get(keys, vals, req.db); + } return { vals }; } @@ -136,7 +197,13 @@ HasResponse LMDBStoreWrapper::has(const HasRequest& req) { verify_store(); std::vector exists; - _store->has(req.entries, exists, req.db); + if (req.txId.has_value()) { + ReadTxData data = get_read_tx(req.txId.value()); + std::lock_guard tx_lock(*data.mtx); + _store->has(req.entries, exists, req.db, data.tx); + } else { + _store->has(req.entries, exists, req.db); + } return { exists }; } @@ -148,29 +215,29 @@ StartCursorResponse LMDBStoreWrapper::start_cursor(const StartCursorRequest& req bool one_page = req.onePage.value_or(false); lmdblib::Key key = req.key; - auto tx = _store->create_shared_read_transaction(); - lmdblib::LMDBCursor::SharedPtr cursor = _store->create_cursor(tx, req.db); - bool start_ok = cursor->set_at_key(key); + lmdblib::LMDBReadTransaction::SharedPtr tx; + std::shared_ptr tx_mtx; + if (req.txId.has_value()) { + ReadTxData data = get_read_tx(req.txId.value()); + tx = std::move(data.tx); + tx_mtx = std::move(data.mtx); + } else { + tx = _store->create_shared_read_transaction(); + tx_mtx = std::make_shared(); + } - if (!start_ok) { - // we couldn't find exactly the requested key. Find the next biggest one. - start_ok = cursor->set_at_key_gte(key); - // if we found a key that's greater _and_ we want to go in reverse order - // then we're actually outside the requested bounds, we need to go back one position - if (start_ok && reverse) { - lmdblib::KeyDupValuesVector entries; - // read_prev returns `true` if there's nothing more to read - // turn this into a "not ok" because there's nothing in the db for this cursor to read - start_ok = !cursor->read_prev(1, entries); - } else if (!start_ok && reverse) { - // we couldn't find a key greater than our starting point _and_ we want to go in reverse.. - // then we start at the end of the database (the client requested to start at a key greater than anything in - // the DB) - start_ok = cursor->set_at_end(); + lmdblib::LMDBCursor::SharedPtr cursor; + bool start_ok = false; + bool done = false; + lmdblib::KeyDupValuesVector first_page; + { + // Opening a cursor can update the transaction's database state, so it needs the lock as much as reads do + std::lock_guard tx_lock(*tx_mtx); + cursor = _store->create_cursor(tx, req.db); + start_ok = _set_cursor_start(*cursor, key, reverse); + if (start_ok) { + std::tie(done, first_page) = _advance_cursor(*cursor, reverse, page_size); } - - // in case we're iterating in ascending order and we can't find the exact key or one that's greater than it - // then that means theren's nothing in the DB for the cursor to read } // we couldn't find a starting position @@ -178,7 +245,6 @@ StartCursorResponse LMDBStoreWrapper::start_cursor(const StartCursorRequest& req return { std::nullopt, {} }; } - auto [done, first_page] = _advance_cursor(*cursor, reverse, page_size); // cursor finished after reading a single page or client only wanted the first page if (done || one_page) { return { std::nullopt, first_page }; @@ -187,12 +253,40 @@ StartCursorResponse LMDBStoreWrapper::start_cursor(const StartCursorRequest& req auto cursor_id = cursor->id(); { std::lock_guard lock(_cursor_mutex); - _cursors[cursor_id] = { cursor, reverse }; + _cursors[cursor_id] = { cursor, reverse, tx_mtx }; } return { cursor_id, first_page }; } +bool LMDBStoreWrapper::_set_cursor_start(const lmdblib::LMDBCursor& cursor, lmdblib::Key& key, bool reverse) +{ + bool start_ok = cursor.set_at_key(key); + + if (!start_ok) { + // we couldn't find exactly the requested key. Find the next biggest one. + start_ok = cursor.set_at_key_gte(key); + // if we found a key that's greater _and_ we want to go in reverse order + // then we're actually outside the requested bounds, we need to go back one position + if (start_ok && reverse) { + lmdblib::KeyDupValuesVector entries; + // read_prev returns `true` if there's nothing more to read + // turn this into a "not ok" because there's nothing in the db for this cursor to read + start_ok = !cursor.read_prev(1, entries); + } else if (!start_ok && reverse) { + // we couldn't find a key greater than our starting point _and_ we want to go in reverse.. + // then we start at the end of the database (the client requested to start at a key greater than anything in + // the DB) + start_ok = cursor.set_at_end(); + } + + // in case we're iterating in ascending order and we can't find the exact key or one that's greater than it + // then that means theren's nothing in the DB for the cursor to read + } + + return start_ok; +} + BoolResponse LMDBStoreWrapper::close_cursor(const CloseCursorRequest& req) { { @@ -212,6 +306,7 @@ AdvanceCursorResponse LMDBStoreWrapper::advance_cursor(const AdvanceCursorReques } uint32_t page_size = req.count.value_or(DEFAULT_CURSOR_PAGE_SIZE); + std::lock_guard tx_lock(*data.txMtx); auto [done, entries] = _advance_cursor(*data.cursor, data.reverse, page_size); return { entries, done }; } @@ -225,6 +320,7 @@ AdvanceCursorCountResponse LMDBStoreWrapper::advance_cursor_count(const AdvanceC data = _cursors.at(req.cursor); } + std::lock_guard tx_lock(*data.txMtx); auto [done, count] = _advance_cursor_count(*data.cursor, data.reverse, req.endKey); return { count, done }; } @@ -267,6 +363,12 @@ BoolResponse LMDBStoreWrapper::close() _cursors.clear(); } + { + // and all of the read transactions still held open on behalf of the JS side + std::lock_guard read_txs(_read_tx_mutex); + _read_txs.clear(); + } + // and finally close the database handle _store.reset(nullptr); diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.hpp b/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.hpp index 4fea656edc5..f8e05a60139 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.hpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/lmdb_store/lmdb_store_wrapper.hpp @@ -9,14 +9,28 @@ #include "barretenberg/nodejs_module/util/message_processor.hpp" #include #include +#include #include #include +#include namespace bb::nodejs::lmdb_store { +/** + * @brief A read transaction held open on behalf of the JavaScript side, together with the mutex that serializes + * access to it. The environment is opened with `MDB_NOTLS`, so a read transaction may move between the libuv worker + * threads, but it must never be used by two of them at once. + */ +struct ReadTxData { + lmdblib::LMDBReadTransaction::SharedPtr tx; + std::shared_ptr mtx; +}; + struct CursorData { lmdblib::LMDBCursor::SharedPtr cursor; bool reverse; + // Guards the cursor's read transaction, which may be shared with gets and other cursors + std::shared_ptr txMtx; }; /** * @brief Manages the interaction between the JavaScript runtime and the LMDB instance. @@ -38,12 +52,38 @@ class LMDBStoreWrapper : public Napi::ObjectWrap { std::mutex _cursor_mutex; std::unordered_map _cursors; + std::mutex _read_tx_mutex; + std::unordered_map _read_txs; + bb::nodejs::AsyncMessageProcessor _msg_processor; void verify_store() const; + /** + * @brief Returns a copy of the registered read transaction, which keeps it alive even if it is closed concurrently. + * @throws std::runtime_error if no read transaction with this id is open + */ + ReadTxData get_read_tx(uint64_t id); + BoolResponse open_database(const OpenDatabaseRequest& req); + /** + * @brief Opens a read transaction that stays open until CLOSE_READ_TX (or CLOSE) and returns its id. It holds one + * of the environment's reader slots for its whole lifetime. + * @note The free slot is only checked here: a START_CURSOR without txId still waits for a slot and holds it until + * the cursor closes. Callers must keep open read transactions plus open cursors below the reader count, or plain + * reads can wait on every libuv worker at once, leaving no worker to run the close that would free a slot. + * @throws std::runtime_error without waiting if taking a slot would leave none free for other reads + */ + StartReadTxResponse start_read_tx(); + + /** + * @brief Unregisters a read transaction. Cursors already opened against it keep it alive until they are closed. + * @return ok is false if no read transaction with this id was open + * @throws std::runtime_error if the request carries no id + */ + BoolResponse close_read_tx(const CloseReadTxRequest& req); + GetResponse get(const GetRequest& req); HasResponse has(const HasRequest& req); @@ -60,6 +100,8 @@ class LMDBStoreWrapper : public Napi::ObjectWrap { BoolResponse copy_store(const CopyStoreRequest& req); + static bool _set_cursor_start(const lmdblib::LMDBCursor& cursor, lmdblib::Key& key, bool reverse); + static std::pair _advance_cursor(const lmdblib::LMDBCursor& cursor, bool reverse, uint64_t page_size); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/verifier_commitment_key.hpp b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/verifier_commitment_key.hpp index 6c07fb3b19d..317400d7dae 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/verifier_commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/verifier_commitment_key.hpp @@ -39,7 +39,7 @@ template class VerifierCommitmentKey { } } - std::vector get_monomial_points() const { return monomial_points; } + const std::vector& get_monomial_points() const { return monomial_points; } private: std::vector monomial_points; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp index a2b9ca13a4e..10d2f2446e2 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp @@ -162,6 +162,23 @@ template class CircuitBuilderBase { const std::vector& get_variables() const { return variables; } + /** + * @brief Release the witness values, the copy-constraint bookkeeping (copy cycles, tags and the tag permutation) + * and the variable names. + * @details For a builder whose circuit has been copied into a prover instance; the variables cannot be read + * afterwards. + */ + void free_variables() + { + std::vector().swap(variables); + std::vector().swap(next_var_index); + std::vector().swap(prev_var_index); + std::vector().swap(real_variable_index); + std::vector().swap(real_variable_tags); + std::unordered_map().swap(_tau); + std::unordered_map().swap(variable_names); + } + /** * @brief Get the value of the variable v_{index} * @param index The index of the variable diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp index b1823ebcd21..87d4f3947cd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp @@ -186,6 +186,16 @@ template class MegaCircuitBuilder_ : public UltraCircuitBuilder_::free_construction_data(); + databus = DataBus{}; + } + /** * @brief Compute a hash of the circuit * @details Hashes all wires and selectors from each block. Note that this encompases all gate data, copy diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp index 5a9ca0c023d..d806c2d5ca5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp @@ -2034,6 +2034,24 @@ template msgpack::sbuffer UltraCircuitBuilder_ void UltraCircuitBuilder_::free_construction_data() +{ + for (auto& block : blocks.get()) { + block.free_data(); + } + this->free_variables(); + std::deque().swap(lookup_tables); + rom_ram_logic = RomRamLogic{}; + std::vector().swap(memory_read_records); + std::vector().swap(memory_write_records); + std::vector().swap(rom_logup_records); + range_lists.clear(); + std::vector().swap(cached_partial_non_native_field_multiplications); + std::unordered_map().swap(constant_variable_indices); + std::vector().swap(used_witnesses); + std::unordered_set().swap(finalize_witnesses); +} + template class UltraCircuitBuilder_; template class UltraCircuitBuilder_; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp index 2fa2189881f..32eb29c3eae 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp @@ -220,6 +220,15 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase