Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,11 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
// Verify output in memory
MemoryValue res_x = mem->get(input.addresses[6]);
MemoryValue res_y = mem->get(input.addresses[6] + 1);
MemoryValue res_inf = mem->get(input.addresses[6] + 2);

EmbeddedCurvePoint result_point = EmbeddedCurvePoint(res_x.as_ff(), res_y.as_ff(), res_inf.as_ff() == FF(1));
EmbeddedCurvePoint result_point = EmbeddedCurvePoint(res_x.as_ff(), res_y.as_ff());

BB_ASSERT(result_point.x() == expected_result.x(), "Result x-coordinate mismatch");
BB_ASSERT(result_point.y() == expected_result.y(), "Result y-coordinate mismatch");
BB_ASSERT(result_point.is_infinity() == expected_result.is_infinity(), "Result infinity flag mismatch");

// Non mem-aware ecmul result:
expected_result = point_p * input.scalar;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,15 @@
namespace bb::avm2 {

/**
* AVM bytecode expects the representation of points to be triplets, the two coordinates and an is_infinity boolean.
* Furthermore, its representation of infinity is inherited from noir's and is expected to be 0,0,true.
* BB, however, uses only the two coordinates to represent points. Infinity in barretenberg is represented as (P+1)/2,0.
* This class is a wrapper of the BB representation, needed to operate with points, that allows us to extract the
* standard representation that AVM bytecode expects.
* The AVM's representation of infinity is inherited from noir's and is expected to be 0,0.
* BB, however, uses only the two coordinates to represent points. Infinity in barretenberg is represented as
* (P+1)/2,0,true. This class is a wrapper of the BB representation, needed to operate with points, that allows us to
* extract the standard representation that AVM bytecode expects.
*
* NOTE: When constructing infinity from BB's two element representation, we keep the original AffinePoint so operations
* can use it in the background, but set extractable coordinates to be our represention of (0,0).
* NOTE: When constructing infinity via BaseFields, input coordinates are overwritten to our representation of (0,0)
* if the input is_infinity is true, so will mismatch the underlying AffinePoint's coordinates.
*
* TODO(#AVM-266): Remove is_infinity flag from point representation.
* NOTE: When constructing infinity via BaseFields (equiv. inputting (0, 0), the underlying AffinePoint is set to BB's
* representation so operations can use it in the background.
*/
template <typename AffinePoint> class StandardAffinePoint {
public:
Expand All @@ -32,12 +29,10 @@ template <typename AffinePoint> class StandardAffinePoint {
, y_coord(val.is_point_at_infinity() ? zero : val.y)
{}

// TODO(MW): Do we want to silently discard input x, y if is_infinity = true?
// Or, discard is_infinity and set point to AffinePoint::infinity() iff x = y = 0?
constexpr StandardAffinePoint(BaseField x, BaseField y, bool is_infinity) noexcept
: point(is_infinity ? AffinePoint::infinity() : AffinePoint(x, y))
, x_coord(is_infinity ? zero : x)
, y_coord(is_infinity ? zero : y)
constexpr StandardAffinePoint(BaseField x, BaseField y) noexcept
: point((x.is_zero() && y.is_zero()) ? AffinePoint::infinity() : AffinePoint(x, y))
, x_coord(x)
, y_coord(y)
{}

constexpr StandardAffinePoint operator+(const StandardAffinePoint& other) const noexcept
Expand Down Expand Up @@ -87,7 +82,7 @@ template <typename AffinePoint> class StandardAffinePoint {

static const StandardAffinePoint& infinity()
{
static auto infinity = StandardAffinePoint(zero, zero, true);
static auto infinity = StandardAffinePoint(zero, zero);
return infinity;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,17 @@ using EmbeddedCurvePoint = StandardAffinePoint<grumpkin::g1::affine_element>;
using Fr = grumpkin::fr;
using Fq = grumpkin::fq;

TEST(StandardAffinePointTest, InfinityDiscardsRawCoordinates)
TEST(StandardAffinePointTest, ConstructingInfinityNormalized)
{
// NOTE: As of #AVM-248, we moved from preserving raw coordinates in
// infinity points to our (0,0) representation when using x() and y().
// The underlying AffinePoint is set to AffinePoint::infinity() for
// bb operations.

// When constructing an infinity point with non-zero coordinates,
// x() and y() should return our standard representation.
Fq raw_x = 1;
Fq raw_y = 2;

// Note that raw x and y are silently discarded.
EmbeddedCurvePoint point(raw_x, raw_y, /*is_infinity=*/true);

EXPECT_TRUE(point.is_infinity());
EXPECT_TRUE(point.x().is_zero());
EXPECT_TRUE(point.y().is_zero());
// Constructing a point with (0,0) coordinates should result in infinity
EmbeddedCurvePoint inf(0, 0);
EXPECT_TRUE(inf.is_infinity());
// Constructing a point with BB's inf should result in infinity with (0,0) coordinates
EmbeddedCurvePoint inf_bb(grumpkin::g1::affine_element::infinity());
EXPECT_TRUE(inf_bb.is_infinity());
EXPECT_TRUE(inf_bb.x().is_zero());
EXPECT_TRUE(inf_bb.y().is_zero());
EXPECT_EQ(inf, inf_bb);
}

TEST(StandardAffinePointTest, NormalPointCoordinates)
Expand Down Expand Up @@ -78,7 +71,7 @@ TEST(StandardAffinePointTest, ScalarMultiplicationResultingInInfinityNormalized)

TEST(StandardAffinePointTest, StaticInfinityHasZeroCoordinates)
{
// The static infinity() method should return (0,0,true)
// The static infinity() method should return (0,0)
auto& inf = EmbeddedCurvePoint::infinity();

EXPECT_TRUE(inf.is_infinity());
Expand All @@ -88,10 +81,8 @@ TEST(StandardAffinePointTest, StaticInfinityHasZeroCoordinates)

TEST(StandardAffinePointTest, NegatingInfinity)
{
// Negating an infinity point should return (0,0,true)
EmbeddedCurvePoint inf(0, 0, /*is_infinity=*/true);

auto neg_inf = -inf;
// Negating an infinity point should return (0,0)
auto neg_inf = -EmbeddedCurvePoint::infinity();

EXPECT_TRUE(neg_inf.is_infinity());
EXPECT_TRUE(neg_inf.x().is_zero());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ using simulation::ToRadixMemoryEvent;
// Known good points for P and Q
FF p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a");
FF p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60");
EmbeddedCurvePoint p(p_x, p_y, false);
EmbeddedCurvePoint p(p_x, p_y);

FF q_x("0x009242167ec31949c00cbe441cd36757607406e87844fa2c8c4364a4403e66d7");
FF q_y("0x0fe3016d64cfa8045609f375284b6b739b5fa282e4cbb75cc7f1687ecc7420e3");
EmbeddedCurvePoint q(q_x, q_y, false);
EmbeddedCurvePoint q(q_x, q_y);

TEST(EccAddConstrainingTest, EccEmptyRow)
{
Expand All @@ -80,7 +80,7 @@ TEST(EccAddConstrainingTest, EccAdd)
// R = P + Q;
FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6");
FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09");
EmbeddedCurvePoint r(r_x, r_y, false);
EmbeddedCurvePoint r(r_x, r_y);

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 1 },
Expand Down Expand Up @@ -124,7 +124,7 @@ TEST(EccAddConstrainingTest, EccDouble)
// R = P + P;
FF r_x("0x088b996194bb5e6e8e5e49733bb671c3e660cf77254f743f366cc8e33534ee3b");
FF r_y("0x2807ffa01c0f522d0be1e1acfb6914ac8eabf1acf420c0629d37beee992e9a0e");
EmbeddedCurvePoint r(r_x, r_y, false);
EmbeddedCurvePoint r(r_x, r_y);

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 0 },
Expand Down Expand Up @@ -174,13 +174,13 @@ TEST(EccAddConstrainingTest, EccAddSameYDifferentX)
// Point P - known valid point on Grumpkin
FF local_p_x("0x04c95d1b26d63d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a");
FF local_p_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60");
EmbeddedCurvePoint local_p(local_p_x, local_p_y, false);
EmbeddedCurvePoint local_p(local_p_x, local_p_y);

// Point Q - p_x * omega (cube root of unity), same y-coordinate!
// omega = 0x0000000000000000b3c4d79d41a917585bfc41088d8daaa78b17ea66b99c90dd
FF local_q_x("0x14dd39aa19e1c8b29e0c530a28106a7d64d2213486baba3c86dce51bdddf75bb");
FF local_q_y("0x035b6dd9e63c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60");
EmbeddedCurvePoint local_q(local_q_x, local_q_y, false);
EmbeddedCurvePoint local_q(local_q_x, local_q_y);

// Verify preconditions: same y, different x
ASSERT_NE(local_p.x(), local_q.x());
Expand All @@ -189,7 +189,7 @@ TEST(EccAddConstrainingTest, EccAddSameYDifferentX)
// Expected result R = P + Q (lambda = 0 since y's are equal)
FF local_r_x("0x16bdb7ada0799a3088b9dd3faade12c3f79dbfe9cb1234783a1a7add546398dc");
FF local_r_y("0x2d08e098faf58cb97223d13f2a1b87dd6614173f3cefe87ca6a74e3034c244a1");
EmbeddedCurvePoint local_r(local_r_x, local_r_y, false);
EmbeddedCurvePoint local_r(local_r_x, local_r_y);

// Use simulation to generate events
EventEmitter<EccAddEvent> ecc_add_event_emitter;
Expand Down Expand Up @@ -222,8 +222,8 @@ TEST(EccAddConstrainingTest, EccAddSameYDifferentX)
TEST(EccAddConstrainingTest, EccAddResultingInInfinity)
{
// R = P + (-P) = O; , where O is the point at infinity
EmbeddedCurvePoint q(p.x(), -p.y(), false);
EmbeddedCurvePoint r(0, 0, true);
EmbeddedCurvePoint q(p.x(), -p.y());
EmbeddedCurvePoint r(0, 0);

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 0 },
Expand Down Expand Up @@ -262,11 +262,11 @@ TEST(EccAddConstrainingTest, EccAddResultingInInfinity)

TEST(EccAddConstrainingTest, EccAddingToInfinity)
{
EmbeddedCurvePoint p(0, 0, true);
EmbeddedCurvePoint p(0, 0);

// R = O + Q = Q; , where O is the point at infinity

EmbeddedCurvePoint r(q.x(), q.y(), false);
EmbeddedCurvePoint r(q.x(), q.y());

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 1 },
Expand Down Expand Up @@ -305,10 +305,10 @@ TEST(EccAddConstrainingTest, EccAddingToInfinity)

TEST(EccAddConstrainingTest, EccAddingInfinity)
{
EmbeddedCurvePoint q(0, 0, true);
EmbeddedCurvePoint q(0, 0);

// R = P + O = P; , where O is the point at infinity
EmbeddedCurvePoint r(p.x(), p.y(), false);
EmbeddedCurvePoint r(p.x(), p.y());

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 1 },
Expand Down Expand Up @@ -348,10 +348,10 @@ TEST(EccAddConstrainingTest, EccAddingInfinity)

TEST(EccAddConstrainingTest, EccDoublingInf)
{
EmbeddedCurvePoint p(0, 0, true);
EmbeddedCurvePoint p(0, 0);

// r = O + O = O; , where O is the point at infinity
EmbeddedCurvePoint r(0, 0, true);
EmbeddedCurvePoint r(0, 0);

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 0 },
Expand Down Expand Up @@ -470,7 +470,7 @@ TEST(EccAddConstrainingTest, EccNegativeBadAdd)

FF r_x("0x20f096ae3de9aea007e0b94a0274b2443d6682d1901f6909f284ec967bc169be");
FF r_y("0x27948713833bb314e828f2b6f45f408da6564a3ac03b9e430a9c6634bb849ef2");
EmbeddedCurvePoint r(r_x, r_y, false);
EmbeddedCurvePoint r(r_x, r_y);

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 1 },
Expand Down Expand Up @@ -514,7 +514,7 @@ TEST(EccAddConstrainingTest, EccNegativeBadDouble)

FF r_x("0x2b01df0ef6d941a826bea23bece8243cbcdc159d5e97fbaa2171f028e05ba9b6");
FF r_y("0x0cc4c71e882bc62b7b3d1964a8540cb5211339dfcddd2e095fd444bf1aed4f09");
EmbeddedCurvePoint r(r_x, r_y, false);
EmbeddedCurvePoint r(r_x, r_y);

auto trace = TestTraceContainer({ {
{ C::ecc_add_op, 0 },
Expand Down Expand Up @@ -772,40 +772,6 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinity)
EventEmitter<ScalarMulEvent> scalar_mul_event_emitter;
NoopEventEmitter<EccAddMemoryEvent> ecc_add_memory_event_emitter;

StrictMock<MockExecutionIdManager> execution_id_manager;
StrictMock<MockGreaterThan> gt;
PureToRadix to_radix_simulator = PureToRadix();
EccSimulator ecc_simulator(execution_id_manager,
gt,
to_radix_simulator,
ecc_add_event_emitter,
scalar_mul_event_emitter,
ecc_add_memory_event_emitter);

EmbeddedCurvePoint result = ecc_simulator.scalar_mul(EmbeddedCurvePoint::infinity(), FF(10));
ASSERT_TRUE(result.is_infinity());

TestTraceContainer trace({
{ { C::precomputed_first_row, 1 } },
});

builder.process_scalar_mul(scalar_mul_event_emitter.dump_events(), trace);
builder.process_add(ecc_add_event_emitter.dump_events(), trace);

check_interaction<EccTraceBuilder, lookup_scalar_mul_double_settings, lookup_scalar_mul_add_settings>(trace);

check_relation<scalar_mul>(trace);
check_relation<ecc>(trace);
}

TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep)
{
EccTraceBuilder builder;

EventEmitter<EccAddEvent> ecc_add_event_emitter;
EventEmitter<ScalarMulEvent> scalar_mul_event_emitter;
NoopEventEmitter<EccAddMemoryEvent> ecc_add_memory_event_emitter;

StrictMock<MockExecutionIdManager> execution_id_manager;
StrictMock<MockGreaterThan> gt;
PureToRadix to_radix_simulator = PureToRadix();
Expand All @@ -817,16 +783,13 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinityRep)
ecc_add_memory_event_emitter);

EmbeddedCurvePoint inf = EmbeddedCurvePoint::infinity();
// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations.
// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf.
// EmbeddedCurvePoint preserves raw coordinates (see StandardAffinePointTest)

EmbeddedCurvePoint inf_bb = EmbeddedCurvePoint(avm2::AffinePoint::infinity());
EmbeddedCurvePoint inf_alt = EmbeddedCurvePoint(1, 2, true);

EmbeddedCurvePoint result = ecc_simulator.scalar_mul(inf_bb, FF(10));
ASSERT_TRUE(result.is_infinity());
result = ecc_simulator.scalar_mul(inf_alt, FF(10));
ASSERT_TRUE(result.is_infinity());
EXPECT_EQ(result.x(), inf.x());
EXPECT_EQ(result.y(), inf.y());

TestTraceContainer trace({
{ { C::precomputed_first_row, 1 } },
Expand Down Expand Up @@ -1309,7 +1272,7 @@ TEST(EccAddMemoryConstrainingTest, EccAddMemoryPointError)
// Point P is not on the curve
FF p_x("0x0000000000063d46918a156cae92db1bcbc4072a27ec81dc82ea959abdbcf16a");
FF p_y("0x00000000000c1370462c74775765d07fc21fd1093cc988149d3aa763bb3dbb60");
EmbeddedCurvePoint p(p_x, p_y, false);
EmbeddedCurvePoint p(p_x, p_y);

uint32_t dst_address = 0x1000;

Expand Down Expand Up @@ -1369,16 +1332,12 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations)
ecc_add_memory_event_emitter);
MemoryAddress dst_address = 5;

// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations.
// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf.

// Point P is infinity
EmbeddedCurvePoint inf = EmbeddedCurvePoint::infinity();
// EmbeddedCurvePoint always sets extractable coordinates as (0,0) and the underlying point as
// AffinePoint::infinity() for input infinity points.
EmbeddedCurvePoint inf_bb = EmbeddedCurvePoint(avm2::AffinePoint::infinity());
EmbeddedCurvePoint inf_alt = EmbeddedCurvePoint(0, 7, true);
EXPECT_EQ(inf_bb, inf_alt);
EXPECT_EQ(inf_bb, inf);
TestTraceContainer trace;

// The circuit correctly assigns double_op = true when doubling inf:
Expand Down Expand Up @@ -1419,25 +1378,6 @@ TEST(EccAddMemoryConstrainingTest, InfinityRepresentations)
trace.set(C::ecc_add_mem_q_y, 0, 2);
EXPECT_THROW_WITH_MESSAGE(check_relation<mem_aware_ecc>(trace, mem_aware_ecc::SR_Q_INF_X_CHECK), "Q_INF_X_CHECK");
EXPECT_THROW_WITH_MESSAGE(check_relation<mem_aware_ecc>(trace, mem_aware_ecc::SR_Q_INF_Y_CHECK), "Q_INF_Y_CHECK");

// TODO(#AVM-266): Rework test when is_infinity flag is removed from point representations.
// We now derive is_inf from coordinates, whereas previously we remapped coordinates /from/ is_inf.
// The below test no longer makes sense since it checks we store non-(0,0) coordinates for an inf
// point, which we do not allow.

// builder.process_add_with_memory(ecc_add_memory_event_emitter.dump_events(), trace);

// // The original coordinates are stored in memory for the read...
// EXPECT_EQ(trace.get(C::ecc_add_mem_q_x, 1), inf_bb.x());
// EXPECT_EQ(trace.get(C::ecc_add_mem_q_y, 1), inf_bb.y());
// // ...but normalised coordinates are sent to the ecc subtrace:
// EXPECT_EQ(trace.get(C::ecc_add_mem_q_x_n, 1), 0);
// EXPECT_EQ(trace.get(C::ecc_add_mem_q_y_n, 1), 0);
// check_relation<mem_aware_ecc>(trace);
// check_relation<ecc>(trace);
// check_all_interactions<EccTraceBuilder>(trace);
// check_interaction<tracegen::ExecutionTraceBuilder,
// bb::avm2::perm_execution_dispatch_to_ecc_add_settings>(trace);
}

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ void Ecc::add(MemoryInterface& memory,
} catch (const InternalEccException& e) {
// Note this point is not on the curve, but corresponds
// to default values the circuit will assign.
EmbeddedCurvePoint res = EmbeddedCurvePoint(0, 0, false);
// TODO(MW): This is now a point on the curve technically (inf) - check this doesnt cause issues now res.is_inf
// is true:
EmbeddedCurvePoint res = EmbeddedCurvePoint(0, 0);
add_memory_events.emit({ .execution_clk = execution_clk,
.space_id = space_id,
.p = p,
Expand Down
Loading
Loading