Skip to content

Replace "witness only" with "non native felt op" attribute on felt arith ops#288

Merged
tim-hoffman merged 4 commits intomainfrom
th/non_native_felt_attr
Jan 26, 2026
Merged

Replace "witness only" with "non native felt op" attribute on felt arith ops#288
tim-hoffman merged 4 commits intomainfrom
th/non_native_felt_attr

Conversation

@tim-hoffman
Copy link
Member

@tim-hoffman tim-hoffman commented Jan 26, 2026

The WitnessGen trait on felt ops is too restrictive because there are scenarios where there exists no way to represent the code in the @constrain function without them (e.g., array.read a[i % 4] where i is a loop index variable). The NotFieldNative trait allows a less restrictive verification on these ops, allowing them to exist even in the @constrain function until the flattening pass (i.e. struct instantiation and loop unrolling) is later performed. After the flattening pass and constant folding, all such ops should be removed or handled in a manner specific to the backend.

Use this on field ops that may appear in constrain code but must be eventually be rewritten in terms of native fields ops or folded to a constant.
@tim-hoffman tim-hoffman requested a review from a team January 26, 2026 20:37
- "Add `function.allow_non_native_field_ops` attribute"
changed:
- "Require `function.allow_non_native_field_ops` attribute on functions that use
non-native field operations, instead of requiring `function.allow_witness` attribute."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm missing context I think, but does this mean allow_non_native_field_ops subsumes allow_witness everywhere? Or are there still cases where you might want one or the other on @compute but not both?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is more for @constrain, where some ops that are not allowed in the constraint backend (e.g., felt.umod) can be allowed earlier in the pipeline for values that will eventually be flattened into constants (e.g., computing an index i % 4 is allowed in @constrain if i will be known at compile time, but we currently don't have a way to encode that)

Copy link
Member Author

Choose a reason for hiding this comment

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

@raghav198 I added a description on the PR that hopefully explains it fully but let me know if anything remains unclear

Copy link
Contributor

@iangneal iangneal left a comment

Choose a reason for hiding this comment

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

Are we planning to do a different PR to allow @constrain functions to have the new attribute? Otherwise LGTM

@tim-hoffman
Copy link
Member Author

Are we planning to do a different PR to allow @constrain functions to have the new attribute? Otherwise LGTM

Nothing preventing that now. I'll add a test.

@tim-hoffman tim-hoffman merged commit 1f8ee7e into main Jan 26, 2026
9 checks passed
@tim-hoffman tim-hoffman deleted the th/non_native_felt_attr branch January 26, 2026 22:07
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.

3 participants