Merge rust-bitcoin/rust-bitcoin#3840: Address mutants in units

286bf900b1 Address mutants in units (Shing Him Ng)

Pull request description:

  Preemptively addressing these mutants before introducing the `cargo-mutants` workflow. There are several types of changes:
  - Changes that address mutants that were actually missing
  - Changes that address test values that cause `cargo-mutants` to think mutants were missed. For example, `cargo-mutants` will replace the return values for unsigned integer types with 0 and 1. While this case might be tested, the test might be testing this with a call that results in 0 or 1. When `cargo-mutants` substitutes the function call with `Ok(1)`, the test will still pass, and it will consider this a mutant. `cargo-mutants` also replaces operations (+, -, /, %), bitwise operations (&, |, ^, etc), so an operation such as `3 - 2` results in a mutant because changing it to `3 / 2` yields the same result
  - TODOs to ignore functions/impls in the future

  I uased the following `mutants.toml` config file when running `cargo mutants` and skipped the `Debug`, `Arbitrary`, `Display`, and `Error` impls and the `kani` test:
  ```
  additional_cargo_args = ["--all-features"]
  examine_globs = ["units/src/**/*.rs"]
  exclude_globs = ["units/src/amount/verification.rs"]
  exclude_re = ["impl Debug", "impl Arbitrary", "impl Display", ".*Error", "<impl Visitor for VisitOptAmt<X>>"]
  ```

  I wasn't sure the code for Displaying things needed to be tested with `cargo mutants`, but I'm less sure about the errors so i can kill those mutants too if needed

ACKs for top commit:
  tcharding:
    ACK 286bf900b1
  apoelstra:
    ACK 286bf900b1c100d2f5d0a0d45f31a5bf5a0a26ce; successfully ran local tests

Tree-SHA512: 3db9598a5ad92f2013d43876221ce9363cc019cbf720a206768b517a812c8355b7f00594212eb0526c0feb2dc578f88e1df12548f72a2b2360c4d4227de749f7

units/src/amount/mod.rs

@@ -285,6 +285,7 @@ fn parse_signed_to_satoshi(
     Ok((is_negative, value))
 }
 
+#[derive(Debug)]
 enum InnerParseError {
     Overflow { is_negative: bool },
     TooPrecise(TooPreciseError),

units/src/amount/serde.rs

@@ -390,6 +390,12 @@ pub mod as_str {
 mod tests {
     use super::*;
 
+    #[test]
+    fn sanity_check() {
+        assert_eq!(Amount::type_prefix(private::Token), "u");
+        assert_eq!(SignedAmount::type_prefix(private::Token), "i");
+    }
+
     #[test]
     fn can_serde_as_sat() {
         #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]

units/src/amount/tests.rs

@@ -16,6 +16,32 @@ use super::*;
 #[cfg(feature = "alloc")]
 use crate::{FeeRate, Weight};
 
+#[test]
+fn sanity_check() {
+    assert_eq!(SignedAmount::from_sat(-100).abs(), SignedAmount::from_sat(100));
+    assert_eq!(SignedAmount::from_sat(i64::MIN + 1).checked_abs().unwrap(), SignedAmount::from_sat(i64::MAX));
+    assert_eq!(SignedAmount::from_sat(-100).signum(), -1);
+    assert_eq!(SignedAmount::from_sat(0).signum(), 0);
+    assert_eq!(SignedAmount::from_sat(100).signum(), 1);
+    assert_eq!(SignedAmount::from(Amount::from_sat(100)), SignedAmount::from_sat(100));
+    assert!(SignedAmount::from_sat(i64::MIN).checked_abs().is_none());
+    assert!(!SignedAmount::from_sat(-100).is_positive());
+    assert!(SignedAmount::from_sat(100).is_positive());
+
+    #[cfg(feature = "alloc")]
+    {
+        assert_eq!(Amount::from_float_in(0_f64, Denomination::Bitcoin).unwrap(), Amount::from_sat(0));
+        assert_eq!(Amount::from_float_in(2_f64, Denomination::Bitcoin).unwrap(), Amount::from_sat(200_000_000));
+        assert!(Amount::from_float_in(-100_f64, Denomination::Bitcoin).is_err());
+    }
+}
+
+#[test]
+fn check_if_num_is_too_precise() {
+    assert_eq!(is_too_precise("1234", 3).unwrap(), 3);
+    assert_eq!(is_too_precise("1234.1234", 3).unwrap(), 3);
+}
+
 #[test]
 #[cfg(feature = "alloc")]
 fn from_str_zero() {
@@ -94,11 +120,27 @@ fn mul_div() {
     assert_eq!(ssat(-14) / 2, ssat(-7));
     assert_eq!(ssat(-14) % 3, ssat(-2));
 
+    let mut a = sat(30);
+    a /= 3;
+    assert_eq!(a, sat(10));
+    a %= 3;
+    assert_eq!(a, sat(1));
+    a *= 3;
+    assert_eq!(a, Amount::from_sat(3));
+
     let mut b = ssat(30);
     b /= 3;
     assert_eq!(b, ssat(10));
     b %= 3;
     assert_eq!(b, ssat(1));
+    b *= 3;
+    assert_eq!(b, SignedAmount::from_sat(3));
+}
+
+#[test]
+fn neg() {
+    let amount = -SignedAmount::from_sat(2);
+    assert_eq!(amount.to_sat(), -2);
 }
 
 #[cfg(feature = "std")]
@@ -125,6 +167,23 @@ fn checked_arithmetic() {
     assert_eq!(ssat(-6).checked_div(2), Some(ssat(-3)));
 }
 
+#[test]
+#[allow(deprecated_in_future)]
+fn unchecked_arithmetic() {
+    assert_eq!(SignedAmount::from_sat(10).unchecked_add(SignedAmount::from_sat(20)), SignedAmount::from_sat(30));
+    assert_eq!(SignedAmount::from_sat(50).unchecked_sub(SignedAmount::from_sat(10)), SignedAmount::from_sat(40));
+    assert_eq!(Amount::from_sat(5).unchecked_add(Amount::from_sat(7)), Amount::from_sat(12));
+    assert_eq!(Amount::from_sat(10).unchecked_sub(Amount::from_sat(7)), Amount::from_sat(3));
+}
+
+#[test]
+fn positive_sub() {
+    assert_eq!(SignedAmount::from_sat(10).positive_sub(SignedAmount::from_sat(7)).unwrap(), SignedAmount::from_sat(3));
+    assert!(SignedAmount::from_sat(-10).positive_sub(SignedAmount::from_sat(7)).is_none());
+    assert!(SignedAmount::from_sat(10).positive_sub(SignedAmount::from_sat(-7)).is_none());
+    assert!(SignedAmount::from_sat(10).positive_sub(SignedAmount::from_sat(11)).is_none());
+}
+
 #[cfg(feature = "alloc")]
 #[test]
 fn amount_checked_div_by_weight_ceil() {
@@ -881,11 +940,15 @@ fn sum_amounts() {
     assert_eq!(SignedAmount::ZERO, [].iter().sum::<SignedAmount>());
 
     let amounts = [Amount::from_sat(42), Amount::from_sat(1337), Amount::from_sat(21)];
+    assert_eq!(amounts.iter().sum::<Amount>(), Amount::from_sat(1400));
+
     let sum = amounts.into_iter().sum::<Amount>();
     assert_eq!(Amount::from_sat(1400), sum);
 
     let amounts =
         [SignedAmount::from_sat(-42), SignedAmount::from_sat(1337), SignedAmount::from_sat(21)];
+    assert_eq!(amounts.iter().sum::<SignedAmount>(), SignedAmount::from_sat(1316));
+
     let sum = amounts.into_iter().sum::<SignedAmount>();
     assert_eq!(SignedAmount::from_sat(1316), sum);
 }

units/src/block.rs

@@ -278,11 +278,29 @@ impl<'a> Arbitrary<'a> for BlockInterval {
 mod tests {
     use super::*;
 
+    #[test]
+    fn sanity_check() {
+        let height: u32 = BlockHeight(100).into();
+        assert_eq!(height, 100);
+
+        let interval: u32 = BlockInterval(100).into();
+        assert_eq!(interval, 100);
+
+        let interval_from_height: BlockInterval = relative::Height::from(10u16).into();
+        assert_eq!(interval_from_height.to_u32(), 10u32);
+
+        let invalid_height_greater = relative::Height::try_from(BlockInterval(u32::from(u16::MAX) + 1));
+        assert!(invalid_height_greater.is_err());
+
+        let valid_height = relative::Height::try_from(BlockInterval(u32::from(u16::MAX)));
+        assert!(valid_height.is_ok());
+    }
+
     // These tests are supposed to comprise an exhaustive list of available operations.
     #[test]
     fn all_available_ops() {
         // height - height = interval
-        assert!(BlockHeight(100) - BlockHeight(99) == BlockInterval(1));
+        assert!(BlockHeight(10) - BlockHeight(7) == BlockInterval(3));
 
         // height + interval = height
         assert!(BlockHeight(100) + BlockInterval(1) == BlockHeight(101));
@@ -294,7 +312,10 @@ mod tests {
         assert!(BlockInterval(1) + BlockInterval(2) == BlockInterval(3));
 
         // interval - interval = interval
-        assert!(BlockInterval(3) - BlockInterval(2) == BlockInterval(1));
+        assert!(BlockInterval(10) - BlockInterval(7) == BlockInterval(3));
+
+        assert!([BlockInterval(1), BlockInterval(2), BlockInterval(3)].iter().sum::<BlockInterval>() == BlockInterval(6));
+        assert!([BlockInterval(4), BlockInterval(5), BlockInterval(6)].into_iter().sum::<BlockInterval>() == BlockInterval(15));
 
         // interval += interval
         let mut int = BlockInterval(1);
@@ -302,8 +323,8 @@ mod tests {
         assert_eq!(int, BlockInterval(3));
 
         // interval -= interval
-        let mut int = BlockInterval(3);
-        int -= BlockInterval(2);
-        assert_eq!(int, BlockInterval(1));
+        let mut int = BlockInterval(10);
+        int -= BlockInterval(7);
+        assert_eq!(int, BlockInterval(3));
     }
 }

units/src/fee_rate/mod.rs

@@ -72,6 +72,9 @@ impl FeeRate {
     pub const fn to_sat_per_vb_floor(self) -> u64 { self.0 / (1000 / 4) }
 
     /// Converts to sat/vB rounding up.
+    /// TODO: cargo-mutants will try to replace - with /, which results in 1000 / 4 / 1 which is also 250.
+    /// Since we're addressing the mutants before introducing the cargo-mutants workflow, come back later
+    /// and skip this function in the mutants.toml config file
     pub const fn to_sat_per_vb_ceil(self) -> u64 { (self.0 + (1000 / 4 - 1)) / (1000 / 4) }
 
     /// Checked multiplication.
@@ -254,6 +257,12 @@ impl<'a> Arbitrary<'a> for FeeRate {
 mod tests {
     use super::*;
 
+    #[test]
+    fn sanity_check() {
+        let fee_rate: u64 = u64::from(FeeRate(100));
+        assert_eq!(fee_rate, 100_u64);
+    }
+
     #[test]
     #[allow(clippy::op_ref)]
     fn addition() {
@@ -270,14 +279,24 @@ mod tests {
     #[test]
     #[allow(clippy::op_ref)]
     fn subtract() {
-        let one = FeeRate(1);
-        let two = FeeRate(2);
         let three = FeeRate(3);
+        let seven = FeeRate(7);
+        let ten = FeeRate(10);
 
-        assert!(three - two == one);
-        assert!(&three - two == one);
-        assert!(three - &two == one);
-        assert!(&three - &two == one);
+        assert_eq!(ten - seven, three);
+        assert_eq!(&ten - seven, three);
+        assert_eq!(ten - &seven, three);
+        assert_eq!(&ten - &seven, three);
+    }
+
+    #[test]
+    #[allow(clippy::op_ref)]
+    fn multiply() {
+        let two = FeeRate::from_sat_per_vb(2).unwrap();
+        let three = Weight::from_vb(3).unwrap();
+        let six = Amount::from_sat(6);
+
+        assert_eq!(two * three, six);
     }
 
     #[test]
@@ -343,7 +362,7 @@ mod tests {
 
     #[test]
     fn fee_rate_from_sat_per_kvb() {
-        let fee_rate = FeeRate::from_sat_per_kvb(10);
+        let fee_rate = FeeRate::from_sat_per_kvb(11);
         assert_eq!(FeeRate(2), fee_rate);
     }
 
@@ -366,10 +385,10 @@ mod tests {
 
     #[test]
     fn raw_feerate() {
-        let fee_rate = FeeRate(333);
-        assert_eq!(333, fee_rate.to_sat_per_kwu());
-        assert_eq!(1, fee_rate.to_sat_per_vb_floor());
-        assert_eq!(2, fee_rate.to_sat_per_vb_ceil());
+        let fee_rate = FeeRate(749);
+        assert_eq!(749, fee_rate.to_sat_per_kwu());
+        assert_eq!(2, fee_rate.to_sat_per_vb_floor());
+        assert_eq!(3, fee_rate.to_sat_per_vb_ceil());
     }
 
     #[test]
@@ -381,6 +400,25 @@ mod tests {
         assert!(fee_rate.is_none());
     }
 
+    #[test]
+    fn fee_wu() {
+        let fee_overflow = FeeRate(10).fee_wu(Weight::MAX);
+        assert!(fee_overflow.is_none());
+
+        let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
+        let weight = Weight::from_vb(3).unwrap();
+        assert_eq!(fee_rate.fee_wu(weight).unwrap(), Amount::from_sat(6));
+    }
+
+    #[test]
+    fn fee_vb() {
+        let fee_overflow = FeeRate(10).fee_vb(Weight::MAX.to_wu());
+        assert!(fee_overflow.is_none());
+
+        let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
+        assert_eq!(fee_rate.fee_vb(3).unwrap(), Amount::from_sat(6));
+    }
+
     #[test]
     fn checked_weight_mul() {
         let weight = Weight::from_vb(10).unwrap();

units/src/locktime/absolute.rs

@@ -472,6 +472,15 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn is_block_height_or_time() {
+        assert!(is_block_height(499_999_999));
+        assert!(!is_block_height(500_000_000));
+
+        assert!(!is_block_time(499_999_999));
+        assert!(is_block_time(500_000_000));
+    }
+
     #[test]
     #[cfg(feature = "serde")]
     pub fn encode_decode_height() {

units/src/locktime/relative.rs

@@ -116,6 +116,8 @@ impl Time {
 
     /// Returns the `u32` value used to encode this locktime in an nSequence field or
     /// argument to `OP_CHECKSEQUENCEVERIFY`.
+    /// TODO: Skip this in cargo-mutants. It will replace | with ^, which will return the same
+    /// value since the XOR is always taken against the u16 and an all-zero bitmask
     #[inline]
     pub const fn to_consensus_u32(self) -> u32 {
         (1u32 << 22) | self.0 as u32 // cast safety: u32 is wider than u16 on all architectures
@@ -193,6 +195,13 @@ mod tests {
 
     const MAXIMUM_ENCODABLE_SECONDS: u32 = u16::MAX as u32 * 512;
 
+    #[test]
+    fn sanity_check() {
+        assert_eq!(Height::MAX.to_consensus_u32(), u32::from(u16::MAX));
+        assert_eq!(Time::from_512_second_intervals(100).value(), 100u16);
+        assert_eq!(Time::from_512_second_intervals(100).to_consensus_u32(), 4_194_404u32); // 0x400064
+    }
+
     #[test]
     fn from_seconds_ceil_success() {
         let actual = Time::from_seconds_ceil(100).unwrap();

units/src/parse.rs

@@ -24,7 +24,7 @@ pub struct ParseIntError {
     pub(crate) input: InputString,
     // for displaying - see Display impl with nice error message below
     pub(crate) bits: u8,
-    // We could represent this as a single bit but it wouldn't actually derease the cost of moving
+    // We could represent this as a single bit, but it wouldn't actually decrease the cost of moving
     // the struct because String contains pointers so there will be padding of bits at least
     // pointer_size - 1 bytes: min 1B in practice.
     pub(crate) is_signed: bool,
@@ -488,8 +488,64 @@ impl std::error::Error for ContainsPrefixError {}
 
 #[cfg(test)]
 mod tests {
+    #[cfg(feature = "std")]
+    use std::panic;
+
     use super::*;
 
+    #[test]
+    fn parse_int() {
+        assert!(int::<u8, _>("1").is_ok());
+        let _ = int::<i8, _>("not a number").map_err(|e| assert!(e.is_signed));
+        let _ = int::<u8, _>("not a number").map_err(|e| assert!(!e.is_signed));
+    }
+
+    #[test]
+    #[cfg(feature = "std")]
+    fn parse_int_panic_when_populating_bits() {
+        // Fields in the test type are never used
+        #[allow(dead_code)]
+        struct TestTypeLargerThanU128(u128, u128);
+        impl_integer!(TestTypeLargerThanU128);
+        impl FromStr for TestTypeLargerThanU128 {
+            type Err = core::num::ParseIntError;
+
+            fn from_str(_: &str) -> Result<Self, Self::Err> {
+                "Always invalid for testing".parse::<u32>().map(|_| TestTypeLargerThanU128(0, 0))
+            }
+        }
+        impl From<i8> for TestTypeLargerThanU128 {
+            fn from(_: i8) -> Self { TestTypeLargerThanU128(0, 0) }
+        }
+
+        let result = panic::catch_unwind(|| int::<TestTypeLargerThanU128, _>("not a number"));
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn remove_prefix() {
+        let lower = "0xhello";
+        assert_eq!(hex_remove_prefix(lower).unwrap(), "hello");
+
+        let upper = "0Xhello";
+        assert_eq!(hex_remove_prefix(upper).unwrap(), "hello");
+
+        let err = "error";
+        assert!(hex_remove_prefix(err).is_err());
+    }
+
+    #[test]
+    fn check_unprefixed() {
+        let lower = "0xhello";
+        assert!(hex_check_unprefixed(lower).is_err());
+
+        let upper = "0Xhello";
+        assert!(hex_check_unprefixed(upper).is_err());
+
+        let valid = "hello";
+        assert_eq!(hex_check_unprefixed(valid).unwrap(), "hello");
+    }
+
     #[test]
     fn parse_u32_from_hex_prefixed() {
         let want = 171;
@@ -504,6 +560,21 @@ mod tests {
         assert_eq!(got, want);
     }
 
+    #[test]
+    fn parse_hex_u32_prefixed() {
+        let want = 171; // 0xab
+        assert_eq!(hex_u32_prefixed("0xab").unwrap(), want);
+        assert!(hex_u32_unprefixed("0xab").is_err());
+
+    }
+
+    #[test]
+    fn parse_hex_u32_unprefixed() {
+        let want = 171; // 0xab
+        assert_eq!(hex_u32_unprefixed("ab").unwrap(), want);
+        assert!(hex_u32_prefixed("ab").is_err());
+    }
+
     #[test]
     fn parse_u128_from_hex_prefixed() {
         let want = 3_735_928_559;
@@ -518,6 +589,21 @@ mod tests {
         assert_eq!(got, want);
     }
 
+    #[test]
+    fn parse_hex_u128_prefixed() {
+        let want = 3_735_928_559;
+        assert_eq!(hex_u128_prefixed("0xdeadbeef").unwrap(), want);
+        assert!(hex_u128_unprefixed("0xdeadbeef").is_err());
+
+    }
+
+    #[test]
+    fn parse_hex_u128_unprefixed() {
+        let want = 3_735_928_559;
+        assert_eq!(hex_u128_unprefixed("deadbeef").unwrap(), want);
+        assert!(hex_u128_prefixed("deadbeef").is_err());
+    }
+
     #[test]
     fn parse_u32_from_hex_unchecked_errors_on_prefix() {
         assert!(hex_u32_unchecked("0xab").is_err());

units/src/weight.rs

@@ -250,6 +250,11 @@ mod tests {
     const TWO: Weight = Weight(2);
     const FOUR: Weight = Weight(4);
 
+    #[test]
+    fn sanity_check() {
+        assert_eq!(Weight::MIN_TRANSACTION, Weight(240));
+    }
+
     #[test]
     fn from_kwu() {
         let got = Weight::from_kwu(1).unwrap();
@@ -302,8 +307,8 @@ mod tests {
 
     #[test]
     fn to_kwu_floor() {
-        assert_eq!(Weight(1_000).to_kwu_floor(), 1);
-        assert_eq!(Weight(1_999).to_kwu_floor(), 1);
+        assert_eq!(Weight(5_000).to_kwu_floor(), 5);
+        assert_eq!(Weight(5_999).to_kwu_floor(), 5);
     }
 
     #[test]
@@ -314,8 +319,8 @@ mod tests {
 
     #[test]
     fn to_vb_floor() {
-        assert_eq!(Weight(4).to_vbytes_floor(), 1);
-        assert_eq!(Weight(5).to_vbytes_floor(), 1);
+        assert_eq!(Weight(8).to_vbytes_floor(), 2);
+        assert_eq!(Weight(9).to_vbytes_floor(), 2);
     }
 
     #[test]
@@ -374,14 +379,33 @@ mod tests {
     #[test]
     #[allow(clippy::op_ref)]
     fn subtract() {
-        let one = Weight(1);
-        let two = Weight(2);
+        let ten = Weight(10);
+        let seven = Weight(7);
         let three = Weight(3);
 
-        assert!(three - two == one);
-        assert!(&three - two == one);
-        assert!(three - &two == one);
-        assert!(&three - &two == one);
+        assert_eq!(ten - seven, three);
+        assert_eq!(&ten - seven, three);
+        assert_eq!(ten - &seven, three);
+        assert_eq!(&ten - &seven, three);
+    }
+
+    #[test]
+    #[allow(clippy::op_ref)]
+    fn multiply() {
+        let two = Weight(2);
+        let six = Weight(6);
+
+        assert_eq!(3_u64 * two, six);
+        assert_eq!(two * 3_u64, six);
+    }
+
+    #[test]
+    fn divide() {
+        let eight = Weight(8);
+        let four = Weight(4);
+
+        assert_eq!(eight / four, 2_u64);
+        assert_eq!(eight / 4_u64, Weight(2));
     }
 
     #[test]
@@ -405,4 +429,18 @@ mod tests {
         f -= &Weight(2);
         assert_eq!(f, Weight(1));
     }
+
+    #[test]
+    fn mul_assign() {
+        let mut w = Weight(3);
+        w *= 2_u64;
+        assert_eq!(w, Weight(6));
+    }
+
+    #[test]
+    fn div_assign() {
+        let mut w = Weight(8);
+        w /= Weight(4).into();
+        assert_eq!(w, Weight(2));
+    }
 }