chore: implement some coderabbit suggestions

Signed-off-by: etorreborre <etorreborre@yahoo.com>

Author: etorreborre

crates/amaru-consensus/src/consensus/headers_tree/data_generation/actions.rs

@@ -336,6 +336,16 @@ impl FromStr for Ratio {
         let denominator = parts[1]
             .parse::<u32>()
             .map_err(|_| format!("Invalid denominator in ratio: {}", parts[1]))?;
+
+        if denominator == 0 {
+            return Err("Ratio denominator must be > 0".to_string());
+        }
+        if numerator > denominator {
+            return Err(format!(
+                "Ratio must be <= 1.0, got {}/{}",
+                numerator, denominator
+            ));
+        }
         Ok(Ratio(numerator, denominator))
     }
 }
@@ -615,6 +625,8 @@ where
 
 #[cfg(test)]
 mod tests {
+    use super::*;
+
     #[test]
     fn transpose_works() {
         let rows = vec![
@@ -631,7 +643,37 @@ mod tests {
             vec![3, 8],
             vec![9],
         ];
-        let result = super::transpose(rows);
+        let result = transpose(rows);
         assert_eq!(result, expected);
     }
+
+    #[test]
+    fn parse_ratio_works() {
+        let ratio: Ratio = "3/4".parse().unwrap();
+        assert_eq!(ratio, Ratio(3, 4));
+
+        let ratio: super::Ratio = "0/1".parse().unwrap();
+        assert_eq!(ratio, Ratio(0, 1));
+
+        let ratio: super::Ratio = "1/1".parse().unwrap();
+        assert_eq!(ratio, Ratio(1, 1));
+    }
+
+    #[test]
+    fn parse_ratio_with_errors() {
+        let err = "3".parse::<Ratio>().unwrap_err();
+        assert_eq!(err, "Ratio must be in the form 'numerator/denominator'");
+
+        let err = "a/2".parse::<Ratio>().unwrap_err();
+        assert_eq!(err, "Invalid numerator in ratio: a");
+
+        let err = "1/b".parse::<Ratio>().unwrap_err();
+        assert_eq!(err, "Invalid denominator in ratio: b");
+
+        let err = "1/0".parse::<Ratio>().unwrap_err();
+        assert_eq!(err, "Ratio denominator must be > 0");
+
+        let err = "5/4".parse::<Ratio>().unwrap_err();
+        assert_eq!(err, "Ratio must be <= 1.0, got 5/4");
+    }
 }

simulation/amaru-sim/src/simulator/data_generation/generate.rs

@@ -30,7 +30,7 @@ pub fn generate_entries<R: Rng>(
     node_config: &NodeConfig,
     start_time: Instant,
     mean_millis: f64,
-) -> impl Fn(&mut R) -> Vec<Entry<ChainSyncMessage>> + use<'_, R> {
+) -> impl Fn(&mut R) -> Vec<Entry<ChainSyncMessage>> {
     move |rng: &mut R| {
         let actions = run_with_rng(
             rng,

simulation/amaru-sim/src/simulator/run.rs

@@ -319,20 +319,25 @@ pub fn make_best_chain_from_downstream_messages(history: &History<ChainSyncMessa
         };
         match &message.body {
             msg @ ChainSyncMessage::Fwd { .. } => {
-                best_chain.push(msg.decode_block_header().unwrap())
+                if let Some(header) = msg.decode_block_header() {
+                    best_chain.push(header)
+                }
             }
             msg @ ChainSyncMessage::Bck { .. } => {
-                let header_hash = msg.header_hash().unwrap();
-                let rollback_position = best_chain.iter().position(|h| h.hash() == header_hash);
-                assert!(
-                    rollback_position.is_some(),
-                    "after the action {}, we have a rollback position that does not exist with hash {}.\nThe best chain is:\n{}. The history is:\n{}",
-                    i + 1,
-                    header_hash,
-                    best_chain.list_to_string(",\n"),
-                    history.0.iter().collect::<Vec<_>>().list_debug("\n")
-                );
-                best_chain.truncate(rollback_position.unwrap() + 1);
+                if let Some(header_hash) = msg.header_hash() {
+                    let rollback_position = best_chain.iter().position(|h| h.hash() == header_hash);
+                    if let Some(rollback_position) = rollback_position {
+                        best_chain.truncate(rollback_position + 1);
+                    } else {
+                        panic!(
+                            "after the action {}, we have a rollback position that does not exist with hash {}.\nThe best chain is:\n{}. The history is:\n{}",
+                            i + 1,
+                            header_hash,
+                            best_chain.list_to_string(",\n"),
+                            history.0.iter().collect::<Vec<_>>().list_debug("\n")
+                        );
+                    }
+                }
             }
             _ => (),
         }