diff --git a/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp b/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp index 6867c97e9db1..75c822f111e7 100644 --- a/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp +++ b/barretenberg/cpp/src/barretenberg/bbapi/bbapi.test.cpp @@ -1,6 +1,7 @@ #include "barretenberg/bbapi/bbapi.hpp" #include "barretenberg/api/file_io.hpp" #include "barretenberg/bbapi/bbapi_shared.hpp" +#include "barretenberg/chonk/private_execution_steps.hpp" #include "barretenberg/common/serialize.hpp" #include "barretenberg/common/utils.hpp" #include "barretenberg/serialize/test_helper.hpp" @@ -113,3 +114,73 @@ TEST(BBApiInputValidation, VkWithCorrectSizeAccepted) std::vector good_vk(expected_size, 0); EXPECT_NO_THROW(bbapi::validate_vk_size(good_vk)); } + +// Helper: pack a vector of PrivateExecutionStepRaw into a byte buffer via msgpack. +namespace { +std::vector pack_steps(const std::vector& steps) +{ + std::stringstream ss; + msgpack::pack(ss, steps); + const std::string s = ss.str(); + return { s.begin(), s.end() }; +} +} // namespace + +TEST(BBApiInputValidation, MsgpackParseUncompressedAcceptsCleanInput) +{ + PrivateExecutionStepRaw step{ + .bytecode = { 0xCA, 0xFE }, .witness = { 0xBE, 0xEF }, .vk = {}, .function_name = "test_fn" + }; + + auto buf = pack_steps({ step }); + auto result = PrivateExecutionStepRaw::parse_uncompressed(buf); + + ASSERT_EQ(result.size(), 1); + EXPECT_EQ(result[0].bytecode, step.bytecode); + EXPECT_EQ(result[0].witness, step.witness); + EXPECT_EQ(result[0].function_name, "test_fn"); +} + +TEST(BBApiInputValidation, MsgpackParseUncompressedRejectsTrailingData) +{ + PrivateExecutionStepRaw step{ .bytecode = {}, .witness = {}, .vk = {}, .function_name = "x" }; + + auto buf = pack_steps({ step }); + buf.push_back(0x00); + + EXPECT_THROW(PrivateExecutionStepRaw::parse_uncompressed(buf), std::invalid_argument); +} + +TEST(BBApiInputValidation, MsgpackLoadAcceptsCleanFile) +{ + PrivateExecutionStepRaw step{ .bytecode = { 1, 2, 3 }, .witness = { 4, 5 }, .vk = {}, .function_name = "file_fn" }; + + auto buf = pack_steps({ step }); + + auto tmp = std::filesystem::temp_directory_path() / "bb_test_clean.msgpack"; + std::ofstream out(tmp, std::ios::binary); + out.write(reinterpret_cast(buf.data()), static_cast(buf.size())); + out.close(); + + auto result = PrivateExecutionStepRaw::load(tmp); + std::filesystem::remove(tmp); + + ASSERT_EQ(result.size(), 1); + EXPECT_EQ(result[0].function_name, "file_fn"); +} + +TEST(BBApiInputValidation, MsgpackLoadRejectsTrailingData) +{ + PrivateExecutionStepRaw step{ .bytecode = {}, .witness = {}, .vk = {}, .function_name = "x" }; + + auto buf = pack_steps({ step }); + buf.push_back(0x00); + + auto tmp = std::filesystem::temp_directory_path() / "bb_test_tailed.msgpack"; + std::ofstream out(tmp, std::ios::binary); + out.write(reinterpret_cast(buf.data()), static_cast(buf.size())); + out.close(); + + EXPECT_THROW(PrivateExecutionStepRaw::load(tmp), std::invalid_argument); + std::filesystem::remove(tmp); +} diff --git a/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp b/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp index beb335c22c72..d84d01155252 100644 --- a/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp +++ b/barretenberg/cpp/src/barretenberg/chonk/private_execution_steps.cpp @@ -86,7 +86,12 @@ template T unpack_from_file(const std::filesystem::path& filename) T result; std::string encoded_data(fsize, '\0'); fin.read(encoded_data.data(), static_cast(fsize)); - msgpack::unpack(encoded_data.data(), fsize).get().convert(result); + std::size_t offset = 0; + msgpack::unpack(encoded_data.data(), fsize, offset).get().convert(result); + if (offset != fsize) { + THROW std::invalid_argument("msgpack input has trailing data (" + std::to_string(fsize - offset) + + " extra bytes)"); + } return result; } @@ -121,7 +126,12 @@ std::vector PrivateExecutionStepRaw::parse_uncompressed { std::vector raw_steps; // Read with msgpack - msgpack::unpack(reinterpret_cast(buf.data()), buf.size()).get().convert(raw_steps); + std::size_t offset = 0; + msgpack::unpack(reinterpret_cast(buf.data()), buf.size(), offset).get().convert(raw_steps); + if (offset != buf.size()) { + THROW std::invalid_argument("msgpack input has trailing data (" + std::to_string(buf.size() - offset) + + " extra bytes)"); + } // Unlike load_and_decompress, we don't need to decompress the bytecode and witness fields return raw_steps; } diff --git a/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp b/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp index 235dba268daa..bc97af4d891d 100644 --- a/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp +++ b/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp @@ -106,6 +106,7 @@ template class Polynomial { */ static Polynomial shiftable(size_t virtual_size) { + BB_ASSERT_GTE(virtual_size, NUM_ZERO_ROWS, "shiftable virtual_size must be >= NUM_ZERO_ROWS"); return Polynomial( /*actual size*/ virtual_size - NUM_ZERO_ROWS, virtual_size, /*shiftable offset*/ NUM_ZERO_ROWS); } @@ -114,6 +115,7 @@ template class Polynomial { */ static Polynomial shiftable(size_t size, size_t virtual_size) { + BB_ASSERT_GTE(size, NUM_ZERO_ROWS, "shiftable size must be >= NUM_ZERO_ROWS"); return Polynomial(/*actual size*/ size - NUM_ZERO_ROWS, virtual_size, /*shiftable offset*/ NUM_ZERO_ROWS); } // Allow polynomials to be entirely reset/dormant diff --git a/barretenberg/cpp/src/barretenberg/polynomials/polynomial_arithmetic.cpp b/barretenberg/cpp/src/barretenberg/polynomials/polynomial_arithmetic.cpp index 3e2e5b7ddbd5..7fd42d2926a1 100644 --- a/barretenberg/cpp/src/barretenberg/polynomials/polynomial_arithmetic.cpp +++ b/barretenberg/cpp/src/barretenberg/polynomials/polynomial_arithmetic.cpp @@ -12,6 +12,7 @@ #include #include #include +#include namespace bb::polynomial_arithmetic { @@ -19,6 +20,8 @@ namespace { template std::shared_ptr get_scratch_space(const size_t num_elements) { + static std::mutex scratch_mutex; + std::lock_guard lock(scratch_mutex); static std::shared_ptr working_memory = nullptr; static size_t current_size = 0; if (num_elements > current_size) { @@ -52,6 +55,8 @@ void scale_by_generator(Fr* coeffs, const Fr& generator_shift, const size_t generator_size) { + BB_ASSERT(generator_size % domain.num_threads == 0, + "generator_size must be divisible by num_threads to avoid silently skipping elements"); parallel_for(domain.num_threads, [&](size_t j) { Fr thread_shift = generator_shift.pow(static_cast(j * (generator_size / domain.num_threads))); Fr work_generator = generator_start * thread_shift; diff --git a/barretenberg/cpp/src/barretenberg/polynomials/row_disabling_polynomial.hpp b/barretenberg/cpp/src/barretenberg/polynomials/row_disabling_polynomial.hpp index ca1983f4535f..bd0c29241a4a 100644 --- a/barretenberg/cpp/src/barretenberg/polynomials/row_disabling_polynomial.hpp +++ b/barretenberg/cpp/src/barretenberg/polynomials/row_disabling_polynomial.hpp @@ -170,7 +170,7 @@ template struct RowDisablingPolynomial { * @param log_circuit_size * @return FF */ - static FF evaluate_at_challenge(std::vector multivariate_challenge, const size_t log_circuit_size) + static FF evaluate_at_challenge(std::span multivariate_challenge, const size_t log_circuit_size) { FF evaluation_at_multivariate_challenge{ 1 }; @@ -188,7 +188,7 @@ template struct RowDisablingPolynomial { * @param padding_indicator_array An array with first log_n entries equal to 1, and the remaining entries are 0. * @return FF */ - static FF evaluate_at_challenge(std::span multivariate_challenge, + static FF evaluate_at_challenge(std::span multivariate_challenge, const std::vector& padding_indicator_array) { FF evaluation_at_multivariate_challenge{ 1 }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index ab689499cd84..b1d7ad6b01ca 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1102,7 +1102,7 @@ void field_t::evaluate_linear_identity( Builder* ctx = validate_context(a.context, b.context, c.context, d.context); if (a.is_constant() && b.is_constant() && c.is_constant() && d.is_constant()) { - BB_ASSERT_EQ(a.get_value() + b.get_value() + c.get_value() + d.get_value(), 0); + BB_ASSERT_EQ(a.get_value() + b.get_value() + c.get_value() + d.get_value(), 0, msg); return; } @@ -1136,7 +1136,7 @@ void field_t::evaluate_polynomial_identity( const field_t& a, const field_t& b, const field_t& c, const field_t& d, const std::string& msg) { if (a.is_constant() && b.is_constant() && c.is_constant() && d.is_constant()) { - BB_ASSERT((a.get_value() * b.get_value() + c.get_value() + d.get_value()).is_zero()); + BB_ASSERT((a.get_value() * b.get_value() + c.get_value() + d.get_value()).is_zero(), msg); return; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp index 249ff0488bdc..fa25cca7e73e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp @@ -332,11 +332,12 @@ template class field_t { // (this.v * this.mul + this.add) * inverse.v == 1; // created by applying `assert_is_not_zero` to `*this` coincides with the constraint created by // `divide_no_zero_check`, hence we can safely apply the latter instead of `/` operator. - auto* ctx = get_context(); if (is_constant()) { BB_ASSERT(!get_value().is_zero(), "field_t::invert denominator is constant 0"); + return field_t(fr::one()).divide_no_zero_check(*this); } + auto* ctx = get_context(); if (get_value().is_zero() && !ctx->failed()) { ctx->failure("field_t::invert denominator is 0"); }