Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ci): correctly run bb tests with asserts #7607

Merged
merged 10 commits into from
Aug 21, 2024
4 changes: 2 additions & 2 deletions barretenberg/cpp/Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ preset-release:

preset-release-assert:
FROM +source
RUN cmake --preset clang16 -Bbuild && cmake --build build --target bb && rm -rf build/{deps,lib,src}
RUN cmake --preset clang16-assert -Bbuild && cmake --build build --target bb && rm -rf build/{deps,lib,src}
SAVE ARTIFACT build/bin

preset-debug:
Expand Down Expand Up @@ -266,4 +266,4 @@ vm-full-test:
build:
BUILD +preset-wasm
BUILD +preset-wasm-threads
BUILD +preset-release
BUILD +preset-release
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ TEST_F(AztecIVCTests, BadProofFailure)
EXPECT_EQ(ivc.verification_queue.size(), 1);
tamper_with_proof(ivc.verification_queue[0].proof); // tamper with the final fold proof

EXPECT_FALSE(ivc.prove_and_verify());
EXPECT_ANY_THROW(ivc.prove_and_verify());
}

EXPECT_TRUE(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "barretenberg/ecc/fields/field_conversion.hpp"
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
#include <gtest/gtest.h>

namespace bb::field_conversion_tests {
Expand Down Expand Up @@ -46,28 +47,46 @@ TEST_F(FieldConversionTest, FieldConversionGrumpkinFr)
check_conversion(x1);
}

namespace {
bb::fq derive_bn254_y(bb::fq x)
{
auto [found, y] = (x.sqr() * x + Bn254G1Params::b).sqrt();
EXPECT_TRUE(found);
return y;
}
} // namespace

/**
* @brief Field conversion test for curve::BN254::AffineElement
*
*/
TEST_F(FieldConversionTest, FieldConversionBN254AffineElement)
{
curve::BN254::AffineElement x1(1, 2);
curve::BN254::AffineElement x1(1, derive_bn254_y(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

oh wait, it's able to find the point for both x values? would not have expected it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm indeed it should crap out if not

check_conversion(x1);

curve::BN254::AffineElement x2(grumpkin::fr::modulus_minus_two, grumpkin::fr::modulus_minus_two);
curve::BN254::AffineElement x2(grumpkin::fr::modulus_minus_two, derive_bn254_y(grumpkin::fr::modulus_minus_two));
check_conversion(x2);
}

namespace {
bb::grumpkin::fq derive_grumpkin_y(bb::grumpkin::fq x)
{
auto [found, y] = (x.sqr() * x + grumpkin::G1Params::b + x * grumpkin::G1Params::a).sqrt();
EXPECT_TRUE(found);
return y;
}
} // namespace

/**
* @brief Field conversion test for curve::Grumpkin::AffineElement
*/
TEST_F(FieldConversionTest, FieldConversionGrumpkinAffineElement)
{
curve::Grumpkin::AffineElement x1(1, 2);
curve::Grumpkin::AffineElement x1(1, derive_grumpkin_y(1));
check_conversion(x1);

curve::Grumpkin::AffineElement x2(bb::fr::modulus_minus_two, bb::fr::modulus_minus_two);
curve::Grumpkin::AffineElement x2(bb::fr::modulus_minus_two, derive_grumpkin_y(bb::fr::modulus_minus_two));
check_conversion(x2);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ TEST(Polynomial, Shifted)
EXPECT_EQ(poly_shifted.size(), poly.size());

// The shift is indeed the shift
for (size_t i = 0; i < poly_shifted.size(); ++i) {
for (size_t i = 0; i < poly_shifted.size() - 1; ++i) {
EXPECT_EQ(poly_shifted.get(i), poly.get(i + 1));
}

// If I change the original polynomial, the shift is updated accordingly
poly[3] = 25;
for (size_t i = 0; i < poly_shifted.size(); ++i) {
for (size_t i = 0; i < poly_shifted.size() - 1; ++i) {
EXPECT_EQ(poly_shifted.get(i), poly.get(i + 1));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,38 @@ TEST_F(GoblinRecursiveVerifierTests, ECCVMFailure)
TEST_F(GoblinRecursiveVerifierTests, TranslatorFailure)
{
auto [proof, verifier_input] = create_goblin_prover_output();

// Tamper with the Translator proof
for (auto& val : proof.translator_proof) {
if (val > 0) { // tamper by finding the first non-zero value and incrementing it by 1
val += 1;
break;
// Tamper with the Translator proof preamble
{
auto tampered_proof = proof;
for (auto& val : tampered_proof.translator_proof) {
if (val > 0) { // tamper by finding the first non-zero value and incrementing it by 1
val += 1;
break;
}
}
}

Builder builder;
GoblinRecursiveVerifier verifier{ &builder, verifier_input };
verifier.verify(proof);
Builder builder;
GoblinRecursiveVerifier verifier{ &builder, verifier_input };
EXPECT_ANY_THROW(verifier.verify(tampered_proof));
}
// Tamper with the Translator proof non-preamble values
{
auto tampered_proof = proof;
int seek = 10;
for (auto& val : tampered_proof.translator_proof) {
if (val > 0) { // tamper by finding the tenth non-zero value and incrementing it by 1
if (--seek == 0) {
val += 1;
break;
}
}
}

EXPECT_FALSE(CircuitChecker::check(builder));
Builder builder;
GoblinRecursiveVerifier verifier{ &builder, verifier_input };
verifier.verify(tampered_proof);
EXPECT_FALSE(CircuitChecker::check(builder));
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1946,7 +1946,6 @@ template <typename Builder, typename T> void bigfield<Builder, T>::assert_equal(
if (remainder_512 != 0) {
std::cerr << "bigfield: remainder not zero!" << std::endl;
}
ASSERT(remainder_512 == 0);
bigfield quotient;

const size_t num_quotient_bits = get_quotient_max_bits({ 0 });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "./translator_recursive_verifier.hpp"
#include "barretenberg/commitment_schemes/zeromorph/zeromorph.hpp"
#include "barretenberg/common/throw_or_abort.hpp"
#include "barretenberg/relations/translator_vm/translator_decomposition_relation_impl.hpp"
#include "barretenberg/relations/translator_vm/translator_delta_range_constraint_relation_impl.hpp"
#include "barretenberg/relations/translator_vm/translator_extra_relations_impl.hpp"
Expand Down Expand Up @@ -74,7 +75,10 @@ std::array<typename Flavor::GroupElement, 2> TranslatorRecursiveVerifier_<Flavor
CommitmentLabels commitment_labels;

const FF circuit_size = transcript->template receive_from_prover<FF>("circuit_size");
ASSERT(static_cast<uint32_t>(circuit_size.get_value()) == key->circuit_size);
if (static_cast<uint32_t>(circuit_size.get_value()) != key->circuit_size) {
throw_or_abort(
"TranslatorRecursiveVerifier::verify_proof: proof circuit size does not match verification key!");
}
evaluation_input_x = transcript->template receive_from_prover<BF>("evaluation_input_x");

const BF accumulated_result = transcript->template receive_from_prover<BF>("accumulated_result");
Expand Down
13 changes: 10 additions & 3 deletions barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "barretenberg/ultra_honk/oink_verifier.hpp"
#include "barretenberg/common/throw_or_abort.hpp"

namespace bb {

Expand Down Expand Up @@ -37,9 +38,15 @@ template <IsUltraFlavor Flavor> void OinkVerifier<Flavor>::execute_preamble_roun
const auto pub_inputs_offset =
transcript->template receive_from_prover<uint32_t>(domain_separator + "pub_inputs_offset");

ASSERT(circuit_size == key->circuit_size);
ASSERT(public_input_size == key->num_public_inputs);
ASSERT(pub_inputs_offset == key->pub_inputs_offset);
if (circuit_size != key->circuit_size) {
throw_or_abort("OinkVerifier::execute_preamble_round: proof circuit size does not match verification key!");
}
if (public_input_size != key->num_public_inputs) {
throw_or_abort("OinkVerifier::execute_preamble_round: public inputs size does not match verification key!");
}
if (pub_inputs_offset != key->pub_inputs_offset) {
throw_or_abort("OinkVerifier::execute_preamble_round: public inputs offset does not match verification key!");
}

for (size_t i = 0; i < public_input_size; ++i) {
auto public_input_i =
Expand Down
Loading