From b935b37ee5ce40d6f77c201e90b0b9a8dc353551 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 1 Jan 2020 20:20:42 -0500 Subject: [PATCH] Support (de)serializing payment_data in onion TLVs and track them This is the first step in Base AMP support, just tracking the relevant data in internal datastructures. --- lightning/src/ln/channelmanager.rs | 70 ++++++++++++++++++++---------- lightning/src/ln/msgs.rs | 47 +++++++++++++++----- lightning/src/ln/onion_utils.rs | 4 +- 3 files changed, 86 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6b637bec3..4f2fbad19 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -74,7 +74,9 @@ enum PendingForwardReceiveHTLCInfo { onion_packet: msgs::OnionPacket, short_channel_id: u64, // This should be NonZero eventually }, - Receive {}, + Receive { + payment_data: Option, + }, } #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug @@ -119,6 +121,12 @@ pub(super) struct HTLCPreviousHopData { incoming_packet_shared_secret: [u8; 32], } +struct ClaimableHTLC { + src: HTLCPreviousHopData, + value: u64, + payment_data: Option, +} + /// Tracks the inbound corresponding to an outbound HTLC #[derive(Clone, PartialEq)] pub(super) enum HTLCSource { @@ -276,12 +284,11 @@ pub(super) struct ChannelHolder { /// guarantees are made about the existence of a channel with the short id here, nor the short /// ids in the PendingHTLCInfo! pub(super) forward_htlcs: HashMap>, - /// payment_hash -> Vec<(amount_received, htlc_source)> for tracking things that were to us and - /// can be failed/claimed by the user + /// Tracks things that were to us and can be failed/claimed by the user /// Note that while this is held in the same mutex as the channels themselves, no consistency /// guarantees are made about the channels given here actually existing anymore by the time you /// go to read them! - pub(super) claimable_htlcs: HashMap>, + claimable_htlcs: HashMap>, /// Messages to send to peers - pushed to in the same lock that they are generated in (except /// for broadcast messages, where ordering isn't as strict). pub(super) pending_msg_events: Vec, @@ -974,13 +981,19 @@ impl ChannelManager where M::T return_err!("Upstream node set CLTV to the wrong value", 18, &byte_utils::be32_to_array(msg.cltv_expiry)); } + let payment_data = match next_hop_data.format { + msgs::OnionHopDataFormat::Legacy { .. } => None, + msgs::OnionHopDataFormat::NonFinalNode { .. } => return_err!("Got non final data with an HMAC of 0", 0x4000 | 22, &[0;0]), + msgs::OnionHopDataFormat::FinalNode { payment_data } => payment_data, + }; + // Note that we could obviously respond immediately with an update_fulfill_htlc // message, however that would leak that we are the recipient of this payment, so // instead we stay symmetric with the forwarding case, only responding (after a // delay) once they've send us a commitment_signed! PendingHTLCStatus::Forward(PendingHTLCInfo { - type_data: PendingForwardReceiveHTLCInfo::Receive {}, + type_data: PendingForwardReceiveHTLCInfo::Receive { payment_data }, payment_hash: msg.payment_hash.clone(), incoming_shared_secret: shared_secret, amt_to_forward: next_hop_data.amt_to_forward, @@ -1531,17 +1544,18 @@ impl ChannelManager where M::T for forward_info in pending_forwards.drain(..) { match forward_info { HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { - type_data: PendingForwardReceiveHTLCInfo::Receive { }, + type_data: PendingForwardReceiveHTLCInfo::Receive { payment_data }, 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: incoming_shared_secret, }; - 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)]); }, - }; + channel_state.claimable_htlcs.entry(payment_hash).or_insert(Vec::new()).push(ClaimableHTLC { + src: prev_hop_data, + value: amt_to_forward, + payment_data, + }); new_events.push(events::Event::PaymentReceived { payment_hash: payment_hash, amt: amt_to_forward, @@ -1614,11 +1628,11 @@ impl ChannelManager where M::T let mut channel_state = Some(self.channel_state.lock().unwrap()); let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(payment_hash); if let Some(mut sources) = removed_source { - for (recvd_value, htlc_with_hash) in sources.drain(..) { + for htlc in sources.drain(..) { if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); } self.fail_htlc_backwards_internal(channel_state.take().unwrap(), - HTLCSource::PreviousHopData(htlc_with_hash), payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(recvd_value).to_vec() }); + HTLCSource::PreviousHopData(htlc.src), payment_hash, + HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(htlc.value).to_vec() }); } true } else { false } @@ -1742,17 +1756,17 @@ impl ChannelManager where M::T let mut channel_state = Some(self.channel_state.lock().unwrap()); let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&payment_hash); if let Some(mut sources) = removed_source { - for (received_amount, htlc_with_hash) in sources.drain(..) { + for htlc in sources.drain(..) { if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); } - if received_amount < expected_amount || received_amount > expected_amount * 2 { - let mut htlc_msat_data = byte_utils::be64_to_array(received_amount).to_vec(); + if htlc.value < expected_amount || htlc.value > expected_amount * 2 { + let mut htlc_msat_data = byte_utils::be64_to_array(htlc.value).to_vec(); let mut height_data = byte_utils::be32_to_array(self.latest_block_height.load(Ordering::Acquire) as u32).to_vec(); htlc_msat_data.append(&mut height_data); self.fail_htlc_backwards_internal(channel_state.take().unwrap(), - HTLCSource::PreviousHopData(htlc_with_hash), &payment_hash, + HTLCSource::PreviousHopData(htlc.src), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_data }); } else { - self.claim_funds_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc_with_hash), payment_preimage); + self.claim_funds_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc.src), payment_preimage); } } true @@ -3063,8 +3077,9 @@ impl Writeable for PendingHTLCInfo { onion_packet.write(writer)?; short_channel_id.write(writer)?; }, - &PendingForwardReceiveHTLCInfo::Receive { } => { + &PendingForwardReceiveHTLCInfo::Receive { ref payment_data } => { 1u8.write(writer)?; + payment_data.write(writer)?; }, } self.incoming_shared_secret.write(writer)?; @@ -3083,7 +3098,9 @@ impl Readable for PendingHTLCInfo { onion_packet: Readable::read(reader)?, short_channel_id: Readable::read(reader)?, }, - 1u8 => PendingForwardReceiveHTLCInfo::Receive { }, + 1u8 => PendingForwardReceiveHTLCInfo::Receive { + payment_data: Readable::read(reader)?, + }, _ => return Err(DecodeError::InvalidValue), }, incoming_shared_secret: Readable::read(reader)?, @@ -3288,9 +3305,10 @@ impl Writeable for ChannelManager for (payment_hash, previous_hops) in channel_state.claimable_htlcs.iter() { payment_hash.write(writer)?; (previous_hops.len() as u64).write(writer)?; - for &(recvd_amt, ref previous_hop) in previous_hops.iter() { - recvd_amt.write(writer)?; - previous_hop.write(writer)?; + for htlc in previous_hops.iter() { + htlc.src.write(writer)?; + htlc.value.write(writer)?; + htlc.payment_data.write(writer)?; } } @@ -3432,7 +3450,11 @@ impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable, M: Deref> R let previous_hops_len: u64 = Readable::read(reader)?; let mut previous_hops = Vec::with_capacity(cmp::min(previous_hops_len as usize, 2)); for _ in 0..previous_hops_len { - previous_hops.push((Readable::read(reader)?, Readable::read(reader)?)); + previous_hops.push(ClaimableHTLC { + src: Readable::read(reader)?, + value: Readable::read(reader)?, + payment_data: Readable::read(reader)?, + }); } claimable_htlcs.insert(payment_hash, previous_hops); } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 03a291780..b20e5d5bc 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -661,6 +661,11 @@ pub trait RoutingMessageHandler : Send + Sync { mod fuzzy_internal_msgs { // These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize // them from untrusted input): + #[derive(Clone)] + pub(crate) struct FinalOnionHopData { + pub(crate) payment_secret: [u8; 32], + pub(crate) total_msat: u64, + } pub(crate) enum OnionHopDataFormat { Legacy { // aka Realm-0 @@ -669,7 +674,9 @@ mod fuzzy_internal_msgs { NonFinalNode { short_channel_id: u64, }, - FinalNode, + FinalNode { + payment_data: Option, + }, } pub struct OnionHopData { @@ -1013,6 +1020,11 @@ impl_writeable!(UpdateAddHTLC, 32+8+8+32+4+1366, { onion_routing_packet }); +impl_writeable!(FinalOnionHopData, 32+8, { + payment_secret, + total_msat +}); + impl Writeable for OnionHopData { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { w.size_hint(33); @@ -1030,11 +1042,19 @@ impl Writeable for OnionHopData { (6, short_channel_id) }); }, - OnionHopDataFormat::FinalNode => { - encode_varint_length_prefixed_tlv!(w, { - (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)), - (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)) - }); + OnionHopDataFormat::FinalNode { ref payment_data } => { + if let &Some(ref final_data) = payment_data { + encode_varint_length_prefixed_tlv!(w, { + (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)), + (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)), + (8, final_data) + }); + } else { + encode_varint_length_prefixed_tlv!(w, { + (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)), + (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)) + }); + } }, } match self.format { @@ -1060,19 +1080,24 @@ impl Readable for OnionHopData { let mut amt = HighZeroBytesDroppedVarInt(0u64); let mut cltv_value = HighZeroBytesDroppedVarInt(0u32); let mut short_id: Option = None; + let mut payment_data: Option = None; decode_tlv!(&mut rd, { (2, amt), (4, cltv_value) }, { - (6, short_id) + (6, short_id), + (8, payment_data) }); rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?; let format = if let Some(short_channel_id) = short_id { + if payment_data.is_some() { return Err(DecodeError::InvalidValue); } OnionHopDataFormat::NonFinalNode { short_channel_id, } } else { - OnionHopDataFormat::FinalNode + OnionHopDataFormat::FinalNode { + payment_data + } }; (format, amt.0, cltv_value.0) } else { @@ -2067,7 +2092,9 @@ mod tests { #[test] fn encoding_final_onion_hop_data() { let mut msg = msgs::OnionHopData { - format: OnionHopDataFormat::FinalNode, + format: OnionHopDataFormat::FinalNode { + payment_data: None, + }, amt_to_forward: 0x0badf00d01020304, outgoing_cltv_value: 0xffffffff, }; @@ -2075,7 +2102,7 @@ mod tests { let target_value = hex::decode("1002080badf00d010203040404ffffffff").unwrap(); assert_eq!(encoded_value, target_value); msg = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); - if let OnionHopDataFormat::FinalNode = msg.format { } else { panic!(); } + if let OnionHopDataFormat::FinalNode { payment_data: None } = msg.format { } else { panic!(); } assert_eq!(msg.amt_to_forward, 0x0badf00d01020304); assert_eq!(msg.outgoing_cltv_value, 0xffffffff); } diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index c2857ad66..92da78333 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -123,7 +123,9 @@ pub(super) fn build_onion_payloads(route: &Route, starting_htlc_offset: u32) -> res.insert(0, msgs::OnionHopData { format: if hop.node_features.supports_variable_length_onion() { if idx == 0 { - msgs::OnionHopDataFormat::FinalNode + msgs::OnionHopDataFormat::FinalNode { + payment_data: None, + } } else { msgs::OnionHopDataFormat::NonFinalNode { short_channel_id: last_short_channel_id, -- 2.39.5