diff --git a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs index d2274ac69..ea32e177d 100644 --- a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs +++ b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs @@ -115,7 +115,16 @@ impl BeaconChainHarness { ); } else { debug!("Generating initial validator deposits..."); - let deposits = generate_deposits_from_keypairs(&keypairs, genesis_time, &spec); + let deposits = generate_deposits_from_keypairs( + &keypairs, + genesis_time, + spec.get_domain(spec.genesis_epoch, Domain::Deposit, &Fork{ + previous_version: spec.genesis_fork_version, + current_version: spec.genesis_fork_version, + epoch: spec.genesis_epoch, + }), + &spec + ); state_builder.process_initial_deposits(&deposits, &spec); }; diff --git a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs index f2d68d644..2baf8984f 100644 --- a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs +++ b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs @@ -1,4 +1,4 @@ -use bls::{create_proof_of_possession, get_withdrawal_credentials}; +use bls::get_withdrawal_credentials; use int_to_bytes::int_to_bytes48; use log::debug; use rayon::prelude::*; @@ -34,6 +34,7 @@ pub fn generate_deterministic_keypairs(validator_count: usize) -> Vec { pub fn generate_deposits_from_keypairs( keypairs: &[Keypair], genesis_time: u64, + domain: u64, spec: &ChainSpec, ) -> Vec { debug!( @@ -44,24 +45,23 @@ pub fn generate_deposits_from_keypairs( let initial_validator_deposits = keypairs .par_iter() - .map(|keypair| Deposit { - branch: vec![], // branch verification is not specified. - index: 0, // index verification is not specified. - deposit_data: DepositData { - amount: 32_000_000_000, // 32 ETH (in Gwei) - timestamp: genesis_time - 1, - deposit_input: DepositInput { - pubkey: keypair.pk.clone(), - // Validator can withdraw using their main keypair. - withdrawal_credentials: Hash256::from_slice( - &get_withdrawal_credentials( - &keypair.pk, - spec.bls_withdrawal_prefix_byte, - )[..], - ), - proof_of_possession: create_proof_of_possession(&keypair), + .map(|keypair| { + let withdrawal_credentials = Hash256::from_slice( + &get_withdrawal_credentials(&keypair.pk, spec.bls_withdrawal_prefix_byte)[..]); + Deposit { + branch: vec![], // branch verification is not specified. + index: 0, // index verification is not specified. + deposit_data: DepositData { + amount: 32_000_000_000, // 32 ETH (in Gwei) + timestamp: genesis_time - 1, + deposit_input: DepositInput { + pubkey: keypair.pk.clone(), + // Validator can withdraw using their main keypair. + withdrawal_credentials: withdrawal_credentials.clone(), + proof_of_possession: DepositInput::create_proof_of_possession(&keypair, &withdrawal_credentials, domain), + }, }, - }, + } }) .collect(); diff --git a/beacon_node/beacon_chain/test_harness/src/test_case.rs b/beacon_node/beacon_chain/test_harness/src/test_case.rs index 7bc7161a8..32a16ff80 100644 --- a/beacon_node/beacon_chain/test_harness/src/test_case.rs +++ b/beacon_node/beacon_chain/test_harness/src/test_case.rs @@ -3,7 +3,7 @@ use crate::beacon_chain_harness::BeaconChainHarness; use beacon_chain::CheckPoint; -use bls::{create_proof_of_possession, get_withdrawal_credentials}; +use bls::get_withdrawal_credentials; use log::{info, warn}; use ssz::SignedRoot; use std::path::Path; @@ -258,11 +258,19 @@ fn build_deposit( index_offset: u64, ) -> (Deposit, Keypair) { let keypair = Keypair::random(); - let proof_of_possession = create_proof_of_possession(&keypair); - let index = harness.beacon_chain.state.read().deposit_index + index_offset; let withdrawal_credentials = Hash256::from_slice( &get_withdrawal_credentials(&keypair.pk, harness.spec.bls_withdrawal_prefix_byte)[..], ); + let proof_of_possession = DepositInput::create_proof_of_possession( + &keypair, + &withdrawal_credentials, + harness.spec.get_domain( + harness.beacon_chain.state.read().current_epoch(&harness.spec), + Domain::Deposit, + &harness.beacon_chain.state.read().fork, + ) + ); + let index = harness.beacon_chain.state.read().deposit_index + index_offset; let deposit = Deposit { // Note: `branch` and `index` will need to be updated once the spec defines their diff --git a/beacon_node/src/main.rs b/beacon_node/src/main.rs index 7606da10d..8fdfa3446 100644 --- a/beacon_node/src/main.rs +++ b/beacon_node/src/main.rs @@ -8,7 +8,6 @@ use std::path::PathBuf; use crate::config::LighthouseConfig; use crate::rpc::start_server; use beacon_chain::BeaconChain; -use bls::create_proof_of_possession; use clap::{App, Arg}; use db::{ stores::{BeaconBlockStore, BeaconStateStore}, @@ -20,8 +19,8 @@ use slot_clock::SystemTimeSlotClock; use ssz::TreeHash; use std::sync::Arc; use types::{ - beacon_state::BeaconStateBuilder, BeaconBlock, ChainSpec, Deposit, DepositData, DepositInput, - Eth1Data, Hash256, Keypair, + beacon_state::BeaconStateBuilder, BeaconBlock, ChainSpec, Domain, Deposit, DepositData, DepositInput, + Eth1Data, Fork, Hash256, Keypair, }; fn main() { @@ -113,7 +112,20 @@ fn main() { deposit_input: DepositInput { pubkey: keypair.pk.clone(), withdrawal_credentials: Hash256::zero(), // Withdrawal not possible. - proof_of_possession: create_proof_of_possession(&keypair, Hash256::zero()), + proof_of_possession: DepositInput::create_proof_of_possession( + &keypair, + &Hash256::zero(), + spec.get_domain( + // Get domain from genesis fork_version + spec.genesis_epoch, + Domain::Deposit, + &Fork { + previous_version: spec.genesis_fork_version, + current_version: spec.genesis_fork_version, + epoch: spec.genesis_epoch, + } + ), + ), }, }, }) diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 603ae2f9d..f98b3e47e 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -1,7 +1,6 @@ use self::epoch_cache::EpochCache; use crate::test_utils::TestRandom; use crate::{validator_registry::get_active_validator_indices, *}; -use bls::verify_proof_of_possession; use helpers::*; use honey_badger_split::SplitExt; use int_to_bytes::int_to_bytes32; @@ -9,7 +8,7 @@ use log::{debug, error, trace}; use rand::RngCore; use rayon::prelude::*; use serde_derive::Serialize; -use ssz::{hash, Decodable, DecodeError, Encodable, SszStream, TreeHash}; +use ssz::{hash, Decodable, DecodeError, Encodable, SignedRoot, SszStream, TreeHash}; use std::collections::HashMap; use swap_or_not_shuffle::shuffle_list; @@ -590,10 +589,8 @@ impl BeaconState { for deposit_data in deposits { let result = self.process_deposit( - deposit_data.deposit_input.pubkey.clone(), + deposit_data.deposit_input.clone(), deposit_data.amount, - deposit_data.deposit_input.proof_of_possession.clone(), - deposit_data.deposit_input.withdrawal_credentials, Some(&pubkey_map), spec, ); @@ -616,18 +613,29 @@ impl BeaconState { /// Spec v0.4.0 pub fn process_deposit( &mut self, - pubkey: PublicKey, + deposit_input: DepositInput, amount: u64, - proof_of_possession: Signature, - withdrawal_credentials: Hash256, pubkey_map: Option<&HashMap>, spec: &ChainSpec, ) -> Result { - // - if !verify_proof_of_possession(&proof_of_possession, &pubkey) { - return Err(()); + + let proof_is_valid = deposit_input.proof_of_possession.verify( + &deposit_input.signed_root(), + spec.get_domain( + self.current_epoch(&spec), + Domain::Deposit, + &self.fork, + ), + &deposit_input.pubkey, + ); + + if !proof_is_valid { + return Err(()) } + let pubkey = deposit_input.pubkey.clone(); + let withdrawal_credentials = deposit_input.withdrawal_credentials.clone(); + let validator_index = if let Some(pubkey_map) = pubkey_map { pubkey_map.get(&pubkey).and_then(|i| Some(*i)) } else { @@ -1055,33 +1063,6 @@ impl BeaconState { self.validator_registry_update_epoch = current_epoch; } - /// Confirm validator owns PublicKey - /// - /// Spec v0.4.0 - 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: Signature::empty_signature(), - }; - - proof_of_possession.verify( - &proof_of_possession_data.hash_tree_root(), - spec.get_domain( - self.slot.epoch(spec.slots_per_epoch), - Domain::Deposit, - &self.fork, - ), - &pubkey, - ) - } - /// Iterate through the validator registry and eject active validators with balance below /// ``EJECTION_BALANCE``. /// diff --git a/eth2/types/src/deposit_input.rs b/eth2/types/src/deposit_input.rs index 32f57ab6e..bb2334672 100644 --- a/eth2/types/src/deposit_input.rs +++ b/eth2/types/src/deposit_input.rs @@ -1,21 +1,37 @@ use super::Hash256; use crate::test_utils::TestRandom; -use bls::{PublicKey, Signature}; +use bls::{Keypair, PublicKey, Signature}; use rand::RngCore; use serde_derive::{Deserialize, Serialize}; -use ssz_derive::{Decode, Encode, TreeHash}; +use ssz_derive::{Decode, Encode, SignedRoot, TreeHash}; +use ssz::{SignedRoot, TreeHash}; use test_random_derive::TestRandom; /// The data supplied by the user to the deposit contract. /// /// Spec v0.4.0 -#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, Encode, Decode, TreeHash, TestRandom)] +#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, Encode, Decode, SignedRoot, TreeHash, TestRandom)] pub struct DepositInput { pub pubkey: PublicKey, pub withdrawal_credentials: Hash256, pub proof_of_possession: Signature, } +impl DepositInput { + /// Generate the 'proof_of_posession' signature for a given DepositInput details. + /// + /// Spec v0.4.0 + pub fn create_proof_of_possession(keypair: &Keypair, withdrawal_credentials: &Hash256, domain: u64) -> Signature { + let signable_deposite_input = DepositInput { + pubkey: keypair.pk.clone(), + withdrawal_credentials: withdrawal_credentials.clone(), + proof_of_possession: Signature::empty_signature(), + }; + let msg = signable_deposite_input.signed_root(); + Signature::new(msg.as_slice(), domain, &keypair.sk) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/eth2/types/src/test_utils/testing_beacon_block_builder.rs b/eth2/types/src/test_utils/testing_beacon_block_builder.rs index db4d887d4..ad3667e41 100644 --- a/eth2/types/src/test_utils/testing_beacon_block_builder.rs +++ b/eth2/types/src/test_utils/testing_beacon_block_builder.rs @@ -153,12 +153,12 @@ impl TestingBeaconBlockBuilder { } /// Insert a `Valid` deposit into the state. - pub fn insert_deposit(&mut self, amount: u64, index: u64, spec: &ChainSpec) { + pub fn insert_deposit(&mut self, amount: u64, index: u64, domain: u64, spec: &ChainSpec) { let keypair = Keypair::random(); let mut builder = TestingDepositBuilder::new(amount); builder.set_index(index); - builder.sign(&keypair, spec); + builder.sign(&keypair, domain, spec); self.block.body.deposits.push(builder.build()) } diff --git a/eth2/types/src/test_utils/testing_deposit_builder.rs b/eth2/types/src/test_utils/testing_deposit_builder.rs index c7eadcfd1..4e754eab0 100644 --- a/eth2/types/src/test_utils/testing_deposit_builder.rs +++ b/eth2/types/src/test_utils/testing_deposit_builder.rs @@ -1,5 +1,5 @@ use crate::*; -use bls::{create_proof_of_possession, get_withdrawal_credentials}; +use bls::{get_withdrawal_credentials}; pub struct TestingDepositBuilder { deposit: Deposit, @@ -30,16 +30,16 @@ impl TestingDepositBuilder { self.deposit.index = index; } - pub fn sign(&mut self, keypair: &Keypair, spec: &ChainSpec) { + pub fn sign(&mut self, keypair: &Keypair, domain: u64, spec: &ChainSpec) { + let withdrawal_credentials = Hash256::from_slice(&get_withdrawal_credentials(&keypair.pk, spec.bls_withdrawal_prefix_byte)[..]); self.deposit.deposit_data.deposit_input.pubkey = keypair.pk.clone(); + self.deposit.deposit_data.deposit_input.withdrawal_credentials = withdrawal_credentials.clone(); self.deposit.deposit_data.deposit_input.proof_of_possession = - create_proof_of_possession(&keypair); - self.deposit - .deposit_data - .deposit_input - .withdrawal_credentials = Hash256::from_slice( - &get_withdrawal_credentials(&keypair.pk, spec.bls_withdrawal_prefix_byte)[..], - ); + DepositInput::create_proof_of_possession( + &keypair, + &withdrawal_credentials, + domain, + ); } pub fn build(self) -> Deposit { diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index 95f993ecb..4888ff567 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -20,7 +20,6 @@ pub const BLS_AGG_SIG_BYTE_SIZE: usize = 96; use hashing::hash; use ssz::ssz_encode; -use types::{DepositInput, Hash256}; /// 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. @@ -30,11 +29,6 @@ pub fn verify_proof_of_possession(sig: &Signature, pubkey: &PublicKey) -> bool { 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, withdrawal_credentials: &Hash256) -> Signature { - Signature::new(&ssz_encode(&keypair.pk), 0, &keypair.sk) -} /// Returns the withdrawal credentials for a given public key. pub fn get_withdrawal_credentials(pubkey: &PublicKey, prefix_byte: u8) -> Vec {