Expose `{prev,next}_user_channel_id` fields in `PaymentForwarded`
authorElias Rohrer <dev@tnull.de>
Thu, 7 Mar 2024 09:37:08 +0000 (10:37 +0100)
committerElias Rohrer <dev@tnull.de>
Tue, 19 Mar 2024 12:29:09 +0000 (13:29 +0100)
This is useful for users that track channels by `user_channel_id`.

For example, in `lightning-liquidity` we currently keep a full
`HashMap<ChanelId, u128>` around *just* to be able to associate
`PaymentForwarded` events with the channels otherwise tracked by
`user_channel_id`.

lightning/src/events/mod.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs

index 485a23e0292e520c28d47a226fe7de51abdd9690..fcd37914e3dd99d6340925629cb5d3dfa7553c7a 100644 (file)
@@ -797,12 +797,24 @@ pub enum Event {
        /// This event is generated when a payment has been successfully forwarded through us and a
        /// forwarding fee earned.
        PaymentForwarded {
-               /// The incoming channel between the previous node and us. This is only `None` for events
-               /// generated or serialized by versions prior to 0.0.107.
+               /// The channel id of the incoming channel between the previous node and us.
+               ///
+               /// This is only `None` for events generated or serialized by versions prior to 0.0.107.
                prev_channel_id: Option<ChannelId>,
-               /// The outgoing channel between the next node and us. This is only `None` for events
-               /// generated or serialized by versions prior to 0.0.107.
+               /// The channel id of the outgoing channel between the next node and us.
+               ///
+               /// This is only `None` for events generated or serialized by versions prior to 0.0.107.
                next_channel_id: Option<ChannelId>,
+               /// The `user_channel_id` of the incoming channel between the previous node and us.
+               ///
+               /// This is only `None` for events generated or serialized by versions prior to 0.0.122.
+               prev_user_channel_id: Option<u128>,
+               /// The `user_channel_id` of the outgoing channel between the next node and us.
+               ///
+               /// This will be `None` if the payment was settled via an on-chain transaction. See the
+               /// caveat described for the `total_fee_earned_msat` field. Moreover it will be `None` for
+               /// events generated or serialized by versions prior to 0.0.122.
+               next_user_channel_id: Option<u128>,
                /// The total fee, in milli-satoshis, which was earned as a result of the payment.
                ///
                /// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC
@@ -1121,8 +1133,9 @@ impl Writeable for Event {
                                });
                        }
                        &Event::PaymentForwarded {
-                               total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx,
-                               next_channel_id, outbound_amount_forwarded_msat, skimmed_fee_msat,
+                               prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id,
+                               total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx,
+                               outbound_amount_forwarded_msat,
                        } => {
                                7u8.write(writer)?;
                                write_tlv_fields!(writer, {
@@ -1132,6 +1145,8 @@ impl Writeable for Event {
                                        (3, next_channel_id, option),
                                        (5, outbound_amount_forwarded_msat, option),
                                        (7, skimmed_fee_msat, option),
+                                       (9, prev_user_channel_id, option),
+                                       (11, next_user_channel_id, option),
                                });
                        },
                        &Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason,
@@ -1427,12 +1442,14 @@ impl MaybeReadable for Event {
                        },
                        7u8 => {
                                let f = || {
-                                       let mut total_fee_earned_msat = None;
                                        let mut prev_channel_id = None;
-                                       let mut claim_from_onchain_tx = false;
                                        let mut next_channel_id = None;
-                                       let mut outbound_amount_forwarded_msat = None;
+                                       let mut prev_user_channel_id = None;
+                                       let mut next_user_channel_id = None;
+                                       let mut total_fee_earned_msat = None;
                                        let mut skimmed_fee_msat = None;
+                                       let mut claim_from_onchain_tx = false;
+                                       let mut outbound_amount_forwarded_msat = None;
                                        read_tlv_fields!(reader, {
                                                (0, total_fee_earned_msat, option),
                                                (1, prev_channel_id, option),
@@ -1440,10 +1457,13 @@ impl MaybeReadable for Event {
                                                (3, next_channel_id, option),
                                                (5, outbound_amount_forwarded_msat, option),
                                                (7, skimmed_fee_msat, option),
+                                               (9, prev_user_channel_id, option),
+                                               (11, next_user_channel_id, option),
                                        });
                                        Ok(Some(Event::PaymentForwarded {
-                                               total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id,
-                                               outbound_amount_forwarded_msat, skimmed_fee_msat,
+                                               prev_channel_id, next_channel_id, prev_user_channel_id,
+                                               next_user_channel_id, total_fee_earned_msat, skimmed_fee_msat,
+                                               claim_from_onchain_tx, outbound_amount_forwarded_msat,
                                        }))
                                };
                                f()
index abfca35f9724765675206a2b903b4683aa746758..78fa47e9272cc735dbe709c1d8cc9286d7b26c19 100644 (file)
@@ -5733,7 +5733,7 @@ where
        fn claim_funds_internal(&self, source: HTLCSource, payment_preimage: PaymentPreimage,
                forwarded_htlc_value_msat: Option<u64>, skimmed_fee_msat: Option<u64>, from_onchain: bool,
                startup_replay: bool, next_channel_counterparty_node_id: Option<PublicKey>,
-               next_channel_outpoint: OutPoint, next_channel_id: ChannelId,
+               next_channel_outpoint: OutPoint, next_channel_id: ChannelId, next_user_channel_id: Option<u128>,
        ) {
                match source {
                        HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
@@ -5752,6 +5752,7 @@ where
                        },
                        HTLCSource::PreviousHopData(hop_data) => {
                                let prev_channel_id = hop_data.channel_id;
+                               let prev_user_channel_id = hop_data.user_channel_id;
                                let completed_blocker = RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data);
                                #[cfg(debug_assertions)]
                                let claiming_chan_funding_outpoint = hop_data.outpoint;
@@ -5838,12 +5839,14 @@ where
                                                                "skimmed_fee_msat must always be included in total_fee_earned_msat");
                                                        Some(MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel {
                                                                event: events::Event::PaymentForwarded {
-                                                                       total_fee_earned_msat,
-                                                                       claim_from_onchain_tx: from_onchain,
                                                                        prev_channel_id: Some(prev_channel_id),
                                                                        next_channel_id: Some(next_channel_id),
-                                                                       outbound_amount_forwarded_msat: forwarded_htlc_value_msat,
+                                                                       prev_user_channel_id,
+                                                                       next_user_channel_id,
+                                                                       total_fee_earned_msat,
                                                                        skimmed_fee_msat,
+                                                                       claim_from_onchain_tx: from_onchain,
+                                                                       outbound_amount_forwarded_msat: forwarded_htlc_value_msat,
                                                                },
                                                                downstream_counterparty_and_funding_outpoint: chan_to_release,
                                                        })
@@ -6817,6 +6820,7 @@ where
 
        fn internal_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), MsgHandleErrInternal> {
                let funding_txo;
+               let next_user_channel_id;
                let (htlc_source, forwarded_htlc_value, skimmed_fee_msat) = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let peer_state_mutex = per_peer_state.get(counterparty_node_id)
@@ -6846,6 +6850,7 @@ where
                                                // outbound HTLC is claimed. This is guaranteed to all complete before we
                                                // process the RAA as messages are processed from single peers serially.
                                                funding_txo = chan.context.get_funding_txo().expect("We won't accept a fulfill until funded");
+                                               next_user_channel_id = chan.context.get_user_id();
                                                res
                                        } else {
                                                return try_chan_phase_entry!(self, Err(ChannelError::Close(
@@ -6857,7 +6862,7 @@ where
                };
                self.claim_funds_internal(htlc_source, msg.payment_preimage.clone(),
                        Some(forwarded_htlc_value), skimmed_fee_msat, false, false, Some(*counterparty_node_id),
-                       funding_txo, msg.channel_id
+                       funding_txo, msg.channel_id, Some(next_user_channel_id),
                );
 
                Ok(())
@@ -7359,7 +7364,7 @@ where
                                                        log_trace!(logger, "Claiming HTLC with preimage {} from our monitor", preimage);
                                                        self.claim_funds_internal(htlc_update.source, preimage,
                                                                htlc_update.htlc_value_satoshis.map(|v| v * 1000), None, true,
-                                                               false, counterparty_node_id, funding_outpoint, channel_id);
+                                                               false, counterparty_node_id, funding_outpoint, channel_id, None);
                                                } else {
                                                        log_trace!(logger, "Failing HTLC with hash {} from our monitor", &htlc_update.payment_hash);
                                                        let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id };
@@ -11319,7 +11324,9 @@ where
                        // don't remember in the `ChannelMonitor` where we got a preimage from, but if the
                        // channel is closed we just assume that it probably came from an on-chain claim.
                        channel_manager.claim_funds_internal(source, preimage, Some(downstream_value), None,
-                               downstream_closed, true, downstream_node_id, downstream_funding, downstream_channel_id);
+                               downstream_closed, true, downstream_node_id, downstream_funding,
+                               downstream_channel_id, None
+                       );
                }
 
                //TODO: Broadcast channel update for closed channels, but only after we've made a
index adf2768b4152700ed82172e9a44ab75c6c3475a2..0c8909b56850d670874297da23b299b93a3490ad 100644 (file)
@@ -2230,8 +2230,8 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
 ) {
        match event {
                Event::PaymentForwarded {
-                       total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id,
-                       outbound_amount_forwarded_msat: _, skimmed_fee_msat
+                       prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id,
+                       total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, ..
                } => {
                        assert_eq!(total_fee_earned_msat, expected_fee);
 
@@ -2240,12 +2240,31 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
                        assert!(skimmed_fee_msat == expected_extra_fees_msat);
                        if !upstream_force_closed {
                                // Is the event prev_channel_id in one of the channels between the two nodes?
-                               assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == prev_node.node().get_our_node_id() && x.channel_id == prev_channel_id.unwrap()));
+                               assert!(node.node().list_channels().iter().any(|x|
+                                       x.counterparty.node_id == prev_node.node().get_our_node_id() &&
+                                       x.channel_id == prev_channel_id.unwrap() &&
+                                       x.user_channel_id == prev_user_channel_id.unwrap()
+                               ));
                        }
                        // We check for force closures since a force closed channel is removed from the
                        // node's channel list
                        if !downstream_force_closed {
-                               assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == next_node.node().get_our_node_id() && x.channel_id == next_channel_id.unwrap()));
+                               // As documented, `next_user_channel_id` will only be `Some` if we didn't settle via an
+                               // onchain transaction, just as the `total_fee_earned_msat` field. Rather than
+                               // introducing yet another variable, we use the latter's state as a flag to detect
+                               // this and only check if it's `Some`.
+                               if total_fee_earned_msat.is_none() {
+                                       assert!(node.node().list_channels().iter().any(|x|
+                                               x.counterparty.node_id == next_node.node().get_our_node_id() &&
+                                               x.channel_id == next_channel_id.unwrap()
+                                       ));
+                               } else {
+                                       assert!(node.node().list_channels().iter().any(|x|
+                                               x.counterparty.node_id == next_node.node().get_our_node_id() &&
+                                               x.channel_id == next_channel_id.unwrap() &&
+                                               x.user_channel_id == next_user_channel_id.unwrap()
+                                       ));
+                               }
                        }
                        assert_eq!(claim_from_onchain_tx, downstream_force_closed);
                },