From c220a5c5cf39c65268c2146d84129873ee20fa2d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 20 Dec 2018 16:15:07 -0500 Subject: [PATCH] Add an enum option to pending forwards to fail backwards --- src/ln/channel.rs | 1 + src/ln/channelmanager.rs | 54 ++++++++++++++++++++++++++++++++++++-- src/ln/functional_tests.rs | 2 ++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/ln/channel.rs b/src/ln/channel.rs index e9866703b..64db97f00 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -3125,6 +3125,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 + /// If an Err is returned, its a ChannelError::Ignore! pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result, ChannelError> { if (self.channel_state & (ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelFunded as u32) { return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down")); diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index f49971ef6..429dc738f 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -214,6 +214,10 @@ pub(super) enum HTLCForwardInfo { prev_htlc_id: u64, forward_info: PendingForwardHTLCInfo, }, + FailHTLC { + htlc_id: u64, + err_packet: msgs::OnionErrorPacket, + }, } /// For events which result in both a RevokeAndACK and a CommitmentUpdate, by default they should @@ -1170,6 +1174,12 @@ impl ChannelManager { }); failed_forwards.push((htlc_source, forward_info.payment_hash, 0x4000 | 10, None)); }, + HTLCForwardInfo::FailHTLC { .. } => { + // Channel went away before we could fail it. This implies + // the channel is now on chain and our counterparty is + // trying to broadcast the HTLC-Timeout, but that's their + // problem, not ours. + } } } continue; @@ -1178,6 +1188,7 @@ impl ChannelManager { let forward_chan = &mut channel_state.by_id.get_mut(&forward_chan_id).unwrap(); let mut add_htlc_msgs = Vec::new(); + let mut fail_htlc_msgs = Vec::new(); for forward_info in pending_forwards.drain(..) { match forward_info { HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info } => { @@ -1187,7 +1198,10 @@ impl ChannelManager { 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) => { + Err(e) => { + if let ChannelError::Ignore(_) = e {} else { + panic!("Stated return value requirements in send_htlc() were not met"); + } let chan_update = self.get_channel_update(forward_chan).unwrap(); failed_forwards.push((htlc_source, forward_info.payment_hash, 0x1000 | 7, Some(chan_update))); continue; @@ -1208,6 +1222,30 @@ impl ChannelManager { } } }, + HTLCForwardInfo::FailHTLC { htlc_id, err_packet } => { + match forward_chan.get_update_fail_htlc(htlc_id, err_packet) { + Err(e) => { + if let ChannelError::Ignore(_) = e {} else { + panic!("Stated return value requirements in get_update_fail_htlc() were not met"); + } + // fail-backs are best-effort, we probably already have one + // pending, and if not that's OK, if not, the channel is on + // the chain and sending the HTLC-Timeout is their problem. + continue; + }, + Ok(Some(msg)) => { fail_htlc_msgs.push(msg); }, + Ok(None) => { + // Nothing to do here...we're waiting on a remote + // revoke_and_ack before we can update the commitment + // transaction. The Channel will automatically handle + // building the update_fail_htlc and commitment_signed + // messages when we can. + // We don't need any kind of timer here as they should fail + // the channel onto the chain if they can't get our + // update_fail_htlc in time, its not our problem. + } + } + }, } } @@ -1230,7 +1268,7 @@ impl ChannelManager { updates: msgs::CommitmentUpdate { update_add_htlcs: add_htlc_msgs, update_fulfill_htlcs: Vec::new(), - update_fail_htlcs: Vec::new(), + update_fail_htlcs: fail_htlc_msgs, update_fail_malformed_htlcs: Vec::new(), update_fee: None, commitment_signed: commitment_msg, @@ -1255,6 +1293,9 @@ impl ChannelManager { amt: forward_info.amt_to_forward, }); }, + HTLCForwardInfo::FailHTLC { .. } => { + panic!("Got pending fail of our own HTLC"); + } } } } @@ -2734,6 +2775,11 @@ impl Writeable for HTLCForwardInfo { prev_htlc_id.write(writer)?; forward_info.write(writer)?; }, + &HTLCForwardInfo::FailHTLC { ref htlc_id, ref err_packet } => { + 1u8.write(writer)?; + htlc_id.write(writer)?; + err_packet.write(writer)?; + }, } Ok(()) } @@ -2747,6 +2793,10 @@ impl Readable for HTLCForwardInfo { prev_htlc_id: Readable::read(reader)?, forward_info: Readable::read(reader)?, }), + 1 => Ok(HTLCForwardInfo::FailHTLC { + htlc_id: Readable::read(reader)?, + err_packet: Readable::read(reader)?, + }), _ => Err(DecodeError::InvalidValue), } } diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index dde350090..a68d065be 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -6263,6 +6263,7 @@ fn test_onion_failure() { match f { &mut HTLCForwardInfo::AddHTLC { ref mut forward_info, .. } => forward_info.outgoing_cltv_value += 1, + _ => {}, } } } @@ -6275,6 +6276,7 @@ fn test_onion_failure() { match f { &mut HTLCForwardInfo::AddHTLC { ref mut forward_info, .. } => forward_info.amt_to_forward -= 1, + _ => {}, } } } -- 2.39.5