chore(l1): remove/justify clones from common

crates/common/serde_utils.rs

@@ -523,7 +523,7 @@ pub mod duration {
         D: Deserializer<'de>,
     {
         let value = String::deserialize(d)?;
-        parse_duration(value.clone())
+        parse_duration(&value)
             .ok_or_else(|| D::Error::custom(format!("Failed to parse Duration: {value}")))
     }
 
@@ -535,7 +535,7 @@ pub mod duration {
             D: Deserializer<'de>,
         {
             if let Some(value) = Option::<String>::deserialize(d)? {
-                Ok(Some(parse_duration(value.clone()).ok_or_else(|| {
+                Ok(Some(parse_duration(&value).ok_or_else(|| {
                     D::Error::custom(format!("Failed to parse Duration: {value}"))
                 })?))
             } else {
@@ -551,7 +551,7 @@ pub mod duration {
 /// For example, a duration such as "1h30m" or "1.6m" will be accepted but "-1s" or "30mh" will not
 /// Some imprecision can be expected when using milliseconds/microseconds/nanoseconds with significant decimal components
 /// If the format is incorrect this function will return None
-fn parse_duration(input: String) -> Option<Duration> {
+fn parse_duration(input: &str) -> Option<Duration> {
     let mut res = Duration::ZERO;
     let mut integer_buffer = String::new();
     let mut chars = input.chars().peekable();
@@ -614,96 +614,60 @@ mod tests {
     #[test]
     fn parse_duration_simple_integers() {
         assert_eq!(
-            parse_duration("24h".to_string()),
+            parse_duration("24h"),
             Some(Duration::from_secs(60 * 60 * 24))
         );
-        assert_eq!(
-            parse_duration("20m".to_string()),
-            Some(Duration::from_secs(60 * 20))
-        );
-        assert_eq!(
-            parse_duration("13s".to_string()),
-            Some(Duration::from_secs(13))
-        );
-        assert_eq!(
-            parse_duration("500ms".to_string()),
-            Some(Duration::from_millis(500))
-        );
-        assert_eq!(
-            parse_duration("900µs".to_string()),
-            Some(Duration::from_micros(900))
-        );
-        assert_eq!(
-            parse_duration("900us".to_string()),
-            Some(Duration::from_micros(900))
-        );
-        assert_eq!(
-            parse_duration("40ns".to_string()),
-            Some(Duration::from_nanos(40))
-        );
+        assert_eq!(parse_duration("20m"), Some(Duration::from_secs(60 * 20)));
+        assert_eq!(parse_duration("13s"), Some(Duration::from_secs(13)));
+        assert_eq!(parse_duration("500ms"), Some(Duration::from_millis(500)));
+        assert_eq!(parse_duration("900µs"), Some(Duration::from_micros(900)));
+        assert_eq!(parse_duration("900us"), Some(Duration::from_micros(900)));
+        assert_eq!(parse_duration("40ns"), Some(Duration::from_nanos(40)));
     }
 
     #[test]
     fn parse_duration_mixed_integers() {
         assert_eq!(
-            parse_duration("24h30m".to_string()),
+            parse_duration("24h30m"),
             Some(Duration::from_secs(60 * 60 * 24 + 30 * 60))
         );
         assert_eq!(
-            parse_duration("20m15s".to_string()),
+            parse_duration("20m15s"),
             Some(Duration::from_secs(60 * 20 + 15))
         );
         assert_eq!(
-            parse_duration("13s4ms".to_string()),
+            parse_duration("13s4ms"),
             Some(Duration::from_secs(13) + Duration::from_millis(4))
         );
         assert_eq!(
-            parse_duration("500ms60µs".to_string()),
+            parse_duration("500ms60µs"),
             Some(Duration::from_millis(500) + Duration::from_micros(60))
         );
         assert_eq!(
-            parse_duration("900us21ns".to_string()),
+            parse_duration("900us21ns"),
             Some(Duration::from_micros(900) + Duration::from_nanos(21))
         );
     }
 
     #[test]
     fn parse_duration_simple_with_decimals() {
-        assert_eq!(
-            parse_duration("1.5h".to_string()),
-            Some(Duration::from_secs(60 * 90))
-        );
-        assert_eq!(
-            parse_duration("0.5m".to_string()),
-            Some(Duration::from_secs(30))
-        );
-        assert_eq!(
-            parse_duration("4.5s".to_string()),
-            Some(Duration::from_secs_f32(4.5))
-        );
-        assert_eq!(
-            parse_duration("0.8ms".to_string()),
-            Some(Duration::from_micros(800))
-        );
-        assert_eq!(
-            parse_duration("0.95us".to_string()),
-            Some(Duration::from_nanos(950))
-        );
+        assert_eq!(parse_duration("1.5h"), Some(Duration::from_secs(60 * 90)));
+        assert_eq!(parse_duration("0.5m"), Some(Duration::from_secs(30)));
+        assert_eq!(parse_duration("4.5s"), Some(Duration::from_secs_f32(4.5)));
+        assert_eq!(parse_duration("0.8ms"), Some(Duration::from_micros(800)));
+        assert_eq!(parse_duration("0.95us"), Some(Duration::from_nanos(950)));
         // Rounded Up
-        assert_eq!(
-            parse_duration("0.75ns".to_string()),
-            Some(Duration::from_nanos(1))
-        );
+        assert_eq!(parse_duration("0.75ns"), Some(Duration::from_nanos(1)));
     }
 
     #[test]
     fn parse_duration_mixed_decimals() {
         assert_eq!(
-            parse_duration("1.5h0.5m10s".to_string()),
+            parse_duration("1.5h0.5m10s"),
             Some(Duration::from_secs(60 * 90 + 30 + 10))
         );
         assert_eq!(
-            parse_duration("0.5m15s".to_string()),
+            parse_duration("0.5m15s"),
             Some(Duration::from_secs(30 + 15))
         );
     }

crates/common/types/blobs_bundle.rs

@@ -80,20 +80,20 @@ impl BlobsBundle {
 
     // In the future we might want to provide a new method that calculates the commitments and proofs using the following.
     #[cfg(feature = "c-kzg")]
-    pub fn create_from_blobs(blobs: &Vec<Blob>) -> Result<Self, BlobsBundleError> {
+    pub fn create_from_blobs(blobs: Vec<Blob>) -> Result<Self, BlobsBundleError> {
         use ethrex_crypto::kzg::blob_to_kzg_commitment_and_proof;
         let mut commitments = Vec::new();
         let mut proofs = Vec::new();
 
         // Populate the commitments and proofs
-        for blob in blobs {
+        for blob in &blobs {
             let (commitment, proof) = blob_to_kzg_commitment_and_proof(blob)?;
             commitments.push(commitment);
             proofs.push(proof);
         }
 
         Ok(Self {
-            blobs: blobs.clone(),
+            blobs,
             commitments,
             proofs,
             version: 0,
@@ -245,6 +245,8 @@ pub enum BlobsBundleError {
 
 #[cfg(test)]
 mod tests {
+    use ethereum_types::Address;
+
     mod shared {
         #[cfg(feature = "c-kzg")]
         pub fn convert_str_to_bytes48(s: &str) -> [u8; 48] {
@@ -266,7 +268,7 @@ mod tests {
             })
             .collect();
 
-        let blobs_bundle = crate::types::BlobsBundle::create_from_blobs(&blobs)
+        let blobs_bundle = crate::types::BlobsBundle::create_from_blobs(blobs)
             .expect("Failed to create blobs bundle");
 
         let blob_versioned_hashes = blobs_bundle.generate_versioned_hashes();
@@ -277,10 +279,10 @@ mod tests {
             max_fee_per_gas: 0,
             max_fee_per_blob_gas: 0.into(),
             gas: 15_000_000,
-            to: crate::Address::from_low_u64_be(1), // Normal tx
-            value: crate::U256::zero(),             // Value zero
-            data: crate::Bytes::default(),          // No data
-            access_list: Default::default(),        // No access list
+            to: Address::from_low_u64_be(1), // Normal tx
+            value: crate::U256::zero(),      // Value zero
+            data: crate::Bytes::default(),   // No data
+            access_list: Default::default(), // No access list
             blob_versioned_hashes,
             ..Default::default()
         };
@@ -320,10 +322,10 @@ mod tests {
             max_fee_per_gas: 0,
             max_fee_per_blob_gas: 0.into(),
             gas: 15_000_000,
-            to: crate::Address::from_low_u64_be(1), // Normal tx
-            value: crate::U256::zero(),             // Value zero
-            data: crate::Bytes::default(),          // No data
-            access_list: Default::default(),        // No access list
+            to: Address::from_low_u64_be(1), // Normal tx
+            value: crate::U256::zero(),      // Value zero
+            data: crate::Bytes::default(),   // No data
+            access_list: Default::default(), // No access list
             blob_versioned_hashes: vec![
                 "01ec8054d05bfec80f49231c6e90528bbb826ccd1464c255f38004099c8918d9",
                 "0180cb2dee9e6e016fabb5da4fb208555f5145c32895ccd13b26266d558cd77d",
@@ -372,10 +374,10 @@ mod tests {
             max_fee_per_gas: 0,
             max_fee_per_blob_gas: 0.into(),
             gas: 15_000_000,
-            to: crate::Address::from_low_u64_be(1), // Normal tx
-            value: crate::U256::zero(),             // Value zero
-            data: crate::Bytes::default(),          // No data
-            access_list: Default::default(),        // No access list
+            to: Address::from_low_u64_be(1), // Normal tx
+            value: crate::U256::zero(),      // Value zero
+            data: crate::Bytes::default(),   // No data
+            access_list: Default::default(), // No access list
             blob_versioned_hashes: vec![
                 "01ec8054d05bfec80f49231c6e90528bbb826ccd1464c255f38004099c8918d9",
                 "0180cb2dee9e6e016fabb5da4fb208555f5145c32895ccd13b26266d558cd77d",
@@ -403,7 +405,7 @@ mod tests {
         let blobs =
             std::iter::repeat_n(blob, super::MAX_BLOB_COUNT_ELECTRA + 1).collect::<Vec<_>>();
 
-        let blobs_bundle = crate::types::BlobsBundle::create_from_blobs(&blobs)
+        let blobs_bundle = crate::types::BlobsBundle::create_from_blobs(blobs)
             .expect("Failed to create blobs bundle");
 
         let blob_versioned_hashes = blobs_bundle.generate_versioned_hashes();
@@ -414,10 +416,10 @@ mod tests {
             max_fee_per_gas: 0,
             max_fee_per_blob_gas: 0.into(),
             gas: 15_000_000,
-            to: crate::Address::from_low_u64_be(1), // Normal tx
-            value: crate::U256::zero(),             // Value zero
-            data: crate::Bytes::default(),          // No data
-            access_list: Default::default(),        // No access list
+            to: Address::from_low_u64_be(1), // Normal tx
+            value: crate::U256::zero(),      // Value zero
+            data: crate::Bytes::default(),   // No data
+            access_list: Default::default(), // No access list
             blob_versioned_hashes,
             ..Default::default()
         };

crates/common/types/block_execution_witness.rs

@@ -287,7 +287,7 @@ impl GuestProgramState {
                 }
 
                 state_trie
-                    .insert(hashed_address.clone(), account_state.encode_to_vec())
+                    .insert(hashed_address.clone(), account_state.encode_to_vec()) // ok-clone: copy of hash address is needed in trie, and account_updates can't be consumed by this point
                     .expect("failed to insert into storage");
             }
         }

crates/common/types/fork_id.rs

@@ -126,7 +126,7 @@ fn get_all_fork_id_combinations(forks: Vec<u64>, genesis_hash: BlockHash) -> Vec
             continue;
         }
         combinations.push((
-            H32::from_slice(&hasher.clone().finalize().to_be_bytes()),
+            H32::from_slice(&hasher.clone().finalize().to_be_bytes()), // ok-clone: a new hasher would need to be initialized for each combination anyways
             activation,
         ));
         hasher.update(&activation.to_be_bytes());
@@ -230,7 +230,7 @@ mod tests {
         for test_case in test_cases {
             let fork_id = ForkId::new(
                 chain_config,
-                genesis_header.clone(),
+                genesis_header.clone(), // ok-clone: value is needed later, clone is in test
                 test_case.time,
                 test_case.head,
             );
@@ -240,7 +240,7 @@ mod tests {
                     test_case.head,
                     test_case.time,
                     chain_config,
-                    genesis_header.clone()
+                    genesis_header.clone() // ok-clone: value is needed later, clone is in test
                 ),
                 test_case.is_valid
             )
@@ -650,7 +650,7 @@ mod tests {
                 is_valid: true,
             },
         ];
-        assert_test_cases(test_cases, genesis.config, genesis_hash.clone());
+        assert_test_cases(test_cases, genesis.config, genesis_hash);
     }
 
     #[test]
@@ -669,7 +669,7 @@ mod tests {
             fork_next: 100,
         };
         let result_a = local_a.is_valid(
-            remote.clone(),
+            remote.clone(), // ok-clone: value is needed later, clone is in test
             local_head_block_number,
             0,
             ChainConfig::default(),

crates/common/types/genesis.rs

@@ -671,7 +671,7 @@ impl Genesis {
             gas_limit: self.gas_limit,
             gas_used: 0,
             timestamp: self.timestamp,
-            extra_data: self.extra_data.clone(),
+            extra_data: self.extra_data.clone(), // ok-clone: this function is only called in tests. We should be careful to not use it elsewhere though
             prev_randao: self.mix_hash,
             nonce: self.nonce,
             base_fee_per_gas,

crates/common/types/receipt.rs

@@ -266,25 +266,25 @@ impl RLPDecode for ReceiptWithBloom {
     }
 }
 
-impl From<&Receipt> for ReceiptWithBloom {
-    fn from(receipt: &Receipt) -> Self {
+impl From<Receipt> for ReceiptWithBloom {
+    fn from(receipt: Receipt) -> Self {
         Self {
             tx_type: receipt.tx_type,
             succeeded: receipt.succeeded,
             cumulative_gas_used: receipt.cumulative_gas_used,
             bloom: bloom_from_logs(&receipt.logs),
-            logs: receipt.logs.clone(),
+            logs: receipt.logs,
         }
     }
 }
 
-impl From<&ReceiptWithBloom> for Receipt {
-    fn from(receipt: &ReceiptWithBloom) -> Self {
+impl From<ReceiptWithBloom> for Receipt {
+    fn from(receipt: ReceiptWithBloom) -> Self {
         Self {
             tx_type: receipt.tx_type,
             succeeded: receipt.succeeded,
             cumulative_gas_used: receipt.cumulative_gas_used,
-            logs: receipt.logs.clone(),
+            logs: receipt.logs,
         }
     }
 }