Split only-receive/forward data out of PendingHTLCInfo into an enum
authorMatt Corallo <git@bluematt.me>
Wed, 1 Jan 2020 22:39:51 +0000 (17:39 -0500)
committerMatt Corallo <git@bluematt.me>
Tue, 14 Apr 2020 23:54:17 +0000 (19:54 -0400)
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

index a471ca3f3675fcab67677381164c7dc69dd8903a..84b595a2d3fcaef5b624b61e3fd27a0513766c75 100644 (file)
@@ -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<u64> 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<msgs::OnionPacket>,
+       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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> 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<W: Writer>(&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<R: ::std::io::Read>(reader: &mut R) -> Result<PendingHTLCInfo, DecodeError> {
                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)?,
                })