Merge branch 'master' into master-sf

Cargo.lock

@@ -250,6 +250,7 @@ version = "0.2.0"
 dependencies = [
  "beacon_chain 0.2.0",
  "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "clap_utils 0.1.0",
  "client 0.2.0",
  "ctrlc 3.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "dirs 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",

beacon_node/Cargo.toml

@@ -41,3 +41,4 @@ eth2-libp2p = { path = "./eth2-libp2p" }
 eth2_ssz = { path = "../eth2/utils/ssz" }
 toml = "0.5.4"
 serde = "1.0.102"
+clap_utils = { path = "../eth2/utils/clap_utils" }

beacon_node/client/src/notifier.rs

@@ -147,6 +147,7 @@ pub fn spawn_notifier<T: BeaconChainTypes>(
                 );
             } else {
                 if sync_state.is_synced() {
+                    let block_info = if current_slot > head_slot { format!("   …  empty") } else { format!("{}", head_root) };
                     info!(
                         log_2,
                         "Synced";
@@ -154,6 +155,7 @@ pub fn spawn_notifier<T: BeaconChainTypes>(
                         "finalized_root" => format!("{}", finalized_root),
                         "finalized_epoch" => finalized_epoch,
                         "epoch" => current_epoch,
+                        "block" => block_info,
                         "slot" => current_slot,
                     );
                 } else {

beacon_node/eth2-libp2p/src/config.rs

@@ -92,7 +92,7 @@ impl Default for Config {
         let gossip_message_id = |message: &GossipsubMessage| {
             MessageId(base64::encode_config(
                 &Sha256::digest(&message.data),
-                base64::URL_SAFE,
+                base64::URL_SAFE_NO_PAD,
             ))
         };
 

beacon_node/genesis/src/eth1_genesis_service.rs

@@ -5,7 +5,7 @@ use eth1::{DepositLog, Eth1Block, Service};
 use parking_lot::Mutex;
 use slog::{debug, error, info, trace, Logger};
 use state_processing::{
-    initialize_beacon_state_from_eth1, is_valid_genesis_state,
+    eth2_genesis_time, initialize_beacon_state_from_eth1, is_valid_genesis_state,
     per_block_processing::process_deposit, process_activations,
 };
 use std::sync::Arc;
@@ -186,9 +186,15 @@ impl Eth1GenesisService {
             .blocks()
             .read()
             .iter()
-            // It's only worth scanning blocks that have timestamps _after_ genesis time. It's
+            // Filter out any blocks that would result in a genesis time that is earlier than
-            // impossible for any other block to trigger genesis.
+            // `MIN_GENESIS_TIME`.
-            .filter(|block| block.timestamp >= spec.min_genesis_time)
+            //
+            // Note: any `SafeArith` errors are suppressed here; we simply skip blocks that cause
+            // overflow/div-by-zero.
+            .filter(|block| {
+                eth2_genesis_time(block.timestamp, spec)
+                    .map_or(false, |t| t >= spec.min_genesis_time)
+            })
             // The block cache might be more recently updated than deposit cache. Restrict any
             // block numbers that are not known by all caches.
             .filter(|block| {

beacon_node/network/src/attestation_service/mod.rs

@@ -10,7 +10,7 @@ use rand::seq::SliceRandom;
 use rest_types::ValidatorSubscription;
 use slog::{crit, debug, error, o, warn};
 use slot_clock::SlotClock;
-use std::collections::{HashMap, VecDeque};
+use std::collections::VecDeque;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 use types::{Attestation, EthSpec, Slot, SubnetId};
@@ -77,8 +77,8 @@ pub struct AttestationService<T: BeaconChainTypes> {
     /// A collection of timeouts for when to unsubscribe from a shard subnet.
     unsubscriptions: HashSetDelay<ExactSubnet>,
 
-    /// A mapping indicating the number of known aggregate validators for a given `ExactSubnet`.
+    /// A collection timeouts to track the existence of aggregate validator subscriptions at an `ExactSubnet`.
-    _aggregate_validators_on_subnet: HashMap<ExactSubnet, usize>,
+    aggregate_validators_on_subnet: HashSetDelay<ExactSubnet>,
 
     /// A collection of seen validators. These dictate how many random subnets we should be
     /// subscribed to. As these time out, we unsubscribe for the required random subnets and update
@@ -124,7 +124,7 @@ impl<T: BeaconChainTypes> AttestationService<T> {
             discover_peers: HashSetDelay::new(default_timeout),
             subscriptions: HashSetDelay::new(default_timeout),
             unsubscriptions: HashSetDelay::new(default_timeout),
-            _aggregate_validators_on_subnet: HashMap::new(),
+            aggregate_validators_on_subnet: HashSetDelay::new(default_timeout),
             known_validators: HashSetDelay::new(last_seen_val_timeout),
             log,
         }
@@ -176,10 +176,12 @@ impl<T: BeaconChainTypes> AttestationService<T> {
             // sophisticated logic should be added using known future forks.
             // TODO: Implement
 
-            // set the subscription timer to subscribe to the next subnet if required
+            if subscription.is_aggregator {
-            if let Err(e) = self.subscribe_to_subnet(exact_subnet) {
+                // set the subscription timer to subscribe to the next subnet if required
-                warn!(self.log, "Subscription to subnet error"; "error" => e);
+                if let Err(e) = self.subscribe_to_subnet(exact_subnet) {
-                return Err(());
+                    warn!(self.log, "Subscription to subnet error"; "error" => e);
+                    return Err(());
+                }
             }
         }
         Ok(())
@@ -208,8 +210,11 @@ impl<T: BeaconChainTypes> AttestationService<T> {
             return false;
         }
 
-        // TODO: Correctly handle validation aggregator checks
+        let exact_subnet = ExactSubnet {
-        true
+            subnet_id: subnet.clone(),
+            slot: attestation.data.slot,
+        };
+        self.aggregate_validators_on_subnet.contains(&exact_subnet)
     }
 
     /* Internal private functions */
@@ -335,6 +340,12 @@ impl<T: BeaconChainTypes> AttestationService<T> {
             }
         };
 
+        // Regardless of whether or not we have already subscribed to a subnet, track the expiration
+        // of aggregate validator subscriptions to exact subnets so we know whether or not to drop
+        // attestations for a given subnet + slot
+        self.aggregate_validators_on_subnet
+            .insert_at(exact_subnet.clone(), expected_end_subscription_duration);
+
         // Checks on current subscriptions
         // Note: We may be connected to a long-lived random subnet. In this case we still add the
         // subscription timeout and check this case when the timeout fires. This is because a
@@ -641,6 +652,8 @@ impl<T: BeaconChainTypes> Stream for AttestationService<T> {
                 {
                     let _ = self.handle_known_validator_expiry();
                 }
+        // poll to remove entries on expiration, no need to act on expiration events
+        let _ = self.aggregate_validators_on_subnet.poll().map_err(|e| { error!(self.log, "Failed to check for aggregate validator on subnet expirations"; "error"=> format!("{}", e)); });
 
         // process any generated events
         if let Some(event) = self.events.pop_front() {

beacon_node/src/config.rs

@@ -1,5 +1,6 @@
 use beacon_chain::builder::PUBKEY_CACHE_FILENAME;
 use clap::ArgMatches;
+use clap_utils::BAD_TESTNET_DIR_MESSAGE;
 use client::{config::DEFAULT_DATADIR, ClientConfig, ClientGenesis};
 use eth2_libp2p::{Enr, Multiaddr};
 use eth2_testnet_config::Eth2TestnetConfig;
@@ -385,14 +386,8 @@ pub fn get_eth2_testnet_config<E: EthSpec>(
         Eth2TestnetConfig::load(testnet_dir.clone())
             .map_err(|e| format!("Unable to open testnet dir at {:?}: {}", testnet_dir, e))?
     } else {
-        Eth2TestnetConfig::hard_coded().map_err(|e| {
+        Eth2TestnetConfig::hard_coded()
-            format!(
+            .map_err(|e| format!("{} Error : {}", BAD_TESTNET_DIR_MESSAGE, e))?
-                "The hard-coded testnet directory was invalid. \
-                 This happens when Lighthouse is migrating between spec versions. \
-                 Error : {}",
-                e
-            )
-        })?
     })
 }
 

eth2/operation_pool/src/lib.rs

@@ -485,7 +485,7 @@ mod release_tests {
         state_builder.teleport_to_slot(slot);
         state_builder.build_caches(&spec).unwrap();
         let (state, keypairs) = state_builder.build();
-        (state, keypairs, MainnetEthSpec::default_spec())
+        (state, keypairs, spec)
     }
 
     #[test]
@@ -890,4 +890,44 @@ mod release_tests {
             seen_indices.extend(fresh_indices);
         }
     }
+
+    /// Insert two slashings for the same proposer and ensure only one is returned.
+    #[test]
+    fn duplicate_proposer_slashing() {
+        let spec = MainnetEthSpec::default_spec();
+        let num_validators = 32;
+        let mut state_builder =
+            TestingBeaconStateBuilder::<MainnetEthSpec>::from_default_keypairs_file_if_exists(
+                num_validators,
+                &spec,
+            );
+        state_builder.build_caches(&spec).unwrap();
+        let (state, keypairs) = state_builder.build();
+        let op_pool = OperationPool::new();
+
+        let proposer_index = 0;
+        let slashing1 = TestingProposerSlashingBuilder::double_vote::<MainnetEthSpec>(
+            ProposerSlashingTestTask::Valid,
+            proposer_index,
+            &keypairs[proposer_index as usize].sk,
+            &state.fork,
+            state.genesis_validators_root,
+            &spec,
+        );
+        let slashing2 = ProposerSlashing {
+            signed_header_1: slashing1.signed_header_2.clone(),
+            signed_header_2: slashing1.signed_header_1.clone(),
+        };
+
+        // Both slashings should be accepted by the pool.
+        op_pool
+            .insert_proposer_slashing(slashing1.clone(), &state, &spec)
+            .unwrap();
+        op_pool
+            .insert_proposer_slashing(slashing2.clone(), &state, &spec)
+            .unwrap();
+
+        // Should only get the second slashing back.
+        assert_eq!(op_pool.get_slashings(&state, &spec).0, vec![slashing2]);
+    }
 }

eth2/state_processing/src/genesis.rs

@@ -1,6 +1,6 @@
 use super::per_block_processing::{errors::BlockProcessingError, process_deposit};
 use crate::common::DepositDataTree;
-use safe_arith::SafeArith;
+use safe_arith::{ArithError, SafeArith};
 use tree_hash::TreeHash;
 use types::DEPOSIT_TREE_DEPTH;
 use types::*;
@@ -15,9 +15,7 @@ pub fn initialize_beacon_state_from_eth1<T: EthSpec>(
     deposits: Vec<Deposit>,
     spec: &ChainSpec,
 ) -> Result<BeaconState<T>, BlockProcessingError> {
-    let genesis_time = eth1_timestamp
+    let genesis_time = eth2_genesis_time(eth1_timestamp, spec)?;
-        .safe_sub(eth1_timestamp.safe_rem(spec.min_genesis_delay)?)?
-        .safe_add(2.safe_mul(spec.min_genesis_delay)?)?;
     let eth1_data = Eth1Data {
         // Temporary deposit root
         deposit_root: Hash256::zero(),
@@ -79,3 +77,14 @@ pub fn process_activations<T: EthSpec>(
     }
     Ok(())
 }
+
+/// Returns the `state.genesis_time` for the corresponding `eth1_timestamp`.
+///
+/// Does _not_ ensure that the time is greater than `MIN_GENESIS_TIME`.
+///
+/// Spec v0.11.1
+pub fn eth2_genesis_time(eth1_timestamp: u64, spec: &ChainSpec) -> Result<u64, ArithError> {
+    eth1_timestamp
+        .safe_sub(eth1_timestamp.safe_rem(spec.min_genesis_delay)?)?
+        .safe_add(2.safe_mul(spec.min_genesis_delay)?)
+}

eth2/state_processing/src/lib.rs

@@ -10,7 +10,10 @@ pub mod per_epoch_processing;
 pub mod per_slot_processing;
 pub mod test_utils;
 
-pub use genesis::{initialize_beacon_state_from_eth1, is_valid_genesis_state, process_activations};
+pub use genesis::{
+    eth2_genesis_time, initialize_beacon_state_from_eth1, is_valid_genesis_state,
+    process_activations,
+};
 pub use per_block_processing::{
     block_signature_verifier, errors::BlockProcessingError, per_block_processing, signature_sets,
     BlockSignatureStrategy, BlockSignatureVerifier, VerifySignatures,

eth2/state_processing/src/per_block_processing.rs

@@ -283,26 +283,25 @@ pub fn process_proposer_slashings<T: EthSpec>(
     verify_signatures: VerifySignatures,
     spec: &ChainSpec,
 ) -> Result<(), BlockProcessingError> {
-    // Verify proposer slashings in parallel.
+    // Verify and apply proposer slashings in series.
+    // We have to verify in series because an invalid block may contain multiple slashings
+    // for the same validator, and we need to correctly detect and reject that.
     proposer_slashings
-        .par_iter()
+        .into_iter()
         .enumerate()
         .try_for_each(|(i, proposer_slashing)| {
             verify_proposer_slashing(proposer_slashing, &state, verify_signatures, spec)
-                .map_err(|e| e.into_with_index(i))
+                .map_err(|e| e.into_with_index(i))?;
-        })?;
 
-    // Update the state.
+            slash_validator(
-    for proposer_slashing in proposer_slashings {
+                state,
-        slash_validator(
+                proposer_slashing.signed_header_1.message.proposer_index as usize,
-            state,
+                None,
-            proposer_slashing.signed_header_1.message.proposer_index as usize,
+                spec,
-            None,
+            )?;
-            spec,
-        )?;
-    }
 
-    Ok(())
+            Ok(())
+        })
 }
 
 /// Validates each `AttesterSlashing` and updates the state, short-circuiting on an invalid object.

eth2/state_processing/src/per_block_processing/tests.rs

@@ -1029,6 +1029,37 @@ fn invalid_proposer_slashing_not_slashable() {
     );
 }
 
+#[test]
+fn invalid_proposer_slashing_duplicate_slashing() {
+    let spec = MainnetEthSpec::default_spec();
+    let builder = get_builder(&spec, SLOT_OFFSET, VALIDATOR_COUNT);
+    let test_task = ProposerSlashingTestTask::Valid;
+    let (mut block, mut state) =
+        builder.build_with_proposer_slashing(test_task, 1, None, None, &spec);
+
+    let slashing = block.message.body.proposer_slashings[0].clone();
+    let slashed_proposer = slashing.signed_header_1.message.proposer_index;
+    block.message.body.proposer_slashings.push(slashing);
+
+    let result = per_block_processing(
+        &mut state,
+        &block,
+        None,
+        BlockSignatureStrategy::NoVerification,
+        &spec,
+    );
+
+    // Expecting ProposerNotSlashable for the 2nd slashing because the validator has been
+    // slashed by the 1st slashing.
+    assert_eq!(
+        result,
+        Err(BlockProcessingError::ProposerSlashingInvalid {
+            index: 1,
+            reason: ProposerSlashingInvalid::ProposerNotSlashable(slashed_proposer)
+        })
+    );
+}
+
 #[test]
 fn invalid_bad_proposal_1_signature() {
     let spec = MainnetEthSpec::default_spec();

eth2/utils/clap_utils/src/lib.rs

@@ -8,6 +8,11 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use types::EthSpec;
 
+pub const BAD_TESTNET_DIR_MESSAGE: &str = "The hard-coded testnet directory was invalid. \
+                                        This happens when Lighthouse is migrating between spec versions \
+                                        or when there is no default public network to connect to. \
+                                        During these times you must specify a --testnet-dir.";
+
 /// Attempts to load the testnet dir at the path if `name` is in `matches`, returning an error if
 /// the path cannot be found or the testnet dir is invalid.
 ///
@@ -20,14 +25,8 @@ pub fn parse_testnet_dir_with_hardcoded_default<E: EthSpec>(
         Eth2TestnetConfig::load(path.clone())
             .map_err(|e| format!("Unable to open testnet dir at {:?}: {}", path, e))
     } else {
-        Eth2TestnetConfig::hard_coded().map_err(|e| {
+        Eth2TestnetConfig::hard_coded()
-            format!(
+            .map_err(|e| format!("{} Error : {}", BAD_TESTNET_DIR_MESSAGE, e))
-                "The hard-coded testnet directory was invalid. \
-                 This happens when Lighthouse is migrating between spec versions. \
-                 Error : {}",
-                e
-            )
-        })
     }
 }
 

lcli/src/interop_genesis.rs

@@ -1,4 +1,5 @@
 use clap::ArgMatches;
+use clap_utils::parse_ssz_optional;
 use environment::Environment;
 use eth2_testnet_config::Eth2TestnetConfig;
 use genesis::interop_genesis_state;
@@ -49,7 +50,9 @@ pub fn run<T: EthSpec>(mut env: Environment<T>, matches: &ArgMatches) -> Result<
             )
         })?;
 
-    spec.genesis_fork_version = [1, 3, 3, 7];
+    if let Some(v) = parse_ssz_optional(matches, "genesis-fork-version")? {
+        spec.genesis_fork_version = v;
+    }
 
     let keypairs = generate_deterministic_keypairs(validator_count);
     let genesis_state = interop_genesis_state(&keypairs, genesis_time, &spec)?;

lcli/src/main.rs

@@ -226,6 +226,14 @@ async fn main() {
                         .takes_value(true)
                         .help("The value for state.genesis_time. Defaults to now."),
                 )
+                .arg(
+                    Arg::with_name("genesis-fork-version")
+                        .long("genesis-fork-version")
+                        .value_name("HEX")
+                        .takes_value(true)
+                        .help("Used to avoid reply attacks between testnets. Recommended to set to
+                              non-default."),
+                )
         )
         .subcommand(
             SubCommand::with_name("change-genesis-time")