Drop `proptest`s in `lightning-liquidity`
What changed, and why it matters
This commit removes the proptest testing library from the lightning-liquidity Rust crate because one of proptest's indirect dependencies bumped its own minimum Rust version, breaking the project's MSRV (Minimum Supported Rust Version) policy. While removing the tests, the author also fixed a subtle rounding bug in the fee calculation helper: it now uses Rust's built-in div_ceil instead of the old add-999999-then-divide-by-1000000 trick. The commit also deletes a large block of URL parsing tests. There is no direct evidence this is a security fix, but the fee rounding change is a correctness improvement and the commit message explicitly notes that the previous tests were broken on most inputs.
Review the new div_ceil-based fee rounding for equivalence with the old behavior across edge cases, especially near u64::MAX. Consider adding focused non-proptest unit tests for compute_opening_fee and LSPSUrl parsing to restore coverage, since the commit removes property-based tests without fully replacing them. Monitor for any downstream MSRV issues from remaining dependencies.
Security signals we found
Functional code change in fee calculation helper (compute_opening_fee)
Author notes prior tests were broken on majority of inputs and proptest failed to catch it
Removal of property-based test coverage for URL parsing and fee calculation
MSRV breakage caused by transitive dependency (tempfile -> rand) in proptest tree
Evidence from the diff
The change is primarily a dependency cleanup: proptest and its CI pin are removed from lightning-liquidity. The functional code change is in compute_opening_fee in lsps2/utils.rs, where the rounding logic changes from checked_add(999999).checked_div(1000000) to div_ceil(1_000_000). This is mathematically equivalent for positive integers but is cleaner and avoids an intermediate overflow path. The commit also replaces one proptest in service.rs with a single randomized unit test and deletes proptest-based URL round-trip tests in lsps5/url_utils.rs. The author states the old proptest-based tests for compute_opening_fee were broken on most inputs and were not catching the issue because proptest was not exercising interesting cases.
Changed components
lightning-liquidity/src/lsps2/utils.rslightning-liquidity/src/lsps2/service.rslightning-liquidity/src/lsps5/url_utils.rslightning-liquidity/Cargo.tomlci/ci-tests.shInspect captured patch +45 / −217
diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh
index 820935f..83b2af2 100755
--- a/ci/ci-tests.sh
+++ b/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
diff --git a/lightning-liquidity/Cargo.toml b/lightning-liquidity/Cargo.toml
index d83d66f..c6cb4ee 100644
--- a/lightning-liquidity/Cargo.toml
+++ b/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 }
diff --git a/lightning-liquidity/src/lsps2/service.rs b/lightning-liquidity/src/lsps2/service.rs
index 1909e87..35942dc 100644
--- a/lightning-liquidity/src/lsps2/service.rs
+++ b/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);
}
}
}
diff --git a/lightning-liquidity/src/lsps2/utils.rs b/lightning-liquidity/src/lsps2/utils.rs
index 9f75a86..998b1d2 100644
--- a/lightning-liquidity/src/lsps2/utils.rs
+++ b/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);
- }
- }
- }
-}
diff --git a/lightning-liquidity/src/lsps5/url_utils.rs b/lightning-liquidity/src/lsps5/url_utils.rs
index c9d5f9e..2d49c10 100644
--- a/lightning-liquidity/src/lsps5/url_utils.rs
+++ b/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());
- }
- }
-}
Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.