Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Bandersnatch VRF #13974

Closed
wants to merge 27 commits into from
Closed

Bandersnatch VRF #13974

wants to merge 27 commits into from

Conversation

davxy
Copy link
Member

@davxy davxy commented Apr 21, 2023

This PR introduces a new kind of VRF which uses bandersnatch-vrfs backend.

  • Primitive codename used within Substrate : bsnvrf (→ Bandersnatch-Vrf)
  • gated behind bandernatch-experimental feature flag

The primitive is used by Sassafras (#11879).

IMO is worth splitting the sassafras PR in smaller pieces to be more easily reviewable (is already +9K)
so first I propose to introduce this VRF to trim it a bit.


Open points:

  1. Key derivation.
  • sr25519 supports both hard and soft secret key derivation (see here). While ecdsa and ed25519 support only hard secret key derivation (e.g. see here). What about bandersnatch?
  1. When a first version bandersnatch-vrfs crate on crates.io?

  2. Missing Ring proof integration. Is not clear if in the end bandersnatch-vrfs crate will provide a single sign function to get both thin and ring proofs.

@davxy davxy self-assigned this Apr 21, 2023
@davxy davxy added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Apr 21, 2023
@paritytech paritytech deleted a comment from paritytech-cicd-pr Apr 21, 2023
@paritytech paritytech deleted a comment from paritytech-cicd-pr Apr 21, 2023
@davxy davxy changed the title [DRAF] Bandersnatch VRF [DRAFT] Bandersnatch VRF Apr 21, 2023
@paritytech paritytech deleted a comment from paritytech-cicd-pr Apr 22, 2023
@paritytech paritytech deleted a comment from paritytech-cicd-pr Apr 25, 2023
@paritytech paritytech deleted a comment from paritytech-cicd-pr Apr 25, 2023
@paritytech paritytech deleted a comment from paritytech-cicd-pr Apr 25, 2023
@davxy davxy marked this pull request as ready for review April 25, 2023 20:42
@davxy davxy changed the title [DRAFT] Bandersnatch VRF Bandersnatch VRF Apr 26, 2023
@davxy davxy added the A0-please_review Pull request needs code review. label Apr 26, 2023
@davxy davxy marked this pull request as draft May 1, 2023 19:41
@davxy
Copy link
Member Author

davxy commented May 1, 2023

requires #14036

@koute
Copy link
Contributor

koute commented May 8, 2023

2. To be fair this is an ed-on-bls12-381-bandersnatch with vrf capabilities etc etc. What is an appropriate (please not verbose) name synthesizing this primitive capabilities and that fits well with the rest of the primitives? (e.g. "schnorr using ristretto -> sr25519)?

Probably incredibly-stupid-random-totally-uninformed-suggestion: maybe bb12381, with the bb pronounced as "baby" (one "b" is for bls, other is for "bandersnatch", and the numbers are numbers). Fits a similar pattern as sr25519. (:

@davxy davxy marked this pull request as ready for review May 18, 2023 18:00
@davxy davxy requested review from a team and removed request for a team May 22, 2023 14:08
@paritytech paritytech deleted a comment from paritytech-cicd-pr May 22, 2023
@davxy
Copy link
Member Author

davxy commented May 22, 2023

Because this key type will be used by a consensus protocol, is required to implement all the traits required by session pallet. One of these is RuntimeAppPublic which in turn required RuntimePublic.

This trait offers some methods requiring access to the keystore: sign and generate_pair

Was my intention to keep the required host functions to the minimum.

  • generate_pair should be implemented. Apparently there is nothing we can do. As it is invoked at startup by the session pallet and also from the rpc interface to generate new keys on demand. I've added an host function for this...

  • sign: this may be useful if we want to sign something from an offchain worker. Currently we don't. Maybe I can just return None here?

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-each-crate-macos
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2873610

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2873593

@davxy davxy mentioned this pull request Jun 13, 2023
5 tasks
@davxy davxy closed this Jun 18, 2023
@davxy davxy removed their assignment Jun 18, 2023
@davxy davxy mentioned this pull request Jun 18, 2023
@davxy davxy deleted the davxy-bandersnatch-vrf branch June 22, 2023 05:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants