amount: return i64 from parse_signed_to_satoshi

This private function is used for string-parsing an amount. It returns a
sign boolean and a u64, but its surrounding logic can be simplified if
it just returns a i64 (which is OK now since the range of `Amount` fits
into the range of i64).

Along the way we eliminate some instances of from_sat_unchecked.

Annoyingly we still need the bool to distinguish -0 from +0; when
parsing Amount we reject -0 (and we have tests for this).

This causes a couple error constructors to no longer be used outside of
unit tests. May be worth looking at whether we need them, or whether we
should be using them in parse_signed_to_satoshi.

units/src/amount/error.rs

@@ -182,8 +182,10 @@ impl OutOfRangeError {
     /// Returns true if the input value was smaller than the minimum allowed value.
     pub fn is_below_min(self) -> bool { !self.is_greater_than_max }
 
+    #[cfg(test)]
     pub(crate) fn too_big(is_signed: bool) -> Self { Self { is_signed, is_greater_than_max: true } }
 
+    #[cfg(test)]
     pub(crate) fn too_small() -> Self {
         Self {
             // implied - negative() is used for the other

units/src/amount/mod.rs

@@ -202,10 +202,12 @@ const INPUT_STRING_LEN_LIMIT: usize = 50;
 
 /// Parses a decimal string in the given denomination into a satoshi value and a
 /// [`bool`] indicator for a negative amount.
+///
+/// The `bool` is only needed to distinguish -0 from 0.
 fn parse_signed_to_satoshi(
     mut s: &str,
     denom: Denomination,
-) -> Result<(bool, u64), InnerParseError> {
+) -> Result<(bool, SignedAmount), InnerParseError> {
     if s.is_empty() {
         return Err(InnerParseError::MissingDigits(MissingDigitsError {
             kind: MissingDigitsKind::Empty,
@@ -237,7 +239,7 @@ fn parse_signed_to_satoshi(
             let last_n = precision_diff.unsigned_abs().into();
             if let Some(position) = is_too_precise(s, last_n) {
                 match s.parse::<i64>() {
-                    Ok(0) => return Ok((is_negative, 0)),
+                    Ok(0) => return Ok((is_negative, SignedAmount::ZERO)),
                     _ =>
                         return Err(InnerParseError::TooPrecise(TooPreciseError {
                             position: position + usize::from(is_negative),
@@ -252,14 +254,14 @@ fn parse_signed_to_satoshi(
     };
 
     let mut decimals = None;
-    let mut value: u64 = 0; // as satoshis
+    let mut value: i64 = 0; // as satoshis
     for (i, c) in s.char_indices() {
         match c {
             '0'..='9' => {
                 // Do `value = 10 * value + digit`, catching overflows.
-                match 10_u64.checked_mul(value) {
+                match 10_i64.checked_mul(value) {
                     None => return Err(InnerParseError::Overflow { is_negative }),
-                    Some(val) => match val.checked_add(u64::from(c as u8 - b'0')) {
+                    Some(val) => match val.checked_add(i64::from(c as u8 - b'0')) {
                         None => return Err(InnerParseError::Overflow { is_negative }),
                         Some(val) => value = val,
                     },
@@ -295,13 +297,18 @@ fn parse_signed_to_satoshi(
     // Decimally shift left by `max_decimals - decimals`.
     let scale_factor = max_decimals - decimals.unwrap_or(0);
     for _ in 0..scale_factor {
-        value = match 10_u64.checked_mul(value) {
+        value = match 10_i64.checked_mul(value) {
             Some(v) => v,
             None => return Err(InnerParseError::Overflow { is_negative }),
         };
     }
 
-    Ok((is_negative, value))
+    let mut ret =
+        SignedAmount::from_sat(value).map_err(|_| InnerParseError::Overflow { is_negative })?;
+    if is_negative {
+        ret = -ret;
+    }
+    Ok((is_negative, ret))
 }
 
 #[derive(Debug)]

units/src/amount/signed.rs

@@ -10,7 +10,7 @@ use core::{default, fmt};
 #[cfg(feature = "arbitrary")]
 use arbitrary::{Arbitrary, Unstructured};
 
-use super::error::{ParseAmountErrorInner, ParseErrorInner};
+use super::error::ParseErrorInner;
 use super::{
     parse_signed_to_satoshi, split_amount_and_denomination, Amount, Denomination, Display,
     DisplayStyle, OutOfRangeError, ParseAmountError, ParseError,
@@ -169,17 +169,9 @@ impl SignedAmount {
     ///
     /// If the amount is too big (positive or negative) or too precise.
     pub fn from_str_in(s: &str, denom: Denomination) -> Result<SignedAmount, ParseAmountError> {
-        match parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(true))? {
+        parse_signed_to_satoshi(s, denom)
-            // (negative, amount)
+            .map(|(_, amount)| amount)
-            (false, sat) if sat > SignedAmount::MAX.to_sat() as u64 => Err(ParseAmountError(
+            .map_err(|error| error.convert(true))
-                ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)),
-            )),
-            (false, sat) => Ok(SignedAmount::from_sat_unchecked(sat as i64)), // Cast ok, value in this arm does not wrap.
-            (true, sat) if sat > SignedAmount::MIN.to_sat().unsigned_abs() => Err(
-                ParseAmountError(ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small())),
-            ),
-            (true, sat) => Ok(SignedAmount::from_sat_unchecked(-(sat as i64))), // Cast ok, value in this arm does not wrap.
-        }
     }
 
     /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`]

units/src/amount/unsigned.rs

@@ -167,19 +167,14 @@ impl Amount {
     ///
     /// If the amount is too precise, negative, or greater than 21,000,000.
     pub fn from_str_in(s: &str, denom: Denomination) -> Result<Amount, ParseAmountError> {
-        let (negative, sats) =
+        let (is_neg, amount) =
             parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(false))?;
-        if negative {
+        if is_neg {
             return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange(
                 OutOfRangeError::negative(),
             )));
         }
-        if sats > Self::MAX.to_sat() {
+        Self::try_from(amount).map_err(|e| ParseAmountError(ParseAmountErrorInner::OutOfRange(e)))
-            return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange(
-                OutOfRangeError::too_big(false),
-            )));
-        }
-        Ok(Self::from_sat_unchecked(sats))
     }
 
     /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`]