Skip to content

Commit 47fc343

Browse files
authored
fix: Add free witness tag to field constructor (#16827)
Missed setting free witness in one of constructors before
1 parent 4d85947 commit 47fc343

File tree

8 files changed

+45
-7
lines changed

8 files changed

+45
-7
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ template <typename Builder> class stdlib_bigfield : public testing::Test {
100100
fr elt_native_lo = fr(uint256_t(elt_native).slice(0, fq_ct::NUM_LIMB_BITS * 2));
101101
fr elt_native_hi = fr(uint256_t(elt_native).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * 4));
102102
fq_ct elt_ct(witness_ct(builder, elt_native_lo), witness_ct(builder, elt_native_hi));
103+
// UNset free witness tag so we don't have to unset it in every test
104+
elt_ct.unset_free_witness_tag();
103105
return std::make_pair(elt_native, elt_ct);
104106
}
105107

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ template <typename Builder, typename T> bigfield<Builder, T>::bigfield(const byt
223223
const uint256_t hi_nibble_shift = uint256_t(1) << 4;
224224
const field_t<Builder> sum = lo_nibble + (hi_nibble * hi_nibble_shift);
225225
sum.assert_equal(split_byte);
226+
lo_nibble.set_origin_tag(split_byte.tag);
227+
hi_nibble.set_origin_tag(split_byte.tag);
226228
return std::make_pair(lo_nibble, hi_nibble);
227229
};
228230

@@ -1892,7 +1894,6 @@ template <typename Builder, typename T> void bigfield<Builder, T>::assert_less_t
18921894
template <typename Builder, typename T> void bigfield<Builder, T>::assert_equal(const bigfield& other) const
18931895
{
18941896
Builder* ctx = this->context ? this->context : other.context;
1895-
(void)OriginTag(get_origin_tag(), other.get_origin_tag());
18961897
if (is_constant() && other.is_constant()) {
18971898
std::cerr << "bigfield: calling assert equal on 2 CONSTANT bigfield elements...is this intended?" << std::endl;
18981899
BB_ASSERT_EQ(get_value(), other.get_value(), "We expect constants to be less than the target modulus");
@@ -1922,6 +1923,12 @@ template <typename Builder, typename T> void bigfield<Builder, T>::assert_equal(
19221923
return;
19231924
} else {
19241925

1926+
// Remove tags, we don't want to cause violations on assert_equal
1927+
const auto original_tag = get_origin_tag();
1928+
const auto other_original_tag = other.get_origin_tag();
1929+
set_origin_tag(OriginTag());
1930+
other.set_origin_tag(OriginTag());
1931+
19251932
bigfield diff = *this - other;
19261933
const uint512_t diff_val = diff.get_value();
19271934
const uint512_t modulus(target_basis.modulus);
@@ -1938,6 +1945,10 @@ template <typename Builder, typename T> void bigfield<Builder, T>::assert_equal(
19381945
false,
19391946
num_quotient_bits);
19401947
unsafe_evaluate_multiply_add(diff, { one() }, {}, quotient, { zero() });
1948+
1949+
// Restore tags
1950+
set_origin_tag(original_tag);
1951+
other.set_origin_tag(other_original_tag);
19411952
}
19421953
}
19431954

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ field_t<Builder>::field_t(const witness_t<Builder>& value)
2929
, additive_constant(bb::fr::zero())
3030
, multiplicative_constant(bb::fr::one())
3131
, witness_index(value.witness_index)
32-
{}
32+
{
33+
set_free_witness_tag();
34+
}
3335

3436
template <typename Builder>
3537
field_t<Builder>::field_t(Builder* parent_context, const bb::fr& value)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,8 +1326,8 @@ template <typename Builder> class stdlib_field : public testing::Test {
13261326
uint256_t(bb::fr::random_element()) & ((uint256_t(1) << grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) - 1);
13271327
auto a = field_ct(witness_ct(&builder, a_val));
13281328
auto b = field_ct(witness_ct(&builder, bb::fr::random_element()));
1329-
EXPECT_TRUE(a.get_origin_tag().is_empty());
1330-
EXPECT_TRUE(b.get_origin_tag().is_empty());
1329+
EXPECT_TRUE(a.get_origin_tag().is_free_witness());
1330+
EXPECT_TRUE(b.get_origin_tag().is_free_witness());
13311331
const size_t parent_id = 0;
13321332

13331333
const auto submitted_value_origin_tag = OriginTag(parent_id, /*round_id=*/0, /*is_submitted=*/true);

barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ safe_uint_t<Builder> safe_uint_t<Builder>::divide(
127127
safe_uint_t<Builder> remainder(
128128
remainder_field, remainder_bit_size, format("divide method remainder: ", description));
129129

130+
const auto merged_tag = OriginTag(get_origin_tag(), other.get_origin_tag());
131+
quotient.set_origin_tag(merged_tag);
132+
remainder.set_origin_tag(merged_tag);
133+
130134
// This line implicitly checks we are not overflowing
131135
safe_uint_t int_val = quotient * other + remainder;
132136

@@ -138,7 +142,6 @@ safe_uint_t<Builder> safe_uint_t<Builder>::divide(
138142

139143
this->assert_equal(int_val, "divide method quotient and/or remainder incorrect");
140144

141-
quotient.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag()));
142145
return quotient;
143146
}
144147

@@ -161,6 +164,10 @@ template <typename Builder> safe_uint_t<Builder> safe_uint_t<Builder>::operator/
161164
safe_uint_t<Builder> remainder(
162165
remainder_field, (size_t)(other.current_max.get_msb() + 1), format("/ operator remainder"));
163166

167+
const auto merged_tag = OriginTag(get_origin_tag(), other.get_origin_tag());
168+
quotient.set_origin_tag(merged_tag);
169+
remainder.set_origin_tag(merged_tag);
170+
164171
// This line implicitly checks we are not overflowing
165172
safe_uint_t int_val = quotient * other + remainder;
166173

@@ -172,7 +179,6 @@ template <typename Builder> safe_uint_t<Builder> safe_uint_t<Builder>::operator/
172179

173180
this->assert_equal(int_val, "/ operator quotient and/or remainder incorrect");
174181

175-
quotient.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag()));
176182
return quotient;
177183
}
178184

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,8 @@ TYPED_TEST(SafeUintTest, TestDivideMethod)
437437

438438
field_ct a1(witness_ct(&builder, 2));
439439
field_ct b1(witness_ct(&builder, 9));
440+
a1.unset_free_witness_tag();
441+
b1.unset_free_witness_tag();
440442
suint_ct c1(a1, 2);
441443
c1.set_origin_tag(submitted_value_origin_tag);
442444
suint_ct d1(b1, 4);

barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ template <typename Builder> struct StdlibTranscriptParams {
2222
{
2323

2424
ASSERT(!data.empty());
25-
2625
return stdlib::poseidon2<Builder>::hash(data);
2726
}
2827
/**

barretenberg/cpp/src/barretenberg/transcript/transcript.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,15 @@ template <typename TranscriptParams> class BaseTranscript {
385385
manifest.add_challenge(round_number, labels...);
386386
}
387387

388+
// In case the transcript is used for recursive verification, we need to sanitize current round data so we don't
389+
// get an origin tag violation inside the hasher. We are doing this to ensure that the free witness tagged
390+
// elements that are sent to the transcript and are assigned tags externally, don't trigger the origin tag
391+
// security mechanism while we are hashing them
392+
if constexpr (in_circuit) {
393+
for (auto& element : current_round_data) {
394+
element.unset_free_witness_tag();
395+
}
396+
}
388397
// Compute the new challenge buffer from which we derive the challenges.
389398

390399
// Create challenges from Frs.
@@ -500,6 +509,13 @@ template <typename TranscriptParams> class BaseTranscript {
500509
*/
501510
DataType hash_independent_buffer()
502511
{
512+
// In case the transcript is used for recursive verification, we need to sanitize current round data so we don't
513+
// get an origin tag violation inside the hasher
514+
if constexpr (in_circuit) {
515+
for (auto& element : independent_hash_buffer) {
516+
element.unset_free_witness_tag();
517+
}
518+
}
503519
DataType buffer_hash = TranscriptParams::hash(independent_hash_buffer);
504520
independent_hash_buffer.clear();
505521
return buffer_hash;

0 commit comments

Comments
 (0)