fix(bn254): drop subgroup check from V0 G2 addition - #1764
Open
valentynkit wants to merge 2 commits into
Open
Conversation
V0 G2 addition accepts any on-curve point; the subgroup check belongs to multiplication and pairing only. Sig decoded both add operands through the subgroup-checking G2.fromBytes, so it rejected on-curve non-subgroup points that Agave and Firedancer accept. That is a consensus divergence in the ALT_BN128_G2_ADD syscall. Thread a Validate flag through the G2 decode: curve-only for add, curve and subgroup for mul and pairing.
isWellFormed took a validate flag, which made the name mean two things at once. Split it into isOnCurve and isInSubgroup composed in fromBytes, so each check stays honest. Add commit-pinned Agave and Firedancer permalinks for the curve-only add rule, and note the regression point is the same one test "edge cases" already rejects.
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
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.
Intent
Fixes #1713.
ALT_BN128_G2_ADDmust accept any on-curve G2 point; the prime-order subgroup check is not part of V0 addition. Sig decodes both add operands through the subgroup-checkingG2.fromBytes, so an on-curve point outside the order-r subgroup returnsr0 = 1(error) on Sig while Agave and Firedancer returnr0 = 0with the point. A program can branch onr0or store the output, so the same syscall forks execution and committed account state.enable_alt_bn128_g2_syscalls(SIMD-0302) is active on all clusters, so this is reachable on mainnet today.Implementation
isWellFormedbecomesisOnCurve(curve equation) andisInSubgroup(order-r membership), composed infromBytesbehind aValidateflag.addSyscallpasses.curve_only;mulSyscallandpairingSyscallpass.curve_and_subgroupand keep their current behavior.into_affine_unchecked(curve-only) vs the subgroup-checkingTryFrom<PodG2>, and Firedancerg2_frombytes_check_eq_onlyvs_check_subgroup.G2.fromBytessince vm: implement LE & G2 alt_bn128 #1226.Ramifications
Tests
test "G2 addition accepts on-curve non-subgroup point":addSyscall(P, infinity)returnsP, whilemulSyscall(P, 1)still errorsNotWellFormed.Pis the on-curve non-subgroup point thattest "edge cases"already rejects viapairingSyscall.add/mul/pairing/edge casesstay green.