Skip to content

Conversation

@IlyasRidhuan
Copy link
Contributor

@IlyasRidhuan IlyasRidhuan commented Dec 27, 2025

Preamble

The representation of the point at infinity differs between BB and noir, with the latter using {x: 0, y: 0, inf: 1}. To ensure we match with noir's expectations we wrap the bb AffinePoint in a struct StandardAffinePoint that outputs the noir representation if you ask for the coordinates of a point at infinity.

The bug

The ECADD opcode loads points from addresses in memory (i.e. x-coord, y-coord, inf-flag). It is possible that some bytecode loads a point such that x != 0, y != 0, inf = 1. When this happens during simulation, the execution trace will load the original coordinate values but when the EC gadget is called, it utilises StandardAffinePoint which incorrectly outputs {x: 0, y:0, inf = 1 when building the EC events.

This causes a Lookup PERM_EXECUTION_DISPATCH_TO_ECC_ADD failed. because the value loaded in the registers in the execution trace no longer match the values in the ECC subtrace.

Solution

The subtlety in StandardAffinePoint is that we only want the point at infinity to be {x: 0, y: 0, inf: 1}, when the result of an operation results in infinity. Otherwise we want to use the original coordinates of the point. This means we need to track the original coordinate values as well in the StandardAffinePoint.

Copy link
Contributor Author

IlyasRidhuan commented Dec 27, 2025

@IlyasRidhuan IlyasRidhuan force-pushed the ir/12-27-fix_avm_preserve_coordinates_in_embedded_curve_point branch from 3d3e783 to 75ab6a6 Compare December 27, 2025 23:15
@IlyasRidhuan IlyasRidhuan marked this pull request as ready for review January 2, 2026 09:04
@IlyasRidhuan IlyasRidhuan requested review from sirasistant and removed request for fcarreiro and jeanmon January 2, 2026 09:04
Copy link
Contributor

@sirasistant sirasistant left a comment

Choose a reason for hiding this comment

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

nice catch!

@IlyasRidhuan IlyasRidhuan force-pushed the ir/12-27-fix_avm_sha256_modulo_add_bug branch from 374ec40 to 1245550 Compare January 2, 2026 12:10
@IlyasRidhuan IlyasRidhuan force-pushed the ir/12-27-fix_avm_preserve_coordinates_in_embedded_curve_point branch from 75ab6a6 to 280572e Compare January 2, 2026 12:10
Base automatically changed from ir/12-27-fix_avm_sha256_modulo_add_bug to merge-train/avm January 2, 2026 13:54
@IlyasRidhuan IlyasRidhuan requested a review from Maddiaa0 as a code owner January 2, 2026 13:54
@IlyasRidhuan IlyasRidhuan removed the request for review from Maddiaa0 January 2, 2026 13:54
@IlyasRidhuan IlyasRidhuan merged commit a414e97 into merge-train/avm Jan 2, 2026
15 of 17 checks passed
@IlyasRidhuan IlyasRidhuan deleted the ir/12-27-fix_avm_preserve_coordinates_in_embedded_curve_point branch January 2, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants