Merge remote-tracking branch 'origin/unstable' into network-altair

Makefile

@@ -151,7 +151,7 @@ arbitrary-fuzz:
 # Runs cargo audit (Audit Cargo.lock files for crates with security vulnerabilities reported to the RustSec Advisory Database)
 audit:
 	cargo install --force cargo-audit
-	cargo audit --ignore RUSTSEC-2021-0073
+	cargo audit --ignore RUSTSEC-2021-0073 --ignore RUSTSEC-2021-0076
 
 # Runs `cargo udeps` to check for unused dependencies
 udeps:

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -373,15 +373,17 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
     pub fn verify(
         signed_aggregate: SignedAggregateAndProof<T::EthSpec>,
         chain: &BeaconChain<T>,
-    ) -> Result<Self, Error> {
+    ) -> Result<Self, (Error, SignedAggregateAndProof<T::EthSpec>)> {
         Self::verify_slashable(signed_aggregate, chain)
             .map(|verified_aggregate| {
                 if let Some(slasher) = chain.slasher.as_ref() {
                     slasher.accept_attestation(verified_aggregate.indexed_attestation.clone());
                 }
                 verified_aggregate
             })
-            .map_err(|slash_info| process_slash_info(slash_info, chain))
+            .map_err(|(slash_info, original_aggregate)| {
+                (process_slash_info(slash_info, chain), original_aggregate)
+            })
     }
 
     /// Run the checks that happen before an indexed attestation is constructed.
@@ -507,17 +509,31 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
     }
 
     /// Verify the attestation, producing extra information about whether it might be slashable.
+    // NOTE: Clippy considers the return too complex. This tuple is not used elsewhere so it is not
+    // worth creating an alias.
+    #[allow(clippy::type_complexity)]
     pub fn verify_slashable(
         signed_aggregate: SignedAggregateAndProof<T::EthSpec>,
         chain: &BeaconChain<T>,
-    ) -> Result<Self, AttestationSlashInfo<T, Error>> {
+    ) -> Result<
+        Self,
+        (
+            AttestationSlashInfo<T, Error>,
+            SignedAggregateAndProof<T::EthSpec>,
+        ),
+    > {
         use AttestationSlashInfo::*;
 
         let attestation = &signed_aggregate.message.aggregate;
         let aggregator_index = signed_aggregate.message.aggregator_index;
         let attestation_root = match Self::verify_early_checks(&signed_aggregate, chain) {
             Ok(root) => root,
-            Err(e) => return Err(SignatureNotChecked(signed_aggregate.message.aggregate, e)),
+            Err(e) => {
+                return Err((
+                    SignatureNotChecked(signed_aggregate.message.aggregate.clone(), e),
+                    signed_aggregate,
+                ))
+            }
         };
 
         let indexed_attestation =
@@ -544,7 +560,12 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
                     .map_err(|e| BeaconChainError::from(e).into())
             }) {
                 Ok(indexed_attestation) => indexed_attestation,
-                Err(e) => return Err(SignatureNotChecked(signed_aggregate.message.aggregate, e)),
+                Err(e) => {
+                    return Err((
+                        SignatureNotChecked(signed_aggregate.message.aggregate.clone(), e),
+                        signed_aggregate,
+                    ))
+                }
             };
 
         // Ensure that all signatures are valid.
@@ -558,11 +579,11 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
                     }
                 })
         {
-            return Err(SignatureInvalid(e));
+            return Err((SignatureInvalid(e), signed_aggregate));
         }
 
         if let Err(e) = Self::verify_late_checks(&signed_aggregate, attestation_root, chain) {
-            return Err(SignatureValid(indexed_attestation, e));
+            return Err((SignatureValid(indexed_attestation, e), signed_aggregate));
         }
 
         Ok(VerifiedAggregatedAttestation {
@@ -713,34 +734,39 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
         attestation: Attestation<T::EthSpec>,
         subnet_id: Option<SubnetId>,
         chain: &BeaconChain<T>,
-    ) -> Result<Self, Error> {
+    ) -> Result<Self, (Error, Attestation<T::EthSpec>)> {
         Self::verify_slashable(attestation, subnet_id, chain)
             .map(|verified_unaggregated| {
                 if let Some(slasher) = chain.slasher.as_ref() {
                     slasher.accept_attestation(verified_unaggregated.indexed_attestation.clone());
                 }
                 verified_unaggregated
             })
-            .map_err(|slash_info| process_slash_info(slash_info, chain))
+            .map_err(|(slash_info, original_attestation)| {
+                (process_slash_info(slash_info, chain), original_attestation)
+            })
     }
 
     /// Verify the attestation, producing extra information about whether it might be slashable.
+    // NOTE: Clippy considers the return too complex. This tuple is not used elsewhere so it is not
+    // worth creating an alias.
+    #[allow(clippy::type_complexity)]
     pub fn verify_slashable(
         attestation: Attestation<T::EthSpec>,
         subnet_id: Option<SubnetId>,
         chain: &BeaconChain<T>,
-    ) -> Result<Self, AttestationSlashInfo<T, Error>> {
+    ) -> Result<Self, (AttestationSlashInfo<T, Error>, Attestation<T::EthSpec>)> {
         use AttestationSlashInfo::*;
 
         if let Err(e) = Self::verify_early_checks(&attestation, chain) {
-            return Err(SignatureNotChecked(attestation, e));
+            return Err((SignatureNotChecked(attestation.clone(), e), attestation));
         }
 
         let (indexed_attestation, committees_per_slot) =
             match obtain_indexed_attestation_and_committees_per_slot(chain, &attestation) {
                 Ok(x) => x,
                 Err(e) => {
-                    return Err(SignatureNotChecked(attestation, e));
+                    return Err((SignatureNotChecked(attestation.clone(), e), attestation));
                 }
             };
 
@@ -752,16 +778,21 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
             chain,
         ) {
             Ok(t) => t,
-            Err(e) => return Err(SignatureNotCheckedIndexed(indexed_attestation, e)),
+            Err(e) => {
+                return Err((
+                    SignatureNotCheckedIndexed(indexed_attestation, e),
+                    attestation,
+                ))
+            }
         };
 
         // The aggregate signature of the attestation is valid.
         if let Err(e) = verify_attestation_signature(chain, &indexed_attestation) {
-            return Err(SignatureInvalid(e));
+            return Err((SignatureInvalid(e), attestation));
         }
 
         if let Err(e) = Self::verify_late_checks(&attestation, validator_index, chain) {
-            return Err(SignatureValid(indexed_attestation, e));
+            return Err((SignatureValid(indexed_attestation, e), attestation));
         }
 
         Ok(Self {

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1324,7 +1324,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         &self,
         unaggregated_attestation: Attestation<T::EthSpec>,
         subnet_id: Option<SubnetId>,
-    ) -> Result<VerifiedUnaggregatedAttestation<T>, AttestationError> {
+    ) -> Result<VerifiedUnaggregatedAttestation<T>, (AttestationError, Attestation<T::EthSpec>)>
+    {
         metrics::inc_counter(&metrics::UNAGGREGATED_ATTESTATION_PROCESSING_REQUESTS);
         let _timer =
             metrics::start_timer(&metrics::UNAGGREGATED_ATTESTATION_GOSSIP_VERIFICATION_TIMES);
@@ -1348,7 +1349,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn verify_aggregated_attestation_for_gossip(
         &self,
         signed_aggregate: SignedAggregateAndProof<T::EthSpec>,
-    ) -> Result<VerifiedAggregatedAttestation<T>, AttestationError> {
+    ) -> Result<
+        VerifiedAggregatedAttestation<T>,
+        (AttestationError, SignedAggregateAndProof<T::EthSpec>),
+    > {
         metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_PROCESSING_REQUESTS);
         let _timer =
             metrics::start_timer(&metrics::AGGREGATED_ATTESTATION_GOSSIP_VERIFICATION_TIMES);
@@ -2185,10 +2189,22 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
         drop(validator_monitor);
 
-        metrics::observe(
-            &metrics::OPERATIONS_PER_BLOCK_ATTESTATION,
-            block.body().attestations().len() as f64,
-        );
+        // Only present some metrics for blocks from the previous epoch or later.
+        //
+        // This helps avoid noise in the metrics during sync.
+        if block.slot().epoch(T::EthSpec::slots_per_epoch()) + 1 >= self.epoch()? {
+            metrics::observe(
+                &metrics::OPERATIONS_PER_BLOCK_ATTESTATION,
+                block.body().attestations().len() as f64,
+            );
+
+            if let Some(sync_aggregate) = block.body().sync_aggregate() {
+                metrics::set_gauge(
+                    &metrics::BLOCK_SYNC_AGGREGATE_SET_BITS,
+                    sync_aggregate.num_set_bits() as i64,
+                );
+            }
+        }
 
         let db_write_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_DB_WRITE);
 

beacon_node/beacon_chain/src/metrics.rs

@@ -56,6 +56,10 @@ lazy_static! {
         "beacon_block_processing_attestation_observation_seconds",
         "Time spent hashing and remembering all the attestations in the block"
     );
+    pub static ref BLOCK_SYNC_AGGREGATE_SET_BITS: Result<IntGauge> = try_create_int_gauge(
+        "block_sync_aggregate_set_bits",
+        "The number of true bits in the last sync aggregate in a block"
+    );
 
     /*
      * Block Production

beacon_node/beacon_chain/tests/attestation_verification.rs

@@ -223,7 +223,7 @@ fn aggregated_gossip_verification() {
                         .expect(&format!(
                             "{} should error during verify_aggregated_attestation_for_gossip",
                             $desc
-                        )),
+                        )).0,
                     $( $error ) |+ $( if $guard )?
                 ),
                 "case: {}",
@@ -606,7 +606,7 @@ fn unaggregated_gossip_verification() {
                         .expect(&format!(
                             "{} should error during verify_unaggregated_attestation_for_gossip",
                             $desc
-                        )),
+                        )).0,
                     $( $error ) |+ $( if $guard )?
                 ),
                 "case: {}",

beacon_node/beacon_chain/tests/store_tests.rs

@@ -332,7 +332,7 @@ fn epoch_boundary_state_attestation_processing() {
         {
             checked_pre_fin = true;
             assert!(matches!(
-                res.err().unwrap(),
+                res.err().unwrap().0,
                 AttnError::PastSlot {
                     attestation_slot,
                     earliest_permissible_slot,

beacon_node/beacon_chain/tests/tests.rs

@@ -538,7 +538,7 @@ fn attestations_with_increasing_slots() {
 
         if expected_attestation_slot < expected_earliest_permissible_slot {
             assert!(matches!(
-                res.err().unwrap(),
+                res.err().unwrap().0,
                 AttnError::PastSlot {
                     attestation_slot,
                     earliest_permissible_slot,

beacon_node/client/src/notifier.rs

@@ -228,7 +228,6 @@ fn eth1_logging<T: BeaconChainTypes>(beacon_chain: &BeaconChain<T>, log: &Logger
                     warn!(
                         log,
                         "Syncing eth1 block cache";
-                        "msg" => "sync can take longer when using remote eth1 nodes",
                         "est_blocks_remaining" => distance,
                     );
                 }