From 15e4aabd8a99a1c5116241810aca2902254ec342 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 7 Mar 2019 16:15:38 +1100 Subject: [PATCH] Add deposit processing, fix clippy lints --- eth2/fork_choice/src/bitwise_lmd_ghost.rs | 2 +- eth2/state_processing/Cargo.toml | 1 + .../src/per_block_processing.rs | 9 +- .../src/per_block_processing/errors.rs | 45 ++++++- .../src/per_block_processing/verify_exit.rs | 4 +- .../verify_proposer_slashing.rs | 6 +- .../per_block_processing/verify_transfer.rs | 120 +++++++++++++++++- eth2/types/src/transfer.rs | 5 +- eth2/utils/bls/src/lib.rs | 10 ++ eth2/utils/merkle_proof/src/lib.rs | 6 +- eth2/utils/ssz_derive/src/lib.rs | 2 +- 11 files changed, 184 insertions(+), 26 deletions(-) diff --git a/eth2/fork_choice/src/bitwise_lmd_ghost.rs b/eth2/fork_choice/src/bitwise_lmd_ghost.rs index d50471f56e..fd1c3dea45 100644 --- a/eth2/fork_choice/src/bitwise_lmd_ghost.rs +++ b/eth2/fork_choice/src/bitwise_lmd_ghost.rs @@ -379,7 +379,7 @@ impl ForkChoice for BitwiseLMDGhost { trace!("Current Step: {}", step); if let Some(clear_winner) = self.get_clear_winner( &latest_votes, - block_height - (block_height % u64::from(step)) + u64::from(step), + block_height - (block_height % step) + step, spec, ) { current_head = clear_winner; diff --git a/eth2/state_processing/Cargo.toml b/eth2/state_processing/Cargo.toml index bdc14c1d74..b673996ae2 100644 --- a/eth2/state_processing/Cargo.toml +++ b/eth2/state_processing/Cargo.toml @@ -13,6 +13,7 @@ criterion = "0.2" env_logger = "0.6.0" [dependencies] +bls = { path = "../utils/bls" } hashing = { path = "../utils/hashing" } int_to_bytes = { path = "../utils/int_to_bytes" } integer-sqrt = "0.1" diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 06a5f81c74..61b12bedb6 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -9,7 +9,7 @@ pub use self::verify_attester_slashing::verify_attester_slashing; pub use validate_attestation::{validate_attestation, validate_attestation_without_signature}; pub use verify_deposit::verify_deposit; pub use verify_exit::verify_exit; -pub use verify_transfer::verify_transfer; +pub use verify_transfer::{execute_transfer, verify_transfer}; pub mod errors; mod validate_attestation; @@ -373,12 +373,7 @@ pub fn process_transfers( ); for (i, transfer) in transfers.iter().enumerate() { verify_transfer(&state, transfer, spec).map_err(|e| e.into_with_index(i))?; - - let block_proposer = state.get_beacon_proposer_index(state.slot, spec)?; - - state.validator_balances[transfer.from as usize] -= transfer.amount + transfer.fee; - state.validator_balances[transfer.to as usize] += transfer.amount + transfer.fee; - state.validator_balances[block_proposer as usize] += transfer.fee; + execute_transfer(state, transfer, spec).map_err(|e| e.into_with_index(i))?; } Ok(()) diff --git a/eth2/state_processing/src/per_block_processing/errors.rs b/eth2/state_processing/src/per_block_processing/errors.rs index 6c9719fc23..59d3f2f80b 100644 --- a/eth2/state_processing/src/per_block_processing/errors.rs +++ b/eth2/state_processing/src/per_block_processing/errors.rs @@ -332,10 +332,51 @@ impl_into_with_index_without_beacon_error!(ExitValidationError, ExitInvalid); pub enum TransferValidationError { /// Validation completed successfully and the object is invalid. Invalid(TransferInvalid), + /// Encountered a `BeaconStateError` whilst attempting to determine validity. + BeaconStateError(BeaconStateError), } /// Describes why an object is invalid. #[derive(Debug, PartialEq)] -pub enum TransferInvalid {} +pub enum TransferInvalid { + /// The validator indicated by `transfer.from` is unknown. + FromValidatorUnknown(u64), + /// The validator indicated by `transfer.to` is unknown. + ToValidatorUnknown(u64), + /// The balance of `transfer.from` is insufficient. + /// + /// (required, available) + FromBalanceInsufficient(u64, u64), + /// Adding `transfer.fee` to `transfer.amount` causes an overflow. + /// + /// (transfer_fee, transfer_amount) + FeeOverflow(u64, u64), + /// This transfer would result in the `transfer.from` account to have `0 < balance < + /// min_deposit_amount` + /// + /// (resulting_amount, min_deposit_amount) + InvalidResultingFromBalance(u64, u64), + /// The state slot does not match `transfer.slot`. + /// + /// (state_slot, transfer_slot) + StateSlotMismatch(Slot, Slot), + /// The `transfer.from` validator has been activated and is not withdrawable. + /// + /// (from_validator) + FromValidatorIneligableForTransfer(u64), + /// The validators withdrawal credentials do not match `transfer.pubkey`. + WithdrawalCredentialsMismatch, + /// The deposit was not signed by `deposit.pubkey`. + BadSignature, + /// Overflow when adding to `transfer.to` balance. + /// + /// (to_balance, transfer_amount) + ToBalanceOverflow(u64, u64), + /// Overflow when adding to beacon proposer balance. + /// + /// (proposer_balance, transfer_fee) + ProposerBalanceOverflow(u64, u64), +} -impl_into_with_index_without_beacon_error!(TransferValidationError, TransferInvalid); +impl_from_beacon_state_error!(TransferValidationError); +impl_into_with_index_with_beacon_error!(TransferValidationError, TransferInvalid); diff --git a/eth2/state_processing/src/per_block_processing/verify_exit.rs b/eth2/state_processing/src/per_block_processing/verify_exit.rs index 408b77077b..8cd54fb690 100644 --- a/eth2/state_processing/src/per_block_processing/verify_exit.rs +++ b/eth2/state_processing/src/per_block_processing/verify_exit.rs @@ -16,9 +16,7 @@ pub fn verify_exit( let validator = state .validator_registry .get(exit.validator_index as usize) - .ok_or(Error::Invalid(Invalid::ValidatorUnknown( - exit.validator_index, - )))?; + .ok_or_else(|| Error::Invalid(Invalid::ValidatorUnknown(exit.validator_index)))?; verify!( validator.exit_epoch diff --git a/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs b/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs index 0350255ec1..c3c0079a96 100644 --- a/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs +++ b/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs @@ -16,9 +16,9 @@ pub fn verify_proposer_slashing( let proposer = state .validator_registry .get(proposer_slashing.proposer_index as usize) - .ok_or(Error::Invalid(Invalid::ProposerUnknown( - proposer_slashing.proposer_index, - )))?; + .ok_or_else(|| { + Error::Invalid(Invalid::ProposerUnknown(proposer_slashing.proposer_index)) + })?; verify!( proposer_slashing.proposal_1.slot == proposer_slashing.proposal_2.slot, diff --git a/eth2/state_processing/src/per_block_processing/verify_transfer.rs b/eth2/state_processing/src/per_block_processing/verify_transfer.rs index cd7bcb42ce..15ec17142e 100644 --- a/eth2/state_processing/src/per_block_processing/verify_transfer.rs +++ b/eth2/state_processing/src/per_block_processing/verify_transfer.rs @@ -1,4 +1,6 @@ use super::errors::{TransferInvalid as Invalid, TransferValidationError as Error}; +use bls::get_withdrawal_credentials; +use ssz::SignedRoot; use types::*; /// Indicates if a `Transfer` is valid to be included in a block in the current epoch of the given @@ -10,11 +12,121 @@ use types::*; /// /// Spec v0.4.0 pub fn verify_transfer( - _state: &BeaconState, - _transfer: &Transfer, - _spec: &ChainSpec, + state: &BeaconState, + transfer: &Transfer, + spec: &ChainSpec, ) -> Result<(), Error> { - // TODO: verify transfer. + let from_balance = *state + .validator_balances + .get(transfer.from as usize) + .ok_or_else(|| Error::Invalid(Invalid::FromValidatorUnknown(transfer.from)))?; + + let total_amount = transfer + .amount + .checked_add(transfer.fee) + .ok_or_else(|| Error::Invalid(Invalid::FeeOverflow(transfer.amount, transfer.fee)))?; + + verify!( + from_balance >= transfer.amount, + Invalid::FromBalanceInsufficient(transfer.amount, from_balance) + ); + + verify!( + from_balance >= transfer.fee, + Invalid::FromBalanceInsufficient(transfer.fee, from_balance) + ); + + verify!( + (from_balance == total_amount) + || (from_balance >= (total_amount + spec.min_deposit_amount)), + Invalid::InvalidResultingFromBalance(from_balance - total_amount, spec.min_deposit_amount) + ); + + verify!( + state.slot == transfer.slot, + Invalid::StateSlotMismatch(state.slot, transfer.slot) + ); + + let from_validator = state + .validator_registry + .get(transfer.from as usize) + .ok_or_else(|| Error::Invalid(Invalid::FromValidatorUnknown(transfer.from)))?; + let epoch = state.slot.epoch(spec.slots_per_epoch); + + verify!( + from_validator.is_withdrawable_at(epoch) + || from_validator.activation_epoch == spec.far_future_epoch, + Invalid::FromValidatorIneligableForTransfer(transfer.from) + ); + + let transfer_withdrawal_credentials = Hash256::from_slice( + &get_withdrawal_credentials(&transfer.pubkey, spec.bls_withdrawal_prefix_byte)[..], + ); + verify!( + from_validator.withdrawal_credentials == transfer_withdrawal_credentials, + Invalid::WithdrawalCredentialsMismatch + ); + + let message = transfer.signed_root(); + let domain = spec.get_domain( + transfer.slot.epoch(spec.slots_per_epoch), + Domain::Transfer, + &state.fork, + ); + + verify!( + transfer + .signature + .verify(&message[..], domain, &transfer.pubkey), + Invalid::BadSignature + ); + + Ok(()) +} + +/// Executes a transfer on the state. +/// +/// Does not check that the transfer is valid, however checks for overflow in all actions. +/// +/// Spec v0.4.0 +pub fn execute_transfer( + state: &mut BeaconState, + transfer: &Transfer, + spec: &ChainSpec, +) -> Result<(), Error> { + let from_balance = *state + .validator_balances + .get(transfer.from as usize) + .ok_or_else(|| Error::Invalid(Invalid::FromValidatorUnknown(transfer.from)))?; + let to_balance = *state + .validator_balances + .get(transfer.to as usize) + .ok_or_else(|| Error::Invalid(Invalid::ToValidatorUnknown(transfer.to)))?; + + let proposer_index = state.get_beacon_proposer_index(state.slot, spec)?; + let proposer_balance = state.validator_balances[proposer_index]; + + let total_amount = transfer + .amount + .checked_add(transfer.fee) + .ok_or_else(|| Error::Invalid(Invalid::FeeOverflow(transfer.amount, transfer.fee)))?; + + state.validator_balances[transfer.from as usize] = + from_balance.checked_sub(total_amount).ok_or_else(|| { + Error::Invalid(Invalid::FromBalanceInsufficient(total_amount, from_balance)) + })?; + + state.validator_balances[transfer.to as usize] = to_balance + .checked_add(transfer.amount) + .ok_or_else(|| Error::Invalid(Invalid::ToBalanceOverflow(to_balance, transfer.amount)))?; + + state.validator_balances[proposer_index] = + proposer_balance.checked_add(transfer.fee).ok_or_else(|| { + Error::Invalid(Invalid::ProposerBalanceOverflow( + proposer_balance, + transfer.fee, + )) + })?; Ok(()) } diff --git a/eth2/types/src/transfer.rs b/eth2/types/src/transfer.rs index f23d3845e2..0382dee11b 100644 --- a/eth2/types/src/transfer.rs +++ b/eth2/types/src/transfer.rs @@ -3,13 +3,14 @@ use crate::test_utils::TestRandom; use bls::{PublicKey, Signature}; use rand::RngCore; use serde_derive::Serialize; -use ssz_derive::{Decode, Encode, TreeHash}; +use ssz::TreeHash; +use ssz_derive::{Decode, Encode, SignedRoot, TreeHash}; use test_random_derive::TestRandom; /// The data submitted to the deposit contract. /// /// Spec v0.4.0 -#[derive(Debug, PartialEq, Clone, Serialize, Encode, Decode, TreeHash, TestRandom)] +#[derive(Debug, PartialEq, Clone, Serialize, Encode, Decode, TreeHash, TestRandom, SignedRoot)] pub struct Transfer { pub from: u64, pub to: u64, diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index 865b8d82d2..bb109b0a1b 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -17,6 +17,7 @@ pub use crate::signature::Signature; 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 @@ -33,6 +34,15 @@ pub fn create_proof_of_possession(keypair: &Keypair) -> 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 { + let hashed = hash(&ssz_encode(pubkey)); + let mut prefixed = vec![prefix_byte]; + prefixed.extend_from_slice(&hashed[1..]); + + prefixed +} + pub fn bls_verify_aggregate( pubkey: &AggregatePublicKey, message: &[u8], diff --git a/eth2/utils/merkle_proof/src/lib.rs b/eth2/utils/merkle_proof/src/lib.rs index 4d0abcea3f..5ff8f79e68 100644 --- a/eth2/utils/merkle_proof/src/lib.rs +++ b/eth2/utils/merkle_proof/src/lib.rs @@ -25,14 +25,14 @@ fn merkle_root_from_branch(leaf: H256, branch: &[H256], depth: usize, index: usi let mut merkle_root = leaf.as_bytes().to_vec(); - for i in 0..depth { + for (i, leaf) in branch.iter().enumerate().take(depth) { let ith_bit = (index >> i) & 0x01; if ith_bit == 1 { - let input = concat(branch[i].as_bytes().to_vec(), merkle_root); + let input = concat(leaf.as_bytes().to_vec(), merkle_root); merkle_root = hash(&input); } else { let mut input = merkle_root; - input.extend_from_slice(branch[i].as_bytes()); + input.extend_from_slice(leaf.as_bytes()); merkle_root = hash(&input); } } diff --git a/eth2/utils/ssz_derive/src/lib.rs b/eth2/utils/ssz_derive/src/lib.rs index 4c6b9dc11e..0d2e17f768 100644 --- a/eth2/utils/ssz_derive/src/lib.rs +++ b/eth2/utils/ssz_derive/src/lib.rs @@ -172,7 +172,7 @@ fn type_ident_is_signature(ident: &syn::Ident) -> bool { /// the final `Ident` in that path. /// /// E.g., for `types::Signature` returns `Signature`. -fn final_type_ident<'a>(field: &'a syn::Field) -> &'a syn::Ident { +fn final_type_ident(field: &syn::Field) -> &syn::Ident { match &field.ty { syn::Type::Path(path) => &path.path.segments.last().unwrap().value().ident, _ => panic!("ssz_derive only supports Path types."),