From 545fb100056e63bfaff4ec749d353146e0ee7176 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 21 May 2019 18:02:31 +1000 Subject: [PATCH] spec v0.6.1: deposit processing (WIP) --- .../src/per_block_processing.rs | 34 +++---- .../src/per_block_processing/errors.rs | 2 +- .../per_block_processing/verify_deposit.rs | 39 ++++---- eth2/types/src/deposit_input.rs | 92 ------------------- 4 files changed, 37 insertions(+), 130 deletions(-) delete mode 100644 eth2/types/src/deposit_input.rs diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index aae621072..c66b8baea 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -89,11 +89,9 @@ fn per_block_processing_signature_optional( process_proposer_slashings(&mut state, &block.body.proposer_slashings, spec)?; process_attester_slashings(&mut state, &block.body.attester_slashings, spec)?; process_attestations(&mut state, &block.body.attestations, spec)?; - /* process_deposits(&mut state, &block.body.deposits, spec)?; process_exits(&mut state, &block.body.voluntary_exits, spec)?; process_transfers(&mut state, &block.body.transfers, spec)?; - */ Ok(()) } @@ -350,21 +348,24 @@ pub fn process_attestations( Ok(()) } -/* /// Validates each `Deposit` and updates the state, short-circuiting on an invalid object. /// /// Returns `Ok(())` if the validation and state updates completed successfully, otherwise returns /// an `Err` describing the invalid object or cause of failure. /// -/// Spec v0.5.1 +/// Spec v0.6.1 pub fn process_deposits( state: &mut BeaconState, deposits: &[Deposit], spec: &ChainSpec, ) -> Result<(), Error> { verify!( - deposits.len() as u64 <= spec.max_deposits, - Invalid::MaxDepositsExceeded + deposits.len() as u64 + == std::cmp::min( + spec.max_deposits, + state.latest_eth1_data.deposit_count - state.deposit_index + ), + Invalid::DepositCountInvalid ); // Verify deposits in parallel. @@ -391,28 +392,28 @@ pub fn process_deposits( let validator_index = get_existing_validator_index(state, deposit).map_err(|e| e.into_with_index(i))?; - let deposit_data = &deposit.deposit_data; - let deposit_input = &deposit.deposit_data.deposit_input; + let amount = deposit.data.amount; if let Some(index) = validator_index { // Update the existing validator balance. - safe_add_assign!( - state.validator_balances[index as usize], - deposit_data.amount - ); + safe_add_assign!(state.balances[index as usize], amount); } else { // Create a new validator. let validator = Validator { - pubkey: deposit_input.pubkey.clone(), - withdrawal_credentials: deposit_input.withdrawal_credentials, + pubkey: deposit.data.pubkey.clone(), + withdrawal_credentials: deposit.data.withdrawal_credentials, + activation_eligibility_epoch: spec.far_future_epoch, activation_epoch: spec.far_future_epoch, exit_epoch: spec.far_future_epoch, withdrawable_epoch: spec.far_future_epoch, - initiated_exit: false, + effective_balance: std::cmp::min( + amount - amount % spec.effective_balance_increment, + spec.max_effective_balance, + ), slashed: false, }; state.validator_registry.push(validator); - state.validator_balances.push(deposit_data.amount); + state.balances.push(deposit.data.amount); } state.deposit_index += 1; @@ -482,4 +483,3 @@ pub fn process_transfers( Ok(()) } -*/ diff --git a/eth2/state_processing/src/per_block_processing/errors.rs b/eth2/state_processing/src/per_block_processing/errors.rs index b7a5cab2c..747664b94 100644 --- a/eth2/state_processing/src/per_block_processing/errors.rs +++ b/eth2/state_processing/src/per_block_processing/errors.rs @@ -77,7 +77,7 @@ pub enum BlockInvalid { MaxAttestationsExceeded, MaxAttesterSlashingsExceed, MaxProposerSlashingsExceeded, - MaxDepositsExceeded, + DepositCountInvalid, MaxExitsExceeded, MaxTransfersExceed, AttestationInvalid(usize, AttestationInvalid), 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 b3ad04c0c..121aa5bc2 100644 --- a/eth2/state_processing/src/per_block_processing/verify_deposit.rs +++ b/eth2/state_processing/src/per_block_processing/verify_deposit.rs @@ -3,6 +3,7 @@ use hashing::hash; use merkle_proof::verify_merkle_proof; use ssz::ssz_encode; use ssz_derive::Encode; +use tree_hash::{SignedRoot, TreeHash}; use types::*; /// Indicates if a `Deposit` is valid to be included in a block in the current epoch of the given @@ -15,25 +16,13 @@ use types::*; /// /// Note: this function is incomplete. /// -/// Spec v0.5.1 +/// Spec v0.6.1 pub fn verify_deposit( state: &BeaconState, deposit: &Deposit, verify_merkle_branch: bool, spec: &ChainSpec, ) -> Result<(), Error> { - verify!( - deposit - .deposit_data - .deposit_input - .validate_proof_of_possession( - state.slot.epoch(spec.slots_per_epoch), - &state.fork, - spec - ), - Invalid::BadProofOfPossession - ); - if verify_merkle_branch { verify!( verify_deposit_merkle_proof(state, deposit, spec), @@ -41,12 +30,23 @@ pub fn verify_deposit( ); } + // NOTE: proof of possession should only be verified when the validator + // is not already part of the registry + verify!( + deposit.data.signature.verify( + &deposit.data.signed_root(), + spec.get_domain(state.current_epoch(), Domain::Deposit, &state.fork), + &deposit.data.pubkey, + ), + Invalid::BadProofOfPossession + ); + Ok(()) } /// Verify that the `Deposit` index is correct. /// -/// Spec v0.5.1 +/// Spec v0.6.1 pub fn verify_deposit_index( state: &BeaconState, deposit: &Deposit, @@ -72,16 +72,15 @@ pub fn get_existing_validator_index( state: &BeaconState, deposit: &Deposit, ) -> Result, Error> { - let deposit_input = &deposit.deposit_data.deposit_input; - - let validator_index = state.get_validator_index(&deposit_input.pubkey)?; + let validator_index = state.get_validator_index(&deposit.data.pubkey)?; + // NOTE: it seems that v0.6.1 doesn't require the withdrawal credentials to be checked match validator_index { None => Ok(None), Some(index) => { verify!( - deposit_input.withdrawal_credentials - == state.validator_registry[index as usize].withdrawal_credentials, + deposit.data.withdrawal_credentials + == state.validator_registry[index].withdrawal_credentials, Invalid::BadWithdrawalCredentials ); Ok(Some(index as u64)) @@ -91,7 +90,7 @@ pub fn get_existing_validator_index( /// Verify that a deposit is included in the state's eth1 deposit root. /// -/// Spec v0.6.0 +/// Spec v0.6.1 fn verify_deposit_merkle_proof( state: &BeaconState, deposit: &Deposit, diff --git a/eth2/types/src/deposit_input.rs b/eth2/types/src/deposit_input.rs deleted file mode 100644 index f44c75f5a..000000000 --- a/eth2/types/src/deposit_input.rs +++ /dev/null @@ -1,92 +0,0 @@ -use crate::test_utils::TestRandom; -use crate::*; -use bls::{PublicKey, Signature}; - -use serde_derive::{Deserialize, Serialize}; -use ssz_derive::{Decode, Encode}; -use test_random_derive::TestRandom; -use tree_hash::{SignedRoot, TreeHash}; -use tree_hash_derive::{CachedTreeHash, SignedRoot, TreeHash}; - -/// The data supplied by the user to the deposit contract. -/// -/// Spec v0.5.1 -#[derive( - Debug, - PartialEq, - Clone, - Serialize, - Deserialize, - Encode, - Decode, - SignedRoot, - TreeHash, - CachedTreeHash, - TestRandom, -)] -pub struct DepositInput { - pub pubkey: PublicKey, - pub withdrawal_credentials: Hash256, - #[signed_root(skip_hashing)] - pub proof_of_possession: Signature, -} - -impl DepositInput { - /// Generate the 'proof_of_posession' signature for a given DepositInput details. - /// - /// Spec v0.5.1 - pub fn create_proof_of_possession( - &self, - secret_key: &SecretKey, - epoch: Epoch, - fork: &Fork, - spec: &ChainSpec, - ) -> Signature { - let msg = self.signed_root(); - let domain = spec.get_domain(epoch, Domain::Deposit, fork); - - Signature::new(msg.as_slice(), domain, secret_key) - } - - /// Verify that proof-of-possession is valid. - /// - /// Spec v0.5.1 - 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)] -mod tests { - use super::*; - - ssz_tests!(DepositInput); - cached_tree_hash_tests!(DepositInput); - - #[test] - fn can_create_and_validate() { - let spec = ChainSpec::foundation(); - let fork = Fork::genesis(&spec); - let keypair = Keypair::random(); - let epoch = Epoch::new(0); - - let mut deposit_input = DepositInput { - pubkey: keypair.pk.clone(), - withdrawal_credentials: Hash256::zero(), - proof_of_possession: Signature::empty_signature(), - }; - - deposit_input.proof_of_possession = - deposit_input.create_proof_of_possession(&keypair.sk, epoch, &fork, &spec); - - assert!(deposit_input.validate_proof_of_possession(epoch, &fork, &spec)); - } -}