f misc fixes

Author: dunxen

lightning/src/ln/functional_tests.rs

@@ -9583,13 +9583,13 @@ fn accept_busted_but_better_fee() {
 	};
 }
 
-fn do_payment_with_custom_min_final_cltv_expiry(valid: bool, use_user_hash: bool) {
+fn do_payment_with_custom_min_final_cltv_expiry(valid_delta: bool, use_user_hash: bool) {
 	let mut chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 	let min_final_cltv_expiry_delta = 120;
-	let final_cltv_expiry_delta = if valid { min_final_cltv_expiry_delta + 2 } else {
+	let final_cltv_expiry_delta = if valid_delta { min_final_cltv_expiry_delta + 2 } else {
 		min_final_cltv_expiry_delta - 2 };
 	let recv_value = 100_000;
 
@@ -9614,7 +9614,7 @@ fn do_payment_with_custom_min_final_cltv_expiry(valid: bool, use_user_hash: bool
 	commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
 	expect_pending_htlcs_forwardable!(nodes[1]);
 
-	if valid {
+	if valid_delta {
 		expect_payment_claimable!(nodes[1], payment_hash, payment_secret, recv_value, if use_user_hash {
 			None } else { Some(payment_preimage) }, nodes[1].node.get_our_node_id());
 

lightning/src/ln/inbound_payment.rs

@@ -181,16 +181,19 @@ fn construct_metadata_bytes(min_value_msat: Option<u64>, payment_type: Method,
 
 	if min_final_cltv_expiry_delta.is_some() {
 		if {
-			// `min_value_msat` should fit in 61 bits as an unsigned integer.
-			min_value_msat.is_some() && min_value_msat.unwrap() > ((1 << 61) - 1) ||
-			// `expiry_timestamp` should fit in 48 bits as an unsigned integer.
-			expiry_timestamp > ((1 << 48) - 1)
+			// `min_value_msat` should fit in (64 bits - 3 payment type bits =) 61 bits as an unsigned integer.
+			// This should leave us with a maximum value greater than the 21M BTC supply cap anyway.
+			min_value_msat.is_some() && min_value_msat.unwrap() > ((1u64 << 61) - 1) ||
+			// `expiry_timestamp` should fit in (64 bits - 2 delta bytes =) 48 bits as an unsigned integer.
+			// Bitcoin's block header timestamps are actually `u32`s, so we're technically already limited to
+			// the much smaller maximum timestamp of `u32::MAX` for now, but we check the u64 `expiry_timestamp`
+			// for future-proofing.
+			expiry_timestamp > ((1u64 << 48) - 1)
 		} { return Err(()); }
 	}
 
-	// Pack the 16 `min_final_cltv_expiry_delta` bits into the first two bytes of invoice expiry
 	if let Some(min_final_cltv_expiry_delta) = min_final_cltv_expiry_delta {
-		let bytes = (min_final_cltv_expiry_delta as u16).to_be_bytes();
+		let bytes = min_final_cltv_expiry_delta.to_be_bytes();
 		expiry_bytes[0] |= bytes[0];
 		expiry_bytes[1] |= bytes[1];
 	}
@@ -294,7 +297,6 @@ pub(super) fn verify<L: Deref>(payment_hash: PaymentHash, payment_data: &msgs::F
 		}
 	}
 
-	// Match again to check for custom `min_final_cltv_expiry_delta`.
 	match payment_type_res {
 		Ok(Method::UserPaymentHashCustomFinalCltv) | Ok(Method::LdkPaymentHashCustomFinalCltv) => {
 			min_final_cltv_expiry_delta = Some(min_final_cltv_expiry_delta_from_metadata(metadata_bytes));