Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEAT(curve): Add Stark252 curve #421

Closed
wants to merge 10 commits into from

Conversation

PatStiles
Copy link
Contributor

Describe the changes

This PR adds a curve config for the Stark252 curve and operations over the Stark252 field element used in starknet and the stone prover.

@PatStiles PatStiles marked this pull request as draft March 5, 2024 21:43
@ChickenLover ChickenLover self-requested a review March 10, 2024 05:20
@DmytroTym
Copy link
Contributor

Hi @PatStiles
Thanks for the PR!
Just fyi: we're currently working on adding support for STARK fields in general, and I assume Stark252 is a STARK field and not a curve, right? I think it'll be much easier to support this field once all the infrastructure for it is ready, which should be next week (I think).

@PatStiles
Copy link
Contributor Author

PatStiles commented Apr 13, 2024

@DmytroTym Thanks for the feedback. This curve is specific to the curve used for ECDSA in starknet https://docs.starkware.co/starkex/crypto/stark-curve.html I hit a hurdle as arkworks does not currently support the stark252 curve and the tests require the corresponding arkworks curve. This pr is eventually needed to finish icicle integration with lambdaworks ref: lambdaclass/lambdaworks#859

@DmytroTym
Copy link
Contributor

@PatStiles gotcha!
Reg failing Rust tests:

  • I think NTT tests fail because you run them in the wrong field. My understanding is that you want to run them in the 2^251+17⋅2^192+1 field which happens to be the base modulus of the curve. However, you instantiate them for the scalar field of the curve, which only has 2-adicity of 1, so doesn't support any NTTs, which is why arkworks throws an error when trying to get_root_of_unity before all the NTT tests. I think you need to move NTTs to the base field.
  • MSM probably fails because icicle version of the curve differs from arkworks version. Unfortunately icicle doesn't allow for curves with non-zero linear term just yet so instead of 𝑦^2≡𝑥^3+𝛼⋅𝑥+𝛽(mod𝑝) you effectively get 𝑦^2≡𝑥^3+𝛽(mod𝑝) in icicle, so the results of MSM are wrong.

My understanding is that the system you linked works roughly like this: some operations that happen on the curve (like ECDSA, Pedersen commitments etc) get compiled into a STARK circuit whose native field is F_{2^251+17⋅2^192+1}. Then you produce a STARK proof using NTTs and Poseidon hashes (probably without any extension fields). Let me know if I'm missing something but if that's more or less accurate then:

  1. If you want to use icicle for the first part: ECDSA, Pedersen etc. (now or in the future), one thing that we need to add support for is elliptic curves with non-zero linear terms. And also probably ECDSA, Pedersen etc. as primitives.
  2. If you only want icicle for the STARK prover part, I'd suggest that you don't really need the full curve but only the F_{2^251+17⋅2^192+1} field with NTT and Poseidon implemented for it. I'd suggest that you implement it on top of https://github.com/ingonyama-zk/icicle/tree/V2 branch similar to how we implemented baby bear there (one difference is that you probably don't need the extension field). I think your current code is good, just need to move things around and probably delete curve-related functionality. Btw your solution with implementing stark252 for arkworks from scratch is probably better than our ad-hoc tests for baby bear. V2 should be released really soon too so you won't have to wait for a long time. If this approach fits, please let me know if there are any issues or questions along the way.

@PatStiles
Copy link
Contributor Author

@DmytroTym Thank you for the helpful suggestions will tackle this this weekend!

@PatStiles
Copy link
Contributor Author

Closing in favor of #493 which implements the Stark252 base field on top of icicle V2

@PatStiles PatStiles closed this Apr 22, 2024
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.

2 participants