WIP: Drop OfferTlvStream

Author: jkczyz

lightning/src/offers/mod.rs

@@ -24,9 +24,10 @@ use core::time::Duration;
 use ln::PaymentHash;
 use ln::features::OfferFeatures;
 use ln::msgs::DecodeError;
-use util::ser::{HighZeroBytesDroppedVarInt, Readable, WithLength, WithoutLength};
+use util::ser::{HighZeroBytesDroppedVarInt, Readable, WithLength, WithoutLength, Writeable};
 
 use prelude::*;
+use io;
 
 #[cfg(feature = "std")]
 use std::time::SystemTime;
@@ -123,7 +124,9 @@ impl Offer {
 		self.signature.as_ref()
 	}
 
-	fn to_tlv_stream(&self) -> OfferTlvStream {
+	fn write(&self, writer: &mut Vec<u8>) -> Result<(), io::Error> {
+		let chains = self.chains.clone().map(|chains| WithoutLength(chains));
+
 		let (currency, amount) = match self.amount.clone() {
 			None => (None, None),
 			Some(Amount::Bitcoin { amount_msats }) => (
@@ -134,33 +137,43 @@ impl Offer {
 			),
 		};
 
+		let description = Some(WithoutLength(self.description.clone()));
+		let features = self.features.as_ref();
+		let absolute_expiry = self.absolute_expiry.map(|d| HighZeroBytesDroppedVarInt(d.as_secs()));
+
 		let (paths, node_id) = match self.destination.clone() {
 			Destination::NodeId(node_id) => (None, Some(node_id)),
 			Destination::Paths(paths) => (Some(WithoutLength(paths)), None),
 		};
 
-		let (send_invoice, refund_for) = match self.send_invoice.clone() {
+		let issuer = self.issuer.clone().map(|issuer| WithoutLength(issuer));
+		let quantity_min = self.quantity_min.map(|quantity| HighZeroBytesDroppedVarInt(quantity));
+		let quantity_max = self.quantity_max.map(|quantity| HighZeroBytesDroppedVarInt(quantity));
+		let recurrence = self.recurrence.as_ref();
+
+		let (send_invoice, refund_for) = match self.send_invoice {
 			None => (None, None),
 			Some(SendInvoice { refund_for }) => (Some(()), refund_for),
 		};
 
-		OfferTlvStream {
-			chains: self.chains.clone().map(|chains| WithoutLength(chains)),
-			currency,
-			amount,
-			description: Some(WithoutLength(self.description.clone())),
-			features: self.features.clone(),
-			absolute_expiry: self.absolute_expiry.map(|d| HighZeroBytesDroppedVarInt(d.as_secs())),
-			paths,
-			issuer: self.issuer.clone().map(|issuer| WithoutLength(issuer)),
-			quantity_min: self.quantity_min.map(|quantity| HighZeroBytesDroppedVarInt(quantity)),
-			quantity_max: self.quantity_max.map(|quantity| HighZeroBytesDroppedVarInt(quantity)),
-			recurrence: self.recurrence.clone(),
-			node_id,
-			send_invoice,
-			refund_for,
-			signature: self.signature,
-		}
+		encode_tlv_stream!(writer, {
+			(2, chains, option),
+			(6, currency, option),
+			(8, amount, option),
+			(10, description, option),
+			(12, self.features, option),
+			(14, absolute_expiry, option),
+			(16, paths, option),
+			(20, issuer, option),
+			(22, quantity_min, option),
+			(24, quantity_max, option),
+			(26, recurrence, option),
+			(30, node_id, option),
+			(34, refund_for, option),
+			(54, send_invoice, option),
+			(240, self.signature, option),
+		});
+		Ok(())
 	}
 }
 
@@ -208,45 +221,6 @@ pub struct SendInvoice {
 	refund_for: Option<PaymentHash>,
 }
 
-/// An `offer` TLV stream without any semantic checks, apart from any checks performed when parsing
-/// the underlying types.
-#[derive(Debug)]
-struct OfferTlvStream {
-	chains: Option<WithoutLength<Vec<BlockHash>>>,
-	currency: Option<WithoutLength<String>>,
-	amount: Option<HighZeroBytesDroppedVarInt<u64>>,
-	description: Option<WithoutLength<String>>,
-	features: Option<OfferFeatures>,
-	absolute_expiry: Option<HighZeroBytesDroppedVarInt<u64>>,
-	paths: Option<WithoutLength<Vec<BlindedPath>>>,
-	issuer: Option<WithoutLength<String>>,
-	quantity_min: Option<HighZeroBytesDroppedVarInt<u64>>,
-	quantity_max: Option<HighZeroBytesDroppedVarInt<u64>>,
-	recurrence: Option<Recurrence>,
-	node_id: Option<XOnlyPublicKey>,
-	send_invoice: Option<()>,
-	refund_for: Option<PaymentHash>,
-	signature: Option<Signature>,
-}
-
-impl_writeable_tlv_stream!(OfferTlvStream, {
-	(2, chains, option),
-	(6, currency, option),
-	(8, amount, option),
-	(10, description, option),
-	(12, features, option),
-	(14, absolute_expiry, option),
-	(16, paths, option),
-	(20, issuer, option),
-	(22, quantity_min, option),
-	(24, quantity_max, option),
-	(26, recurrence, option),
-	(30, node_id, option),
-	(34, refund_for, option),
-	(54, send_invoice, option),
-	(240, signature, option),
-});
-
 #[derive(Clone, Debug)]
 ///
 pub struct BlindedPath {
@@ -273,9 +247,8 @@ struct Recurrence {
 
 impl_writeable!(Recurrence, { time_unit, period });
 
-/// An offer parsed from a bech32-encoded string as a TLV stream and the corresponding bytes. The
-/// latter is used for signature verification.
-struct ParsedOffer(OfferTlvStream, Vec<u8>);
+/// An offer parsed from a bech32-encoded string as serialized bytes.
+struct ParsedOffer(Vec<u8>);
 
 /// Error when parsing a bech32 encoded message using [`str::parse`].
 #[derive(Debug, PartialEq)]
@@ -337,9 +310,9 @@ impl From<SemanticError> for ParseError {
 	}
 }
 
-impl From<secp256k1::Error> for SemanticError {
+impl From<secp256k1::Error> for ParseError {
 	fn from(error: secp256k1::Error) -> Self {
-		Self::InvalidSignature(error)
+		SemanticError::InvalidSignature(error).into()
 	}
 }
 
@@ -352,13 +325,41 @@ impl FromStr for Offer {
 }
 
 impl TryFrom<ParsedOffer> for Offer {
-	type Error = SemanticError;
+	type Error = ParseError;
 
 	fn try_from(offer: ParsedOffer) -> Result<Self, Self::Error> {
-		let ParsedOffer(OfferTlvStream {
-			chains, currency, amount, description, features, absolute_expiry, paths, issuer,
-			quantity_min, quantity_max, recurrence, node_id, send_invoice, refund_for, signature,
-		}, data) = offer;
+		let mut chains: Option<WithoutLength<Vec<BlockHash>>> = None;
+		let mut currency: Option<WithoutLength<String>> = None;
+		let mut amount = None;
+		let mut description: Option<WithoutLength<String>> = None;
+		let mut features = None;
+		let mut absolute_expiry: Option<HighZeroBytesDroppedVarInt<u64>> = None;
+		let mut paths: Option<WithoutLength<Vec<BlindedPath>>> = None;
+		let mut issuer: Option<WithoutLength<String>> = None;
+		let mut quantity_min = None;
+		let mut quantity_max = None;
+		let mut recurrence = None;
+		let mut node_id = None;
+		let mut send_invoice = None;
+		let mut refund_for = None;
+		let mut signature = None;
+		decode_tlv_stream!(&mut &offer.0[..], {
+			(2, chains, option),
+			(6, currency, option),
+			(8, amount, option),
+			(10, description, option),
+			(12, features, option),
+			(14, absolute_expiry, option),
+			(16, paths, option),
+			(20, issuer, option),
+			(22, quantity_min, option),
+			(24, quantity_max, option),
+			(26, recurrence, option),
+			(30, node_id, option),
+			(34, refund_for, option),
+			(54, send_invoice, option),
+			(240, signature, option),
+		});
 
 		let supported_chains = [
 			genesis_block(Network::Bitcoin).block_hash(),
@@ -371,59 +372,59 @@ impl TryFrom<ParsedOffer> for Offer {
 			Some(WithoutLength(chains)) => match chains.first() {
 				None => Some(chains),
 				Some(chain) if supported_chains.contains(chain) => Some(chains),
-				_ => return Err(SemanticError::UnsupportedChain),
+				_ => return Err(SemanticError::UnsupportedChain.into()),
 			},
 		};
 
 		let amount = match (currency, amount) {
 			(None, None) => None,
 			(None, Some(amount_msats)) => Some(Amount::Bitcoin { amount_msats: amount_msats.0 }),
-			(Some(_), None) => return Err(SemanticError::UnexpectedCurrency),
+			(Some(_), None) => return Err(SemanticError::UnexpectedCurrency.into()),
 			(Some(WithoutLength(iso4217_code)), Some(HighZeroBytesDroppedVarInt(amount))) => {
 				if iso4217_code.len() != ISO4217_CODE_LEN {
-					return Err(SemanticError::InvalidCurrencyEncoding);
+					return Err(SemanticError::InvalidCurrencyEncoding.into());
 				}
 				Some(Amount::Currency { iso4217_code, amount })
 			},
 		};
 
 		if description.is_none() {
-			return Err(SemanticError::MissingDescription);
+			return Err(SemanticError::MissingDescription.into());
 		}
 
 		let destination = match (node_id, paths) {
-			(None, None) => return Err(SemanticError::MissingDestination),
-			(Some(_), Some(_)) => return Err(SemanticError::DuplicateDestination),
+			(None, None) => return Err(SemanticError::MissingDestination.into()),
+			(Some(_), Some(_)) => return Err(SemanticError::DuplicateDestination.into()),
 			(Some(node_id), None) => Destination::NodeId(node_id),
-			(None, Some(paths)) if paths.0.is_empty() => return Err(SemanticError::MissingPaths),
+			(None, Some(paths)) if paths.0.is_empty() => return Err(SemanticError::MissingPaths.into()),
 			(None, Some(WithoutLength(paths))) => Destination::Paths(paths),
 		};
 
 		if let Some(HighZeroBytesDroppedVarInt(quantity_min)) = quantity_min {
 			if quantity_min < 1 {
-				return Err(SemanticError::InvalidQuantity);
+				return Err(SemanticError::InvalidQuantity.into());
 			}
 
 			if let Some(HighZeroBytesDroppedVarInt(quantity_max)) = quantity_max {
 				if quantity_min > quantity_max {
-					return Err(SemanticError::InvalidQuantity);
+					return Err(SemanticError::InvalidQuantity.into());
 				}
 			}
 		}
 
 		if let Some(HighZeroBytesDroppedVarInt(quantity_max)) = quantity_max {
 			if quantity_max < 1 {
-				return Err(SemanticError::InvalidQuantity);
+				return Err(SemanticError::InvalidQuantity.into());
 			}
 		}
 
 		let send_invoice = match (send_invoice, refund_for) {
 			(None, None) => None,
-			(None, Some(_)) => return Err(SemanticError::UnexpectedRefund),
+			(None, Some(_)) => return Err(SemanticError::UnexpectedRefund.into()),
 			(Some(_), _) => Some(SendInvoice { refund_for }),
 		};
 
-		let id = merkle::root_hash(&data);
+		let id = merkle::root_hash(&offer.0);
 		if let Some(signature) = &signature {
 			let tag = sha256::Hash::hash(concat!("lightning", "offer", "signature").as_bytes());
 			let message = Message::from_slice(&merkle::tagged_hash(tag, id)).unwrap();
@@ -472,26 +473,18 @@ impl FromStr for ParsedOffer {
 		}
 
 		let data = Vec::<u8>::from_base32(&data)?;
-		Ok(ParsedOffer(Readable::read(&mut &data[..])?, data))
+		Ok(ParsedOffer(data))
 	}
 }
 
 impl core::fmt::Display for Offer {
 	fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
-		self.to_tlv_stream().fmt(f)
-	}
-}
-
-impl core::fmt::Display for OfferTlvStream {
-	fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
-		use util::ser::Writeable;
 		let mut buffer = Vec::new();
 		self.write(&mut buffer).unwrap();
 
 		use bitcoin::bech32::ToBase32;
 		let data = buffer.to_base32();
 		bech32::encode_without_checksum_to_fmt(f, OFFER_BECH32_HRP, data).expect("HRP is valid").unwrap();
-
 		Ok(())
 	}
 }

lightning/src/util/ser_macros.rs

@@ -135,7 +135,7 @@ macro_rules! check_missing_tlv {
 		#[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true
 		let missing_req_type = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type;
 		if missing_req_type {
-			return Err(DecodeError::InvalidValue);
+			return Err(DecodeError::InvalidValue.into());
 		}
 	}};
 	($last_seen_type: expr, $type: expr, $field: ident, vec_type) => {{
@@ -194,18 +194,18 @@ macro_rules! decode_tlv_stream {
 						if !tracking_reader.have_read {
 							break 'tlv_read;
 						} else {
-							return Err(DecodeError::ShortRead);
+							return Err(DecodeError::ShortRead.into());
 						}
 					},
-					Err(e) => return Err(e),
+					Err(e) => return Err(e.into()),
 					Ok(t) => t,
 				}
 			};
 
 			// Types must be unique and monotonically increasing:
 			match last_seen_type {
 				Some(t) if typ.0 <= t => {
-					return Err(DecodeError::InvalidValue);
+					return Err(DecodeError::InvalidValue.into());
 				},
 				_ => {},
 			}
@@ -223,11 +223,11 @@ macro_rules! decode_tlv_stream {
 					decode_tlv!(s, $field, $fieldty);
 					if s.bytes_remain() {
 						s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
-						return Err(DecodeError::InvalidValue);
+						return Err(DecodeError::InvalidValue.into());
 					}
 				},)*
 				x if x % 2 == 0 => {
-					return Err(DecodeError::UnknownRequiredFeature);
+					return Err(DecodeError::UnknownRequiredFeature.into());
 				},
 				_ => {},
 			}