Merge rust-bitcoin/rust-bitcoin#3719: Change `SignedAmount` `MAX` and `MIN` to equal +/- `MAX_MONEY`

4b926e1908 Change`MAX and `MIN` to equal `MAX_MONEY` (Jamil Lambert, PhD)

Pull request description:

  To prevent rounding errors converting to and from f64 change `SignedAmount` `MAX` and `MIN` to +/- `MAX_MONEY` which are within the limit in f64 that has issues.

  Add checks to `from_str_in`, `checked_add`,  `checked_sub` and `checked_mul` that the result is within MIN and MAX.

  Modify tests to work with new `MIN` and `MAX`.

  Discussed in #3688 and #3691

ACKs for top commit:
  tcharding:
    ACK 4b926e1908
  apoelstra:
    ACK 4b926e1908f72af98d24cc64d7e1eef44d624e4e; successfully ran local tests

Tree-SHA512: 9053e761b3b74a7a9d826ae7271ced41cf7919752ac8ed8977a20b5b1a746ac8a6bfff68159f4a0dea733ea00f49cf41c0422de53c7aff39efd8482f8cba6069

units/src/amount/signed.rs

@@ -58,9 +58,9 @@ impl SignedAmount {
     /// The maximum value allowed as an amount. Useful for sanity checking.
     pub const MAX_MONEY: SignedAmount = SignedAmount(21_000_000 * 100_000_000);
     /// The minimum value of an amount.
-    pub const MIN: SignedAmount = SignedAmount(i64::MIN);
+    pub const MIN: SignedAmount = SignedAmount(-21_000_000 * 100_000_000);
     /// The maximum value of an amount.
-    pub const MAX: SignedAmount = SignedAmount(i64::MAX);
+    pub const MAX: SignedAmount = SignedAmount::MAX_MONEY;
 
     /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis.
     pub const fn from_sat(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) }
@@ -112,8 +112,7 @@ impl SignedAmount {
                 ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)),
             )),
             (false, sat) => Ok(SignedAmount(sat as i64)),
-            (true, sat) if sat == i64::MIN.unsigned_abs() => Ok(SignedAmount(i64::MIN)),
+            (true, sat) if sat > SignedAmount::MIN.to_sat().unsigned_abs() => Err(ParseAmountError(
-            (true, sat) if sat > i64::MIN.unsigned_abs() => Err(ParseAmountError(
                 ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small()),
             )),
             (true, sat) => Ok(SignedAmount(-(sat as i64))),
@@ -256,7 +255,7 @@ impl SignedAmount {
     pub const fn checked_add(self, rhs: SignedAmount) -> Option<SignedAmount> {
         // No `map()` in const context.
         match self.0.checked_add(rhs.0) {
-            Some(res) => Some(SignedAmount(res)),
+            Some(res) => SignedAmount(res).check_min_max(),
             None => None,
         }
     }
@@ -268,7 +267,7 @@ impl SignedAmount {
     pub const fn checked_sub(self, rhs: SignedAmount) -> Option<SignedAmount> {
         // No `map()` in const context.
         match self.0.checked_sub(rhs.0) {
-            Some(res) => Some(SignedAmount(res)),
+            Some(res) => SignedAmount(res).check_min_max(),
             None => None,
         }
     }
@@ -280,7 +279,7 @@ impl SignedAmount {
     pub const fn checked_mul(self, rhs: i64) -> Option<SignedAmount> {
         // No `map()` in const context.
         match self.0.checked_mul(rhs) {
-            Some(res) => Some(SignedAmount(res)),
+            Some(res) => SignedAmount(res).check_min_max(),
             None => None,
         }
     }
@@ -351,6 +350,15 @@ impl SignedAmount {
             Ok(Amount::from_sat(self.to_sat() as u64))
         }
     }
+
+    /// Checks the amount is within the allowed range.
+    const fn check_min_max(self) -> Option<SignedAmount> {
+        if self.0 < Self::MIN.0 || self.0 > Self::MAX.0 {
+            None
+        } else {
+            Some(self)
+        }
+    }
 }
 
 impl default::Default for SignedAmount {

units/src/amount/tests.rs

@@ -283,7 +283,7 @@ fn parsing() {
     assert_eq!(p("1", btc), Ok(Amount::from_sat(1_000_000_00)));
     assert_eq!(sp("-.5", btc), Ok(SignedAmount::from_sat(-500_000_00)));
     #[cfg(feature = "alloc")]
-    assert_eq!(sp(&i64::MIN.to_string(), sat), Ok(SignedAmount::from_sat(i64::MIN)));
+    assert_eq!(sp(&SignedAmount::MIN.to_sat().to_string(), sat), Ok(SignedAmount::MIN));
     assert_eq!(p("1.1", btc), Ok(Amount::from_sat(1_100_000_00)));
     assert_eq!(p("100", sat), Ok(Amount::from_sat(100)));
     assert_eq!(p("55", sat), Ok(Amount::from_sat(55)));
@@ -571,7 +571,11 @@ fn from_str() {
     scase("-0.1 satoshi", Err(TooPreciseError { position: 3 }));
     case("0.123456 mBTC", Err(TooPreciseError { position: 7 }));
     scase("-1.001 bits", Err(TooPreciseError { position: 5 }));
-    scase("-200000000000 BTC", Err(OutOfRangeError::too_small()));
+    scase("-21000001 BTC", Err(OutOfRangeError::too_small()));
+    scase("21000001 BTC", Err(OutOfRangeError::too_big(true)));
+    scase("-2100000000000001 SAT", Err(OutOfRangeError::too_small()));
+    scase("2100000000000001 SAT", Err(OutOfRangeError::too_big(true)));
+    case("21000001 BTC", Err(OutOfRangeError::too_big(false)));
     case("18446744073709551616 sat", Err(OutOfRangeError::too_big(false)));
 
     ok_case(".5 bits", Amount::from_sat(50));
@@ -580,8 +584,9 @@ fn from_str() {
     ok_scase("-5 satoshi", SignedAmount::from_sat(-5));
     ok_case("0.10000000 BTC", Amount::from_sat(100_000_00));
     ok_scase("-100 bits", SignedAmount::from_sat(-10_000));
-    #[cfg(feature = "alloc")]
+    ok_case("21000000 BTC", Amount::MAX);
-    ok_scase(&format!("{} SAT", i64::MIN), SignedAmount::from_sat(i64::MIN));
+    ok_scase("21000000 BTC", SignedAmount::MAX);
+    ok_scase("-21000000 BTC", SignedAmount::MIN);
 }
 
 #[cfg(feature = "alloc")]