Skip to content

Commit f21d35e

Browse files
authored
chore: expanded bigfield tests (veridise report) (#16802)
As per the suggestion of adding more tests and including several edge cases, we have added multiple more test cases with critical edge cases and multiple witness-constant configurations.
1 parent 86e7ad0 commit f21d35e

File tree

9 files changed

+1794
-361
lines changed

9 files changed

+1794
-361
lines changed

barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,9 @@ template <typename Builder, typename T> class bigfield {
939939

940940
static_assert(PROHIBITED_LIMB_BITS < MAXIMUM_LIMB_SIZE_THAT_WOULDNT_OVERFLOW);
941941

942+
// For testing purposes only
943+
friend class bigfield_test_access;
944+
942945
private:
943946
/**
944947
* @brief Get the witness indices of the (normalized) binary basis limbs
@@ -1123,6 +1126,31 @@ template <typename Builder, typename T> class bigfield {
11231126

11241127
}; // namespace stdlib
11251128

1129+
// NOTE: For testing private functions in bigfield
1130+
class bigfield_test_access {
1131+
public:
1132+
template <typename bigfield>
1133+
static void unsafe_evaluate_multiply_add(const bigfield& input_left,
1134+
const bigfield& input_to_mul,
1135+
const std::vector<bigfield>& to_add,
1136+
const bigfield& input_quotient,
1137+
const std::vector<bigfield>& input_remainders)
1138+
{
1139+
bigfield::unsafe_evaluate_multiply_add(input_left, input_to_mul, to_add, input_quotient, input_remainders);
1140+
}
1141+
1142+
template <typename bigfield>
1143+
static void unsafe_evaluate_multiple_multiply_add(const std::vector<bigfield>& input_left,
1144+
const std::vector<bigfield>& input_right,
1145+
const std::vector<bigfield>& to_add,
1146+
const bigfield& input_quotient,
1147+
const std::vector<bigfield>& input_remainders)
1148+
{
1149+
bigfield::unsafe_evaluate_multiple_multiply_add(
1150+
input_left, input_right, to_add, input_quotient, input_remainders);
1151+
}
1152+
};
1153+
11261154
template <typename C, typename T> inline std::ostream& operator<<(std::ostream& os, bigfield<T, C> const& v)
11271155
{
11281156
return os << v.get_value();

barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp

Lines changed: 1169 additions & 339 deletions
Large diffs are not rendered by default.

barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_edge_cases.test.cpp

Lines changed: 514 additions & 0 deletions
Large diffs are not rendered by default.

barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ bigfield<Builder, T>::bigfield(const field_t<Builder>& low_bits_in,
8181
// if maximum_bitlength is set, this supercedes can_overflow
8282
if (maximum_bitlength > 0) {
8383
BB_ASSERT_GT(maximum_bitlength, 3 * NUM_LIMB_BITS);
84+
BB_ASSERT_LTE(maximum_bitlength, 4 * NUM_LIMB_BITS);
8485
num_last_limb_bits = maximum_bitlength - (3 * NUM_LIMB_BITS);
8586
}
8687
// We create the high limb values similar to the low limb ones above
@@ -519,7 +520,9 @@ bigfield<Builder, T> bigfield<Builder, T>::operator-(const bigfield& other) cons
519520
if (other.is_constant()) {
520521
uint512_t right = other.get_value() % modulus_u512;
521522
uint512_t neg_right = (modulus_u512 - right) % modulus_u512;
522-
return operator+(bigfield(ctx, uint256_t(neg_right.lo)));
523+
bigfield summand = bigfield(ctx, uint256_t(neg_right.lo));
524+
summand.set_origin_tag(OriginTag(other.get_origin_tag()));
525+
return operator+(summand);
523526
}
524527

525528
/**
@@ -1506,7 +1509,7 @@ bigfield<Builder, T> bigfield<Builder, T>::msub_div(const std::vector<bigfield>&
15061509
for (auto& element : to_sub) {
15071510
new_tag = OriginTag(new_tag, element.get_origin_tag());
15081511
}
1509-
// Gett he context
1512+
// Get the context
15101513
Builder* ctx = divisor.context;
15111514
if (ctx == NULL) {
15121515
for (auto& el : mul_left) {
@@ -1628,10 +1631,9 @@ bigfield<Builder, T> bigfield<Builder, T>::conditional_select(const bigfield& ot
16281631
{
16291632
// If the predicate is constant, the conditional selection can be done out of circuit
16301633
if (predicate.is_constant()) {
1631-
if (predicate.get_value()) {
1632-
return other;
1633-
}
1634-
return *this;
1634+
bigfield result = predicate.get_value() ? other : *this;
1635+
result.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag(), predicate.get_origin_tag()));
1636+
return result;
16351637
}
16361638

16371639
// If both elements are the same, we can just return one of them
@@ -2237,8 +2239,11 @@ void bigfield<Builder, T>::unsafe_evaluate_multiply_add(const bigfield& input_le
22372239
const auto [lo_idx, hi_idx] = ctx->evaluate_non_native_field_multiplication(witnesses);
22382240

22392241
bb::fr neg_prime = -bb::fr(uint256_t(target_basis.modulus));
2240-
field_t<Builder>::evaluate_polynomial_identity(
2241-
left.prime_basis_limb, to_mul.prime_basis_limb, quotient.prime_basis_limb * neg_prime, -remainder_prime_limb);
2242+
field_t<Builder>::evaluate_polynomial_identity(left.prime_basis_limb,
2243+
to_mul.prime_basis_limb,
2244+
quotient.prime_basis_limb * neg_prime,
2245+
-remainder_prime_limb,
2246+
"bigfield: prime limb identity failed");
22422247

22432248
field_t lo = field_t<Builder>::from_witness_index(ctx, lo_idx) + borrow_lo;
22442249
field_t hi = field_t<Builder>::from_witness_index(ctx, hi_idx);
@@ -2251,8 +2256,14 @@ void bigfield<Builder, T>::unsafe_evaluate_multiply_add(const bigfield& input_le
22512256
static_cast<size_t>(carry_hi_msb),
22522257
static_cast<size_t>(carry_lo_msb));
22532258
} else {
2254-
ctx->decompose_into_default_range(hi.get_normalized_witness_index(), carry_hi_msb);
2255-
ctx->decompose_into_default_range(lo.get_normalized_witness_index(), carry_lo_msb);
2259+
ctx->decompose_into_default_range(hi.get_normalized_witness_index(),
2260+
carry_hi_msb,
2261+
Builder::DEFAULT_PLOOKUP_RANGE_BITNUM,
2262+
"bigfield: carry_hi too large");
2263+
ctx->decompose_into_default_range(lo.get_normalized_witness_index(),
2264+
carry_lo_msb,
2265+
Builder::DEFAULT_PLOOKUP_RANGE_BITNUM,
2266+
"bigfield: carry_lo too large");
22562267
}
22572268
}
22582269

@@ -2527,7 +2538,8 @@ void bigfield<Builder, T>::unsafe_evaluate_multiple_multiply_add(const std::vect
25272538
field_t<Builder>::evaluate_polynomial_identity(left[0].prime_basis_limb,
25282539
right[0].prime_basis_limb,
25292540
quotient.prime_basis_limb * neg_prime,
2530-
-remainder_prime_limb);
2541+
-remainder_prime_limb,
2542+
"bigfield: prime limb identity failed");
25312543

25322544
field_t lo = field_t<Builder>::from_witness_index(ctx, lo_1_idx) + borrow_lo;
25332545
field_t hi = field_t<Builder>::from_witness_index(ctx, hi_1_idx);
@@ -2550,8 +2562,14 @@ void bigfield<Builder, T>::unsafe_evaluate_multiple_multiply_add(const std::vect
25502562
static_cast<size_t>(carry_hi_msb),
25512563
static_cast<size_t>(carry_lo_msb));
25522564
} else {
2553-
ctx->decompose_into_default_range(hi.get_normalized_witness_index(), carry_hi_msb);
2554-
ctx->decompose_into_default_range(lo.get_normalized_witness_index(), carry_lo_msb);
2565+
ctx->decompose_into_default_range(hi.get_normalized_witness_index(),
2566+
carry_hi_msb,
2567+
Builder::DEFAULT_PLOOKUP_RANGE_BITNUM,
2568+
"bigfield: carry_hi too large");
2569+
ctx->decompose_into_default_range(lo.get_normalized_witness_index(),
2570+
carry_lo_msb,
2571+
Builder::DEFAULT_PLOOKUP_RANGE_BITNUM,
2572+
"bigfield: carry_hi too large");
25552573
}
25562574
}
25572575

barretenberg/cpp/src/barretenberg/stdlib/primitives/curves/secp256k1.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ template <typename CircuitType> struct secp256k1 {
2121
using fr = ::bb::secp256k1::fr;
2222
using g1 = ::bb::secp256k1::g1;
2323

24+
// Native types
25+
using ScalarFieldNative = ::bb::secp256k1::fr;
26+
using BaseFieldNative = ::bb::secp256k1::fq;
27+
using GroupNative = ::bb::secp256k1::g1;
28+
using ElementNative = GroupNative::element;
29+
using AffineElementNative = GroupNative::affine_element;
30+
31+
// Stdlib types
32+
using ScalarField = bigfield<CircuitType, typename ::bb::secp256k1::FrParams>;
33+
using BaseField = bigfield<CircuitType, typename ::bb::secp256k1::FqParams>;
34+
using Group = element<CircuitType, BaseField, ScalarField, GroupNative>;
35+
using Element = Group;
36+
using AffineElement = Group;
37+
2438
using Builder = CircuitType;
2539
using witness_ct = witness_t<Builder>;
2640
using public_witness_ct = public_witness_t<Builder>;

barretenberg/cpp/src/barretenberg/stdlib/primitives/curves/secp256r1.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ template <typename CircuitType> struct secp256r1 {
2121
using fr = bb::secp256r1::fr;
2222
using g1 = bb::secp256r1::g1;
2323

24+
// Native types
25+
using ScalarFieldNative = ::bb::secp256r1::fr;
26+
using BaseFieldNative = ::bb::secp256r1::fq;
27+
using GroupNative = ::bb::secp256r1::g1;
28+
using ElementNative = GroupNative::element;
29+
using AffineElementNative = GroupNative::affine_element;
30+
31+
// Stdlib types
32+
using ScalarField = bigfield<CircuitType, typename ::bb::secp256r1::FrParams>;
33+
using BaseField = bigfield<CircuitType, typename ::bb::secp256r1::FqParams>;
34+
using Group = element<CircuitType, BaseField, ScalarField, GroupNative>;
35+
using Element = Group;
36+
using AffineElement = Group;
37+
2438
using Builder = CircuitType;
2539
using witness_ct = witness_t<Builder>;
2640
using public_witness_ct = public_witness_t<Builder>;

barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,8 @@ field_t<Builder> field_t<Builder>::select_from_three_bit_table(const std::array<
10751075
* @brief Constrain a + b + c + d to be equal to 0
10761076
*/
10771077
template <typename Builder>
1078-
void field_t<Builder>::evaluate_linear_identity(const field_t& a, const field_t& b, const field_t& c, const field_t& d)
1078+
void field_t<Builder>::evaluate_linear_identity(
1079+
const field_t& a, const field_t& b, const field_t& c, const field_t& d, const std::string& msg)
10791080
{
10801081
Builder* ctx = validate_context(a.context, b.context, c.context, d.context);
10811082

@@ -1084,6 +1085,10 @@ void field_t<Builder>::evaluate_linear_identity(const field_t& a, const field_t&
10841085
return;
10851086
}
10861087

1088+
if (a.get_value() + b.get_value() + c.get_value() + d.get_value() != bb::fr::zero()) {
1089+
ctx->failure(msg);
1090+
}
1091+
10871092
// validate that a + b + c + d = 0
10881093
bb::fr const_scaling = a.additive_constant + b.additive_constant + c.additive_constant + d.additive_constant;
10891094

@@ -1105,10 +1110,8 @@ void field_t<Builder>::evaluate_linear_identity(const field_t& a, const field_t&
11051110
* by creating a `big_mul_gate`.
11061111
*/
11071112
template <typename Builder>
1108-
void field_t<Builder>::evaluate_polynomial_identity(const field_t& a,
1109-
const field_t& b,
1110-
const field_t& c,
1111-
const field_t& d)
1113+
void field_t<Builder>::evaluate_polynomial_identity(
1114+
const field_t& a, const field_t& b, const field_t& c, const field_t& d, const std::string& msg)
11121115
{
11131116
if (a.is_constant() && b.is_constant() && c.is_constant() && d.is_constant()) {
11141117
ASSERT((a.get_value() * b.get_value() + c.get_value() + d.get_value()).is_zero());
@@ -1117,6 +1120,10 @@ void field_t<Builder>::evaluate_polynomial_identity(const field_t& a,
11171120

11181121
Builder* ctx = validate_context(a.context, b.context, c.context, d.context);
11191122

1123+
if ((a.get_value() * b.get_value() + c.get_value() + d.get_value()) != bb::fr::zero()) {
1124+
ctx->failure(msg);
1125+
}
1126+
11201127
// validate that a * b + c + d = 0
11211128
bb::fr mul_scaling = a.multiplicative_constant * b.multiplicative_constant;
11221129
bb::fr a_scaling = a.multiplicative_constant * b.additive_constant;

barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,16 @@ template <typename Builder_> class field_t {
373373
const bool_t<Builder>& t1,
374374
const bool_t<Builder>& t0);
375375

376-
static void evaluate_linear_identity(const field_t& a, const field_t& b, const field_t& c, const field_t& d);
377-
static void evaluate_polynomial_identity(const field_t& a, const field_t& b, const field_t& c, const field_t& d);
376+
static void evaluate_linear_identity(const field_t& a,
377+
const field_t& b,
378+
const field_t& c,
379+
const field_t& d,
380+
const std::string& msg = "field_t::evaluate_linear_identity");
381+
static void evaluate_polynomial_identity(const field_t& a,
382+
const field_t& b,
383+
const field_t& c,
384+
const field_t& d,
385+
const std::string& msg = "field_t::evaluate_polynomial_identity");
378386

379387
static field_t accumulate(const std::vector<field_t>& input);
380388

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,10 +1682,10 @@ void UltraCircuitBuilder_<ExecutionTrace>::range_constrain_two_limbs(const uint3
16821682

16831683
for (size_t i = 0; i < 5; i++) {
16841684
if (lo_masks[i] != 0) {
1685-
create_new_range_constraint(lo_sublimbs[i], lo_masks[i]);
1685+
create_new_range_constraint(lo_sublimbs[i], lo_masks[i], "ultra_circuit_builder: sublimb of low too large");
16861686
}
16871687
if (hi_masks[i] != 0) {
1688-
create_new_range_constraint(hi_sublimbs[i], hi_masks[i]);
1688+
create_new_range_constraint(hi_sublimbs[i], hi_masks[i], "ultra_circuit_builder: sublimb of hi too large");
16891689
}
16901690
}
16911691
};

0 commit comments

Comments
 (0)