feat(avm)!: ecc add error handling by IlyasRidhuan · Pull Request #15781 · AztecProtocol/aztec-packages

IlyasRidhuan · 2025-07-16T17:10:26Z

PR adds ECC error handling (is on curve checks) to the opcode handling variety. Internally used gadgets assume points are on curve.

This is more optimal as we can make reasonable assumptions about the points when using them internally

IlyasRidhuan · 2025-07-16T17:10:51Z

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

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

dbanks12

LGTM! Just one comment about inf. Your comments are helpful!

dbanks12 · 2025-07-22T15:01:13Z

barretenberg/cpp/pil/vm2/ecc_mem.pil

+    pol P_X3 = p_x * p_x * p_x;
+    pol P_Y2 = p_y * p_y;
+    #[P_CURVE_EQN]
+    p_is_on_curve_eqn = sel * (P_Y2 - (P_X3 - 17)) * (1 - p_is_inf); // Infinity considered as on curve


"Infinity considered as on curve", but if p_is_inf = 1, we get p_is_on_curve_eqn = 0

yep that is intended. In short, the curve equation is y^2 = x^3 - 17, so it gets re-arranged to p_is_on_curve_eqn = y^2 - (x^3 - 17) and to be on the curve we want p_is_on_curve_eqn = 0.

Now a quirk is that the point at infinity actually doesnt satisfy the equation. So im multiplifying it by zero essentially to ensure we get p_is_on_curve_eqn = 0.

@MirandaWood is actually mathematically qualified so please tell me if this is correct.

Wait so p_is_on_curve == "p is NOT on the curve"?

Sounds correct to me!
Looks like we want p_is_on_curve_eqn = 0 iff P is on the curve, which is true when y^2 - (x^3 - 17) = 0 or we have the point at infinity. So LGTM 🎉

@dbanks12 it's basically a commited column to represent an evaluation of the "point is on curve" equation. It can have values between [0, p - 1] (basically any field value). If this value !=0 we raise the "not_on_curve_err"

I see.... A bit confusing because the variable name sounds like a boolean, but I guess the _eqn suffix is meant to signal otherwise.

dbanks12 · 2025-07-22T15:03:14Z

barretenberg/cpp/pil/vm2/ecc_mem.pil

+    pol Q_X3 = q_x * q_x * q_x;
+    pol Q_Y2 = q_y * q_y;
+    #[Q_CURVE_EQN]
+    q_is_on_curve_eqn = sel * (Q_Y2 - (Q_X3 - 17)) * (1 - q_is_inf); // Infinity considered as on curve


MirandaWood

Nice!! Just nits really (sorry!), could be addressed separately so happy to merge 🚀

barretenberg/cpp/pil/vm2/ecc_mem.pil

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp

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

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

barretenberg/cpp/src/barretenberg/vm2/tracegen/execution_trace.cpp

See [merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md). BEGIN_COMMIT_OVERRIDE feat!: constrain AVM EmitNullifier opcode (#15853) feat(avm)!: ecc add error handling (#15781) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com> Co-authored-by: Ilyas Ridhuan <ilyas@aztecprotocol.com> Co-authored-by: MirandaWood <miranda@aztecprotocol.com>

This was referenced Jul 16, 2025

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

Merged

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

Merged

IlyasRidhuan changed the base branch from ir/mem_aware_ecc_add to graphite-base/15781 July 16, 2025 17:15

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from 62e4c6e to 09f2ac3 Compare July 16, 2025 17:23

IlyasRidhuan force-pushed the graphite-base/15781 branch from c1d7551 to b739a3d Compare July 16, 2025 17:23

IlyasRidhuan changed the base branch from graphite-base/15781 to ir/mem_aware_ecc_add July 16, 2025 17:23

IlyasRidhuan changed the title ~~feat: integrate gt circuit logic~~ feat(avm)!: ecc add error handling Jul 16, 2025

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch 2 times, most recently from 5d77ab8 to 6333894 Compare July 17, 2025 10:01

IlyasRidhuan force-pushed the ir/mem_aware_ecc_add branch 2 times, most recently from 14b9f70 to 623e336 Compare July 17, 2025 14:49

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch 2 times, most recently from b604a92 to a284392 Compare July 17, 2025 15:05

IlyasRidhuan changed the base branch from ir/mem_aware_ecc_add to graphite-base/15781 July 17, 2025 15:15

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from a284392 to 52d8fcd Compare July 19, 2025 14:58

IlyasRidhuan force-pushed the graphite-base/15781 branch from 623e336 to 43d9627 Compare July 19, 2025 14:58

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from 52d8fcd to b902cb8 Compare July 19, 2025 14:59

IlyasRidhuan changed the base branch from graphite-base/15781 to ir/mem_aware_ecc_add July 19, 2025 14:59

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

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from b902cb8 to 2aedb0f Compare July 19, 2025 15:02

IlyasRidhuan changed the base branch from ir/mem_aware_ecc_add to graphite-base/15781 July 21, 2025 14:01

IlyasRidhuan force-pushed the graphite-base/15781 branch from 704a568 to e7063f1 Compare July 22, 2025 07:20

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from 2aedb0f to d94484a Compare July 22, 2025 07:20

IlyasRidhuan changed the base branch from graphite-base/15781 to merge-train/avm July 22, 2025 07:20

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from d94484a to 33ed091 Compare July 22, 2025 07:47

IlyasRidhuan marked this pull request as ready for review July 22, 2025 07:49

IlyasRidhuan requested review from Maddiaa0, fcarreiro and jeanmon as code owners July 22, 2025 07:49

IlyasRidhuan requested review from MirandaWood and dbanks12 and removed request for Maddiaa0, fcarreiro and jeanmon July 22, 2025 07:49

dbanks12 approved these changes Jul 22, 2025

View reviewed changes

Base automatically changed from merge-train/avm to next July 22, 2025 15:13

IlyasRidhuan requested a review from LeilaWang as a code owner July 22, 2025 15:13

IlyasRidhuan removed the request for review from LeilaWang July 22, 2025 15:36

MirandaWood approved these changes Jul 22, 2025

View reviewed changes

MirandaWood and others added 3 commits July 22, 2025 19:26

feat(avm)!: err handling

0a43ba1

feat: ecc_mem handles point error

c1ad8fa

address comments

b5a0f1c

IlyasRidhuan changed the base branch from next to graphite-base/15781 July 22, 2025 19:50

IlyasRidhuan force-pushed the ir/ec_add_err_handle branch from 33ed091 to b5a0f1c Compare July 22, 2025 19:51

IlyasRidhuan changed the base branch from graphite-base/15781 to merge-train/avm July 22, 2025 19:51

IlyasRidhuan merged commit 6351f9e into merge-train/avm Jul 22, 2025
9 of 10 checks passed

IlyasRidhuan deleted the ir/ec_add_err_handle branch July 22, 2025 21:13

AztecBot mentioned this pull request Jul 22, 2025

feat: merge-train/avm #15884

Merged

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!

dbanks12 left a comment

Choose a reason for hiding this comment

Uh oh!

dbanks12 Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasRidhuan Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

dbanks12 Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

MirandaWood Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasRidhuan Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

dbanks12 Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

dbanks12 Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

MirandaWood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IlyasRidhuan commented Jul 16, 2025 •

edited

Loading

IlyasRidhuan commented Jul 16, 2025 •

edited

Loading