Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,14 @@ ChonkStart::Response ChonkStart::execute(BBApiRequest& request) &&
BB_BENCH_NAME(MSGPACK_SCHEMA_NAME);

request.ivc_in_progress = std::make_shared<Chonk>(num_circuits);

request.ivc_stack_depth = 0;

// Clear any stale loaded-circuit state from a previous session so that
// ChonkAccumulate cannot silently reuse a circuit loaded before this ChonkStart.
request.loaded_circuit_name.clear();
request.loaded_circuit_constraints.reset();
request.loaded_circuit_vk.clear();

return Response{};
}

Expand Down Expand Up @@ -63,6 +69,15 @@ ChonkAccumulate::Response ChonkAccumulate::execute(BBApiRequest& request) &&
acir_format::WitnessVector witness_data = acir_format::witness_buf_to_witness_vector(std::move(witness));
acir_format::AcirProgram program{ std::move(request.loaded_circuit_constraints.value()), std::move(witness_data) };

// Clear loaded state immediately after moving out of it. This ensures that if any subsequent
// step throws, the request won't appear to still have a valid circuit loaded (the optional
// would be in a moved-from state, which is technically has_value()==true but poisoned).
auto loaded_vk = std::move(request.loaded_circuit_vk);
auto circuit_name = std::move(request.loaded_circuit_name);
request.loaded_circuit_constraints.reset();
request.loaded_circuit_vk.clear();
request.loaded_circuit_name.clear();

const acir_format::ProgramMetadata metadata{ .ivc = request.ivc_in_progress };
auto circuit = acir_format::create_circuit<IVCBase::ClientCircuit>(program, metadata);

Expand All @@ -71,17 +86,17 @@ ChonkAccumulate::Response ChonkAccumulate::execute(BBApiRequest& request) &&
if (request.vk_policy == VkPolicy::RECOMPUTE) {
precomputed_vk = nullptr;
} else if (request.vk_policy == VkPolicy::DEFAULT || request.vk_policy == VkPolicy::CHECK) {
if (!request.loaded_circuit_vk.empty()) {
validate_vk_size<Chonk::MegaVerificationKey>(request.loaded_circuit_vk);
precomputed_vk = from_buffer<std::shared_ptr<Chonk::MegaVerificationKey>>(request.loaded_circuit_vk);
if (!loaded_vk.empty()) {
validate_vk_size<Chonk::MegaVerificationKey>(loaded_vk);
precomputed_vk = from_buffer<std::shared_ptr<Chonk::MegaVerificationKey>>(loaded_vk);

if (request.vk_policy == VkPolicy::CHECK) {
auto prover_instance = std::make_shared<Chonk::ProverInstance>(circuit);
auto computed_vk = std::make_shared<Chonk::MegaVerificationKey>(prover_instance->get_precomputed());

// Dereference to compare VK contents
if (*precomputed_vk != *computed_vk) {
throw_or_abort("VK check failed for circuit '" + request.loaded_circuit_name +
throw_or_abort("VK check failed for circuit '" + circuit_name +
"': provided VK does not match computed VK");
}
}
Expand All @@ -90,16 +105,13 @@ ChonkAccumulate::Response ChonkAccumulate::execute(BBApiRequest& request) &&
throw_or_abort("Invalid VK policy. Valid options: default, check, recompute");
}

info("ChonkAccumulate - accumulating circuit '", request.loaded_circuit_name, "'");
info("ChonkAccumulate - accumulating circuit '", circuit_name, "'");
if (detail::use_memory_profile) {
detail::GLOBAL_MEMORY_PROFILE.set_circuit_name(request.loaded_circuit_name);
detail::GLOBAL_MEMORY_PROFILE.set_circuit_name(circuit_name);
}
request.ivc_in_progress->accumulate(circuit, precomputed_vk);
request.ivc_stack_depth++;

request.loaded_circuit_constraints.reset();
request.loaded_circuit_vk.clear();

return Response{};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ typename BatchedHonkTranslatorVerifier_<Curve>::OinkResult BatchedHonkTranslator
mega_zk_verifier_instance = std::make_shared<MegaZKVerifierInstance>(mega_zk_vk_and_hash);

// Derive num_public_inputs from the Oink-only MegaZK proof.
if (mega_zk_proof.size() < ProofLength::Oink<MegaZKFlavorT>::LENGTH_WITHOUT_PUB_INPUTS) {
throw_or_abort("MegaZK Oink proof too short to derive num_public_inputs");
}
const size_t num_public_inputs = mega_zk_proof.size() - ProofLength::Oink<MegaZKFlavorT>::LENGTH_WITHOUT_PUB_INPUTS;

OinkVerifier<MegaZKFlavorT> oink_verifier{ mega_zk_verifier_instance, transcript, num_public_inputs };
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/chonk/chonk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ void Chonk::update_native_verifier_accumulator(const VerifierInputs& queue_entry
info("Chonk accumulate: hash of verifier accumulator computed natively set in previous kernel IO: ",
native_verifier_accum_hash);
}
has_last_app_been_accumulated = num_circuits_accumulated + 1 == num_circuits - 4;
has_last_app_been_accumulated = num_circuits_accumulated + 1 == num_circuits - 3;
is_previous_circuit_a_kernel = queue_entry.is_kernel;

info("======= END OF DEBUGGING INFO FOR NATIVE FOLDING STEP =======");
Expand Down
5 changes: 3 additions & 2 deletions barretenberg/cpp/src/barretenberg/chonk/chonk.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,9 @@ TEST_F(ChonkTests, MsgpackProofFromFileOrBuffer)
std::generate(random_bytes.begin(), random_bytes.end(), []() { return static_cast<uint8_t>(rand() % 256); });
std::copy(random_bytes.begin(), random_bytes.end(), buffer.data());

// Expect deserialization to fail with error msgpack::v1::type_error with description "std::bad_cast"
EXPECT_THROW(ChonkProof::from_msgpack_buffer(buffer), msgpack::v1::type_error);
// Expect deserialization to fail (either msgpack parse error, type mismatch, trailing data,
// or non-canonical field encoding)
EXPECT_ANY_THROW(ChonkProof::from_msgpack_buffer(buffer));
}
};

Expand Down
4 changes: 3 additions & 1 deletion barretenberg/cpp/src/barretenberg/chonk/chonk_proof.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ ChonkProof_<IsRecursive> ChonkProof_<IsRecursive>::from_field_elements(const std
// MegaZK Oink proof size = total - all other fixed-size components.
// This correctly accounts for any ACIR public inputs prepended to the oink portion.
constexpr size_t fixed_total = merge_size + eccvm_size + ipa_size + joint_size;
BB_ASSERT_GTE(fields.size(), fixed_total, "ChonkProof::from_field_elements: proof too short");
if (fields.size() < fixed_total) {
throw_or_abort("ChonkProof::from_field_elements: proof too short");
}
const size_t mega_zk_oink_length = fields.size() - fixed_total;

auto it = fields.begin();
Expand Down
10 changes: 7 additions & 3 deletions barretenberg/cpp/src/barretenberg/chonk/chonk_proof.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,14 @@ template <bool IsRecursive = false> struct ChonkProof_ {
static ChonkProof_ from_msgpack_buffer(const msgpack::sbuffer& buffer)
requires(!IsRecursive)
{
msgpack::object_handle oh = msgpack::unpack(buffer.data(), buffer.size());
msgpack::object obj = oh.get();
std::size_t offset = 0;
msgpack::object_handle oh = msgpack::unpack(buffer.data(), buffer.size(), offset);
if (offset != buffer.size()) {
throw_or_abort("ChonkProof::from_msgpack_buffer: trailing data (" + std::to_string(buffer.size() - offset) +
" extra bytes)");
}
ChonkProof_ proof;
obj.convert(proof);
oh.get().convert(proof);
return proof;
}

Expand Down
44 changes: 33 additions & 11 deletions barretenberg/cpp/src/barretenberg/chonk/proof_compression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ class ProofCompressor {

static uint256_t read_u256(const std::vector<uint8_t>& data, size_t& pos)
{
BB_ASSERT(pos + 32 <= data.size());
if (pos + 32 > data.size()) {
throw_or_abort("proof_compression: read_u256 out of bounds");
}
uint256_t val{ 0, 0, 0, 0 };
for (int i = 31; i >= 0; --i) {
val.data[i / 8] |= static_cast<uint64_t>(data[pos++]) << (8 * (i % 8));
Expand Down Expand Up @@ -423,10 +425,14 @@ class ProofCompressor {
*/
static size_t compressed_mega_num_public_inputs(size_t compressed_bytes)
{
BB_ASSERT(compressed_bytes % 32 == 0);
if (compressed_bytes % 32 != 0) {
throw_or_abort("proof_compression: compressed size not aligned to 32 bytes");
}
size_t total_elements = compressed_bytes / 32;
size_t fixed_elements = compressed_element_count(0);
BB_ASSERT(total_elements >= fixed_elements);
if (total_elements < fixed_elements) {
throw_or_abort("proof_compression: compressed proof too short");
}
return total_elements - fixed_elements;
}

Expand Down Expand Up @@ -493,7 +499,9 @@ class ProofCompressor {
size_t mega_num_pub_inputs =
proof.hiding_oink_proof.size() - ProofLength::Oink<MegaZKFlavor>::LENGTH_WITHOUT_PUB_INPUTS;
walk_chonk_proof(bn254_scalar, bn254_comm, grumpkin_scalar, grumpkin_comm, mega_num_pub_inputs);
BB_ASSERT(offset == flat.size());
if (offset != flat.size()) {
throw_or_abort("proof_compression: compress did not consume all proof elements");
}
return out;
}

Expand All @@ -505,7 +513,9 @@ class ProofCompressor {
// BN254 callbacks
auto bn254_scalar = [&]() {
uint256_t raw = read_u256(compressed, pos);
BB_ASSERT(raw < Fr::modulus);
if (raw >= Fr::modulus) {
throw_or_abort("proof_compression: BN254 scalar out of range");
}
flat.emplace_back(raw);
};

Expand All @@ -523,11 +533,15 @@ class ProofCompressor {
return;
}

BB_ASSERT(x_val < Fq::modulus);
if (x_val >= Fq::modulus) {
throw_or_abort("proof_compression: BN254 x-coordinate out of range");
}
Fq x(x_val);
Fq y_squared = x * x * x + Bn254G1Params::b;
auto [is_square, y] = y_squared.sqrt();
BB_ASSERT(is_square);
if (!is_square) {
throw_or_abort("proof_compression: BN254 point not on curve");
}

if (y_is_negative(y) != sign) {
y = -y;
Expand Down Expand Up @@ -555,11 +569,15 @@ class ProofCompressor {
return;
}

BB_ASSERT(x_val < Fr::modulus);
if (x_val >= Fr::modulus) {
throw_or_abort("proof_compression: Grumpkin x-coordinate out of range");
}
Fr x(x_val);
Fr y_squared = x * x * x + grumpkin::G1Params::b;
auto [is_square, y] = y_squared.sqrt();
BB_ASSERT(is_square);
if (!is_square) {
throw_or_abort("proof_compression: Grumpkin point not on curve");
}

if (y_is_negative(y) != sign) {
y = -y;
Expand All @@ -571,15 +589,19 @@ class ProofCompressor {

auto grumpkin_scalar = [&]() {
uint256_t raw = read_u256(compressed, pos);
BB_ASSERT(raw < Fq::modulus);
if (raw >= Fq::modulus) {
throw_or_abort("proof_compression: Grumpkin scalar out of range");
}
Fq fq_val(raw);
auto [lo, hi] = split_fq(fq_val);
flat.emplace_back(lo);
flat.emplace_back(hi);
};

walk_chonk_proof(bn254_scalar, bn254_comm, grumpkin_scalar, grumpkin_comm, mega_num_public_inputs);
BB_ASSERT(pos == compressed.size());
if (pos != compressed.size()) {
throw_or_abort("proof_compression: decompression did not consume all bytes");
}
return ChonkProof::from_field_elements(flat);
}
};
Expand Down
18 changes: 15 additions & 3 deletions barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
scalar_multiplication::pippenger_unsafe<Curve>(data.s_vec, { &srs_elements[0], /*size*/ poly_length });
}
if (G_zero != data.G_zero_from_prover) {
info("IPA verification failed: G_0 mismatch");
return false;
}

Expand Down Expand Up @@ -669,8 +670,14 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
requires(!Curve::is_stdlib_type)
{
const size_t num_claims = opening_claims.size();
BB_ASSERT(num_claims == transcripts.size());
BB_ASSERT(num_claims > 0);
if (num_claims != transcripts.size()) {
info("IPA batch verification failed: claims/transcripts size mismatch");
return false;
}
if (num_claims == 0) {
info("IPA batch verification failed: no claims provided");
return false;
}

// Phase 1: Per-proof transcript processing (sequential, each proof is cheap)
std::vector<GroupElement> C_zeros(num_claims);
Expand Down Expand Up @@ -770,6 +777,12 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
static bool full_verify_recursive(const VK& vk, const OpeningClaim<Curve>& opening_claim, auto& transcript)
requires Curve::is_stdlib_type
{
// Check SRS size up front before any circuit construction
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

if (vk.get_monomial_points().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()) + ")");
}

add_claim_to_hash_buffer(opening_claim, transcript);
VerifierAccumulator verifier_accumulator = reduce_verify_internal_recursive(opening_claim, transcript);
auto round_challenges_inv = verifier_accumulator.u_challenges_inv;
Expand Down Expand Up @@ -806,7 +819,6 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
// Compute G_zero
// In the native verifier, this uses pippenger. Here we use batch_mul.
std::vector<Commitment> srs_elements = vk.get_monomial_points();
BB_ASSERT_GTE(srs_elements.size(), poly_length, "Not enough SRS points for IPA!");
srs_elements.resize(poly_length);
Commitment computed_G_zero = Commitment::batch_mul(srs_elements, s_vec);
// check the computed G_zero and the claimed G_zero are the same.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ template <typename Curve_> class KZG {

// Validate the final MSM size if expected size is provided
if (expected_final_msm_size != 0) {
BB_ASSERT_EQ(batch_opening_claim.commitments.size(), expected_final_msm_size);
if (batch_opening_claim.commitments.size() != expected_final_msm_size) {
throw_or_abort("KZG verification: unexpected final MSM size " +
std::to_string(batch_opening_claim.commitments.size()) + " (expected " +
std::to_string(expected_final_msm_size) + ")");
}
}

// Compute C + [W]₁ ⋅ z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,9 @@ template <typename Curve, bool HasZK = false> class ShpleminiVerifier_ {
// Used in ECCVM and BatchedHonkTranslator. The nu power offset in batch_sumcheck_round_claims
// assumes ZK claims (NUM_SMALL_IPA_EVALUATIONS) precede sumcheck round claims in the batching order.
if (committed_sumcheck) {
BB_ASSERT(HasZK, "committed sumcheck requires ZK for correct nu power indexing");
if constexpr (!HasZK) {
throw_or_abort("Shplemini: committed sumcheck requires ZK for correct nu power indexing");
}
batch_sumcheck_round_claims(commitments,
scalars,
constant_term_accumulator,
Expand Down Expand Up @@ -522,10 +524,11 @@ template <typename Curve, bool HasZK = false> class ShpleminiVerifier_ {
// next duplicate; the original at original_start + i is unaffected since
// we erase higher-index ranges first.
if constexpr (!Curve::is_stdlib_type) {
BB_ASSERT(commitments[duplicate_start] == commitments[original_start + i],
"remove_repeated_commitments: commitment mismatch at duplicate index " +
std::to_string(duplicate_start) + " vs original index " +
std::to_string(original_start + i));
if (commitments[duplicate_start] != commitments[original_start + i]) {
throw_or_abort("remove_repeated_commitments: commitment mismatch at duplicate index " +
std::to_string(duplicate_start) + " vs original index " +
std::to_string(original_start + i));
}
}
scalars.erase(scalars.begin() + static_cast<std::ptrdiff_t>(duplicate_start));
commitments.erase(commitments.begin() + static_cast<std::ptrdiff_t>(duplicate_start));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,9 @@ template <typename Curve> class ShplonkVerifier_ {
, commitments({ quotient })
, scalars{ Fr{ 1 } }
{
BB_ASSERT_GT(num_claims, 1U, "Using Shplonk with just one claim. Should use batch reduction.");
if (num_claims <= 1U) {
throw_or_abort("Using Shplonk with just one claim. Should use batch reduction.");
}
const size_t num_commitments = commitments.size();
commitments.reserve(num_commitments);
scalars.reserve(num_commitments);
Expand Down
7 changes: 6 additions & 1 deletion barretenberg/cpp/src/barretenberg/ecc/fields/field_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,13 @@ template <class Params> void field<Params>::msgpack_unpack(auto o)
data[i] = reversed[i];
}

// Reject non-canonical field encodings (values >= modulus) to ensure strict parsing.
if (uint256_t{ data[0], data[1], data[2], data[3] } >= modulus) {
throw_or_abort("msgpack field deserialization: non-canonical encoding (value >= modulus)");
}

// Finally, the field is converted back to Montgomery form, just like in the old format.
*this = to_montgomery_form_reduced();
*this = to_montgomery_form();
}

} // namespace bb
Expand Down
40 changes: 40 additions & 0 deletions barretenberg/cpp/src/barretenberg/ecc/fields/prime_field.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,46 @@ TYPED_TEST(PrimeFieldTest, Msgpack)
EXPECT_EQ(actual, expected);
}

TYPED_TEST(PrimeFieldTest, MsgpackRejectsNonCanonical)
{
using F = TypeParam;

// Use a small value so that (value + modulus) is guaranteed to fit in 256 bits,
// even for fields with moduli close to 2^256 (e.g., secp256k1, secp256r1).
F a = F(1);
msgpack::sbuffer buffer;
msgpack::pack(buffer, a);

// Find the 32-byte binary payload inside the msgpack buffer.
// pack_bin(32) produces: 0xc4 0x20 <32 bytes>
uint8_t* buf = reinterpret_cast<uint8_t*>(buffer.data());
size_t buf_size = buffer.size();
uint8_t* field_bytes = nullptr;
for (size_t i = 0; i + 33 <= buf_size; i++) {
if (buf[i] == 0xc4 && buf[i + 1] == 0x20) {
field_bytes = &buf[i + 2];
break;
}
}
ASSERT_NE(field_bytes, nullptr) << "Could not find 32-byte bin payload in msgpack buffer";

// Replace the payload with (1 + modulus), a non-canonical encoding of 1.
uint256_t non_canonical = uint256_t(1) + F::modulus;
for (int i = 31; i >= 0; i--) {
field_bytes[i] = static_cast<uint8_t>(non_canonical.data[0] & 0xFF);
non_canonical >>= 8;
}

// Deserializing the mutated buffer must throw
EXPECT_THROW(
{
msgpack::object_handle oh = msgpack::unpack(buffer.data(), buffer.size());
F result;
oh.get().convert(result);
},
std::runtime_error);
}

// ================================
// Montgomery Form Conversion Tests (254-bit fields)
// ================================
Expand Down
Loading
Loading