refactor(ec): store EcState in projective coordinates (#1654)

`EcState` was stored as an affine pair, so every `ec_state_add` ended in
`to_affine()` — a modular inversion — only for the next add to undo it.
Storing the accumulator projectively defers that to a single inversion in
`ec_state_try_finalize_nz`, so the cost of a chain of adds no longer grows
with its length.

Author: TomerStarkware

Cargo.toml

@@ -86,7 +86,7 @@ sierra-emu = { path = "debug_utils/sierra-emu", version = "0.9.0-rc.7" }
 smallvec = "1.13.2"
 starknet-crypto = "0.8.1"
 starknet-curve = "0.6.0"
-starknet-types-core = { version = "0.2.3", features = ["hash"]}
+starknet-types-core = { version = "0.2.4", features = ["hash"]}
 stats_alloc = "0.1.10"
 tempfile = "3.15.0"
 test-case = "3.3"

debug_utils/sierra-emu/src/value.rs

@@ -54,9 +54,17 @@ pub enum Value {
         x: Felt,
         y: Felt,
     },
+    /// An elliptic curve accumulator in **projective** coordinates `[x : y : z]`,
+    /// representing the affine point `(x/z, y/z)`. `z == 0` is the point at
+    /// infinity.
+    ///
+    /// Note that `PartialEq` here is bitwise on the representative, not
+    /// projective equivalence, so the emulator must perform the same sequence of
+    /// curve operations as the runtime.
     EcState {
         x: Felt,
         y: Felt,
+        z: Felt,
     },
     I128(i128),
     I64(i64),

debug_utils/sierra-emu/src/vm.rs

@@ -262,6 +262,7 @@ impl VirtualMachine {
                         CoreTypeConcrete::RangeCheck(_)
                         | CoreTypeConcrete::RangeCheck96(_)
                         | CoreTypeConcrete::Bitwise(_)
+                        | CoreTypeConcrete::EcOp(_)
                         | CoreTypeConcrete::Pedersen(_)
                         | CoreTypeConcrete::Poseidon(_)
                         | CoreTypeConcrete::SegmentArena(_)

debug_utils/sierra-emu/src/vm/ec.rs

@@ -112,11 +112,13 @@ fn eval_state_init(
     _info: &SignatureOnlyConcreteLibfunc,
     _args: Vec<Value>,
 ) -> EvalAction {
+    // The canonical projective identity `[0 : 1 : 0]`.
     EvalAction::NormalBranch(
         0,
         smallvec![Value::EcState {
             x: 0.into(),
-            y: 0.into()
+            y: 1.into(),
+            z: 0.into(),
         }],
     )
 }
@@ -126,65 +128,80 @@ fn eval_state_add(
     _info: &SignatureOnlyConcreteLibfunc,
     args: Vec<Value>,
 ) -> EvalAction {
-    let [Value::EcState { x: s_x, y: s_y }, Value::EcPoint { x, y }]: [Value; 2] =
-        args.try_into().unwrap()
+    let [Value::EcState {
+        x: s_x,
+        y: s_y,
+        z: s_z,
+    }, Value::EcPoint { x, y }]: [Value; 2] = args.try_into().unwrap()
     else {
         panic!()
     };
 
-    if s_x.is_zero() && s_y.is_zero() {
-        return EvalAction::NormalBranch(0, smallvec![Value::EcState { x, y }]);
-    }
-    let mut state = ProjectivePoint::from_affine(s_x, s_y).unwrap();
-    let point = AffinePoint::new(x, y).unwrap();
+    let mut state = ProjectivePoint::new_unchecked(s_x, s_y, s_z);
+    let point = ProjectivePoint::from_affine(x, y).unwrap();
 
     state += &point;
-    let (x, y) = match state.to_affine() {
-        Ok(state) => (state.x(), state.y()),
-        Err(_) => (Felt::ZERO, Felt::ZERO),
-    };
-    EvalAction::NormalBranch(0, smallvec![Value::EcState { x, y }])
+    EvalAction::NormalBranch(
+        0,
+        smallvec![Value::EcState {
+            x: state.x(),
+            y: state.y(),
+            z: state.z(),
+        }],
+    )
 }
 
 fn eval_state_add_mul(
     _registry: &ProgramRegistry<CoreType, CoreLibfunc>,
     _info: &SignatureOnlyConcreteLibfunc,
     args: Vec<Value>,
 ) -> EvalAction {
-    let [ec @ Value::Unit, Value::EcState { x: s_x, y: s_y }, Value::Felt(scalar), Value::EcPoint { x, y }]: [Value; 4] =
-        args.try_into().unwrap()
+    let [ec @ Value::Unit, Value::EcState {
+        x: s_x,
+        y: s_y,
+        z: s_z,
+    }, Value::Felt(scalar), Value::EcPoint { x, y }]: [Value; 4] = args.try_into().unwrap()
     else {
         panic!()
     };
 
-    let mut state = if s_x.is_zero() && s_y.is_zero() {
-        ProjectivePoint::identity()
-    } else {
-        ProjectivePoint::from_affine(s_x, s_y).unwrap()
-    };
+    let mut state = ProjectivePoint::new_unchecked(s_x, s_y, s_z);
     let point = ProjectivePoint::from_affine(x, y).unwrap();
 
     state += &point.mul(scalar);
-    let (x, y) = match state.to_affine() {
-        Ok(state) => (state.x(), state.y()),
-        Err(_) => (Felt::ZERO, Felt::ZERO),
-    };
-    EvalAction::NormalBranch(0, smallvec![ec, Value::EcState { x, y }])
+    EvalAction::NormalBranch(
+        0,
+        smallvec![
+            ec,
+            Value::EcState {
+                x: state.x(),
+                y: state.y(),
+                z: state.z(),
+            }
+        ],
+    )
 }
 
 fn eval_state_finalize(
     _registry: &ProgramRegistry<CoreType, CoreLibfunc>,
     _info: &SignatureOnlyConcreteLibfunc,
     args: Vec<Value>,
 ) -> EvalAction {
-    let [Value::EcState { x, y }]: [Value; 1] = args.try_into().unwrap() else {
+    let [Value::EcState { x, y, z }]: [Value; 1] = args.try_into().unwrap() else {
         panic!()
     };
 
-    if x.is_zero() && y.is_zero() {
-        EvalAction::NormalBranch(1, smallvec![])
-    } else {
-        EvalAction::NormalBranch(0, smallvec![Value::EcPoint { x, y }])
+    // The single normalisation of the pipeline. `to_affine` fails exactly when
+    // `z == 0`, so it doubles as the point-at-infinity test.
+    match ProjectivePoint::new_unchecked(x, y, z).to_affine() {
+        Ok(point) => EvalAction::NormalBranch(
+            0,
+            smallvec![Value::EcPoint {
+                x: point.x(),
+                y: point.y(),
+            }],
+        ),
+        Err(_) => EvalAction::NormalBranch(1, smallvec![]),
     }
 }
 

debug_utils/sierra-emu/tests/common/mod.rs

@@ -29,9 +29,13 @@ pub fn value_to_felt(value: &Value) -> Vec<Felt> {
             felts.push(modulus.into());
             felts
         }
-        Value::EcPoint { x, y } | Value::EcState { x, y } => {
+        Value::EcPoint { x, y } => {
             vec![*x, *y]
         }
+        // Projective: three felts, unlike `EcPoint`.
+        Value::EcState { x, y, z } => {
+            vec![*x, *y, *z]
+        }
         Value::Enum {
             self_ty,
             index,

docs/debugging.md

@@ -408,7 +408,8 @@ Once you have generated the traces for both the Sierra emulator and Cairo Native
     ```
 2. Look for the first significant difference between the traces. Not all the differences are significant, for example:
     1. Sometimes the emulator and Cairo Native differ in the Gas builtin. It usually doesn’t affect the outcome of the contract.
-    2. The ec_state_init libfunc randomizes an elliptic curve point, which is why they always differ.
+    2. An `EcState` is projective; `[X : Y : Z]` and `[λX : λY : λZ]` are the same point — compare
+       `X/Z`, `Y/Z` before concluding a real difference.
 3. Find the index of the statement executed immediately previous to the first difference.
 4. Open `traces/prog_0.sierra` and look for that statement.
     1. If it’s a return, then you are dealing with a control flow bug. These are difficult to debug.

src/arch.rs

@@ -113,8 +113,10 @@ impl AbiArgument for ValueWithInfoWrapper<'_> {
                 y.to_bytes(buffer)?;
             }
             (Value::EcState(x, y), CoreTypeConcrete::EcState(_)) => {
-                x.to_bytes(buffer)?;
-                y.to_bytes(buffer)?;
+                // Native representation is projective; see `crate::types::ec_state`.
+                for felt in crate::types::ec_state::to_projective(*x, *y) {
+                    felt.to_bytes(buffer)?;
+                }
             }
             (Value::QM31(a, b, c, d), CoreTypeConcrete::QM31(_)) => {
                 a.to_bytes(buffer)?;

src/libfuncs/ec.rs

@@ -6,6 +6,7 @@ use crate::{
     execution_result::EC_OP_BUILTIN_SIZE,
     libfuncs::increment_builtin_counter_conditionally_by,
     metadata::{runtime_bindings::RuntimeBindingsMeta, MetadataStorage},
+    types::{ec_point::ec_point_ty, ec_state::ec_state_ty},
     utils::{get_integer_layout, ProgramRegistryExt, PRIME},
 };
 use cairo_lang_sierra::{
@@ -178,14 +179,7 @@ pub fn build_point_from_x<'ctx, 'this>(
     metadata: &mut MetadataStorage,
     _info: &SignatureOnlyConcreteLibfunc,
 ) -> Result<()> {
-    let ec_point_ty = llvm::r#type::r#struct(
-        context,
-        &[
-            IntegerType::new(context, 252).into(),
-            IntegerType::new(context, 252).into(),
-        ],
-        false,
-    );
+    let ec_point_ty = ec_point_ty(context);
 
     let point_ptr = helper.init_block().alloca1(
         context,
@@ -240,14 +234,7 @@ pub fn build_state_add<'ctx, 'this>(
     metadata: &mut MetadataStorage,
     _info: &SignatureOnlyConcreteLibfunc,
 ) -> Result<()> {
-    let ec_state_ty = llvm::r#type::r#struct(
-        context,
-        &[
-            IntegerType::new(context, 252).into(),
-            IntegerType::new(context, 252).into(),
-        ],
-        false,
-    );
+    let ec_state_ty = ec_state_ty(context);
 
     let state_ptr = helper.init_block().alloca1(
         context,
@@ -258,7 +245,7 @@ pub fn build_state_add<'ctx, 'this>(
     let point_ptr = helper.init_block().alloca1(
         context,
         location,
-        ec_state_ty,
+        ec_point_ty(context),
         get_integer_layout(252).align(),
     )?;
 
@@ -296,8 +283,8 @@ pub fn build_state_add_mul<'ctx, 'this>(
     )?;
 
     let felt252_ty = IntegerType::new(context, 252).into();
-    let ec_state_ty = llvm::r#type::r#struct(context, &[felt252_ty, felt252_ty], false);
-    let ec_point_ty = llvm::r#type::r#struct(context, &[felt252_ty, felt252_ty], false);
+    let ec_state_ty = ec_state_ty(context);
+    let ec_point_ty = ec_point_ty(context);
 
     let state_ptr = helper.init_block().alloca1(
         context,
@@ -344,9 +331,8 @@ pub fn build_state_finalize<'ctx, 'this>(
     metadata: &mut MetadataStorage,
     _info: &SignatureOnlyConcreteLibfunc,
 ) -> Result<()> {
-    let felt252_ty = IntegerType::new(context, 252).into();
-    let ec_state_ty = llvm::r#type::r#struct(context, &[felt252_ty, felt252_ty], false);
-    let ec_point_ty = llvm::r#type::r#struct(context, &[felt252_ty, felt252_ty], false);
+    let ec_state_ty = ec_state_ty(context);
+    let ec_point_ty = ec_point_ty(context);
 
     let point_ptr = helper.init_block().alloca1(
         context,
@@ -385,13 +371,17 @@ pub fn build_state_init<'ctx, 'this>(
     _metadata: &mut MetadataStorage,
     _info: &SignatureOnlyConcreteLibfunc,
 ) -> Result<()> {
-    let felt252_ty = IntegerType::new(context, 252).into();
-    let ec_state_ty = llvm::r#type::r#struct(context, &[felt252_ty, felt252_ty], false);
+    let ec_state_ty = ec_state_ty(context);
 
+    // The canonical projective identity `[0 : 1 : 0]`. Any `Z == 0` is the point
+    // at infinity, but emitting the canonical form keeps memory dumps readable
+    // and matches `ProjectivePoint::identity`.
     let k0 = entry.const_int(context, location, 0, 252)?;
+    let k1 = entry.const_int(context, location, 1, 252)?;
     let state = entry.append_op_result(llvm::undef(ec_state_ty, location))?;
     let state = entry.insert_value(context, location, state, k0, 0)?;
-    let state = entry.insert_value(context, location, state, k0, 1)?;
+    let state = entry.insert_value(context, location, state, k1, 1)?;
+    let state = entry.insert_value(context, location, state, k0, 2)?;
 
     helper.br(entry, 0, &[state], location)
 }
@@ -406,14 +396,7 @@ pub fn build_try_new<'ctx, 'this>(
     metadata: &mut MetadataStorage,
     _info: &SignatureOnlyConcreteLibfunc,
 ) -> Result<()> {
-    let ec_point_ty = llvm::r#type::r#struct(
-        context,
-        &[
-            IntegerType::new(context, 252).into(),
-            IntegerType::new(context, 252).into(),
-        ],
-        false,
-    );
+    let ec_point_ty = ec_point_ty(context);
 
     let point_ptr = helper.init_block().alloca1(
         context,

src/metadata/trace_dump.rs

@@ -229,7 +229,7 @@ pub mod trace_dump_runtime {
     use crate::{
         starknet::ArrayAbi,
         types::TypeBuilder,
-        utils::{get_integer_layout, layout_repeat},
+        utils::{felt_from_slot, get_integer_layout, layout_repeat},
     };
 
     use crate::runtime::FeltDict;
@@ -296,6 +296,14 @@ pub mod trace_dump_runtime {
             .push(StateDump::new(StatementIdx(statement_idx as usize), items));
     }
 
+    /// Read `N` consecutive `felt252` slots starting at `ptr`.
+    ///
+    /// Each slot is 32 bytes: `get_integer_layout(252)` is size 32, align 16, so
+    /// repeating it gives a stride of 32.
+    unsafe fn read_felts<const N: usize>(ptr: NonNull<()>) -> [Felt; N] {
+        std::array::from_fn(|i| felt_from_slot(ptr.byte_add(i * 32).cast().as_ref()))
+    }
+
     /// TODO: Can we reuse `cairo_native::Value::from_ptr`?
     unsafe fn value_from_ptr(
         registry: &ProgramRegistry<CoreType, CoreLibfunc>,
@@ -309,7 +317,8 @@ pub mod trace_dump_runtime {
             | CoreTypeConcrete::Starknet(StarknetTypeConcrete::ClassHash(_))
             | CoreTypeConcrete::Starknet(StarknetTypeConcrete::StorageAddress(_))
             | CoreTypeConcrete::Starknet(StarknetTypeConcrete::StorageBaseAddress(_)) => {
-                Value::Felt(Felt::from_bytes_le(value_ptr.cast().as_ref()))
+                let [value] = read_felts(value_ptr);
+                Value::Felt(value)
             }
             CoreTypeConcrete::Uint8(_) => Value::U8(value_ptr.cast().read()),
             CoreTypeConcrete::Uint16(_) => Value::U16(value_ptr.cast().read()),
@@ -338,42 +347,15 @@ pub mod trace_dump_runtime {
             }
 
             CoreTypeConcrete::EcPoint(_) => {
-                let layout = Layout::new::<()>();
-                let (x, layout) = {
-                    let (layout, offset) = layout.extend(Layout::new::<[u128; 2]>()).unwrap();
-                    (
-                        Felt::from_bytes_le(value_ptr.byte_add(offset).cast().as_ref()),
-                        layout,
-                    )
-                };
-                let (y, _) = {
-                    let (layout, offset) = layout.extend(Layout::new::<[u128; 2]>()).unwrap();
-                    (
-                        Felt::from_bytes_le(value_ptr.byte_add(offset).cast().as_ref()),
-                        layout,
-                    )
-                };
-
+                let [x, y] = read_felts(value_ptr);
                 Value::EcPoint { x, y }
             }
             CoreTypeConcrete::EcState(_) => {
-                let layout = Layout::new::<()>();
-                let (x, layout) = {
-                    let (layout, offset) = layout.extend(Layout::new::<[u128; 2]>()).unwrap();
-                    (
-                        Felt::from_bytes_le(value_ptr.byte_add(offset).cast().as_ref()),
-                        layout,
-                    )
-                };
-                let (y, _) = {
-                    let (layout, offset) = layout.extend(Layout::new::<[u128; 2]>()).unwrap();
-                    (
-                        Felt::from_bytes_le(value_ptr.byte_add(offset).cast().as_ref()),
-                        layout,
-                    )
-                };
-
-                Value::EcState { x, y }
+                // Projective `[x : y : z]`, matching the native representation;
+                // `sierra_emu::Value::EcState` is projective too, so no
+                // normalisation is needed for the traces to compare.
+                let [x, y, z] = read_felts(value_ptr);
+                Value::EcState { x, y, z }
             }
 
             CoreTypeConcrete::Uninitialized(info) => Value::Uninitialized {