Addressed Pawan / Michael's latest comments

sigp · Oct 29, 2022 · 0330fff · 0330fff
1 parent 15edbe6
commit 0330fff
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 58 deletions.
diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs
@@ -277,19 +277,50 @@ where
                     BeaconNodeHttpClient::new(url, Timeouts::set_all(CHECKPOINT_SYNC_HTTP_TIMEOUT));
                 let slots_per_epoch = TEthSpec::slots_per_epoch();
 
-                // We want to fetch deposit snapshot before fetching the finalized beacon state to
-                // ensure that the snapshot is not newer than the beacon state that satisfies the
-                // deposit finalization conditions
-                debug!(context.log(), "Downloading deposit snapshot");
-                let deposit_snapshot_result =
-                    remote.get_deposit_snapshot().await.map_err(|e| match e {
-                        ApiError::InvalidSsz(e) => format!(
-                            "Unable to parse SSZ: {:?}. Ensure the checkpoint-sync-url refers to a \
-                            node for the correct network",
-                            e
-                        ),
-                        e => format!("Error fetching deposit snapshot from remote: {:?}", e),
-                    });
+                let deposit_snapshot = if config.sync_eth1_chain {
+                    // We want to fetch deposit snapshot before fetching the finalized beacon state to
+                    // ensure that the snapshot is not newer than the beacon state that satisfies the
+                    // deposit finalization conditions
+                    debug!(context.log(), "Downloading deposit snapshot");
+                    let deposit_snapshot_result = remote
+                        .get_deposit_snapshot()
+                        .await
+                        .map_err(|e| match e {
+                            ApiError::InvalidSsz(e) => format!(
+                                "Unable to parse SSZ: {:?}. Ensure the checkpoint-sync-url refers to a \
+                                node for the correct network",
+                                e
+                            ),
+                            e => format!("Error fetching deposit snapshot from remote: {:?}", e),
+                        });
+                    match deposit_snapshot_result {
+                        Ok(Some(deposit_snapshot)) => {
+                            if deposit_snapshot.is_valid() {
+                                Some(deposit_snapshot)
+                            } else {
+                                warn!(context.log(), "Remote BN sent invalid deposit snapshot!");
+                                None
+                            }
+                        }
+                        Ok(None) => {
+                            warn!(
+                                context.log(),
+                                "Remote BN does not support EIP-4881 fast deposit sync"
+                            );
+                            None
+                        }
+                        Err(e) => {
+                            warn!(
+                                context.log(),
+                                "Remote BN does not support EIP-4881 fast deposit sync";
+                                "error" => e
+                            );
+                            None
+                        }
+                    }
+                } else {
+                    None
+                };
 
                 debug!(context.log(), "Downloading finalized block");
                 // Find a suitable finalized block on an epoch boundary.
@@ -375,52 +406,30 @@ where
                     "state_root" => ?state_root,
                 );
 
-                let service = match deposit_snapshot_result {
-                    Ok(Some(deposit_snapshot)) => {
-                        if deposit_snapshot.is_valid() {
-                            match Eth1Service::from_deposit_snapshot(
-                                config.eth1,
-                                context.log().clone(),
-                                spec,
-                                &deposit_snapshot,
-                            ) {
-                                Ok(service) => {
-                                    info!(
-                                        context.log(),
-                                        "Loaded deposit tree snapshot";
-                                        "deposits loaded" => deposit_snapshot.deposit_count,
-                                    );
-                                    Some(service)
-                                }
-                                Err(e) => {
-                                    warn!(context.log(),
-                                        "Unable to load deposit snapshot";
-                                        "error" => ?e
-                                    );
-                                    None
-                                }
-                            }
-                        } else {
-                            warn!(context.log(), "Remote BN sent invalid deposit snapshot!");
+                let service = deposit_snapshot.map(|snapshot| {
+                    match Eth1Service::from_deposit_snapshot(
+                        config.eth1,
+                        context.log().clone(),
+                        spec,
+                        &snapshot,
+                    ) {
+                        Ok(service) => {
+                            info!(
+                                context.log(),
+                                "Loaded deposit tree snapshot";
+                                "deposits loaded" => deposit_snapshot.deposit_count,
+                            );
+                            service
+                        }
+                        Err(e) => {
+                            warn!(context.log(),
+                                "Unable to load deposit snapshot";
+                                "error" => ?e
+                            );
                             None
                         }
                     }
-                    Ok(None) => {
-                        warn!(
-                            context.log(),
-                            "Remote BN does not support EIP-4881 fast deposit sync"
-                        );
-                        None
-                    }
-                    Err(e) => {
-                        warn!(
-                            context.log(),
-                            "Remote BN does not support EIP-4881 fast deposit sync";
-                            "error" => e
-                        );
-                        None
-                    }
-                };
+                });
 
                 builder
                     .weak_subjectivity_state(state, block, genesis_state)

diff --git a/consensus/types/src/deposit_tree_snapshot.rs b/consensus/types/src/deposit_tree_snapshot.rs
@@ -72,8 +72,7 @@ impl DepositTreeSnapshot {
     }
     pub fn is_valid(&self) -> bool {
         self.calculate_root()
-            .map(|calculated| self.deposit_root == calculated)
-            .unwrap_or(false)
+            .map_or(false, |calculated| self.deposit_root == calculated)
     }
 }