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

[crypto]: Host function for verifying secp256r1 signatures #113

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RomarQ
Copy link

@RomarQ RomarQ commented Aug 16, 2024

No description provided.

@RomarQ RomarQ changed the title secp256r1 host function [crypto]: host function for verifying secp256r1 signatures Aug 16, 2024
@RomarQ RomarQ changed the title [crypto]: host function for verifying secp256r1 signatures [crypto]: Host function for verifying secp256r1 signatures Aug 16, 2024
@burdges
Copy link

burdges commented Aug 16, 2024

As a rule, all new elliptic curves should use our new versatile crypto framework:

We'd ideally audit that framework too before deploying it, because I've made one or two "unique" choices there, like host calls return points in projective coordinates. It'd be nice if the auditor could discuss more generic framework ideas too, like providing the curve parameters.

In theorey, it'd be cool if we supported the rust crypto traits and provided a drop in for p256, but rust crypto traits are a bloody complex mess, so maybe arkworks provides a cleaner trait framework.

As for doing a simple hostcall like this, we'd ideally want strong evidence the curve would never be used other than via the simple hostcall, but obviously P256 should apear in zk glue protocols, hence its existance in https://github.com/arkworks-rs/algebra/tree/master/curves/secp256r1

Anyways we should definitely att P256 to https://github.com/paritytech/arkworks-extensions and the curve hostcalls, and update everything to more recent arkwroks.

@RomarQ
Copy link
Author

RomarQ commented Aug 16, 2024

As a rule, all new elliptic curves should use our new versatile crypto framework:

We'd ideally audit that framework too before deploying it, because I've made one or two "unique" choices there, like host calls return points in projective coordinates. It'd be nice if the auditor could discuss more generic framework ideas too, like providing the curve parameters.

In theorey, it'd be cool if we supported the rust crypto traits and provided a drop in for p256, but rust crypto traits are a bloody complex mess, so maybe arkworks provides a cleaner trait framework.

As for doing a simple hostcall like this, we'd ideally want strong evidence the curve would never be used other than via the simple hostcall, but obviously P256 should apear in zk glue protocols, hence its existance in https://github.com/arkworks-rs/algebra/tree/master/curves/secp256r1

Anyways we should definitely att P256 to https://github.com/paritytech/arkworks-extensions and the curve hostcalls, and update everything to more recent arkwroks.

Thank you for the comments.

Substrate already uses k256 for the secp256k1 host functions, p256 seems like a good candidate implementation since it comes from the same repository and has been partially audited.

The initial idea was to add a new function secp256r1_ecdsa_verify_prehashed(sig, hash, pk) to substrate/primitives/io/src/lib.rs(Crypto) and use p256 to verify the signature.

If paritytech/arkworks-extensions is now the correct way for adding new elliptic curve host functions, it means that we are now enforced to use curves from https://github.com/arkworks-rs/algebra? Also, can you confirm if paritytech/arkworks-extensions host functions are available in Polkadot/Kusama?

@burdges
Copy link

burdges commented Aug 16, 2024

If paritytech/arkworks-extensions is now the correct way for adding new elliptic curve host functions, it means that we are now enforced to use curves from https://github.com/arkworks-rs/algebra? Also

Not really. In principle any elliptic curve crate like k256 or p256 could be forked to use the scalar multiplications provided by the host call, with any other crate that implements the same curve living behind the host call.

Arkworks does not have optimal implementations of anything, so ideally most arkworks curve crates should've forks that call better crates too.

Also, can you confirm if paritytech/arkworks-extensions host functions are available in Polkadot/Kusama?

Not yet, but they're part of sassafras & jam. We can push them through faster too, but..

We make the hostcalls using uncompressed affine coordinates. I'd like to have some second opinions on how I've designed this. It's not absolutely critical, since you can always convert to/from cordinates, but someone who knows how all the different SW curve implementations work could be informative, and if osme conversion uses a division then best avoided.

About your applications:

Apple's Secure Enclave could likely use the hostcall you suggest here, and ECDSA lacks nice batch verification, so that's a clear fit for exactly this hostcall, provided it does not break anonymity.

Webauthn and Passkeys would likely add anonymity like keyless accounts, so then ideally you'd have some plonk with folking using P256 directly in places. This really need like all of arkworks.

Android Keystore could go either way I guess.

Or wait, does each enclave have a different secret key? If so, then you might need the keyless accounts snark tricks for everything.

@burdges
Copy link

burdges commented Aug 16, 2024

Anyways someone should do a PR to paritytech/arkworks-extensions and substrate that uses the arkworks secp256r1. We'll deploy secp256r1 when we deploy all the others, which must happen anyways.

@0xSalman
Copy link

0xSalman commented Aug 16, 2024

In principle any elliptic curve crate like k256 or p256 could be forked to use the scalar multiplications provided by the host call, with any other crate that implements the same curve living behind the host call.

I maybe misunderstanding you but if this is true, can't we open a PR to polkadot-sdk/substrate to add a host call that uses p256 crate?

Anyways someone should do a PR to paritytech/arkworks-extensions and substrate that uses the arkworks secp256r1. We'll deploy secp256r1 when we deploy all the others, which must happen anyways.

Just to confirm, are you suggesting that we can open PRs to paritytech/arkworks-extensions and polkadot-sdk/substrate that use arkworks' secp256r1 impl without modifying and improving it as you suggested earlier?

@davxy
Copy link
Contributor

davxy commented Aug 16, 2024

I agree with @burdges on this point. I believe that we should work on an RFC to promote the inclusion of the host functions for arkworks (def here) on kusama/polkadot.

These will not only be necessary if some parachain adopts Sassafras in the future, but they will also unlock a whole new class of arbitrary zk applications for the Polkadot ecosystem.

Currently, the secp256r1 hooks are still missing, by the way.

@burdges
Copy link

burdges commented Aug 17, 2024

At a high level, we want rust cryptographic code from the wider rust ecosystem to work cleanly inside runtimes. This is far nicer for developers than blockchains like ETH which require reimplementing everything. It's also defends our ecosystem from compeating ones like cosmos who lack on-chain upgrades and thus benefit from native code directly.

In elliptic curve, our big performance hits come from scalar multiplications and multi-scalar multiplication, and also miller loops and final exponentiations for pairing curves. We propose stable host calls for only those operations, along with facade crates that provide drop in replacements for elliptic curve crates.

I maybe misunderstanding you but if this is true, can't we open a PR to polkadot-sdk/substrate to add a host call that uses p256 crate?

We ideally want one set of sec256r1/p256 host calls: one multi scalar multiplication, one projective scalar multiplication, and optionally one affine->projective scalar multiplication if performacne differs much from the projective one.

Ideally, this one set of host calls should permit both a sub-ark-secp256r1 drop in replacement for ark-secp256r1 and a sub-p256 drop in repalcement for p256. Also ideally they'd be faster than arkworks onces.

Step 0. Descide if affine->projective scalar multiplication is much faster than regular projective scalar multiplication, aka read the p256 benchmarks. This tells us if we want 2 or 3 hostcalls.

Step 1. Write an ark-secp256r1-rustcrypto crate with tests showing equivelence to ark-secp256r1. A priori, this requires taking inspiration from https://github.com/MystenLabs/fastcrypto/tree/main/fastcrypto-zkp/src/bls12381 because there maybe under the hood differences in point representations, but secp256r1 is way more standard than bls12-381 so maybe no differences.

Step 2. Write a sub-ark-secp256r1 PR for paritytech/arkworks-extensions and a PR for substrate hostcalls, and equivelence tests to ark-secp256r1.

Step 3. Write a sub-p256 fork of p256 which passes equivelence tests to p256. This maybe easy, or maybe nasty or impossible, just depends upon their their elliptic curve traits.

We'd push for tressury funding for external people of course.

Just to confirm, are you suggesting that we can open PRs to paritytech/arkworks-extensions and polkadot-sdk/substrate that use arkworks' secp256r1 impl without modifying and improving it as you suggested earlier?

We could skip steps 1 and 3 above if we cared little about the speed difference between rustcrypto and arkworks, but we do afaik care about this speed difference for secp256k1, so may as well do both exactly the same, no?

I'd like someone to attempt step 1 for at least secp256k1, or else show benchmarks that arkworks is faster than I think.

@RomarQ
Copy link
Author

RomarQ commented Aug 20, 2024

@burdges @davxy Thanks for the suggestions 👍

Do you guys have any ETA for when these new curve host functions will be available in Polkadot/Kusama?

@burdges
Copy link

burdges commented Aug 22, 2024

Before JAM obviously. They're probably worth pushing forward faster though.

We're obviously seeing more community interest now, so maybe worth push forward soon. They improve many other priorities too, like keyless accounts, identity, and games.

@piotrmbf
Copy link

Hello, I am Piotr - head of Product at Moonbeam Foundation. This feature would significantly speed up the adoption of Smart Wallets. Do you have any estimates of when this could be incorporated and shipped?

@burdges
Copy link

burdges commented Oct 16, 2024

I've an outline for the work at https://hackmd.io/@rgbPIkIdTwSICPuAq67Jbw/r1dRCb86C but so far I've not pushed anyone towards doing it, just mentioned it exists. We should figure out who wants to do this work.

Audits are cool, but actually my "audit" questions can be handled in simpler ways, so I'll do that this week.

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.

5 participants