From 47f8766da62e0881806f4d00092c8c0a5340bb78 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sat, 13 May 2023 04:20:13 -0400 Subject: [PATCH] Use proper messages for ValidatorSets/InInstructions pallet Provides a DST, and associated metadata as beneficial. Also utilizes MuSig's context to session-bind. Since set_keys_messages also binds to set, this is semi-redundant, yet that's appreciated. --- Cargo.lock | 6 ++- crypto/dkg/Cargo.toml | 15 ++++++- processor/src/substrate_signer.rs | 11 ++--- processor/src/tests/substrate_signer.rs | 3 +- .../client/tests/common/in_instructions.rs | 6 +-- .../client/tests/common/validator_sets.rs | 13 +++--- substrate/client/tests/validator_sets.rs | 12 +---- substrate/in-instructions/pallet/Cargo.toml | 2 +- substrate/in-instructions/pallet/src/lib.rs | 4 +- .../in-instructions/primitives/Cargo.toml | 2 +- .../in-instructions/primitives/src/lib.rs | 5 +++ substrate/tokens/pallet/Cargo.toml | 2 +- substrate/tokens/primitives/Cargo.toml | 2 +- substrate/validator-sets/pallet/Cargo.toml | 9 ++-- substrate/validator-sets/pallet/src/lib.rs | 23 ++++------ .../validator-sets/primitives/Cargo.toml | 12 +++-- .../validator-sets/primitives/src/lib.rs | 45 +++++++++++++++---- 17 files changed, 102 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db97c1ab..c07929ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8780,6 +8780,7 @@ dependencies = [ "ciphersuite", "dalek-ff-group", "dkg", + "dleq", "flexible-transcript", "minimal-ed448", "monero-generators", @@ -10998,8 +10999,6 @@ dependencies = [ name = "validator-sets-pallet" version = "0.1.0" dependencies = [ - "ciphersuite", - "dkg", "frame-support", "frame-system", "hashbrown 0.13.2", @@ -11015,11 +11014,14 @@ dependencies = [ name = "validator-sets-primitives" version = "0.1.0" dependencies = [ + "ciphersuite", + "dkg", "parity-scale-codec", "scale-info", "serai-primitives", "serde", "sp-core", + "sp-std 5.0.0", "zeroize", ] diff --git a/crypto/dkg/Cargo.toml b/crypto/dkg/Cargo.toml index c7c1be77..dbbabd1f 100644 --- a/crypto/dkg/Cargo.toml +++ b/crypto/dkg/Cargo.toml @@ -37,7 +37,20 @@ rand_core = { version = "0.6", default-features = false, features = ["getrandom" ciphersuite = { path = "../ciphersuite", version = "0.3", default-features = false, features = ["ristretto"] } [features] -std = ["thiserror", "rand_core/std", "std-shims/std", "ciphersuite/std", "multiexp/[batch, std]", "schnorr/std", "dleq/serialize"] +std = [ + "thiserror", + "rand_core/std", + + "std-shims/std", + + "ciphersuite/std", + + "multiexp/batch", + "multiexp/std", + + "schnorr/std", + "dleq/serialize" +] serde = ["dep:serde"] tests = ["rand_core/getrandom"] default = ["std"] diff --git a/processor/src/substrate_signer.rs b/processor/src/substrate_signer.rs index 87ed7ea6..f8f00f9b 100644 --- a/processor/src/substrate_signer.rs +++ b/processor/src/substrate_signer.rs @@ -20,7 +20,7 @@ use log::{info, debug, warn}; use serai_client::{ primitives::BlockHash, - in_instructions::primitives::{Batch, SignedBatch}, + in_instructions::primitives::{Batch, SignedBatch, batch_message}, }; use messages::{sign::SignId, coordinator::*}; @@ -253,10 +253,11 @@ impl SubstrateSigner { Err(e) => todo!("malicious signer: {:?}", e), }; - let (machine, share) = match machine.sign(preprocesses, &self.signable[&id.id].encode()) { - Ok(res) => res, - Err(e) => todo!("malicious signer: {:?}", e), - }; + let (machine, share) = + match machine.sign(preprocesses, &batch_message(&self.signable[&id.id])) { + Ok(res) => res, + Err(e) => todo!("malicious signer: {:?}", e), + }; self.signing.insert(id.id, machine); // Broadcast our share diff --git a/processor/src/tests/substrate_signer.rs b/processor/src/tests/substrate_signer.rs index 503acb76..a0fdec07 100644 --- a/processor/src/tests/substrate_signer.rs +++ b/processor/src/tests/substrate_signer.rs @@ -9,7 +9,6 @@ use frost::{ dkg::tests::{key_gen, clone_without}, }; -use scale::Encode; use sp_application_crypto::{RuntimePublic, sr25519::Public}; use serai_db::{DbTxn, Db, MemDb}; @@ -143,7 +142,7 @@ async fn test_substrate_signer() { { assert_eq!(signed_batch.batch, batch); assert!(Public::from_raw(actual_id.key.clone().try_into().unwrap()) - .verify(&batch.encode(), &signed_batch.signature)); + .verify(&batch_message(&batch), &signed_batch.signature)); } else { panic!("didn't get signed batch back"); } diff --git a/substrate/client/tests/common/in_instructions.rs b/substrate/client/tests/common/in_instructions.rs index 85c2e5fd..bb531506 100644 --- a/substrate/client/tests/common/in_instructions.rs +++ b/substrate/client/tests/common/in_instructions.rs @@ -1,12 +1,10 @@ -use scale::Encode; - use sp_core::Pair; use serai_client::{ primitives::insecure_pair_from_name, validator_sets::primitives::{Session, ValidatorSet}, in_instructions::{ - primitives::{Batch, SignedBatch}, + primitives::{Batch, SignedBatch, batch_message}, InInstructionsEvent, }, Serai, @@ -32,7 +30,7 @@ pub async fn provide_batch(batch: Batch) -> [u8; 32] { let block = publish_tx(&Serai::execute_batch(SignedBatch { batch: batch.clone(), - signature: pair.sign(&batch.encode()), + signature: pair.sign(&batch_message(&batch)), })) .await; diff --git a/substrate/client/tests/common/validator_sets.rs b/substrate/client/tests/common/validator_sets.rs index 750d932a..152a16c3 100644 --- a/substrate/client/tests/common/validator_sets.rs +++ b/substrate/client/tests/common/validator_sets.rs @@ -3,18 +3,16 @@ use std::collections::HashMap; use zeroize::Zeroizing; use rand_core::OsRng; -use scale::Encode; - use sp_core::{Pair, sr25519::Signature}; use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; -use frost::dkg::musig::*; +use frost::dkg::musig::musig; use schnorrkel::Schnorrkel; use serai_client::{ primitives::insecure_pair_from_name, validator_sets::{ - primitives::{ValidatorSet, KeyPair}, + primitives::{ValidatorSet, KeyPair, musig_context, musig_key, set_keys_message}, ValidatorSetsEvent, }, Serai, @@ -31,7 +29,7 @@ pub async fn set_validator_set_keys(set: ValidatorSet, key_pair: KeyPair) -> [u8 let public_key = ::read_G::<&[u8]>(&mut public.0.as_ref()).unwrap(); assert_eq!( serai.get_validator_set_musig_key(set).await.unwrap().unwrap(), - musig_key::(&[public_key]).unwrap().to_bytes() + musig_key(set, &[public]).0 ); let secret_key = ::read_F::<&[u8]>( @@ -39,7 +37,8 @@ pub async fn set_validator_set_keys(set: ValidatorSet, key_pair: KeyPair) -> [u8 ) .unwrap(); assert_eq!(Ristretto::generator() * secret_key, public_key); - let threshold_keys = musig::(&Zeroizing::new(secret_key), &[public_key]).unwrap(); + let threshold_keys = + musig::(&musig_context(set), &Zeroizing::new(secret_key), &[public_key]).unwrap(); assert_eq!( serai.get_validator_set_musig_key(set).await.unwrap().unwrap(), threshold_keys.group_key().to_bytes() @@ -52,7 +51,7 @@ pub async fn set_validator_set_keys(set: ValidatorSet, key_pair: KeyPair) -> [u8 Schnorrkel::new(b"substrate"), &HashMap::from([(threshold_keys.params().i(), threshold_keys.into())]), ), - &key_pair.encode(), + &set_keys_message(&set, &key_pair), ); // Vote in a key pair diff --git a/substrate/client/tests/validator_sets.rs b/substrate/client/tests/validator_sets.rs index 6e4acb9b..140df96d 100644 --- a/substrate/client/tests/validator_sets.rs +++ b/substrate/client/tests/validator_sets.rs @@ -2,13 +2,10 @@ use rand_core::{RngCore, OsRng}; use sp_core::{sr25519::Public, Pair}; -use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; -use frost::dkg::musig::musig_key; - use serai_client::{ primitives::{NETWORKS, NetworkId, insecure_pair_from_name}, validator_sets::{ - primitives::{Session, ValidatorSet}, + primitives::{Session, ValidatorSet, musig_key}, ValidatorSetsEvent, }, Serai, @@ -56,12 +53,7 @@ serai_test!( assert_eq!(participants_ref, [(public, set_data.bond)].as_ref()); assert_eq!( serai.get_validator_set_musig_key(set).await.unwrap().unwrap(), - musig_key::(&[::read_G::<&[u8]>( - &mut public.0.as_ref() - ) - .unwrap()]) - .unwrap() - .to_bytes() + musig_key(set, &[public]).0 ); let block = set_validator_set_keys(set, key_pair.clone()).await; diff --git a/substrate/in-instructions/pallet/Cargo.toml b/substrate/in-instructions/pallet/Cargo.toml index 3913b90b..1dedc2fd 100644 --- a/substrate/in-instructions/pallet/Cargo.toml +++ b/substrate/in-instructions/pallet/Cargo.toml @@ -23,7 +23,7 @@ sp-runtime = { git = "https://github.com/serai-dex/substrate", default-features frame-system = { git = "https://github.com/serai-dex/substrate", default-features = false } frame-support = { git = "https://github.com/serai-dex/substrate", default-features = false } -serai-primitives = { path = "../..//primitives", default-features = false } +serai-primitives = { path = "../../primitives", default-features = false } in-instructions-primitives = { path = "../primitives", default-features = false } tokens-pallet = { path = "../../tokens/pallet", default-features = false } diff --git a/substrate/in-instructions/pallet/src/lib.rs b/substrate/in-instructions/pallet/src/lib.rs index a1a0eb41..828bcb61 100644 --- a/substrate/in-instructions/pallet/src/lib.rs +++ b/substrate/in-instructions/pallet/src/lib.rs @@ -9,7 +9,7 @@ use sp_runtime::RuntimeDebug; use serai_primitives::{BlockHash, NetworkId}; pub use in_instructions_primitives as primitives; -use primitives::{InInstruction, InInstructionWithBalance, SignedBatch}; +use primitives::*; #[derive(Clone, Copy, Encode, RuntimeDebug)] #[cfg_attr(feature = "std", derive(scale::Decode, thiserror::Error))] @@ -141,7 +141,7 @@ pub mod pallet { } }; - if !key.verify(&batch.batch.encode(), &batch.signature) { + if !key.verify(&batch_message(&batch.batch), &batch.signature) { Err(InvalidTransaction::BadProof)?; } diff --git a/substrate/in-instructions/primitives/Cargo.toml b/substrate/in-instructions/primitives/Cargo.toml index 680dfadd..b97ad5fe 100644 --- a/substrate/in-instructions/primitives/Cargo.toml +++ b/substrate/in-instructions/primitives/Cargo.toml @@ -22,7 +22,7 @@ sp-application-crypto = { git = "https://github.com/serai-dex/substrate", defaul sp-std = { git = "https://github.com/serai-dex/substrate", default-features = false } sp-runtime = { git = "https://github.com/serai-dex/substrate", default-features = false } -serai-primitives = { path = "../..//primitives", default-features = false } +serai-primitives = { path = "../../primitives", default-features = false } tokens-primitives = { path = "../../tokens/primitives", default-features = false } [features] diff --git a/substrate/in-instructions/primitives/src/lib.rs b/substrate/in-instructions/primitives/src/lib.rs index ab05490e..70d921f1 100644 --- a/substrate/in-instructions/primitives/src/lib.rs +++ b/substrate/in-instructions/primitives/src/lib.rs @@ -79,3 +79,8 @@ impl Zeroize for SignedBatch { self.signature.as_mut().zeroize(); } } + +/// The message for the batch signature. +pub fn batch_message(batch: &Batch) -> Vec { + [b"InInstructions-batch".as_ref(), &batch.encode()].concat() +} diff --git a/substrate/tokens/pallet/Cargo.toml b/substrate/tokens/pallet/Cargo.toml index 3f3b1842..9dc2af94 100644 --- a/substrate/tokens/pallet/Cargo.toml +++ b/substrate/tokens/pallet/Cargo.toml @@ -20,7 +20,7 @@ frame-support = { git = "https://github.com/serai-dex/substrate", default-featur pallet-assets = { git = "https://github.com/serai-dex/substrate", default-features = false } -serai-primitives = { path = "../..//primitives", default-features = false } +serai-primitives = { path = "../../primitives", default-features = false } tokens-primitives = { path = "../primitives", default-features = false } [features] diff --git a/substrate/tokens/primitives/Cargo.toml b/substrate/tokens/primitives/Cargo.toml index c33e276a..60b85c56 100644 --- a/substrate/tokens/primitives/Cargo.toml +++ b/substrate/tokens/primitives/Cargo.toml @@ -18,7 +18,7 @@ scale-info = { version = "2", default-features = false, features = ["derive"] } serde = { version = "1", features = ["derive"], optional = true } -serai-primitives = { path = "../..//primitives", default-features = false } +serai-primitives = { path = "../../primitives", default-features = false } [dev-dependencies] sp-runtime = { git = "https://github.com/serai-dex/substrate", default-features = false } diff --git a/substrate/validator-sets/pallet/Cargo.toml b/substrate/validator-sets/pallet/Cargo.toml index d540a463..fcf1548d 100644 --- a/substrate/validator-sets/pallet/Cargo.toml +++ b/substrate/validator-sets/pallet/Cargo.toml @@ -14,9 +14,6 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] hashbrown = { version = "0.13", default-features = false } -ciphersuite = { version = "0.3", path = "../../../crypto/ciphersuite", default-features = false, features = ["ristretto"] } -dkg = { version = "0.4", path = "../../../crypto/dkg", default-features = false } - scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } scale-info = { version = "2", default-features = false, features = ["derive"] } @@ -31,12 +28,12 @@ validator-sets-primitives = { path = "../primitives", default-features = false } [features] std = [ - "ciphersuite/std", - "dkg/std", - "scale/std", "scale-info/std", + "sp-core/std", + "sp-application-crypto/std", + "frame-system/std", "frame-support/std", diff --git a/substrate/validator-sets/pallet/src/lib.rs b/substrate/validator-sets/pallet/src/lib.rs index 39ecc57e..33d837ca 100644 --- a/substrate/validator-sets/pallet/src/lib.rs +++ b/substrate/validator-sets/pallet/src/lib.rs @@ -69,23 +69,14 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; - - let hash_set = self.participants.iter().map(|key| key.0).collect::>(); - if hash_set.len() != self.participants.len() - { + let hash_set = + self.participants.iter().map(|key| key.0).collect::>(); + if hash_set.len() != self.participants.len() { panic!("participants contained duplicates"); } let mut participants = Vec::new(); - let mut keys = Vec::new(); for participant in self.participants.clone() { - keys.push( - ::read_G::<&[u8]>( - &mut participant.0.as_ref(), - ) - .expect("invalid participant"), - ); participants.push((participant, self.bond)); } let participants = BoundedVec::try_from(participants).unwrap(); @@ -99,7 +90,7 @@ pub mod pallet { Some(ValidatorSetData { bond: self.bond, network, participants: participants.clone() }), ); - MuSigKeys::::set(set, Some(Public(dkg::musig::musig_key::(&keys).unwrap().to_bytes()))); + MuSigKeys::::set(set, Some(musig_key(set, &self.participants))); Pallet::::deposit_event(Event::NewSet { set }) } } @@ -128,7 +119,7 @@ pub mod pallet { let Some(musig_key) = MuSigKeys::::get(set) else { Err(Error::NonExistentValidatorSet)? }; - if !musig_key.verify(&key_pair.encode(), signature) { + if !musig_key.verify(&set_keys_message(&set, key_pair), signature) { Err(Error::BadSignature)?; } @@ -179,7 +170,9 @@ pub mod pallet { let set = ValidatorSet { session, network: *network }; match Self::verify_signature(set, key_pair, signature) { Err(Error::AlreadyGeneratedKeys) => Err(InvalidTransaction::Stale)?, - Err(Error::NonExistentValidatorSet) | Err(Error::BadSignature) => Err(InvalidTransaction::BadProof)?, + Err(Error::NonExistentValidatorSet) | Err(Error::BadSignature) => { + Err(InvalidTransaction::BadProof)? + } Err(Error::__Ignore(_, _)) => unreachable!(), Ok(()) => (), } diff --git a/substrate/validator-sets/primitives/Cargo.toml b/substrate/validator-sets/primitives/Cargo.toml index 30510ee1..791fa8c7 100644 --- a/substrate/validator-sets/primitives/Cargo.toml +++ b/substrate/validator-sets/primitives/Cargo.toml @@ -14,15 +14,19 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] zeroize = { version = "^1.5", features = ["derive"], optional = true } +serde = { version = "1", features = ["derive"], optional = true } + +ciphersuite = { path = "../../../crypto/ciphersuite", version = "0.3", default-features = false, features = ["alloc", "ristretto"] } +dkg = { path = "../../../crypto/dkg", version = "0.4", default-features = false } + scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive", "max-encoded-len"] } scale-info = { version = "2", default-features = false, features = ["derive"] } -serde = { version = "1", features = ["derive"], optional = true } - sp-core = { git = "https://github.com/serai-dex/substrate", default-features = false } +sp-std = { git = "https://github.com/serai-dex/substrate", default-features = false } -serai-primitives = { path = "../..//primitives", default-features = false } +serai-primitives = { path = "../../primitives", default-features = false } [features] -std = ["zeroize", "scale/std", "scale-info/std", "serde", "sp-core/std", "serai-primitives/std"] +std = ["zeroize", "serde", "ciphersuite/std", "dkg/std", "scale/std", "scale-info/std", "sp-core/std", "sp-std/std", "serai-primitives/std"] default = ["std"] diff --git a/substrate/validator-sets/primitives/src/lib.rs b/substrate/validator-sets/primitives/src/lib.rs index 43d39ca7..b9e615e3 100644 --- a/substrate/validator-sets/primitives/src/lib.rs +++ b/substrate/validator-sets/primitives/src/lib.rs @@ -3,13 +3,17 @@ #[cfg(feature = "std")] use zeroize::Zeroize; -use scale::{Encode, Decode, MaxEncodedLen}; -use scale_info::TypeInfo; - #[cfg(feature = "std")] use serde::{Serialize, Deserialize}; -use sp_core::{ConstU32, sr25519, bounded::BoundedVec}; +use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; + +use scale::{Encode, Decode, MaxEncodedLen}; +use scale_info::TypeInfo; + +use sp_core::{ConstU32, sr25519::Public, bounded::BoundedVec}; +#[cfg(not(feature = "std"))] +use sp_std::vec::Vec; use serai_primitives::{NetworkId, Network, Amount}; @@ -37,13 +41,38 @@ pub struct ValidatorSetData { // Participant and their amount bonded to this set // Limit each set to 100 participants for now - pub participants: BoundedVec<(sr25519::Public, Amount), ConstU32<100>>, + pub participants: BoundedVec<(Public, Amount), ConstU32<100>>, } type MaxKeyLen = ConstU32; /// The type representing a Key from an external network. pub type ExternalKey = BoundedVec; -/// A Validator Set's Ristretto key, used for signing InInstructions, and their key on the external -/// network. -pub type KeyPair = (sr25519::Public, ExternalKey); +/// The key pair for a validator set. +/// +/// This is their Ristretto key, used for signing Batches, and their key on the external network. +pub type KeyPair = (Public, ExternalKey); + +/// The MuSig context for a validator set. +pub fn musig_context(set: ValidatorSet) -> Vec { + [b"ValidatorSets-musig_key".as_ref(), &set.encode()].concat() +} + +/// The MuSig public key for a validator set. +/// +/// This function panics on invalid input. +pub fn musig_key(set: ValidatorSet, set_keys: &[Public]) -> Public { + let mut keys = Vec::new(); + for key in set_keys { + keys.push( + ::read_G::<&[u8]>(&mut key.0.as_ref()) + .expect("invalid participant"), + ); + } + Public(dkg::musig::musig_key::(&musig_context(set), &keys).unwrap().to_bytes()) +} + +/// The message for the set_keys signature. +pub fn set_keys_message(set: &ValidatorSet, key_pair: &KeyPair) -> Vec { + [b"ValidatorSets-key_pair".as_ref(), &(set, key_pair).encode()].concat() +}