Remove write_option in ChannelMonitor serializer

Drop Box usage in remote_claimabe_outpoint

Author: Antoine Riard

src/ln/channel.rs

@@ -3359,7 +3359,7 @@ impl Channel {
 		let (res, remote_commitment_tx, htlcs) = match self.send_commitment_no_state_update() {
 			Ok((res, (remote_commitment_tx, mut htlcs))) => {
 				// Update state now that we've passed all the can-fail calls...
-				let htlcs_no_ref = htlcs.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
+				let htlcs_no_ref = htlcs.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| source_ref.clone()))).collect();
 				(res, remote_commitment_tx, htlcs_no_ref)
 			},
 			Err(e) => return Err(e),

src/ln/channelmonitor.rs

@@ -362,7 +362,7 @@ pub struct ChannelMonitor {
 	their_to_self_delay: Option<u16>,
 
 	old_secrets: [([u8; 32], u64); 49],
-	remote_claimable_outpoints: HashMap<Sha256dHash, Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>,
+	remote_claimable_outpoints: HashMap<Sha256dHash, Vec<(HTLCOutputInCommitment, Option<HTLCSource>)>>,
 	/// We cannot identify HTLC-Success or HTLC-Timeout transactions by themselves on the chain.
 	/// Nor can we figure out their commitment numbers without the commitment transaction they are
 	/// spending. Thus, in order to claim them via revocation key, we track all the remote
@@ -564,7 +564,7 @@ impl ChannelMonitor {
 	/// The monitor watches for it to be broadcasted and then uses the HTLC information (and
 	/// possibly future revocation/preimage information) to claim outputs where possible.
 	/// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers.
-	pub(super) fn provide_latest_remote_commitment_tx_info(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey) {
+	pub(super) fn provide_latest_remote_commitment_tx_info(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<HTLCSource>)>, commitment_number: u64, their_revocation_point: PublicKey) {
 		// TODO: Encrypt the htlc_outputs data with the single-hash of the commitment transaction
 		// so that a remote monitor doesn't learn anything unless there is a malicious close.
 		// (only maybe, sadly we cant do the same for local info, as we need to be aware of
@@ -781,18 +781,6 @@ impl ChannelMonitor {
 		// 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 revocation_base_key, ref htlc_base_key, ref delayed_payment_base_key, ref payment_base_key, ref shutdown_pubkey, ref prev_latest_per_commitment_point, ref latest_per_commitment_point, ref funding_info, ref current_remote_commitment_txid, ref prev_remote_commitment_txid } => {
 				writer.write_all(&[0; 1])?;
@@ -801,18 +789,8 @@ impl ChannelMonitor {
 				writer.write_all(&delayed_payment_base_key[..])?;
 				writer.write_all(&payment_base_key[..])?;
 				writer.write_all(&shutdown_pubkey.serialize())?;
-				if let Some(ref prev_latest_per_commitment_point) = *prev_latest_per_commitment_point {
-					writer.write_all(&[1; 1])?;
-					writer.write_all(&prev_latest_per_commitment_point.serialize())?;
-				} else {
-					writer.write_all(&[0; 1])?;
-				}
-				if let Some(ref latest_per_commitment_point) = *latest_per_commitment_point {
-					writer.write_all(&[1; 1])?;
-					writer.write_all(&latest_per_commitment_point.serialize())?;
-				} else {
-					writer.write_all(&[0; 1])?;
-				}
+				prev_latest_per_commitment_point.write(writer)?;
+				latest_per_commitment_point.write(writer)?;
 				match funding_info  {
 					&Some((ref outpoint, ref script)) => {
 						writer.write_all(&outpoint.txid[..])?;
@@ -823,8 +801,8 @@ impl ChannelMonitor {
 						debug_assert!(false, "Try to serialize a useless Local monitor !");
 					},
 				}
-				write_option!(current_remote_commitment_txid);
-				write_option!(prev_remote_commitment_txid);
+				current_remote_commitment_txid.write(writer)?;
+				prev_remote_commitment_txid.write(writer)?;
 			},
 			Storage::Watchtower { .. } => unimplemented!(),
 		}
@@ -864,7 +842,7 @@ impl ChannelMonitor {
 				writer.write_all(&byte_utils::be64_to_array($htlc_output.amount_msat))?;
 				writer.write_all(&byte_utils::be32_to_array($htlc_output.cltv_expiry))?;
 				writer.write_all(&$htlc_output.payment_hash.0[..])?;
-				write_option!(&$htlc_output.transaction_output_index);
+				$htlc_output.transaction_output_index.write(writer)?;
 			}
 		}
 
@@ -874,7 +852,7 @@ impl ChannelMonitor {
 			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.write(writer)?;
 			}
 		}
 
@@ -923,7 +901,8 @@ impl ChannelMonitor {
 					} else {
 						0u8.write(writer)?;
 					}
-					write_option!(htlc_source);
+					htlc_source.write(writer)?;
+					//write_option!(htlc_source);
 				}
 			}
 		}
@@ -1188,7 +1167,7 @@ impl ChannelMonitor {
 							for &(ref htlc, ref source_option) in outpoints.iter() {
 								if let &Some(ref source) = source_option {
 									log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
-									htlc_updated.push(((**source).clone(), None, htlc.payment_hash.clone()));
+									htlc_updated.push(((*source).clone(), None, htlc.payment_hash.clone()));
 								}
 							}
 						}
@@ -1266,7 +1245,7 @@ impl ChannelMonitor {
 									}
 								}
 								log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
-								htlc_updated.push(((**source).clone(), None, htlc.payment_hash.clone()));
+								htlc_updated.push(((*source).clone(), None, htlc.payment_hash.clone()));
 							}
 						}
 					}
@@ -1903,7 +1882,7 @@ impl ChannelMonitor {
 							if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat {
 								if let &Some(ref source) = pending_source {
 									log_claim!("revoked remote commitment tx", false, pending_htlc, true);
-									payment_data = Some(((**source).clone(), $htlc_output.payment_hash));
+									payment_data = Some(((*source).clone(), $htlc_output.payment_hash));
 									break;
 								}
 							}
@@ -1956,7 +1935,7 @@ impl ChannelMonitor {
 				}
 			}
 			if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(&input.previous_output.txid) {
-				scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()).map(|boxed| &**boxed))),
+				scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()))),
 					"remote commitment tx", false);
 			}
 
@@ -2085,7 +2064,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
 			let htlcs_count: u64 = Readable::read(reader)?;
 			let mut htlcs = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / 32));
 			for _ in 0..htlcs_count {
-				htlcs.push((read_htlc_in_commitment!(), <Option<HTLCSource> as Readable<R>>::read(reader)?.map(|o: HTLCSource| Box::new(o))));
+				htlcs.push((read_htlc_in_commitment!(), <Option<HTLCSource> as Readable<R>>::read(reader)?));
 			}
 			if let Some(_) = remote_claimable_outpoints.insert(txid, htlcs) {
 				return Err(DecodeError::InvalidValue);