diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a47030440e..3cdcb7f9aa 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1361,6 +1361,11 @@ impl BeaconChain { let (proposer_slashings, attester_slashings) = self.op_pool.get_slashings(&state, &self.spec); + let eth1_data = eth1_chain.eth1_data_for_block_production(&state, &self.spec)?; + let deposits = eth1_chain + .deposits_for_block_inclusion(&state, ð1_data, &self.spec)? + .into(); + let mut block = BeaconBlock { slot: state.slot, parent_root, @@ -1369,14 +1374,12 @@ impl BeaconChain { signature: Signature::empty_signature(), body: BeaconBlockBody { randao_reveal, - eth1_data: eth1_chain.eth1_data_for_block_production(&state, &self.spec)?, + eth1_data, graffiti, proposer_slashings: proposer_slashings.into(), attester_slashings: attester_slashings.into(), attestations: self.op_pool.get_attestations(&state, &self.spec).into(), - deposits: eth1_chain - .deposits_for_block_inclusion(&state, &self.spec)? - .into(), + deposits, voluntary_exits: self.op_pool.get_voluntary_exits(&state, &self.spec).into(), }, }; diff --git a/beacon_node/beacon_chain/src/eth1_chain.rs b/beacon_node/beacon_chain/src/eth1_chain.rs index 9888c81904..357f33fa1c 100644 --- a/beacon_node/beacon_chain/src/eth1_chain.rs +++ b/beacon_node/beacon_chain/src/eth1_chain.rs @@ -5,6 +5,7 @@ use futures::Future; use integer_sqrt::IntegerSquareRoot; use rand::prelude::*; use slog::{crit, debug, error, trace, Logger}; +use state_processing::per_block_processing::get_new_eth1_data; use std::collections::HashMap; use std::iter::DoubleEndedIterator; use std::iter::FromIterator; @@ -89,16 +90,21 @@ where /// Returns a list of `Deposits` that may be included in a block. /// /// Including all of the returned `Deposits` in a block should _not_ cause it to become - /// invalid. + /// invalid (i.e., this function should respect the maximum). + /// + /// The `eth1_data_vote` is the `Eth1Data` that the block producer would include in their + /// block. This vote may change the `state.eth1_data` value, which would change the deposit + /// count and therefore change the output of this function. pub fn deposits_for_block_inclusion( &self, state: &BeaconState, + eth1_data_vote: &Eth1Data, spec: &ChainSpec, ) -> Result, Error> { if self.use_dummy_backend { - DummyEth1ChainBackend::default().queued_deposits(state, spec) + DummyEth1ChainBackend::default().queued_deposits(state, eth1_data_vote, spec) } else { - self.backend.queued_deposits(state, spec) + self.backend.queued_deposits(state, eth1_data_vote, spec) } } } @@ -119,6 +125,7 @@ pub trait Eth1ChainBackend: Sized + Send + Sync { fn queued_deposits( &self, beacon_state: &BeaconState, + eth1_data_vote: &Eth1Data, spec: &ChainSpec, ) -> Result, Error>; } @@ -146,7 +153,12 @@ impl Eth1ChainBackend for DummyEth1ChainBackend { }) } - fn queued_deposits(&self, _: &BeaconState, _: &ChainSpec) -> Result, Error> { + fn queued_deposits( + &self, + _: &BeaconState, + _: &Eth1Data, + _: &ChainSpec, + ) -> Result, Error> { Ok(vec![]) } } @@ -291,10 +303,15 @@ impl Eth1ChainBackend for CachingEth1Backend { fn queued_deposits( &self, state: &BeaconState, + eth1_data_vote: &Eth1Data, _spec: &ChainSpec, ) -> Result, Error> { - let deposit_count = state.eth1_data.deposit_count; let deposit_index = state.eth1_deposit_index; + let deposit_count = if let Some(new_eth1_data) = get_new_eth1_data(state, eth1_data_vote) { + new_eth1_data.deposit_count + } else { + state.eth1_data.deposit_count + }; if deposit_index > deposit_count { Err(Error::DepositIndexTooHigh) @@ -346,7 +363,7 @@ fn eth1_block_hash_at_start_of_voting_period( ) -> Result { let period = T::SlotsPerEth1VotingPeriod::to_u64(); - if (state.eth1_data_votes.len() as u64) < period / 2 { + if !eth1_data_change_is_possible(state) { // If there are less than 50% of the votes in the current state, it's impossible that the // `eth1_data.block_hash` has changed from the value at `state.eth1_data.block_hash`. Ok(state.eth1_data.block_hash) @@ -367,6 +384,12 @@ fn eth1_block_hash_at_start_of_voting_period( } } +/// Returns true if there are enough eth1 votes in the given `state` to have updated +/// `state.eth1_data`. +fn eth1_data_change_is_possible(state: &BeaconState) -> bool { + state.eth1_data_votes.len() as u64 >= E::SlotsPerEth1VotingPeriod::to_u64() / 2 +} + /// Calculates and returns `(new_eth1_data, all_eth1_data)` for the given `state`, based upon the /// blocks in the `block` iterator. /// @@ -584,7 +607,7 @@ mod test { assert!( eth1_chain - .deposits_for_block_inclusion(&state, spec) + .deposits_for_block_inclusion(&state, &random_eth1_data(), spec) .is_ok(), "should succeed if cache is empty but no deposits are required" ); @@ -593,7 +616,7 @@ mod test { assert!( eth1_chain - .deposits_for_block_inclusion(&state, spec) + .deposits_for_block_inclusion(&state, &random_eth1_data(), spec) .is_err(), "should fail to get deposits if required, but cache is empty" ); @@ -637,7 +660,7 @@ mod test { assert!( eth1_chain - .deposits_for_block_inclusion(&state, spec) + .deposits_for_block_inclusion(&state, &random_eth1_data(), spec) .is_ok(), "should succeed if no deposits are required" ); @@ -649,7 +672,7 @@ mod test { state.eth1_data.deposit_count = i as u64; let deposits_for_inclusion = eth1_chain - .deposits_for_block_inclusion(&state, spec) + .deposits_for_block_inclusion(&state, &random_eth1_data(), spec) .expect(&format!("should find deposit for {}", i)); let expected_len = diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index c60b89a0c3..bbac4fd7d2 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -213,19 +213,35 @@ pub fn process_eth1_data( state: &mut BeaconState, eth1_data: &Eth1Data, ) -> Result<(), Error> { + if let Some(new_eth1_data) = get_new_eth1_data(state, eth1_data) { + state.eth1_data = new_eth1_data; + } + state.eth1_data_votes.push(eth1_data.clone())?; + Ok(()) +} + +/// Returns `Some(eth1_data)` if adding the given `eth1_data` to `state.eth1_data_votes` would +/// result in a change to `state.eth1_data`. +/// +/// Spec v0.9.1 +pub fn get_new_eth1_data( + state: &BeaconState, + eth1_data: &Eth1Data, +) -> Option { let num_votes = state .eth1_data_votes .iter() .filter(|vote| *vote == eth1_data) .count(); - if num_votes * 2 > T::SlotsPerEth1VotingPeriod::to_usize() { - state.eth1_data = eth1_data.clone(); + // The +1 is to account for the `eth1_data` supplied to the function. + if num_votes * 2 + 1 > T::SlotsPerEth1VotingPeriod::to_usize() { + Some(eth1_data.clone()) + } else { + None } - - Ok(()) } /// Validates each `ProposerSlashing` and updates the state, short-circuiting on an invalid object.