Skip to content

Commit 908d08b

Browse files
MirandaWoodMirandaWood
authored
feat(avm)!: WIP remove is_infinite flags from ECADD opcode (AVM only) (#22945)
This branch includes the changes to remove the `is_infinite` flags from the ECADD opcode fn signature. The actual EC logic changes come above this PR in the stack, and any changes outside the AVM will be below. For ease of review, I've separated into commits: - **feat: remove inf flags from ecadd opcode - ec flow only** Isolated to the EC flow only (does not change registers so non avm tests will fail) - f**eat: rem infs from fuzzer (only gadget fuzzer tested)** Isolated fixes to get the fuzzer(s) compiling Will partially close [Foundation AVM Issue 19](https://linear.app/aztec-foundation/issue/AVM-19/) (the following PR #23031 with ts/rs changes will fully close it). Note that the opcode mismatches that in ts so **CI will fail** until #23031 is merged into this branch! --- Stack: - #22745 - #22564 - #22921 - #22795 - `mw/avm-rem-inf-opcode-ecadd` <-- here - #23031
1 parent dfaf685 commit 908d08b

11 files changed

Lines changed: 53 additions & 138 deletions

File tree

‎barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/instruction.hpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -573,12 +573,10 @@ struct SUCCESSCOPY_Instruction {
573573
struct ECADD_Instruction {
574574
ParamRef p1_x;
575575
ParamRef p1_y;
576-
ParamRef p1_infinite;
577576
ParamRef p2_x;
578577
ParamRef p2_y;
579-
ParamRef p2_infinite;
580578
AddressRef result;
581-
SERIALIZATION_FIELDS(p1_x, p1_y, p1_infinite, p2_x, p2_y, p2_infinite, result);
579+
SERIALIZATION_FIELDS(p1_x, p1_y, p2_x, p2_y, result);
582580
};
583581

584582
/// @brief POSEIDON2PERM: Perform Poseidon2 permutation on 4 FF values
@@ -881,8 +879,8 @@ inline std::ostream& operator<<(std::ostream& os, const FuzzInstruction& instruc
881879
<< arg.dst_address;
882880
},
883881
[&](ECADD_Instruction arg) {
884-
os << "ECADD_Instruction " << arg.p1_x << " " << arg.p1_y << " " << arg.p1_infinite << " " << arg.p2_x
885-
<< " " << arg.p2_y << " " << arg.p2_infinite << " " << arg.result;
882+
os << "ECADD_Instruction " << arg.p1_x << " " << arg.p1_y << " " << arg.p2_x << " " << arg.p2_y << " "
883+
<< arg.result;
886884
},
887885
[&](POSEIDON2PERM_Instruction arg) {
888886
os << "POSEIDON2PERM_Instruction " << arg.src_address << " " << arg.dst_address;

‎barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/program_block.cpp‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,40 +1287,32 @@ void ProgramBlock::process_ecadd_instruction(ECADD_Instruction instruction)
12871287
#endif
12881288
auto p1_x = memory_manager.get_resolved_address_and_operand_16(instruction.p1_x);
12891289
auto p1_y = memory_manager.get_resolved_address_and_operand_16(instruction.p1_y);
1290-
auto p1_inf = memory_manager.get_resolved_address_and_operand_16(instruction.p1_infinite);
12911290
auto p2_x = memory_manager.get_resolved_address_and_operand_16(instruction.p2_x);
12921291
auto p2_y = memory_manager.get_resolved_address_and_operand_16(instruction.p2_y);
1293-
auto p2_inf = memory_manager.get_resolved_address_and_operand_16(instruction.p2_infinite);
12941292
auto result = memory_manager.get_resolved_address_and_operand_16(instruction.result);
12951293

1296-
if (!p1_x.has_value() || !p1_y.has_value() || !p1_inf.has_value() || !p2_x.has_value() || !p2_y.has_value() ||
1297-
!p2_inf.has_value() || !result.has_value()) {
1294+
if (!p1_x.has_value() || !p1_y.has_value() || !p2_x.has_value() || !p2_y.has_value() || !result.has_value()) {
12981295
return;
12991296
}
13001297

13011298
preprocess_memory_addresses(p1_x.value().first);
13021299
preprocess_memory_addresses(p1_y.value().first);
1303-
preprocess_memory_addresses(p1_inf.value().first);
13041300
preprocess_memory_addresses(p2_x.value().first);
13051301
preprocess_memory_addresses(p2_y.value().first);
1306-
preprocess_memory_addresses(p2_inf.value().first);
13071302
preprocess_memory_addresses(result.value().first);
13081303

13091304
auto ecadd_instruction = bb::avm2::testing::InstructionBuilder(bb::avm2::WireOpCode::ECADD)
13101305
.operand(p1_x.value().second)
13111306
.operand(p1_y.value().second)
1312-
.operand(p1_inf.value().second)
13131307
.operand(p2_x.value().second)
13141308
.operand(p2_y.value().second)
1315-
.operand(p2_inf.value().second)
13161309
.operand(result.value().second)
13171310
.build();
13181311
instructions.push_back(ecadd_instruction);
13191312

1320-
// ECADD writes 3 consecutive memory locations: result_x (FF), result_y (FF), result_is_inf (U1)
1313+
// ECADD writes 2 consecutive memory locations: result_x (FF), result_y (FF)
13211314
memory_manager.set_memory_address(bb::avm2::MemoryTag::FF, result.value().first.absolute_address);
13221315
memory_manager.set_memory_address(bb::avm2::MemoryTag::FF, result.value().first.absolute_address + 1);
1323-
memory_manager.set_memory_address(bb::avm2::MemoryTag::U1, result.value().first.absolute_address + 2);
13241316
}
13251317

13261318
void ProgramBlock::process_poseidon2perm_instruction(POSEIDON2PERM_Instruction instruction)

‎barretenberg/cpp/src/barretenberg/avm_fuzzer/harness/ecc.fuzzer.cpp‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ struct EccFuzzerInput {
9494
AffinePoint q = AffinePoint::one();
9595
Fq scalar = Fq::zero();
9696
// Addresses are organised as:
97-
// p_x, p_y, p_inf, q_x, q_y, q_inf, output_addr
98-
std::array<MemoryAddress, 7> addresses{};
97+
// p_x, p_y, q_x, q_y, output_addr
98+
std::array<MemoryAddress, 5> addresses{};
9999
EccFuzzerInput() = default;
100100

101101
// Serialize to buffer
@@ -109,7 +109,7 @@ struct EccFuzzerInput {
109109
Fq::serialize_to_buffer(scalar, buffer + offset);
110110
offset += sizeof(Fq);
111111
// Serialize memory addresses
112-
std::memcpy(buffer + offset, &addresses[0], sizeof(MemoryAddress) * 7);
112+
std::memcpy(buffer + offset, &addresses[0], sizeof(MemoryAddress) * 5);
113113
}
114114

115115
static EccFuzzerInput from_buffer(const uint8_t* buffer)
@@ -137,7 +137,7 @@ struct EccFuzzerInput {
137137
input.scalar = Fq::serialize_from_buffer(buffer + offset);
138138
offset += sizeof(Fq);
139139
// Deserialize memory addresses
140-
std::memcpy(&input.addresses[0], buffer + offset, sizeof(MemoryAddress) * 7);
140+
std::memcpy(&input.addresses[0], buffer + offset, sizeof(MemoryAddress) * 5);
141141

142142
return input;
143143
}
@@ -210,7 +210,7 @@ extern "C" size_t LLVMFuzzerCustomMutator(uint8_t* data, size_t size, size_t max
210210
case 7: {
211211
// Mutate memory addresses
212212
// Select a random address to mutate
213-
std::uniform_int_distribution<size_t> addr_dist(0, 6);
213+
std::uniform_int_distribution<size_t> addr_dist(0, 4);
214214
size_t addr_index = addr_dist(rng);
215215
input.addresses[addr_index] = mutate_memory_address(input.addresses[addr_index], rng);
216216
break;
@@ -268,15 +268,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
268268

269269
mem->set(/*p_x_addr*/ input.addresses[0], MemoryValue::from_tag(MemoryTag::FF, point_p.x()));
270270
mem->set(/*p_y_addr*/ input.addresses[1], MemoryValue::from_tag(MemoryTag::FF, point_p.y()));
271-
mem->set(/*p_inf*/ input.addresses[2], MemoryValue::from_tag(MemoryTag::U1, point_p.is_infinity() ? FF(1) : FF(0)));
272-
mem->set(/*q_x_addr*/ input.addresses[3], MemoryValue::from_tag(MemoryTag::FF, point_q.x()));
273-
mem->set(/*q_y_addr*/ input.addresses[4], MemoryValue::from_tag(MemoryTag::FF, point_q.y()));
274-
mem->set(/*q_inf*/ input.addresses[5], MemoryValue::from_tag(MemoryTag::U1, point_q.is_infinity() ? FF(1) : FF(0)));
271+
mem->set(/*q_x_addr*/ input.addresses[2], MemoryValue::from_tag(MemoryTag::FF, point_q.x()));
272+
mem->set(/*q_y_addr*/ input.addresses[3], MemoryValue::from_tag(MemoryTag::FF, point_q.y()));
275273

276274
EmbeddedCurvePoint scalar_mul_result;
277275

278276
try {
279-
ecc.add(*mem, point_p, point_q, /* output_addr */ input.addresses[6]);
277+
ecc.add(*mem, point_p, point_q, /* output_addr */ input.addresses[4]);
280278
scalar_mul_result = ecc.scalar_mul(input.p, FF(uint256_t(input.scalar)));
281279
} catch (std::exception& e) {
282280
// info("Caught exception during ECC add: {}", e.what());
@@ -286,8 +284,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
286284
EmbeddedCurvePoint expected_result = point_p + point_q;
287285

288286
// Verify output in memory
289-
MemoryValue res_x = mem->get(input.addresses[6]);
290-
MemoryValue res_y = mem->get(input.addresses[6] + 1);
287+
MemoryValue res_x = mem->get(input.addresses[4]);
288+
MemoryValue res_y = mem->get(input.addresses[4] + 1);
291289

292290
EmbeddedCurvePoint result_point = EmbeddedCurvePoint(res_x.as_ff(), res_y.as_ff());
293291

@@ -307,15 +305,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
307305
auto trace = TestTraceContainer({ {
308306
{ Column::execution_context_id, 0 },
309307
// Point P
310-
{ Column::execution_register_0_, point_p.x() }, // = px
311-
{ Column::execution_register_1_, point_p.y() }, // = py
312-
{ Column::execution_register_2_, point_p.is_infinity() ? FF(1) : FF(0) }, // = p_inf
308+
{ Column::execution_register_0_, point_p.x() }, // = px
309+
{ Column::execution_register_1_, point_p.y() }, // = py
313310
// Point Q
314-
{ Column::execution_register_3_, point_q.x() }, // = qx
315-
{ Column::execution_register_4_, point_q.y() }, // = qy
316-
{ Column::execution_register_5_, point_q.is_infinity() ? FF(1) : FF(0) }, // = q_inf
311+
{ Column::execution_register_2_, point_q.x() }, // = qx
312+
{ Column::execution_register_3_, point_q.y() }, // = qy
317313
// Dst address
318-
{ Column::execution_rop_6_, input.addresses[6] }, // = dst_addr
314+
{ Column::execution_rop_4_, input.addresses[4] }, // = dst_addr
319315
{ Column::execution_sel_exec_dispatch_ecc_add, 1 }, // = sel
320316
{ Column::execution_sel_opcode_error, error ? 1 : 0 }, // = sel_err
321317
} });

‎barretenberg/cpp/src/barretenberg/avm_fuzzer/mutations/instructions/instruction.cpp‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -428,17 +428,15 @@ std::vector<FuzzInstruction> InstructionMutator::generate_ecadd_instruction(std:
428428
// Random mode: use existing memory values (may fail if not valid points on curve)
429429
return { ECADD_Instruction{ .p1_x = generate_variable_ref(rng),
430430
.p1_y = generate_variable_ref(rng),
431-
.p1_infinite = generate_variable_ref(rng),
432431
.p2_x = generate_variable_ref(rng),
433432
.p2_y = generate_variable_ref(rng),
434-
.p2_infinite = generate_variable_ref(rng),
435433
.result = generate_address_ref(rng, MAX_16BIT_OPERAND) } };
436434
}
437435

438436
// Backfill mode: generate valid points on the Grumpkin curve and SET them
439-
// 6 SET instructions (2 points * 3 fields each) + 1 ECADD = 7 instructions
437+
// 4 SET instructions (2 points * 4 fields each) + 1 ECADD = 5 instructions
440438
std::vector<FuzzInstruction> instructions;
441-
instructions.reserve(7);
439+
instructions.reserve(5);
442440

443441
// Generate a valid point via scalar multiplication of the generator (always on curve)
444442
auto generate_point = [&rng]() {
@@ -447,17 +445,12 @@ std::vector<FuzzInstruction> InstructionMutator::generate_ecadd_instruction(std:
447445
};
448446

449447
// Generate SET instructions to backfill a point at the given addresses
450-
auto backfill_point = [&instructions](const bb::avm2::EmbeddedCurvePoint& point,
451-
AddressRef x_addr,
452-
AddressRef y_addr,
453-
AddressRef inf_addr) {
448+
auto backfill_point = [&instructions](
449+
const bb::avm2::EmbeddedCurvePoint& point, AddressRef x_addr, AddressRef y_addr) {
454450
instructions.push_back(
455451
SET_FF_Instruction{ .value_tag = bb::avm2::MemoryTag::FF, .result_address = x_addr, .value = point.x() });
456452
instructions.push_back(
457453
SET_FF_Instruction{ .value_tag = bb::avm2::MemoryTag::FF, .result_address = y_addr, .value = point.y() });
458-
instructions.push_back(SET_8_Instruction{ .value_tag = bb::avm2::MemoryTag::U1,
459-
.result_address = inf_addr,
460-
.value = static_cast<uint8_t>(point.is_infinity() ? 1 : 0) });
461454
};
462455

463456
auto p1 = generate_point();
@@ -466,20 +459,16 @@ std::vector<FuzzInstruction> InstructionMutator::generate_ecadd_instruction(std:
466459
// Generate addresses (SET_FF uses 16-bit, SET_8 uses 8-bit operands)
467460
AddressRef p1_x_addr = generate_address_ref(rng, MAX_16BIT_OPERAND);
468461
AddressRef p1_y_addr = generate_address_ref(rng, MAX_16BIT_OPERAND);
469-
AddressRef p1_inf_addr = generate_address_ref(rng, MAX_8BIT_OPERAND);
470462
AddressRef p2_x_addr = generate_address_ref(rng, MAX_16BIT_OPERAND);
471463
AddressRef p2_y_addr = generate_address_ref(rng, MAX_16BIT_OPERAND);
472-
AddressRef p2_inf_addr = generate_address_ref(rng, MAX_8BIT_OPERAND);
473464

474-
backfill_point(p1, p1_x_addr, p1_y_addr, p1_inf_addr);
475-
backfill_point(p2, p2_x_addr, p2_y_addr, p2_inf_addr);
465+
backfill_point(p1, p1_x_addr, p1_y_addr);
466+
backfill_point(p2, p2_x_addr, p2_y_addr);
476467

477468
instructions.push_back(ECADD_Instruction{ .p1_x = p1_x_addr,
478469
.p1_y = p1_y_addr,
479-
.p1_infinite = p1_inf_addr,
480470
.p2_x = p2_x_addr,
481471
.p2_y = p2_y_addr,
482-
.p2_infinite = p2_inf_addr,
483472
.result = generate_address_ref(rng, MAX_16BIT_OPERAND) });
484473

485474
return instructions;
@@ -1476,8 +1465,8 @@ void InstructionMutator::mutate_successcopy_instruction(SUCCESSCOPY_Instruction&
14761465

14771466
void InstructionMutator::mutate_ecadd_instruction(ECADD_Instruction& instruction, std::mt19937_64& rng)
14781467
{
1479-
// ECADD has 7 operands, select one to mutate
1480-
int choice = std::uniform_int_distribution<int>(0, 6)(rng);
1468+
// ECADD has 5 operands, select one to mutate
1469+
int choice = std::uniform_int_distribution<int>(0, 4)(rng);
14811470
switch (choice) {
14821471
case 0:
14831472
mutate_param_ref(instruction.p1_x, rng, MemoryTag::FF, MAX_16BIT_OPERAND);
@@ -1486,18 +1475,12 @@ void InstructionMutator::mutate_ecadd_instruction(ECADD_Instruction& instruction
14861475
mutate_param_ref(instruction.p1_y, rng, MemoryTag::FF, MAX_16BIT_OPERAND);
14871476
break;
14881477
case 2:
1489-
mutate_param_ref(instruction.p1_infinite, rng, MemoryTag::U1, MAX_16BIT_OPERAND);
1490-
break;
1491-
case 3:
14921478
mutate_param_ref(instruction.p2_x, rng, MemoryTag::FF, MAX_16BIT_OPERAND);
14931479
break;
1494-
case 4:
1480+
case 3:
14951481
mutate_param_ref(instruction.p2_y, rng, MemoryTag::FF, MAX_16BIT_OPERAND);
14961482
break;
1497-
case 5:
1498-
mutate_param_ref(instruction.p2_infinite, rng, MemoryTag::U1, MAX_16BIT_OPERAND);
1499-
break;
1500-
case 6:
1483+
case 4:
15011484
mutate_address_ref(instruction.result, rng, MAX_16BIT_OPERAND);
15021485
break;
15031486
}

‎barretenberg/cpp/src/barretenberg/vm2/common/instruction_spec.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ const std::unordered_map<WireOpCode, std::array<uint8_t, NUM_OP_DC_SELECTORS>>&
9999
{ WireOpCode::POSEIDON2PERM, { 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
100100
{ WireOpCode::SHA256COMPRESSION, { 0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
101101
{ WireOpCode::KECCAKF1600, { 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
102-
{ WireOpCode::ECADD, { 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
102+
{ WireOpCode::ECADD, { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
103103
// Conversions
104104
{ WireOpCode::TORADIXBE, { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
105105
};
@@ -420,7 +420,7 @@ const std::unordered_map<WireOpCode, WireInstructionSpec>& get_wire_instruction_
420420
.op_dc_selectors = get_wire_opcode_dc_selectors().at(WireOpCode::KECCAKF1600) } },
421421
{ WireOpCode::ECADD,
422422
{ .exec_opcode = ExecutionOpCode::ECADD,
423-
.size_in_bytes = 17,
423+
.size_in_bytes = 13,
424424
.op_dc_selectors = get_wire_opcode_dc_selectors().at(WireOpCode::ECADD) } },
425425
// Conversions
426426
{ WireOpCode::TORADIXBE,
@@ -737,14 +737,12 @@ const std::unordered_map<ExecutionOpCode, ExecInstructionSpec>& get_exec_instruc
737737
{ .num_addresses = 2,
738738
.gas_cost = { .opcode_gas = AVM_KECCAKF1600_BASE_L2_GAS, .base_da = 0, .dyn_l2 = 0, .dyn_da = 0 } } },
739739
{ ExecutionOpCode::ECADD,
740-
{ .num_addresses = 7,
740+
{ .num_addresses = 5,
741741
.gas_cost = { .opcode_gas = AVM_ECADD_BASE_L2_GAS, .base_da = 0, .dyn_l2 = 0, .dyn_da = 0 },
742742
.register_info = RegisterInfo().add_inputs({ /*p_x=*/ValueTag::FF,
743743
/*p_y=*/ValueTag::FF,
744-
/*p_inf*/ ValueTag::U1,
745744
/*q_x*/ ValueTag::FF,
746-
/*q_y*/ ValueTag::FF,
747-
/*q_inf*/ ValueTag::U1 }) } },
745+
/*q_y*/ ValueTag::FF }) } },
748746
{ ExecutionOpCode::TORADIXBE,
749747
{ .num_addresses = 5,
750748
.gas_cost = { .opcode_gas = AVM_TORADIXBE_BASE_L2_GAS,

0 commit comments

Comments
 (0)