fix: guard point-at-infinity in projective→affine conversion - #17
Merged
Conversation
jacobian_t::affine_t::operator= and xyzz_t::affine_t::operator= computed the affine coordinates by dividing by Z (resp. ZZZ) unconditionally. For a point at infinity (Z==0 / ZZZ==0) that is a field division by zero. Guard the zero-denominator case and return the affine infinity encoding (0,0) that the matching is_inf() already recognizes, instead of dividing. The nonzero path is unchanged (the guard is false for every affine point with a nonzero denominator), so this is behavior-preserving for all finite points.
xyzz_t defines conversion operators returning jacobian_t<field_t> but neither
included nor forward-declared jacobian_t, so including xyzz_t.hpp on its own
failed to compile ("jacobian_t is not a template"). Add a forward declaration
of the jacobian_t class template. A declaration suffices: the conversion-
operator bodies that construct a jacobian_t are only instantiated when used,
and every current consumer includes jacobian_t.hpp first.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Last reviewed head: |
Advances the openvm deps from the temporary chore/stark-backend-main-track branch (97f24c2) to openvm main (9e523422), and the openvm-org/stark-backend git deps to main (cf013ae8) — pulling in the mont_t inverse carry fix (#393) that the BN254 field path relies on. openvm main already tracks stark-backend main and drops the removed INTERACTION_MEMORY_OVERHEAD symbol (#385), so the two openvm-org deps unify consistently. openvm's Sdk::verify_evm_halo2_proof/prove_for_evm now returns a fallible EvmProof, so the benchmark propagates it with ?. Removes the now-obsolete main-track workaround comment (its stated condition 'once the openvm change lands' is met).
gaxiom
force-pushed
the
grigorii/int-7614-sppark-follow-ups
branch
from
July 31, 2026 20:19
7490204 to
4cd11ed
Compare
primitives
|
stephenh-axiom-xyz
approved these changes
Aug 3, 2026
stephenh-axiom-xyz
left a comment
stephenh-axiom-xyz
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Noting that this PR doesn't resolve the following finding, but I don't think we need to fix it anyways
## halo2-gpu (halo2_proofs/cuda/include/field/alt_bn128.hpp shim + cuda/blst/*)
- The shim still embeds host-side blst typedefs because every kernel header uses typedef fr_t scalar_t; outside __CUDA_ARCH__. Replace the host path with an opaque placeholder type and drop cuda/blst/*.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes INT-7614
Two latent, sppark-inherited defects in the CUDA curve headers, fixed with the smallest correct change :