Implement Writeable/Readable for Option<T>

Author: Antoine Riard

src/ln/channel.rs

@@ -3689,14 +3689,6 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 			});
 		}
 
-		macro_rules! read_option { () => {
-			match <u8 as Readable<R>>::read(reader)? {
-				0 => None,
-				1 => Some(Readable::read(reader)?),
-				_ => return Err(DecodeError::InvalidValue { byte: None }),
-			}
-		} }
-
 		let pending_outbound_htlc_count: u64 = Readable::read(reader)?;
 		let mut pending_outbound_htlcs = Vec::with_capacity(cmp::min(pending_outbound_htlc_count as usize, OUR_MAX_HTLCS as usize));
 		for _ in 0..pending_outbound_htlc_count {
@@ -3706,7 +3698,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 				cltv_expiry: Readable::read(reader)?,
 				payment_hash: Readable::read(reader)?,
 				source: Readable::read(reader)?,
-				fail_reason: read_option!(),
+				fail_reason: Readable::read(reader)?,
 				state: match <u8 as Readable<R>>::read(reader)? {
 					0 => OutboundHTLCState::LocalAnnounced(Box::new(Readable::read(reader)?)),
 					1 => OutboundHTLCState::Committed,
@@ -3764,8 +3756,8 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 			monitor_pending_failures.push((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?));
 		}
 
-		let pending_update_fee = read_option!();
-		let holding_cell_update_fee = read_option!();
+		let pending_update_fee = Readable::read(reader)?;
+		let holding_cell_update_fee = Readable::read(reader)?;
 
 		let next_local_htlc_id = Readable::read(reader)?;
 		let next_remote_htlc_id = Readable::read(reader)?;
@@ -3787,8 +3779,8 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 			_ => return Err(DecodeError::InvalidValue { byte: None }),
 		};
 
-		let funding_tx_confirmed_in = read_option!();
-		let short_channel_id = read_option!();
+		let funding_tx_confirmed_in = Readable::read(reader)?;
+		let short_channel_id = Readable::read(reader)?;
 
 		let last_block_connected = Readable::read(reader)?;
 		let funding_tx_confirmations = Readable::read(reader)?;
@@ -3803,17 +3795,17 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 		let their_max_accepted_htlcs = Readable::read(reader)?;
 		let minimum_depth = Readable::read(reader)?;
 
-		let their_funding_pubkey = read_option!();
-		let their_revocation_basepoint = read_option!();
-		let their_payment_basepoint = read_option!();
-		let their_delayed_payment_basepoint = read_option!();
-		let their_htlc_basepoint = read_option!();
-		let their_cur_commitment_point = read_option!();
+		let their_funding_pubkey = Readable::read(reader)?;
+		let their_revocation_basepoint = Readable::read(reader)?;
+		let their_payment_basepoint = Readable::read(reader)?;
+		let their_delayed_payment_basepoint = Readable::read(reader)?;
+		let their_htlc_basepoint = Readable::read(reader)?;
+		let their_cur_commitment_point = Readable::read(reader)?;
 
-		let their_prev_commitment_point = read_option!();
+		let their_prev_commitment_point = Readable::read(reader)?;
 		let their_node_id = Readable::read(reader)?;
 
-		let their_shutdown_scriptpubkey = read_option!();
+		let their_shutdown_scriptpubkey = Readable::read(reader)?;
 		let (monitor_last_block, channel_monitor) = ReadableArgs::read(reader, logger.clone())?;
 		// We drop the ChannelMonitor's last block connected hash cause we don't actually bother
 		// doing full block connection operations on the internal CHannelMonitor copies

src/ln/channelmanager.rs

@@ -2612,12 +2612,7 @@ const MIN_SERIALIZATION_VERSION: u8 = 1;
 
 impl Writeable for PendingForwardHTLCInfo {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-		if let &Some(ref onion) = &self.onion_packet {
-			1u8.write(writer)?;
-			onion.write(writer)?;
-		} else {
-			0u8.write(writer)?;
-		}
+		self.onion_packet.write(writer)?;
 		self.incoming_shared_secret.write(writer)?;
 		self.payment_hash.write(writer)?;
 		self.short_channel_id.write(writer)?;
@@ -2629,13 +2624,8 @@ impl Writeable for PendingForwardHTLCInfo {
 
 impl<R: ::std::io::Read> Readable<R> for PendingForwardHTLCInfo {
 	fn read(reader: &mut R) -> Result<PendingForwardHTLCInfo, DecodeError> {
-		let onion_packet = match <u8 as Readable<R>>::read(reader)? {
-			0 => None,
-			1 => Some(msgs::OnionPacket::read(reader)?),
-			_ => return Err(DecodeError::InvalidValue { byte: None }),
-		};
 		Ok(PendingForwardHTLCInfo {
-			onion_packet,
+			onion_packet: Readable::read(reader)?,
 			incoming_shared_secret: Readable::read(reader)?,
 			payment_hash: Readable::read(reader)?,
 			short_channel_id: Readable::read(reader)?,

src/ln/channelmonitor.rs

@@ -1925,33 +1925,17 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
 				let delayed_payment_base_key = Readable::read(reader)?;
 				let payment_base_key = Readable::read(reader)?;
 				let shutdown_pubkey = Readable::read(reader)?;
-				let prev_latest_per_commitment_point = match <u8 as Readable<R>>::read(reader)? {
-					0 => None,
-					1 => Some(Readable::read(reader)?),
-					_ => return Err(DecodeError::InvalidValue { byte: None }),
-				};
-				let latest_per_commitment_point = match <u8 as Readable<R>>::read(reader)? {
-					0 => None,
-					1 => Some(Readable::read(reader)?),
-					_ => return Err(DecodeError::InvalidValue { byte: None }),
-				};
+				let prev_latest_per_commitment_point = Readable::read(reader)?;
+				let latest_per_commitment_point = Readable::read(reader)?;
 				// Technically this can fail and serialize fail a round-trip, but only for serialization of
 				// barely-init'd ChannelMonitors that we can't do anything with.
 				let outpoint = OutPoint {
 					txid: Readable::read(reader)?,
 					index: Readable::read(reader)?,
 				};
 				let funding_info = Some((outpoint, Readable::read(reader)?));
-				let current_remote_commitment_txid = match <u8 as Readable<R>>::read(reader)? {
-					0 => None,
-					1 => Some(Readable::read(reader)?),
-					_ => return Err(DecodeError::InvalidValue { byte: None }),
-				};
-				let prev_remote_commitment_txid = match <u8 as Readable<R>>::read(reader)? {
-					0 => None,
-					1 => Some(Readable::read(reader)?),
-					_ => return Err(DecodeError::InvalidValue { byte: None }),
-				};
+				let current_remote_commitment_txid = Readable::read(reader)?;
+				let prev_remote_commitment_txid = Readable::read(reader)?;
 				Storage::Local {
 					revocation_base_key,
 					htlc_base_key,
@@ -2014,13 +1998,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
 		macro_rules! read_htlc_source {
 			() => {
 				{
-					(Readable::read(reader)?, Readable::read(reader)?,
-						match <u8 as Readable<R>>::read(reader)? {
-							0 => None,
-							1 => Some(Readable::read(reader)?),
-							_ => return Err(DecodeError::InvalidValue {  byte: None }),
-						}
-					)
+					(Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)
 				}
 			}
 		}

src/ln/msgs.rs

@@ -784,8 +784,8 @@ impl From<::std::io::Error> for DecodeError {
 }
 
 impl_writeable_len_match!(AcceptChannel, {
-		{AcceptChannel{ shutdown_scriptpubkey: Some(ref script), ..}, 270 + 2 + script.len()},
-		{_, 270}
+		{AcceptChannel{ shutdown_scriptpubkey: Some(ref script), ..}, 271 + 2 + script.len()},
+		{_, 271}
 	}, {
 	temporary_channel_id,
 	dust_limit_satoshis,
@@ -905,8 +905,8 @@ impl_writeable_len_match!(Init, {
 });
 
 impl_writeable_len_match!(OpenChannel, {
-		{ OpenChannel { shutdown_scriptpubkey: Some(ref script), .. }, 319 + 2 + script.len() },
-		{ OpenChannel { shutdown_scriptpubkey: None, .. }, 319 }
+		{ OpenChannel { shutdown_scriptpubkey: Some(ref script), .. }, 320 + 2 + script.len() },
+		{ OpenChannel { shutdown_scriptpubkey: None, .. }, 320 }
 	}, {
 	chain_hash,
 	temporary_channel_id,

src/ln/peer_handler.rs

@@ -478,8 +478,14 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 													log_debug!(self, "Got a channel/node announcement with an known required feature flag, you may want to udpate!");
 													continue;
 												},
-												msgs::DecodeError::InvalidValue { .. } => return Err(PeerHandleError{ no_connection_possible: false }),
-												msgs::DecodeError::ShortRead => return Err(PeerHandleError{ no_connection_possible: false }),
+												msgs::DecodeError::InvalidValue { .. } => {
+													log_debug!(self, "Got an invalid value while deserializing message");
+													return Err(PeerHandleError{ no_connection_possible: false });
+												},
+												msgs::DecodeError::ShortRead => {
+													log_debug!(self, "Deserialization failed due to shortness of message");
+													return Err(PeerHandleError{ no_connection_possible: false });
+												},
 												msgs::DecodeError::ExtraAddressesPerType => {
 													log_debug!(self, "Error decoding message, ignoring due to lnd spec incompatibility. See https://github.com/lightningnetwork/lnd/issues/1407");
 													continue;

src/ln/router.rs

@@ -126,11 +126,7 @@ impl<R: ::std::io::Read> Readable<R> for DirectionalChannelInfo {
 		let htlc_minimum_msat = Readable::read(reader)?;
 		let fee_base_msat = Readable::read(reader)?;
 		let fee_proportional_millionths = Readable::read(reader)?;
-		let last_update_message = match <u8 as Readable<R>>::read(reader)? {
-			0 => None,
-			1 => Some(msgs::ChannelUpdate::read(reader)?),
-			_ => return Err(DecodeError::InvalidValue { byte: None }),
-		};
+		let last_update_message = Readable::read(reader)?;
 		Ok(DirectionalChannelInfo {
 			src_node_id,
 			last_update,
@@ -182,11 +178,7 @@ impl<R: ::std::io::Read> Readable<R> for ChannelInfo {
 		let features = Readable::read(reader)?;
 		let one_to_two = Readable::read(reader)?;
 		let two_to_one = Readable::read(reader)?;
-		let announcement_message = match <u8 as Readable<R>>::read(reader)? {
-			0 => None,
-			1 => Some(msgs::ChannelAnnouncement::read(reader)?),
-			_ => return Err(DecodeError::InvalidValue { byte: None }),
-		};
+		let announcement_message = Readable::read(reader)?;
 		Ok(ChannelInfo {
 			features,
 			one_to_two,
@@ -268,13 +260,7 @@ impl<R: ::std::io::Read> Readable<R> for NodeInfo {
 		for _ in 0..addresses_count {
 			addresses.push(Readable::read(reader)?);
 		}
-		let announcement_message = match <u8 as Readable<R>>::read(reader)? {
-			0 => None,
-			1 => Some(msgs::NodeAnnouncement::read(reader)?),
-			_ => return Err(DecodeError::InvalidValue { byte: None }),
-
-
-		};
+		let announcement_message = Readable::read(reader)?;
 		Ok(NodeInfo {
 			channels,
 			lowest_inbound_channel_fee_base_msat,

src/util/ser.rs

@@ -306,29 +306,6 @@ impl<R: Read> Readable<R> for Script {
 	}
 }
 
-impl Writeable for Option<Script> {
-	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-		if let &Some(ref script) = self {
-			script.write(w)?;
-		}
-		Ok(())
-	}
-}
-
-impl<R: Read> Readable<R> for Option<Script> {
-	fn read(r: &mut R) -> Result<Self, DecodeError> {
-		match <u16 as Readable<R>>::read(r) {
-			Ok(len) => {
-				let mut buf = vec![0; len as usize];
-				r.read_exact(&mut buf)?;
-				Ok(Some(Script::from(buf)))
-			},
-			Err(DecodeError::ShortRead) => Ok(None),
-			Err(e) => Err(e)
-		}
-	}
-}
-
 impl Writeable for PublicKey {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
 		self.serialize().write(w)
@@ -417,3 +394,34 @@ impl<R: Read> Readable<R> for PaymentHash {
 		Ok(PaymentHash(buf))
 	}
 }
+
+impl<T: Writeable> Writeable for Option<T> {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+		match *self {
+			None => 0u8.write(w)?,
+			Some(ref data) => {
+				1u8.write(w)?;
+				data.write(w)?;
+			}
+		}
+		Ok(())
+	}
+}
+
+impl<R, T> Readable<R> for Option<T>
+	where R: Read,
+	      T: Readable<R>
+{
+	fn read(r: &mut R) -> Result<Self, DecodeError> {
+		match <u8 as Readable<R>>::read(r) {
+			Ok(res) => {
+				match res {
+					0 => Ok(None),
+					1 => Ok(Some(Readable::read(r)?)),
+					_ => return Err(DecodeError::InvalidValue { byte: None }),
+				}
+			},
+			Err(_) => return Ok(None),
+		}
+	}
+}