feat(avm)!: memory aware ecc add by IlyasRidhuan · Pull Request #15780 · AztecProtocol/aztec-packages

IlyasRidhuan · 2025-07-16T17:08:08Z

Makes ECC Add memory aware and integrates into the execution trace.

Currently only handles errors related to memory - another error handling PR to follow

IlyasRidhuan · 2025-07-16T17:08:27Z

feat(avm)!: ecc add error handling #15781
feat(avm)!: memory aware ecc add #15780 👈 (View in Graphite)
merge-train/avm

This stack of pull requests is managed by Graphite. Learn more about stacking.

IlyasRidhuan · 2025-07-21T14:24:13Z

barretenberg/cpp/src/barretenberg/vm2/tracegen/poseidon2_trace.cpp

        .add<lookup_poseidon2_mem_check_dst_addr_in_range_settings, InteractionType::LookupGeneric>()
        // Dispatch from Execution Trace
        .add<perm_poseidon2_mem_dispatch_exec_pos2_settings, InteractionType::Permutation>();
-//


chore: clean up from previous PR

dbanks12

LGTM! Mostly nits and one real question about where to consume gas in execution sim.

dbanks12 · 2025-07-21T14:38:51Z

barretenberg/cpp/src/barretenberg/vm2/simulation/execution.cpp

+{
+    constexpr auto opcode = ExecutionOpCode::ECADD;
+    auto& memory = context.get_memory();
+    get_gas_tracker().consume_gas();
+
+    // Read the points from memory.
+    MemoryValue p_x = memory.get(p_x_addr);
+    MemoryValue p_y = memory.get(p_y_addr);
+    MemoryValue p_inf = memory.get(p_inf_addr);
+
+    MemoryValue q_x = memory.get(q_x_addr);
+    MemoryValue q_y = memory.get(q_y_addr);
+    MemoryValue q_inf = memory.get(q_inf_addr);
+
+    set_and_validate_inputs(opcode, { p_x, p_y, p_inf, q_x, q_y, q_inf });


Should consume_gas come after set_and_validate_inputs? @fcarreiro mentioned that it should in this comment (after @sirasistant made that change in his PR).

ah ok i didnt realise it was order dependent

barretenberg/cpp/src/barretenberg/vm2/simulation/execution.cpp

dbanks12 · 2025-07-21T14:43:11Z

barretenberg/cpp/src/barretenberg/vm2/simulation/ecc.cpp

+    try {
+        // The resulting EmbeddedCurvePoint is a triple of (x, y, is_infinity).
+        // The x and y coordinates are stored at dst_address and dst_address + 1 respectively,
+        // and the is_infinity flag is stored at dst_address + 2.
+        // Therefore, the maximum address that needs to be written to is dst_address + 2.
+        uint64_t max_write_address = static_cast<uint64_t>(dst_address) + 2;
+        if (cmp.gt(max_write_address, AVM_HIGHEST_MEM_ADDRESS)) {
+            throw std::runtime_error("dst address out of range");
+        }


If more efficient, could consider omitting the call to cmp.gt and just checking if the dst_address is == MAX or == MAX-1 which are the only cases where dst_address+1 or +2 can be out of bounds. Similar to what I did for GetContractInstance.

hmm i think it ends up being less efficient for cases where there are > 1 options (i.e. a slice write of 2 or more) since it requires at least 1 batched inverse check and a higher degree error. In any case these range check rows are "free" since the memory operation is already 9 rows long

barretenberg/cpp/pil/vm2/ecc_mem.pil

dbanks12 · 2025-07-21T14:57:48Z

barretenberg/cpp/pil/vm2/ecc_mem.pil

+    ////////////////////////////////////////////////
+    // Dispatch from execution trace to ECC Add
+    ////////////////////////////////////////////////
+    #[DISPATCH_EXEC_ECC_ADD]
+    execution.sel_execute_ecc_add {


I'll be honest.... It feels confusing to me to have this inside the gadget 😅

yeah i think there was some discussion about a exec_dispatch.pil file to house all the lookups. The downside of putting it in execution.pil is it gets really messy

See [merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md). BEGIN_COMMIT_OVERRIDE feat!: constrain NullifierExists AVM opcode (#15636) feat(avm)!: memory aware ecc add (#15780) END_COMMIT_OVERRIDE --------- Co-authored-by: ludamad <adam.domurad@gmail.com> Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Ilyas Ridhuan <ilyas@aztecprotocol.com>

IlyasRidhuan mentioned this pull request Jul 16, 2025

feat(avm)!: Poseidon2 memory load and error handling #15693

Merged

IlyasRidhuan changed the title ~~feat(avm)! : comparison subtrace~~ feat(avm)! : memory aware ecc add Jul 16, 2025

IlyasRidhuan mentioned this pull request Jul 16, 2025

feat(avm)!: ecc add error handling #15781

Merged

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch 2 times, most recently from 9bd8bcd to b739a3d Compare July 16, 2025 17:16

IlyasRidhuan changed the title ~~feat(avm)! : memory aware ecc add~~ feat(avm)!: memory aware ecc add Jul 16, 2025

IlyasRidhuan force-pushed the ir/07-04-feat_avm_poseidon2_mem_errs branch from 9a654e3 to 3edaf30 Compare July 17, 2025 10:01

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch 3 times, most recently from 623e336 to b1e014e Compare July 17, 2025 15:15

IlyasRidhuan changed the base branch from ir/07-04-feat_avm_poseidon2_mem_errs to graphite-base/15780 July 17, 2025 16:02

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch from b1e014e to 43d9627 Compare July 19, 2025 14:43

IlyasRidhuan force-pushed the graphite-base/15780 branch from 3edaf30 to 0515513 Compare July 19, 2025 14:43

IlyasRidhuan changed the base branch from graphite-base/15780 to next July 19, 2025 14:43

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch from 43d9627 to 704a568 Compare July 19, 2025 15:02

IlyasRidhuan changed the base branch from next to graphite-base/15780 July 21, 2025 14:01

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch from 704a568 to 42c0ea4 Compare July 21, 2025 14:01

IlyasRidhuan changed the base branch from graphite-base/15780 to merge-train/avm July 21, 2025 14:01

IlyasRidhuan commented Jul 21, 2025

View reviewed changes

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch 2 times, most recently from e90a03c to 8ba5247 Compare July 21, 2025 14:31

IlyasRidhuan requested a review from dbanks12 July 21, 2025 14:32

IlyasRidhuan marked this pull request as ready for review July 21, 2025 14:32

IlyasRidhuan requested review from Maddiaa0, fcarreiro and jeanmon as code owners July 21, 2025 14:32

IlyasRidhuan removed request for Maddiaa0, fcarreiro and jeanmon July 21, 2025 14:32

dbanks12 approved these changes Jul 21, 2025

View reviewed changes

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch from 8ba5247 to fcb3687 Compare July 21, 2025 16:44

feat(avm)!: mem aware ec_add

307867f

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch from fcb3687 to 307867f Compare July 21, 2025 17:00

IlyasRidhuan merged commit 66ee06d into merge-train/avm Jul 21, 2025
5 checks passed

IlyasRidhuan deleted the ir/mem_aware_ecc_add branch July 21, 2025 17:35

AztecBot mentioned this pull request Jul 22, 2025

feat!: merge train AVM #15883

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(avm)!: memory aware ecc add#15780

feat(avm)!: memory aware ecc add#15780
IlyasRidhuan merged 1 commit intomerge-train/avmfrom
ir/mem_aware_ecc_add

IlyasRidhuan commented Jul 16, 2025 •

edited

Loading

Uh oh!

IlyasRidhuan commented Jul 16, 2025 •

edited

Loading

Uh oh!

IlyasRidhuan Jul 21, 2025

Uh oh!

dbanks12 left a comment

Uh oh!

dbanks12 Jul 21, 2025

Uh oh!

IlyasRidhuan Jul 21, 2025

Uh oh!

Uh oh!

dbanks12 Jul 21, 2025

Uh oh!

IlyasRidhuan Jul 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dbanks12 Jul 21, 2025

Uh oh!

IlyasRidhuan Jul 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

IlyasRidhuan commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IlyasRidhuan commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IlyasRidhuan Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

dbanks12 left a comment

Choose a reason for hiding this comment

Uh oh!

dbanks12 Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasRidhuan Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dbanks12 Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasRidhuan Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dbanks12 Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasRidhuan Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IlyasRidhuan commented Jul 16, 2025 •

edited

Loading

IlyasRidhuan commented Jul 16, 2025 •

edited

Loading