review comments

tari-project · Oct 13, 2023 · abdf660 · abdf660
1 parent dd63e81
commit abdf660
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 19 deletions.
diff --git a/base_layer/core/src/transactions/tari_amount.rs b/base_layer/core/src/transactions/tari_amount.rs
@@ -120,10 +120,7 @@ impl MicroMinotari {
 
     pub fn checked_sub<T>(&self, v: T) -> Option<MicroMinotari>
     where T: AsRef<MicroMinotari> {
-        if self >= v.as_ref() {
-            return Some(self - v.as_ref());
-        }
-        None
+        self.as_u64().checked_sub(v.as_ref().as_u64()).map(Into::into)
     }
 
     pub fn checked_mul<T>(&self, v: T) -> Option<MicroMinotari>
@@ -138,15 +135,12 @@ impl MicroMinotari {
 
     pub fn saturating_sub<T>(&self, v: T) -> MicroMinotari
     where T: AsRef<MicroMinotari> {
-        if self >= v.as_ref() {
-            return self - v.as_ref();
-        }
-        Self(0)
+        self.as_u64().saturating_sub(v.as_ref().as_u64()).into()
     }
 
     pub fn saturating_add<T>(&self, v: T) -> MicroMinotari
     where T: AsRef<MicroMinotari> {
-        self.0.saturating_add(v.as_ref().0).into()
+        self.as_u64().saturating_add(v.as_ref().as_u64()).into()
     }
 
     #[inline]
@@ -182,16 +176,18 @@ impl From<MicroMinotari> for u64 {
     }
 }
 
-impl std::str::FromStr for MicroMinotari {
+impl FromStr for MicroMinotari {
     type Err = MicroMinotariError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let processed = s.replace([',', ' '], "").to_ascii_lowercase();
         // Is this Tari or MicroMinotari
         let is_micro_tari = if processed.ends_with("ut") || processed.ends_with("µt") {
             true
+        } else if processed.ends_with('t') {
+            false
         } else {
-            !processed.ends_with('t')
+            !processed.contains('.')
         };
 
         let processed = processed.replace("ut", "").replace("µt", "").replace('t', "");
@@ -345,16 +341,22 @@ impl FromStr for Minotari {
     type Err = MicroMinotariError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let d = Decimal::from_str(s).map_err(|e| MicroMinotariError::ParseError(e.to_string()))?;
-        Self::try_from(d)
+        if s.to_ascii_lowercase().contains('t') {
+            let val = MicroMinotari::from_str(s)?;
+            Ok(Minotari::from(val))
+        } else {
+            let d = Decimal::from_str(s).map_err(|e| MicroMinotariError::ParseError(e.to_string()))?;
+            Self::try_from(d)
+        }
     }
 }
 
 impl Display for Minotari {
     fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
-        // User can choose decimal precision, but default is 6
+        let d1 = Decimal::try_from(self.0.as_u64()).expect("will succeed");
+        let d2 = Decimal::try_from(1_000_000f64).expect("will succeed");
         let precision = f.precision().unwrap_or(6);
-        write!(f, "{1:.*} T", precision, self.0.as_u64() as f64 / 1_000_000f64)
+        write!(f, "{1:.*} T", precision, d1 / d2)
     }
 }
 
@@ -521,4 +523,43 @@ mod test {
         );
         assert_eq!(s, "99.100000 T");
     }
+
+    #[test]
+    fn to_string_from_string_max_conversion() {
+        let max_value = MicroMinotari(u64::MAX);
+
+        assert_eq!(max_value.as_u64().to_string(), "18446744073709551615");
+        let max_str_with_currency = format!("{}", max_value);
+        assert_eq!(&max_str_with_currency, "18446744073709.551615 T");
+        let max_str_no_currency = max_str_with_currency[0..max_str_with_currency.len() - 2].to_string();
+        assert_eq!(&max_str_no_currency, "18446744073709.551615");
+
+        assert_eq!(max_value, MicroMinotari::from_str(&max_str_with_currency).unwrap());
+        assert_eq!(max_value, MicroMinotari::from_str(&max_str_no_currency).unwrap());
+        assert_eq!(
+            Minotari::from(max_value),
+            Minotari::from_str(&max_str_with_currency).unwrap()
+        );
+        assert_eq!(
+            Minotari::from(max_value),
+            Minotari::from_str(&max_str_no_currency).unwrap()
+        );
+
+        assert!(MicroMinotari::from_str("18446744073709.551615 T").is_ok());
+        assert!(MicroMinotari::from_str("18446744073709.551615 uT").is_err());
+        assert!(MicroMinotari::from_str("18446744073709.551615T").is_ok());
+        assert!(MicroMinotari::from_str("18446744073709.551615uT").is_err());
+        assert!(MicroMinotari::from_str("18446744073709.551615").is_ok());
+        assert!(MicroMinotari::from_str("18446744073709551615").is_ok());
+
+        assert!(Minotari::from_str("18446744073709.551615 T").is_ok());
+        assert!(Minotari::from_str("18446744073709.551615 uT").is_err());
+        assert!(Minotari::from_str("18446744073709.551615T").is_ok());
+        assert!(Minotari::from_str("18446744073709.551615uT").is_err());
+        assert!(Minotari::from_str("18446744073709.551615").is_ok());
+        assert_eq!(
+            &Minotari::from_str("18446744073709551615").unwrap_err().to_string(),
+            "Failed to convert value: numeric overflow"
+        );
+    }
 }
diff --git a/base_layer/core/tests/tests/block_validation.rs b/base_layer/core/tests/tests/block_validation.rs
@@ -1303,8 +1303,8 @@ async fn test_fee_overflow() {
     // This will call `Fee::calculate(...)`, which will overflow if regressed
     let build_result = stx_builder.build().await;
     assert!(build_result.is_err());
-    assert!(format!("{:?}", build_result.unwrap_err())
-        // Using less decimal points in the comparison to make provision for the rounding error - the actual value
-        // should be `18446744073691.551615 T`, but the formatting results in `18446744073709.550781 T`
-        .contains("You are spending more than you're providing: provided 10.000000 T, required 18446744073709.55"));
+    assert_eq!(
+        &format!("{:?}", build_result.unwrap_err()),
+        "You are spending more than you're providing: provided 10.000000 T, required 18446744073709.551615 T."
+    );
 }