From 6a6b33f359f19ced885c4d946eb927123a64a1d3 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Tue, 2 Jun 2020 16:50:31 +0200 Subject: [PATCH 01/30] Introduce trait --- primitives/core/src/traits.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 880b34a1ed191..fa2a4faaffee7 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -174,6 +174,19 @@ pub trait BareCryptoStore: Send + Sync { } } +/// VRF-signing capability. +pub trait VRFSigner { + /// VRF-sign round + fn vrf_sign( + &self, + pair: &sr25519::Pair, + label: &'static [u8], + randomness: &[u8], + slot_number: u64, + epoch: u64 + ) -> (Vec, Vec, Vec); +} + /// A pointer to the key store. pub type BareCryptoStorePtr = Arc>; From 363bb02f67505b211c6050cd113956c6b4dfdb17 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Tue, 2 Jun 2020 16:50:56 +0200 Subject: [PATCH 02/30] Implement VRFSigner in keystore --- Cargo.lock | 2 ++ client/keystore/Cargo.toml | 4 +++- client/keystore/src/lib.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06b1b42b10453..2805716b75479 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6428,8 +6428,10 @@ version = "2.0.0-rc2" dependencies = [ "derive_more", "hex", + "merlin", "parking_lot 0.10.2", "rand 0.7.3", + "schnorrkel", "serde_json", "sp-application-crypto", "sp-core", diff --git a/client/keystore/Cargo.toml b/client/keystore/Cargo.toml index f02869762d52f..93ad9f52a0a28 100644 --- a/client/keystore/Cargo.toml +++ b/client/keystore/Cargo.toml @@ -18,10 +18,12 @@ derive_more = "0.99.2" sp-core = { version = "2.0.0-rc2", path = "../../primitives/core" } sp-application-crypto = { version = "2.0.0-rc2", path = "../../primitives/application-crypto" } hex = "0.4.0" +merlin = { version = "2.0", default-features = false } +parking_lot = "0.10.0" rand = "0.7.2" serde_json = "1.0.41" subtle = "2.1.1" -parking_lot = "0.10.0" +schnorrkel = { version = "0.9.1", features = ["preaudit_deprecated", "u64_backend"], default-features = false} [dev-dependencies] tempfile = "3.1.0" diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 6510bb82327b1..54d2f272132b6 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -20,11 +20,13 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, BareCryptoStoreError as TraitError}, + traits::{BareCryptoStore, BareCryptoStoreError as TraitError, VRFSigner}, + sr25519::Pair as Sr25519Pair, Encode, }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519, ecdsa}; use parking_lot::RwLock; +use merlin::Transcript; /// Keystore pointer pub type KeyStorePtr = Arc>; @@ -296,6 +298,20 @@ impl Store { Ok(public_keys) } + + fn make_vrf_transcript( + &self, + label: &'static [u8], + randomness: &[u8], + slot_number: u64, + epoch: u64 + ) -> Transcript { + let mut transcript = Transcript::new(label.clone()); + transcript.append_u64(b"slot number", slot_number); + transcript.append_u64(b"current epoch", epoch); + transcript.append_message(b"chain randomness", &randomness[..]); + transcript + } } impl BareCryptoStore for Store { @@ -440,6 +456,22 @@ impl BareCryptoStore for Store { } } +impl VRFSigner for Store { + fn vrf_sign( + &self, + pair: &Sr25519Pair, + label: &'static [u8], + randomness: &[u8], + slot_number: u64, + epoch: u64 + ) -> (Vec, Vec, Vec) { + let transcript = self.make_vrf_transcript(label, randomness, slot_number, epoch); + let pair = pair.as_ref(); + let (inout, proof, proof_batchable) = pair.vrf_sign(transcript); + (inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec(), proof_batchable.to_bytes().to_vec()) + } +} + #[cfg(test)] mod tests { use super::*; From 45c758277f4c14246966e9070272974ee420d951 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Tue, 2 Jun 2020 16:51:28 +0200 Subject: [PATCH 03/30] Use vrf_sign from keystore --- client/consensus/babe/src/authorship.rs | 54 ++++++++++++++++++------- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 1a6852c0c186d..b567a44200b65 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -17,14 +17,16 @@ //! BABE authority selection and slot claiming. use sp_consensus_babe::{ - make_transcript, AuthorityId, BabeAuthorityWeight, BABE_VRF_PREFIX, + BABE_ENGINE_ID, BABE_VRF_PREFIX, + AuthorityId, BabeAuthorityWeight, SlotNumber, AuthorityPair, + make_transcript, }; use sp_consensus_babe::digests::{ PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, }; use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof}; -use sp_core::{U256, blake2_256}; +use sp_core::{U256, blake2_256, traits::VRFSigner}; use codec::Encode; use schnorrkel::vrf::VRFInOut; use sp_core::Pair; @@ -188,7 +190,7 @@ pub fn claim_slot( }) .collect::>() }; - claim_slot_using_key_pairs(slot_number, epoch, &key_pairs) + claim_slot_using_key_pairs(slot_number, epoch, keystore, &key_pairs) } /// Like `claim_slot`, but allows passing an explicit set of key pairs. Useful if we intend @@ -196,9 +198,10 @@ pub fn claim_slot( pub fn claim_slot_using_key_pairs( slot_number: SlotNumber, epoch: &Epoch, + signer: &KeyStorePtr, key_pairs: &[(AuthorityPair, usize)], ) -> Option<(PreDigest, AuthorityId)> { - claim_primary_slot(slot_number, epoch, epoch.config.c, &key_pairs) + claim_primary_slot(slot_number, epoch, epoch.config.c, signer, &key_pairs) .or_else(|| { if epoch.config.allowed_slots.is_secondary_plain_slots_allowed() || epoch.config.allowed_slots.is_secondary_vrf_slots_allowed() @@ -228,12 +231,13 @@ fn claim_primary_slot( slot_number: SlotNumber, epoch: &Epoch, c: (u64, u64), + signer: &KeyStorePtr, key_pairs: &[(AuthorityPair, usize)], ) -> Option<(PreDigest, AuthorityId)> { let Epoch { authorities, randomness, epoch_index, .. } = epoch; for (pair, authority_index) in key_pairs { - let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); + // let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); // Compute the threshold we will use. // @@ -241,16 +245,36 @@ fn claim_primary_slot( // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let pre_digest = get_keypair(pair) - .vrf_sign_after_check(transcript, |inout| super::authorship::check_primary_threshold(inout, threshold)) - .map(|s| { - PreDigest::Primary(PrimaryPreDigest { - slot_number, - vrf_output: VRFOutput(s.0.to_output()), - vrf_proof: VRFProof(s.1), - authority_index: *authority_index as u32, - }) - }); + let (inout, proof, proof_batchable) = signer.read().vrf_sign( + pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, + ); + let proof = match schnorrkel::vrf::VRFProof::from_bytes(&proof) { + Ok(proof) => proof, + Err(_) => return None, + }; + let output = match schnorrkel::vrf::VRFOutput::from_bytes(&inout) { + Ok(output) => output, + Err(_) => return None, + }; + let pre_digest = None; + if super::authorship::check_primary_threshold(inout, threshold) { + pre_digest = Some(PreDigest::Primary(PrimaryPreDigest { + slot_number, + vrf_output: VRFOutput(output), + vrf_proof: VRFProof(proof), + authority_index: *authority_index as u32, + })) + } + // let pre_digest = get_keypair(pair) + // .vrf_sign_after_check(transcript, |inout| super::authorship::check_primary_threshold(inout, threshold)) + // .map(|s| { + // PreDigest::Primary(PrimaryPreDigest { + // slot_number, + // vrf_output: VRFOutput(s.0.to_output()), + // vrf_proof: VRFProof(s.1), + // authority_index: *authority_index as u32, + // }) + // }); // early exit on first successful claim if let Some(pre_digest) = pre_digest { From 4fcf607e95bf7f6f6fd519012431af4abdecaf87 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 3 Jun 2020 00:24:20 +0200 Subject: [PATCH 04/30] Convert output to VRFInOut --- client/consensus/babe/src/authorship.rs | 24 +++++++++--------------- client/keystore/src/lib.rs | 6 +++--- primitives/core/src/traits.rs | 2 +- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index b567a44200b65..29ce3ab4780fa 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -237,7 +237,7 @@ fn claim_primary_slot( let Epoch { authorities, randomness, epoch_index, .. } = epoch; for (pair, authority_index) in key_pairs { - // let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); + let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); // Compute the threshold we will use. // @@ -245,19 +245,23 @@ fn claim_primary_slot( // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let (inout, proof, proof_batchable) = signer.read().vrf_sign( + let (output, proof) = signer.read().vrf_sign( pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, ); let proof = match schnorrkel::vrf::VRFProof::from_bytes(&proof) { Ok(proof) => proof, Err(_) => return None, }; - let output = match schnorrkel::vrf::VRFOutput::from_bytes(&inout) { + let output = match schnorrkel::vrf::VRFOutput::from_bytes(&output) { Ok(output) => output, Err(_) => return None, }; - let pre_digest = None; - if super::authorship::check_primary_threshold(inout, threshold) { + let inout = match output.attach_input_hash(&pair.as_ref().as_ref().public, transcript) { + Ok(inout) => inout, + Err(_) => return None, + }; + let mut pre_digest = None; + if super::authorship::check_primary_threshold(&inout, threshold) { pre_digest = Some(PreDigest::Primary(PrimaryPreDigest { slot_number, vrf_output: VRFOutput(output), @@ -265,16 +269,6 @@ fn claim_primary_slot( authority_index: *authority_index as u32, })) } - // let pre_digest = get_keypair(pair) - // .vrf_sign_after_check(transcript, |inout| super::authorship::check_primary_threshold(inout, threshold)) - // .map(|s| { - // PreDigest::Primary(PrimaryPreDigest { - // slot_number, - // vrf_output: VRFOutput(s.0.to_output()), - // vrf_proof: VRFProof(s.1), - // authority_index: *authority_index as u32, - // }) - // }); // early exit on first successful claim if let Some(pre_digest) = pre_digest { diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 54d2f272132b6..765c664657716 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -464,11 +464,11 @@ impl VRFSigner for Store { randomness: &[u8], slot_number: u64, epoch: u64 - ) -> (Vec, Vec, Vec) { + ) -> (Vec, Vec) { let transcript = self.make_vrf_transcript(label, randomness, slot_number, epoch); let pair = pair.as_ref(); - let (inout, proof, proof_batchable) = pair.vrf_sign(transcript); - (inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec(), proof_batchable.to_bytes().to_vec()) + let (inout, proof, _) = pair.vrf_sign(transcript); + (inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec()) } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index fa2a4faaffee7..ec688e3cfee4b 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -184,7 +184,7 @@ pub trait VRFSigner { randomness: &[u8], slot_number: u64, epoch: u64 - ) -> (Vec, Vec, Vec); + ) -> (Vec, Vec); } /// A pointer to the key store. From 3c148b7e16442a595f94e1bca90439d5156d839e Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 3 Jun 2020 00:37:38 +0200 Subject: [PATCH 05/30] Simplify conversion --- client/consensus/babe/src/authorship.rs | 28 +++++++++++-------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 29ce3ab4780fa..4abd1abaeeec3 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -248,27 +248,23 @@ fn claim_primary_slot( let (output, proof) = signer.read().vrf_sign( pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, ); - let proof = match schnorrkel::vrf::VRFProof::from_bytes(&proof) { - Ok(proof) => proof, - Err(_) => return None, - }; - let output = match schnorrkel::vrf::VRFOutput::from_bytes(&output) { - Ok(output) => output, - Err(_) => return None, - }; - let inout = match output.attach_input_hash(&pair.as_ref().as_ref().public, transcript) { - Ok(inout) => inout, - Err(_) => return None, - }; - let mut pre_digest = None; - if super::authorship::check_primary_threshold(&inout, threshold) { - pre_digest = Some(PreDigest::Primary(PrimaryPreDigest { + let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; + let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; + let inout = output.attach_input_hash( + &pair.as_ref().as_ref().public, + transcript + ).ok()?; + + let pre_digest = if super::authorship::check_primary_threshold(&inout, threshold) { + Some(PreDigest::Primary(PrimaryPreDigest { slot_number, vrf_output: VRFOutput(output), vrf_proof: VRFProof(proof), authority_index: *authority_index as u32, })) - } + } else { + None + }; // early exit on first successful claim if let Some(pre_digest) = pre_digest { From 3ca49f63273b5314bc427ea364331ec9ace555c9 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 3 Jun 2020 00:45:53 +0200 Subject: [PATCH 06/30] vrf_sign secondary slot using keystore --- client/consensus/babe/src/authorship.rs | 29 ++++++++++--------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 4abd1abaeeec3..33e24e6bc3ee3 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -127,6 +127,7 @@ fn claim_secondary_slot( slot_number: SlotNumber, epoch: &Epoch, key_pairs: &[(AuthorityPair, usize)], + keystore: &KeyStorePtr, author_secondary_vrf: bool, ) -> Option<(PreDigest, AuthorityId)> { let Epoch { authorities, randomness, epoch_index, .. } = epoch; @@ -144,18 +145,16 @@ fn claim_secondary_slot( for (pair, authority_index) in key_pairs { if pair.public() == *expected_author { let pre_digest = if author_secondary_vrf { - let transcript = super::authorship::make_transcript( - randomness, - slot_number, - *epoch_index, + let (output, proof) = keystore.read().vrf_sign( + pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, ); - - let s = get_keypair(&pair).vrf_sign(transcript); + let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; + let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; PreDigest::SecondaryVRF(SecondaryVRFPreDigest { slot_number, - vrf_output: VRFOutput(s.0.to_output()), - vrf_proof: VRFProof(s.1), + vrf_output: VRFOutput(output), + vrf_proof: VRFProof(proof), authority_index: *authority_index as u32, }) } else { @@ -198,10 +197,10 @@ pub fn claim_slot( pub fn claim_slot_using_key_pairs( slot_number: SlotNumber, epoch: &Epoch, - signer: &KeyStorePtr, + keystore: &KeyStorePtr, key_pairs: &[(AuthorityPair, usize)], ) -> Option<(PreDigest, AuthorityId)> { - claim_primary_slot(slot_number, epoch, epoch.config.c, signer, &key_pairs) + claim_primary_slot(slot_number, epoch, epoch.config.c, keystore, &key_pairs) .or_else(|| { if epoch.config.allowed_slots.is_secondary_plain_slots_allowed() || epoch.config.allowed_slots.is_secondary_vrf_slots_allowed() @@ -210,6 +209,7 @@ pub fn claim_slot_using_key_pairs( slot_number, &epoch, &key_pairs, + keystore, epoch.config.allowed_slots.is_secondary_vrf_slots_allowed(), ) } else { @@ -218,11 +218,6 @@ pub fn claim_slot_using_key_pairs( }) } -fn get_keypair(q: &AuthorityPair) -> &schnorrkel::Keypair { - use sp_core::crypto::IsWrappedBy; - sp_core::sr25519::Pair::from_ref(q).as_ref() -} - /// Claim a primary slot if it is our turn. Returns `None` if it is not our turn. /// This hashes the slot number, epoch, genesis hash, and chain randomness into /// the VRF. If the VRF produces a value less than `threshold`, it is our turn, @@ -231,7 +226,7 @@ fn claim_primary_slot( slot_number: SlotNumber, epoch: &Epoch, c: (u64, u64), - signer: &KeyStorePtr, + keystore: &KeyStorePtr, key_pairs: &[(AuthorityPair, usize)], ) -> Option<(PreDigest, AuthorityId)> { let Epoch { authorities, randomness, epoch_index, .. } = epoch; @@ -245,7 +240,7 @@ fn claim_primary_slot( // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let (output, proof) = signer.read().vrf_sign( + let (output, proof) = keystore.read().vrf_sign( pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, ); let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; From f292623f3192ab677900dedbb5bac1fac8527d50 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 3 Jun 2020 01:06:19 +0200 Subject: [PATCH 07/30] Fix RPC call to claim_slot --- client/consensus/babe/rpc/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index 8e1282a8d79a7..af2903f0dbbe9 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -141,7 +141,7 @@ impl BabeApi for BabeRpcHandler for slot_number in epoch_start..epoch_end { if let Some((claim, key)) = - authorship::claim_slot_using_key_pairs(slot_number, &epoch, &key_pairs) + authorship::claim_slot_using_key_pairs(slot_number, &epoch, &keystore, &key_pairs) { match claim { PreDigest::Primary { .. } => { From df6063de7b0797e3e1d233a227dc0004f6dcf8ce Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 3 Jun 2020 01:59:18 +0200 Subject: [PATCH 08/30] Use Public instead of Pair --- client/consensus/babe/rpc/src/lib.rs | 18 +++----- client/consensus/babe/src/authorship.rs | 59 +++++++++++++------------ client/keystore/src/lib.rs | 13 +++--- primitives/core/src/traits.rs | 5 ++- 4 files changed, 45 insertions(+), 50 deletions(-) diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index af2903f0dbbe9..834413e09d553 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -126,22 +126,14 @@ impl BabeApi for BabeRpcHandler let mut claims: HashMap = HashMap::new(); - let key_pairs = { - let keystore = keystore.read(); - epoch.authorities.iter() - .enumerate() - .flat_map(|(i, a)| { - keystore - .key_pair::(&a.0) - .ok() - .map(|kp| (kp, i)) - }) - .collect::>() - }; + let keys = epoch.authorities.iter() + .enumerate() + .map(|(i, a)| (a.0.clone(), i)) + .collect::>(); for slot_number in epoch_start..epoch_end { if let Some((claim, key)) = - authorship::claim_slot_using_key_pairs(slot_number, &epoch, &keystore, &key_pairs) + authorship::claim_slot_using_key_pairs(slot_number, &epoch, &keystore, &keys) { match claim { PreDigest::Primary { .. } => { diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 33e24e6bc3ee3..dfd6a0b8ac896 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -16,20 +16,20 @@ //! BABE authority selection and slot claiming. +use sp_application_crypto::AppKey; use sp_consensus_babe::{ BABE_ENGINE_ID, BABE_VRF_PREFIX, AuthorityId, BabeAuthorityWeight, - SlotNumber, AuthorityPair, + SlotNumber, make_transcript, }; use sp_consensus_babe::digests::{ PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, }; use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof}; -use sp_core::{U256, blake2_256, traits::VRFSigner}; +use sp_core::{U256, blake2_256, traits::VRFSigner, crypto::Public}; use codec::Encode; -use schnorrkel::vrf::VRFInOut; -use sp_core::Pair; +use schnorrkel::{vrf::VRFInOut, keys::PublicKey}; use sc_keystore::KeyStorePtr; use super::Epoch; @@ -126,7 +126,7 @@ pub(super) fn secondary_slot_author( fn claim_secondary_slot( slot_number: SlotNumber, epoch: &Epoch, - key_pairs: &[(AuthorityPair, usize)], + keys: &[(AuthorityId, usize)], keystore: &KeyStorePtr, author_secondary_vrf: bool, ) -> Option<(PreDigest, AuthorityId)> { @@ -142,12 +142,17 @@ fn claim_secondary_slot( *randomness, )?; - for (pair, authority_index) in key_pairs { - if pair.public() == *expected_author { + for (authority_id, authority_index) in keys { + if authority_id == expected_author { let pre_digest = if author_secondary_vrf { let (output, proof) = keystore.read().vrf_sign( - pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, - ); + AuthorityId::ID, + authority_id.as_ref(), + &BABE_ENGINE_ID, + randomness, + slot_number, + *epoch_index, + ).ok()?; let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; @@ -164,7 +169,7 @@ fn claim_secondary_slot( }) }; - return Some((pre_digest, pair.public())); + return Some((pre_digest, authority_id.clone())); } } @@ -180,16 +185,11 @@ pub fn claim_slot( epoch: &Epoch, keystore: &KeyStorePtr, ) -> Option<(PreDigest, AuthorityId)> { - let key_pairs = { - let keystore = keystore.read(); - epoch.authorities.iter() - .enumerate() - .flat_map(|(i, a)| { - keystore.key_pair::(&a.0).ok().map(|kp| (kp, i)) - }) - .collect::>() - }; - claim_slot_using_key_pairs(slot_number, epoch, keystore, &key_pairs) + let authorities = epoch.authorities.iter() + .enumerate() + .map(|(index, a)| (a.0.clone(), index)) + .collect::>(); + claim_slot_using_key_pairs(slot_number, epoch, keystore, &authorities) } /// Like `claim_slot`, but allows passing an explicit set of key pairs. Useful if we intend @@ -198,9 +198,9 @@ pub fn claim_slot_using_key_pairs( slot_number: SlotNumber, epoch: &Epoch, keystore: &KeyStorePtr, - key_pairs: &[(AuthorityPair, usize)], + keys: &[(AuthorityId, usize)], ) -> Option<(PreDigest, AuthorityId)> { - claim_primary_slot(slot_number, epoch, epoch.config.c, keystore, &key_pairs) + claim_primary_slot(slot_number, epoch, epoch.config.c, keystore, &keys) .or_else(|| { if epoch.config.allowed_slots.is_secondary_plain_slots_allowed() || epoch.config.allowed_slots.is_secondary_vrf_slots_allowed() @@ -208,7 +208,7 @@ pub fn claim_slot_using_key_pairs( claim_secondary_slot( slot_number, &epoch, - &key_pairs, + keys, keystore, epoch.config.allowed_slots.is_secondary_vrf_slots_allowed(), ) @@ -227,11 +227,11 @@ fn claim_primary_slot( epoch: &Epoch, c: (u64, u64), keystore: &KeyStorePtr, - key_pairs: &[(AuthorityPair, usize)], + keys: &[(AuthorityId, usize)], ) -> Option<(PreDigest, AuthorityId)> { let Epoch { authorities, randomness, epoch_index, .. } = epoch; - for (pair, authority_index) in key_pairs { + for (authority_id, authority_index) in keys { let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); // Compute the threshold we will use. @@ -241,12 +241,13 @@ fn claim_primary_slot( let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); let (output, proof) = keystore.read().vrf_sign( - pair.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, - ); + AuthorityId::ID, authority_id.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, + ).ok()?; let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; + let public_key = PublicKey::from_bytes(authority_id.as_slice()).ok()?; let inout = output.attach_input_hash( - &pair.as_ref().as_ref().public, + &public_key, transcript ).ok()?; @@ -263,7 +264,7 @@ fn claim_primary_slot( // early exit on first successful claim if let Some(pre_digest) = pre_digest { - return Some((pre_digest, pair.public())); + return Some((pre_digest, authority_id.clone())); } } diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 765c664657716..e48883f4b7d9e 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -21,7 +21,7 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io:: use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, traits::{BareCryptoStore, BareCryptoStoreError as TraitError, VRFSigner}, - sr25519::Pair as Sr25519Pair, + sr25519::{Public as Sr25519Public, Pair as Sr25519Pair}, Encode, }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519, ecdsa}; @@ -459,16 +459,17 @@ impl BareCryptoStore for Store { impl VRFSigner for Store { fn vrf_sign( &self, - pair: &Sr25519Pair, + key_type: KeyTypeId, + public: &Sr25519Public, label: &'static [u8], randomness: &[u8], slot_number: u64, epoch: u64 - ) -> (Vec, Vec) { + ) -> std::result::Result<(Vec, Vec), ()> { let transcript = self.make_vrf_transcript(label, randomness, slot_number, epoch); - let pair = pair.as_ref(); - let (inout, proof, _) = pair.vrf_sign(transcript); - (inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec()) + let pair = self.key_pair_by_type::(public, key_type).map_err(|_| ())?; + let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); + Ok((inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec())) } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index ec688e3cfee4b..01572033ccc41 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -179,12 +179,13 @@ pub trait VRFSigner { /// VRF-sign round fn vrf_sign( &self, - pair: &sr25519::Pair, + key_type: KeyTypeId, + public: &sr25519::Public, label: &'static [u8], randomness: &[u8], slot_number: u64, epoch: u64 - ) -> (Vec, Vec); + ) -> Result<(Vec, Vec), ()>; } /// A pointer to the key store. From 76bcb2c5f8147492da712d7e1244d1be8dbcffa6 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 4 Jun 2020 11:33:36 +0200 Subject: [PATCH 09/30] Check primary threshold in signer --- client/consensus/babe/src/authorship.rs | 78 +++++++++++++------------ client/keystore/src/lib.rs | 15 ++++- primitives/core/src/traits.rs | 4 +- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index dfd6a0b8ac896..70a50e3064d89 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -21,15 +21,14 @@ use sp_consensus_babe::{ BABE_ENGINE_ID, BABE_VRF_PREFIX, AuthorityId, BabeAuthorityWeight, SlotNumber, - make_transcript, }; use sp_consensus_babe::digests::{ PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, }; use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof}; -use sp_core::{U256, blake2_256, traits::VRFSigner, crypto::Public}; +use sp_core::{U256, blake2_256, traits::VRFSigner}; use codec::Encode; -use schnorrkel::{vrf::VRFInOut, keys::PublicKey}; +use schnorrkel::vrf::VRFInOut; use sc_keystore::KeyStorePtr; use super::Epoch; @@ -145,31 +144,39 @@ fn claim_secondary_slot( for (authority_id, authority_index) in keys { if authority_id == expected_author { let pre_digest = if author_secondary_vrf { - let (output, proof) = keystore.read().vrf_sign( + let result = keystore.read().vrf_sign( AuthorityId::ID, authority_id.as_ref(), &BABE_ENGINE_ID, + BABE_VRF_PREFIX, randomness, slot_number, *epoch_index, - ).ok()?; - let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; - let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; - - PreDigest::SecondaryVRF(SecondaryVRFPreDigest { - slot_number, - vrf_output: VRFOutput(output), - vrf_proof: VRFProof(proof), - authority_index: *authority_index as u32, - }) + u128::MAX, + ).ok(); + if let Some((output, proof)) = result { + let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; + let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; + + Some(PreDigest::SecondaryVRF(SecondaryVRFPreDigest { + slot_number, + vrf_output: VRFOutput(output), + vrf_proof: VRFProof(proof), + authority_index: *authority_index as u32, + })) + } else { + None + } } else { - PreDigest::SecondaryPlain(SecondaryPlainPreDigest { + Some(PreDigest::SecondaryPlain(SecondaryPlainPreDigest { slot_number, authority_index: *authority_index as u32, - }) + })) }; - return Some((pre_digest, authority_id.clone())); + if let Some(pre_digest) = pre_digest { + return Some((pre_digest, authority_id.clone())); + } } } @@ -232,38 +239,33 @@ fn claim_primary_slot( let Epoch { authorities, randomness, epoch_index, .. } = epoch; for (authority_id, authority_index) in keys { - let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); - // Compute the threshold we will use. // // We already checked that authorities contains `key.public()`, so it can't // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let (output, proof) = keystore.read().vrf_sign( - AuthorityId::ID, authority_id.as_ref(), &BABE_ENGINE_ID, randomness, slot_number, *epoch_index, - ).ok()?; - let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; - let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; - let public_key = PublicKey::from_bytes(authority_id.as_slice()).ok()?; - let inout = output.attach_input_hash( - &public_key, - transcript - ).ok()?; - - let pre_digest = if super::authorship::check_primary_threshold(&inout, threshold) { - Some(PreDigest::Primary(PrimaryPreDigest { + let result = keystore.read().vrf_sign( + AuthorityId::ID, + authority_id.as_ref(), + &BABE_ENGINE_ID, + BABE_VRF_PREFIX, + randomness, + slot_number, + *epoch_index, + threshold, + ).ok(); + if let Some((output, proof)) = result { + let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; + let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; + + let pre_digest = PreDigest::Primary(PrimaryPreDigest { slot_number, vrf_output: VRFOutput(output), vrf_proof: VRFProof(proof), authority_index: *authority_index as u32, - })) - } else { - None - }; + }); - // early exit on first successful claim - if let Some(pre_digest) = pre_digest { return Some((pre_digest, authority_id.clone())); } } diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index e48883f4b7d9e..8365d1e511016 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -27,6 +27,7 @@ use sp_core::{ use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519, ecdsa}; use parking_lot::RwLock; use merlin::Transcript; +use schnorrkel::vrf::VRFInOut; /// Keystore pointer pub type KeyStorePtr = Arc>; @@ -312,6 +313,10 @@ impl Store { transcript.append_message(b"chain randomness", &randomness[..]); transcript } + + fn check_primary_threshold(&self, prefix: &'static [u8], inout: &VRFInOut, threshold: u128) -> bool { + u128::from_le_bytes(inout.make_bytes::<[u8; 16]>(prefix)) < threshold + } } impl BareCryptoStore for Store { @@ -462,14 +467,20 @@ impl VRFSigner for Store { key_type: KeyTypeId, public: &Sr25519Public, label: &'static [u8], + prefix: &'static [u8], randomness: &[u8], slot_number: u64, - epoch: u64 + epoch: u64, + threshold: u128, ) -> std::result::Result<(Vec, Vec), ()> { let transcript = self.make_vrf_transcript(label, randomness, slot_number, epoch); let pair = self.key_pair_by_type::(public, key_type).map_err(|_| ())?; let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); - Ok((inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec())) + if self.check_primary_threshold(prefix, &inout, threshold) { + Ok((inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec())) + } else { + Err(()) + } } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 01572033ccc41..a8ac82b569c9c 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -182,9 +182,11 @@ pub trait VRFSigner { key_type: KeyTypeId, public: &sr25519::Public, label: &'static [u8], + prefix: &'static [u8], randomness: &[u8], slot_number: u64, - epoch: u64 + epoch: u64, + threshold: u128, ) -> Result<(Vec, Vec), ()>; } From 81e31c5587b42d2bfe81af68fedcd583baa69f18 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 4 Jun 2020 11:45:24 +0200 Subject: [PATCH 10/30] Fix interface to return error --- client/consensus/babe/src/authorship.rs | 8 +++---- client/keystore/src/lib.rs | 12 ++++++----- primitives/core/src/testing.rs | 28 ++++++++++++------------- primitives/core/src/traits.rs | 25 +++++++++++----------- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 70a50e3064d89..d426f7fd460cf 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -153,8 +153,8 @@ fn claim_secondary_slot( slot_number, *epoch_index, u128::MAX, - ).ok(); - if let Some((output, proof)) = result { + ); + if let Ok(Some((output, proof))) = result { let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; @@ -254,8 +254,8 @@ fn claim_primary_slot( slot_number, *epoch_index, threshold, - ).ok(); - if let Some((output, proof)) = result { + ); + if let Ok(Some((output, proof))) = result { let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 8365d1e511016..336aa23db426d 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -20,7 +20,7 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, BareCryptoStoreError as TraitError, VRFSigner}, + traits::{BareCryptoStore, Error as TraitError, VRFSigner}, sr25519::{Public as Sr25519Public, Pair as Sr25519Pair}, Encode, }; @@ -472,14 +472,16 @@ impl VRFSigner for Store { slot_number: u64, epoch: u64, threshold: u128, - ) -> std::result::Result<(Vec, Vec), ()> { + ) -> std::result::Result, Vec)>, TraitError> { let transcript = self.make_vrf_transcript(label, randomness, slot_number, epoch); - let pair = self.key_pair_by_type::(public, key_type).map_err(|_| ())?; + let pair = self.key_pair_by_type::(public, key_type) + .map_err(|e| TraitError::PairNotFound(e.to_string()))?; + let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); if self.check_primary_threshold(prefix, &inout, threshold) { - Ok((inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec())) + Ok(Some((inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec()))) } else { - Err(()) + Ok(None) } } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index e14eb6a7f37c6..8f3c187b4f69c 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -22,7 +22,7 @@ use crate::crypto::KeyTypeId; use crate::{ crypto::{Pair, Public, CryptoTypePublicPair}, ed25519, sr25519, ecdsa, - traits::BareCryptoStoreError + traits::Error }; #[cfg(feature = "std")] use std::collections::HashSet; @@ -76,7 +76,7 @@ impl KeyStore { #[cfg(feature = "std")] impl crate::traits::BareCryptoStore for KeyStore { - fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError> { + fn keys(&self, id: KeyTypeId) -> Result, Error> { self.keys .get(&id) .map(|map| { @@ -106,11 +106,11 @@ impl crate::traits::BareCryptoStore for KeyStore { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result { + ) -> Result { match seed { Some(seed) => { let pair = sr25519::Pair::from_string(seed, None) - .map_err(|_| BareCryptoStoreError::ValidationError("Generates an `sr25519` pair.".to_owned()))?; + .map_err(|_| Error::ValidationError("Generates an `sr25519` pair.".to_owned()))?; self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into()); Ok(pair.public()) }, @@ -137,11 +137,11 @@ impl crate::traits::BareCryptoStore for KeyStore { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result { + ) -> Result { match seed { Some(seed) => { let pair = ed25519::Pair::from_string(seed, None) - .map_err(|_| BareCryptoStoreError::ValidationError("Generates an `ed25519` pair.".to_owned()))?; + .map_err(|_| Error::ValidationError("Generates an `ed25519` pair.".to_owned()))?; self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into()); Ok(pair.public()) }, @@ -168,11 +168,11 @@ impl crate::traits::BareCryptoStore for KeyStore { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result { + ) -> Result { match seed { Some(seed) => { let pair = ecdsa::Pair::from_string(seed, None) - .map_err(|_| BareCryptoStoreError::ValidationError("Generates an `ecdsa` pair.".to_owned()))?; + .map_err(|_| Error::ValidationError("Generates an `ecdsa` pair.".to_owned()))?; self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into()); Ok(pair.public()) }, @@ -201,7 +201,7 @@ impl crate::traits::BareCryptoStore for KeyStore { &self, id: KeyTypeId, keys: Vec, - ) -> std::result::Result, BareCryptoStoreError> { + ) -> std::result::Result, Error> { let provided_keys = keys.into_iter().collect::>(); let all_keys = self.keys(id)?.into_iter().collect::>(); @@ -213,29 +213,29 @@ impl crate::traits::BareCryptoStore for KeyStore { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8], - ) -> Result, BareCryptoStoreError> { + ) -> Result, Error> { use codec::Encode; match key.0 { ed25519::CRYPTO_ID => { let key_pair: ed25519::Pair = self .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice())) - .ok_or(BareCryptoStoreError::PairNotFound("ed25519".to_owned()))?; + .ok_or(Error::PairNotFound("ed25519".to_owned()))?; return Ok(key_pair.sign(msg).encode()); } sr25519::CRYPTO_ID => { let key_pair: sr25519::Pair = self .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice())) - .ok_or(BareCryptoStoreError::PairNotFound("sr25519".to_owned()))?; + .ok_or(Error::PairNotFound("sr25519".to_owned()))?; return Ok(key_pair.sign(msg).encode()); } ecdsa::CRYPTO_ID => { let key_pair: ecdsa::Pair = self .ecdsa_key_pair(id, &ecdsa::Public::from_slice(key.1.as_slice())) - .ok_or(BareCryptoStoreError::PairNotFound("ecdsa".to_owned()))?; + .ok_or(Error::PairNotFound("ecdsa".to_owned()))?; return Ok(key_pair.sign(msg).encode()); } - _ => Err(BareCryptoStoreError::KeyNotSupported(id)) + _ => Err(Error::KeyNotSupported(id)) } } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index a8ac82b569c9c..7c6cd95540e85 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -33,7 +33,7 @@ pub use sp_externalities::{Externalities, ExternalitiesExt}; /// BareCryptoStore error #[derive(Debug, derive_more::Display)] -pub enum BareCryptoStoreError { +pub enum Error { /// Public key type is not supported #[display(fmt="Key not supported: {:?}", _0)] KeyNotSupported(KeyTypeId), @@ -51,6 +51,7 @@ pub enum BareCryptoStoreError { Other(String) } + /// Something that generates, stores and provides access to keys. pub trait BareCryptoStore: Send + Sync { /// Returns all sr25519 public keys for the given key type. @@ -64,7 +65,7 @@ pub trait BareCryptoStore: Send + Sync { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result; + ) -> Result; /// Returns all ed25519 public keys for the given key type. fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec; /// Generate a new ed25519 key pair for the given key type and an optional seed. @@ -76,7 +77,7 @@ pub trait BareCryptoStore: Send + Sync { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result; + ) -> Result; /// Returns all ecdsa public keys for the given key type. fn ecdsa_public_keys(&self, id: KeyTypeId) -> Vec; /// Generate a new ecdsa key pair for the given key type and an optional seed. @@ -88,7 +89,7 @@ pub trait BareCryptoStore: Send + Sync { &mut self, id: KeyTypeId, seed: Option<&str>, - ) -> Result; + ) -> Result; /// Insert a new key. This doesn't require any known of the crypto; but a public key must be /// manually provided. @@ -108,11 +109,11 @@ pub trait BareCryptoStore: Send + Sync { &self, id: KeyTypeId, keys: Vec - ) -> Result, BareCryptoStoreError>; + ) -> Result, Error>; /// List all supported keys /// /// Returns a set of public keys the signer supports. - fn keys(&self, id: KeyTypeId) -> Result, BareCryptoStoreError>; + fn keys(&self, id: KeyTypeId) -> Result, Error>; /// Checks if the private keys for the given public key and key type combinations exist. /// @@ -131,7 +132,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, key: &CryptoTypePublicPair, msg: &[u8], - ) -> Result, BareCryptoStoreError>; + ) -> Result, Error>; /// Sign with any key /// @@ -144,7 +145,7 @@ pub trait BareCryptoStore: Send + Sync { id: KeyTypeId, keys: Vec, msg: &[u8] - ) -> Result<(CryptoTypePublicPair, Vec), BareCryptoStoreError> { + ) -> Result<(CryptoTypePublicPair, Vec), Error> { if keys.len() == 1 { return self.sign_with(id, &keys[0], msg).map(|s| (keys[0].clone(), s)); } else { @@ -154,7 +155,7 @@ pub trait BareCryptoStore: Send + Sync { } } } - Err(BareCryptoStoreError::KeyNotSupported(id)) + Err(Error::KeyNotSupported(id)) } /// Sign with all keys @@ -163,13 +164,13 @@ pub trait BareCryptoStore: Send + Sync { /// each key given that the key is supported. /// /// Returns a list of `Result`s each representing the SCALE encoded - /// signature of each key or a BareCryptoStoreError for non-supported keys. + /// signature of each key or a Error for non-supported keys. fn sign_with_all( &self, id: KeyTypeId, keys: Vec, msg: &[u8], - ) -> Result, BareCryptoStoreError>>, ()>{ + ) -> Result, Error>>, ()>{ Ok(keys.iter().map(|k| self.sign_with(id, k, msg)).collect()) } } @@ -187,7 +188,7 @@ pub trait VRFSigner { slot_number: u64, epoch: u64, threshold: u128, - ) -> Result<(Vec, Vec), ()>; + ) -> Result, Vec)>, Error>; } /// A pointer to the key store. From 1d64e560c0e97061bb9b48122f2fd8416ca1b331 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 4 Jun 2020 11:52:11 +0200 Subject: [PATCH 11/30] Move vrf_sign to BareCryptoStore --- client/consensus/babe/src/authorship.rs | 2 +- client/keystore/src/lib.rs | 4 +--- primitives/core/src/testing.rs | 14 ++++++++++++++ primitives/core/src/traits.rs | 5 +---- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index d426f7fd460cf..66fb81d74e8d4 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -26,7 +26,7 @@ use sp_consensus_babe::digests::{ PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, }; use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof}; -use sp_core::{U256, blake2_256, traits::VRFSigner}; +use sp_core::{U256, blake2_256, traits::BareCryptoStore}; use codec::Encode; use schnorrkel::vrf::VRFInOut; use sc_keystore::KeyStorePtr; diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 336aa23db426d..365da3decb0e3 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -20,7 +20,7 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, Error as TraitError, VRFSigner}, + traits::{BareCryptoStore, Error as TraitError}, sr25519::{Public as Sr25519Public, Pair as Sr25519Pair}, Encode, }; @@ -459,9 +459,7 @@ impl BareCryptoStore for Store { fn has_keys(&self, public_keys: &[(Vec, KeyTypeId)]) -> bool { public_keys.iter().all(|(p, t)| self.key_phrase_by_type(&p, *t).is_ok()) } -} -impl VRFSigner for Store { fn vrf_sign( &self, key_type: KeyTypeId, diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 8f3c187b4f69c..901e1ac3b2cec 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -238,6 +238,20 @@ impl crate::traits::BareCryptoStore for KeyStore { _ => Err(Error::KeyNotSupported(id)) } } + + fn vrf_sign( + &self, + _key_type: KeyTypeId, + _public: &sr25519::Public, + _label: &'static [u8], + _prefix: &'static [u8], + _randomness: &[u8], + _slot_number: u64, + _epoch: u64, + _threshold: u128 + ) -> Result, Vec)>, Error> { + Err(Error::Unavailable) + } } /// Macro for exporting functions from wasm in with the expected signature for using it with the diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 7c6cd95540e85..85d0e9095ad57 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -173,11 +173,8 @@ pub trait BareCryptoStore: Send + Sync { ) -> Result, Error>>, ()>{ Ok(keys.iter().map(|k| self.sign_with(id, k, msg)).collect()) } -} -/// VRF-signing capability. -pub trait VRFSigner { - /// VRF-sign round + /// Generate VRF proof for given transacript data. fn vrf_sign( &self, key_type: KeyTypeId, From f62b1d7fa5bfba19e500daa52ad83a68dfc89747 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Thu, 4 Jun 2020 15:59:19 +0200 Subject: [PATCH 12/30] Fix authorship_works test --- client/consensus/babe/rpc/Cargo.toml | 2 +- client/consensus/babe/rpc/src/lib.rs | 15 +++++++++++++-- client/consensus/babe/src/authorship.rs | 4 ++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/client/consensus/babe/rpc/Cargo.toml b/client/consensus/babe/rpc/Cargo.toml index 0986eea2b177f..12aef7aa692a8 100644 --- a/client/consensus/babe/rpc/Cargo.toml +++ b/client/consensus/babe/rpc/Cargo.toml @@ -27,12 +27,12 @@ derive_more = "0.99.2" sp-api = { version = "2.0.0-rc2", path = "../../../../primitives/api" } sp-consensus = { version = "0.8.0-rc2", path = "../../../../primitives/consensus/common" } sp-core = { version = "2.0.0-rc2", path = "../../../../primitives/core" } +sp-application-crypto = { version = "2.0.0-rc2", path = "../../../../primitives/application-crypto" } sc-keystore = { version = "2.0.0-rc2", path = "../../../keystore" } [dev-dependencies] sc-consensus = { version = "0.8.0-rc2", path = "../../../consensus/common" } serde_json = "1.0.50" -sp-application-crypto = { version = "2.0.0-rc2", path = "../../../../primitives/application-crypto" } sp-keyring = { version = "2.0.0-rc2", path = "../../../../primitives/keyring" } substrate-test-runtime-client = { version = "2.0.0-rc2", path = "../../../../test-utils/runtime/client" } tempfile = "3.1.0" diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index 834413e09d553..11ad75b33c24a 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -32,6 +32,11 @@ use sp_consensus_babe::{ digests::PreDigest, }; use serde::{Deserialize, Serialize}; +use sp_core::{ + crypto::Public, + traits::BareCryptoStore, +}; +use sp_application_crypto::AppKey; use sc_keystore::KeyStorePtr; use sc_rpc_api::DenyUnsafe; use sp_api::{ProvideRuntimeApi, BlockId}; @@ -128,12 +133,18 @@ impl BabeApi for BabeRpcHandler let keys = epoch.authorities.iter() .enumerate() - .map(|(i, a)| (a.0.clone(), i)) + .filter_map(|(i, a)| { + if keystore.read().has_keys(&[(a.0.to_raw_vec(), AuthorityId::ID)]) { + Some((a.0.clone(), i)) + } else { + None + } + }) .collect::>(); for slot_number in epoch_start..epoch_end { if let Some((claim, key)) = - authorship::claim_slot_using_key_pairs(slot_number, &epoch, &keystore, &keys) + authorship::claim_slot_using_keys(slot_number, &epoch, &keystore, &keys) { match claim { PreDigest::Primary { .. } => { diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 66fb81d74e8d4..052b59a0ee9ab 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -196,12 +196,12 @@ pub fn claim_slot( .enumerate() .map(|(index, a)| (a.0.clone(), index)) .collect::>(); - claim_slot_using_key_pairs(slot_number, epoch, keystore, &authorities) + claim_slot_using_keys(slot_number, epoch, keystore, &authorities) } /// Like `claim_slot`, but allows passing an explicit set of key pairs. Useful if we intend /// to make repeated calls for different slots using the same key pairs. -pub fn claim_slot_using_key_pairs( +pub fn claim_slot_using_keys( slot_number: SlotNumber, epoch: &Epoch, keystore: &KeyStorePtr, From 04ec073d4fe30f0bbfe10a1f6368ca686dc38b79 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Sun, 7 Jun 2020 07:40:23 +0200 Subject: [PATCH 13/30] Fix BABE logic leaks --- client/consensus/babe/src/authorship.rs | 66 ++++++++++++++----------- client/keystore/src/lib.rs | 40 +++------------ primitives/core/src/testing.rs | 13 ++--- primitives/core/src/traits.rs | 32 +++++++++--- 4 files changed, 72 insertions(+), 79 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 052b59a0ee9ab..2e752be60136a 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -18,17 +18,21 @@ use sp_application_crypto::AppKey; use sp_consensus_babe::{ - BABE_ENGINE_ID, BABE_VRF_PREFIX, + BABE_VRF_PREFIX, AuthorityId, BabeAuthorityWeight, SlotNumber, + make_transcript, }; use sp_consensus_babe::digests::{ PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, }; use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof}; -use sp_core::{U256, blake2_256, traits::BareCryptoStore}; +use sp_core::{U256, blake2_256, crypto::Public, traits::BareCryptoStore}; use codec::Encode; -use schnorrkel::vrf::VRFInOut; +use schnorrkel::{ + keys::PublicKey, + vrf::VRFInOut, +}; use sc_keystore::KeyStorePtr; use super::Epoch; @@ -144,19 +148,19 @@ fn claim_secondary_slot( for (authority_id, authority_index) in keys { if authority_id == expected_author { let pre_digest = if author_secondary_vrf { - let result = keystore.read().vrf_sign( - AuthorityId::ID, - authority_id.as_ref(), - &BABE_ENGINE_ID, - BABE_VRF_PREFIX, + let transcript = super::authorship::make_transcript( randomness, slot_number, *epoch_index, - u128::MAX, ); - if let Ok(Some((output, proof))) = result { - let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; - let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; + let result = keystore.read().vrf_sign( + AuthorityId::ID, + authority_id.as_ref(), + transcript, + ); + if let Ok(signature) = result { + let proof = schnorrkel::vrf::VRFProof::from_bytes(&signature.proof).ok()?; + let output = schnorrkel::vrf::VRFOutput::from_bytes(&signature.output).ok()?; Some(PreDigest::SecondaryVRF(SecondaryVRFPreDigest { slot_number, @@ -239,6 +243,7 @@ fn claim_primary_slot( let Epoch { authorities, randomness, epoch_index, .. } = epoch; for (authority_id, authority_index) in keys { + let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); // Compute the threshold we will use. // // We already checked that authorities contains `key.public()`, so it can't @@ -248,25 +253,26 @@ fn claim_primary_slot( let result = keystore.read().vrf_sign( AuthorityId::ID, authority_id.as_ref(), - &BABE_ENGINE_ID, - BABE_VRF_PREFIX, - randomness, - slot_number, - *epoch_index, - threshold, + transcript.clone(), ); - if let Ok(Some((output, proof))) = result { - let proof = schnorrkel::vrf::VRFProof::from_bytes(&proof).ok()?; - let output = schnorrkel::vrf::VRFOutput::from_bytes(&output).ok()?; - - let pre_digest = PreDigest::Primary(PrimaryPreDigest { - slot_number, - vrf_output: VRFOutput(output), - vrf_proof: VRFProof(proof), - authority_index: *authority_index as u32, - }); - - return Some((pre_digest, authority_id.clone())); + if let Ok(signature) = result { + let proof = schnorrkel::vrf::VRFProof::from_bytes(&signature.proof).ok()?; + let output = schnorrkel::vrf::VRFOutput::from_bytes(&signature.output).ok()?; + let public = PublicKey::from_bytes(&authority_id.to_raw_vec()).ok()?; + let inout = match output.attach_input_hash(&public, transcript) { + Ok(inout) => inout, + Err(_) => continue, + }; + if super::authorship::check_primary_threshold(&inout, threshold) { + let pre_digest = PreDigest::Primary(PrimaryPreDigest { + slot_number, + vrf_output: VRFOutput(output), + vrf_proof: VRFProof(proof), + authority_index: *authority_index as u32, + }); + + return Some((pre_digest, authority_id.clone())); + } } } diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index 365da3decb0e3..c9c7f3c1923e0 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -20,14 +20,13 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, Error as TraitError}, + traits::{BareCryptoStore, Error as TraitError, VRFSignature}, sr25519::{Public as Sr25519Public, Pair as Sr25519Pair}, Encode, }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519, ecdsa}; use parking_lot::RwLock; use merlin::Transcript; -use schnorrkel::vrf::VRFInOut; /// Keystore pointer pub type KeyStorePtr = Arc>; @@ -299,24 +298,6 @@ impl Store { Ok(public_keys) } - - fn make_vrf_transcript( - &self, - label: &'static [u8], - randomness: &[u8], - slot_number: u64, - epoch: u64 - ) -> Transcript { - let mut transcript = Transcript::new(label.clone()); - transcript.append_u64(b"slot number", slot_number); - transcript.append_u64(b"current epoch", epoch); - transcript.append_message(b"chain randomness", &randomness[..]); - transcript - } - - fn check_primary_threshold(&self, prefix: &'static [u8], inout: &VRFInOut, threshold: u128) -> bool { - u128::from_le_bytes(inout.make_bytes::<[u8; 16]>(prefix)) < threshold - } } impl BareCryptoStore for Store { @@ -464,23 +445,16 @@ impl BareCryptoStore for Store { &self, key_type: KeyTypeId, public: &Sr25519Public, - label: &'static [u8], - prefix: &'static [u8], - randomness: &[u8], - slot_number: u64, - epoch: u64, - threshold: u128, - ) -> std::result::Result, Vec)>, TraitError> { - let transcript = self.make_vrf_transcript(label, randomness, slot_number, epoch); + transcript: Transcript, + ) -> std::result::Result { let pair = self.key_pair_by_type::(public, key_type) .map_err(|e| TraitError::PairNotFound(e.to_string()))?; let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); - if self.check_primary_threshold(prefix, &inout, threshold) { - Ok(Some((inout.to_output().to_bytes().to_vec(), proof.to_bytes().to_vec()))) - } else { - Ok(None) - } + Ok(VRFSignature { + output: inout.to_output().to_bytes().to_vec(), + proof: proof.to_bytes().to_vec(), + }) } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 901e1ac3b2cec..81f9511859593 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -22,10 +22,12 @@ use crate::crypto::KeyTypeId; use crate::{ crypto::{Pair, Public, CryptoTypePublicPair}, ed25519, sr25519, ecdsa, - traits::Error + traits::{Error, VRFSignature}, }; #[cfg(feature = "std")] use std::collections::HashSet; +#[cfg(feature = "std")] +use merlin::Transcript; /// Key type for generic Ed25519 key. pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); /// Key type for generic Sr 25519 key. @@ -243,13 +245,8 @@ impl crate::traits::BareCryptoStore for KeyStore { &self, _key_type: KeyTypeId, _public: &sr25519::Public, - _label: &'static [u8], - _prefix: &'static [u8], - _randomness: &[u8], - _slot_number: u64, - _epoch: u64, - _threshold: u128 - ) -> Result, Vec)>, Error> { + _transcript: Transcript, + ) -> Result { Err(Error::Unavailable) } } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 85d0e9095ad57..a19a0c7a5360c 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -28,6 +28,7 @@ use std::{ panic::UnwindSafe, sync::Arc, }; +use merlin::Transcript; pub use sp_externalities::{Externalities, ExternalitiesExt}; @@ -51,6 +52,13 @@ pub enum Error { Other(String) } +/// VRF signature data +pub struct VRFSignature { + /// The VRFOutput serialized + pub output: Vec, + /// The calculated VRFProof + pub proof: Vec, +} /// Something that generates, stores and provides access to keys. pub trait BareCryptoStore: Send + Sync { @@ -174,18 +182,26 @@ pub trait BareCryptoStore: Send + Sync { Ok(keys.iter().map(|k| self.sign_with(id, k, msg)).collect()) } - /// Generate VRF proof for given transacript data. + /// Generate VRF signature for given transcript data. + /// + /// Receives KeyTypeId and Public key to be able to map + /// them to a private key that exists in the keystore which + /// is, in turn, used for signing the provided transcript. + /// + /// Returns a result containing the signature data. + /// Namely, VRFOutput and VRFProof which are returned + /// inside the `VRFSignature` container struct. + /// + /// This function will return an error in the cases where + /// the public key and key type provided do not match a private + /// key in the keystore. Or, in the context of remote signing + /// an error could be a network one. fn vrf_sign( &self, key_type: KeyTypeId, public: &sr25519::Public, - label: &'static [u8], - prefix: &'static [u8], - randomness: &[u8], - slot_number: u64, - epoch: u64, - threshold: u128, - ) -> Result, Vec)>, Error>; + transcript: Transcript, + ) -> Result; } /// A pointer to the key store. From 6c6f38003aebd7af7379303ddf0d59aa498aec71 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Sun, 7 Jun 2020 07:43:10 +0200 Subject: [PATCH 14/30] Acquire a read lock once --- client/consensus/babe/src/authorship.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 2e752be60136a..ef080a33c5954 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -145,6 +145,7 @@ fn claim_secondary_slot( *randomness, )?; + let ks = keystore.read(); for (authority_id, authority_index) in keys { if authority_id == expected_author { let pre_digest = if author_secondary_vrf { @@ -153,7 +154,7 @@ fn claim_secondary_slot( slot_number, *epoch_index, ); - let result = keystore.read().vrf_sign( + let result = ks.vrf_sign( AuthorityId::ID, authority_id.as_ref(), transcript, @@ -242,6 +243,7 @@ fn claim_primary_slot( ) -> Option<(PreDigest, AuthorityId)> { let Epoch { authorities, randomness, epoch_index, .. } = epoch; + let ks = keystore.read(); for (authority_id, authority_index) in keys { let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); // Compute the threshold we will use. @@ -250,7 +252,7 @@ fn claim_primary_slot( // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let result = keystore.read().vrf_sign( + let result = ks.vrf_sign( AuthorityId::ID, authority_id.as_ref(), transcript.clone(), From c572d13604e23cf3cfd8a6bc34f90cc9667d88af Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Sun, 7 Jun 2020 07:46:07 +0200 Subject: [PATCH 15/30] Also fix RPC acquiring the read lock once --- client/consensus/babe/rpc/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index 11ad75b33c24a..fd95c44645971 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -131,10 +131,11 @@ impl BabeApi for BabeRpcHandler let mut claims: HashMap = HashMap::new(); + let ks = keystore.read(); let keys = epoch.authorities.iter() .enumerate() .filter_map(|(i, a)| { - if keystore.read().has_keys(&[(a.0.to_raw_vec(), AuthorityId::ID)]) { + if ks.has_keys(&[(a.0.to_raw_vec(), AuthorityId::ID)]) { Some((a.0.clone(), i)) } else { None From e4db9bff6a7e62aabbadca83e2c82fe4fad83187 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 12:32:17 +0200 Subject: [PATCH 16/30] Implement a generic way to construct VRF Transcript --- client/keystore/src/lib.rs | 22 +++++++++++++++++++--- primitives/core/src/testing.rs | 7 +++---- primitives/core/src/traits.rs | 20 +++++++++++++++++--- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index c9c7f3c1923e0..ccab0dea38746 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -20,7 +20,7 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, Error as TraitError, VRFSignature}, + traits::{BareCryptoStore, Error as TraitError, VRFTranscriptData, VRFTranscriptValue, VRFSignature}, sr25519::{Public as Sr25519Public, Pair as Sr25519Pair}, Encode, }; @@ -298,6 +298,21 @@ impl Store { Ok(public_keys) } + + fn make_transcript(&self, data: VRFTranscriptData) -> Transcript { + let mut transcript = Transcript::new(data.label); + for (label, value) in data.items.into_iter() { + match value { + VRFTranscriptValue::Bytes(bytes) => { + transcript.append_message(label.as_bytes(), bytes); + }, + VRFTranscriptValue::U64(val) => { + transcript.append_u64(label.as_bytes(), val); + } + } + } + transcript + } } impl BareCryptoStore for Store { @@ -441,12 +456,13 @@ impl BareCryptoStore for Store { public_keys.iter().all(|(p, t)| self.key_phrase_by_type(&p, *t).is_ok()) } - fn vrf_sign( + fn sr25519_vrf_sign( &self, key_type: KeyTypeId, public: &Sr25519Public, - transcript: Transcript, + transcript_data: VRFTranscriptData, ) -> std::result::Result { + let transcript = self.make_transcript(transcript_data); let pair = self.key_pair_by_type::(public, key_type) .map_err(|e| TraitError::PairNotFound(e.to_string()))?; diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 81f9511859593..debfc7571a806 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -22,12 +22,11 @@ use crate::crypto::KeyTypeId; use crate::{ crypto::{Pair, Public, CryptoTypePublicPair}, ed25519, sr25519, ecdsa, - traits::{Error, VRFSignature}, + traits::{Error, VRFTranscriptData, VRFSignature}, }; #[cfg(feature = "std")] use std::collections::HashSet; #[cfg(feature = "std")] -use merlin::Transcript; /// Key type for generic Ed25519 key. pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); /// Key type for generic Sr 25519 key. @@ -241,11 +240,11 @@ impl crate::traits::BareCryptoStore for KeyStore { } } - fn vrf_sign( + fn sr25519_vrf_sign( &self, _key_type: KeyTypeId, _public: &sr25519::Public, - _transcript: Transcript, + _transcript: VRFTranscriptData, ) -> Result { Err(Error::Unavailable) } diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index a19a0c7a5360c..9ff407104ce91 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -28,7 +28,6 @@ use std::{ panic::UnwindSafe, sync::Arc, }; -use merlin::Transcript; pub use sp_externalities::{Externalities, ExternalitiesExt}; @@ -52,6 +51,21 @@ pub enum Error { Other(String) } +/// An enum whose variants represent possible +/// accepted values to construct the VRF transcript +pub enum VRFTranscriptValue<'a> { + /// Value is an array of bytes + Bytes(&'a [u8]), + /// Value is a u64 integer + U64(u64), +} +/// VRF Transcript data +pub struct VRFTranscriptData<'a> { + /// The transcript's label + pub label: &'static [u8], + /// Additional data to be registered into the transcript + pub items: Vec<(&'static str, VRFTranscriptValue<'a>)>, +} /// VRF signature data pub struct VRFSignature { /// The VRFOutput serialized @@ -196,11 +210,11 @@ pub trait BareCryptoStore: Send + Sync { /// the public key and key type provided do not match a private /// key in the keystore. Or, in the context of remote signing /// an error could be a network one. - fn vrf_sign( + fn sr25519_vrf_sign( &self, key_type: KeyTypeId, public: &sr25519::Public, - transcript: Transcript, + transcript_data: VRFTranscriptData, ) -> Result; } From b581c6ce5f84bdd4e3e651b4897bc9144621f70f Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 12:32:49 +0200 Subject: [PATCH 17/30] Use make_transcript_data to call sr25519_vrf_sign --- Cargo.lock | 1 + client/consensus/babe/src/authorship.rs | 16 +++++++--------- primitives/consensus/babe/Cargo.toml | 1 + primitives/consensus/babe/src/lib.rs | 17 +++++++++++++++++ 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd828dac8b0bc..faf944f5a6cc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7310,6 +7310,7 @@ dependencies = [ "sp-application-crypto", "sp-consensus", "sp-consensus-vrf", + "sp-core", "sp-inherents", "sp-runtime", "sp-std", diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index ef080a33c5954..a3d4a9bf5109f 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -22,6 +22,7 @@ use sp_consensus_babe::{ AuthorityId, BabeAuthorityWeight, SlotNumber, make_transcript, + make_transcript_data, }; use sp_consensus_babe::digests::{ PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, @@ -149,15 +150,11 @@ fn claim_secondary_slot( for (authority_id, authority_index) in keys { if authority_id == expected_author { let pre_digest = if author_secondary_vrf { - let transcript = super::authorship::make_transcript( - randomness, - slot_number, - *epoch_index, - ); - let result = ks.vrf_sign( + let transcript_data = super::authorship::make_transcript_data(randomness, slot_number, *epoch_index); + let result = ks.sr25519_vrf_sign( AuthorityId::ID, authority_id.as_ref(), - transcript, + transcript_data, ); if let Ok(signature) = result { let proof = schnorrkel::vrf::VRFProof::from_bytes(&signature.proof).ok()?; @@ -246,16 +243,17 @@ fn claim_primary_slot( let ks = keystore.read(); for (authority_id, authority_index) in keys { let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); + let transcript_data = super::authorship::make_transcript_data(randomness, slot_number, *epoch_index); // Compute the threshold we will use. // // We already checked that authorities contains `key.public()`, so it can't // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let result = ks.vrf_sign( + let result = ks.sr25519_vrf_sign( AuthorityId::ID, authority_id.as_ref(), - transcript.clone(), + transcript_data, ); if let Ok(signature) = result { let proof = schnorrkel::vrf::VRFProof::from_bytes(&signature.proof).ok()?; diff --git a/primitives/consensus/babe/Cargo.toml b/primitives/consensus/babe/Cargo.toml index 6cda2695d96e7..c49cc4a041b3a 100644 --- a/primitives/consensus/babe/Cargo.toml +++ b/primitives/consensus/babe/Cargo.toml @@ -17,6 +17,7 @@ codec = { package = "parity-scale-codec", version = "1.3.0", default-features = merlin = { version = "2.0", default-features = false } sp-std = { version = "2.0.0-rc2", default-features = false, path = "../../std" } sp-api = { version = "2.0.0-rc2", default-features = false, path = "../../api" } +sp-core = { version = "2.0.0-rc2", default-features = false, path = "../../core" } sp-consensus = { version = "0.8.0-rc2", optional = true, path = "../common" } sp-consensus-vrf = { version = "0.8.0-rc2", path = "../vrf", default-features = false } sp-inherents = { version = "2.0.0-rc2", default-features = false, path = "../../inherents" } diff --git a/primitives/consensus/babe/src/lib.rs b/primitives/consensus/babe/src/lib.rs index 9848715a47f6f..52c2ce1441bcc 100644 --- a/primitives/consensus/babe/src/lib.rs +++ b/primitives/consensus/babe/src/lib.rs @@ -31,6 +31,7 @@ pub use merlin::Transcript; use codec::{Encode, Decode}; use sp_std::vec::Vec; use sp_runtime::{ConsensusEngineId, RuntimeDebug}; +use sp_core::traits::{VRFTranscriptData, VRFTranscriptValue}; use crate::digests::{NextEpochDescriptor, NextConfigDescriptor}; mod app { @@ -94,6 +95,22 @@ pub fn make_transcript( transcript } +/// Make a VRF transcript data container +pub fn make_transcript_data( + randomness: &Randomness, + slot_number: u64, + epoch: u64, +) -> VRFTranscriptData { + VRFTranscriptData { + label: &BABE_ENGINE_ID, + items: vec![ + ("slot number", VRFTranscriptValue::U64(slot_number)), + ("current epoch", VRFTranscriptValue::U64(epoch)), + ("chain randomness", VRFTranscriptValue::Bytes(&randomness[..])), + ] + } +} + /// An consensus log item for BABE. #[derive(Decode, Encode, Clone, PartialEq, Eq)] pub enum ConsensusLog { From 022b706ed6c4ee95835c931c84a146c8176cadcf Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 12:41:16 +0200 Subject: [PATCH 18/30] Make sure VRFTranscriptData is serializable --- client/keystore/src/lib.rs | 2 +- primitives/core/src/traits.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index ccab0dea38746..ef30c82d9b478 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -304,7 +304,7 @@ impl Store { for (label, value) in data.items.into_iter() { match value { VRFTranscriptValue::Bytes(bytes) => { - transcript.append_message(label.as_bytes(), bytes); + transcript.append_message(label.as_bytes(), &bytes); }, VRFTranscriptValue::U64(val) => { transcript.append_u64(label.as_bytes(), val); diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 9ff407104ce91..771be1b3465a3 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -21,7 +21,7 @@ use crate::{ crypto::{KeyTypeId, CryptoTypePublicPair}, ed25519, sr25519, ecdsa, }; - +use codec::Encode; use std::{ borrow::Cow, fmt::{Debug, Display}, @@ -53,6 +53,7 @@ pub enum Error { /// An enum whose variants represent possible /// accepted values to construct the VRF transcript +#[derive(Encode)] pub enum VRFTranscriptValue<'a> { /// Value is an array of bytes Bytes(&'a [u8]), @@ -60,6 +61,7 @@ pub enum VRFTranscriptValue<'a> { U64(u64), } /// VRF Transcript data +#[derive(Encode)] pub struct VRFTranscriptData<'a> { /// The transcript's label pub label: &'static [u8], From 0c23a485b919599eb9317859b0f8bc7c8a7a6aa8 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 12:45:34 +0200 Subject: [PATCH 19/30] Cleanup --- Cargo.lock | 1 - client/consensus/babe/src/authorship.rs | 18 +++++++++++++++--- client/keystore/Cargo.toml | 1 - 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index faf944f5a6cc1..40243416a673f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6309,7 +6309,6 @@ dependencies = [ "merlin", "parking_lot 0.10.2", "rand 0.7.3", - "schnorrkel", "serde_json", "sp-application-crypto", "sp-core", diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index a3d4a9bf5109f..1926f91a49ea2 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -150,7 +150,11 @@ fn claim_secondary_slot( for (authority_id, authority_index) in keys { if authority_id == expected_author { let pre_digest = if author_secondary_vrf { - let transcript_data = super::authorship::make_transcript_data(randomness, slot_number, *epoch_index); + let transcript_data = super::authorship::make_transcript_data( + randomness, + slot_number, + *epoch_index + ); let result = ks.sr25519_vrf_sign( AuthorityId::ID, authority_id.as_ref(), @@ -242,8 +246,16 @@ fn claim_primary_slot( let ks = keystore.read(); for (authority_id, authority_index) in keys { - let transcript = super::authorship::make_transcript(randomness, slot_number, *epoch_index); - let transcript_data = super::authorship::make_transcript_data(randomness, slot_number, *epoch_index); + let transcript = super::authorship::make_transcript( + randomness, + slot_number, + *epoch_index + ); + let transcript_data = super::authorship::make_transcript_data( + randomness, + slot_number, + *epoch_index + ); // Compute the threshold we will use. // // We already checked that authorities contains `key.public()`, so it can't diff --git a/client/keystore/Cargo.toml b/client/keystore/Cargo.toml index 93ad9f52a0a28..12e3d8da826cc 100644 --- a/client/keystore/Cargo.toml +++ b/client/keystore/Cargo.toml @@ -23,7 +23,6 @@ parking_lot = "0.10.0" rand = "0.7.2" serde_json = "1.0.41" subtle = "2.1.1" -schnorrkel = { version = "0.9.1", features = ["preaudit_deprecated", "u64_backend"], default-features = false} [dev-dependencies] tempfile = "3.1.0" From c3f17defc66e0e3412471a3306754a736f8e3415 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 13:13:59 +0200 Subject: [PATCH 20/30] Move VRF to it's own module --- client/keystore/src/lib.rs | 21 ++------- primitives/consensus/babe/src/lib.rs | 2 +- primitives/core/src/lib.rs | 2 + primitives/core/src/testing.rs | 3 +- primitives/core/src/traits.rs | 27 +---------- primitives/core/src/vrf.rs | 67 ++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 46 deletions(-) create mode 100644 primitives/core/src/vrf.rs diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index ef30c82d9b478..d30c4602d5c56 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -20,13 +20,13 @@ use std::{collections::{HashMap, HashSet}, path::PathBuf, fs::{self, File}, io::{self, Write}, sync::Arc}; use sp_core::{ crypto::{IsWrappedBy, CryptoTypePublicPair, KeyTypeId, Pair as PairT, Protected, Public}, - traits::{BareCryptoStore, Error as TraitError, VRFTranscriptData, VRFTranscriptValue, VRFSignature}, + traits::{BareCryptoStore, Error as TraitError}, sr25519::{Public as Sr25519Public, Pair as Sr25519Pair}, + vrf::{VRFTranscriptData, VRFSignature, make_transcript}, Encode, }; use sp_application_crypto::{AppKey, AppPublic, AppPair, ed25519, sr25519, ecdsa}; use parking_lot::RwLock; -use merlin::Transcript; /// Keystore pointer pub type KeyStorePtr = Arc>; @@ -298,21 +298,6 @@ impl Store { Ok(public_keys) } - - fn make_transcript(&self, data: VRFTranscriptData) -> Transcript { - let mut transcript = Transcript::new(data.label); - for (label, value) in data.items.into_iter() { - match value { - VRFTranscriptValue::Bytes(bytes) => { - transcript.append_message(label.as_bytes(), &bytes); - }, - VRFTranscriptValue::U64(val) => { - transcript.append_u64(label.as_bytes(), val); - } - } - } - transcript - } } impl BareCryptoStore for Store { @@ -462,7 +447,7 @@ impl BareCryptoStore for Store { public: &Sr25519Public, transcript_data: VRFTranscriptData, ) -> std::result::Result { - let transcript = self.make_transcript(transcript_data); + let transcript = make_transcript(transcript_data); let pair = self.key_pair_by_type::(public, key_type) .map_err(|e| TraitError::PairNotFound(e.to_string()))?; diff --git a/primitives/consensus/babe/src/lib.rs b/primitives/consensus/babe/src/lib.rs index 52c2ce1441bcc..7f40b7948fd30 100644 --- a/primitives/consensus/babe/src/lib.rs +++ b/primitives/consensus/babe/src/lib.rs @@ -31,7 +31,7 @@ pub use merlin::Transcript; use codec::{Encode, Decode}; use sp_std::vec::Vec; use sp_runtime::{ConsensusEngineId, RuntimeDebug}; -use sp_core::traits::{VRFTranscriptData, VRFTranscriptValue}; +use sp_core::vrf::{VRFTranscriptData, VRFTranscriptValue}; use crate::digests::{NextEpochDescriptor, NextConfigDescriptor}; mod app { diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index 56dbbc7b7898d..595cb581fca54 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -73,6 +73,8 @@ pub mod traits; pub mod testing; #[cfg(feature = "std")] pub mod tasks; +#[cfg(feature = "std")] +pub mod vrf; pub use self::hash::{H160, H256, H512, convert_hash}; pub use self::uint::{U256, U512}; diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index debfc7571a806..6c3a12baf1bc1 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -22,7 +22,8 @@ use crate::crypto::KeyTypeId; use crate::{ crypto::{Pair, Public, CryptoTypePublicPair}, ed25519, sr25519, ecdsa, - traits::{Error, VRFTranscriptData, VRFSignature}, + traits::Error, + vrf::{VRFTranscriptData, VRFSignature, make_transcript}, }; #[cfg(feature = "std")] use std::collections::HashSet; diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 771be1b3465a3..eed25c38df200 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -19,9 +19,9 @@ use crate::{ crypto::{KeyTypeId, CryptoTypePublicPair}, + vrf::{VRFTranscriptData, VRFSignature}, ed25519, sr25519, ecdsa, }; -use codec::Encode; use std::{ borrow::Cow, fmt::{Debug, Display}, @@ -51,31 +51,6 @@ pub enum Error { Other(String) } -/// An enum whose variants represent possible -/// accepted values to construct the VRF transcript -#[derive(Encode)] -pub enum VRFTranscriptValue<'a> { - /// Value is an array of bytes - Bytes(&'a [u8]), - /// Value is a u64 integer - U64(u64), -} -/// VRF Transcript data -#[derive(Encode)] -pub struct VRFTranscriptData<'a> { - /// The transcript's label - pub label: &'static [u8], - /// Additional data to be registered into the transcript - pub items: Vec<(&'static str, VRFTranscriptValue<'a>)>, -} -/// VRF signature data -pub struct VRFSignature { - /// The VRFOutput serialized - pub output: Vec, - /// The calculated VRFProof - pub proof: Vec, -} - /// Something that generates, stores and provides access to keys. pub trait BareCryptoStore: Send + Sync { /// Returns all sr25519 public keys for the given key type. diff --git a/primitives/core/src/vrf.rs b/primitives/core/src/vrf.rs new file mode 100644 index 0000000000000..8ee9bc34465f3 --- /dev/null +++ b/primitives/core/src/vrf.rs @@ -0,0 +1,67 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! VRF-specifc data types and helpers + +/// An enum whose variants represent possible +/// accepted values to construct the VRF transcript + +use codec::Encode; +use merlin::Transcript; + +/// A enum which holds a single VRF transcript +/// supported value. +#[derive(Clone, Encode)] +pub enum VRFTranscriptValue<'a> { + /// Value is an array of bytes + Bytes(&'a [u8]), + /// Value is a u64 integer + U64(u64), +} +/// VRF Transcript data +#[derive(Clone, Encode)] +pub struct VRFTranscriptData<'a> { + /// The transcript's label + pub label: &'static [u8], + /// Additional data to be registered into the transcript + pub items: Vec<(&'static str, VRFTranscriptValue<'a>)>, +} +/// VRF signature data +pub struct VRFSignature { + /// The VRFOutput serialized + pub output: Vec, + /// The calculated VRFProof + pub proof: Vec, +} + +/// Construct a `Transcript` object from data. +/// +/// Returns `merlin::Transcript` +pub fn make_transcript(data: VRFTranscriptData) -> Transcript { + let mut transcript = Transcript::new(data.label); + for (label, value) in data.items.into_iter() { + match value { + VRFTranscriptValue::Bytes(bytes) => { + transcript.append_message(label.as_bytes(), &bytes); + }, + VRFTranscriptValue::U64(val) => { + transcript.append_u64(label.as_bytes(), val); + } + } + } + transcript +} From a691f24304d0af301657d21b5b77faf2d6d12389 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 13:14:12 +0200 Subject: [PATCH 21/30] Implement & test VRF signing in testing module --- primitives/core/src/testing.rs | 55 +++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index 6c3a12baf1bc1..e89aa438b48b5 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -243,11 +243,19 @@ impl crate::traits::BareCryptoStore for KeyStore { fn sr25519_vrf_sign( &self, - _key_type: KeyTypeId, - _public: &sr25519::Public, - _transcript: VRFTranscriptData, + key_type: KeyTypeId, + public: &sr25519::Public, + transcript_data: VRFTranscriptData, ) -> Result { - Err(Error::Unavailable) + let transcript = make_transcript(transcript_data); + let pair = self.sr25519_key_pair(key_type, public) + .ok_or(Error::PairNotFound("Not found".to_owned()))?; + + let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); + Ok(VRFSignature { + output: inout.to_output().to_bytes().to_vec(), + proof: proof.to_bytes().to_vec(), + }) } } @@ -380,6 +388,7 @@ mod tests { use super::*; use crate::sr25519; use crate::testing::{ED25519, SR25519}; + use crate::vrf::VRFTranscriptValue; #[test] fn store_key_and_extract() { @@ -411,4 +420,42 @@ mod tests { assert!(public_keys.contains(&key_pair.public().into())); } + + #[test] + fn vrf_sign() { + let store = KeyStore::new(); + + let secret_uri = "//Alice"; + let key_pair = sr25519::Pair::from_string(secret_uri, None).expect("Generates key pair"); + + let transcript_data = VRFTranscriptData { + label: b"Test", + items: vec![ + ("one", VRFTranscriptValue::U64(1)), + ("two", VRFTranscriptValue::U64(2)), + ("three", VRFTranscriptValue::Bytes("test".as_bytes())), + ] + }; + + let result = store.read().sr25519_vrf_sign( + SR25519, + &key_pair.public(), + transcript_data.clone(), + ); + assert!(result.is_err()); + + store.write().insert_unknown( + SR25519, + secret_uri, + key_pair.public().as_ref(), + ).expect("Inserts unknown key"); + + let result = store.read().sr25519_vrf_sign( + SR25519, + &key_pair.public(), + transcript_data, + ); + + assert!(result.is_ok()); + } } From b5322c307793b5c53cad3b4ed2fe5c18ce29baf7 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 13:41:19 +0200 Subject: [PATCH 22/30] Remove leftover --- primitives/core/src/testing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index e89aa438b48b5..a310bca2c56ad 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -27,7 +27,7 @@ use crate::{ }; #[cfg(feature = "std")] use std::collections::HashSet; -#[cfg(feature = "std")] + /// Key type for generic Ed25519 key. pub const ED25519: KeyTypeId = KeyTypeId(*b"ed25"); /// Key type for generic Sr 25519 key. From 53bed3987b60032e81831b2c1a1f76732f97df82 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 14:16:54 +0200 Subject: [PATCH 23/30] Fix feature requirements --- primitives/consensus/babe/Cargo.toml | 1 + primitives/consensus/babe/src/lib.rs | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/primitives/consensus/babe/Cargo.toml b/primitives/consensus/babe/Cargo.toml index c49cc4a041b3a..cfe128830c83c 100644 --- a/primitives/consensus/babe/Cargo.toml +++ b/primitives/consensus/babe/Cargo.toml @@ -27,6 +27,7 @@ sp-timestamp = { version = "2.0.0-rc2", default-features = false, path = "../../ [features] default = ["std"] std = [ + "sp-core/std", "sp-application-crypto/std", "codec/std", "merlin/std", diff --git a/primitives/consensus/babe/src/lib.rs b/primitives/consensus/babe/src/lib.rs index 7f40b7948fd30..8a53018d9b153 100644 --- a/primitives/consensus/babe/src/lib.rs +++ b/primitives/consensus/babe/src/lib.rs @@ -31,6 +31,7 @@ pub use merlin::Transcript; use codec::{Encode, Decode}; use sp_std::vec::Vec; use sp_runtime::{ConsensusEngineId, RuntimeDebug}; +#[cfg(feature = "std")] use sp_core::vrf::{VRFTranscriptData, VRFTranscriptValue}; use crate::digests::{NextEpochDescriptor, NextConfigDescriptor}; @@ -96,18 +97,19 @@ pub fn make_transcript( } /// Make a VRF transcript data container +#[cfg(feature = "std")] pub fn make_transcript_data( randomness: &Randomness, slot_number: u64, epoch: u64, ) -> VRFTranscriptData { + let mut items = Vec::new(); + items.push(("slot_number", VRFTranscriptValue::U64(slot_number))); + items.push(("current epoch", VRFTranscriptValue::U64(epoch))); + items.push(("chain randomness", VRFTranscriptValue::Bytes(&randomness[..]))); VRFTranscriptData { label: &BABE_ENGINE_ID, - items: vec![ - ("slot number", VRFTranscriptValue::U64(slot_number)), - ("current epoch", VRFTranscriptValue::U64(epoch)), - ("chain randomness", VRFTranscriptValue::Bytes(&randomness[..])), - ] + items, } } From 43596d7f874c3cb63c7488c231c69129b4049f27 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 14:27:38 +0200 Subject: [PATCH 24/30] Revert removing vec macro --- primitives/consensus/babe/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/primitives/consensus/babe/src/lib.rs b/primitives/consensus/babe/src/lib.rs index 8a53018d9b153..344ed0ad92c66 100644 --- a/primitives/consensus/babe/src/lib.rs +++ b/primitives/consensus/babe/src/lib.rs @@ -103,13 +103,13 @@ pub fn make_transcript_data( slot_number: u64, epoch: u64, ) -> VRFTranscriptData { - let mut items = Vec::new(); - items.push(("slot_number", VRFTranscriptValue::U64(slot_number))); - items.push(("current epoch", VRFTranscriptValue::U64(epoch))); - items.push(("chain randomness", VRFTranscriptValue::Bytes(&randomness[..]))); VRFTranscriptData { label: &BABE_ENGINE_ID, - items, + items: vec![ + ("slot_number", VRFTranscriptValue::U64(slot_number)), + ("current epoch", VRFTranscriptValue::U64(epoch)), + ("chain randomness", VRFTranscriptValue::Bytes(&randomness[..])), + ] } } From 61e3b8d53674687790d2b30bc450cd59e09f563d Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 16:27:21 +0200 Subject: [PATCH 25/30] Drop keystore pointer to prevent deadlock --- client/consensus/babe/rpc/src/lib.rs | 1 + client/consensus/babe/src/authorship.rs | 6 ++---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index fd95c44645971..c41b697b08d3d 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -142,6 +142,7 @@ impl BabeApi for BabeRpcHandler } }) .collect::>(); + drop(ks); for slot_number in epoch_start..epoch_end { if let Some((claim, key)) = diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 1926f91a49ea2..092a65005c336 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -146,7 +146,6 @@ fn claim_secondary_slot( *randomness, )?; - let ks = keystore.read(); for (authority_id, authority_index) in keys { if authority_id == expected_author { let pre_digest = if author_secondary_vrf { @@ -155,7 +154,7 @@ fn claim_secondary_slot( slot_number, *epoch_index ); - let result = ks.sr25519_vrf_sign( + let result = keystore.read().sr25519_vrf_sign( AuthorityId::ID, authority_id.as_ref(), transcript_data, @@ -244,7 +243,6 @@ fn claim_primary_slot( ) -> Option<(PreDigest, AuthorityId)> { let Epoch { authorities, randomness, epoch_index, .. } = epoch; - let ks = keystore.read(); for (authority_id, authority_index) in keys { let transcript = super::authorship::make_transcript( randomness, @@ -262,7 +260,7 @@ fn claim_primary_slot( // be empty. Therefore, this division in `calculate_threshold` is safe. let threshold = super::authorship::calculate_primary_threshold(c, authorities, *authority_index); - let result = ks.sr25519_vrf_sign( + let result = keystore.read().sr25519_vrf_sign( AuthorityId::ID, authority_id.as_ref(), transcript_data, From 383bf44b6ce52e9ba095129fd43ee847be3dc5b0 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 16:28:05 +0200 Subject: [PATCH 26/30] Nitpicks --- client/consensus/babe/src/authorship.rs | 2 +- primitives/core/src/vrf.rs | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 092a65005c336..350deb0f21d84 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -152,7 +152,7 @@ fn claim_secondary_slot( let transcript_data = super::authorship::make_transcript_data( randomness, slot_number, - *epoch_index + *epoch_index, ); let result = keystore.read().sr25519_vrf_sign( AuthorityId::ID, diff --git a/primitives/core/src/vrf.rs b/primitives/core/src/vrf.rs index 8ee9bc34465f3..13be72a98214a 100644 --- a/primitives/core/src/vrf.rs +++ b/primitives/core/src/vrf.rs @@ -17,14 +17,11 @@ //! VRF-specifc data types and helpers -/// An enum whose variants represent possible -/// accepted values to construct the VRF transcript - use codec::Encode; use merlin::Transcript; -/// A enum which holds a single VRF transcript -/// supported value. +/// An enum whose variants represent possible +/// accepted values to construct the VRF transcript #[derive(Clone, Encode)] pub enum VRFTranscriptValue<'a> { /// Value is an array of bytes From 2508594203dc438ccabf31158477757e74a4e0e8 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 8 Jun 2020 16:59:50 +0200 Subject: [PATCH 27/30] Add test to make sure make_transcript works --- Cargo.lock | 1 + primitives/core/Cargo.toml | 1 + primitives/core/src/vrf.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index a7b6f2c0005db..e3e01eb1e0454 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7385,6 +7385,7 @@ dependencies = [ "pretty_assertions", "primitive-types", "rand 0.7.3", + "rand_chacha 0.2.2", "regex", "schnorrkel", "serde", diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 1c06163843634..11cdf7585bd8e 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -59,6 +59,7 @@ hex-literal = "0.2.1" rand = "0.7.2" criterion = "0.2.11" serde_json = "1.0" +rand_chacha = "0.2.2" [[bench]] name = "bench" diff --git a/primitives/core/src/vrf.rs b/primitives/core/src/vrf.rs index 13be72a98214a..d1b46e977b703 100644 --- a/primitives/core/src/vrf.rs +++ b/primitives/core/src/vrf.rs @@ -62,3 +62,36 @@ pub fn make_transcript(data: VRFTranscriptData) -> Transcript { } transcript } + + +#[cfg(test)] +mod tests { + use super::*; + use crate::vrf::VRFTranscriptValue; + use rand::RngCore; + use rand_chacha::rand_core::SeedableRng; + use rand_chacha::ChaChaRng; + + #[test] + fn transcript_creation_matches() { + let mut orig_transcript = Transcript::new(b"My label"); + orig_transcript.append_u64(b"one", 1); + orig_transcript.append_message(b"two", "test".as_bytes()); + + let new_transcript = make_transcript(VRFTranscriptData { + label: b"My label", + items: vec![ + ("one", VRFTranscriptValue::U64(1)), + ("two", VRFTranscriptValue::Bytes("test".as_bytes())), + ], + }); + let test = |t: Transcript| -> [u8; 16] { + let mut b = [0u8; 16]; + t.build_rng() + .finalize(&mut ChaChaRng::from_seed([0u8;32])) + .fill_bytes(&mut b); + b + }; + debug_assert!(test(orig_transcript) == test(new_transcript)); + } +} From 1fd6936981f86d7424f097380778e8d460006dc1 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 10 Jun 2020 11:13:58 +0200 Subject: [PATCH 28/30] Fix mismatch in VRF transcript --- primitives/consensus/babe/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/consensus/babe/src/lib.rs b/primitives/consensus/babe/src/lib.rs index 344ed0ad92c66..10d4aa5ae50b7 100644 --- a/primitives/consensus/babe/src/lib.rs +++ b/primitives/consensus/babe/src/lib.rs @@ -106,7 +106,7 @@ pub fn make_transcript_data( VRFTranscriptData { label: &BABE_ENGINE_ID, items: vec![ - ("slot_number", VRFTranscriptValue::U64(slot_number)), + ("slot number", VRFTranscriptValue::U64(slot_number)), ("current epoch", VRFTranscriptValue::U64(epoch)), ("chain randomness", VRFTranscriptValue::Bytes(&randomness[..])), ] From b45a1ca1fbdeba407eb438cd2db1f50a404f811d Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 10 Jun 2020 11:15:06 +0200 Subject: [PATCH 29/30] Add a test to verify transcripts match in babe --- Cargo.lock | 1 + client/consensus/babe/Cargo.toml | 1 + client/consensus/babe/src/tests.rs | 48 ++++++++++++++++++++++++++++-- primitives/core/src/vrf.rs | 6 ++-- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e3e01eb1e0454..4bca73fcaa6e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5997,6 +5997,7 @@ dependencies = [ "parking_lot 0.10.2", "pdqselect", "rand 0.7.3", + "rand_chacha 0.2.2", "sc-block-builder", "sc-client-api", "sc-consensus-epochs", diff --git a/client/consensus/babe/Cargo.toml b/client/consensus/babe/Cargo.toml index ca77b14d0ed0a..8afcd15d03278 100644 --- a/client/consensus/babe/Cargo.toml +++ b/client/consensus/babe/Cargo.toml @@ -58,6 +58,7 @@ sc-service = { version = "0.8.0-rc2", default-features = false, path = "../../se substrate-test-runtime-client = { version = "2.0.0-rc2", path = "../../../test-utils/runtime/client" } sc-block-builder = { version = "0.8.0-rc2", path = "../../block-builder" } env_logger = "0.7.0" +rand_chacha = "0.2.2" tempfile = "3.1.0" [features] diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index ada1332295d46..1caed18c1781e 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -21,8 +21,14 @@ #![allow(deprecated)] use super::*; use authorship::claim_slot; -use sp_core::crypto::Pair; -use sp_consensus_babe::{AuthorityPair, SlotNumber, AllowedSlots}; +use sp_core::{crypto::Pair, vrf::make_transcript as transcript_from_data}; +use sp_consensus_babe::{ + AuthorityPair, + SlotNumber, + AllowedSlots, + make_transcript, + make_transcript_data, +}; use sc_block_builder::{BlockBuilder, BlockBuilderProvider}; use sp_consensus::{ NoNetwork as DummyOracle, Proposal, RecordProof, @@ -35,6 +41,11 @@ use sp_runtime::{generic::DigestItem, traits::{Block as BlockT, DigestFor}}; use sc_client_api::{BlockchainEvents, backend::TransactionFor}; use log::debug; use std::{time::Duration, cell::RefCell, task::Poll}; +use rand::RngCore; +use rand_chacha::{ + rand_core::SeedableRng, + ChaChaRng, +}; type Item = DigestItem; @@ -796,3 +807,36 @@ fn verify_slots_are_strictly_increasing() { &mut block_import, ); } + +#[test] +fn babe_transcript_generation_match() { + let _ = env_logger::try_init(); + let keystore_path = tempfile::tempdir().expect("Creates keystore path"); + let keystore = sc_keystore::Store::open(keystore_path.path(), None).expect("Creates keystore"); + let pair = keystore.write().insert_ephemeral_from_seed::("//Alice") + .expect("Generates authority pair"); + + let epoch = Epoch { + start_slot: 0, + authorities: vec![(pair.public(), 1)], + randomness: [0; 32], + epoch_index: 1, + duration: 100, + config: BabeEpochConfiguration { + c: (3, 10), + allowed_slots: AllowedSlots::PrimaryAndSecondaryPlainSlots, + }, + }; + + let orig_transcript = make_transcript(&epoch.randomness.clone(), 1, epoch.epoch_index); + let new_transcript = make_transcript_data(&epoch.randomness, 1, epoch.epoch_index); + + let test = |t: merlin::Transcript| -> [u8; 16] { + let mut b = [0u8; 16]; + t.build_rng() + .finalize(&mut ChaChaRng::from_seed([0u8;32])) + .fill_bytes(&mut b); + b + }; + debug_assert!(test(orig_transcript) == test(transcript_from_data(new_transcript))); +} diff --git a/primitives/core/src/vrf.rs b/primitives/core/src/vrf.rs index d1b46e977b703..c842cd632c59a 100644 --- a/primitives/core/src/vrf.rs +++ b/primitives/core/src/vrf.rs @@ -69,8 +69,10 @@ mod tests { use super::*; use crate::vrf::VRFTranscriptValue; use rand::RngCore; - use rand_chacha::rand_core::SeedableRng; - use rand_chacha::ChaChaRng; + use rand_chacha::{ + rand_core::SeedableRng, + ChaChaRng, + }; #[test] fn transcript_creation_matches() { From 67b1c0361cf19dbd1e94bda360f7cd0befa81589 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Wed, 10 Jun 2020 11:26:54 +0200 Subject: [PATCH 30/30] Return VRFOutput and VRFProof from keystore --- client/consensus/babe/rpc/src/lib.rs | 25 +++++++++++++------------ client/consensus/babe/src/authorship.rs | 15 +++++---------- client/keystore/src/lib.rs | 4 ++-- primitives/core/src/testing.rs | 4 ++-- primitives/core/src/vrf.rs | 6 +++--- 5 files changed, 25 insertions(+), 29 deletions(-) diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index c41b697b08d3d..09f79b3bcbbdb 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -131,18 +131,19 @@ impl BabeApi for BabeRpcHandler let mut claims: HashMap = HashMap::new(); - let ks = keystore.read(); - let keys = epoch.authorities.iter() - .enumerate() - .filter_map(|(i, a)| { - if ks.has_keys(&[(a.0.to_raw_vec(), AuthorityId::ID)]) { - Some((a.0.clone(), i)) - } else { - None - } - }) - .collect::>(); - drop(ks); + let keys = { + let ks = keystore.read(); + epoch.authorities.iter() + .enumerate() + .filter_map(|(i, a)| { + if ks.has_keys(&[(a.0.to_raw_vec(), AuthorityId::ID)]) { + Some((a.0.clone(), i)) + } else { + None + } + }) + .collect::>() + }; for slot_number in epoch_start..epoch_end { if let Some((claim, key)) = diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs index 350deb0f21d84..dfca491eaa8bc 100644 --- a/client/consensus/babe/src/authorship.rs +++ b/client/consensus/babe/src/authorship.rs @@ -160,13 +160,10 @@ fn claim_secondary_slot( transcript_data, ); if let Ok(signature) = result { - let proof = schnorrkel::vrf::VRFProof::from_bytes(&signature.proof).ok()?; - let output = schnorrkel::vrf::VRFOutput::from_bytes(&signature.output).ok()?; - Some(PreDigest::SecondaryVRF(SecondaryVRFPreDigest { slot_number, - vrf_output: VRFOutput(output), - vrf_proof: VRFProof(proof), + vrf_output: VRFOutput(signature.output), + vrf_proof: VRFProof(signature.proof), authority_index: *authority_index as u32, })) } else { @@ -266,18 +263,16 @@ fn claim_primary_slot( transcript_data, ); if let Ok(signature) = result { - let proof = schnorrkel::vrf::VRFProof::from_bytes(&signature.proof).ok()?; - let output = schnorrkel::vrf::VRFOutput::from_bytes(&signature.output).ok()?; let public = PublicKey::from_bytes(&authority_id.to_raw_vec()).ok()?; - let inout = match output.attach_input_hash(&public, transcript) { + let inout = match signature.output.attach_input_hash(&public, transcript) { Ok(inout) => inout, Err(_) => continue, }; if super::authorship::check_primary_threshold(&inout, threshold) { let pre_digest = PreDigest::Primary(PrimaryPreDigest { slot_number, - vrf_output: VRFOutput(output), - vrf_proof: VRFProof(proof), + vrf_output: VRFOutput(signature.output), + vrf_proof: VRFProof(signature.proof), authority_index: *authority_index as u32, }); diff --git a/client/keystore/src/lib.rs b/client/keystore/src/lib.rs index d30c4602d5c56..5be4d6d12c65a 100644 --- a/client/keystore/src/lib.rs +++ b/client/keystore/src/lib.rs @@ -453,8 +453,8 @@ impl BareCryptoStore for Store { let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); Ok(VRFSignature { - output: inout.to_output().to_bytes().to_vec(), - proof: proof.to_bytes().to_vec(), + output: inout.to_output(), + proof, }) } } diff --git a/primitives/core/src/testing.rs b/primitives/core/src/testing.rs index a310bca2c56ad..ddd6a950c7531 100644 --- a/primitives/core/src/testing.rs +++ b/primitives/core/src/testing.rs @@ -253,8 +253,8 @@ impl crate::traits::BareCryptoStore for KeyStore { let (inout, proof, _) = pair.as_ref().vrf_sign(transcript); Ok(VRFSignature { - output: inout.to_output().to_bytes().to_vec(), - proof: proof.to_bytes().to_vec(), + output: inout.to_output(), + proof, }) } } diff --git a/primitives/core/src/vrf.rs b/primitives/core/src/vrf.rs index c842cd632c59a..d392587cb72e7 100644 --- a/primitives/core/src/vrf.rs +++ b/primitives/core/src/vrf.rs @@ -19,7 +19,7 @@ use codec::Encode; use merlin::Transcript; - +use schnorrkel::vrf::{VRFOutput, VRFProof}; /// An enum whose variants represent possible /// accepted values to construct the VRF transcript #[derive(Clone, Encode)] @@ -40,9 +40,9 @@ pub struct VRFTranscriptData<'a> { /// VRF signature data pub struct VRFSignature { /// The VRFOutput serialized - pub output: Vec, + pub output: VRFOutput, /// The calculated VRFProof - pub proof: Vec, + pub proof: VRFProof, } /// Construct a `Transcript` object from data.