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

BLS12-381 signature verification #2106

Merged
merged 80 commits into from
May 21, 2024
Merged

BLS12-381 signature verification #2106

merged 80 commits into from
May 21, 2024

Conversation

aumetra
Copy link
Member

@aumetra aumetra commented Apr 10, 2024

Closes #2094

This PR adds common BLS12-381 operations as VM intrinsics.

@aumetra
Copy link
Member Author

aumetra commented Apr 10, 2024

Oh yeah, MSRV for tests.. I forgot.

@webmaster128 webmaster128 changed the title BLS12-381 operations BLS12-381 signature verification Apr 10, 2024
@aumetra aumetra force-pushed the bls12_381 branch 6 times, most recently from dd6d5ac to 2e1ad7a Compare April 15, 2024 14:41
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Great work. Some first thoughts, not a full review

packages/std/Cargo.toml Outdated Show resolved Hide resolved
packages/crypto/src/bls12_318/pairing.rs Outdated Show resolved Hide resolved
packages/crypto/src/bls12_318/pairing.rs Outdated Show resolved Hide resolved
@aumetra aumetra force-pushed the bls12_381 branch 4 times, most recently from 8501cb6 to 868c49b Compare April 16, 2024 08:55
@aumetra
Copy link
Member Author

aumetra commented Apr 16, 2024

Well, we still need a good metric as how to judge point aggregations.
I'm not quite sure about a good benchmark here to find out how much gas should be consumed.

@webmaster128 any ideas about this?

@webmaster128
Copy link
Member

webmaster128 commented Apr 16, 2024

The point aggregations are a list of sums. Aggregating n points means m = n-1 sums. In Ethereum this is just a constant 500/800 gas for BLS12_G1ADD/BLS12_G2ADD. EDIT: well, this is EIP-2537 – not implemented

As we execute them in parallel, we get machine-specific performance. It would be good to measure using only a small number of cores (let's say 4) and assume production systems usually perform better.

To benchmark this we can calculate m sums with m = 1, 2, 4, 8, 16, 32, 64, 128 with arbitrary points. Drand generates a point in G1 (the signature) for every round, see https://github.com/noislabs/nois-contracts/blob/main/tests/src/bot.ts#L20-L21. Doing the same for drand network ID 8990e7a9aaed2ffed73dbd7092123d6f289930540d7651336225dc172e51b2ce (classic mainnet) gives you points in G2.

@aumetra aumetra force-pushed the bls12_381 branch 2 times, most recently from 15bb41d to 205b8c3 Compare April 17, 2024 14:55
@aumetra aumetra marked this pull request as ready for review April 17, 2024 15:32
@aumetra aumetra force-pushed the bls12_381 branch 3 times, most recently from 2134266 to b77e03c Compare April 22, 2024 11:36
@aumetra
Copy link
Member Author

aumetra commented Apr 22, 2024

So about the mismatch of the benchmarks in CI and the values in the environment: The benches for that were run locally by me.

But it was run as follows:

  • CPU: AMD Ryzen 7 7700X 8-Core Processor at stock clock rates
  • Rayon limited to 4 threads via RAYON_NUM_THREADS

So it wasn't run with the full horse-power of the CPU parallelism, but with the full clock speed. The differences are probably clock speed and parallelism (since IIRC the CircleCI medium class only has two cores instead of 4).

But we can pretty much assume that production environment processors will be at least as fast as the Ryzen with 4 cores utilized IMO.

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Super nice. I did not read it all yet, but some comments for today

packages/crypto/Cargo.toml Show resolved Hide resolved
packages/crypto/src/bls12_318/aggregate.rs Outdated Show resolved Hide resolved
packages/crypto/src/bls12_318/aggregate.rs Show resolved Hide resolved
packages/crypto/src/bls12_318/aggregate.rs Outdated Show resolved Hide resolved
packages/crypto/src/bls12_318/hash.rs Show resolved Hide resolved
packages/crypto/src/bls12_318/hash.rs Show resolved Hide resolved
packages/std/src/traits.rs Show resolved Hide resolved
packages/crypto/src/bls12_318/pairing.rs Outdated Show resolved Hide resolved
packages/crypto/src/bls12_318/pairing.rs Outdated Show resolved Hide resolved
contracts/crypto-verify/src/contract.rs Outdated Show resolved Hide resolved
contracts/crypto-verify/tests/integration.rs Outdated Show resolved Hide resolved
contracts/crypto-verify/Cargo.toml Outdated Show resolved Hide resolved
packages/crypto/src/bls12_318/hash.rs Show resolved Hide resolved
packages/crypto/src/bls12_318/pairing.rs Outdated Show resolved Hide resolved
@aumetra aumetra requested a review from webmaster128 May 10, 2024 12:56
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

🐎🐎🐎🐎🐎🐎🐎

@aumetra aumetra merged commit d904fa2 into main May 21, 2024
30 of 31 checks passed
@aumetra aumetra deleted the bls12_381 branch May 21, 2024 15:08
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.

BLS12-381 support
2 participants