From e81f1c31c9890ae34be093fdbd11523853983902 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 11 Mar 2019 19:47:33 +1100 Subject: [PATCH] Fix proof-of-possession issues. These were introduced in an earlier commit --- .../benches/block_processing_benches.rs | 2 +- .../per_block_processing/verify_deposit.rs | 16 +++++++------- eth2/types/src/deposit_input.rs | 22 ++++++++++++++++--- .../testing_beacon_block_builder.rs | 10 +++++++-- .../src/test_utils/testing_deposit_builder.rs | 6 ++++- eth2/utils/bls/src/lib.rs | 8 ------- 6 files changed, 41 insertions(+), 23 deletions(-) diff --git a/eth2/state_processing/benches/block_processing_benches.rs b/eth2/state_processing/benches/block_processing_benches.rs index 06563b05b..9d9a5647b 100644 --- a/eth2/state_processing/benches/block_processing_benches.rs +++ b/eth2/state_processing/benches/block_processing_benches.rs @@ -140,7 +140,7 @@ fn build_block(state: &mut BeaconState, keypairs: &[Keypair], spec: &ChainSpec) // Insert the maximum possible number of `Deposit` objects. for i in 0..spec.max_deposits { - builder.insert_deposit(32_000_000_000, state.deposit_index + i, spec); + builder.insert_deposit(32_000_000_000, state.deposit_index + i, state, spec); } // Insert the maximum possible number of `Exit` objects. diff --git a/eth2/state_processing/src/per_block_processing/verify_deposit.rs b/eth2/state_processing/src/per_block_processing/verify_deposit.rs index 0cf2a078f..1aabbb973 100644 --- a/eth2/state_processing/src/per_block_processing/verify_deposit.rs +++ b/eth2/state_processing/src/per_block_processing/verify_deposit.rs @@ -1,5 +1,4 @@ use super::errors::{DepositInvalid as Invalid, DepositValidationError as Error}; -use bls::verify_proof_of_possession; use hashing::hash; use merkle_proof::verify_merkle_proof; use ssz::ssz_encode; @@ -27,13 +26,14 @@ pub fn verify_deposit( spec: &ChainSpec, ) -> Result<(), Error> { verify!( - // TODO: update proof of possession. - // - // https://github.com/sigp/lighthouse/issues/239 - verify_proof_of_possession( - &deposit.deposit_data.deposit_input.proof_of_possession, - &deposit.deposit_data.deposit_input.pubkey - ), + deposit + .deposit_data + .deposit_input + .validate_proof_of_possession( + state.slot.epoch(spec.slots_per_epoch), + &state.fork, + spec + ), Invalid::BadProofOfPossession ); diff --git a/eth2/types/src/deposit_input.rs b/eth2/types/src/deposit_input.rs index 2a61efd9c..1b506894d 100644 --- a/eth2/types/src/deposit_input.rs +++ b/eth2/types/src/deposit_input.rs @@ -1,5 +1,5 @@ -use super::Hash256; use crate::test_utils::TestRandom; +use crate::*; use bls::{Keypair, PublicKey, Signature}; use rand::RngCore; use serde_derive::{Deserialize, Serialize}; @@ -37,14 +37,30 @@ impl DepositInput { withdrawal_credentials: &Hash256, domain: u64, ) -> Signature { - let signable_deposite_input = DepositInput { + let signable_deposit_input = DepositInput { pubkey: keypair.pk.clone(), withdrawal_credentials: withdrawal_credentials.clone(), proof_of_possession: Signature::empty_signature(), }; - let msg = signable_deposite_input.signed_root(); + let msg = signable_deposit_input.signed_root(); + Signature::new(msg.as_slice(), domain, &keypair.sk) } + + /// Verify that proof-of-possession is valid. + /// + /// Spec v0.4.0 + pub fn validate_proof_of_possession( + &self, + epoch: Epoch, + fork: &Fork, + spec: &ChainSpec, + ) -> bool { + let msg = self.signed_root(); + let domain = spec.get_domain(epoch, Domain::Deposit, fork); + + self.proof_of_possession.verify(&msg, domain, &self.pubkey) + } } #[cfg(test)] 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 ad3667e41..bbdc5046b 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,18 @@ impl TestingBeaconBlockBuilder { } /// Insert a `Valid` deposit into the state. - pub fn insert_deposit(&mut self, amount: u64, index: u64, domain: u64, spec: &ChainSpec) { + pub fn insert_deposit( + &mut self, + amount: u64, + index: u64, + state: &BeaconState, + spec: &ChainSpec, + ) { let keypair = Keypair::random(); let mut builder = TestingDepositBuilder::new(amount); builder.set_index(index); - builder.sign(&keypair, domain, spec); + builder.sign(&keypair, state, 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 80e039a89..56e81cad0 100644 --- a/eth2/types/src/test_utils/testing_deposit_builder.rs +++ b/eth2/types/src/test_utils/testing_deposit_builder.rs @@ -30,10 +30,14 @@ impl TestingDepositBuilder { self.deposit.index = index; } - pub fn sign(&mut self, keypair: &Keypair, domain: u64, spec: &ChainSpec) { + pub fn sign(&mut self, keypair: &Keypair, state: &BeaconState, spec: &ChainSpec) { let withdrawal_credentials = Hash256::from_slice( &get_withdrawal_credentials(&keypair.pk, spec.bls_withdrawal_prefix_byte)[..], ); + + let epoch = state.current_epoch(spec); + let domain = spec.get_domain(epoch, Domain::Deposit, &state.fork); + self.deposit.deposit_data.deposit_input.pubkey = keypair.pk.clone(); self.deposit .deposit_data diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index b995b78c9..38a129908 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -21,14 +21,6 @@ pub const BLS_AGG_SIG_BYTE_SIZE: usize = 96; use hashing::hash; use ssz::ssz_encode; -/// 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) -} - /// Returns the withdrawal credentials for a given public key. pub fn get_withdrawal_credentials(pubkey: &PublicKey, prefix_byte: u8) -> Vec { let hashed = hash(&ssz_encode(pubkey));