From 72e32e7af617636aa8ffa02df60aa4a2fbf9babf Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 1 Jan 2020 15:56:03 -0500 Subject: [PATCH] Clarify the in-flight HTLC state-tracking structs a bit. This also renames PendingForwardHTLCInfo to PendingHTLCInfo since it now also encompasses Pending *Received* HTLCs. --- lightning/src/ln/channel.rs | 10 ++--- lightning/src/ln/channelmanager.rs | 64 ++++++++++++++++-------------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ff2a28b1f..d98059332 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -19,7 +19,7 @@ use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; use ln::channelmonitor::ChannelMonitor; -use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingForwardHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT}; +use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use ln::chan_utils::{LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys}; use ln::chan_utils; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; @@ -269,7 +269,7 @@ pub(super) struct Channel { monitor_pending_funding_locked: bool, monitor_pending_revoke_and_ack: bool, monitor_pending_commitment_signed: bool, - monitor_pending_forwards: Vec<(PendingForwardHTLCInfo, u64)>, + monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>, monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, // pending_update_fee is filled when sending and receiving update_fee @@ -1986,7 +1986,7 @@ impl Channel { /// waiting on this revoke_and_ack. The generation of this new commitment_signed may also fail, /// generating an appropriate error *after* the channel state has been updated based on the /// revoke_and_ack message. - pub fn revoke_and_ack(&mut self, msg: &msgs::RevokeAndACK, fee_estimator: &FeeEstimator) -> Result<(Option, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option, ChannelMonitor), ChannelError> { + pub fn revoke_and_ack(&mut self, msg: &msgs::RevokeAndACK, fee_estimator: &FeeEstimator) -> Result<(Option, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option, ChannelMonitor), ChannelError> { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { return Err(ChannelError::Close("Got revoke/ACK message when channel was not in an operational state")); } @@ -2292,7 +2292,7 @@ impl Channel { /// which failed. The messages which were generated from that call which generated the /// monitor update failure must *not* have been sent to the remote end, and must instead /// have been dropped. They will be regenerated when monitor_updating_restored is called. - pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, mut pending_forwards: Vec<(PendingForwardHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) { + pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) { assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, 0); self.monitor_pending_revoke_and_ack = resend_raa; self.monitor_pending_commitment_signed = resend_commitment; @@ -2306,7 +2306,7 @@ impl Channel { /// Indicates that the latest ChannelMonitor update has been committed by the client /// successfully and we should restore normal operation. Returns messages which should be sent /// to the remote side. - pub fn monitor_updating_restored(&mut self) -> (Option, Option, RAACommitmentOrder, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option) { + pub fn monitor_updating_restored(&mut self) -> (Option, Option, RAACommitmentOrder, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option) { assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32); self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2a7c0949d..9c4b7c80f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -57,15 +57,19 @@ use std::ops::Deref; // forward the HTLC with information it will give back to us when it does so, or if it should Fail // the HTLC with the relevant message for the Channel to handle giving to the remote peer. // -// When a Channel forwards an HTLC to its peer, it will give us back the PendingForwardHTLCInfo -// which we will use to construct an outbound HTLC, with a relevant HTLCSource::PreviousHopData -// filled in to indicate where it came from (which we can use to either fail-backwards or fulfill -// the HTLC backwards along the relevant path). +// Once said HTLC is committed in the Channel, if the PendingHTLCStatus indicated Forward, the +// Channel will return the PendingHTLCInfo back to us, and we will create an HTLCForwardInfo +// with it to track where it came from (in case of onwards-forward error), waiting a random delay +// before we forward it. +// +// We will then use HTLCForwardInfo's PendingHTLCInfo to construct an outbound HTLC, with a +// relevant HTLCSource::PreviousHopData filled in to indicate where it came from (which we can use +// to either fail-backwards or fulfill the HTLC backwards along the relevant path). // 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. -/// Stores the info we will need to send when we want to forward an HTLC onwards + #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug -pub(super) struct PendingForwardHTLCInfo { +pub(super) struct PendingHTLCInfo { onion_packet: Option, incoming_shared_secret: [u8; 32], payment_hash: PaymentHash, @@ -83,10 +87,22 @@ pub(super) enum HTLCFailureMsg { /// Stores whether we can't forward an HTLC or relevant forwarding info #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug pub(super) enum PendingHTLCStatus { - Forward(PendingForwardHTLCInfo), + Forward(PendingHTLCInfo), Fail(HTLCFailureMsg), } +pub(super) enum HTLCForwardInfo { + AddHTLC { + prev_short_channel_id: u64, + prev_htlc_id: u64, + forward_info: PendingHTLCInfo, + }, + FailHTLC { + htlc_id: u64, + err_packet: msgs::OnionErrorPacket, + }, +} + /// Tracks the inbound corresponding to an outbound HTLC #[derive(Clone, PartialEq)] pub(super) struct HTLCPreviousHopData { @@ -231,18 +247,6 @@ impl MsgHandleErrInternal { /// second to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly. const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100; -pub(super) enum HTLCForwardInfo { - AddHTLC { - prev_short_channel_id: u64, - prev_htlc_id: u64, - forward_info: PendingForwardHTLCInfo, - }, - FailHTLC { - htlc_id: u64, - err_packet: msgs::OnionErrorPacket, - }, -} - /// For events which result in both a RevokeAndACK and a CommitmentUpdate, by default they should /// be sent in the order they appear in the return value, however sometimes the order needs to be /// variable at runtime (eg Channel::channel_reestablish needs to re-send messages in the order @@ -262,7 +266,7 @@ pub(super) struct ChannelHolder { /// short channel id -> forward infos. Key of 0 means payments received /// Note that while this is held in the same mutex as the channels themselves, no consistency /// guarantees are made about the existence of a channel with the short id here, nor the short - /// ids in the PendingForwardHTLCInfo! + /// 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 @@ -574,7 +578,7 @@ macro_rules! handle_monitor_err { } else if $resend_commitment { "commitment" } else if $resend_raa { "RAA" } else { "nothing" }, - (&$failed_forwards as &Vec<(PendingForwardHTLCInfo, u64)>).len(), + (&$failed_forwards as &Vec<(PendingHTLCInfo, u64)>).len(), (&$failed_fails as &Vec<(HTLCSource, PaymentHash, HTLCFailReason)>).len()); if !$resend_commitment { debug_assert!($action_type == RAACommitmentOrder::RevokeAndACKFirst || !$resend_raa); @@ -969,7 +973,7 @@ impl ChannelManager ChannelManager ChannelManager ChannelManager ChannelManager)]) { + fn forward_htlcs(&self, per_source_pending_forwards: &mut [(u64, Vec<(PendingHTLCInfo, u64)>)]) { for &mut (prev_short_channel_id, ref mut pending_forwards) in per_source_pending_forwards { let mut forward_event = None; if !pending_forwards.is_empty() { @@ -3016,7 +3020,7 @@ impl Ch const SERIALIZATION_VERSION: u8 = 1; const MIN_SERIALIZATION_VERSION: u8 = 1; -impl Writeable for PendingForwardHTLCInfo { +impl Writeable for PendingHTLCInfo { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { self.onion_packet.write(writer)?; self.incoming_shared_secret.write(writer)?; @@ -3028,9 +3032,9 @@ impl Writeable for PendingForwardHTLCInfo { } } -impl Readable for PendingForwardHTLCInfo { - fn read(reader: &mut R) -> Result { - Ok(PendingForwardHTLCInfo { +impl Readable for PendingHTLCInfo { + fn read(reader: &mut R) -> Result { + Ok(PendingHTLCInfo { onion_packet: Readable::read(reader)?, incoming_shared_secret: Readable::read(reader)?, payment_hash: Readable::read(reader)?, -- 2.39.5