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

Generic EC #186

Open
wants to merge 80 commits into
base: master
Choose a base branch
from
Open

Generic EC #186

wants to merge 80 commits into from

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented Feb 3, 2025

Make synderion generic over the elliptic curve type.

Very noisy PR, but the gist of it is that we make the SchemeParams trait agnostic over the elliptic curve used and thread that change across the code base while removing all explicit dependencies on k256.
Introduces a new generic parameter P: SchemeParams in many places, hence most of the noise.

The main areas to consider during review are:

  • Deserialization bounds. I have to admit that I often have very little clue as to why certain #[serde(bound(deserialize(STUFF))] are necessary or even why they work beyond "serde magic". Not great.
  • bip32 related functionality. BIP32 is only really well-defined for secp256k1 and while the code in this PR works for other curves as well, nothing but secp256k1 should ever be used in production without careful analysis.
  • The added HashOutput associated type on SchemeParams should be considered temporary. The goal is to remove the need for it and then we can remove it entirely.
  • The added WideCurveUint associated type on SchemeParams is there to allow reduced bias when generating scalars from extendable hashes. It's not great to have to do this, but I have found no better way. :/
  • There is an unfortunate delay in releasing updates to the RustCrypto stack following the release of crypto-bigint 0.6, which means that in this PR we're juggling two different versions of the bigint library. Hopefully we can get rid of this Real Soon™.

One outstanding question not addressed here is how to tweak the TestParams for optimal speed. For TinyCurve64 these settings seem to work:

-    const L_BOUND: u32 = 256;
-    const LP_BOUND: u32 = 256;
-    const EPS_BOUND: u32 = 320;
+    const L_BOUND: u32 = 64;
+    const LP_BOUND: u32 = 320;
+    const EPS_BOUND: u32 = 128;

…along with a PRIME_BITS setting of 256.

Closes #27.

@fjarri
Copy link
Member

fjarri commented Feb 21, 2025

One outstanding question not addressed here is how to tweak the TestParams for optimal speed. For TinyCurve64 these settings seem to work:

Yes, that looks correct, I added a comment in params.rs before reading the PR description :)
You can check the parameter validity by adding the test for are_self_consistent(), for them.

Also, there are three remaining comments mentioning #27 (in curve.rs, sigma/fac.rs, and paillier/encryption.rs) which can be now removed/amended.

@dvdplm dvdplm requested a review from fjarri March 3, 2025 13:01
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.

Generalize the library to use any curve
2 participants