refactor: crossfield hints in non-native #1539
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the hint-calling convention to introduce a generic, multi-field hint API and migrates all existing hint methods and tests to use it.
- Adds
NewHintGenericandNewVarGenericHintwith aHintContextwrapper. - Updates
UnwrapHint*,NewHint,NewHintWithNativeOutput, andNewHintWithNativeInputto delegate to the new generic mechanism. - Refactors all native and emulated hint implementations and their tests to use the new context-based API.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| std/math/emulated/field_test.go | Removed legacy multi-hint tests, retained only sub-constant test |
| std/math/emulated/field_hint_test.go | Updated unit tests to call generic hints and renamed package for internal access |
| std/math/emulated/field_hint_example_test.go | Added an example demonstrating the new generic hint usage |
| std/math/emulated/field_hint.go | Introduced HintContext, NewHintGeneric, NewVarGenericHint, removed old wrap |
| std/algebra/native/sw_bls12377/hints.go | Migrated halfGCDEisenstein to the new context-based API, removed sign-only hint |
| std/algebra/native/sw_bls12377/g1.go | Switched GLV/fake-GLV hint calls to use NewHintGeneric |
| std/algebra/emulated/sw_emulated/point.go | Updated scalar‐mul GLV hints to use the generic API |
| std/algebra/emulated/sw_emulated/hints.go | Refactored all SW emulated hints to use HintContext |
| std/algebra/emulated/sw_bls12381/hints.go | Refactored BLS12-381 decomposition hints to the new API |
| std/algebra/emulated/sw_bls12381/g2.go | Updated G2 GLV decomposition calls to use NewHintGeneric |
Comments suppressed due to low confidence (5)
std/math/emulated/field_hint_test.go:309
- [nitpick] There's a typo in the function name:
Emulatedeshould beEmulated. Consider renaming totestGenericHintNativeInEmulatedOutfor clarity.
func testGenericHintNativeInEmulatedeOut[T FieldParams](t *testing.T) {
std/math/emulated/field_hint_test.go:327
- [nitpick] Typo in the test name:
Emulatedeshould beEmulated. Rename toTestGenericHintNativeInEmulatedOutto match the function under test.
func TestGenericHintNativeInEmulatedeOut(t *testing.T) {
std/math/emulated/field_hint_test.go:655
- Consider adding unit tests for
Field.NewHintWithNativeOutputandField.NewHintWithNativeInputto verify those code paths are covered by the test suite.
}
std/math/emulated/field_hint.go:326
- This loop uses
range nbEmulated1Outputs, butnbEmulated1Outputsis an integer, not a slice. It will not compile. Consider iterating over the sliceemulated1Outputs(e.g.,for i := range emulated1Outputs) or usingfor i := 0; i < nbEmulated1Outputs; i++.
for i := range nbEmulated1Outputs {
std/math/emulated/field_hint.go:336
- Similarly,
nbEmulated2Outputsis an integer, sorange nbEmulated2Outputsis invalid. Update this to iterate by index or over theemulated2Outputsslice.
for i := range nbEmulated2Outputs {
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
gbotrel
approved these changes
Jul 15, 2025
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.
Description
Previously we had mechanisms for being able to call hints in non-native gadget where either the inputs xor the outputs are in native field. But this wasn't sufficient for all use cases we had, sometimes we had to be able to provide the inputs from two different fields or we would have outputs in different fields. And the fields could be either native or non-native.
This PR refactors the hint calling convention to be more generic, allowing for all possible use cases. It additionally adds an option to be able to have inputs over multiple emulated fields. As we use type parametrization, then it is currently limited to 2 emulated fields, but the wrapping/unwrapping in principle supports arbitrary number of emulated fields.
I refactored current wrapping/unwrapping to use the new mechanism. All changes are backwards compatible, so any internal/external uses should keep working as is.
This also enables #1303 as now we can also pass in A, B to the scalarmul hint nicely and just do generic SW scalar multiplication.
Finally, it also allowed to change the hints in EC arithmetic where we previously called similar hints multiple times to return the emulated values and signs.
Type of change
How has this been tested?
Checklist:
golangci-lintdoes not output errors locally