From 9fe1729a3360d433b5a8444d7d7b1cba53672710 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 5 Sep 2025 18:09:16 +0000 Subject: [PATCH] fix: Add free witness tag to field constructor --- .../stdlib/primitives/bigfield/bigfield.test.cpp | 2 ++ .../stdlib/primitives/bigfield/bigfield_impl.hpp | 13 ++++++++++++- .../stdlib/primitives/field/field.cpp | 4 +++- .../stdlib/primitives/field/field.test.cpp | 4 ++-- .../stdlib/primitives/safe_uint/safe_uint.cpp | 10 ++++++++-- .../primitives/safe_uint/safe_uint.test.cpp | 2 ++ .../stdlib/transcript/transcript.hpp | 1 - .../src/barretenberg/transcript/transcript.hpp | 16 ++++++++++++++++ 8 files changed, 45 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp index 4f2080befce9..2ccbe85433a8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp @@ -100,6 +100,8 @@ template class stdlib_bigfield : public testing::Test { fr elt_native_lo = fr(uint256_t(elt_native).slice(0, fq_ct::NUM_LIMB_BITS * 2)); fr elt_native_hi = fr(uint256_t(elt_native).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * 4)); fq_ct elt_ct(witness_ct(builder, elt_native_lo), witness_ct(builder, elt_native_hi)); + // UNset free witness tag so we don't have to unset it in every test + elt_ct.unset_free_witness_tag(); return std::make_pair(elt_native, elt_ct); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index a43f5a966b95..12c546b973aa 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -223,6 +223,8 @@ template bigfield::bigfield(const byt const uint256_t hi_nibble_shift = uint256_t(1) << 4; const field_t sum = lo_nibble + (hi_nibble * hi_nibble_shift); sum.assert_equal(split_byte); + lo_nibble.set_origin_tag(split_byte.tag); + hi_nibble.set_origin_tag(split_byte.tag); return std::make_pair(lo_nibble, hi_nibble); }; @@ -1892,7 +1894,6 @@ template void bigfield::assert_less_t template void bigfield::assert_equal(const bigfield& other) const { Builder* ctx = this->context ? this->context : other.context; - (void)OriginTag(get_origin_tag(), other.get_origin_tag()); if (is_constant() && other.is_constant()) { std::cerr << "bigfield: calling assert equal on 2 CONSTANT bigfield elements...is this intended?" << std::endl; BB_ASSERT_EQ(get_value(), other.get_value(), "We expect constants to be less than the target modulus"); @@ -1922,6 +1923,12 @@ template void bigfield::assert_equal( return; } else { + // Remove tags, we don't want to cause violations on assert_equal + const auto original_tag = get_origin_tag(); + const auto other_original_tag = other.get_origin_tag(); + set_origin_tag(OriginTag()); + other.set_origin_tag(OriginTag()); + bigfield diff = *this - other; const uint512_t diff_val = diff.get_value(); const uint512_t modulus(target_basis.modulus); @@ -1938,6 +1945,10 @@ template void bigfield::assert_equal( false, num_quotient_bits); unsafe_evaluate_multiply_add(diff, { one() }, {}, quotient, { zero() }); + + // Restore tags + set_origin_tag(original_tag); + other.set_origin_tag(other_original_tag); } } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 1406280082a0..4e731bd45205 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -29,7 +29,9 @@ field_t::field_t(const witness_t& value) , additive_constant(bb::fr::zero()) , multiplicative_constant(bb::fr::one()) , witness_index(value.witness_index) -{} +{ + set_free_witness_tag(); +} template field_t::field_t(Builder* parent_context, const bb::fr& value) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp index e3431b164bb4..e551b5d7f657 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp @@ -1326,8 +1326,8 @@ template class stdlib_field : public testing::Test { uint256_t(bb::fr::random_element()) & ((uint256_t(1) << grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) - 1); auto a = field_ct(witness_ct(&builder, a_val)); auto b = field_ct(witness_ct(&builder, bb::fr::random_element())); - EXPECT_TRUE(a.get_origin_tag().is_empty()); - EXPECT_TRUE(b.get_origin_tag().is_empty()); + EXPECT_TRUE(a.get_origin_tag().is_free_witness()); + EXPECT_TRUE(b.get_origin_tag().is_free_witness()); const size_t parent_id = 0; const auto submitted_value_origin_tag = OriginTag(parent_id, /*round_id=*/0, /*is_submitted=*/true); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp index f0bcd7968164..91577fb0b04a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp @@ -127,6 +127,10 @@ safe_uint_t safe_uint_t::divide( safe_uint_t remainder( remainder_field, remainder_bit_size, format("divide method remainder: ", description)); + const auto merged_tag = OriginTag(get_origin_tag(), other.get_origin_tag()); + quotient.set_origin_tag(merged_tag); + remainder.set_origin_tag(merged_tag); + // This line implicitly checks we are not overflowing safe_uint_t int_val = quotient * other + remainder; @@ -138,7 +142,6 @@ safe_uint_t safe_uint_t::divide( this->assert_equal(int_val, "divide method quotient and/or remainder incorrect"); - quotient.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag())); return quotient; } @@ -161,6 +164,10 @@ template safe_uint_t safe_uint_t::operator/ safe_uint_t remainder( remainder_field, (size_t)(other.current_max.get_msb() + 1), format("/ operator remainder")); + const auto merged_tag = OriginTag(get_origin_tag(), other.get_origin_tag()); + quotient.set_origin_tag(merged_tag); + remainder.set_origin_tag(merged_tag); + // This line implicitly checks we are not overflowing safe_uint_t int_val = quotient * other + remainder; @@ -172,7 +179,6 @@ template safe_uint_t safe_uint_t::operator/ this->assert_equal(int_val, "/ operator quotient and/or remainder incorrect"); - quotient.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag())); return quotient; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp index 195a1365913e..a6d3839fc37d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp @@ -437,6 +437,8 @@ TYPED_TEST(SafeUintTest, TestDivideMethod) field_ct a1(witness_ct(&builder, 2)); field_ct b1(witness_ct(&builder, 9)); + a1.unset_free_witness_tag(); + b1.unset_free_witness_tag(); suint_ct c1(a1, 2); c1.set_origin_tag(submitted_value_origin_tag); suint_ct d1(b1, 4); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index 2326ceab03e4..01e837830bcc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -22,7 +22,6 @@ template struct StdlibTranscriptParams { { ASSERT(!data.empty()); - return stdlib::poseidon2::hash(data); } /** diff --git a/barretenberg/cpp/src/barretenberg/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/transcript/transcript.hpp index e64c3c4b4867..368c9df2643f 100644 --- a/barretenberg/cpp/src/barretenberg/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/transcript/transcript.hpp @@ -385,6 +385,15 @@ template class BaseTranscript { manifest.add_challenge(round_number, labels...); } + // In case the transcript is used for recursive verification, we need to sanitize current round data so we don't + // get an origin tag violation inside the hasher. We are doing this to ensure that the free witness tagged + // elements that are sent to the transcript and are assigned tags externally, don't trigger the origin tag + // security mechanism while we are hashing them + if constexpr (in_circuit) { + for (auto& element : current_round_data) { + element.unset_free_witness_tag(); + } + } // Compute the new challenge buffer from which we derive the challenges. // Create challenges from Frs. @@ -500,6 +509,13 @@ template class BaseTranscript { */ DataType hash_independent_buffer() { + // In case the transcript is used for recursive verification, we need to sanitize current round data so we don't + // get an origin tag violation inside the hasher + if constexpr (in_circuit) { + for (auto& element : independent_hash_buffer) { + element.unset_free_witness_tag(); + } + } DataType buffer_hash = TranscriptParams::hash(independent_hash_buffer); independent_hash_buffer.clear(); return buffer_hash;