From 977f3edfb688a67ad098d45a52834a4bc4294c77 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Fri, 15 Feb 2019 13:58:14 +1100 Subject: [PATCH 1/5] Add domain to all signature funcitons, modify validate_proof_of_possession() --- .../src/attestation_aggregator.rs | 2 +- .../src/validator_harness/local_signer.rs | 16 +++++----- eth2/attester/src/lib.rs | 3 +- eth2/attester/src/test_utils/local_signer.rs | 4 +-- eth2/attester/src/traits.rs | 2 +- eth2/block_producer/src/lib.rs | 4 +-- .../src/test_utils/local_signer.rs | 8 ++--- eth2/block_producer/src/traits.rs | 4 +-- eth2/fork_choice/src/optimised_lmd_ghost.rs | 11 ++++--- eth2/fork_choice/src/slow_lmd_ghost.rs | 2 +- .../state_processing/src/block_processable.rs | 10 +++--- eth2/types/src/attestation.rs | 5 ++- eth2/types/src/beacon_state.rs | 32 ++++++++++++++++--- eth2/types/src/fork.rs | 16 ++++++++++ eth2/types/src/test_utils/signature.rs | 2 +- eth2/utils/bls/Cargo.toml | 2 +- eth2/utils/bls/src/aggregate_signature.rs | 6 ++-- eth2/utils/bls/src/lib.rs | 14 +++----- eth2/utils/bls/src/signature.rs | 20 ++++++------ 19 files changed, 98 insertions(+), 65 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index 149f0d60d..e8576276c 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -112,7 +112,7 @@ impl AttestationAggregator { if !free_attestation .signature - .verify(&signable_message, &validator_record.pubkey) + .verify(&signable_message, spec.domain_attestation, &validator_record.pubkey) { return Ok(Outcome { valid: false, diff --git a/beacon_node/beacon_chain/test_harness/src/validator_harness/local_signer.rs b/beacon_node/beacon_chain/test_harness/src/validator_harness/local_signer.rs index 8e901b057..f176a6889 100644 --- a/beacon_node/beacon_chain/test_harness/src/validator_harness/local_signer.rs +++ b/beacon_node/beacon_chain/test_harness/src/validator_harness/local_signer.rs @@ -25,23 +25,23 @@ impl LocalSigner { } /// Sign some message. - fn bls_sign(&self, message: &[u8]) -> Option { - Some(Signature::new(message, &self.keypair.sk)) + fn bls_sign(&self, message: &[u8], domain: u64) -> Option { + Some(Signature::new(message, domain, &self.keypair.sk)) } } impl BlockProposerSigner for LocalSigner { - fn sign_block_proposal(&self, message: &[u8]) -> Option { - self.bls_sign(message) + fn sign_block_proposal(&self, message: &[u8], domain: u64) -> Option { + self.bls_sign(message, domain) } - fn sign_randao_reveal(&self, message: &[u8]) -> Option { - self.bls_sign(message) + fn sign_randao_reveal(&self, message: &[u8], domain: u64) -> Option { + self.bls_sign(message, domain) } } impl AttesterSigner for LocalSigner { - fn sign_attestation_message(&self, message: &[u8]) -> Option { - self.bls_sign(message) + fn sign_attestation_message(&self, message: &[u8], domain: u64) -> Option { + self.bls_sign(message, domain) } } diff --git a/eth2/attester/src/lib.rs b/eth2/attester/src/lib.rs index 7352dd2ea..f2bbd6db3 100644 --- a/eth2/attester/src/lib.rs +++ b/eth2/attester/src/lib.rs @@ -10,6 +10,7 @@ pub use self::traits::{ }; const PHASE_0_CUSTODY_BIT: bool = false; +const DOMAIN_ATTESTATION: u64 = 1; #[derive(Debug, PartialEq)] pub enum PollOutcome { @@ -137,7 +138,7 @@ impl Attester Option { - Some(Signature::new(message, &self.keypair.sk)) + fn sign_attestation_message(&self, message: &[u8], domain: u64) -> Option { + Some(Signature::new(message, domain, &self.keypair.sk)) } } diff --git a/eth2/attester/src/traits.rs b/eth2/attester/src/traits.rs index 53bce3aaa..6062460cb 100644 --- a/eth2/attester/src/traits.rs +++ b/eth2/attester/src/traits.rs @@ -45,5 +45,5 @@ pub trait DutiesReader: Send + Sync { /// Signs message using an internally-maintained private key. pub trait Signer { - fn sign_attestation_message(&self, message: &[u8]) -> Option; + fn sign_attestation_message(&self, message: &[u8], domain: u64) -> Option; } diff --git a/eth2/block_producer/src/lib.rs b/eth2/block_producer/src/lib.rs index f6a0fd6df..e5651780a 100644 --- a/eth2/block_producer/src/lib.rs +++ b/eth2/block_producer/src/lib.rs @@ -134,7 +134,7 @@ impl BlockProducer return Ok(PollOutcome::SignerRejection(slot)), Some(signature) => signature, } @@ -168,7 +168,7 @@ impl BlockProducer None, Some(signature) => { diff --git a/eth2/block_producer/src/test_utils/local_signer.rs b/eth2/block_producer/src/test_utils/local_signer.rs index 0ebefa29d..d7f490c30 100644 --- a/eth2/block_producer/src/test_utils/local_signer.rs +++ b/eth2/block_producer/src/test_utils/local_signer.rs @@ -25,11 +25,11 @@ impl LocalSigner { } impl Signer for LocalSigner { - fn sign_block_proposal(&self, message: &[u8]) -> Option { - Some(Signature::new(message, &self.keypair.sk)) + fn sign_block_proposal(&self, message: &[u8], domain: u64) -> Option { + Some(Signature::new(message, domain, &self.keypair.sk)) } - fn sign_randao_reveal(&self, message: &[u8]) -> Option { - Some(Signature::new(message, &self.keypair.sk)) + fn sign_randao_reveal(&self, message: &[u8], domain: u64) -> Option { + Some(Signature::new(message, domain, &self.keypair.sk)) } } diff --git a/eth2/block_producer/src/traits.rs b/eth2/block_producer/src/traits.rs index 5eb27bce7..c6e57d833 100644 --- a/eth2/block_producer/src/traits.rs +++ b/eth2/block_producer/src/traits.rs @@ -44,6 +44,6 @@ pub trait DutiesReader: Send + Sync { /// Signs message using an internally-maintained private key. pub trait Signer { - fn sign_block_proposal(&self, message: &[u8]) -> Option; - fn sign_randao_reveal(&self, message: &[u8]) -> Option; + fn sign_block_proposal(&self, message: &[u8], domain: u64) -> Option; + fn sign_randao_reveal(&self, message: &[u8], domain: u64) -> Option; } diff --git a/eth2/fork_choice/src/optimised_lmd_ghost.rs b/eth2/fork_choice/src/optimised_lmd_ghost.rs index 7104834cb..6b73c2a8f 100644 --- a/eth2/fork_choice/src/optimised_lmd_ghost.rs +++ b/eth2/fork_choice/src/optimised_lmd_ghost.rs @@ -31,7 +31,8 @@ use std::collections::HashMap; use std::sync::Arc; use types::{ readers::BeaconBlockReader, - slot_epoch_height::{Height, Slot}, + slot_epoch::Slot, + slot_height::SlotHeight, validator_registry::get_active_validator_indices, BeaconBlock, Hash256, }; @@ -77,7 +78,7 @@ pub struct OptimisedLMDGhost { block_store: Arc>, /// State storage access. state_store: Arc>, - max_known_height: Height, + max_known_height: SlotHeight, } impl OptimisedLMDGhost @@ -93,7 +94,7 @@ where ancestors: vec![HashMap::new(); 16], latest_attestation_targets: HashMap::new(), children: HashMap::new(), - max_known_height: Height::new(0), + max_known_height: SlotHeight::new(0), block_store, state_store, } @@ -137,7 +138,7 @@ where } /// Gets the ancestor at a given height `at_height` of a block specified by `block_hash`. - fn get_ancestor(&mut self, block_hash: Hash256, at_height: Height) -> Option { + fn get_ancestor(&mut self, block_hash: Hash256, at_height: SlotHeight) -> Option { // return None if we can't get the block from the db. let block_height = { let block_slot = self @@ -186,7 +187,7 @@ where fn get_clear_winner( &mut self, latest_votes: &HashMap, - block_height: Height, + block_height: SlotHeight, ) -> Option { // map of vote counts for every hash at this height let mut current_votes: HashMap = HashMap::new(); diff --git a/eth2/fork_choice/src/slow_lmd_ghost.rs b/eth2/fork_choice/src/slow_lmd_ghost.rs index e0e347cef..44b429fa8 100644 --- a/eth2/fork_choice/src/slow_lmd_ghost.rs +++ b/eth2/fork_choice/src/slow_lmd_ghost.rs @@ -29,7 +29,7 @@ use std::collections::HashMap; use std::sync::Arc; use types::{ readers::{BeaconBlockReader, BeaconStateReader}, - slot_epoch_height::Slot, + slot_epoch::Slot, validator_registry::get_active_validator_indices, BeaconBlock, Hash256, }; diff --git a/eth2/state_processing/src/block_processable.rs b/eth2/state_processing/src/block_processable.rs index f043a723d..1bf6022ec 100644 --- a/eth2/state_processing/src/block_processable.rs +++ b/eth2/state_processing/src/block_processable.rs @@ -374,14 +374,12 @@ fn validate_attestation_signature_optional( Ok(()) } -fn get_domain(_fork: &Fork, _epoch: Epoch, _domain_type: u64) -> u64 { - // TODO: stubbed out. - 0 +fn get_domain(fork: &Fork, epoch: Epoch, domain_type: u64) -> u64 { + fork.get_domain(epoch, domain_type) } -fn bls_verify(pubkey: &PublicKey, message: &[u8], signature: &Signature, _domain: u64) -> bool { - // TODO: add domain - signature.verify(message, pubkey) +fn bls_verify(pubkey: &PublicKey, message: &[u8], signature: &Signature, domain: u64) -> bool { + signature.verify(message, domain, pubkey) } impl From for Error { diff --git a/eth2/types/src/attestation.rs b/eth2/types/src/attestation.rs index eb375d490..be0b12d9e 100644 --- a/eth2/types/src/attestation.rs +++ b/eth2/types/src/attestation.rs @@ -25,11 +25,10 @@ impl Attestation { &self, group_public_key: &AggregatePublicKey, custody_bit: bool, - // TODO: use domain. - _domain: u64, + domain: u64, ) -> bool { self.aggregate_signature - .verify(&self.signable_message(custody_bit), group_public_key) + .verify(&self.signable_message(custody_bit), domain, group_public_key) } } diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index ed53bfea9..278569609 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -1,10 +1,9 @@ use crate::test_utils::TestRandom; use crate::{ validator::StatusFlags, validator_registry::get_active_validator_indices, AttestationData, - Bitfield, ChainSpec, Crosslink, Deposit, Epoch, Eth1Data, Eth1DataVote, Fork, Hash256, + Bitfield, ChainSpec, Crosslink, Deposit, DepositInput, Epoch, Eth1Data, Eth1DataVote, Fork, Hash256, PendingAttestation, PublicKey, Signature, Slot, Validator, }; -use bls::verify_proof_of_possession; use honey_badger_split::SplitExt; use rand::RngCore; use serde_derive::Serialize; @@ -587,6 +586,32 @@ impl BeaconState { self.validator_registry_update_epoch = current_epoch; } + + /// Confirm validator owns PublicKey + pub fn validate_proof_of_possession( + &self, + pubkey: PublicKey, + proof_of_possession: Signature, + withdrawal_credentials: Hash256, + spec: &ChainSpec + ) -> bool { + let proof_of_possession_data = DepositInput { + pubkey: pubkey.clone(), + withdrawal_credentials, + proof_of_possession: proof_of_possession.clone(), + }; + + proof_of_possession.verify( + &proof_of_possession_data.hash_tree_root(), + self.fork.get_domain( + self.slot.epoch(spec.epoch_length), + spec.domain_deposit, + ), + &pubkey, + ) + } + + /// Process a validator deposit, returning the validator index if the deposit is valid. /// /// Spec v0.2.0 @@ -598,8 +623,7 @@ impl BeaconState { withdrawal_credentials: Hash256, spec: &ChainSpec, ) -> Result { - // TODO: ensure verify proof-of-possession represents the spec accurately. - if !verify_proof_of_possession(&proof_of_possession, &pubkey) { + if !self.validate_proof_of_possession(pubkey.clone(), proof_of_possession, withdrawal_credentials, &spec) { return Err(()); } diff --git a/eth2/types/src/fork.rs b/eth2/types/src/fork.rs index 1c96a34ac..c103a2653 100644 --- a/eth2/types/src/fork.rs +++ b/eth2/types/src/fork.rs @@ -10,6 +10,22 @@ pub struct Fork { pub epoch: Epoch, } +impl Fork { + /// Return the fork version of the given ``epoch``. + pub fn get_fork_version(&self, epoch: Epoch) -> u64 { + if epoch < self.epoch { + return self.previous_version; + } + self.current_version + } + + /// Get the domain number that represents the fork meta and signature domain. + pub fn get_domain(&self, epoch: Epoch, domain_type: u64) -> u64 { + let fork_version = self.get_fork_version(epoch); + fork_version * u64::pow(2,32) + domain_type + } +} + impl Encodable for Fork { fn ssz_append(&self, s: &mut SszStream) { s.append(&self.previous_version); diff --git a/eth2/types/src/test_utils/signature.rs b/eth2/types/src/test_utils/signature.rs index 9ec7aec60..d9995835a 100644 --- a/eth2/types/src/test_utils/signature.rs +++ b/eth2/types/src/test_utils/signature.rs @@ -8,6 +8,6 @@ impl TestRandom for Signature { let mut message = vec![0; 32]; rng.fill_bytes(&mut message); - Signature::new(&message, &secret_key) + Signature::new(&message, 0, &secret_key) } } diff --git a/eth2/utils/bls/Cargo.toml b/eth2/utils/bls/Cargo.toml index 465510c59..c8204ca7a 100644 --- a/eth2/utils/bls/Cargo.toml +++ b/eth2/utils/bls/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Paul Hauner "] edition = "2018" [dependencies] -bls-aggregates = { git = "https://github.com/sigp/signature-schemes", tag = "v0.3.0" } +bls-aggregates = { git = "https://github.com/sigp/signature-schemes", tag = "0.4.1" } hashing = { path = "../hashing" } hex = "0.3" serde = "1.0" diff --git a/eth2/utils/bls/src/aggregate_signature.rs b/eth2/utils/bls/src/aggregate_signature.rs index 6fed183f0..b684c2b5b 100644 --- a/eth2/utils/bls/src/aggregate_signature.rs +++ b/eth2/utils/bls/src/aggregate_signature.rs @@ -27,8 +27,8 @@ impl AggregateSignature { /// /// Only returns `true` if the set of keys in the `AggregatePublicKey` match the set of keys /// that signed the `AggregateSignature`. - pub fn verify(&self, msg: &[u8], aggregate_public_key: &AggregatePublicKey) -> bool { - self.0.verify(msg, aggregate_public_key) + pub fn verify(&self, msg: &[u8], domain: u64, aggregate_public_key: &AggregatePublicKey) -> bool { + self.0.verify(msg, domain, aggregate_public_key) } } @@ -73,7 +73,7 @@ mod tests { let keypair = Keypair::random(); let mut original = AggregateSignature::new(); - original.add(&Signature::new(&[42, 42], &keypair.sk)); + original.add(&Signature::new(&[42, 42], 0, &keypair.sk)); let bytes = ssz_encode(&original); let (decoded, _) = AggregateSignature::ssz_decode(&bytes, 0).unwrap(); diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index 646047d18..39d4a95f2 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -29,24 +29,18 @@ fn extend_if_needed(hash: &mut Vec) { /// For some signature and public key, ensure that the signature message was the public key and it /// was signed by the secret key that corresponds to that public key. -pub fn verify_proof_of_possession(sig: &Signature, pubkey: &PublicKey) -> bool { - let mut hash = hash(&ssz_encode(pubkey)); - extend_if_needed(&mut hash); - sig.verify_hashed(&hash, &pubkey) -} + pub fn create_proof_of_possession(keypair: &Keypair) -> Signature { - let mut hash = hash(&ssz_encode(&keypair.pk)); - extend_if_needed(&mut hash); - Signature::new_hashed(&hash, &keypair.sk) + Signature::new(&ssz_encode(&keypair.pk), 0, &keypair.sk) } pub fn bls_verify_aggregate( pubkey: &AggregatePublicKey, message: &[u8], signature: &AggregateSignature, - _domain: u64, + domain: u64, ) -> bool { // TODO: add domain - signature.verify(message, pubkey) + signature.verify(message, domain, pubkey) } diff --git a/eth2/utils/bls/src/signature.rs b/eth2/utils/bls/src/signature.rs index 396e4eab7..61440498e 100644 --- a/eth2/utils/bls/src/signature.rs +++ b/eth2/utils/bls/src/signature.rs @@ -14,24 +14,24 @@ pub struct Signature(RawSignature); impl Signature { /// Instantiate a new Signature from a message and a SecretKey. - pub fn new(msg: &[u8], sk: &SecretKey) -> Self { - Signature(RawSignature::new(msg, sk.as_raw())) + pub fn new(msg: &[u8], domain: u64, sk: &SecretKey) -> Self { + Signature(RawSignature::new(msg, domain, sk.as_raw())) } /// Instantiate a new Signature from a message and a SecretKey, where the message has already /// been hashed. - pub fn new_hashed(msg_hashed: &[u8], sk: &SecretKey) -> Self { - Signature(RawSignature::new_hashed(msg_hashed, sk.as_raw())) + pub fn new_hashed(x_real_hashed: &[u8], x_imaginary_hashed: &[u8], sk: &SecretKey) -> Self { + Signature(RawSignature::new_hashed(x_real_hashed, x_imaginary_hashed, sk.as_raw())) } /// Verify the Signature against a PublicKey. - pub fn verify(&self, msg: &[u8], pk: &PublicKey) -> bool { - self.0.verify(msg, pk.as_raw()) + pub fn verify(&self, msg: &[u8], domain: u64, pk: &PublicKey) -> bool { + self.0.verify(msg, domain, pk.as_raw()) } /// Verify the Signature against a PublicKey, where the message has already been hashed. - pub fn verify_hashed(&self, msg_hash: &[u8], pk: &PublicKey) -> bool { - self.0.verify_hashed(msg_hash, pk.as_raw()) + pub fn verify_hashed(&self, x_real_hashed: &[u8], x_imaginary_hashed: &[u8], pk: &PublicKey) -> bool { + self.0.verify_hashed(x_real_hashed, x_imaginary_hashed, pk.as_raw()) } /// Returns the underlying signature. @@ -41,7 +41,7 @@ impl Signature { /// Returns a new empty signature. pub fn empty_signature() -> Self { - let empty: Vec = vec![0; 97]; + let empty: Vec = vec![0; 96]; Signature(RawSignature::from_bytes(&empty).unwrap()) } } @@ -85,7 +85,7 @@ mod tests { pub fn test_ssz_round_trip() { let keypair = Keypair::random(); - let original = Signature::new(&[42, 42], &keypair.sk); + let original = Signature::new(&[42, 42], 0, &keypair.sk); let bytes = ssz_encode(&original); let (decoded, _) = Signature::ssz_decode(&bytes, 0).unwrap(); From 9c4a1f1d1f6814dee9f1761ec2ae4cbb815c8ca7 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Mon, 18 Feb 2019 10:50:40 +1100 Subject: [PATCH 2/5] Update to signature-scheme 0.5.2 --- .../src/attestation_aggregator.rs | 9 +++--- eth2/attester/src/lib.rs | 6 ++-- eth2/block_producer/src/lib.rs | 13 ++++---- eth2/fork_choice/src/optimised_lmd_ghost.rs | 7 ++--- eth2/fork_choice/src/protolambda_lmd_ghost.rs | 1 + eth2/types/src/attestation.rs | 7 +++-- eth2/types/src/beacon_state.rs | 20 +++++++------ eth2/types/src/fork.rs | 2 +- eth2/utils/bls/Cargo.toml | 2 +- eth2/utils/bls/src/aggregate_signature.rs | 7 ++++- eth2/utils/bls/src/lib.rs | 1 - eth2/utils/bls/src/signature.rs | 30 ++++++++++++++----- 12 files changed, 67 insertions(+), 38 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index e8576276c..abedf62f6 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -110,10 +110,11 @@ impl AttestationAggregator { Message::BadValidatorIndex ); - if !free_attestation - .signature - .verify(&signable_message, spec.domain_attestation, &validator_record.pubkey) - { + if !free_attestation.signature.verify( + &signable_message, + spec.domain_attestation, + &validator_record.pubkey, + ) { return Ok(Outcome { valid: false, message: Message::BadSignature, diff --git a/eth2/attester/src/lib.rs b/eth2/attester/src/lib.rs index f2bbd6db3..13a1d72bb 100644 --- a/eth2/attester/src/lib.rs +++ b/eth2/attester/src/lib.rs @@ -137,8 +137,10 @@ impl Attester Option { self.store_produce(attestation_data); - self.signer - .sign_attestation_message(&attestation_data.signable_message(PHASE_0_CUSTODY_BIT)[..], DOMAIN_ATTESTATION) + self.signer.sign_attestation_message( + &attestation_data.signable_message(PHASE_0_CUSTODY_BIT)[..], + DOMAIN_ATTESTATION, + ) } /// Returns `true` if signing some attestation_data is safe (non-slashable). diff --git a/eth2/block_producer/src/lib.rs b/eth2/block_producer/src/lib.rs index e5651780a..fefaa7c04 100644 --- a/eth2/block_producer/src/lib.rs +++ b/eth2/block_producer/src/lib.rs @@ -134,7 +134,10 @@ impl BlockProducer return Ok(PollOutcome::SignerRejection(slot)), Some(signature) => signature, } @@ -166,10 +169,10 @@ impl BlockProducer Option { self.store_produce(&block); - match self - .signer - .sign_block_proposal(&block.proposal_root(&self.spec)[..], self.spec.domain_proposal) - { + match self.signer.sign_block_proposal( + &block.proposal_root(&self.spec)[..], + self.spec.domain_proposal, + ) { None => None, Some(signature) => { block.signature = signature; diff --git a/eth2/fork_choice/src/optimised_lmd_ghost.rs b/eth2/fork_choice/src/optimised_lmd_ghost.rs index 6b73c2a8f..dcf9c8380 100644 --- a/eth2/fork_choice/src/optimised_lmd_ghost.rs +++ b/eth2/fork_choice/src/optimised_lmd_ghost.rs @@ -30,11 +30,8 @@ use fast_math::log2_raw; use std::collections::HashMap; use std::sync::Arc; use types::{ - readers::BeaconBlockReader, - slot_epoch::Slot, - slot_height::SlotHeight, - validator_registry::get_active_validator_indices, - BeaconBlock, Hash256, + readers::BeaconBlockReader, slot_epoch::Slot, slot_height::SlotHeight, + validator_registry::get_active_validator_indices, BeaconBlock, Hash256, }; //TODO: Pruning - Children diff --git a/eth2/fork_choice/src/protolambda_lmd_ghost.rs b/eth2/fork_choice/src/protolambda_lmd_ghost.rs index e69de29bb..8b1378917 100644 --- a/eth2/fork_choice/src/protolambda_lmd_ghost.rs +++ b/eth2/fork_choice/src/protolambda_lmd_ghost.rs @@ -0,0 +1 @@ + diff --git a/eth2/types/src/attestation.rs b/eth2/types/src/attestation.rs index be0b12d9e..2c4281fff 100644 --- a/eth2/types/src/attestation.rs +++ b/eth2/types/src/attestation.rs @@ -27,8 +27,11 @@ impl Attestation { custody_bit: bool, domain: u64, ) -> bool { - self.aggregate_signature - .verify(&self.signable_message(custody_bit), domain, group_public_key) + self.aggregate_signature.verify( + &self.signable_message(custody_bit), + domain, + group_public_key, + ) } } diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 278569609..34d0a5a1f 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -1,8 +1,8 @@ use crate::test_utils::TestRandom; use crate::{ validator::StatusFlags, validator_registry::get_active_validator_indices, AttestationData, - Bitfield, ChainSpec, Crosslink, Deposit, DepositInput, Epoch, Eth1Data, Eth1DataVote, Fork, Hash256, - PendingAttestation, PublicKey, Signature, Slot, Validator, + Bitfield, ChainSpec, Crosslink, Deposit, DepositInput, Epoch, Eth1Data, Eth1DataVote, Fork, + Hash256, PendingAttestation, PublicKey, Signature, Slot, Validator, }; use honey_badger_split::SplitExt; use rand::RngCore; @@ -593,7 +593,7 @@ impl BeaconState { pubkey: PublicKey, proof_of_possession: Signature, withdrawal_credentials: Hash256, - spec: &ChainSpec + spec: &ChainSpec, ) -> bool { let proof_of_possession_data = DepositInput { pubkey: pubkey.clone(), @@ -603,15 +603,12 @@ impl BeaconState { proof_of_possession.verify( &proof_of_possession_data.hash_tree_root(), - self.fork.get_domain( - self.slot.epoch(spec.epoch_length), - spec.domain_deposit, - ), + self.fork + .get_domain(self.slot.epoch(spec.epoch_length), spec.domain_deposit), &pubkey, ) } - /// Process a validator deposit, returning the validator index if the deposit is valid. /// /// Spec v0.2.0 @@ -623,7 +620,12 @@ impl BeaconState { withdrawal_credentials: Hash256, spec: &ChainSpec, ) -> Result { - if !self.validate_proof_of_possession(pubkey.clone(), proof_of_possession, withdrawal_credentials, &spec) { + if !self.validate_proof_of_possession( + pubkey.clone(), + proof_of_possession, + withdrawal_credentials, + &spec, + ) { return Err(()); } diff --git a/eth2/types/src/fork.rs b/eth2/types/src/fork.rs index c103a2653..67a8c90eb 100644 --- a/eth2/types/src/fork.rs +++ b/eth2/types/src/fork.rs @@ -22,7 +22,7 @@ impl Fork { /// Get the domain number that represents the fork meta and signature domain. pub fn get_domain(&self, epoch: Epoch, domain_type: u64) -> u64 { let fork_version = self.get_fork_version(epoch); - fork_version * u64::pow(2,32) + domain_type + fork_version * u64::pow(2, 32) + domain_type } } diff --git a/eth2/utils/bls/Cargo.toml b/eth2/utils/bls/Cargo.toml index c8204ca7a..7a436307b 100644 --- a/eth2/utils/bls/Cargo.toml +++ b/eth2/utils/bls/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Paul Hauner "] edition = "2018" [dependencies] -bls-aggregates = { git = "https://github.com/sigp/signature-schemes", tag = "0.4.1" } +bls-aggregates = { git = "https://github.com/sigp/signature-schemes", tag = "0.5.2" } hashing = { path = "../hashing" } hex = "0.3" serde = "1.0" diff --git a/eth2/utils/bls/src/aggregate_signature.rs b/eth2/utils/bls/src/aggregate_signature.rs index b684c2b5b..8463b26b3 100644 --- a/eth2/utils/bls/src/aggregate_signature.rs +++ b/eth2/utils/bls/src/aggregate_signature.rs @@ -27,7 +27,12 @@ impl AggregateSignature { /// /// Only returns `true` if the set of keys in the `AggregatePublicKey` match the set of keys /// that signed the `AggregateSignature`. - pub fn verify(&self, msg: &[u8], domain: u64, aggregate_public_key: &AggregatePublicKey) -> bool { + pub fn verify( + &self, + msg: &[u8], + domain: u64, + aggregate_public_key: &AggregatePublicKey, + ) -> bool { self.0.verify(msg, domain, aggregate_public_key) } } diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index 39d4a95f2..074929b32 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -30,7 +30,6 @@ fn extend_if_needed(hash: &mut Vec) { /// For some signature and public key, ensure that the signature message was the public key and it /// was signed by the secret key that corresponds to that public key. - pub fn create_proof_of_possession(keypair: &Keypair) -> Signature { Signature::new(&ssz_encode(&keypair.pk), 0, &keypair.sk) } diff --git a/eth2/utils/bls/src/signature.rs b/eth2/utils/bls/src/signature.rs index 61440498e..23b0c0834 100644 --- a/eth2/utils/bls/src/signature.rs +++ b/eth2/utils/bls/src/signature.rs @@ -21,7 +21,11 @@ impl Signature { /// Instantiate a new Signature from a message and a SecretKey, where the message has already /// been hashed. pub fn new_hashed(x_real_hashed: &[u8], x_imaginary_hashed: &[u8], sk: &SecretKey) -> Self { - Signature(RawSignature::new_hashed(x_real_hashed, x_imaginary_hashed, sk.as_raw())) + Signature(RawSignature::new_hashed( + x_real_hashed, + x_imaginary_hashed, + sk.as_raw(), + )) } /// Verify the Signature against a PublicKey. @@ -30,8 +34,14 @@ impl Signature { } /// Verify the Signature against a PublicKey, where the message has already been hashed. - pub fn verify_hashed(&self, x_real_hashed: &[u8], x_imaginary_hashed: &[u8], pk: &PublicKey) -> bool { - self.0.verify_hashed(x_real_hashed, x_imaginary_hashed, pk.as_raw()) + pub fn verify_hashed( + &self, + x_real_hashed: &[u8], + x_imaginary_hashed: &[u8], + pk: &PublicKey, + ) -> bool { + self.0 + .verify_hashed(x_real_hashed, x_imaginary_hashed, pk.as_raw()) } /// Returns the underlying signature. @@ -41,7 +51,9 @@ impl Signature { /// Returns a new empty signature. pub fn empty_signature() -> Self { - let empty: Vec = vec![0; 96]; + let mut empty: Vec = vec![0; 96]; + // TODO: Modify the way flags are used (b_flag should not be used for empty_signature in the future) + empty[0] += u8::pow(2, 6); Signature(RawSignature::from_bytes(&empty).unwrap()) } } @@ -99,9 +111,13 @@ mod tests { let sig_as_bytes: Vec = sig.as_raw().as_bytes(); - assert_eq!(sig_as_bytes.len(), 97); - for one_byte in sig_as_bytes.iter() { - assert_eq!(*one_byte, 0); + assert_eq!(sig_as_bytes.len(), 96); + for (i, one_byte) in sig_as_bytes.iter().enumerate() { + if i == 0 { + assert_eq!(*one_byte, u8::pow(2, 6)); + } else { + assert_eq!(*one_byte, 0); + } } } } From 21d75f18536aec031dc772a86cf57bae20f8abd2 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Mon, 18 Feb 2019 12:06:47 +1100 Subject: [PATCH 3/5] Use verify_proof_of_possession --- eth2/types/src/beacon_state.rs | 19 +++++++++++++------ eth2/utils/bls/src/lib.rs | 10 ++++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 18b5fc989..2bdf85fcc 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -4,6 +4,7 @@ use crate::{ Bitfield, ChainSpec, Crosslink, Deposit, DepositInput, Epoch, Eth1Data, Eth1DataVote, Fork, Hash256, PendingAttestation, PublicKey, Signature, Slot, Validator, }; +use bls::verify_proof_of_possession; use honey_badger_split::SplitExt; use log::trace; use rand::RngCore; @@ -389,6 +390,7 @@ impl BeaconState { &self, slot: Slot, registry_change: bool, + spec: &ChainSpec, ) -> Result, u64)>, BeaconStateError> { let epoch = slot.epoch(spec.epoch_length); @@ -668,12 +670,17 @@ impl BeaconState { withdrawal_credentials: Hash256, spec: &ChainSpec, ) -> Result { - if !self.validate_proof_of_possession( - pubkey.clone(), - proof_of_possession, - withdrawal_credentials, - &spec, - ) { + // TODO: update proof of possession to function written above ( + // requires bls::create_proof_of_possession to be updated + // https://github.com/sigp/lighthouse/issues/239 + if !verify_proof_of_possession(&proof_of_possession, &pubkey) + //if !self.validate_proof_of_possession( + // pubkey.clone(), + // proof_of_possession, + // withdrawal_credentials, + // &spec, + // ) + { return Err(()); } diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index 074929b32..4d0864a90 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -16,7 +16,7 @@ pub use crate::signature::Signature; pub use self::bls_aggregates::AggregatePublicKey; -pub const BLS_AGG_SIG_BYTE_SIZE: usize = 97; +pub const BLS_AGG_SIG_BYTE_SIZE: usize = 96; use hashing::hash; use ssz::ssz_encode; @@ -29,7 +29,14 @@ fn extend_if_needed(hash: &mut Vec) { /// For some signature and public key, ensure that the signature message was the public key and it /// was signed by the secret key that corresponds to that public key. +pub fn verify_proof_of_possession(sig: &Signature, pubkey: &PublicKey) -> bool { + // TODO: replace this function with state.validate_proof_of_possession + // https://github.com/sigp/lighthouse/issues/239 + sig.verify(&ssz_encode(pubkey), 0, &pubkey) +} +// TODO: Update this method +// https://github.com/sigp/lighthouse/issues/239 pub fn create_proof_of_possession(keypair: &Keypair) -> Signature { Signature::new(&ssz_encode(&keypair.pk), 0, &keypair.sk) } @@ -40,6 +47,5 @@ pub fn bls_verify_aggregate( signature: &AggregateSignature, domain: u64, ) -> bool { - // TODO: add domain signature.verify(message, domain, pubkey) } From 9f9b466f95a12867f0b4bb858fc2eab8b769f30b Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Sat, 23 Feb 2019 14:39:54 +1100 Subject: [PATCH 4/5] Modify attestion_aggregation to use frok version in domain --- beacon_node/beacon_chain/src/attestation_aggregator.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index d5b8c090f..d70d732f8 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -114,7 +114,10 @@ impl AttestationAggregator { if !free_attestation.signature.verify( &signable_message, - spec.domain_attestation, + cached_state.state.fork.get_domain( + cached_state.state.current_epoch(spec), + spec.domain_attestation, + ), &validator_record.pubkey, ) { return Ok(Outcome { From 4c3b0a65753d72c690652af20ef8b2f2f5c74ba5 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Mon, 25 Feb 2019 10:38:04 +1100 Subject: [PATCH 5/5] Formatting --- .../beacon_chain/src/attestation_aggregator.rs | 18 +++++++----------- eth2/types/src/beacon_state.rs | 4 ++-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index 3aa312c84..54f178068 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -129,17 +129,13 @@ impl AttestationAggregator { Some(validator_record) => validator_record, }; - if !free_attestation - .signature - .verify( - &signable_message, - cached_state.fork.get_domain( - cached_state.current_epoch(spec), - spec.domain_attestation, - ), - &validator_record.pubkey, - ) - { + if !free_attestation.signature.verify( + &signable_message, + cached_state + .fork + .get_domain(cached_state.current_epoch(spec), spec.domain_attestation), + &validator_record.pubkey, + ) { invalid_outcome!(Message::BadSignature); } diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index b8e9e1689..85b49bf01 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -2,8 +2,8 @@ use self::epoch_cache::EpochCache; use crate::test_utils::TestRandom; use crate::{ validator::StatusFlags, validator_registry::get_active_validator_indices, AttestationData, - Bitfield, ChainSpec, Crosslink, Deposit, DepositData, DepositInput, Epoch, Eth1Data, Eth1DataVote, Fork, - Hash256, PendingAttestation, PublicKey, Signature, Slot, Validator, + Bitfield, ChainSpec, Crosslink, Deposit, DepositData, DepositInput, Epoch, Eth1Data, + Eth1DataVote, Fork, Hash256, PendingAttestation, PublicKey, Signature, Slot, Validator, }; use bls::verify_proof_of_possession; use honey_badger_split::SplitExt;