Skip to content

Add NonIdentityPoint::new_from_constant#55

Merged
ConstanceBeguier merged 3 commits into
zsa1from
non_identity_constant_point
May 4, 2026
Merged

Add NonIdentityPoint::new_from_constant#55
ConstanceBeguier merged 3 commits into
zsa1from
non_identity_constant_point

Conversation

@ConstanceBeguier
Copy link
Copy Markdown

@ConstanceBeguier ConstanceBeguier commented Apr 30, 2026

This PR adds NonIdentityPoint::new_from_constant to enable creating non-identity points that are properly pinned to constants, which is required by the Orchard ZSA circuit (Orchard PR).

More precisely, in the Orchard ZSA circuit, q_init_zec / q_init_zsa must be constrained to fixed constants, as they define the initial point Q of the Sinsemilla hash. If constructed via NonIdentityPoint::new, they remain unconstrained witnesses, allowing a prover to inject an arbitrary on-curve point and break commitment soundness.

Copy link
Copy Markdown

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

added recommendation for better docs.

Please update PR description on why exactly this is needed + link to orchard/zsa1 commit where it is used

Comment thread halo2_gadgets/src/ecc.rs Outdated
point.map(|inner| NonIdentityPoint { chip, inner })
}

/// Constructs a new point with the given value.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/// Witnesses the given constant point with both coordinates pinned via
/// fixed columns.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread halo2_gadgets/src/ecc.rs Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/// Witnesses the given point with only on-curve / non-identity constraints.
/// For known-constant points use [`NonIdentityPoint::new_from_constant`];

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread halo2_gadgets/src/ecc.rs Outdated
Comment on lines +80 to +82
/// Witnesses the given constant point as a private input to the circuit.
/// This returns an error if the point is the identity.
fn witness_point_non_id_from_constant(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/// Witnesses the given constant point with both coordinates pinned via
/// fixed columns. Returns an error if the point is the identity.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

.map(|(x, y)| NonIdentityEccPoint::from_coordinates_unchecked(x, y))
}

/// Assigns a constant non-identity point.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/// Assigns a constant non-identity point with both coordinates pinned via
/// fixed columns.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

@PaulLaux PaulLaux changed the base branch from main to zsa1 May 1, 2026 11:25
@ConstanceBeguier ConstanceBeguier force-pushed the non_identity_constant_point branch from 8ace9db to 46687fe Compare May 4, 2026 10:49
@ConstanceBeguier ConstanceBeguier merged commit bbc42b6 into zsa1 May 4, 2026
24 checks passed
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.

2 participants