Merge rust-bitcoin#4695: Remove CheckedSum trait

bdf9e00 Remove CheckedSum trait (Tobin C. Harding)
7a6f363 Remove checked_sum impl for Weight (Tobin C. Harding)

Pull request description:

  First remove the impl from `Weight` then remove the trait all together.
  
  Close: rust-bitcoin#4670


ACKs for top commit:
  apoelstra:
    ACK bdf9e00; successfully ran local tests


Tree-SHA512: 2b0d8f506707ea69aee3d9d90d665e1ccae0193d2d4f86de1728e2c1ab19b2c73c834b32f509ef7ad66c2c1a38485e913aebfd6ec1570258978abd7a02a0db2e

Author: apoelstra

bitcoin/src/lib.rs

@@ -203,7 +203,6 @@ pub mod amount {
 
     #[rustfmt::skip]            // Keep public re-exports separate.
     #[doc(inline)]
-    pub use units::CheckedSum;
     #[cfg(feature = "serde")]
     pub use units::amount::serde;
     pub use units::amount::{

units/src/amount/mod.rs

@@ -26,7 +26,6 @@ use core::str::FromStr;
 use arbitrary::{Arbitrary, Unstructured};
 
 use self::error::{MissingDigitsKind, ParseAmountErrorInner, ParseErrorInner};
-use crate::CheckedSum;
 
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[doc(inline)]
@@ -594,22 +593,6 @@ enum DisplayStyle {
     DynamicDenomination,
 }
 
-impl<T> CheckedSum<Amount> for T
-where
-    T: Iterator<Item = Amount>,
-{
-    fn checked_sum(mut self) -> Option<Amount> { self.try_fold(Amount::ZERO, Amount::checked_add) }
-}
-
-impl<T> CheckedSum<SignedAmount> for T
-where
-    T: Iterator<Item = SignedAmount>,
-{
-    fn checked_sum(mut self) -> Option<SignedAmount> {
-        self.try_fold(SignedAmount::ZERO, SignedAmount::checked_add)
-    }
-}
-
 #[cfg(feature = "arbitrary")]
 impl<'a> Arbitrary<'a> for Denomination {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {

units/src/amount/tests.rs

@@ -841,68 +841,47 @@ fn to_string_with_denomination_from_str_roundtrip() {
 
 #[test]
 fn sum_amounts() {
-    assert_eq!([].iter().sum::<NumOpResult<Amount>>(), Amount::ZERO.into());
-    assert_eq!([].iter().sum::<NumOpResult<SignedAmount>>(), SignedAmount::ZERO.into());
-
-    let results =
-        [NumOpResult::Valid(sat(42)), NumOpResult::Valid(sat(1337)), NumOpResult::Valid(sat(21))];
-    assert_eq!(results.iter().sum::<NumOpResult<Amount>>(), NumOpResult::Valid(sat(1400)));
-
-    let signed_results = [
-        NumOpResult::Valid(ssat(42)),
-        NumOpResult::Valid(ssat(1337)),
-        NumOpResult::Valid(ssat(21)),
-    ];
-    assert_eq!(
-        signed_results.iter().sum::<NumOpResult<SignedAmount>>(),
-        NumOpResult::Valid(ssat(1400))
-    );
-
-    let amounts = [sat(42), sat(1337), sat(21)];
-    assert_eq!(
-        amounts.iter().map(|a| NumOpResult::Valid(*a)).sum::<NumOpResult<Amount>>(),
-        sat(1400).into(),
-    );
-    assert_eq!(
-        amounts.into_iter().map(NumOpResult::Valid).sum::<NumOpResult<Amount>>(),
-        sat(1400).into(),
-    );
-
-    let amounts = [ssat(-42), ssat(1337), ssat(21)];
-    assert_eq!(
-        amounts.iter().map(NumOpResult::from).sum::<NumOpResult<SignedAmount>>(),
-        ssat(1316).into(),
-    );
-    assert_eq!(
-        amounts.into_iter().map(NumOpResult::from).sum::<NumOpResult<SignedAmount>>(),
-        ssat(1316).into()
-    );
-}
-
-#[test]
-fn checked_sum_amounts() {
-    assert_eq!([].into_iter().checked_sum(), Some(Amount::ZERO));
-    assert_eq!([].into_iter().checked_sum(), Some(SignedAmount::ZERO));
+    let empty: [NumOpResult<Amount>; 0] = [];
+    assert_eq!(empty.into_iter().sum::<NumOpResult<_>>(), NumOpResult::Valid(Amount::ZERO));
+    let empty: [NumOpResult<SignedAmount>; 0] = [];
+    assert_eq!(empty.into_iter().sum::<NumOpResult<_>>(), NumOpResult::Valid(SignedAmount::ZERO));
 
     let amounts = [sat(42), sat(1337), sat(21)];
-    let sum = amounts.into_iter().checked_sum();
-    assert_eq!(sum, Some(sat(1400)));
+    let sum = amounts
+        .into_iter()
+        .map(NumOpResult::from)
+        .sum::<NumOpResult<Amount>>()
+        .unwrap();
+    assert_eq!(sum, sat(1400));
 
     let amounts = [Amount::MAX_MONEY, sat(1337), sat(21)];
-    let sum = amounts.into_iter().checked_sum();
-    assert_eq!(sum, None);
+    assert!(amounts
+        .into_iter()
+        .map(NumOpResult::from)
+        .sum::<NumOpResult<Amount>>()
+        .is_error());
 
     let amounts = [SignedAmount::MIN, ssat(-1), ssat(21)];
-    let sum = amounts.into_iter().checked_sum();
-    assert_eq!(sum, None);
+    assert!(amounts
+        .into_iter()
+        .map(NumOpResult::from)
+        .sum::<NumOpResult<SignedAmount>>()
+        .is_error());
 
     let amounts = [SignedAmount::MAX, ssat(1), ssat(21)];
-    let sum = amounts.into_iter().checked_sum();
-    assert_eq!(sum, None);
+    assert!(amounts
+        .into_iter()
+        .map(NumOpResult::from)
+        .sum::<NumOpResult<SignedAmount>>()
+        .is_error());
 
     let amounts = [ssat(42), ssat(3301), ssat(21)];
-    let sum = amounts.into_iter().checked_sum();
-    assert_eq!(sum, Some(ssat(3364)));
+    let sum = amounts
+        .into_iter()
+        .map(NumOpResult::from)
+        .sum::<NumOpResult<SignedAmount>>()
+        .unwrap();
+    assert_eq!(sum, ssat(3364));
 }
 
 #[test]

units/src/lib.rs

@@ -69,21 +69,3 @@ pub(crate) use self::result::OptionExt;
 #[deprecated(since = "TBD", note = "use `BlockHeightInterval` instead")]
 #[doc(hidden)]
 pub type BlockInterval = BlockHeightInterval;
-
-/// Calculates the sum over the iterator using checked arithmetic.
-pub trait CheckedSum<R>: sealed::Sealed<R> {
-    /// Calculates the sum over the iterator using checked arithmetic. If an
-    /// overflow happens it returns [`None`].
-    fn checked_sum(self) -> Option<R>;
-}
-
-mod sealed {
-    use super::{Amount, SignedAmount, Weight};
-
-    /// Used to seal the `CheckedSum` trait
-    pub trait Sealed<A> {}
-
-    impl<T> Sealed<Amount> for T where T: Iterator<Item = Amount> {}
-    impl<T> Sealed<SignedAmount> for T where T: Iterator<Item = SignedAmount> {}
-    impl<T> Sealed<Weight> for T where T: Iterator<Item = Weight> {}
-}

units/src/weight.rs

@@ -10,7 +10,7 @@ use arbitrary::{Arbitrary, Unstructured};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
-use crate::{Amount, CheckedSum, FeeRate, NumOpResult};
+use crate::{Amount, FeeRate, NumOpResult};
 
 /// The factor that non-witness serialization data is multiplied by during weight calculation.
 pub const WITNESS_SCALE_FACTOR: usize = 4;
@@ -251,13 +251,6 @@ impl ops::RemAssign<u64> for Weight {
     fn rem_assign(&mut self, rhs: u64) { *self = Weight::from_wu(self.to_wu() % rhs); }
 }
 
-impl<T> CheckedSum<Weight> for T
-where
-    T: Iterator<Item = Weight>,
-{
-    fn checked_sum(mut self) -> Option<Weight> { self.try_fold(Weight::ZERO, Weight::checked_add) }
-}
-
 impl core::iter::Sum for Weight {
     fn sum<I>(iter: I) -> Self
     where
@@ -541,16 +534,4 @@ mod tests {
         weight %= 3;
         assert_eq!(weight, Weight::from_wu(1));
     }
-
-    #[test]
-    #[cfg(feature = "alloc")]
-    fn checked_sum_weights() {
-        assert_eq!([].into_iter().checked_sum(), Some(Weight::ZERO));
-
-        let sum = alloc::vec![0, 1, 2].iter().map(|&w| Weight::from_wu(w)).checked_sum().unwrap();
-        assert_eq!(sum, Weight::from_wu(3));
-
-        let sum = alloc::vec![1, u64::MAX].iter().map(|&w| Weight::from_wu(w)).checked_sum();
-        assert!(sum.is_none());
-    }
 }

units/tests/api.rs

@@ -15,8 +15,8 @@ use arbitrary::{Arbitrary, Unstructured};
 use bitcoin_units::locktime::{absolute, relative}; // Typical usage is `absolute::LockTime`.
 use bitcoin_units::{
     amount, block, fee_rate, locktime, parse, time, weight, Amount, BlockHeight,
-    BlockHeightInterval, BlockMtp, BlockMtpInterval, BlockTime, CheckedSum, FeeRate, MathOp,
-    NumOpResult, SignedAmount, Weight,
+    BlockHeightInterval, BlockMtp, BlockMtpInterval, BlockTime, FeeRate, MathOp, NumOpResult,
+    SignedAmount, Weight,
 };
 
 /// A struct that includes all public non-error enums.
@@ -283,8 +283,6 @@ fn regression_default() {
 fn dyn_compatible() {
     // If this builds then traits are dyn compatible.
     struct Traits {
-        a: Box<dyn CheckedSum<Amount>>,
-        b: Box<dyn CheckedSum<Weight>>,
         // These traits are explicitly not dyn compatible.
         // b: Box<dyn amount::serde::SerdeAmount>,
         // c: Box<dyn amount::serde::SerdeAmountForOpt>,