Modify create_proof_of_possession, and verifying proof_of_possession in process_deposits

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);
         };
 

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<Keypair> {
 pub fn generate_deposits_from_keypairs(
     keypairs: &[Keypair],
     genesis_time: u64,
+    domain: u64,
     spec: &ChainSpec,
 ) -> Vec<Deposit> {
     debug!(
@@ -44,7 +45,10 @@ pub fn generate_deposits_from_keypairs(
     let initial_validator_deposits =
         keypairs
             .par_iter()
-            .map(|keypair| Deposit {
+            .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 {
@@ -53,15 +57,11 @@ pub fn generate_deposits_from_keypairs(
                         deposit_input: DepositInput {
                             pubkey: keypair.pk.clone(),
                             // Validator can withdraw using their main keypair.
-                        withdrawal_credentials: Hash256::from_slice(
+                            withdrawal_credentials: withdrawal_credentials.clone(),
-                            &get_withdrawal_credentials(
+                            proof_of_possession: DepositInput::create_proof_of_possession(&keypair, &withdrawal_credentials, domain),
-                                &keypair.pk,
-                                spec.bls_withdrawal_prefix_byte,
-                            )[..],
-                        ),
-                        proof_of_possession: create_proof_of_possession(&keypair),
                         },
                     },
+                }
             })
             .collect();
 

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

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,
+    beacon_state::BeaconStateBuilder, BeaconBlock, ChainSpec, Domain, Deposit, DepositData, DepositInput,
-    Eth1Data, Hash256, Keypair,
+    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,
+                            }
+                        ),
+                    ),
                 },
             },
         })

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<PublicKey, usize>>,
         spec: &ChainSpec,
     ) -> Result<usize, ()> {
-        //
+
-        if !verify_proof_of_possession(&proof_of_possession, &pubkey) {
+        let proof_is_valid = deposit_input.proof_of_possession.verify(
-            return Err(());
+            &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``.
     ///

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::*;

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())
     }

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,15 +30,15 @@ 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);
+            DepositInput::create_proof_of_possession(
-        self.deposit
+                &keypair,
-            .deposit_data
+                &withdrawal_credentials,
-            .deposit_input
+                domain,
-            .withdrawal_credentials = Hash256::from_slice(
-            &get_withdrawal_credentials(&keypair.pk, spec.bls_withdrawal_prefix_byte)[..],
             );
     }
 

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<u8> {