Skip to content

test: Felt constructor edge cases#991

Open
mooori wants to merge 1 commit intonextfrom
mooori/test-felt-construction
Open

test: Felt constructor edge cases#991
mooori wants to merge 1 commit intonextfrom
mooori/test-felt-construction

Conversation

@mooori
Copy link
Contributor

@mooori mooori commented Mar 3, 2026

Closes #931

@mooori mooori marked this pull request as ready for review March 3, 2026 09:52
@mooori mooori requested a review from greenhat March 3, 2026 09:52
Copy link
Contributor

@greenhat greenhat left a comment

Choose a reason for hiding this comment

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

Nice! I'm sorry I should have articulated it more, but after our "wrapping nature of the felt" discussion on slack I made the Felt::new on miden target in the miden-crypto to behave like the native felt (wrapping). So in the test we should check that Felt:new compiled to the miden target is semantically equivalent to the one compiled to the default target. However, this will be possible after we migrate to the VM v0.21 with the "unified" Felt from the miden-crypto repo.

@bobbinth
Copy link
Contributor

bobbinth commented Mar 4, 2026

Felt::new on miden target in the miden-crypto to behave like the native felt (wrapping)

After the migration to Plonky3, this is not longer the case - i.e., Felt::new() does not reduce the passed-in value to a canonical value. Though, I think Felt::new(x).to_canonical_u64() would give us the wrapped value pack (I haven't double-checked this though).

@greenhat
Copy link
Contributor

greenhat commented Mar 5, 2026

Felt::new on miden target in the miden-crypto to behave like the native felt (wrapping)

After the migration to Plonky3, this is not longer the case - i.e., Felt::new() does not reduce the passed-in value to a canonical value. Though, I think Felt::new(x).to_canonical_u64() would give us the wrapped value pack (I haven't double-checked this though).

Yes. Thank you! This is what I was trying to say. I meant "non-reduced" instead of wrapped. It becomes wrapped after the "roundtrip". My bad.

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.

test: Felt construction edge cases

3 participants