Drop proptests in lightning-liquidity

`proptest`'s transitive dependency tree has always been somewhat
large, but one of them (`rusty-fork`'s `tempfile` dependency) just
went ahead with a bump of their `rand` dependency, breaking our
MSRV yet again.

Because we don't actually use `proptest` for anything interesting,
the simplest solution is to simply drop it, which we do here. Note
that we'll likely transition the LSPS5 URL type to simply use the
`bitreq` URL type over the next few days anyway, so there's not
much reason to care about its continued test coverage.

Further, in writing this commit it was discovered that our tests in
`lsps2/utils.rs` were actually broken on the vast majority of
inputs, but proptest wasn't testing with any interesting test cases
at all, causing it to be missed entirely!

Author: TheBlueMatt

ci/ci-tests.sh

@@ -16,9 +16,6 @@ PIN_RELEASE_DEPS # pin the release dependencies in our main workspace
 # The backtrace v0.3.75 crate relies on rustc 1.82
 [ "$RUSTC_MINOR_VERSION" -lt 82 ] && cargo update -p backtrace --precise "0.3.74" --quiet
 
-# proptest 1.9.0 requires rustc 1.82.0
-[ "$RUSTC_MINOR_VERSION" -lt 82 ] && cargo update -p proptest --precise "1.8.0" --quiet
-
 # Starting with version 1.2.0, the `idna_adapter` crate has an MSRV of rustc 1.81.0.
 [ "$RUSTC_MINOR_VERSION" -lt 81 ] && cargo update -p idna_adapter --precise "1.1.0" --quiet
 

lightning-liquidity/Cargo.toml

@@ -39,7 +39,6 @@ lightning = { version = "0.3.0", path = "../lightning", default-features = false
 lightning-invoice = { version = "0.35.0", path = "../lightning-invoice", default-features = false, features = ["serde", "std"] }
 lightning-persister = { version = "0.3.0", path = "../lightning-persister", default-features = false }
 
-proptest = "1.0.0"
 tokio = { version = "1.35", default-features = false, features = [ "rt-multi-thread", "time", "sync", "macros" ] }
 parking_lot = { version = "0.12", default-features = false }
 

lightning-liquidity/src/lsps2/service.rs

@@ -2351,64 +2351,58 @@ mod tests {
 
 	use crate::lsps0::ser::LSPSDateTime;
 
-	use proptest::prelude::*;
-
 	use bitcoin::{absolute::LockTime, transaction::Version};
 	use core::str::FromStr;
 
 	const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;
 
-	fn arb_forward_amounts() -> impl Strategy<Value = (u64, u64, u64, u64)> {
-		(1u64..MAX_VALUE_MSAT, 1u64..MAX_VALUE_MSAT, 1u64..MAX_VALUE_MSAT, 1u64..MAX_VALUE_MSAT)
-			.prop_map(|(a, b, c, d)| {
-				(a, b, c, core::cmp::min(d, a.saturating_add(b).saturating_add(c)))
-			})
-	}
+	#[test]
+	fn rand_test_calculate_amount_to_forward() {
+		use std::collections::hash_map::RandomState;
+		use std::hash::{BuildHasher, Hasher};
+
+		let total_fee_msat = RandomState::new().build_hasher().finish() % MAX_VALUE_MSAT;
+		let htlc_count = (RandomState::new().build_hasher().finish() % 10) as u8;
+
+		let mut htlcs = Vec::new();
+		let mut total_received_msat = 0;
+		let mut htlc_values = Vec::new();
+		for i in 0..htlc_count {
+			let expected_outbound_amount_msat =
+				RandomState::new().build_hasher().finish() % MAX_VALUE_MSAT;
+			if total_received_msat + expected_outbound_amount_msat > MAX_VALUE_MSAT {
+				break;
+			}
+			total_received_msat += expected_outbound_amount_msat;
+			htlc_values.push(total_received_msat);
+			htlcs.push(InterceptedHTLC {
+				intercept_id: InterceptId([i; 32]),
+				expected_outbound_amount_msat,
+				payment_hash: PaymentHash([i; 32]),
+			});
+		}
 
-	proptest! {
-		#[test]
-		fn proptest_calculate_amount_to_forward((o_0, o_1, o_2, total_fee_msat) in arb_forward_amounts()) {
-			let htlcs = vec![
-				InterceptedHTLC {
-					intercept_id: InterceptId([0; 32]),
-					expected_outbound_amount_msat: o_0,
-					payment_hash: PaymentHash([0; 32]),
-				},
-				InterceptedHTLC {
-					intercept_id: InterceptId([1; 32]),
-					expected_outbound_amount_msat: o_1,
-					payment_hash: PaymentHash([0; 32]),
-				},
-				InterceptedHTLC {
-					intercept_id: InterceptId([2; 32]),
-					expected_outbound_amount_msat: o_2,
-					payment_hash: PaymentHash([0; 32]),
-				},
-			];
+		if total_fee_msat > total_received_msat {
+			return;
+		}
 
-			let result = calculate_amount_to_forward_per_htlc(&htlcs, total_fee_msat);
-			let total_received_msat = o_0 + o_1 + o_2;
+		let result = calculate_amount_to_forward_per_htlc(&htlcs, total_fee_msat);
 
-			if total_received_msat < total_fee_msat {
-				assert_eq!(result.len(), 0);
-			} else {
-				assert_ne!(result.len(), 0);
-				assert_eq!(result[0].0, htlcs[0].intercept_id);
-				assert_eq!(result[1].0, htlcs[1].intercept_id);
-				assert_eq!(result[2].0, htlcs[2].intercept_id);
-				assert!(result[0].1 <= o_0);
-				assert!(result[1].1 <= o_1);
-				assert!(result[2].1 <= o_2);
-
-				let result_sum = result.iter().map(|(_, f)| f).sum::<u64>();
-				assert_eq!(total_received_msat - result_sum, total_fee_msat);
-				let five_pct = result_sum as f32 * 0.05;
-				let fair_share_0 = (o_0 as f32 / total_received_msat as f32) * result_sum as f32;
-				assert!(result[0].1 as f32 <= fair_share_0 + five_pct);
-				let fair_share_1 = (o_1 as f32 / total_received_msat as f32) * result_sum as f32;
-				assert!(result[1].1 as f32 <= fair_share_1 + five_pct);
-				let fair_share_2 = (o_2 as f32 / total_received_msat as f32) * result_sum as f32;
-				assert!(result[2].1 as f32 <= fair_share_2 + five_pct);
+		if total_received_msat < total_fee_msat {
+			assert_eq!(result.len(), 0);
+		} else {
+			assert_eq!(result.len(), htlcs.len());
+			let result_sum = result.iter().map(|(_, f)| f).sum::<u64>();
+			assert_eq!(total_received_msat - result_sum, total_fee_msat);
+			let five_pct = result_sum as f32 * 0.05;
+
+			for ((htlc, htlc_value), res) in htlcs.iter().zip(htlc_values).zip(result.iter()) {
+				assert_eq!(res.0, htlc.intercept_id);
+				assert!(res.1 <= htlc_value);
+
+				let fair_share =
+					(htlc_value as f32 / total_received_msat as f32) * result_sum as f32;
+				assert!(res.1 as f32 <= fair_share + five_pct);
 			}
 		}
 	}

lightning-liquidity/src/lsps2/utils.rs

@@ -60,33 +60,6 @@ pub fn compute_opening_fee(
 ) -> Option<u64> {
 	payment_size_msat
 		.checked_mul(opening_fee_proportional)
-		.and_then(|f| f.checked_add(999999))
-		.and_then(|f| f.checked_div(1000000))
+		.map(|f| f.div_ceil(1_000_000))
 		.map(|f| core::cmp::max(f, opening_fee_min_fee_msat))
 }
-
-#[cfg(test)]
-mod tests {
-	use super::*;
-	use proptest::prelude::*;
-
-	const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;
-
-	fn arb_opening_fee_params() -> impl Strategy<Value = (u64, u64, u64)> {
-		(0u64..MAX_VALUE_MSAT, 0u64..MAX_VALUE_MSAT, 0u64..MAX_VALUE_MSAT)
-	}
-
-	proptest! {
-		#[test]
-		fn test_compute_opening_fee((payment_size_msat, opening_fee_min_fee_msat, opening_fee_proportional) in arb_opening_fee_params()) {
-			if let Some(res) = compute_opening_fee(payment_size_msat, opening_fee_min_fee_msat, opening_fee_proportional) {
-				assert!(res >= opening_fee_min_fee_msat);
-				assert_eq!(res as f32, (payment_size_msat as f32 * opening_fee_proportional as f32));
-			} else {
-				// Check we actually overflowed.
-				let max_value = u64::MAX as u128;
-				assert!((payment_size_msat as u128 * opening_fee_proportional as u128) > max_value);
-			}
-		}
-	}
-}

lightning-liquidity/src/lsps5/url_utils.rs

@@ -102,138 +102,3 @@ impl Readable for LSPSUrl {
 		Ok(Self(Readable::read(reader)?))
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use super::*;
-	use crate::alloc::string::ToString;
-	use alloc::vec::Vec;
-	use proptest::prelude::*;
-
-	#[test]
-	fn test_extremely_long_url() {
-		let url_str = format!("https://{}/path", "a".repeat(1000)).to_string();
-		let url_chars = url_str.chars().count();
-		let result = LSPSUrl::parse(url_str);
-
-		assert!(result.is_ok());
-		let url = result.unwrap();
-		assert_eq!(url.0 .0.chars().count(), url_chars);
-	}
-
-	#[test]
-	fn test_parse_http_url() {
-		let url_str = "http://example.com/path".to_string();
-		let url = LSPSUrl::parse(url_str).unwrap_err();
-		assert_eq!(url, LSPS5ProtocolError::UnsupportedProtocol);
-	}
-
-	#[test]
-	fn valid_lsps_url() {
-		let test_vec: Vec<&'static str> = vec![
-			"https://www.example.org/push?l=1234567890abcopqrstuv&c=best",
-			"https://www.example.com/path",
-			"https://example.org",
-			"https://example.com:8080/path",
-			"https://api.example.com/v1/resources",
-			"https://example.com/page#section1",
-			"https://example.com/search?q=test#results",
-			"https://user:pass@example.com/",
-			"https://192.168.1.1/admin",
-			"https://example.com://path",
-			"https://example.com/path%20with%20spaces",
-			"https://example_example.com/path?query=with&spaces=true",
-		];
-		for url_str in test_vec {
-			let url = LSPSUrl::parse(url_str.to_string());
-			assert!(url.is_ok(), "Failed to parse URL: {}", url_str);
-		}
-	}
-
-	#[test]
-	fn invalid_lsps_url() {
-		let test_vec = vec![
-			"ftp://ftp.example.org/pub/files/document.pdf",
-			"sftp://user:password@sftp.example.com:22/uploads/",
-			"ssh://username@host.com:2222",
-			"lightning://03a.example.com/invoice?amount=10000",
-			"ftp://user@ftp.example.com/files/",
-			"https://例子.测试/path",
-			"a123+-.://example.com",
-			"a123+-.://example.com",
-			"https:\\\\example.com\\path",
-			"https:///whatever",
-			"https://example.com/path with spaces",
-		];
-		for url_str in test_vec {
-			let url = LSPSUrl::parse(url_str.to_string());
-			assert!(url.is_err(), "Expected error for URL: {}", url_str);
-		}
-	}
-
-	#[test]
-	fn parsing_errors() {
-		let test_vec = vec![
-			"example.com/path",
-			"https://bad domain.com/",
-			"https://example.com\0/path",
-			"https://",
-			"ht@ps://example.com",
-			"http!://example.com",
-			"1https://example.com",
-			"https://://example.com",
-			"https://example.com:port/path",
-			"https://:8080/path",
-			"https:",
-			"://",
-			"https://example.com\0/path",
-		];
-		for url_str in test_vec {
-			let url = LSPSUrl::parse(url_str.to_string());
-			assert!(url.is_err(), "Expected error for URL: {}", url_str);
-		}
-	}
-
-	fn host_strategy() -> impl Strategy<Value = String> {
-		prop_oneof![
-			proptest::string::string_regex(
-				"[a-z0-9]+(?:-[a-z0-9]+)*(?:\\.[a-z0-9]+(?:-[a-z0-9]+)*)*"
-			)
-			.unwrap(),
-			(0u8..=255u8, 0u8..=255u8, 0u8..=255u8, 0u8..=255u8)
-				.prop_map(|(a, b, c, d)| format!("{}.{}.{}.{}", a, b, c, d))
-		]
-	}
-
-	proptest! {
-		#[test]
-		fn proptest_parse_round_trip(
-			host in host_strategy(),
-			port in proptest::option::of(0u16..=65535u16),
-			path in proptest::option::of(proptest::string::string_regex("[a-zA-Z0-9._%&=:@/-]{0,20}").unwrap()),
-			query in proptest::option::of(proptest::string::string_regex("[a-zA-Z0-9._%&=:@/-]{0,20}").unwrap()),
-			fragment in proptest::option::of(proptest::string::string_regex("[a-zA-Z0-9._%&=:@/-]{0,20}").unwrap())
-		) {
-			let mut url = format!("https://{}", host);
-			if let Some(p) = port {
-				url.push_str(&format!(":{}", p));
-			}
-			if let Some(pth) = &path {
-				url.push('/');
-				url.push_str(pth);
-			}
-			if let Some(q) = &query {
-				url.push('?');
-				url.push_str(q);
-			}
-			if let Some(f) = &fragment {
-				url.push('#');
-				url.push_str(f);
-			}
-
-			let parsed = LSPSUrl::parse(url.clone()).expect("should parse");
-			prop_assert_eq!(parsed.url(), url.as_str());
-			prop_assert_eq!(parsed.url_length(), url.chars().count());
-		}
-	}
-}