Error handling improvements

Author: optout21

lightning-invoice/src/de.rs

@@ -9,7 +9,7 @@ use core::str;
 use core::str::FromStr;
 
 use bech32::{Bech32, Fe32, Fe32IterExt};
-use bech32::primitives::decode::{CheckedHrpstring, CheckedHrpstringError, ChecksumError};
+use bech32::primitives::decode::{CheckedHrpstring, CheckedHrpstringError};
 
 use bitcoin::{PubkeyHash, ScriptHash, WitnessVersion};
 use bitcoin::hashes::Hash;
@@ -39,27 +39,28 @@ pub trait FromBase32: Sized {
 // FromBase32 implementations are here, because the trait is in this module.
 
 impl FromBase32 for Vec<u8> {
-	type Err = CheckedHrpstringError;
+	type Err = Bolt11ParseError;
 
 	fn from_base32(data: &[Fe32]) -> Result<Self, Self::Err> {
 		Ok(data.iter().copied().fes_to_bytes().collect::<Self>())
 	}
 }
 
 impl<const N: usize> FromBase32 for [u8; N] {
-	type Err = CheckedHrpstringError;
+	type Err = Bolt11ParseError;
 
 	fn from_base32(data: &[Fe32]) -> Result<Self, Self::Err> {
-		Ok(data.iter().copied().fes_to_bytes().collect::<Vec<_>>().try_into().unwrap())
+		data.iter().copied().fes_to_bytes().collect::<Vec<_>>().try_into()
+			.map_err(|_| Bolt11ParseError::InvalidSliceLength(data.len(), (N * 8 + 4) / 5, "<[u8; N]>::from_base32()".into()))
 	}
 }
 
 impl FromBase32 for PaymentSecret {
-	type Err = CheckedHrpstringError;
+	type Err = Bolt11ParseError;
 
 	fn from_base32(field_data: &[Fe32]) -> Result<Self, Self::Err> {
 		if field_data.len() != 52 {
-			return Err(CheckedHrpstringError::Checksum(ChecksumError::InvalidLength)) // TODO(bech32): not entirely accurate
+			return Err(Bolt11ParseError::InvalidSliceLength(field_data.len(), 52, "PaymentSecret::from_base32()".into()));
 		}
 		let data_bytes = Vec::<u8>::from_base32(field_data)?;
 		let mut payment_secret = [0; 32];
@@ -69,7 +70,7 @@ impl FromBase32 for PaymentSecret {
 }
 
 impl FromBase32 for Bolt11InvoiceFeatures {
-	type Err = CheckedHrpstringError;
+	type Err = Bolt11ParseError;
 
 	/// Convert to byte values, by packing the 5-bit groups,
 	/// putting the 5-bit values from left to-right (reverse order),
@@ -440,7 +441,7 @@ impl FromBase32 for PositiveTimestamp {
 
 	fn from_base32(b32: &[Fe32]) -> Result<Self, Self::Err> {
 		if b32.len() != 7 {
-			return Err(Bolt11ParseError::InvalidSliceLength("PositiveTimestamp::from_base32()".into()));
+			return Err(Bolt11ParseError::InvalidSliceLength(b32.len(), 7, "PositiveTimestamp::from_base32()".into()));
 		}
 		let timestamp: u64 = parse_u64_be(b32)
 			.expect("7*5bit < 64bit, no overflow possible");
@@ -455,7 +456,7 @@ impl FromBase32 for Bolt11InvoiceSignature {
 	type Err = Bolt11ParseError;
 	fn from_base32(signature: &[Fe32]) -> Result<Self, Self::Err> {
 		if signature.len() != 104 {
-			return Err(Bolt11ParseError::InvalidSliceLength("Bolt11InvoiceSignature::from_base32()".into()));
+			return Err(Bolt11ParseError::InvalidSliceLength(signature.len(), 104, "Bolt11InvoiceSignature::from_base32()".into()));
 		}
 		let recoverable_signature_bytes = Vec::<u8>::from_base32(signature)?;
 		let signature = &recoverable_signature_bytes[0..64];
@@ -508,7 +509,7 @@ fn parse_tagged_parts(data: &[Fe32]) -> Result<Vec<RawTaggedField>, Bolt11ParseE
 			Ok(field) => {
 				parts.push(RawTaggedField::KnownSemantics(field))
 			},
-			Err(Bolt11ParseError::Skip)|Err(Bolt11ParseError::Bech32Error(_)) => {
+			Err(Bolt11ParseError::Skip) | Err(Bolt11ParseError::InvalidSliceLength(_, _, _)) | Err(Bolt11ParseError::Bech32Error(_)) => {
 				parts.push(RawTaggedField::UnknownSemantics(field.into()))
 			},
 			Err(e) => {return Err(e)}
@@ -725,8 +726,8 @@ impl Display for Bolt11ParseError {
 			Bolt11ParseError::DescriptionDecodeError(ref e) => {
 				write!(f, "Description is not a valid utf-8 string: {}", e)
 			}
-			Bolt11ParseError::InvalidSliceLength(ref function) => {
-				write!(f, "Slice in function {} had the wrong length", function)
+			Bolt11ParseError::InvalidSliceLength(ref len, ref expected, ref function) => {
+				write!(f, "Slice had length {} instead of {} in function {}", len, expected, function)
 			}
 			Bolt11ParseError::BadPrefix => f.write_str("did not begin with 'ln'"),
 			Bolt11ParseError::UnknownCurrency => f.write_str("currency code unknown"),
@@ -792,9 +793,7 @@ from_error!(Bolt11ParseError::DescriptionDecodeError, str::Utf8Error);
 
 impl From<CheckedHrpstringError> for Bolt11ParseError {
 	fn from(e: CheckedHrpstringError) -> Self {
-		match e {
-			_ => Bolt11ParseError::Bech32Error(e)
-		}
+		Self::Bech32Error(e)
 	}
 }
 

lightning-invoice/src/lib.rs

@@ -105,7 +105,8 @@ pub enum Bolt11ParseError {
 	InvalidPubKeyHashLength,
 	InvalidScriptHashLength,
 	InvalidRecoveryId,
-	InvalidSliceLength(String),
+	// Invalid length, with actual length, expected length, and function info
+	InvalidSliceLength(usize, usize, String),
 
 	/// Not an error, but used internally to signal that a part of the invoice should be ignored
 	/// according to BOLT11