From 286bf900b1c100d2f5d0a0d45f31a5bf5a0a26ce Mon Sep 17 00:00:00 2001 From: Shing Him Ng Date: Mon, 30 Dec 2024 09:18:05 -0600 Subject: [PATCH] Address mutants in units 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 a function might be tested, the test might be testing the function 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 --- units/src/amount/mod.rs | 1 + units/src/amount/serde.rs | 6 +++ units/src/amount/tests.rs | 63 ++++++++++++++++++++++++ units/src/block.rs | 31 ++++++++++-- units/src/fee_rate/mod.rs | 60 ++++++++++++++++++----- units/src/locktime/absolute.rs | 9 ++++ units/src/locktime/relative.rs | 9 ++++ units/src/parse.rs | 88 +++++++++++++++++++++++++++++++++- units/src/weight.rs | 58 ++++++++++++++++++---- 9 files changed, 298 insertions(+), 27 deletions(-) diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index 79d2e7ec0..2c7f59f2b 100644 --- a/units/src/amount/mod.rs +++ b/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), diff --git a/units/src/amount/serde.rs b/units/src/amount/serde.rs index 7eb35f6e4..f022ba159 100644 --- a/units/src/amount/serde.rs +++ b/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)] diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 4f85f1347..f6b888840 100644 --- a/units/src/amount/tests.rs +++ b/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::()); let amounts = [Amount::from_sat(42), Amount::from_sat(1337), Amount::from_sat(21)]; + assert_eq!(amounts.iter().sum::(), Amount::from_sat(1400)); + let sum = amounts.into_iter().sum::(); 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::from_sat(1316)); + let sum = amounts.into_iter().sum::(); assert_eq!(SignedAmount::from_sat(1316), sum); } diff --git a/units/src/block.rs b/units/src/block.rs index 9e8efa521..8eaca0da4 100644 --- a/units/src/block.rs +++ b/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(6)); + assert!([BlockInterval(4), BlockInterval(5), BlockInterval(6)].into_iter().sum::() == 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)); } } diff --git a/units/src/fee_rate/mod.rs b/units/src/fee_rate/mod.rs index b12c08f45..2b5fe8205 100644 --- a/units/src/fee_rate/mod.rs +++ b/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(); diff --git a/units/src/locktime/absolute.rs b/units/src/locktime/absolute.rs index 966f6e129..b7ee722e6 100644 --- a/units/src/locktime/absolute.rs +++ b/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() { diff --git a/units/src/locktime/relative.rs b/units/src/locktime/relative.rs index 25c875ee0..a217391da 100644 --- a/units/src/locktime/relative.rs +++ b/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(); diff --git a/units/src/parse.rs b/units/src/parse.rs index b51b60fea..22401cc5d 100644 --- a/units/src/parse.rs +++ b/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::("1").is_ok()); + let _ = int::("not a number").map_err(|e| assert!(e.is_signed)); + let _ = int::("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 { + "Always invalid for testing".parse::().map(|_| TestTypeLargerThanU128(0, 0)) + } + } + impl From for TestTypeLargerThanU128 { + fn from(_: i8) -> Self { TestTypeLargerThanU128(0, 0) } + } + + let result = panic::catch_unwind(|| int::("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()); diff --git a/units/src/weight.rs b/units/src/weight.rs index 73bed18f1..df654e9d4 100644 --- a/units/src/weight.rs +++ b/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)); + } }