Time out incoming HTLCs when we reach cltv_expiry (+ test)
authorMatt Corallo <git@bluematt.me>
Thu, 9 Jan 2020 19:09:25 +0000 (14:09 -0500)
committerMatt Corallo <git@bluematt.me>
Sat, 25 Jan 2020 22:12:08 +0000 (17:12 -0500)
We only do this for incoming HTLCs directly as we rely on channel
closure and HTLC-Timeout broadcast to fail any HTLCs which we
relayed onwards where our next-hop doesn't update_fail in time.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/events.rs

index deb4eccddb6151481a029148b3738568f4068e81..eb1fabecca1ae0d8cbdffd838ac3cc0049ff1f33 100644 (file)
@@ -76,6 +76,7 @@ enum PendingForwardReceiveHTLCInfo {
        },
        Receive {
                payment_data: Option<msgs::FinalOnionHopData>,
+               incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed
        },
 }
 
@@ -125,6 +126,7 @@ struct ClaimableHTLC {
        src: HTLCPreviousHopData,
        value: u64,
        payment_data: Option<msgs::FinalOnionHopData>,
+       cltv_expiry: u32,
 }
 
 /// Tracks the inbound corresponding to an outbound HTLC
@@ -1013,7 +1015,10 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
                                // delay) once they've send us a commitment_signed!
 
                                PendingHTLCStatus::Forward(PendingHTLCInfo {
-                                       type_data: PendingForwardReceiveHTLCInfo::Receive { payment_data },
+                                       type_data: PendingForwardReceiveHTLCInfo::Receive {
+                                               payment_data,
+                                               incoming_cltv_expiry: msg.cltv_expiry,
+                                       },
                                        payment_hash: msg.payment_hash.clone(),
                                        incoming_shared_secret: shared_secret,
                                        amt_to_forward: next_hop_data.amt_to_forward,
@@ -1621,7 +1626,7 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
                                        for forward_info in pending_forwards.drain(..) {
                                                match forward_info {
                                                        HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
-                                                                       type_data: PendingForwardReceiveHTLCInfo::Receive { payment_data },
+                                                                       type_data: PendingForwardReceiveHTLCInfo::Receive { payment_data, incoming_cltv_expiry },
                                                                        incoming_shared_secret, payment_hash, amt_to_forward, .. }, } => {
                                                                let prev_hop_data = HTLCPreviousHopData {
                                                                        short_channel_id: prev_short_channel_id,
@@ -1637,6 +1642,7 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
                                                                        src: prev_hop_data,
                                                                        value: amt_to_forward,
                                                                        payment_data: payment_data.clone(),
+                                                                       cltv_expiry: incoming_cltv_expiry,
                                                                });
                                                                if let &Some(ref data) = &payment_data {
                                                                        for htlc in htlcs.iter() {
@@ -2789,6 +2795,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send> ChainListener for ChannelM
                log_trace!(self, "Block {} at height {} connected with {} txn matched", header_hash, height, txn_matched.len());
                let _ = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
+               let mut timed_out_htlcs = Vec::new();
                {
                        let mut channel_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_lock;
@@ -2855,10 +2862,29 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send> ChainListener for ChannelM
                                }
                                true
                        });
+
+                       channel_state.claimable_htlcs.retain(|&(ref payment_hash, _), htlcs| {
+                               htlcs.retain(|htlc| {
+                                       if height >= htlc.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS {
+                                               timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.src.clone()), payment_hash.clone(), htlc.value));
+                                               false
+                                       } else { true }
+                               });
+                               !htlcs.is_empty()
+                       });
                }
                for failure in failed_channels.drain(..) {
                        self.finish_force_close_channel(failure);
                }
+
+               for (source, payment_hash, value) in timed_out_htlcs.drain(..) {
+                       // Call it preimage_unknown as the issue, ultimately, is that the user failed to
+                       // provide us a preimage within the cltv_expiry time window.
+                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), source, &payment_hash, HTLCFailReason::Reason {
+                               failure_code: 0x4000 | 15,
+                               data: byte_utils::be64_to_array(value).to_vec()
+                       });
+               }
                self.latest_block_height.store(height as usize, Ordering::Release);
                *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header_hash;
        }
@@ -3195,9 +3221,10 @@ impl Writeable for PendingHTLCInfo {
                                onion_packet.write(writer)?;
                                short_channel_id.write(writer)?;
                        },
-                       &PendingForwardReceiveHTLCInfo::Receive { ref payment_data } => {
+                       &PendingForwardReceiveHTLCInfo::Receive { ref payment_data, ref incoming_cltv_expiry } => {
                                1u8.write(writer)?;
                                payment_data.write(writer)?;
+                               incoming_cltv_expiry.write(writer)?;
                        },
                }
                self.incoming_shared_secret.write(writer)?;
@@ -3218,6 +3245,7 @@ impl<R: ::std::io::Read> Readable<R> for PendingHTLCInfo {
                                },
                                1u8 => PendingForwardReceiveHTLCInfo::Receive {
                                        payment_data: Readable::read(reader)?,
+                                       incoming_cltv_expiry: Readable::read(reader)?,
                                },
                                _ => return Err(DecodeError::InvalidValue),
                        },
@@ -3427,6 +3455,7 @@ impl<ChanSigner: ChannelKeys + Writeable, M: Deref> Writeable for ChannelManager
                                htlc.src.write(writer)?;
                                htlc.value.write(writer)?;
                                htlc.payment_data.write(writer)?;
+                               htlc.cltv_expiry.write(writer)?;
                        }
                }
 
@@ -3572,6 +3601,7 @@ impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>, M: Deref> R
                                        src: Readable::read(reader)?,
                                        value: Readable::read(reader)?,
                                        payment_data: Readable::read(reader)?,
+                                       cltv_expiry: Readable::read(reader)?,
                                });
                        }
                        claimable_htlcs.insert(payment_hash, previous_hops);
index a1f85ab4ca0780c8f77100c5fa65be6d8e9322dd..f8e84ea9ce6599a677869a87835500b6251a1d59 100644 (file)
@@ -2152,6 +2152,15 @@ fn claim_htlc_outputs_single_tx() {
                let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
                nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200);
                nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200);
+
+               // Expect pending failures, but we don't bother trying to update the channel state with them.
+               let events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 1);
+               match events[0] {
+                       Event::PendingHTLCsForwardable { .. } => { },
+                       _ => panic!("Unexpected event"),
+               };
+
                connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash());
 
                let events = nodes[1].node.get_and_clear_pending_events();
@@ -3406,6 +3415,48 @@ fn test_drop_messages_peer_disconnect_dual_htlc() {
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
 }
 
+#[test]
+fn test_htlc_timeout() {
+       // If the user fails to claim/fail an HTLC within the HTLC CLTV timeout we fail it for them
+       // to avoid our counterparty failing the channel.
+       let node_cfgs = create_node_cfgs(2);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::supported(), InitFeatures::supported());
+       let (_, our_payment_hash) = route_payment(&nodes[0], &[&nodes[1]], 100000);
+
+       let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       nodes[0].block_notifier.block_connected_checked(&header, 101, &[], &[]);
+       nodes[1].block_notifier.block_connected_checked(&header, 101, &[], &[]);
+       for i in 102..TEST_FINAL_CLTV + 100 + 1 - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS {
+               header.prev_blockhash = header.bitcoin_hash();
+               nodes[0].block_notifier.block_connected_checked(&header, i, &[], &[]);
+               nodes[1].block_notifier.block_connected_checked(&header, i, &[], &[]);
+       }
+
+       expect_pending_htlcs_forwardable!(nodes[1]);
+
+       check_added_monitors!(nodes[1], 1);
+       let htlc_timeout_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+       assert!(htlc_timeout_updates.update_add_htlcs.is_empty());
+       assert_eq!(htlc_timeout_updates.update_fail_htlcs.len(), 1);
+       assert!(htlc_timeout_updates.update_fail_malformed_htlcs.is_empty());
+       assert!(htlc_timeout_updates.update_fee.is_none());
+
+       nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_timeout_updates.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[1], htlc_timeout_updates.commitment_signed, false);
+       let events = nodes[0].node.get_and_clear_pending_events();
+       match events[0] {
+               Event::PaymentFailed { payment_hash, rejected_by_dest, error_code } => {
+                       assert_eq!(payment_hash, our_payment_hash);
+                       assert!(rejected_by_dest);
+                       assert_eq!(error_code.unwrap(), 0x4000 | 15);
+               },
+               _ => panic!("Unexpected event"),
+       }
+}
+
 #[test]
 fn test_invalid_channel_announcement() {
        //Test BOLT 7 channel_announcement msg requirement for final node, gather data to build customed channel_announcement msgs
@@ -6659,6 +6710,15 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
 
        // Broadcast set of revoked txn on A
        let header_128 = connect_blocks(&nodes[0].block_notifier, 128, 0,  true, header.bitcoin_hash());
+
+       // Expect pending failures, but we don't bother trying to update the channel state with them.
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
+               Event::PendingHTLCsForwardable { .. } => { },
+               _ => panic!("Unexpected event"),
+       };
+
        let header_129 = BlockHeader { version: 0x20000000, prev_blockhash: header_128, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[0].block_notifier.block_connected(&Block { header: header_129, txdata: vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone(), revoked_htlc_txn[1].clone()] }, 129);
        let first;
@@ -6990,6 +7050,15 @@ fn test_bump_txn_sanitize_tracking_maps() {
 
        // Broadcast set of revoked txn on A
        let header_128 = connect_blocks(&nodes[0].block_notifier, 128, 0,  false, Default::default());
+
+       // Expect pending failures, but we don't bother trying to update the channel state with them.
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
+               Event::PendingHTLCsForwardable { .. } => { },
+               _ => panic!("Unexpected event"),
+       };
+
        let header_129 = BlockHeader { version: 0x20000000, prev_blockhash: header_128, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[0].block_notifier.block_connected(&Block { header: header_129, txdata: vec![revoked_local_txn[0].clone()] }, 129);
        check_closed_broadcast!(nodes[0], false);
index 3d0b680ace45ba6a4ab6ac18b23b282f0e0478ed..b3e85bad069bca8737d5dae690d7927a5f2960dd 100644 (file)
@@ -57,6 +57,9 @@ pub enum Event {
        /// ChannelManager::fail_htlc_backwards to free up resources for this HTLC.
        /// The amount paid should be considered 'incorrect' when it is less than or more than twice
        /// the amount expected.
+       /// If you fail to call either ChannelManager::claim_funds of
+       /// ChannelManager::fail_htlc_backwards within the HTLC's timeout, the HTLC will be
+       /// automatically failed with PaymentFailReason::PreimageUnknown.
        PaymentReceived {
                /// The hash for which the preimage should be handed to the ChannelManager.
                payment_hash: PaymentHash,