blockifier: add secp256r1 syscall validation

[Yoni-Starkware]

Summary

• Add input validation for secp256r1 syscall points
• Add a test

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Changes Starknet execution semantics for secp256r1 syscalls; contracts that previously accepted zero-x points will now fail, though behavior is scoped to r1 and covered by broad tests.

Overview
secp256r1 syscalls now reject non-infinity points with x-coordinate 0 (hard InvalidSyscallInput, not a revert). secp256k1 is unchanged via a SecpZeroXPolicy flag.

Shared checks live in secp::reject_zero_x_point on outputs from new, get_point_from_x, and point allocation. add and mul also run secp_shadow, which mirrors the Cairo ec.cairo control flow so operands and intermediate values during scalar multiplication are validated the same way as in the VM—even when the final result has x ≠ 0.

The same rules apply on both the Cairo VM syscall path and Cairo Native handlers. New unit and integration tests cover VM, Native, and a direct mul case for intermediate zero-x rejection.

^{Reviewed by Cursor Bugbot for commit bd6251f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2026-06-09T16:34:44Z
• Started at 2026-06-09T17:05:06Z
• Started at 2026-06-10T05:57:04Z
• Started at 2026-06-10T06:16:34Z
• Started at 2026-06-10T08:23:07Z
• Started at 2026-06-10T10:23:47Z
• Started at 2026-06-10T10:36:02Z
• Started at 2026-06-10T11:03:29Z
• Started at 2026-06-10T11:31:02Z

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit cab984e. Configure here.}

[Yoni-Starkware]

Added an input-validation test for each secp256r1 point-producing syscall (new, get_point_from_x, add, mul), under both the VM and Cairo Native paths.

[ilyalesokhin-starkware]

@ilyalesokhin-starkware reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on liorgold2).

[liorgold2]

@liorgold2 reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: 2 of 7 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 45 at r4 (raw file):

    if is_zero_x_point(point) {
        // Re-derive the affine point only on the (rare) rejection path, reusing the shared error.
        return reject_zero_x_point(&Affine::from(*point));

Why call this function that also has an if inside it.

[liorgold2]

@liorgold2 made 1 comment.
Reviewable status: 1 of 7 files reviewed, 2 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 63 at r4 (raw file):

/// Mirror of `fast_ec_add`: the cairo routine checks both operands for the point at infinity (which
/// it identifies by `x == 0`), so reject a non-infinity `x == 0` operand of either.
fn fast_ec_add<Curve: SWCurveConfig + SecpZeroXPolicy>(

Identical to ec_add, so I'd remove this function, and write that it models both.

[liorgold2]

@liorgold2 made 1 comment.
Reviewable status: 1 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 139 at r5 (raw file):

    let mut res = table[nibbles[63]];
    // First inner call (cairo m = 124): nibbles 62..=32, most significant first.
    res = fast_ec_mul_inner(&table, res, (32..=62).rev().map(|k| nibbles[k]))?;

rev here was confusing...

Code quote:

 (32..=62).rev().map(|k| nibbles[k])

[liorgold2]

@liorgold2 made 1 comment and resolved 1 discussion.
Reviewable status: 1 of 7 files reviewed, 2 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 139 at r5 (raw file):

Previously, liorgold2 wrote…

rev here was confusing...

Never mind.

[liorgold2]

@liorgold2 made 1 comment.
Reviewable status: 1 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 163 at r5 (raw file):

    // Only the rejection matters; the computed point is discarded.
    ec_add(Projective::from(*point0), Projective::from(*point1)).map(|_| ())
}

Not sure why this function is here.

Code quote:

/// Rejects a `secp256r1_add(point0, point1)` syscall that involves a point with x-coordinate 0.
/// No-op for curves without [`SecpZeroXPolicy::REJECT_ZERO_X_POINT`].
pub fn reject_zero_x_in_add<Curve: SWCurveConfig + SecpZeroXPolicy>(
    point0: &Affine<Curve>,
    point1: &Affine<Curve>,
) -> ShadowResult<()>
where
    Curve::BaseField: PrimeField,
{
    if !Curve::REJECT_ZERO_X_POINT {
        return Ok(());
    }
    // Only the rejection matters; the computed point is discarded.
    ec_add(Projective::from(*point0), Projective::from(*point1)).map(|_| ())
}

[liorgold2]

@liorgold2 resolved 1 discussion.
Reviewable status: 1 of 7 files reviewed, 2 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

[liorgold2]

@liorgold2 made 1 comment.
Reviewable status: 1 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 177 at r5 (raw file):

        return Ok(());
    }
    // Only the rejection matters; the computed point is discarded.

Can you test locally (without changing the PR) that we get the expected result?

Code quote:

    // Only the rejection matters; the computed point is discarded.

[Yoni-Starkware]

@Yoni-Starkware made 2 comments.
Reviewable status: 1 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and liorgold2).

---

crates/blockifier/src/execution/secp_shadow.rs line 45 at r4 (raw file):

Previously, liorgold2 wrote…

Why call this function that also has an if inside it.

Done

---

crates/blockifier/src/execution/secp_shadow.rs line 63 at r4 (raw file):

Previously, liorgold2 wrote…

Identical to ec_add, so I'd remove this function, and write that it models both.

Done.

[liorgold2]

@liorgold2 partially reviewed 1 file and made 1 comment.
Reviewable status: 1 of 7 files reviewed, 4 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 178 at r5 (raw file):

    }
    // Only the rejection matters; the computed point is discarded.
    ec_mul_by_uint256(point, scalar).map(|_| ())

Let's make sure the result is not the x=0 point as well

[liorgold2]

@liorgold2 reviewed 1 file and resolved 2 discussions.
Reviewable status: 2 of 7 files reviewed, 2 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

[liorgold2]

@liorgold2 reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: 3 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/syscalls/secp.rs line 73 at r6 (raw file):

    }

    pub fn secp_get_point_from_x(

Let's block it here and in secp_new as well.

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: 3 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and liorgold2).

---

crates/blockifier/src/execution/secp_shadow.rs line 178 at r5 (raw file):

Previously, liorgold2 wrote…

Let's make sure the result is not the x=0 point as well

The result is checked by the original fix - we check every allocated point

    fn allocate_point(
        &mut self,
        ec_point: short_weierstrass::Affine<Curve>,
        vm: &mut VirtualMachine,
        points_segment_base: &mut Option<Relocatable>,
        id: usize,
    ) -> SyscallBaseResult<Relocatable> {
        reject_zero_x_point(&ec_point)?;

Should I add it anyway?

[liorgold2]

@liorgold2 made 1 comment.
Reviewable status: 3 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

---

crates/blockifier/src/execution/syscalls/secp.rs line 73 at r6 (raw file):

Previously, liorgold2 wrote…

Let's block it here and in secp_new as well.

Actually, it's covered in allocate_point, right?

[liorgold2]

@liorgold2 reviewed 1 file.
Reviewable status: 4 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and Yoni-Starkware).

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: 4 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and liorgold2).

---

crates/blockifier/src/execution/secp_shadow.rs line 177 at r5 (raw file):

Previously, liorgold2 wrote…

Can you test locally (without changing the PR) that we get the expected result?

Claude ran the following successfully

    #[test]
    fn temp_shadow_mul_result_matches_reference() {
        use ark_ec::short_weierstrass::Projective;
        use ark_ec::CurveGroup;
        use ark_ff::PrimeField;

        let g = generator();
        let g_proj = Projg>::from(g);
        let scalars = [
            BigUint::from
            BigUint::from(2u32),
            BigUint::from
            BigUint::from(15u32),
            BigUint::from
            BigUint::from(255u32),
            BigUint::from
            BigUint::from(u64::MAX),
            (BigUint::fro
        ];
        for k in &scalars
            let shadow = super::ec_mul_by_uint256(&g, k).unwrap();
            let reference
                g_proj * ark_secp256r1::Fr::from_le_bytes_mod_order(&k.to_bytes_le
());
            assert_eq!(shadow.into_affine(), reference.into_affine(), "k={k}");
        }
        println!("OK: shadow mul matches reference for {} scalars", scalars.len())
;
    }

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: 4 of 7 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware and liorgold2).

---

crates/blockifier/src/execution/syscalls/secp.rs line 73 at r6 (raw file):

Previously, liorgold2 wrote…

Actually, it's covered in allocate_point, right?

Yes. EVERY syscall result goes through allocate_point

[liorgold2]

@liorgold2 made 1 comment and resolved 2 discussions.
Reviewable status: 4 of 7 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).

---

crates/blockifier/src/execution/secp_shadow.rs line 178 at r5 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

The result is checked by the original fix - we check every allocated point

    fn allocate_point(
        &mut self,
        ec_point: short_weierstrass::Affine<Curve>,
        vm: &mut VirtualMachine,
        points_segment_base: &mut Option<Relocatable>,
        id: usize,
    ) -> SyscallBaseResult<Relocatable> {
        reject_zero_x_point(&ec_point)?;

Should I add it anyway?

No. Resolved.

[liorgold2]

@liorgold2 resolved 1 discussion.
Reviewable status: 4 of 7 files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

[liorgold2]

@liorgold2 reviewed 1 file.
Reviewable status: 5 of 7 files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

[liorgold2]

(didn't review native fixes and tests)

@liorgold2 made 1 comment.
Reviewable status: 5 of 7 files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).