Merge pull request #167 from TheBlueMatt/2018-09-dup-htlc
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 12 Sep 2018 17:07:13 +0000 (13:07 -0400)
committerGitHub <noreply@github.com>
Wed, 12 Sep 2018 17:07:13 +0000 (13:07 -0400)
 Allow duplicate-payment_hash HTLCs for HTLC forwards

fuzz/fuzz_targets/channel_target.rs
src/ln/channel.rs
src/ln/channelmanager.rs

index 7eed4fb55d3fa4bf40bc124d78fbb7271b89dd05..d33ac94192a5efeaecc3829dd26c6ab9d7e94751 100644 (file)
@@ -8,7 +8,7 @@ use bitcoin::util::hash::Sha256dHash;
 use bitcoin::network::serialize::{serialize, BitcoinHash};
 
 use lightning::ln::channel::{Channel, ChannelKeys};
-use lightning::ln::channelmanager::{HTLCFailReason, PendingHTLCStatus};
+use lightning::ln::channelmanager::{HTLCFailReason, HTLCSource, PendingHTLCStatus};
 use lightning::ln::msgs;
 use lightning::ln::msgs::{ErrorAction};
 use lightning::chain::chaininterface::{FeeEstimator, ConfirmationTarget};
@@ -261,7 +261,7 @@ pub fn do_test(data: &[u8]) {
        loop {
                match get_slice!(1)[0] {
                        0 => {
-                               test_err!(channel.send_htlc(slice_to_be64(get_slice!(8)), [42; 32], slice_to_be32(get_slice!(4)), msgs::OnionPacket {
+                               test_err!(channel.send_htlc(slice_to_be64(get_slice!(8)), [42; 32], slice_to_be32(get_slice!(4)), HTLCSource::dummy(), msgs::OnionPacket {
                                        version: get_slice!(1)[0],
                                        public_key: PublicKey::from_slice(&secp_ctx, get_slice!(33)),
                                        hop_data: [0; 20*65],
index d9633ea836284277c8c54fd6b7ab34dcea25850c..0ed2dde04ec9f216141663111ffebf979b743459 100644 (file)
@@ -16,7 +16,7 @@ use crypto::hkdf::{hkdf_extract,hkdf_expand};
 use ln::msgs;
 use ln::msgs::{ErrorAction, HandleError, MsgEncodable};
 use ln::channelmonitor::ChannelMonitor;
-use ln::channelmanager::{PendingHTLCStatus, PendingForwardHTLCInfo, HTLCFailReason, HTLCFailureMsg};
+use ln::channelmanager::{PendingHTLCStatus, HTLCSource, PendingForwardHTLCInfo, HTLCFailReason, HTLCFailureMsg};
 use ln::chan_utils::{TxCreationKeys,HTLCOutputInCommitment,HTLC_SUCCESS_TX_WEIGHT,HTLC_TIMEOUT_TX_WEIGHT};
 use ln::chan_utils;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
@@ -87,24 +87,49 @@ impl ChannelKeys {
 }
 
 #[derive(PartialEq)]
-enum HTLCState {
+enum InboundHTLCState {
        /// Added by remote, to be included in next local commitment tx.
-       /// Implies HTLCOutput::outbound: false
        RemoteAnnounced,
        /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but
        /// the remote side hasn't yet revoked their previous state, which we need them to do before we
        /// accept this HTLC. Implies AwaitingRemoteRevoke.
        /// We also have not yet included this HTLC in a commitment_signed message, and are waiting on
        /// a remote revoke_and_ack on a previous state before we can do so.
-       /// Implies HTLCOutput::outbound: false
        AwaitingRemoteRevokeToAnnounce,
        /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but
        /// the remote side hasn't yet revoked their previous state, which we need them to do before we
        /// accept this HTLC. Implies AwaitingRemoteRevoke.
        /// We have included this HTLC in our latest commitment_signed and are now just waiting on a
        /// revoke_and_ack.
-       /// Implies HTLCOutput::outbound: true
        AwaitingAnnouncedRemoteRevoke,
+       Committed,
+       /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we
+       /// created it we would have put it in the holding cell instead). When they next revoke_and_ack
+       /// we'll drop it.
+       /// Note that we have to keep an eye on the HTLC until we've received a broadcastable
+       /// commitment transaction without it as otherwise we'll have to force-close the channel to
+       /// claim it before the timeout (obviously doesn't apply to revoked HTLCs that we can't claim
+       /// anyway). That said, ChannelMonitor does this for us (see
+       /// ChannelMonitor::would_broadcast_at_height) so we actually remove the HTLC from our own
+       /// local state before then, once we're sure that the next commitment_signed and
+       /// ChannelMonitor::provide_latest_local_commitment_tx_info will not include this HTLC.
+       LocalRemoved,
+}
+
+struct InboundHTLCOutput {
+       htlc_id: u64,
+       amount_msat: u64,
+       cltv_expiry: u32,
+       payment_hash: [u8; 32],
+       state: InboundHTLCState,
+       /// If we're in LocalRemoved, set to true if we fulfilled the HTLC, and can claim money
+       local_removed_fulfilled: bool,
+       /// state pre-Committed implies pending_forward_state, otherwise it must be None
+       pending_forward_state: Option<PendingHTLCStatus>,
+}
+
+#[derive(PartialEq)]
+enum OutboundHTLCState {
        /// Added by us and included in a commitment_signed (if we were AwaitingRemoteRevoke when we
        /// created it we would have put it in the holding cell instead). When they next revoke_and_ack
        /// we will promote to Committed (note that they may not accept it until the next time we
@@ -115,63 +140,43 @@ enum HTLCState {
        ///    allowed to remove it, the "can only be removed once committed on both sides" requirement
        ///    doesn't matter to us and its up to them to enforce it, worst-case they jump ahead but
        ///    we'll never get out of sync).
-       /// Implies HTLCOutput::outbound: true
        LocalAnnounced,
        Committed,
        /// Remote removed this (outbound) HTLC. We're waiting on their commitment_signed to finalize
        /// the change (though they'll need to revoke before we fail the payment).
-       /// Implies HTLCOutput::outbound: true
        RemoteRemoved,
        /// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but
        /// the remote side hasn't yet revoked their previous state, which we need them to do before we
        /// can do any backwards failing. Implies AwaitingRemoteRevoke.
        /// We also have not yet removed this HTLC in a commitment_signed message, and are waiting on a
        /// remote revoke_and_ack on a previous state before we can do so.
-       /// Implies HTLCOutput::outbound: true
        AwaitingRemoteRevokeToRemove,
        /// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but
        /// the remote side hasn't yet revoked their previous state, which we need them to do before we
        /// can do any backwards failing. Implies AwaitingRemoteRevoke.
        /// We have removed this HTLC in our latest commitment_signed and are now just waiting on a
        /// revoke_and_ack to drop completely.
-       /// Implies HTLCOutput::outbound: true
        AwaitingRemovedRemoteRevoke,
-       /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we
-       /// created it we would have put it in the holding cell instead). When they next revoke_and_ack
-       /// we'll drop it.
-       /// Note that we have to keep an eye on the HTLC until we've received a broadcastable
-       /// commitment transaction without it as otherwise we'll have to force-close the channel to
-       /// claim it before the timeout (obviously doesn't apply to revoked HTLCs that we can't claim
-       /// anyway). That said, ChannelMonitor does this for us (see
-       /// ChannelMonitor::would_broadcast_at_height) so we actually remove the HTLC from our own
-       /// local state before then, once we're sure that the next commitment_signed and
-       /// ChannelMonitor::provide_latest_local_commitment_tx_info will not include this HTLC.
-       /// Implies HTLCOutput::outbound: false
-       LocalRemoved,
 }
 
-struct HTLCOutput { //TODO: Refactor into Outbound/InboundHTLCOutput (will save memory and fewer panics)
-       outbound: bool, // ie to an HTLC-Timeout transaction
+struct OutboundHTLCOutput {
        htlc_id: u64,
        amount_msat: u64,
        cltv_expiry: u32,
        payment_hash: [u8; 32],
-       state: HTLCState,
-       /// If we're in a Remote* removed state, set if they failed, otherwise None
+       state: OutboundHTLCState,
+       source: HTLCSource,
+       /// If we're in a removed state, set if they failed, otherwise None
        fail_reason: Option<HTLCFailReason>,
-       /// If we're in LocalRemoved*, set to true if we fulfilled the HTLC, and can claim money
-       local_removed_fulfilled: bool,
-       /// state pre-committed Remote* implies pending_forward_state, otherwise it must be None
-       pending_forward_state: Option<PendingHTLCStatus>,
 }
 
-impl HTLCOutput {
-       fn get_in_commitment(&self, offered: bool) -> HTLCOutputInCommitment {
+macro_rules! get_htlc_in_commitment {
+       ($htlc: expr, $offered: expr) => {
                HTLCOutputInCommitment {
-                       offered: offered,
-                       amount_msat: self.amount_msat,
-                       cltv_expiry: self.cltv_expiry,
-                       payment_hash: self.payment_hash,
+                       offered: $offered,
+                       amount_msat: $htlc.amount_msat,
+                       cltv_expiry: $htlc.cltv_expiry,
+                       payment_hash: $htlc.payment_hash,
                        transaction_output_index: 0
                }
        }
@@ -184,15 +189,16 @@ enum HTLCUpdateAwaitingACK {
                amount_msat: u64,
                cltv_expiry: u32,
                payment_hash: [u8; 32],
+               source: HTLCSource,
                onion_routing_packet: msgs::OnionPacket,
                time_created: Instant, //TODO: Some kind of timeout thing-a-majig
        },
        ClaimHTLC {
                payment_preimage: [u8; 32],
-               payment_hash: [u8; 32], // Only here for effecient duplicate detection
+               htlc_id: u64,
        },
        FailHTLC {
-               payment_hash: [u8; 32],
+               htlc_id: u64,
                err_packet: msgs::OnionErrorPacket,
        },
 }
@@ -262,7 +268,8 @@ pub struct Channel {
        cur_local_commitment_transaction_number: u64,
        cur_remote_commitment_transaction_number: u64,
        value_to_self_msat: u64, // Excluding all pending_htlcs, excluding fees
-       pending_htlcs: Vec<HTLCOutput>,
+       pending_inbound_htlcs: Vec<InboundHTLCOutput>,
+       pending_outbound_htlcs: Vec<OutboundHTLCOutput>,
        holding_cell_htlc_updates: Vec<HTLCUpdateAwaitingACK>,
        next_local_htlc_id: u64,
        next_remote_htlc_id: u64,
@@ -421,7 +428,8 @@ impl Channel {
                        cur_local_commitment_transaction_number: (1 << 48) - 1,
                        cur_remote_commitment_transaction_number: (1 << 48) - 1,
                        value_to_self_msat: channel_value_satoshis * 1000 - push_msat,
-                       pending_htlcs: Vec::new(),
+                       pending_inbound_htlcs: Vec::new(),
+                       pending_outbound_htlcs: Vec::new(),
                        holding_cell_htlc_updates: Vec::new(),
                        next_local_htlc_id: 0,
                        next_remote_htlc_id: 0,
@@ -578,7 +586,8 @@ impl Channel {
                        cur_local_commitment_transaction_number: (1 << 48) - 1,
                        cur_remote_commitment_transaction_number: (1 << 48) - 1,
                        value_to_self_msat: msg.push_msat,
-                       pending_htlcs: Vec::new(),
+                       pending_inbound_htlcs: Vec::new(),
+                       pending_outbound_htlcs: Vec::new(),
                        holding_cell_htlc_updates: Vec::new(),
                        next_local_htlc_id: 0,
                        next_remote_htlc_id: 0,
@@ -686,66 +695,83 @@ impl Channel {
                        ins
                };
 
-               let mut txouts: Vec<(TxOut, Option<HTLCOutputInCommitment>)> = Vec::with_capacity(self.pending_htlcs.len() + 2);
+               let mut txouts: Vec<(TxOut, Option<HTLCOutputInCommitment>)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
 
                let dust_limit_satoshis = if local { self.our_dust_limit_satoshis } else { self.their_dust_limit_satoshis };
                let mut remote_htlc_total_msat = 0;
                let mut local_htlc_total_msat = 0;
                let mut value_to_self_msat_offset = 0;
 
-               for ref htlc in self.pending_htlcs.iter() {
-                       let include = match htlc.state {
-                               HTLCState::RemoteAnnounced => !generated_by_local,
-                               HTLCState::AwaitingRemoteRevokeToAnnounce => !generated_by_local,
-                               HTLCState::AwaitingAnnouncedRemoteRevoke => true,
-                               HTLCState::LocalAnnounced => generated_by_local,
-                               HTLCState::Committed => true,
-                               HTLCState::RemoteRemoved => generated_by_local,
-                               HTLCState::AwaitingRemoteRevokeToRemove => generated_by_local,
-                               HTLCState::AwaitingRemovedRemoteRevoke => false,
-                               HTLCState::LocalRemoved => !generated_by_local,
-                       };
-
-                       if include {
-                               if htlc.outbound == local { // "offered HTLC output"
-                                       if htlc.amount_msat / 1000 >= dust_limit_satoshis + (self.feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
-                                               let htlc_in_tx = htlc.get_in_commitment(true);
+               macro_rules! add_htlc_output {
+                       ($htlc: expr, $outbound: expr) => {
+                               if $outbound == local { // "offered HTLC output"
+                                       if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (self.feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
+                                               let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
                                                txouts.push((TxOut {
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
-                                                       value: htlc.amount_msat / 1000
+                                                       value: $htlc.amount_msat / 1000
                                                }, Some(htlc_in_tx)));
                                        }
                                } else {
-                                       if htlc.amount_msat / 1000 >= dust_limit_satoshis + (self.feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
-                                               let htlc_in_tx = htlc.get_in_commitment(false);
+                                       if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (self.feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
+                                               let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
                                                txouts.push((TxOut { // "received HTLC output"
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
-                                                       value: htlc.amount_msat / 1000
+                                                       value: $htlc.amount_msat / 1000
                                                }, Some(htlc_in_tx)));
                                        }
-                               };
-                               if htlc.outbound {
-                                       local_htlc_total_msat += htlc.amount_msat;
-                               } else {
-                                       remote_htlc_total_msat += htlc.amount_msat;
                                }
+                       }
+               }
+
+               for ref htlc in self.pending_inbound_htlcs.iter() {
+                       let include = match htlc.state {
+                               InboundHTLCState::RemoteAnnounced => !generated_by_local,
+                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce => !generated_by_local,
+                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke => true,
+                               InboundHTLCState::Committed => true,
+                               InboundHTLCState::LocalRemoved => !generated_by_local,
+                       };
+
+                       if include {
+                               add_htlc_output!(htlc, false);
+                               remote_htlc_total_msat += htlc.amount_msat;
+                       } else {
+                               match htlc.state {
+                                       InboundHTLCState::LocalRemoved => {
+                                               if generated_by_local && htlc.local_removed_fulfilled {
+                                                       value_to_self_msat_offset += htlc.amount_msat as i64;
+                                               }
+                                       },
+                                       _ => {},
+                               }
+                       }
+               }
+
+               for ref htlc in self.pending_outbound_htlcs.iter() {
+                       let include = match htlc.state {
+                               OutboundHTLCState::LocalAnnounced => generated_by_local,
+                               OutboundHTLCState::Committed => true,
+                               OutboundHTLCState::RemoteRemoved => generated_by_local,
+                               OutboundHTLCState::AwaitingRemoteRevokeToRemove => generated_by_local,
+                               OutboundHTLCState::AwaitingRemovedRemoteRevoke => false,
+                       };
+
+                       if include {
+                               add_htlc_output!(htlc, true);
+                               local_htlc_total_msat += htlc.amount_msat;
                        } else {
                                match htlc.state {
-                                       HTLCState::AwaitingRemoteRevokeToRemove|HTLCState::AwaitingRemovedRemoteRevoke => {
+                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove|OutboundHTLCState::AwaitingRemovedRemoteRevoke => {
                                                if htlc.fail_reason.is_none() {
                                                        value_to_self_msat_offset -= htlc.amount_msat as i64;
                                                }
                                        },
-                                       HTLCState::RemoteRemoved => {
+                                       OutboundHTLCState::RemoteRemoved => {
                                                if !generated_by_local && htlc.fail_reason.is_none() {
                                                        value_to_self_msat_offset -= htlc.amount_msat as i64;
                                                }
                                        },
-                                       HTLCState::LocalRemoved => {
-                                               if generated_by_local && htlc.local_removed_fulfilled {
-                                                       value_to_self_msat_offset += htlc.amount_msat as i64;
-                                               }
-                                       },
                                        _ => {},
                                }
                        }
@@ -820,7 +846,8 @@ impl Channel {
                        ins
                };
 
-               assert!(self.pending_htlcs.is_empty());
+               assert!(self.pending_inbound_htlcs.is_empty());
+               assert!(self.pending_outbound_htlcs.is_empty());
                let mut txouts: Vec<(TxOut, ())> = Vec::new();
 
                let mut total_fee_satoshis = proposed_total_fee_satoshis;
@@ -994,7 +1021,7 @@ impl Channel {
                Ok(our_sig)
        }
 
-       fn get_update_fulfill_htlc(&mut self, payment_preimage_arg: [u8; 32]) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitor>), HandleError> {
+       fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: [u8; 32]) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitor>), HandleError> {
                // Either ChannelFunded got set (which means it wont bet unset) or there is no way any
                // caller thought we could have something claimed (cause we wouldn't have accepted in an
                // incoming HTLC anyway). If we got to ShutdownComplete, callers aren't allowed to call us,
@@ -1010,20 +1037,17 @@ impl Channel {
                sha.result(&mut payment_hash_calc);
 
                let mut pending_idx = std::usize::MAX;
-               for (idx, htlc) in self.pending_htlcs.iter().enumerate() {
-                       if !htlc.outbound && htlc.payment_hash == payment_hash_calc &&
-                                       htlc.state != HTLCState::LocalRemoved {
-                               if let Some(PendingHTLCStatus::Fail(_)) = htlc.pending_forward_state {
-                               } else {
-                                       if pending_idx != std::usize::MAX {
-                                               panic!("Duplicate HTLC payment_hash, ChannelManager should have prevented this!");
-                                       }
+               for (idx, htlc) in self.pending_inbound_htlcs.iter().enumerate() {
+                       if htlc.htlc_id == htlc_id_arg {
+                               assert_eq!(htlc.payment_hash, payment_hash_calc);
+                               if htlc.state != InboundHTLCState::LocalRemoved {
                                        pending_idx = idx;
+                                       break;
                                }
                        }
                }
                if pending_idx == std::usize::MAX {
-                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given payment preimage", action: None});
+                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given HTLC ID", action: None});
                }
 
                // Now update local state:
@@ -1035,31 +1059,31 @@ impl Channel {
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
                        for pending_update in self.holding_cell_htlc_updates.iter() {
                                match pending_update {
-                                       &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, .. } => {
-                                               if payment_preimage_arg == *payment_preimage {
+                                       &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
+                                               if htlc_id_arg == htlc_id {
                                                        return Ok((None, None));
                                                }
                                        },
-                                       &HTLCUpdateAwaitingACK::FailHTLC { ref payment_hash, .. } => {
-                                               if payment_hash_calc == *payment_hash {
-                                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given payment preimage", action: None});
+                                       &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
+                                               if htlc_id_arg == htlc_id {
+                                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given HTLC ID", action: None});
                                                }
                                        },
                                        _ => {}
                                }
                        }
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
-                               payment_preimage: payment_preimage_arg, payment_hash: payment_hash_calc,
+                               payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
                        return Ok((None, Some(self.channel_monitor.clone())));
                }
 
-               let htlc_id = {
-                       let htlc = &mut self.pending_htlcs[pending_idx];
-                       if htlc.state == HTLCState::Committed {
-                               htlc.state = HTLCState::LocalRemoved;
+               {
+                       let htlc = &mut self.pending_inbound_htlcs[pending_idx];
+                       if htlc.state == InboundHTLCState::Committed {
+                               htlc.state = InboundHTLCState::LocalRemoved;
                                htlc.local_removed_fulfilled = true;
-                       } else if htlc.state == HTLCState::RemoteAnnounced || htlc.state == HTLCState::AwaitingRemoteRevokeToAnnounce || htlc.state == HTLCState::AwaitingAnnouncedRemoteRevoke {
+                       } else if htlc.state == InboundHTLCState::RemoteAnnounced || htlc.state == InboundHTLCState::AwaitingRemoteRevokeToAnnounce || htlc.state == InboundHTLCState::AwaitingAnnouncedRemoteRevoke {
                                // Theoretically we can hit this if we get the preimage on an HTLC prior to us
                                // having forwarded it to anyone. This implies that the sender is busted as someone
                                // else knows the preimage, but handling this case and implementing the logic to
@@ -1068,21 +1092,20 @@ impl Channel {
                                // channel_monitor and pretend we didn't just see the preimage.
                                return Ok((None, Some(self.channel_monitor.clone())));
                        } else {
-                               // LocalRemoved/LocalRemovedAwaitingCOmmitment handled in the search loop
+                               // LocalRemoved handled in the search loop
                                panic!("Have an inbound HTLC when not awaiting remote revoke that had a garbage state");
                        }
-                       htlc.htlc_id
-               };
+               }
 
                Ok((Some(msgs::UpdateFulfillHTLC {
                        channel_id: self.channel_id(),
-                       htlc_id: htlc_id,
+                       htlc_id: htlc_id_arg,
                        payment_preimage: payment_preimage_arg,
                }), Some(self.channel_monitor.clone())))
        }
 
-       pub fn get_update_fulfill_htlc_and_commit(&mut self, payment_preimage: [u8; 32]) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option<ChannelMonitor>), HandleError> {
-               match self.get_update_fulfill_htlc(payment_preimage)? {
+       pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: [u8; 32]) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option<ChannelMonitor>), HandleError> {
+               match self.get_update_fulfill_htlc(htlc_id, payment_preimage)? {
                        (Some(update_fulfill_htlc), _) => {
                                let (commitment, monitor_update) = self.send_commitment_no_status_check()?;
                                Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
@@ -1092,7 +1115,7 @@ impl Channel {
                }
        }
 
-       pub fn get_update_fail_htlc(&mut self, payment_hash_arg: &[u8; 32], err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, HandleError> {
+       pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        panic!("Was asked to fail an HTLC when channel was not in an operational state");
                }
@@ -1102,13 +1125,13 @@ impl Channel {
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
                        for pending_update in self.holding_cell_htlc_updates.iter() {
                                match pending_update {
-                                       &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_hash, .. } => {
-                                               if *payment_hash_arg == *payment_hash {
-                                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given payment preimage", action: None});
+                                       &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
+                                               if htlc_id_arg == htlc_id {
+                                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given HTLC ID", action: None});
                                                }
                                        },
-                                       &HTLCUpdateAwaitingACK::FailHTLC { ref payment_hash, .. } => {
-                                               if *payment_hash_arg == *payment_hash {
+                                       &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
+                                               if htlc_id_arg == htlc_id {
                                                        return Ok(None);
                                                }
                                        },
@@ -1116,52 +1139,40 @@ impl Channel {
                                }
                        }
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::FailHTLC {
-                               payment_hash: payment_hash_arg.clone(),
+                               htlc_id: htlc_id_arg,
                                err_packet,
                        });
                        return Ok(None);
                }
 
-               let mut htlc_id = 0;
                let mut htlc_amount_msat = 0;
-               for htlc in self.pending_htlcs.iter_mut() {
-                       if !htlc.outbound && htlc.payment_hash == *payment_hash_arg {
-                               if htlc.state == HTLCState::Committed {
-                                       htlc.state = HTLCState::LocalRemoved;
-                               } else if htlc.state == HTLCState::RemoteAnnounced {
-                                       if let Some(PendingHTLCStatus::Forward(_)) = htlc.pending_forward_state {
-                                               panic!("Somehow forwarded HTLC prior to remote revocation!");
-                                       } else {
-                                               // We have to pretend this isn't here - we're probably a duplicate with the
-                                               // same payment_hash as some other HTLC, and the other is getting failed,
-                                               // we'll fail this one as soon as remote commits to it.
-                                               continue;
-                                       }
-                               } else if htlc.state == HTLCState::LocalRemoved {
-                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given payment preimage", action: None});
+               for htlc in self.pending_inbound_htlcs.iter_mut() {
+                       if htlc.htlc_id == htlc_id_arg {
+                               if htlc.state == InboundHTLCState::Committed {
+                                       htlc.state = InboundHTLCState::LocalRemoved;
+                               } else if htlc.state == InboundHTLCState::RemoteAnnounced {
+                                       panic!("Somehow forwarded HTLC prior to remote revocation!");
+                               } else if htlc.state == InboundHTLCState::LocalRemoved {
+                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given HTLC ID", action: None});
                                } else {
                                        panic!("Have an inbound HTLC when not awaiting remote revoke that had a garbage state");
                                }
-                               if htlc_id != 0 {
-                                       panic!("Duplicate HTLC payment_hash, you probably re-used payment preimages, NEVER DO THIS!");
-                               }
-                               htlc_id = htlc.htlc_id;
                                htlc_amount_msat += htlc.amount_msat;
                        }
                }
                if htlc_amount_msat == 0 {
-                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given payment preimage", action: None});
+                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given HTLC ID", action: None});
                }
 
                Ok(Some(msgs::UpdateFailHTLC {
                        channel_id: self.channel_id(),
-                       htlc_id,
+                       htlc_id: htlc_id_arg,
                        reason: err_packet
                }))
        }
 
-       pub fn get_update_fail_htlc_and_commit(&mut self, payment_hash: &[u8; 32], err_packet: msgs::OnionErrorPacket) -> Result<Option<(msgs::UpdateFailHTLC, msgs::CommitmentSigned, ChannelMonitor)>, HandleError> {
-               match self.get_update_fail_htlc(payment_hash, err_packet)? {
+       pub fn get_update_fail_htlc_and_commit(&mut self, htlc_id: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<(msgs::UpdateFailHTLC, msgs::CommitmentSigned, ChannelMonitor)>, HandleError> {
+               match self.get_update_fail_htlc(htlc_id, err_packet)? {
                        Some(update_fail_htlc) => {
                                let (commitment, monitor_update) = self.send_commitment_no_status_check()?;
                                Ok(Some((update_fail_htlc, commitment, monitor_update)))
@@ -1361,30 +1372,34 @@ impl Channel {
        /// totals, though there is little reason to outside of further avoiding any race condition
        /// issues.
        fn get_pending_htlc_stats(&self, for_remote_update_check: bool) -> (u32, u32, u64, u64) {
+               //TODO: Can probably split this into inbound/outbound
                let mut inbound_htlc_count: u32 = 0;
                let mut outbound_htlc_count: u32 = 0;
                let mut htlc_outbound_value_msat = 0;
                let mut htlc_inbound_value_msat = 0;
-               for ref htlc in self.pending_htlcs.iter() {
+               for ref htlc in self.pending_inbound_htlcs.iter() {
                        match htlc.state {
-                               HTLCState::RemoteAnnounced => {},
-                               HTLCState::AwaitingRemoteRevokeToAnnounce => {},
-                               HTLCState::AwaitingAnnouncedRemoteRevoke => {},
-                               HTLCState::LocalAnnounced => { if for_remote_update_check { continue; } },
-                               HTLCState::Committed => {},
-                               HTLCState::RemoteRemoved => { if for_remote_update_check { continue; } },
-                               HTLCState::AwaitingRemoteRevokeToRemove => { if for_remote_update_check { continue; } },
-                               HTLCState::AwaitingRemovedRemoteRevoke => { if for_remote_update_check { continue; } },
-                               HTLCState::LocalRemoved => {},
+                               InboundHTLCState::RemoteAnnounced => {},
+                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce => {},
+                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke => {},
+                               InboundHTLCState::Committed => {},
+                               InboundHTLCState::LocalRemoved => {},
                        }
-                       if !htlc.outbound {
-                               inbound_htlc_count += 1;
-                               htlc_inbound_value_msat += htlc.amount_msat;
-                       } else {
-                               outbound_htlc_count += 1;
-                               htlc_outbound_value_msat += htlc.amount_msat;
+                       inbound_htlc_count += 1;
+                       htlc_inbound_value_msat += htlc.amount_msat;
+               }
+               for ref htlc in self.pending_outbound_htlcs.iter() {
+                       match htlc.state {
+                               OutboundHTLCState::LocalAnnounced => { if for_remote_update_check { continue; } },
+                               OutboundHTLCState::Committed => {},
+                               OutboundHTLCState::RemoteRemoved => { if for_remote_update_check { continue; } },
+                               OutboundHTLCState::AwaitingRemoteRevokeToRemove => { if for_remote_update_check { continue; } },
+                               OutboundHTLCState::AwaitingRemovedRemoteRevoke => { if for_remote_update_check { continue; } },
                        }
+                       outbound_htlc_count += 1;
+                       htlc_outbound_value_msat += htlc.amount_msat;
                }
+
                (inbound_htlc_count, outbound_htlc_count, htlc_outbound_value_msat, htlc_inbound_value_msat)
        }
 
@@ -1425,14 +1440,12 @@ impl Channel {
 
                // Now update local state:
                self.next_remote_htlc_id += 1;
-               self.pending_htlcs.push(HTLCOutput {
-                       outbound: false,
+               self.pending_inbound_htlcs.push(InboundHTLCOutput {
                        htlc_id: msg.htlc_id,
                        amount_msat: msg.amount_msat,
                        payment_hash: msg.payment_hash,
                        cltv_expiry: msg.cltv_expiry,
-                       state: HTLCState::RemoteAnnounced,
-                       fail_reason: None,
+                       state: InboundHTLCState::RemoteAnnounced,
                        local_removed_fulfilled: false,
                        pending_forward_state: Some(pending_forward_state),
                });
@@ -1442,9 +1455,9 @@ impl Channel {
 
        /// Removes an outbound HTLC which has been commitment_signed by the remote end
        #[inline]
-       fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option<[u8; 32]>, fail_reason: Option<HTLCFailReason>) -> Result<[u8; 32], HandleError> {
-               for htlc in self.pending_htlcs.iter_mut() {
-                       if htlc.outbound && htlc.htlc_id == htlc_id {
+       fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option<[u8; 32]>, fail_reason: Option<HTLCFailReason>) -> Result<&HTLCSource, HandleError> {
+               for htlc in self.pending_outbound_htlcs.iter_mut() {
+                       if htlc.htlc_id == htlc_id {
                                match check_preimage {
                                        None => {},
                                        Some(payment_hash) =>
@@ -1452,23 +1465,23 @@ impl Channel {
                                                        return Err(HandleError{err: "Remote tried to fulfill HTLC with an incorrect preimage", action: None});
                                                }
                                };
-                               if htlc.state == HTLCState::LocalAnnounced {
-                                       return Err(HandleError{err: "Remote tried to fulfill HTLC before it had been committed", action: None});
-                               } else if htlc.state == HTLCState::Committed {
-                                       htlc.state = HTLCState::RemoteRemoved;
-                                       htlc.fail_reason = fail_reason;
-                               } else if htlc.state == HTLCState::AwaitingRemoteRevokeToRemove || htlc.state == HTLCState::AwaitingRemovedRemoteRevoke || htlc.state == HTLCState::RemoteRemoved {
-                                       return Err(HandleError{err: "Remote tried to fulfill HTLC that they'd already fulfilled", action: None});
-                               } else {
-                                       panic!("Got a non-outbound state on an outbound HTLC");
+                               match htlc.state {
+                                       OutboundHTLCState::LocalAnnounced =>
+                                               return Err(HandleError{err: "Remote tried to fulfill HTLC before it had been committed", action: None}),
+                                       OutboundHTLCState::Committed => {
+                                               htlc.state = OutboundHTLCState::RemoteRemoved;
+                                               htlc.fail_reason = fail_reason;
+                                       },
+                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove | OutboundHTLCState::AwaitingRemovedRemoteRevoke | OutboundHTLCState::RemoteRemoved =>
+                                               return Err(HandleError{err: "Remote tried to fulfill HTLC that they'd already fulfilled", action: None}),
                                }
-                               return Ok(htlc.payment_hash.clone());
+                               return Ok(&htlc.source);
                        }
                }
                Err(HandleError{err: "Remote tried to fulfill/fail an HTLC we couldn't find", action: None})
        }
 
-       pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(), HandleError> {
+       pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<&HTLCSource, HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(HandleError{err: "Got add HTLC message when channel was not in an operational state", action: None});
                }
@@ -1478,11 +1491,10 @@ impl Channel {
                let mut payment_hash = [0; 32];
                sha.result(&mut payment_hash);
 
-               self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None)?;
-               Ok(())
+               self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None)
        }
 
-       pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<[u8; 32], HandleError> {
+       pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<&HTLCSource, HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(HandleError{err: "Got add HTLC message when channel was not in an operational state", action: None});
                }
@@ -1490,13 +1502,12 @@ impl Channel {
                self.mark_outbound_htlc_removed(msg.htlc_id, None, Some(fail_reason))
        }
 
-       pub fn update_fail_malformed_htlc(&mut self, msg: &msgs::UpdateFailMalformedHTLC, fail_reason: HTLCFailReason) -> Result<(), HandleError> {
+       pub fn update_fail_malformed_htlc<'a>(&mut self, msg: &msgs::UpdateFailMalformedHTLC, fail_reason: HTLCFailReason) -> Result<&HTLCSource, HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(HandleError{err: "Got add HTLC message when channel was not in an operational state", action: None});
                }
 
-               self.mark_outbound_htlc_removed(msg.htlc_id, None, Some(fail_reason))?;
-               Ok(())
+               self.mark_outbound_htlc_removed(msg.htlc_id, None, Some(fail_reason))
        }
 
        pub fn commitment_signed(&mut self, msg: &msgs::CommitmentSigned) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>, ChannelMonitor), HandleError> {
@@ -1543,12 +1554,15 @@ impl Channel {
                self.channel_monitor.provide_latest_local_commitment_tx_info(local_commitment_tx.0, local_keys, self.feerate_per_kw, htlcs_and_sigs);
 
                let mut need_our_commitment = false;
-               for htlc in self.pending_htlcs.iter_mut() {
-                       if htlc.state == HTLCState::RemoteAnnounced {
-                               htlc.state = HTLCState::AwaitingRemoteRevokeToAnnounce;
+               for htlc in self.pending_inbound_htlcs.iter_mut() {
+                       if htlc.state == InboundHTLCState::RemoteAnnounced {
+                               htlc.state = InboundHTLCState::AwaitingRemoteRevokeToAnnounce;
                                need_our_commitment = true;
-                       } else if htlc.state == HTLCState::RemoteRemoved {
-                               htlc.state = HTLCState::AwaitingRemoteRevokeToRemove;
+                       }
+               }
+               for htlc in self.pending_outbound_htlcs.iter_mut() {
+                       if htlc.state == OutboundHTLCState::RemoteRemoved {
+                               htlc.state = OutboundHTLCState::AwaitingRemoteRevokeToRemove;
                                need_our_commitment = true;
                        }
                }
@@ -1591,24 +1605,24 @@ impl Channel {
                                        self.holding_cell_htlc_updates.push(htlc_update);
                                } else {
                                        match &htlc_update {
-                                               &HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, payment_hash, ref onion_routing_packet, ..} => {
-                                                       match self.send_htlc(amount_msat, payment_hash, cltv_expiry, onion_routing_packet.clone()) {
+                                               &HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, payment_hash, ref source, ref onion_routing_packet, ..} => {
+                                                       match self.send_htlc(amount_msat, payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone()) {
                                                                Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()),
                                                                Err(e) => {
                                                                        err = Some(e);
                                                                }
                                                        }
                                                },
-                                               &HTLCUpdateAwaitingACK::ClaimHTLC { payment_preimage, .. } => {
-                                                       match self.get_update_fulfill_htlc(payment_preimage) {
+                                               &HTLCUpdateAwaitingACK::ClaimHTLC { payment_preimage, htlc_id, .. } => {
+                                                       match self.get_update_fulfill_htlc(htlc_id, payment_preimage) {
                                                                Ok(update_fulfill_msg_option) => update_fulfill_htlcs.push(update_fulfill_msg_option.0.unwrap()),
                                                                Err(e) => {
                                                                        err = Some(e);
                                                                }
                                                        }
                                                },
-                                               &HTLCUpdateAwaitingACK::FailHTLC { payment_hash, ref err_packet } => {
-                                                       match self.get_update_fail_htlc(&payment_hash, err_packet.clone()) {
+                                               &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
+                                                       match self.get_update_fail_htlc(htlc_id, err_packet.clone()) {
                                                                Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()),
                                                                Err(e) => {
                                                                        err = Some(e);
@@ -1646,7 +1660,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) -> Result<(Option<msgs::CommitmentUpdate>, Vec<PendingForwardHTLCInfo>, Vec<([u8; 32], HTLCFailReason)>, ChannelMonitor), HandleError> {
+       pub fn revoke_and_ack(&mut self, msg: &msgs::RevokeAndACK) -> Result<(Option<msgs::CommitmentUpdate>, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, [u8; 32], HTLCFailReason)>, ChannelMonitor), HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(HandleError{err: "Got revoke/ACK message when channel was not in an operational state", action: None});
                }
@@ -1673,15 +1687,18 @@ impl Channel {
                let mut require_commitment = false;
                let mut value_to_self_msat_diff: i64 = 0;
                // We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
-               self.pending_htlcs.retain(|htlc| {
-                       if htlc.state == HTLCState::LocalRemoved {
+               self.pending_inbound_htlcs.retain(|htlc| {
+                       if htlc.state == InboundHTLCState::LocalRemoved {
                                if htlc.local_removed_fulfilled {
                                        value_to_self_msat_diff += htlc.amount_msat as i64;
                                }
                                false
-                       } else if htlc.state == HTLCState::AwaitingRemovedRemoteRevoke {
+                       } else { true }
+               });
+               self.pending_outbound_htlcs.retain(|htlc| {
+                       if htlc.state == OutboundHTLCState::AwaitingRemovedRemoteRevoke {
                                if let Some(reason) = htlc.fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
-                                       revoked_htlcs.push((htlc.payment_hash, reason));
+                                       revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
                                } else {
                                        // They fulfilled, so we sent them money
                                        value_to_self_msat_diff -= htlc.amount_msat as i64;
@@ -1689,16 +1706,14 @@ impl Channel {
                                false
                        } else { true }
                });
-               for htlc in self.pending_htlcs.iter_mut() {
-                       if htlc.state == HTLCState::LocalAnnounced {
-                               htlc.state = HTLCState::Committed;
-                       } else if htlc.state == HTLCState::AwaitingRemoteRevokeToAnnounce {
-                               htlc.state = HTLCState::AwaitingAnnouncedRemoteRevoke;
+               for htlc in self.pending_inbound_htlcs.iter_mut() {
+                       if htlc.state == InboundHTLCState::AwaitingRemoteRevokeToAnnounce {
+                               htlc.state = InboundHTLCState::AwaitingAnnouncedRemoteRevoke;
                                require_commitment = true;
-                       } else if htlc.state == HTLCState::AwaitingAnnouncedRemoteRevoke {
+                       } else if htlc.state == InboundHTLCState::AwaitingAnnouncedRemoteRevoke {
                                match htlc.pending_forward_state.take().unwrap() {
                                        PendingHTLCStatus::Fail(fail_msg) => {
-                                               htlc.state = HTLCState::LocalRemoved;
+                                               htlc.state = InboundHTLCState::LocalRemoved;
                                                require_commitment = true;
                                                match fail_msg {
                                                        HTLCFailureMsg::Relay(msg) => update_fail_htlcs.push(msg),
@@ -1706,12 +1721,17 @@ impl Channel {
                                                }
                                        },
                                        PendingHTLCStatus::Forward(forward_info) => {
-                                               to_forward_infos.push(forward_info);
-                                               htlc.state = HTLCState::Committed;
+                                               to_forward_infos.push((forward_info, htlc.htlc_id));
+                                               htlc.state = InboundHTLCState::Committed;
                                        }
                                }
-                       } else if htlc.state == HTLCState::AwaitingRemoteRevokeToRemove {
-                               htlc.state = HTLCState::AwaitingRemovedRemoteRevoke;
+                       }
+               }
+               for htlc in self.pending_outbound_htlcs.iter_mut() {
+                       if htlc.state == OutboundHTLCState::LocalAnnounced {
+                               htlc.state = OutboundHTLCState::Committed;
+                       } else if htlc.state == OutboundHTLCState::AwaitingRemoteRevokeToRemove {
+                               htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke;
                                require_commitment = true;
                        }
                }
@@ -1756,14 +1776,14 @@ impl Channel {
                Ok(())
        }
 
-       pub fn shutdown(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Vec<[u8; 32]>), HandleError> {
+       pub fn shutdown(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Vec<(HTLCSource, [u8; 32])>), HandleError> {
                if self.channel_state < ChannelState::FundingSent as u32 {
                        self.channel_state = ChannelState::ShutdownComplete as u32;
                        self.channel_update_count += 1;
                        return Ok((None, None, Vec::new()));
                }
-               for htlc in self.pending_htlcs.iter() {
-                       if htlc.state == HTLCState::RemoteAnnounced {
+               for htlc in self.pending_inbound_htlcs.iter() {
+                       if htlc.state == InboundHTLCState::RemoteAnnounced {
                                return Err(HandleError{err: "Got shutdown with remote pending HTLCs", action: None});
                        }
                }
@@ -1789,7 +1809,7 @@ impl Channel {
 
                let our_closing_script = self.get_closing_scriptpubkey();
 
-               let (proposed_feerate, proposed_fee, our_sig) = if self.channel_outbound && self.pending_htlcs.is_empty() {
+               let (proposed_feerate, proposed_fee, our_sig) = if self.channel_outbound && self.pending_inbound_htlcs.is_empty() && self.pending_outbound_htlcs.is_empty() {
                        let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
                        if self.feerate_per_kw > proposed_feerate {
                                proposed_feerate = self.feerate_per_kw;
@@ -1815,15 +1835,15 @@ impl Channel {
                let mut dropped_outbound_htlcs = Vec::with_capacity(self.holding_cell_htlc_updates.len());
                self.holding_cell_htlc_updates.retain(|htlc_update| {
                        match htlc_update {
-                               &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, .. } => {
-                                       dropped_outbound_htlcs.push(payment_hash.clone());
+                               &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => {
+                                       dropped_outbound_htlcs.push((source.clone(), payment_hash.clone()));
                                        false
                                },
                                _ => true
                        }
                });
-               for htlc in self.pending_htlcs.iter() {
-                       if htlc.state == HTLCState::LocalAnnounced {
+               for htlc in self.pending_outbound_htlcs.iter() {
+                       if htlc.state == OutboundHTLCState::LocalAnnounced {
                                return Ok((None, None, dropped_outbound_htlcs));
                        }
                }
@@ -1839,7 +1859,7 @@ impl Channel {
 
                self.channel_state |= ChannelState::LocalShutdownSent as u32;
                self.channel_update_count += 1;
-               if self.pending_htlcs.is_empty() && self.channel_outbound {
+               if self.pending_inbound_htlcs.is_empty() && self.pending_outbound_htlcs.is_empty() && self.channel_outbound {
                        // There are no more HTLCs and we're the funder, this means we start the closing_signed
                        // dance with an initial fee proposal!
                        self.last_sent_closing_fee = Some((proposed_feerate.unwrap(), proposed_fee.unwrap()));
@@ -1857,7 +1877,7 @@ impl Channel {
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != BOTH_SIDES_SHUTDOWN_MASK {
                        return Err(HandleError{err: "Remote end sent us a closing_signed before both sides provided a shutdown", action: None});
                }
-               if !self.pending_htlcs.is_empty() {
+               if !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() {
                        return Err(HandleError{err: "Remote end sent us a closing_signed while there were still pending HTLCs", action: None});
                }
                if msg.fee_satoshis > 21000000 * 10000000 {
@@ -2321,7 +2341,7 @@ impl Channel {
        /// waiting on the remote peer to send us a revoke_and_ack during which time we cannot add new
        /// HTLCs on the wire or we wouldn't be able to determine what they actually ACK'ed.
        /// You MUST call send_commitment prior to any other calls on this Channel
-       pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: [u8; 32], cltv_expiry: u32, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, HandleError> {
+       pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: [u8; 32], cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelFunded as u32) {
                        return Err(HandleError{err: "Cannot send HTLC until channel is fully established and we haven't started shutting down", action: None});
                }
@@ -2356,22 +2376,21 @@ impl Channel {
                                amount_msat: amount_msat,
                                payment_hash: payment_hash,
                                cltv_expiry: cltv_expiry,
+                               source,
                                onion_routing_packet: onion_routing_packet,
                                time_created: Instant::now(),
                        });
                        return Ok(None);
                }
 
-               self.pending_htlcs.push(HTLCOutput {
-                       outbound: true,
+               self.pending_outbound_htlcs.push(OutboundHTLCOutput {
                        htlc_id: self.next_local_htlc_id,
                        amount_msat: amount_msat,
                        payment_hash: payment_hash.clone(),
                        cltv_expiry: cltv_expiry,
-                       state: HTLCState::LocalAnnounced,
+                       state: OutboundHTLCState::LocalAnnounced,
+                       source,
                        fail_reason: None,
-                       local_removed_fulfilled: false,
-                       pending_forward_state: None
                });
 
                let res = msgs::UpdateAddHTLC {
@@ -2399,8 +2418,8 @@ impl Channel {
                        panic!("Cannot create commitment tx until remote revokes their previous commitment");
                }
                let mut have_updates = false; // TODO initialize with "have we sent a fee update?"
-               for htlc in self.pending_htlcs.iter() {
-                       if htlc.state == HTLCState::LocalAnnounced {
+               for htlc in self.pending_outbound_htlcs.iter() {
+                       if htlc.state == OutboundHTLCState::LocalAnnounced {
                                have_updates = true;
                        }
                        if have_updates { break; }
@@ -2417,11 +2436,14 @@ impl Channel {
                // We can upgrade the status of some HTLCs that are waiting on a commitment, even if we
                // fail to generate this, we still are at least at a position where upgrading their status
                // is acceptable.
-               for htlc in self.pending_htlcs.iter_mut() {
-                       if htlc.state == HTLCState::AwaitingRemoteRevokeToAnnounce {
-                               htlc.state = HTLCState::AwaitingAnnouncedRemoteRevoke;
-                       } else if htlc.state == HTLCState::AwaitingRemoteRevokeToRemove {
-                               htlc.state = HTLCState::AwaitingRemovedRemoteRevoke;
+               for htlc in self.pending_inbound_htlcs.iter_mut() {
+                       if htlc.state == InboundHTLCState::AwaitingRemoteRevokeToAnnounce {
+                               htlc.state = InboundHTLCState::AwaitingAnnouncedRemoteRevoke;
+                       }
+               }
+               for htlc in self.pending_outbound_htlcs.iter_mut() {
+                       if htlc.state == OutboundHTLCState::AwaitingRemoteRevokeToRemove {
+                               htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke;
                        }
                }
 
@@ -2456,8 +2478,8 @@ impl Channel {
        /// to send to the remote peer in one go.
        /// Shorthand for calling send_htlc() followed by send_commitment(), see docs on those for
        /// more info.
-       pub fn send_htlc_and_commit(&mut self, amount_msat: u64, payment_hash: [u8; 32], cltv_expiry: u32, onion_routing_packet: msgs::OnionPacket) -> Result<Option<(msgs::UpdateAddHTLC, msgs::CommitmentSigned, ChannelMonitor)>, HandleError> {
-               match self.send_htlc(amount_msat, payment_hash, cltv_expiry, onion_routing_packet)? {
+       pub fn send_htlc_and_commit(&mut self, amount_msat: u64, payment_hash: [u8; 32], cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<(msgs::UpdateAddHTLC, msgs::CommitmentSigned, ChannelMonitor)>, HandleError> {
+               match self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet)? {
                        Some(update_add_htlc) => {
                                let (commitment_signed, monitor_update) = self.send_commitment_no_status_check()?;
                                Ok(Some((update_add_htlc, commitment_signed, monitor_update)))
@@ -2468,9 +2490,9 @@ impl Channel {
 
        /// Begins the shutdown process, getting a message for the remote peer and returning all
        /// holding cell HTLCs for payment failure.
-       pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<[u8; 32]>), HandleError> {
-               for htlc in self.pending_htlcs.iter() {
-                       if htlc.state == HTLCState::LocalAnnounced {
+       pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, [u8; 32])>), HandleError> {
+               for htlc in self.pending_outbound_htlcs.iter() {
+                       if htlc.state == OutboundHTLCState::LocalAnnounced {
                                return Err(HandleError{err: "Cannot begin shutdown with pending HTLCs, call send_commitment first", action: None});
                        }
                }
@@ -2495,8 +2517,8 @@ impl Channel {
                let mut dropped_outbound_htlcs = Vec::with_capacity(self.holding_cell_htlc_updates.len());
                self.holding_cell_htlc_updates.retain(|htlc_update| {
                        match htlc_update {
-                               &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, .. } => {
-                                       dropped_outbound_htlcs.push(payment_hash.clone());
+                               &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => {
+                                       dropped_outbound_htlcs.push((source.clone(), payment_hash.clone()));
                                        false
                                },
                                _ => true
@@ -2514,7 +2536,7 @@ impl Channel {
        /// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
        /// Also returns the list of payment_hashes for channels which we can safely fail backwards
        /// immediately (others we will have to allow to time out).
-       pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<[u8; 32]>) {
+       pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, [u8; 32])>) {
                assert!(self.channel_state != ChannelState::ShutdownComplete as u32);
 
                // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
@@ -2522,16 +2544,16 @@ impl Channel {
                let mut dropped_outbound_htlcs = Vec::with_capacity(self.holding_cell_htlc_updates.len());
                for htlc_update in self.holding_cell_htlc_updates.drain(..) {
                        match htlc_update {
-                               HTLCUpdateAwaitingACK::AddHTLC { payment_hash, .. } => {
-                                       dropped_outbound_htlcs.push(payment_hash);
+                               HTLCUpdateAwaitingACK::AddHTLC { source, payment_hash, .. } => {
+                                       dropped_outbound_htlcs.push((source, payment_hash));
                                },
                                _ => {}
                        }
                }
 
-               for htlc in self.pending_htlcs.drain(..) {
-                       if htlc.state == HTLCState::LocalAnnounced {
-                               dropped_outbound_htlcs.push(htlc.payment_hash);
+               for htlc in self.pending_outbound_htlcs.drain(..) {
+                       if htlc.state == OutboundHTLCState::LocalAnnounced {
+                               dropped_outbound_htlcs.push((htlc.source, htlc.payment_hash));
                        }
                        //TODO: Do something with the remaining HTLCs
                        //(we need to have the ChannelManager monitor them so we can claim the inbound HTLCs
@@ -2554,7 +2576,8 @@ mod tests {
        use bitcoin::blockdata::script::Script;
        use bitcoin::blockdata::transaction::Transaction;
        use hex;
-       use ln::channel::{Channel,ChannelKeys,HTLCOutput,HTLCState,HTLCOutputInCommitment,TxCreationKeys};
+       use ln::channelmanager::HTLCSource;
+       use ln::channel::{Channel,ChannelKeys,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,TxCreationKeys};
        use ln::channel::MAX_FUNDING_SATOSHIS;
        use ln::chan_utils;
        use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
@@ -2693,15 +2716,13 @@ mod tests {
                                         "02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b8002c0c62d0000000000160014ccf1af2f2aabee14bb40fa3851ab2301de84311054a56a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e0400473044022051b75c73198c6deee1a875871c3961832909acd297c6b908d59e3319e5185a46022055c419379c5051a78d00dbbce11b5b664a0c22815fbcc6fcef6b1937c383693901483045022100f51d2e566a70ba740fc5d8c0f07b9b93d2ed741c3c0860c613173de7d39e7968022041376d520e9c0e1ad52248ddf4b22e12be8763007df977253ef45a4ca3bdb7c001475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220");
                }
 
-               chan.pending_htlcs.push({
-                       let mut out = HTLCOutput{
+               chan.pending_inbound_htlcs.push({
+                       let mut out = InboundHTLCOutput{
                                htlc_id: 0,
-                               outbound: false,
                                amount_msat: 1000000,
                                cltv_expiry: 500,
                                payment_hash: [0; 32],
-                               state: HTLCState::Committed,
-                               fail_reason: None,
+                               state: InboundHTLCState::Committed,
                                local_removed_fulfilled: false,
                                pending_forward_state: None,
                        };
@@ -2710,15 +2731,13 @@ mod tests {
                        sha.result(&mut out.payment_hash);
                        out
                });
-               chan.pending_htlcs.push({
-                       let mut out = HTLCOutput{
+               chan.pending_inbound_htlcs.push({
+                       let mut out = InboundHTLCOutput{
                                htlc_id: 1,
-                               outbound: false,
                                amount_msat: 2000000,
                                cltv_expiry: 501,
                                payment_hash: [0; 32],
-                               state: HTLCState::Committed,
-                               fail_reason: None,
+                               state: InboundHTLCState::Committed,
                                local_removed_fulfilled: false,
                                pending_forward_state: None,
                        };
@@ -2727,49 +2746,43 @@ mod tests {
                        sha.result(&mut out.payment_hash);
                        out
                });
-               chan.pending_htlcs.push({
-                       let mut out = HTLCOutput{
+               chan.pending_outbound_htlcs.push({
+                       let mut out = OutboundHTLCOutput{
                                htlc_id: 2,
-                               outbound: true,
                                amount_msat: 2000000,
                                cltv_expiry: 502,
                                payment_hash: [0; 32],
-                               state: HTLCState::Committed,
+                               state: OutboundHTLCState::Committed,
+                               source: HTLCSource::dummy(),
                                fail_reason: None,
-                               local_removed_fulfilled: false,
-                               pending_forward_state: None,
                        };
                        let mut sha = Sha256::new();
                        sha.input(&hex::decode("0202020202020202020202020202020202020202020202020202020202020202").unwrap());
                        sha.result(&mut out.payment_hash);
                        out
                });
-               chan.pending_htlcs.push({
-                       let mut out = HTLCOutput{
+               chan.pending_outbound_htlcs.push({
+                       let mut out = OutboundHTLCOutput{
                                htlc_id: 3,
-                               outbound: true,
                                amount_msat: 3000000,
                                cltv_expiry: 503,
                                payment_hash: [0; 32],
-                               state: HTLCState::Committed,
+                               state: OutboundHTLCState::Committed,
+                               source: HTLCSource::dummy(),
                                fail_reason: None,
-                               local_removed_fulfilled: false,
-                               pending_forward_state: None,
                        };
                        let mut sha = Sha256::new();
                        sha.input(&hex::decode("0303030303030303030303030303030303030303030303030303030303030303").unwrap());
                        sha.result(&mut out.payment_hash);
                        out
                });
-               chan.pending_htlcs.push({
-                       let mut out = HTLCOutput{
+               chan.pending_inbound_htlcs.push({
+                       let mut out = InboundHTLCOutput{
                                htlc_id: 4,
-                               outbound: false,
                                amount_msat: 4000000,
                                cltv_expiry: 504,
                                payment_hash: [0; 32],
-                               state: HTLCState::Committed,
-                               fail_reason: None,
+                               state: InboundHTLCState::Committed,
                                local_removed_fulfilled: false,
                                pending_forward_state: None,
                        };
index baf211fdba527f7f65b2980b2eb2a5ca9cdf1c33..e111a53163cb3273c24c5fa46fe285435ea8b281 100644 (file)
@@ -36,16 +36,31 @@ use std::sync::{Mutex,MutexGuard,Arc};
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::time::{Instant,Duration};
 
+/// We hold various information about HTLC relay in the HTLC objects in Channel itself:
+///
+/// Upon receipt of an HTLC from a peer, we'll give it a PendingHTLCStatus indicating if it should
+/// 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).
+/// 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.
 mod channel_held_info {
        use ln::msgs;
+       use ln::router::Route;
+       use secp256k1::key::SecretKey;
+       use secp256k1::ecdh::SharedSecret;
 
        /// 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 struct PendingForwardHTLCInfo {
                pub(super) onion_packet: Option<msgs::OnionPacket>,
+               pub(super) incoming_shared_secret: SharedSecret,
                pub(super) payment_hash: [u8; 32],
                pub(super) short_channel_id: u64,
-               pub(super) prev_short_channel_id: u64,
                pub(super) amt_to_forward: u64,
                pub(super) outgoing_cltv_value: u32,
        }
@@ -66,17 +81,47 @@ mod channel_held_info {
        #[cfg(feature = "fuzztarget")]
        impl PendingHTLCStatus {
                pub fn dummy() -> Self {
+                       let secp_ctx = ::secp256k1::Secp256k1::signing_only();
                        PendingHTLCStatus::Forward(PendingForwardHTLCInfo {
                                onion_packet: None,
+                               incoming_shared_secret: SharedSecret::new(&secp_ctx,
+                                               &::secp256k1::key::PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[1; 32]).unwrap()),
+                                               &SecretKey::from_slice(&secp_ctx, &[1; 32]).unwrap()),
                                payment_hash: [0; 32],
                                short_channel_id: 0,
-                               prev_short_channel_id: 0,
                                amt_to_forward: 0,
                                outgoing_cltv_value: 0,
                        })
                }
        }
 
+       /// Tracks the inbound corresponding to an outbound HTLC
+       #[derive(Clone)]
+       pub struct HTLCPreviousHopData {
+               pub(super) short_channel_id: u64,
+               pub(super) htlc_id: u64,
+               pub(super) incoming_packet_shared_secret: SharedSecret,
+       }
+
+       /// Tracks the inbound corresponding to an outbound HTLC
+       #[derive(Clone)]
+       pub enum HTLCSource {
+               PreviousHopData(HTLCPreviousHopData),
+               OutboundRoute {
+                       route: Route,
+                       session_priv: SecretKey,
+               },
+       }
+       #[cfg(any(test, feature = "fuzztarget"))]
+       impl HTLCSource {
+               pub fn dummy() -> Self {
+                       HTLCSource::OutboundRoute {
+                               route: Route { hops: Vec::new() },
+                               session_priv: SecretKey::from_slice(&::secp256k1::Secp256k1::without_caps(), &[1; 32]).unwrap(),
+                       }
+               }
+       }
+
        #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
        pub enum HTLCFailReason {
                ErrorPacket {
@@ -102,24 +147,6 @@ pub use self::channel_held_info::*;
 #[cfg(not(feature = "fuzztarget"))]
 pub(crate) use self::channel_held_info::*;
 
-enum PendingOutboundHTLC {
-       IntermediaryHopData {
-               source_short_channel_id: u64,
-               incoming_packet_shared_secret: SharedSecret,
-       },
-       OutboundRoute {
-               route: Route,
-               session_priv: SecretKey,
-       },
-       /// Used for channel rebalancing
-       CycledRoute {
-               source_short_channel_id: u64,
-               incoming_packet_shared_secret: SharedSecret,
-               route: Route,
-               session_priv: SecretKey,
-       }
-}
-
 struct MsgHandleErrInternal {
        err: msgs::HandleError,
        needs_channel_force_close: bool,
@@ -171,6 +198,12 @@ impl MsgHandleErrInternal {
 /// probably increase this significantly.
 const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u32 = 50;
 
+struct HTLCForwardInfo {
+       prev_short_channel_id: u64,
+       prev_htlc_id: u64,
+       forward_info: PendingForwardHTLCInfo,
+}
+
 struct ChannelHolder {
        by_id: HashMap<[u8; 32], Channel>,
        short_to_id: HashMap<u64, [u8; 32]>,
@@ -179,18 +212,18 @@ struct ChannelHolder {
        /// Note that while this is held in the same mutex as the channels themselves, no consistency
        /// guarantees are made about there existing a channel with the short id here, nor the short
        /// ids in the PendingForwardHTLCInfo!
-       forward_htlcs: HashMap<u64, Vec<PendingForwardHTLCInfo>>,
+       forward_htlcs: HashMap<u64, Vec<HTLCForwardInfo>>,
        /// 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!
-       claimable_htlcs: HashMap<[u8; 32], PendingOutboundHTLC>,
+       claimable_htlcs: HashMap<[u8; 32], Vec<HTLCPreviousHopData>>,
 }
 struct MutChannelHolder<'a> {
        by_id: &'a mut HashMap<[u8; 32], Channel>,
        short_to_id: &'a mut HashMap<u64, [u8; 32]>,
        next_forward: &'a mut Instant,
-       forward_htlcs: &'a mut HashMap<u64, Vec<PendingForwardHTLCInfo>>,
-       claimable_htlcs: &'a mut HashMap<[u8; 32], PendingOutboundHTLC>,
+       forward_htlcs: &'a mut HashMap<u64, Vec<HTLCForwardInfo>>,
+       claimable_htlcs: &'a mut HashMap<[u8; 32], Vec<HTLCPreviousHopData>>,
 }
 impl ChannelHolder {
        fn borrow_parts(&mut self) -> MutChannelHolder {
@@ -392,7 +425,7 @@ impl ChannelManager {
        /// pending HTLCs, the channel will be closed on chain.
        /// May generate a SendShutdown event on success, which should be relayed.
        pub fn close_channel(&self, channel_id: &[u8; 32]) -> Result<(), HandleError> {
-               let (res, node_id, chan_option) = {
+               let (mut res, node_id, chan_option) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = channel_state_lock.borrow_parts();
                        match channel_state.by_id.entry(channel_id.clone()) {
@@ -408,9 +441,9 @@ impl ChannelManager {
                                hash_map::Entry::Vacant(_) => return Err(HandleError{err: "No such channel", action: None})
                        }
                };
-               for payment_hash in res.1 {
+               for htlc_source in res.1.drain(..) {
                        // unknown_next_peer...I dunno who that is anymore....
-                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
+                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
                }
                let chan_update = if let Some(chan) = chan_option {
                        if let Ok(update) = self.get_channel_update(&chan) {
@@ -433,11 +466,11 @@ impl ChannelManager {
        }
 
        #[inline]
-       fn finish_force_close_channel(&self, shutdown_res: (Vec<Transaction>, Vec<[u8; 32]>)) {
-               let (local_txn, failed_htlcs) = shutdown_res;
-               for payment_hash in failed_htlcs {
+       fn finish_force_close_channel(&self, shutdown_res: (Vec<Transaction>, Vec<(HTLCSource, [u8; 32])>)) {
+               let (local_txn, mut failed_htlcs) = shutdown_res;
+               for htlc_source in failed_htlcs.drain(..) {
                        // unknown_next_peer...I dunno who that is anymore....
-                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
+                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
                }
                for tx in local_txn {
                        self.tx_broadcaster.broadcast_transaction(&tx);
@@ -722,7 +755,7 @@ impl ChannelManager {
                ChannelManager::encrypt_failure_packet(shared_secret, &failure_packet.encode()[..])
        }
 
-       fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> (PendingHTLCStatus, Option<SharedSecret>, MutexGuard<ChannelHolder>) {
+       fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> (PendingHTLCStatus, MutexGuard<ChannelHolder>) {
                macro_rules! get_onion_hash {
                        () => {
                                {
@@ -742,7 +775,7 @@ impl ChannelManager {
                                htlc_id: msg.htlc_id,
                                sha256_of_onion: get_onion_hash!(),
                                failure_code: 0x8000 | 0x4000 | 6,
-                       })), None, self.channel_state.lock().unwrap());
+                       })), self.channel_state.lock().unwrap());
                }
 
                let shared_secret = SharedSecret::new(&self.secp_ctx, &msg.onion_routing_packet.public_key.unwrap(), &self.our_network_key);
@@ -760,7 +793,7 @@ impl ChannelManager {
                                                channel_id: msg.channel_id,
                                                htlc_id: msg.htlc_id,
                                                reason: ChannelManager::build_first_hop_failure_packet(&shared_secret, $err_code, $data),
-                                       })), Some(shared_secret), channel_state.unwrap());
+                                       })), channel_state.unwrap());
                                }
                        }
                }
@@ -818,7 +851,7 @@ impl ChannelManager {
                                        onion_packet: None,
                                        payment_hash: msg.payment_hash.clone(),
                                        short_channel_id: 0,
-                                       prev_short_channel_id: 0,
+                                       incoming_shared_secret: shared_secret.clone(),
                                        amt_to_forward: next_hop_data.data.amt_to_forward,
                                        outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
                                })
@@ -858,7 +891,7 @@ impl ChannelManager {
                                        onion_packet: Some(outgoing_packet),
                                        payment_hash: msg.payment_hash.clone(),
                                        short_channel_id: next_hop_data.data.short_channel_id,
-                                       prev_short_channel_id: 0,
+                                       incoming_shared_secret: shared_secret.clone(),
                                        amt_to_forward: next_hop_data.data.amt_to_forward,
                                        outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
                                })
@@ -896,7 +929,7 @@ impl ChannelManager {
                        }
                }
 
-               (pending_forward_info, Some(shared_secret), channel_state.unwrap())
+               (pending_forward_info, channel_state.unwrap())
        }
 
        /// only fails if the channel does not yet have an assigned short_id
@@ -975,11 +1008,6 @@ impl ChannelManager {
                                Some(id) => id.clone()
                        };
 
-                       let claimable_htlc_entry = channel_state.claimable_htlcs.entry(payment_hash.clone());
-                       if let hash_map::Entry::Occupied(_) = claimable_htlc_entry {
-                               return Err(HandleError{err: "Already had pending HTLC with the same payment_hash", action: None});
-                       }
-
                        let res = {
                                let chan = channel_state.by_id.get_mut(&id).unwrap();
                                if chan.get_their_node_id() != route.hops.first().unwrap().pubkey {
@@ -988,16 +1016,14 @@ impl ChannelManager {
                                if !chan.is_live() {
                                        return Err(HandleError{err: "Peer for first hop currently disconnected!", action: None});
                                }
-                               chan.send_htlc_and_commit(htlc_msat, payment_hash, htlc_cltv, onion_packet)?
+                               chan.send_htlc_and_commit(htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
+                                       route: route.clone(),
+                                       session_priv: session_priv.clone(),
+                               }, onion_packet)?
                        };
 
                        let first_hop_node_id = route.hops.first().unwrap().pubkey;
 
-                       claimable_htlc_entry.or_insert(PendingOutboundHTLC::OutboundRoute {
-                               route,
-                               session_priv,
-                       });
-
                        match res {
                                Some(msgs) => (first_hop_node_id, msgs),
                                None => return Ok(()),
@@ -1005,7 +1031,7 @@ impl ChannelManager {
                };
 
                if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
-                       unimplemented!(); // maybe remove from claimable_htlcs?
+                       unimplemented!();
                }
 
                let mut events = self.pending_events.lock().unwrap();
@@ -1060,7 +1086,7 @@ impl ChannelManager {
                        }
                }; // Release channel lock for install_watch_outpoint call,
                if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
-                       unimplemented!(); // maybe remove from claimable_htlcs?
+                       unimplemented!();
                }
                add_pending_event!(events::Event::SendFundingCreated {
                        node_id: chan.get_their_node_id(),
@@ -1110,14 +1136,19 @@ impl ChannelManager {
                                return;
                        }
 
-                       for (short_chan_id, pending_forwards) in channel_state.forward_htlcs.drain() {
+                       for (short_chan_id, mut pending_forwards) in channel_state.forward_htlcs.drain() {
                                if short_chan_id != 0 {
                                        let forward_chan_id = match channel_state.short_to_id.get(&short_chan_id) {
                                                Some(chan_id) => chan_id.clone(),
                                                None => {
                                                        failed_forwards.reserve(pending_forwards.len());
-                                                       for forward_info in pending_forwards {
-                                                               failed_forwards.push((forward_info.payment_hash, 0x4000 | 10, None));
+                                                       for HTLCForwardInfo { prev_short_channel_id, prev_htlc_id, forward_info } in pending_forwards.drain(..) {
+                                                               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,
+                                                               });
+                                                               failed_forwards.push((htlc_source, forward_info.payment_hash, 0x4000 | 10, None));
                                                        }
                                                        continue;
                                                }
@@ -1125,11 +1156,16 @@ impl ChannelManager {
                                        let forward_chan = &mut channel_state.by_id.get_mut(&forward_chan_id).unwrap();
 
                                        let mut add_htlc_msgs = Vec::new();
-                                       for forward_info in pending_forwards {
-                                               match forward_chan.send_htlc(forward_info.amt_to_forward, forward_info.payment_hash, forward_info.outgoing_cltv_value, forward_info.onion_packet.unwrap()) {
+                                       for HTLCForwardInfo { prev_short_channel_id, prev_htlc_id, forward_info } in pending_forwards.drain(..) {
+                                               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,
+                                               });
+                                               match forward_chan.send_htlc(forward_info.amt_to_forward, forward_info.payment_hash, forward_info.outgoing_cltv_value, htlc_source.clone(), forward_info.onion_packet.unwrap()) {
                                                        Err(_e) => {
                                                                let chan_update = self.get_channel_update(forward_chan).unwrap();
-                                                               failed_forwards.push((forward_info.payment_hash, 0x1000 | 7, Some(chan_update)));
+                                                               failed_forwards.push((htlc_source, forward_info.payment_hash, 0x1000 | 7, Some(chan_update)));
                                                                continue;
                                                        },
                                                        Ok(update_add) => {
@@ -1174,7 +1210,16 @@ impl ChannelManager {
                                                }));
                                        }
                                } else {
-                                       for forward_info in pending_forwards {
+                                       for HTLCForwardInfo { prev_short_channel_id, prev_htlc_id, forward_info } in pending_forwards.drain(..) {
+                                               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,
+                                               };
+                                               match channel_state.claimable_htlcs.entry(forward_info.payment_hash) {
+                                                       hash_map::Entry::Occupied(mut entry) => entry.get_mut().push(prev_hop_data),
+                                                       hash_map::Entry::Vacant(mut entry) => { entry.insert(vec![prev_hop_data]); },
+                                               };
                                                new_events.push((None, events::Event::PaymentReceived {
                                                        payment_hash: forward_info.payment_hash,
                                                        amt: forward_info.amt_to_forward,
@@ -1184,10 +1229,10 @@ impl ChannelManager {
                        }
                }
 
-               for failed_forward in failed_forwards.drain(..) {
-                       match failed_forward.2 {
-                               None => self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &failed_forward.0, HTLCFailReason::Reason { failure_code: failed_forward.1, data: Vec::new() }),
-                               Some(chan_update) => self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &failed_forward.0, HTLCFailReason::Reason { failure_code: failed_forward.1, data: chan_update.encode_with_len() }),
+               for (htlc_source, payment_hash, failure_code, update) in failed_forwards.drain(..) {
+                       match update {
+                               None => self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source, &payment_hash, HTLCFailReason::Reason { failure_code, data: Vec::new() }),
+                               Some(chan_update) => self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source, &payment_hash, HTLCFailReason::Reason { failure_code, data: chan_update.encode_with_len() }),
                        };
                }
 
@@ -1211,7 +1256,15 @@ impl ChannelManager {
 
        /// Indicates that the preimage for payment_hash is unknown after a PaymentReceived event.
        pub fn fail_htlc_backwards(&self, payment_hash: &[u8; 32]) -> bool {
-               self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: Vec::new() })
+               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 htlc_with_hash 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: Vec::new() });
+                       }
+                       true
+               } else { false }
        }
 
        /// Fails an HTLC backwards to the sender of it to us.
@@ -1220,37 +1273,17 @@ impl ChannelManager {
        /// to fail and take the channel_state lock for each iteration (as we take ownership and may
        /// drop it). In other words, no assumptions are made that entries in claimable_htlcs point to
        /// still-available channels.
-       fn fail_htlc_backwards_internal(&self, mut channel_state: MutexGuard<ChannelHolder>, payment_hash: &[u8; 32], onion_error: HTLCFailReason) -> bool {
-               let mut pending_htlc = {
-                       match channel_state.claimable_htlcs.remove(payment_hash) {
-                               Some(pending_htlc) => pending_htlc,
-                               None => return false,
-                       }
-               };
-
-               match pending_htlc {
-                       PendingOutboundHTLC::CycledRoute { source_short_channel_id, incoming_packet_shared_secret, route, session_priv } => {
-                               channel_state.claimable_htlcs.insert(payment_hash.clone(), PendingOutboundHTLC::OutboundRoute {
-                                       route,
-                                       session_priv,
-                               });
-                               pending_htlc = PendingOutboundHTLC::IntermediaryHopData { source_short_channel_id, incoming_packet_shared_secret };
-                       },
-                       _ => {}
-               }
-
-               match pending_htlc {
-                       PendingOutboundHTLC::CycledRoute { .. } => unreachable!(),
-                       PendingOutboundHTLC::OutboundRoute { .. } => {
+       fn fail_htlc_backwards_internal(&self, mut channel_state: MutexGuard<ChannelHolder>, source: HTLCSource, payment_hash: &[u8; 32], onion_error: HTLCFailReason) {
+               match source {
+                       HTLCSource::OutboundRoute { .. } => {
                                mem::drop(channel_state);
 
                                let mut pending_events = self.pending_events.lock().unwrap();
                                pending_events.push(events::Event::PaymentFailed {
                                        payment_hash: payment_hash.clone()
                                });
-                               false
                        },
-                       PendingOutboundHTLC::IntermediaryHopData { source_short_channel_id, incoming_packet_shared_secret } => {
+                       HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret }) => {
                                let err_packet = match onion_error {
                                        HTLCFailReason::Reason { failure_code, data } => {
                                                let packet = ChannelManager::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
@@ -1262,17 +1295,17 @@ impl ChannelManager {
                                };
 
                                let (node_id, fail_msgs) = {
-                                       let chan_id = match channel_state.short_to_id.get(&source_short_channel_id) {
+                                       let chan_id = match channel_state.short_to_id.get(&short_channel_id) {
                                                Some(chan_id) => chan_id.clone(),
-                                               None => return false
+                                               None => return
                                        };
 
                                        let chan = channel_state.by_id.get_mut(&chan_id).unwrap();
-                                       match chan.get_update_fail_htlc_and_commit(payment_hash, err_packet) {
+                                       match chan.get_update_fail_htlc_and_commit(htlc_id, err_packet) {
                                                Ok(msg) => (chan.get_their_node_id(), msg),
                                                Err(_e) => {
                                                        //TODO: Do something with e?
-                                                       return false;
+                                                       return;
                                                },
                                        }
                                };
@@ -1299,8 +1332,6 @@ impl ChannelManager {
                                        },
                                        None => {},
                                }
-
-                               true
                        },
                }
        }
@@ -1310,69 +1341,51 @@ impl ChannelManager {
        /// should probably kick the net layer to go send messages if this returns true!
        /// May panic if called except in response to a PaymentReceived event.
        pub fn claim_funds(&self, payment_preimage: [u8; 32]) -> bool {
-               self.claim_funds_internal(payment_preimage, true)
-       }
-       fn claim_funds_internal(&self, payment_preimage: [u8; 32], from_user: bool) -> bool {
                let mut sha = Sha256::new();
                sha.input(&payment_preimage);
                let mut payment_hash = [0; 32];
                sha.result(&mut payment_hash);
 
-               let mut channel_state = self.channel_state.lock().unwrap();
-               let mut pending_htlc = {
-                       match channel_state.claimable_htlcs.remove(&payment_hash) {
-                               Some(pending_htlc) => pending_htlc,
-                               None => return false,
+               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 htlc_with_hash in sources.drain(..) {
+                               if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
+                               self.claim_funds_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc_with_hash), payment_preimage);
                        }
-               };
-
-               match pending_htlc {
-                       PendingOutboundHTLC::CycledRoute { source_short_channel_id, incoming_packet_shared_secret, route, session_priv } => {
-                               if from_user { // This was the end hop back to us
-                                       pending_htlc = PendingOutboundHTLC::IntermediaryHopData { source_short_channel_id, incoming_packet_shared_secret };
-                                       channel_state.claimable_htlcs.insert(payment_hash, PendingOutboundHTLC::OutboundRoute { route, session_priv });
-                               } else { // This came from the first upstream node
-                                       // Bank error in our favor! Maybe we should tell the user this somehow???
-                                       pending_htlc = PendingOutboundHTLC::OutboundRoute { route, session_priv };
-                                       channel_state.claimable_htlcs.insert(payment_hash, PendingOutboundHTLC::IntermediaryHopData { source_short_channel_id, incoming_packet_shared_secret });
-                               }
-                       },
-                       _ => {},
-               }
-
-               match pending_htlc {
-                       PendingOutboundHTLC::CycledRoute { .. } => unreachable!(),
-                       PendingOutboundHTLC::OutboundRoute { .. } => {
-                               if from_user {
-                                       panic!("Called claim_funds with a preimage for an outgoing payment. There is nothing we can do with this, and something is seriously wrong if you knew this...");
-                               }
+                       true
+               } else { false }
+       }
+       fn claim_funds_internal(&self, mut channel_state: MutexGuard<ChannelHolder>, source: HTLCSource, payment_preimage: [u8; 32]) {
+               match source {
+                       HTLCSource::OutboundRoute { .. } => {
                                mem::drop(channel_state);
                                let mut pending_events = self.pending_events.lock().unwrap();
                                pending_events.push(events::Event::PaymentSent {
                                        payment_preimage
                                });
-                               false
                        },
-                       PendingOutboundHTLC::IntermediaryHopData { source_short_channel_id, .. } => {
+                       HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, .. }) => {
+                               //TODO: Delay the claimed_funds relaying just like we do outbound relay!
                                let (node_id, fulfill_msgs) = {
-                                       let chan_id = match channel_state.short_to_id.get(&source_short_channel_id) {
+                                       let chan_id = match channel_state.short_to_id.get(&short_channel_id) {
                                                Some(chan_id) => chan_id.clone(),
                                                None => {
                                                        // TODO: There is probably a channel manager somewhere that needs to
                                                        // learn the preimage as the channel already hit the chain and that's
                                                        // why its missing.
-                                                       return false
+                                                       return
                                                }
                                        };
 
                                        let chan = channel_state.by_id.get_mut(&chan_id).unwrap();
-                                       match chan.get_update_fulfill_htlc_and_commit(payment_preimage) {
+                                       match chan.get_update_fulfill_htlc_and_commit(htlc_id, payment_preimage) {
                                                Ok(msg) => (chan.get_their_node_id(), msg),
                                                Err(_e) => {
                                                        // TODO: There is probably a channel manager somewhere that needs to
                                                        // learn the preimage as the channel may be about to hit the chain.
                                                        //TODO: Do something with e?
-                                                       return false;
+                                                       return
                                                },
                                        }
                                };
@@ -1397,7 +1410,6 @@ impl ChannelManager {
                                                }
                                        });
                                }
-                               true
                        },
                }
        }
@@ -1556,7 +1568,7 @@ impl ChannelManager {
        }
 
        fn internal_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>), MsgHandleErrInternal> {
-               let (res, chan_option) = {
+               let (mut res, chan_option) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = channel_state_lock.borrow_parts();
 
@@ -1577,9 +1589,9 @@ impl ChannelManager {
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                        }
                };
-               for payment_hash in res.2 {
+               for htlc_source in res.2.drain(..) {
                        // unknown_next_peer...I dunno who that is anymore....
-                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
+                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
                }
                if let Some(chan) = chan_option {
                        if let Ok(update) = self.get_channel_update(&chan) {
@@ -1642,38 +1654,10 @@ impl ChannelManager {
                //encrypted with the same key. Its not immediately obvious how to usefully exploit that,
                //but we should prevent it anyway.
 
-               let (mut pending_forward_info, shared_secret, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg);
+               let (pending_forward_info, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg);
                let channel_state = channel_state_lock.borrow_parts();
 
-               let claimable_htlcs_entry = channel_state.claimable_htlcs.entry(msg.payment_hash.clone());
-
-               // We dont correctly handle payments that route through us twice on their way to their
-               // destination. That's OK since those nodes are probably busted or trying to do network
-               // mapping through repeated loops. In either case, we want them to stop talking to us, so
-               // we send permanent_node_failure.
-               let mut will_forward = false;
-               if let PendingHTLCStatus::Forward(PendingForwardHTLCInfo { short_channel_id, .. }) = pending_forward_info {
-                       if let &hash_map::Entry::Occupied(ref e) = &claimable_htlcs_entry {
-                               let mut acceptable_cycle = false;
-                               if let &PendingOutboundHTLC::OutboundRoute { .. } = e.get() {
-                                       acceptable_cycle = short_channel_id == 0;
-                               }
-                               if !acceptable_cycle {
-                                       log_info!(self, "Failed to accept incoming HTLC: Payment looped through us twice");
-                                       pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
-                                               channel_id: msg.channel_id,
-                                               htlc_id: msg.htlc_id,
-                                               reason: ChannelManager::build_first_hop_failure_packet(&shared_secret.unwrap(), 0x4000 | 0x2000 | 2, &[0;0]),
-                                       }));
-                               } else {
-                                       will_forward = true;
-                               }
-                       } else {
-                               will_forward = true;
-                       }
-               }
-
-               let (source_short_channel_id, res) = match channel_state.by_id.get_mut(&msg.channel_id) {
+               match channel_state.by_id.get_mut(&msg.channel_id) {
                        Some(chan) => {
                                if chan.get_their_node_id() != *their_node_id {
                                        //TODO: here MsgHandleErrInternal, #153 case
@@ -1682,66 +1666,31 @@ impl ChannelManager {
                                if !chan.is_usable() {
                                        return Err(MsgHandleErrInternal::from_no_close(HandleError{err: "Channel not yet available for receiving HTLCs", action: Some(msgs::ErrorAction::IgnoreError)}));
                                }
-                               let short_channel_id = chan.get_short_channel_id().unwrap();
-                               if let PendingHTLCStatus::Forward(ref mut forward_info) = pending_forward_info {
-                                       forward_info.prev_short_channel_id = short_channel_id;
-                               }
-                               (short_channel_id, chan.update_add_htlc(&msg, pending_forward_info).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?)
+                               chan.update_add_htlc(&msg, pending_forward_info).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))
                        },
                        None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
-               };
-
-               if will_forward {
-                       match claimable_htlcs_entry {
-                               hash_map::Entry::Occupied(mut e) => {
-                                       let outbound_route = e.get_mut();
-                                       let (route, session_priv) = match outbound_route {
-                                               &mut PendingOutboundHTLC::OutboundRoute { ref route, ref session_priv } => {
-                                                       (route.clone(), session_priv.clone())
-                                               },
-                                               _ => unreachable!(),
-                                       };
-                                       *outbound_route = PendingOutboundHTLC::CycledRoute {
-                                               source_short_channel_id,
-                                               incoming_packet_shared_secret: shared_secret.unwrap(),
-                                               route,
-                                               session_priv,
-                                       };
-                               },
-                               hash_map::Entry::Vacant(e) => {
-                                       e.insert(PendingOutboundHTLC::IntermediaryHopData {
-                                               source_short_channel_id,
-                                               incoming_packet_shared_secret: shared_secret.unwrap(),
-                                       });
-                               }
-                       }
                }
-
-               Ok(res)
        }
 
        fn internal_update_fulfill_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), MsgHandleErrInternal> {
-               //TODO: Delay the claimed_funds relaying just like we do outbound relay!
-               // Claim funds first, cause we don't really care if the channel we received the message on
-               // is broken, we may have enough info to get our own money!
-               self.claim_funds_internal(msg.payment_preimage.clone(), false);
-
                let mut channel_state = self.channel_state.lock().unwrap();
-               match channel_state.by_id.get_mut(&msg.channel_id) {
+               let htlc_source = match channel_state.by_id.get_mut(&msg.channel_id) {
                        Some(chan) => {
                                if chan.get_their_node_id() != *their_node_id {
                                        //TODO: here and below MsgHandleErrInternal, #153 case
                                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                }
-                               chan.update_fulfill_htlc(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))
+                               chan.update_fulfill_htlc(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?.clone()
                        },
                        None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
-               }
+               };
+               self.claim_funds_internal(channel_state, htlc_source, msg.payment_preimage.clone());
+               Ok(())
        }
 
        fn internal_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<Option<msgs::HTLCFailChannelUpdate>, MsgHandleErrInternal> {
                let mut channel_state = self.channel_state.lock().unwrap();
-               let payment_hash = match channel_state.by_id.get_mut(&msg.channel_id) {
+               let htlc_source = match channel_state.by_id.get_mut(&msg.channel_id) {
                        Some(chan) => {
                                if chan.get_their_node_id() != *their_node_id {
                                        //TODO: here and below MsgHandleErrInternal, #153 case
@@ -1752,66 +1701,62 @@ impl ChannelManager {
                        None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                }?;
 
-               if let Some(pending_htlc) = channel_state.claimable_htlcs.get(&payment_hash) {
-                       match pending_htlc {
-                               &PendingOutboundHTLC::OutboundRoute { ref route, ref session_priv } => {
-                                       // Handle packed channel/node updates for passing back for the route handler
-                                       let mut packet_decrypted = msg.reason.data.clone();
-                                       let mut res = None;
-                                       Self::construct_onion_keys_callback(&self.secp_ctx, &route, &session_priv, |shared_secret, _, _, route_hop| {
-                                               if res.is_some() { return; }
-
-                                               let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret);
-
-                                               let mut decryption_tmp = Vec::with_capacity(packet_decrypted.len());
-                                               decryption_tmp.resize(packet_decrypted.len(), 0);
-                                               let mut chacha = ChaCha20::new(&ammag, &[0u8; 8]);
-                                               chacha.process(&packet_decrypted, &mut decryption_tmp[..]);
-                                               packet_decrypted = decryption_tmp;
-
-                                               if let Ok(err_packet) = msgs::DecodedOnionErrorPacket::decode(&packet_decrypted) {
-                                                       if err_packet.failuremsg.len() >= 2 {
-                                                               let um = ChannelManager::gen_um_from_shared_secret(&shared_secret);
-
-                                                               let mut hmac = Hmac::new(Sha256::new(), &um);
-                                                               hmac.input(&err_packet.encode()[32..]);
-                                                               let mut calc_tag = [0u8; 32];
-                                                               hmac.raw_result(&mut calc_tag);
-                                                               if crypto::util::fixed_time_eq(&calc_tag, &err_packet.hmac) {
-                                                                       const UNKNOWN_CHAN: u16 = 0x4000|10;
-                                                                       const TEMP_CHAN_FAILURE: u16 = 0x4000|7;
-                                                                       match byte_utils::slice_to_be16(&err_packet.failuremsg[0..2]) {
-                                                                               TEMP_CHAN_FAILURE => {
-                                                                                       if err_packet.failuremsg.len() >= 4 {
-                                                                                               let update_len = byte_utils::slice_to_be16(&err_packet.failuremsg[2..4]) as usize;
-                                                                                               if err_packet.failuremsg.len() >= 4 + update_len {
-                                                                                                       if let Ok(chan_update) = msgs::ChannelUpdate::decode(&err_packet.failuremsg[4..4 + update_len]) {
-                                                                                                               res = Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage {
-                                                                                                                       msg: chan_update,
-                                                                                                               });
-                                                                                                       }
+               match htlc_source {
+                       &HTLCSource::OutboundRoute { ref route, ref session_priv, .. } => {
+                               // Handle packed channel/node updates for passing back for the route handler
+                               let mut packet_decrypted = msg.reason.data.clone();
+                               let mut res = None;
+                               Self::construct_onion_keys_callback(&self.secp_ctx, &route, &session_priv, |shared_secret, _, _, route_hop| {
+                                       if res.is_some() { return; }
+
+                                       let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret);
+
+                                       let mut decryption_tmp = Vec::with_capacity(packet_decrypted.len());
+                                       decryption_tmp.resize(packet_decrypted.len(), 0);
+                                       let mut chacha = ChaCha20::new(&ammag, &[0u8; 8]);
+                                       chacha.process(&packet_decrypted, &mut decryption_tmp[..]);
+                                       packet_decrypted = decryption_tmp;
+
+                                       if let Ok(err_packet) = msgs::DecodedOnionErrorPacket::decode(&packet_decrypted) {
+                                               if err_packet.failuremsg.len() >= 2 {
+                                                       let um = ChannelManager::gen_um_from_shared_secret(&shared_secret);
+
+                                                       let mut hmac = Hmac::new(Sha256::new(), &um);
+                                                       hmac.input(&err_packet.encode()[32..]);
+                                                       let mut calc_tag = [0u8; 32];
+                                                       hmac.raw_result(&mut calc_tag);
+                                                       if crypto::util::fixed_time_eq(&calc_tag, &err_packet.hmac) {
+                                                               const UNKNOWN_CHAN: u16 = 0x4000|10;
+                                                               const TEMP_CHAN_FAILURE: u16 = 0x4000|7;
+                                                               match byte_utils::slice_to_be16(&err_packet.failuremsg[0..2]) {
+                                                                       TEMP_CHAN_FAILURE => {
+                                                                               if err_packet.failuremsg.len() >= 4 {
+                                                                                       let update_len = byte_utils::slice_to_be16(&err_packet.failuremsg[2..4]) as usize;
+                                                                                       if err_packet.failuremsg.len() >= 4 + update_len {
+                                                                                               if let Ok(chan_update) = msgs::ChannelUpdate::decode(&err_packet.failuremsg[4..4 + update_len]) {
+                                                                                                       res = Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage {
+                                                                                                               msg: chan_update,
+                                                                                                       });
                                                                                                }
                                                                                        }
-                                                                               },
-                                                                               UNKNOWN_CHAN => {
-                                                                                       // No such next-hop. We know this came from the
-                                                                                       // current node as the HMAC validated.
-                                                                                       res = Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
-                                                                                               short_channel_id: route_hop.short_channel_id
-                                                                                       });
-                                                                               },
-                                                                               _ => {}, //TODO: Enumerate all of these!
-                                                                       }
+                                                                               }
+                                                                       },
+                                                                       UNKNOWN_CHAN => {
+                                                                               // No such next-hop. We know this came from the
+                                                                               // current node as the HMAC validated.
+                                                                               res = Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
+                                                                                       short_channel_id: route_hop.short_channel_id
+                                                                               });
+                                                                       },
+                                                                       _ => {}, //TODO: Enumerate all of these!
                                                                }
                                                        }
                                                }
-                                       }).unwrap();
-                                       Ok(res)
-                               },
-                               _ => { Ok(None) },
-                       }
-               } else {
-                       Ok(None)
+                                       }
+                               }).unwrap();
+                               Ok(res)
+                       },
+                       _ => { Ok(None) },
                }
        }
 
@@ -1823,7 +1768,8 @@ impl ChannelManager {
                                        //TODO: here and below MsgHandleErrInternal, #153 case
                                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                }
-                               chan.update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() }).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))
+                               chan.update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() }).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                               Ok(())
                        },
                        None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                }
@@ -1851,7 +1797,7 @@ impl ChannelManager {
        }
 
        fn internal_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<Option<msgs::CommitmentUpdate>, MsgHandleErrInternal> {
-               let (res, mut pending_forwards, mut pending_failures, chan_monitor) = {
+               let ((res, mut pending_forwards, mut pending_failures, chan_monitor), short_channel_id) = {
                        let mut channel_state = self.channel_state.lock().unwrap();
                        match channel_state.by_id.get_mut(&msg.channel_id) {
                                Some(chan) => {
@@ -1859,7 +1805,7 @@ impl ChannelManager {
                                                //TODO: here and below MsgHandleErrInternal, #153 case
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                        }
-                                       chan.revoke_and_ack(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?
+                                       (chan.revoke_and_ack(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?, chan.get_short_channel_id().expect("RAA should only work on a short-id-available channel"))
                                },
                                None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                        }
@@ -1868,7 +1814,7 @@ impl ChannelManager {
                        unimplemented!();
                }
                for failure in pending_failures.drain(..) {
-                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &failure.0, failure.1);
+                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), failure.0, &failure.1, failure.2);
                }
 
                let mut forward_event = None;
@@ -1878,13 +1824,13 @@ impl ChannelManager {
                                forward_event = Some(Instant::now() + Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64));
                                channel_state.next_forward = forward_event.unwrap();
                        }
-                       for forward_info in pending_forwards.drain(..) {
+                       for (forward_info, prev_htlc_id) in pending_forwards.drain(..) {
                                match channel_state.forward_htlcs.entry(forward_info.short_channel_id) {
                                        hash_map::Entry::Occupied(mut entry) => {
-                                               entry.get_mut().push(forward_info);
+                                               entry.get_mut().push(HTLCForwardInfo { prev_short_channel_id: short_channel_id, prev_htlc_id, forward_info });
                                        },
                                        hash_map::Entry::Vacant(entry) => {
-                                               entry.insert(vec!(forward_info));
+                                               entry.insert(vec!(HTLCForwardInfo { prev_short_channel_id: short_channel_id, prev_htlc_id, forward_info }));
                                        }
                                }
                        }
@@ -2266,8 +2212,10 @@ mod tests {
 
        use rand::{thread_rng,Rng};
 
+       use std::cell::RefCell;
        use std::collections::HashMap;
        use std::default::Default;
+       use std::rc::Rc;
        use std::sync::{Arc, Mutex};
        use std::time::Instant;
        use std::mem;
@@ -2438,9 +2386,10 @@ mod tests {
                chan_monitor: Arc<test_utils::TestChannelMonitor>,
                node: Arc<ChannelManager>,
                router: Router,
+               network_payment_count: Rc<RefCell<u8>>,
+               network_chan_count: Rc<RefCell<u32>>,
        }
 
-       static mut CHAN_COUNT: u32 = 0;
        fn create_chan_between_nodes(node_a: &Node, node_b: &Node) -> (msgs::ChannelAnnouncement, msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
                node_a.node.create_channel(node_b.node.get_our_node_id(), 100000, 10001, 42).unwrap();
 
@@ -2456,7 +2405,7 @@ mod tests {
 
                node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &accept_chan).unwrap();
 
-               let chan_id = unsafe { CHAN_COUNT };
+               let chan_id = *node_a.network_chan_count.borrow();
                let tx;
                let funding_output;
 
@@ -2562,9 +2511,7 @@ mod tests {
                        _ => panic!("Unexpected event"),
                };
 
-               unsafe {
-                       CHAN_COUNT += 1;
-               }
+               *node_a.network_chan_count.borrow_mut() += 1;
 
                ((*announcement).clone(), (*as_update).clone(), (*bs_update).clone(), channel_id, tx)
        }
@@ -2666,10 +2613,9 @@ mod tests {
                }
        }
 
-       static mut PAYMENT_COUNT: u8 = 0;
        fn send_along_route(origin_node: &Node, route: Route, expected_route: &[&Node], recv_value: u64) -> ([u8; 32], [u8; 32]) {
-               let our_payment_preimage = unsafe { [PAYMENT_COUNT; 32] };
-               unsafe { PAYMENT_COUNT += 1 };
+               let our_payment_preimage = [*origin_node.network_payment_count.borrow(); 32];
+               *origin_node.network_payment_count.borrow_mut() += 1;
                let our_payment_hash = {
                        let mut sha = Sha256::new();
                        sha.input(&our_payment_preimage[..]);
@@ -2861,8 +2807,8 @@ mod tests {
                        assert_eq!(hop.pubkey, node.node.get_our_node_id());
                }
 
-               let our_payment_preimage = unsafe { [PAYMENT_COUNT; 32] };
-               unsafe { PAYMENT_COUNT += 1 };
+               let our_payment_preimage = [*origin_node.network_payment_count.borrow(); 32];
+               *origin_node.network_payment_count.borrow_mut() += 1;
                let our_payment_hash = {
                        let mut sha = Sha256::new();
                        sha.input(&our_payment_preimage[..]);
@@ -2973,6 +2919,9 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let logger: Arc<Logger> = Arc::new(test_utils::TestLogger::new());
 
+               let chan_count = Rc::new(RefCell::new(0));
+               let payment_count = Rc::new(RefCell::new(0));
+
                for _ in 0..node_count {
                        let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
                        let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger)));
@@ -2985,7 +2934,10 @@ mod tests {
                        };
                        let node = ChannelManager::new(node_id.clone(), 0, true, Network::Testnet, feeest.clone(), chan_monitor.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger)).unwrap();
                        let router = Router::new(PublicKey::from_secret_key(&secp_ctx, &node_id), chain_monitor.clone(), Arc::clone(&logger));
-                       nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router });
+                       nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router,
+                               network_payment_count: payment_count.clone(),
+                               network_chan_count: chan_count.clone(),
+                       });
                }
 
                nodes
@@ -3109,6 +3061,32 @@ mod tests {
                }
        }
 
+       #[test]
+       fn duplicate_htlc_test() {
+               // Test that we accept duplicate payment_hash HTLCs across the network and that
+               // claiming/failing them are all separate and don't effect each other
+               let mut nodes = create_network(6);
+
+               // Create some initial channels to route via 3 to 4/5 from 0/1/2
+               create_announced_chan_between_nodes(&nodes, 0, 3);
+               create_announced_chan_between_nodes(&nodes, 1, 3);
+               create_announced_chan_between_nodes(&nodes, 2, 3);
+               create_announced_chan_between_nodes(&nodes, 3, 4);
+               create_announced_chan_between_nodes(&nodes, 3, 5);
+
+               let (payment_preimage, payment_hash) = route_payment(&nodes[0], &vec!(&nodes[3], &nodes[4])[..], 1000000);
+
+               *nodes[0].network_payment_count.borrow_mut() -= 1;
+               assert_eq!(route_payment(&nodes[1], &vec!(&nodes[3])[..], 1000000).0, payment_preimage);
+
+               *nodes[0].network_payment_count.borrow_mut() -= 1;
+               assert_eq!(route_payment(&nodes[2], &vec!(&nodes[3], &nodes[5])[..], 1000000).0, payment_preimage);
+
+               claim_payment(&nodes[0], &vec!(&nodes[3], &nodes[4])[..], payment_preimage);
+               fail_payment(&nodes[2], &vec!(&nodes[3], &nodes[5])[..], payment_hash);
+               claim_payment(&nodes[1], &vec!(&nodes[3])[..], payment_preimage);
+       }
+
        #[derive(PartialEq)]
        enum HTLCType { NONE, TIMEOUT, SUCCESS }
        fn test_txn_broadcast(node: &Node, chan: &(msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction), commitment_tx: Option<Transaction>, has_htlc_tx: HTLCType) -> Vec<Transaction> {