From f26e3733965c79dafda751edbceb14291bf6824b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 1 Jan 2020 17:39:51 -0500 Subject: [PATCH] Split only-receive/forward data out of PendingHTLCInfo into an enum This should avoid blowing up the size of the struct when we add additional data that is only relevant for receive. --- lightning/src/ln/channelmanager.rs | 94 +++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 27 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a471ca3f3..84b595a2d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -68,12 +68,20 @@ use std::ops::Deref; // Alternatively, we can fill an outbound HTLC with a HTLCSource::OutboundRoute indicating this is // our payment, which we can use to decode errors or inform the user that the payment was sent. +#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug +enum PendingHTLCRouting { + Forward { + onion_packet: msgs::OnionPacket, + short_channel_id: u64, // This should be NonZero eventually when we bump MSRV + }, + Receive {}, +} + #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug pub(super) struct PendingHTLCInfo { - onion_packet: Option, + routing: PendingHTLCRouting, incoming_shared_secret: [u8; 32], payment_hash: PaymentHash, - short_channel_id: u64, pub(super) amt_to_forward: u64, pub(super) outgoing_cltv_value: u32, } @@ -1005,9 +1013,8 @@ impl ChannelMan // delay) once they've send us a commitment_signed! PendingHTLCStatus::Forward(PendingHTLCInfo { - onion_packet: None, + routing: PendingHTLCRouting::Receive {}, payment_hash: msg.payment_hash.clone(), - short_channel_id: 0, incoming_shared_secret: shared_secret, amt_to_forward: next_hop_data.amt_to_forward, outgoing_cltv_value: next_hop_data.outgoing_cltv_value, @@ -1051,15 +1058,17 @@ impl ChannelMan let short_channel_id = match next_hop_data.format { msgs::OnionHopDataFormat::Legacy { short_channel_id } => short_channel_id, msgs::OnionHopDataFormat::NonFinalNode { short_channel_id } => short_channel_id, - msgs::OnionHopDataFormat::FinalNode => { + msgs::OnionHopDataFormat::FinalNode { .. } => { return_err!("Final Node OnionHopData provided for us as an intermediary node", 0x4000 | 22, &[0;0]); }, }; PendingHTLCStatus::Forward(PendingHTLCInfo { - onion_packet: Some(outgoing_packet), + routing: PendingHTLCRouting::Forward { + onion_packet: outgoing_packet, + short_channel_id: short_channel_id, + }, payment_hash: msg.payment_hash.clone(), - short_channel_id: short_channel_id, incoming_shared_secret: shared_secret, amt_to_forward: next_hop_data.amt_to_forward, outgoing_cltv_value: next_hop_data.outgoing_cltv_value, @@ -1067,8 +1076,11 @@ impl ChannelMan }; channel_state = Some(self.channel_state.lock().unwrap()); - if let &PendingHTLCStatus::Forward(PendingHTLCInfo { ref onion_packet, ref short_channel_id, ref amt_to_forward, ref outgoing_cltv_value, .. }) = &pending_forward_info { - if onion_packet.is_some() { // If short_channel_id is 0 here, we'll reject them in the body here + if let &PendingHTLCStatus::Forward(PendingHTLCInfo { ref routing, ref amt_to_forward, ref outgoing_cltv_value, .. }) = &pending_forward_info { + // If short_channel_id is 0 here, we'll reject the HTLC as there cannot be a channel + // with a short_channel_id of 0. This is important as various things later assume + // short_channel_id is non-0 in any ::Forward. + if let &PendingHTLCRouting::Forward { ref short_channel_id, .. } = routing { let id_option = channel_state.as_ref().unwrap().short_to_id.get(&short_channel_id).cloned(); let forwarding_id = match id_option { None => { // unknown_next_peer @@ -1450,22 +1462,25 @@ impl ChannelMan let mut fail_htlc_msgs = Vec::new(); for forward_info in pending_forwards.drain(..) { match forward_info { - HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info } => { - log_trace!(self, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", log_bytes!(forward_info.payment_hash.0), prev_short_channel_id, short_chan_id); + HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { + routing: PendingHTLCRouting::Forward { + onion_packet, .. + }, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value }, } => { + log_trace!(self, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", log_bytes!(payment_hash.0), prev_short_channel_id, short_chan_id); let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: prev_short_channel_id, htlc_id: prev_htlc_id, - incoming_packet_shared_secret: forward_info.incoming_shared_secret, + incoming_packet_shared_secret: incoming_shared_secret, }); - match chan.get_mut().send_htlc(forward_info.amt_to_forward, forward_info.payment_hash, forward_info.outgoing_cltv_value, htlc_source.clone(), forward_info.onion_packet.unwrap()) { + match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet) { Err(e) => { if let ChannelError::Ignore(msg) = e { - log_trace!(self, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(forward_info.payment_hash.0), msg); + log_trace!(self, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg); } else { panic!("Stated return value requirements in send_htlc() were not met"); } let chan_update = self.get_channel_update(chan.get()).unwrap(); - failed_forwards.push((htlc_source, forward_info.payment_hash, 0x1000 | 7, Some(chan_update))); + failed_forwards.push((htlc_source, payment_hash, 0x1000 | 7, Some(chan_update))); continue; }, Ok(update_add) => { @@ -1484,6 +1499,9 @@ impl ChannelMan } } }, + HTLCForwardInfo::AddHTLC { .. } => { + panic!("short_channel_id != 0 should imply any pending_forward entries are of type Forward"); + }, HTLCForwardInfo::FailHTLC { htlc_id, err_packet } => { log_trace!(self, "Failing HTLC back to channel with short id {} after delay", short_chan_id); match chan.get_mut().get_update_fail_htlc(htlc_id, err_packet) { @@ -1561,21 +1579,26 @@ impl ChannelMan } else { for forward_info in pending_forwards.drain(..) { match forward_info { - HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info } => { + HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { + routing: PendingHTLCRouting::Receive { }, + incoming_shared_secret, payment_hash, amt_to_forward, .. }, } => { let prev_hop_data = HTLCPreviousHopData { short_channel_id: prev_short_channel_id, htlc_id: prev_htlc_id, - incoming_packet_shared_secret: forward_info.incoming_shared_secret, + incoming_packet_shared_secret: incoming_shared_secret, }; - match channel_state.claimable_htlcs.entry(forward_info.payment_hash) { - hash_map::Entry::Occupied(mut entry) => entry.get_mut().push((forward_info.amt_to_forward, prev_hop_data)), - hash_map::Entry::Vacant(entry) => { entry.insert(vec![(forward_info.amt_to_forward, prev_hop_data)]); }, + match channel_state.claimable_htlcs.entry(payment_hash) { + hash_map::Entry::Occupied(mut entry) => entry.get_mut().push((amt_to_forward, prev_hop_data)), + hash_map::Entry::Vacant(entry) => { entry.insert(vec![(amt_to_forward, prev_hop_data)]); }, }; new_events.push(events::Event::PaymentReceived { - payment_hash: forward_info.payment_hash, - amt: forward_info.amt_to_forward, + payment_hash: payment_hash, + amt: amt_to_forward, }); }, + HTLCForwardInfo::AddHTLC { .. } => { + panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive"); + }, HTLCForwardInfo::FailHTLC { .. } => { panic!("Got pending fail of our own HTLC"); } @@ -2388,7 +2411,10 @@ impl ChannelMan forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS)) } for (forward_info, prev_htlc_id) in pending_forwards.drain(..) { - match channel_state.forward_htlcs.entry(forward_info.short_channel_id) { + match channel_state.forward_htlcs.entry(match forward_info.routing { + PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id, + PendingHTLCRouting::Receive { .. } => 0, + }) { hash_map::Entry::Occupied(mut entry) => { entry.get_mut().push(HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info }); }, @@ -3066,10 +3092,18 @@ const MIN_SERIALIZATION_VERSION: u8 = 1; impl Writeable for PendingHTLCInfo { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { - self.onion_packet.write(writer)?; + match &self.routing { + &PendingHTLCRouting::Forward { ref onion_packet, ref short_channel_id } => { + 0u8.write(writer)?; + onion_packet.write(writer)?; + short_channel_id.write(writer)?; + }, + &PendingHTLCRouting::Receive { } => { + 1u8.write(writer)?; + }, + } self.incoming_shared_secret.write(writer)?; self.payment_hash.write(writer)?; - self.short_channel_id.write(writer)?; self.amt_to_forward.write(writer)?; self.outgoing_cltv_value.write(writer)?; Ok(()) @@ -3079,10 +3113,16 @@ impl Writeable for PendingHTLCInfo { impl Readable for PendingHTLCInfo { fn read(reader: &mut R) -> Result { Ok(PendingHTLCInfo { - onion_packet: Readable::read(reader)?, + routing: match Readable::read(reader)? { + 0u8 => PendingHTLCRouting::Forward { + onion_packet: Readable::read(reader)?, + short_channel_id: Readable::read(reader)?, + }, + 1u8 => PendingHTLCRouting::Receive { }, + _ => return Err(DecodeError::InvalidValue), + }, incoming_shared_secret: Readable::read(reader)?, payment_hash: Readable::read(reader)?, - short_channel_id: Readable::read(reader)?, amt_to_forward: Readable::read(reader)?, outgoing_cltv_value: Readable::read(reader)?, }) -- 2.39.5