Review, fmt fixes

Author: optout21

lightning-invoice/src/de.rs

@@ -52,19 +52,16 @@ impl<const N: usize> FromBase32 for [u8; N] {
 	fn from_base32(data: &[Fe32]) -> Result<Self, Self::Err> {
 		let mut res_arr = [0; N];
 		// Do in a for loop to place in the array directly, not using `collect`
-		let mut idx = 0;
+		let mut count = 0;
 		for elem in data.iter().copied().fes_to_bytes() {
-			if idx >= N {
-				break; // too many inputs, stop early, OK
+			if count < N {
+				res_arr[count] = elem;
 			}
-			res_arr[idx] = elem;
-			idx += 1;
+			count += 1;
 		}
-		if idx != N {
+		if count != N {
 			return Err(Bolt11ParseError::InvalidSliceLength(
-				data.len(),
-				(N * 8 + 4) / 5,
-				"<[u8; N]>",
+				count, N, "<[u8; N]>",
 			));
 		}
 		Ok(res_arr)
@@ -79,7 +76,7 @@ impl FromBase32 for PaymentSecret {
 			return Err(Bolt11ParseError::InvalidSliceLength(
 				field_data.len(),
 				52,
-				"payment secret",
+				"PaymentSecret",
 			));
 		}
 		let data_bytes = <[u8; 32]>::from_base32(field_data)?;
@@ -383,10 +380,11 @@ impl FromStr for SignedRawBolt11Invoice {
 	fn from_str(s: &str) -> Result<Self, Self::Err> {
 		let parsed = CheckedHrpstring::new::<Bech32>(s)?;
 		let hrp = parsed.hrp();
-		// Access original non-packed 32 byte values (as Fe32s) (iterator type is needed for API hack)
-		let data: Vec<_> = parsed.fe32_iter::<alloc::boxed::Box<dyn Iterator<Item = u8>>>().collect();
+		// Access original non-packed 32 byte values (as Fe32s)
+		// Note: the type argument is needed due to the API peculiarities, but it's not used
+		let data: Vec<_> = parsed.fe32_iter::<&mut dyn Iterator<Item = u8>>().collect();
 
-		const SIGNATURE_LEN5: usize = 104; // 32-bit values, 65 bytes
+		const SIGNATURE_LEN5: usize = 104; // number of the 5-bit values (equals to 65 bytes)
 		if data.len() < SIGNATURE_LEN5 {
 			return Err(Bolt11ParseError::TooShortDataPart);
 		}
@@ -467,7 +465,7 @@ impl FromBase32 for PositiveTimestamp {
 			return Err(Bolt11ParseError::InvalidSliceLength(
 				b32.len(),
 				7,
-				"timestamp",
+				"PositiveTimestamp",
 			));
 		}
 		let timestamp: u64 = parse_u64_be(b32)
@@ -486,7 +484,7 @@ impl FromBase32 for Bolt11InvoiceSignature {
 			return Err(Bolt11ParseError::InvalidSliceLength(
 				signature.len(),
 				104,
-				"signature",
+				"Bolt11InvoiceSignature",
 			));
 		}
 		let recoverable_signature_bytes = <[u8; 65]>::from_base32(signature)?;

lightning-invoice/src/test_ser_de.rs

@@ -76,16 +76,19 @@ fn array_u8_error_invalid_length() {
 		.err()
 		.unwrap()
 		.to_string(),
-		"Slice had length 4 instead of 5 for element <[u8; N]>"
+		"Slice had length 2 instead of 3 for element <[u8; N]>"
 	);
 
 	// input too long
 	assert_eq!(
 		<[u8; 3]>::from_base32(
-			&"pqqqql".to_string().chars().map(|c| Fe32::from_char(c).unwrap()).collect::<Vec<_>>()[..]
+			&"pqqqqql".to_string().chars().map(|c| Fe32::from_char(c).unwrap()).collect::<Vec<_>>()
+				[..]
 		)
-		.unwrap(),
-		[8, 0, 0]
+		.err()
+		.unwrap()
+		.to_string(),
+		"Slice had length 4 instead of 3 for element <[u8; N]>"
 	);
 }
 
@@ -151,17 +154,16 @@ fn bolt11_invoice_features() {
 	// To test skipping 0's in deserialization, we have to start with deserialization
 	assert_eq!(
 		Bolt11InvoiceFeatures::from_base32(
-			&"qqqqqry"
-				.to_string().chars().map(|c| Fe32::from_char(c).unwrap()).collect::<Vec<_>>()[..]
+			&"qqqqqry".to_string().chars().map(|c| Fe32::from_char(c).unwrap()).collect::<Vec<_>>()
+				[..]
 		)
 		.unwrap()
 		.le_flags(),
 		vec![100]
 	);
 	assert_eq!(
 		Bolt11InvoiceFeatures::from_base32(
-			&"ry"
-				.to_string().chars().map(|c| Fe32::from_char(c).unwrap()).collect::<Vec<_>>()[..]
+			&"ry".to_string().chars().map(|c| Fe32::from_char(c).unwrap()).collect::<Vec<_>>()[..]
 		)
 		.unwrap()
 		.le_flags(),

lightning-types/Cargo.toml

@@ -17,7 +17,6 @@ _test_utils = []
 
 [dependencies]
 bitcoin = { version = "0.32.2", default-features = false }
-bech32 = { version = "0.11.0", default-features = false }
 
 [lints]
 workspace = true