Skip to content

fix: default pp handling #20516

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.

Sign up for GitHub

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

Merged
notnotraju merged 1 commit into from
Feb 14, 2026
Merged

fix: default pp handling #20516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ template <typename TestType> class stdlib_biggroup : public testing::Test {
{
Builder builder;
STANDARD_TESTING_TAGS;
// Suppress unused warnings for tags not used in this test
Copy link
Collaborator

@ludamad ludamad Feb 14, 2026

Choose a reason for hiding this comment

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

Prefer BB_UNUSED on declaration or [[maybe_unused]]

(void)first_and_third_merged_tag;
(void)first_second_third_merged_tag;
(void)first_to_fourth_merged_tag;
(void)instant_death_tag;
(void)next_submitted_value_origin_tag;
(void)next_challenge_tag;

// Setup: two points with different tags
auto [input_a, a] = get_random_point(&builder, InputType::WITNESS);
Expand Down
25 changes: 19 additions & 6 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/pairing_points.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ template <typename Curve> struct PairingPoints {
const Group& P1() const { return _points[1]; }

bool is_populated() const { return has_data_; }
bool is_default() const { return is_default_; }

PairingPoints() = default;

Expand Down Expand Up @@ -187,6 +188,7 @@ template <typename Curve> struct PairingPoints {
transcript.add_to_hash_buffer("Aggregated_P1", other.P1());
auto recursion_separator =
transcript.template get_challenge<typename Curve::ScalarField>("recursion_separator");
is_default_ = false; // After aggregation, points are no longer default
// If Mega Builder is in use, the EC operations are deferred via Goblin.
// batch_mul with constant scalar 1 is optimal here (Goblin uses add instead of mul).
if constexpr (std::is_same_v<Builder, MegaCircuitBuilder>) {
Expand Down Expand Up @@ -215,11 +217,21 @@ template <typename Curve> struct PairingPoints {

/**
* @brief Set the witness indices for the pairing points to public
* @details For default (infinity) pairing points, uses set_default_to_public which directly adds zero limbs
* as public inputs, bypassing bigfield::set_public() which cannot handle constant-coordinate infinity points.
* @param ctx Optional builder context; required for default pairing points which have no circuit context.
* @return uint32_t The index into the public inputs array at which the representation is stored
*/
uint32_t set_public()
uint32_t set_public(Builder* ctx = nullptr)
{
BB_ASSERT(this->has_data_, "Calling set_public on empty pairing points.");
if (is_default_) {
Builder* builder = validate_context<Builder>(ctx, P0().get_context(), P1().get_context());
BB_ASSERT(builder != nullptr, "set_public on default pairing points requires a builder context.");
return set_default_to_public(builder);
}
Builder* builder = validate_context<Builder>(ctx, P0().get_context(), P1().get_context());
builder->pairing_points_tagging.set_public_pairing_points();
Copy link
Contributor

@notnotraju notnotraju Feb 14, 2026

Choose a reason for hiding this comment

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

much better here!

uint32_t start_idx = P0().set_public();
P1().set_public();
return start_idx;
Expand Down Expand Up @@ -255,15 +267,13 @@ template <typename Curve> struct PairingPoints {
*/
static uint32_t set_default_to_public(Builder* builder)
{
builder->pairing_points_tagging.set_public_pairing_points();
// Infinity is represented as (0,0) in biggroup. Directly add zero limbs as public inputs, bypassing bigfield's
// self_reduce.
uint32_t start_idx = 0;
uint32_t start_idx = static_cast<uint32_t>(builder->num_public_inputs());
for (size_t i = 0; i < PUBLIC_INPUTS_SIZE; i++) {
uint32_t idx = builder->add_public_variable(bb::fr(0));
builder->fix_witness(idx, bb::fr(0));
if (i == 0) {
start_idx = idx;
}
}
return start_idx;
}
Expand All @@ -276,12 +286,15 @@ template <typename Curve> struct PairingPoints {
{
Group P0(Fq(0), Fq(0), /*assert_on_curve=*/false);
Group P1(Fq(0), Fq(0), /*assert_on_curve=*/false);
return PairingPoints(P0, P1);
PairingPoints pp(P0, P1);
pp.is_default_ = true;
return pp;
}

private:
std::array<Group, 2> _points;
bool has_data_ = false;
bool is_default_ = false; // True for default (infinity) pairing points from construct_default()
};

template <typename NCT> std::ostream& operator<<(std::ostream& os, PairingPoints<NCT> const& as)
Expand Down
21 changes: 12 additions & 9 deletions barretenberg/cpp/src/barretenberg/stdlib/primitives/pairing_points.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@ TYPED_TEST(PairingPointsTests, DefaultPointsAreValid)
Builder builder;

auto pp = PP::construct_default();
pp.P0().convert_constant_to_fixed_witness(&builder);
pp.P1().convert_constant_to_fixed_witness(&builder);
pp.set_public();
EXPECT_TRUE(pp.is_default());
pp.set_public(&builder);
EXPECT_TRUE(CircuitChecker::check(builder));

// Validate default PairingPoints via native pairing check
Expand All @@ -100,12 +99,15 @@ TYPED_TEST(PairingPointsTests, Aggregate)

Builder builder;
auto pps = TestFixture::template create_valid_pairing_points<Curve>(&builder, 2);
EXPECT_FALSE(pps[0].is_default());
EXPECT_FALSE(pps[1].is_default());

// Save the original values before aggregation mutates pps[0]
auto orig_P0 = pps[0].P0();
auto orig_P1 = pps[0].P1();

pps[0].aggregate(pps[1]);
EXPECT_FALSE(pps[0].is_default());

// Replicate the transcript to derive the same separator
bb::StdlibTranscript<Builder> transcript{};
Expand Down Expand Up @@ -138,10 +140,12 @@ TYPED_TEST(PairingPointsTests, AggregateIntoEmpty)
// Default-constructed PairingPoints has no data
PP empty;
EXPECT_FALSE(empty.is_populated());
EXPECT_FALSE(empty.is_default());

// Aggregating into empty should just copy the other
// Aggregating into empty should just copy the other (including is_default flag)
empty.aggregate(pps[0]);
EXPECT_TRUE(empty.is_populated());
EXPECT_FALSE(empty.is_default());
EXPECT_EQ(empty.P0().get_value(), pps[0].P0().get_value());
EXPECT_EQ(empty.P1().get_value(), pps[0].P1().get_value());
}
Expand Down Expand Up @@ -174,6 +178,7 @@ TYPED_TEST(PairingPointsTests, AggregateMultiple)
std::vector<PP> pp_vec = { pps[0], pps[1], pps[2] };
PP aggregated = PP::aggregate_multiple(pp_vec);

EXPECT_FALSE(aggregated.is_default());
EXPECT_EQ(aggregated.P0().get_value(), expected_P0.get_value());
EXPECT_EQ(aggregated.P1().get_value(), expected_P1.get_value());
EXPECT_TRUE(CircuitChecker::check(builder));
Expand All @@ -190,14 +195,12 @@ TYPED_TEST(PairingPointsTests, PublicInputSerdeRoundTrip)

Builder builder;

// Create witness pairing points from known defaults
// Create default (infinity) pairing points
PP original = PP::construct_default();
original.P0().convert_constant_to_fixed_witness(&builder);
original.P1().convert_constant_to_fixed_witness(&builder);
EXPECT_TRUE(original.is_populated());

// Serialize to public inputs
uint32_t start_idx = original.set_public();
// Serialize to public inputs (set_public detects default and uses set_default_to_public)
uint32_t start_idx = original.set_public(&builder);

// Extract the public inputs as field_t elements
std::vector<Fr> public_inputs;
Expand Down
32 changes: 3 additions & 29 deletions barretenberg/cpp/src/barretenberg/stdlib/special_public_inputs/special_public_inputs.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,22 +113,14 @@ class KernelIO {
{
Builder* builder = output_hn_accum_hash.get_context();

Builder* pairing_ctx = validate_context<Builder>(pairing_inputs);
if (pairing_ctx == nullptr) {
// Both points are constant - add the default pairing points to public inputs
PairingInputs::set_default_to_public(builder);
} else {
pairing_inputs.set_public();
}
pairing_inputs.set_public(builder);
kernel_return_data.set_public();
app_return_data.set_public();
for (auto& table_commitment : ecc_op_tables) {
table_commitment.set_public();
}
output_hn_accum_hash.set_public();

// Record that pairing points have been set to public
builder->pairing_points_tagging.set_public_pairing_points();
// Finalize the public inputs to ensure no more public inputs can be added hereafter.
builder->finalize_public_inputs();
}
Expand Down Expand Up @@ -197,8 +189,6 @@ template <typename Builder_> class DefaultIO {

pairing_inputs.set_public();

// Record that pairing points have been set to public
builder->pairing_points_tagging.set_public_pairing_points();
// Finalize the public inputs to ensure no more public inputs can be added hereafter.
builder->finalize_public_inputs();
}
Expand Down Expand Up @@ -264,8 +254,6 @@ template <typename Builder_> class GoblinAvmIO {
transcript_hash.set_public();
pairing_inputs.set_public();

// Record that pairing points have been set to public
builder->pairing_points_tagging.set_public_pairing_points();
// Finalize the public inputs to ensure no more public inputs can be added hereafter.
builder->finalize_public_inputs();
}
Expand Down Expand Up @@ -321,19 +309,12 @@ template <class Builder_> class HidingKernelIO {
{
Builder* builder = ecc_op_tables[0].get_context();

if (validate_context<Builder>(pairing_inputs) == nullptr) {
// Both points are constant - add the default pairing points to public inputs
PairingInputs::set_default_to_public(builder);
} else {
pairing_inputs.set_public();
}
pairing_inputs.set_public(builder);
kernel_return_data.set_public();
for (auto& commitment : ecc_op_tables) {
commitment.set_public();
}

// Record that pairing points have been set to public
builder->pairing_points_tagging.set_public_pairing_points();
// Finalize the public inputs to ensure no more public inputs can be added hereafter.
builder->finalize_public_inputs();
}
Expand Down Expand Up @@ -402,16 +383,9 @@ class RollupIO {
{
Builder* builder = ipa_claim.commitment.get_context();

if (validate_context<Builder>(pairing_inputs) == nullptr) {
// Both points are constant - add the default pairing points to public inputs
PairingInputs::set_default_to_public(builder);
} else {
pairing_inputs.set_public();
}
pairing_inputs.set_public(builder);
ipa_claim.set_public();

// Record that pairing points have been set to public
builder->pairing_points_tagging.set_public_pairing_points();
// Finalize the public inputs to ensure no more public inputs can be added hereafter.
builder->finalize_public_inputs();
}
Expand Down
Loading