Merge #4773

4773: Fix: remove unused connection symmetries and other code cleanups r=EdHastingsCasperAssociation a=moubctez Changes: * remove unused connection symmetries (fixes memory draining) * remove unnecessary cloning * don't derive DataSize when all fields are skipped * hide some code that is needed only for tests * code cleanup Co-authored-by: Adam Ciarciński <[email protected]>
casper-network · Sep 11, 2024 · 615306e · 615306e
2 parents e6456b7 + 79512bd
commit 615306e
Show file tree

Hide file tree

Showing 39 changed files with 291 additions and 332 deletions.
diff --git a/node/build.rs b/node/build.rs
@@ -20,24 +20,23 @@ fn main() {
         .output()
     {
         Ok(output) => {
-            //In the event the git command is successful, export the properly formatted git hash to
+            // In the event the git command is successful, export the properly formatted git hash to
             // cargo at compile time.
             let git_hash_raw =
                 String::from_utf8(output.stdout).expect("Failed to obtain commit hash to string");
             let git_hash = git_hash_raw.trim_end_matches('\n');
 
-            println!("cargo:rustc-env={}={}", NODE_GIT_HASH_ENV_VAR, git_hash);
+            println!("cargo:rustc-env={NODE_GIT_HASH_ENV_VAR}={git_hash}");
         }
 
         Err(error) => {
-            println!("cargo:warning={}", error);
+            println!("cargo:warning={error}");
             println!("cargo:warning=casper-node build version will not include git short hash");
         }
     }
 
     println!(
-        "cargo:rustc-env={}={}",
-        NODE_BUILD_PROFILE_ENV_VAR,
+        "cargo:rustc-env={NODE_BUILD_PROFILE_ENV_VAR}={}",
         env::var(CARGO_BUILD_PROFILE_ENV_VAR).unwrap()
     );
 }
diff --git a/node/src/app/main.rs b/node/src/app/main.rs
@@ -25,12 +25,12 @@ fn panic_hook(info: &PanicInfo) {
 
     // Print panic info
     if let Some(s) = info.payload().downcast_ref::<&str>() {
-        eprintln!("node panicked: {}", s);
+        eprintln!("node panicked: {s}");
     // TODO - use `info.message()` once https://github.com/rust-lang/rust/issues/66745 is fixed
     // } else if let Some(message) = info.message() {
-    //     eprintln!("{}", message);
+    //     eprintln!("{message}");
     } else {
-        eprintln!("{}", info);
+        eprintln!("{info}");
     }
 
     // Abort after a panic, even if only a worker thread panicked.
@@ -55,7 +55,7 @@ fn main() -> anyhow::Result<()> {
         // Parse CLI args and run selected subcommand.
         let opts = Cli::from_args();
 
-        runtime.block_on(async { opts.run().await })?
+        runtime.block_on(opts.run())?
     };
 
     info!(%exit_code, "exiting casper-node");

diff --git a/node/src/cli.rs b/node/src/cli.rs
@@ -203,8 +203,7 @@ impl Cli {
 
                 let old_root = old_config
                     .parent()
-                    .map(|path| path.to_owned())
-                    .unwrap_or_else(|| "/".into());
+                    .map_or_else(|| "/".into(), Path::to_path_buf);
                 let encoded_old_config = fs::read_to_string(&old_config)
                     .context("could not read old configuration file")
                     .with_context(|| old_config.display().to_string())?;
@@ -225,8 +224,7 @@ impl Cli {
 
                 let old_root = old_config
                     .parent()
-                    .map(|path| path.to_owned())
-                    .unwrap_or_else(|| "/".into());
+                    .map_or_else(|| "/".into(), Path::to_path_buf);
                 let encoded_old_config = fs::read_to_string(&old_config)
                     .context("could not read old configuration file")
                     .with_context(|| old_config.display().to_string())?;
@@ -251,8 +249,7 @@ impl Cli {
         // Otherwise, we default to `/`.
         let root = config
             .parent()
-            .map(|path| path.to_owned())
-            .unwrap_or_else(|| "/".into());
+            .map_or_else(|| "/".into(), Path::to_path_buf);
 
         // The app supports running without a config file, using default values.
         let encoded_config = fs::read_to_string(config)

diff --git a/node/src/cli/arglang.rs b/node/src/cli/arglang.rs
@@ -64,14 +64,14 @@ fn tokenize(input: &str) -> Result<Vec<Token>, Error> {
         // Check if we need to complete a token.
         if !buffer.is_empty() {
             match ch {
-                Some(' ') | Some('"') | Some('[') | Some(']') | Some(',') | None => {
+                Some(' ' | '"' | '[' | ']' | ',') | None => {
                     // Try to parse as number or bool first.
                     if let Ok(value) = i64::from_str(&buffer) {
                         tokens.push(Token::I64(value));
                     } else if let Ok(value) = bool::from_str(&buffer) {
                         tokens.push(Token::Boolean(value));
                     } else {
-                        tokens.push(Token::String(buffer.clone()))
+                        tokens.push(Token::String(buffer.clone()));
                     }
 
                     buffer.clear();
@@ -161,7 +161,7 @@ where
                 }
             }
         }
-        Some(t @ Token::CloseBracket) | Some(t @ Token::Comma) => Err(Error::UnexpectedToken(t)),
+        Some(t @ (Token::CloseBracket | Token::Comma)) => Err(Error::UnexpectedToken(t)),
         None => Err(Error::UnexpectedEndOfInput),
     }
 }

diff --git a/node/src/components/binary_port.rs b/node/src/components/binary_port.rs
@@ -1199,7 +1199,7 @@ where
             };
             let block_rewards = match era_end.rewards() {
                 Rewards::V2(rewards) => rewards,
-                _ => {
+                Rewards::V1(_) => {
                     return BinaryResponse::new_error(
                         ErrorCode::UnsupportedRewardsV1Request,
                         protocol_version,
@@ -1349,7 +1349,7 @@ where
         .await
         .map_or_else(
             |err| BinaryResponse::new_error(err.into(), protocol_version),
-            |_| BinaryResponse::new_empty(protocol_version),
+            |()| BinaryResponse::new_empty(protocol_version),
         )
 }
 
@@ -1794,7 +1794,7 @@ where
                         let response =
                             handle_request(request, effect_builder, &config, &chainspec, &metrics)
                                 .await;
-                        responder.respond(response).await
+                        responder.respond(response).await;
                     }
                     .ignore()
                 }

diff --git a/node/src/components/block_accumulator/block_acceptor.rs b/node/src/components/block_accumulator/block_acceptor.rs
@@ -286,27 +286,26 @@ impl BlockAcceptor {
                         },
                         faulty_senders,
                     );
-                } else {
-                    if meta_block
-                        .state
-                        .register_as_stored()
-                        .was_already_registered()
-                    {
-                        error!(
-                            %block_hash,
-                            block_height = meta_block.block.height(),
-                            meta_block_state = ?meta_block.state,
-                            "should not store the same block more than once"
-                        );
-                    }
-                    return (
-                        ShouldStore::SufficientlySignedBlock {
-                            meta_block: meta_block.clone(),
-                            block_signatures,
-                        },
-                        faulty_senders,
+                }
+                if meta_block
+                    .state
+                    .register_as_stored()
+                    .was_already_registered()
+                {
+                    error!(
+                        %block_hash,
+                        block_height = meta_block.block.height(),
+                        meta_block_state = ?meta_block.state,
+                        "should not store the same block more than once"
                     );
                 }
+                return (
+                    ShouldStore::SufficientlySignedBlock {
+                        meta_block: meta_block.clone(),
+                        block_signatures,
+                    },
+                    faulty_senders,
+                );
             }
         }
 

diff --git a/node/src/components/block_synchronizer/execution_results_acquisition.rs b/node/src/components/block_synchronizer/execution_results_acquisition.rs
@@ -3,7 +3,7 @@ mod tests;
 
 use std::{
     collections::HashMap,
-    fmt::{self, Debug, Display, Formatter},
+    fmt::{self, Display, Formatter},
 };
 
 use datasize::DataSize;

diff --git a/node/src/components/block_validator.rs b/node/src/components/block_validator.rs
@@ -552,7 +552,7 @@ impl BlockValidator {
             Err(error) => warn!(%transaction_hash, %error, "could not fetch transaction"),
         }
         match result {
-            Ok(FetchedData::FromStorage { item }) | Ok(FetchedData::FromPeer { item, .. }) => {
+            Ok(FetchedData::FromStorage { item } | FetchedData::FromPeer { item, .. }) => {
                 let item_hash = item.hash();
                 if item_hash != transaction_hash {
                     // Hard failure - change state to Invalid.
@@ -670,7 +670,7 @@ impl BlockValidator {
             }
         }
         match result {
-            Ok(FetchedData::FromStorage { .. }) | Ok(FetchedData::FromPeer { .. }) => {
+            Ok(FetchedData::FromStorage { .. } | FetchedData::FromPeer { .. }) => {
                 let mut effects = Effects::new();
                 for state in self.validation_states.values_mut() {
                     let responders = state.try_add_signature(&finality_signature_id);

diff --git a/node/src/components/consensus/highway_core/state.rs b/node/src/components/consensus/highway_core/state.rs
@@ -452,7 +452,7 @@ impl<C: Context> State<C> {
         }
         let idx = evidence.perpetrator();
         match self.faults.get(&idx) {
-            Some(&Fault::Banned) | Some(&Fault::Direct(_)) => return false,
+            Some(&Fault::Banned | &Fault::Direct(_)) => return false,
             None | Some(&Fault::Indirect) => (),
         }
         // TODO: Should use Display, not Debug!

diff --git a/node/src/components/consensus/highway_core/state/panorama.rs b/node/src/components/consensus/highway_core/state/panorama.rs
@@ -215,7 +215,7 @@ impl<C: Context> Panorama<C> {
                 .find(|(_, unit)| unit.timestamp <= timestamp)
                 .map(|(vh, _)| *vh)
                 .map_or(Observation::None, Observation::Correct),
-            obs @ Observation::None | obs @ Observation::Faulty => obs.clone(),
+            obs @ (Observation::None | Observation::Faulty) => obs.clone(),
         };
         Panorama::from(self.iter().map(obs_cutoff).collect_vec())
     }

diff --git a/node/src/components/consensus/protocols/highway.rs b/node/src/components/consensus/protocols/highway.rs
@@ -339,9 +339,8 @@ impl<C: Context + 'static> HighwayProtocol<C> {
                     });
                 }
                 return outcomes;
-            } else {
-                self.log_proposal(vertex, "proposal does not need validation");
             }
+            self.log_proposal(vertex, "proposal does not need validation");
         }
 
         // Either consensus value doesn't need validation or it's not a proposal.

diff --git a/node/src/components/consensus/protocols/zug/participation.rs b/node/src/components/consensus/protocols/zug/participation.rs
@@ -50,9 +50,8 @@ impl ParticipationStatus {
             {
                 if r_id.saturating_add(2) < zug.current_round {
                     return Some(ParticipationStatus::LastSeenInRound(*r_id));
-                } else {
-                    return None; // Seen recently; considered currently active.
                 }
+                return None; // Seen recently; considered currently active.
             }
         }
         Some(ParticipationStatus::Inactive)

diff --git a/node/src/components/event_stream_server.rs b/node/src/components/event_stream_server.rs
@@ -232,7 +232,7 @@ where
 
     fn handle_event(
         &mut self,
-        _effect_builder: EffectBuilder<REv>,
+        effect_builder: EffectBuilder<REv>,
         _rng: &mut NodeRng,
         event: Self::Event,
     ) -> Effects<Self::Event> {
@@ -256,7 +256,7 @@ where
             }
             ComponentState::Initializing => match event {
                 Event::Initialize => {
-                    let (effects, state) = self.bind(self.config.enable_server, _effect_builder);
+                    let (effects, state) = self.bind(self.config.enable_server, effect_builder);
                     <Self as InitializedComponent<MainEvent>>::set_state(self, state);
                     effects
                 }

diff --git a/node/src/components/event_stream_server/sse_server.rs b/node/src/components/event_stream_server/sse_server.rs
@@ -234,7 +234,7 @@ pub(super) struct NewSubscriberInfo {
 
 /// Maps the `event` to a warp event, or `None` if it's a malformed event (ie.: `ApiVersion` event
 /// with `id` set or event other than `ApiVersion` without `id`)
-async fn map_server_sent_event(
+fn map_server_sent_event(
     event: &ServerSentEvent,
 ) -> Option<Result<WarpServerSentEvent, RecvError>> {
     let id = match event.id {
@@ -292,7 +292,7 @@ async fn map_server_sent_event(
 ///
 /// If `query` is not empty, returns a 422 response if `query` doesn't have exactly one entry,
 /// "starts_from" mapped to a value representing an event ID.
-fn parse_query(query: HashMap<String, String>) -> Result<Option<Id>, Response> {
+fn parse_query(query: &HashMap<String, String>) -> Result<Option<Id>, Response> {
     if query.is_empty() {
         return Ok(None);
     }
@@ -374,7 +374,7 @@ impl ChannelsAndFilter {
                 return create_503();
             }
 
-            let start_from = match parse_query(query) {
+            let start_from = match parse_query(&query) {
                 Ok(maybe_id) => maybe_id,
                 Err(error_response) => return error_response,
             };
@@ -489,7 +489,7 @@ fn stream_to_client(
         .chain(ongoing_stream)
         .filter_map(move |result| async move {
             match result {
-                Ok(event) => map_server_sent_event(&event).await,
+                Ok(event) => map_server_sent_event(&event),
                 Err(error) => Some(Err(error)),
             }
         })

diff --git a/node/src/components/fetcher/item_fetcher.rs b/node/src/components/fetcher/item_fetcher.rs
@@ -241,10 +241,10 @@ pub(super) trait ItemFetcher<T: FetchItem + 'static> {
                 }
             }
             Err(
-                error @ Error::Absent { .. }
-                | error @ Error::Rejected { .. }
-                | error @ Error::CouldNotConstructGetRequest { .. }
-                | error @ Error::ValidationMetadataMismatch { .. },
+                error @ (Error::Absent { .. }
+                | Error::Rejected { .. }
+                | Error::CouldNotConstructGetRequest { .. }
+                | Error::ValidationMetadataMismatch { .. }),
             ) => {
                 // For all other error variants we can safely respond with failure as there's no
                 // chance for the request to succeed.

diff --git a/node/src/components/gossiper/gossip_table.rs b/node/src/components/gossiper/gossip_table.rs
@@ -136,9 +136,8 @@ impl State {
                     exclude_peers: self.attempted_to_infect.clone(),
                     is_already_held: !is_new,
                 });
-            } else {
-                return GossipAction::Noop;
             }
+            return GossipAction::Noop;
         }
 
         if is_new {