Bandersnatch VRF #14412

davxy · 2023-06-18T16:12:18Z

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

The primitive can operate in two modes:

as a classical VRF (much like sr25519)
as a ring VRF

The code introduced by this PR has been partially extracted from Sassafras PR where the primitive integration has been proven to fulfill the protocol requirements (PoC node provided within the sassafras PR).

⚠️ Experimental Feature ⚠️

The feature is still experimental and should not be used in production since the implementation and interface may still be subject to significant changes.

To experiment with the primitive the bandersnatch-experimental feature should be enabled.

Open points:

ring-proof, fflonk and bandersnatch-vrfs dependencies

Currently we depend on version published on github of bandersnatch-vrfs crate (also indirectly for its dependencies: w3f fflonk, and ring-proof) .

Waiting for the crates to be published on crates.io

Step towards #11879

Superseeds #13974

Polkadot companion: paritytech/polkadot#7547

Cumulus companion: paritytech/cumulus#2938

davxy · 2023-06-18T16:19:14Z

@burdges the implemented API fullfill the requirements of sassafras as implemented here #11879

primitives/application-crypto/src/bandersnatch.rs

primitives/application-crypto/src/traits.rs

primitives/core/src/bandersnatch.rs

burdges · 2023-06-18T23:56:21Z

I'll remain stuck doing something else most of this week, but I'll dig into this soon!

Answering your initial questions though:

I'd name it bandersnatch_vrf or bandersnatch_rvrf myself. We've no need for a short primitive name here since our usage of sergey's ring_proof is kinda niche: We update a relatively small 1000 node ring every 6 hours. Also, ring_proof closely resembles the beefy bridge work. We'll never put this primitive into a wallet I think.

Although sr25519's VRF is safe to use with soft derivation, we cannot safely do soft derivations in bandersnatch_vrf because our ring VRF optimizations prevent hashing the public key like schnorrkel does. It's not quite as bad as how soft derivations break BLS signature entirely, but they'd break the VRF properties.

At a high level, soft derivations are kinda an artifact of the UTXO model, which seems largely deprecated outside privacy coins. And legacy stuff like bitcoin. We do them in sr25519 because we can do so without messing up anything more important.

We want a siloed hard delegation scheme instead of key derivations for the more scalable identity ring VRF planned for wallets, but this serves a different purpose: Allow different dApps to use a perfectly deterministically derived ring VRF key, without continually accessing your air gapped wallet, and also without any dApp learning the keys delegated to other dApps.

skunert

Looks Good!

skunert · 2023-08-08T07:28:36Z

primitives/core/src/bandersnatch.rs

+//  2048        → 295 KB
+// NOTE: This is quite big but looks like there is an upcoming fix
+// in the backend.
+const RING_CONTEXT_SERIALIZED_LEN: usize = 147752;


Maybe DQ, but how did you come up with this max size?

@skunert Good question. I was wondering this myself. (:

If this is supposed to be 147KB because the max supported ring size is 1024 then it's either too small (147 * 1024 = 150528) or too big (147 * 1000 = 147000) or the serialized sizes in the comment are just approximations. Maybe @davxy you could add an extra comment how this was calculated and why?

This is the SCALE encoded size of the KZG backend.
The w3f guys told be that probably this size can be reduced. But I'll keep this as an internal detail and at the current stage it doesn't matter a lot

primitives/core/src/bandersnatch.rs

skunert · 2023-08-08T08:28:34Z

primitives/core/src/bandersnatch.rs

+		}
+
+		/// Create challenge from the transcript contained within the signing data.
+		pub fn challenge<const N: usize>(&self) -> [u8; N] {


What is this used for?

This is a good question. And I'm glad you asked so I can share something I hope is useful.

To understand the trivial answer some context is required.

Once upon a time Micali et al came out with a paper presenting Interactive Proof Systems (aka IP).
The idea of an IP system is that is able to prove a statement by sharing ~zero knowledge (well, just one bit of knowledge: true/false). I.e. prove if a statement is true without sharing any additional information (for example not sharing why a statement is true). This is in contrast with traditional proofs (think about math proofs where you give a proof as a set of logical steps).

Babai came out with what are called Arthur-Merlin protocols (aka AM). Quite similar but not identical to IP (e.g. randomness of verifier can be shared with the prover).

Has been proven that what can be proven via an IC can proved via a AM protocol (and vice versa)

AM and IP protocols are composed of an exchange of messages (challenges from verifier, responses from the prover) between two parties.

In what is called random-oracle model we assume that there exist some ideal hash function indistinguishable from a real randomness source. In practice we just use some well known hash function (e.g. SHA256).
In this model the challenge that in AM or IM come from the Verifier is replaced by an "auto-generated" challenge that is a function (hash) of what you want to prove (what is called Fiat-Shamir transform).
You can generate a set of challenge/responses. I.e. each challenge will be the some hash of the prev response.

Is assumed that the Prover is not in control of what is the output of the hash. I.e. indistinguishable from what would be a challenge from a "real" Verifier.

In this way we go from an interactive ZK proof sys to a non interactive ZK proofs.

This challenge() can be used to get the computed challenge.

Is used by sassafras in the slot claim step

andresilva

lgtm. my review of the meaty part of this PR (primitives/core/src/bandersnatch.rs) is very light since I'm not too familiar with the inner workings of bandersnatch. judging from previous reviews it seems that both Sergey and Jeff already validated it

andresilva · 2023-08-08T13:23:17Z

primitives/keystore/src/lib.rs

+	/// The signature is valid if the signing key is part of the ring from which
+	/// the `RingProver` has been derived.


What happens if the signing key isn't part of the ring?

If a signature is produced with a key whose corresponding public key doesn't belong to the ring (aka a group of public keys) then nothing bad happens. However the signature is useless as it can't be verified using the ring verifier.

I added some docs and a test clarifying this in 2b2676b

primitives/keystore/src/testing.rs

primitives/application-crypto/src/bandersnatch.rs

andresilva · 2023-08-08T13:42:43Z

primitives/keystore/src/lib.rs

+	///
+	/// The signature is valid if the signing key is part of the ring from which
+	/// the `RingProver` has been derived.
+	///


Why isn't a bandersnatch_ring_vrf_verify primitive provided? Is it because it requires the creation of a RingVerifier context?

koute

Some minor things but looks mostly fine to me.

client/keystore/src/local.rs

primitives/core/Cargo.toml

koute · 2023-08-08T14:14:53Z

primitives/core/src/bandersnatch.rs

+//  2048        → 295 KB
+// NOTE: This is quite big but looks like there is an upcoming fix
+// in the backend.
+const RING_CONTEXT_SERIALIZED_LEN: usize = 147752;


@skunert Good question. I was wondering this myself. (:

If this is supposed to be 147KB because the max supported ring size is 1024 then it's either too small (147 * 1024 = 150528) or too big (147 * 1000 = 147000) or the serialized sizes in the comment are just approximations. Maybe @davxy you could add an extra comment how this was calculated and why?

koute · 2023-08-08T14:17:27Z

primitives/core/src/bandersnatch.rs

+impl Derive for Public {}
+
+impl sp_std::fmt::Debug for Public {
+	#[cfg(feature = "std")]


This should be feature = "serde", no?

I'll eventually modify this for the others primitives as well. Follow up PR

koute · 2023-08-08T14:20:21Z

primitives/core/src/bandersnatch.rs

+impl UncheckedFrom<[u8; SIGNATURE_SERIALIZED_LEN]> for Signature {
+	fn unchecked_from(raw: [u8; SIGNATURE_SERIALIZED_LEN]) -> Self {
+		Signature(raw)
+	}
+}


Hmm... maybe a silly question, but why isn't this just simply From?

To me UncheckedFrom implies that there should be a TryFrom which does some extra invariant checking, but the TryFrom implementation doesn't check any extra invariants besides checking the size, which for a From<[u8; SIGNATURE_SERIALIZED_LEN]> would be checked at compile time.

Are there plans to extend the TryFrom so that it's actually checking something else besides that the size matches?

The crypto primitive implements the same traits implemented by the other crypto types and using the same strategies.
Nevertheless, you are right and indeed my plan is to give some full overhaul to the crypto traits and interfaces in another PR

koute · 2023-08-08T14:23:08Z

primitives/core/src/bandersnatch.rs

+}
+
+impl sp_std::fmt::Debug for Signature {
+	#[cfg(feature = "std")]


Is this feature gate necessary? This should also work in no_std, no?

I'll eventually modify this for the others primitives as well. Follow up PR

koute · 2023-08-08T14:25:25Z

primitives/core/src/bandersnatch.rs

+	fn derive<Iter: Iterator<Item = DeriveJunction>>(
+		&self,
+		path: Iter,
+		_seed: Option<Seed>,


Maybe a silly question, but why is this argument ignored? Perhaps put a comment here and/or return an error if it is Some?

Tools which uses the InspectKeyCmd may end up calling (after some hopping) the from_string_with_seed which in turn will call pair.derive(junctions, seed).

With:

seed the seed generated from the secret phrase according to bip39.

Junctions are the hard/soft junctions after the phrase

Is left to the implementation what to do with the seed during the derive which has been already used to generate the pair itself.

The only implementation which uses it is sr25519 and frankly I don't know why it requires it.
But I don't find it of any usage for bandersnatch as to derive a subkey we use the junctions.

Returning an error is not an option since is called in the same way for all the possible pairs

primitives/core/src/bandersnatch.rs

primitives/keyring/Cargo.toml

Co-authored-by: Koute <koute@users.noreply.github.com>

davxy · 2023-08-09T14:53:23Z

bot merge

paritytech-processbot · 2023-08-09T14:53:31Z

Error: "Check reviews" status is not passing for paritytech/polkadot#7547

davxy · 2023-08-09T15:01:53Z

bot merge

paritytech-processbot · 2023-08-09T15:02:09Z

Waiting for commit status.

davxy added 3 commits June 18, 2023 14:57

Introduce bandersnatch vrf

8c39821

Some documentation

a9446fd

Fix tests

bb3e022

davxy self-assigned this Jun 18, 2023

davxy added 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. B1-note_worthy Changes should be noted in the release notes labels Jun 18, 2023

davxy requested a review from a team June 18, 2023 16:19

Fix docs refs

bdd3df4

paritytech deleted a comment from paritytech-cicd-pr Jun 18, 2023

davxy added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Jun 18, 2023

davxy commented Jun 18, 2023

View reviewed changes

primitives/application-crypto/src/bandersnatch.rs Show resolved Hide resolved

davxy commented Jun 18, 2023

View reviewed changes

primitives/application-crypto/src/traits.rs Outdated Show resolved Hide resolved

davxy commented Jun 18, 2023

View reviewed changes

primitives/core/src/bandersnatch.rs Outdated Show resolved Hide resolved

davxy commented Jun 18, 2023

View reviewed changes

primitives/core/src/bandersnatch.rs Outdated Show resolved Hide resolved

davxy commented Jun 18, 2023

View reviewed changes

primitives/core/src/bandersnatch.rs Outdated Show resolved Hide resolved

davxy mentioned this pull request Jun 19, 2023

Sassafras Consensus Protocol #11879

Draft

5 tasks

davxy added 3 commits June 19, 2023 18:00

Some more docs

e0e493a

Comments about key derivation

b665397

Make clippy happy

1faf0e2

paritytech deleted a comment from paritytech-cicd-pr Jun 21, 2023

davxy added 2 commits June 21, 2023 17:41

Merge branch 'master' into bandersnatch-vrf

2a55c7d

Fix ring context enc/dec test

7812da8

davxy added T0-node This PR/Issue is related to the topic “node”. and removed B0-silent Changes should not be mentioned in any release notes labels Jun 21, 2023

Fix docs

dd53abc

davxy added 6 commits July 28, 2023 12:14

Remove leftovers from Cargo.toml

a5e4cd4

Remove some TODO notes

7bf3c70

Simplification of structs construction

efd39bd

Merge branch 'master' into bandersnatch-vrf

ad76f57

Merge branch 'master' into bandersnatch-vrf

062b13e

Trigger CI

c562181

skunert approved these changes Aug 8, 2023

View reviewed changes

andresilva approved these changes Aug 8, 2023

View reviewed changes

koute approved these changes Aug 8, 2023

View reviewed changes

davxy and others added 4 commits August 9, 2023 08:48

Apply review suggestion

b1f6c58

Co-authored-by: Koute <koute@users.noreply.github.com>

Docs typo

9342856

Merge branch 'master' into bandersnatch-vrf

64ec625

Fix keystore tests

ece4016

paritytech deleted a comment from paritytech-cicd-pr Aug 9, 2023

davxy added 6 commits August 9, 2023 10:50

Consistence

3b553b6

Add ref to git rependency

c2b00b5

Static check of MAX_VRF_IOS value

b339b3a

Clarify behavior for out of ring keys signatures

2b2676b

Add test for ring-vrf to the keystore

0739e0f

Fix docs

a457359

paritytech-processbot bot merged commit 19eb56a into master Aug 9, 2023
5 checks passed

paritytech-processbot bot deleted the bandersnatch-vrf branch August 9, 2023 15:09

davxy mentioned this pull request Aug 31, 2023

[PoC] Sassafras Consensus paritytech/polkadot-sdk#1336

Draft

5 tasks

crystalin mentioned this pull request Oct 20, 2023

Update substrate/polkadot/cumulus from v0.9.43 to v1.1.0 moonbeam-foundation/moonbeam#2535

Closed

JesseAbram mentioned this pull request Oct 26, 2023

Update substrate and subxt entropyxyz/entropy-core#435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bandersnatch VRF #14412

Bandersnatch VRF #14412

davxy commented Jun 18, 2023 •

edited

Loading

davxy commented Jun 18, 2023 •

edited

Loading

burdges commented Jun 18, 2023 •

edited

Loading

skunert left a comment

skunert Aug 8, 2023

koute Aug 8, 2023

davxy Aug 9, 2023 •

edited

Loading

skunert Aug 8, 2023

davxy Aug 9, 2023 •

edited

Loading

davxy Aug 9, 2023

andresilva left a comment

andresilva Aug 8, 2023

davxy Aug 9, 2023 •

edited

Loading

andresilva Aug 8, 2023

koute left a comment

koute Aug 8, 2023

koute Aug 8, 2023

davxy Aug 9, 2023

koute Aug 8, 2023

davxy Aug 8, 2023 •

edited

Loading

koute Aug 8, 2023

davxy Aug 9, 2023

koute Aug 8, 2023

davxy Aug 9, 2023

davxy commented Aug 9, 2023

paritytech-processbot bot commented Aug 9, 2023

davxy commented Aug 9, 2023

paritytech-processbot bot commented Aug 9, 2023

		/// The signature is valid if the signing key is part of the ring from which
		/// the `RingProver` has been derived.

Bandersnatch VRF #14412

Bandersnatch VRF #14412

Conversation

davxy commented Jun 18, 2023 • edited Loading

⚠️ Experimental Feature ⚠️

Open points:

ring-proof, fflonk and bandersnatch-vrfs dependencies

davxy commented Jun 18, 2023 • edited Loading

burdges commented Jun 18, 2023 • edited Loading

skunert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davxy Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davxy Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andresilva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davxy Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koute left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davxy Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davxy commented Aug 9, 2023

paritytech-processbot bot commented Aug 9, 2023

davxy commented Aug 9, 2023

paritytech-processbot bot commented Aug 9, 2023

davxy commented Jun 18, 2023 •

edited

Loading

davxy commented Jun 18, 2023 •

edited

Loading

burdges commented Jun 18, 2023 •

edited

Loading

davxy Aug 9, 2023 •

edited

Loading

davxy Aug 9, 2023 •

edited

Loading

davxy Aug 9, 2023 •

edited

Loading

davxy Aug 8, 2023 •

edited

Loading