Change Option<T>::read serialization format

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -3883,18 +3883,6 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
 			}
 		}
 
-		macro_rules! write_option {
-			($thing: expr) => {
-				match &$thing {
-					&None => 0u8.write(writer)?,
-					&Some(ref v) => {
-						1u8.write(writer)?;
-						v.write(writer)?;
-					},
-				}
-			}
-		}
-
 		(self.pending_outbound_htlcs.len() as u64).write(writer)?;
 		for htlc in self.pending_outbound_htlcs.iter() {
 			htlc.htlc_id.write(writer)?;
@@ -3912,15 +3900,15 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
 				},
 				&OutboundHTLCState::RemoteRemoved(ref fail_reason) => {
 					2u8.write(writer)?;
-					write_option!(*fail_reason);
+					fail_reason.write(writer)?;
 				},
 				&OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref fail_reason) => {
 					3u8.write(writer)?;
-					write_option!(*fail_reason);
+					fail_reason.write(writer)?;
 				},
 				&OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref fail_reason) => {
 					4u8.write(writer)?;
-					write_option!(*fail_reason);
+					fail_reason.write(writer)?;
 				},
 			}
 		}
@@ -3971,8 +3959,8 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
 			fail_reason.write(writer)?;
 		}
 
-		write_option!(self.pending_update_fee);
-		write_option!(self.holding_cell_update_fee);
+		self.pending_update_fee.write(writer)?;
+		self.holding_cell_update_fee.write(writer)?;
 
 		self.next_local_htlc_id.write(writer)?;
 		(self.next_remote_htlc_id - dropped_inbound_htlcs).write(writer)?;
@@ -3989,9 +3977,9 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
 			None => 0u8.write(writer)?,
 		}
 
-		write_option!(self.funding_txo);
-		write_option!(self.funding_tx_confirmed_in);
-		write_option!(self.short_channel_id);
+		self.funding_txo.write(writer)?;
+		self.funding_tx_confirmed_in.write(writer)?;
+		self.short_channel_id.write(writer)?;
 
 		self.last_block_connected.write(writer)?;
 		self.funding_tx_confirmations.write(writer)?;
@@ -4007,13 +3995,13 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
 		self.their_max_accepted_htlcs.write(writer)?;
 		self.minimum_depth.write(writer)?;
 
-		write_option!(self.their_pubkeys);
-		write_option!(self.their_cur_commitment_point);
+		self.their_pubkeys.write(writer)?;
+		self.their_cur_commitment_point.write(writer)?;
 
-		write_option!(self.their_prev_commitment_point);
+		self.their_prev_commitment_point.write(writer)?;
 		self.their_node_id.write(writer)?;
 
-		write_option!(self.their_shutdown_scriptpubkey);
+		self.their_shutdown_scriptpubkey.write(writer)?;
 
 		self.commitment_secrets.write(writer)?;
 

lightning/src/ln/channelmonitor.rs

@@ -489,11 +489,7 @@ impl Writeable for InputMaterial  {
 				script.write(writer)?;
 				pubkey.write(writer)?;
 				writer.write_all(&key[..])?;
-				if *is_htlc {
-					writer.write_all(&[0; 1])?;
-				} else {
-					writer.write_all(&[1; 1])?;
-				}
+				is_htlc.write(writer)?;
 				writer.write_all(&byte_utils::be64_to_array(*amount))?;
 			},
 			&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
@@ -524,11 +520,7 @@ impl Readable for InputMaterial {
 				let script = Readable::read(reader)?;
 				let pubkey = Readable::read(reader)?;
 				let key = Readable::read(reader)?;
-				let is_htlc = match <u8 as Readable>::read(reader)? {
-					0 => true,
-					1 => false,
-					_ => return Err(DecodeError::InvalidValue),
-				};
+				let is_htlc = Readable::read(reader)?;
 				let amount = Readable::read(reader)?;
 				InputMaterial::Revoked {
 					script,
@@ -698,13 +690,7 @@ impl Writeable for ChannelMonitorUpdateStep {
 				(htlc_outputs.len() as u64).write(w)?;
 				for &(ref output, ref source) in htlc_outputs.iter() {
 					output.write(w)?;
-					match source {
-						&None => 0u8.write(w)?,
-						&Some(ref s) => {
-							1u8.write(w)?;
-							s.write(w)?;
-						},
-					}
+					source.as_ref().map(|b| b.as_ref()).write(w)?;
 				}
 			},
 			&ChannelMonitorUpdateStep::PaymentPreimage { ref payment_preimage } => {
@@ -973,18 +959,6 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 		// Set in initial Channel-object creation, so should always be set by now:
 		U48(self.commitment_transaction_number_obscure_factor).write(writer)?;
 
-		macro_rules! write_option {
-			($thing: expr) => {
-				match $thing {
-					&Some(ref t) => {
-						1u8.write(writer)?;
-						t.write(writer)?;
-					},
-					&None => 0u8.write(writer)?,
-				}
-			}
-		}
-
 		match self.key_storage {
 			Storage::Local { ref keys, ref funding_key, ref revocation_base_key, ref htlc_base_key, ref delayed_payment_base_key, ref payment_base_key, ref shutdown_pubkey, ref funding_info, ref current_remote_commitment_txid, ref prev_remote_commitment_txid } => {
 				writer.write_all(&[0; 1])?;
@@ -1055,7 +1029,7 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 			writer.write_all(&byte_utils::be64_to_array(htlc_infos.len() as u64))?;
 			for &(ref htlc_output, ref htlc_source) in htlc_infos.iter() {
 				serialize_htlc_in_commitment!(htlc_output);
-				write_option!(htlc_source);
+				htlc_source.as_ref().map(|b| b.as_ref()).write(writer)?;
 			}
 		}
 
@@ -1098,7 +1072,7 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 					} else {
 						0u8.write(writer)?;
 					}
-					write_option!(htlc_source);
+					htlc_source.write(writer)?;
 				}
 			}
 		}

lightning/src/util/ser.rs

@@ -172,6 +172,10 @@ pub trait Writeable {
 	}
 }
 
+impl<T: Writeable> Writeable for &T {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> { (*self).write(writer) }
+}
+
 /// A trait that various rust-lightning types implement allowing them to be read in from a Read
 pub trait Readable
 	where Self: Sized
@@ -592,7 +596,9 @@ impl<T: Writeable> Writeable for Option<T> {
 		match *self {
 			None => 0u8.write(w)?,
 			Some(ref data) => {
-				1u8.write(w)?;
+				let mut len_calc = LengthCalculatingWriter(0);
+				data.write(&mut len_calc).expect("No in-memory data may fail to serialize");
+				BigSize(len_calc.0 as u64 + 1).write(w)?;
 				data.write(w)?;
 			}
 		}
@@ -603,10 +609,12 @@ impl<T: Writeable> Writeable for Option<T> {
 impl<T: Readable> Readable for Option<T>
 {
 	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-		match <u8 as Readable>::read(r)? {
+		match BigSize::read(r)?.0 {
 			0 => Ok(None),
-			1 => Ok(Some(Readable::read(r)?)),
-			_ => return Err(DecodeError::InvalidValue),
+			len => {
+				let mut reader = FixedLengthReader::new(r, len - 1);
+				Ok(Some(Readable::read(&mut reader)?))
+			}
 		}
 	}
 }