-
Notifications
You must be signed in to change notification settings - Fork 130
Bump rust-lightning to include trampoline changes #825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,9 @@ use lightning::events::bump_transaction::BumpTransactionEvent; | |
| #[cfg(not(feature = "uniffi"))] | ||
| use lightning::events::PaidBolt12Invoice; | ||
| use lightning::events::{ | ||
| ClosureReason, Event as LdkEvent, FundingInfo, PaymentFailureReason, PaymentPurpose, | ||
| ReplayEvent, | ||
| ClosureReason, Event as LdkEvent, FundingInfo, HTLCLocator as LdkHTLCLocator, | ||
| PaymentFailureReason, PaymentPurpose, ReplayEvent, | ||
| }; | ||
| use lightning::impl_writeable_tlv_based_enum; | ||
| use lightning::ln::channelmanager::PaymentId; | ||
| use lightning::ln::types::ChannelId; | ||
| use lightning::routing::gossip::NodeId; | ||
|
|
@@ -32,6 +31,7 @@ use lightning::util::config::{ | |
| use lightning::util::errors::APIError; | ||
| use lightning::util::persist::KVStore; | ||
| use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer}; | ||
| use lightning::{impl_writeable_tlv_based, impl_writeable_tlv_based_enum}; | ||
| use lightning_liquidity::lsps2::utils::compute_opening_fee; | ||
| use lightning_types::payment::{PaymentHash, PaymentPreimage}; | ||
|
|
||
|
|
@@ -61,6 +61,50 @@ use crate::{ | |
| UserChannelId, | ||
| }; | ||
|
|
||
| /// Identifies the channel and counterparty that a HTLC was processed with. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
| pub struct HTLCLocator { | ||
| /// The channel that the HTLC was sent or received on. | ||
| pub channel_id: ChannelId, | ||
| /// The `user_channel_id` for the channel. | ||
| /// | ||
| /// Will only be `None` for events serialized with LDK Node v0.3.0 or prior, or if the | ||
| /// payment was settled via an on-chain transaction. | ||
| pub user_channel_id: Option<UserChannelId>, | ||
| /// The node id of the counterparty for this HTLC. | ||
| /// | ||
| /// This is only `None` for HTLCs received prior to LDK Node v0.5 or for events serialized by | ||
| /// versions prior to v0.5. | ||
| pub node_id: Option<PublicKey>, | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(HTLCLocator, { | ||
| (1, channel_id, required), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make these even, given its a new struct? |
||
| (3, user_channel_id, option), | ||
| (5, node_id, option), | ||
| }); | ||
|
|
||
| impl From<LdkHTLCLocator> for HTLCLocator { | ||
| fn from(value: LdkHTLCLocator) -> Self { | ||
| HTLCLocator { | ||
| channel_id: value.channel_id, | ||
| user_channel_id: value.user_channel_id.map(|u| UserChannelId(u)), | ||
| node_id: value.node_id, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<HTLCLocator> for LdkHTLCLocator { | ||
| fn from(value: HTLCLocator) -> Self { | ||
| LdkHTLCLocator { | ||
| channel_id: value.channel_id, | ||
| user_channel_id: value.user_channel_id.map(|u| u.0), | ||
| node_id: value.node_id, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// An event emitted by [`Node`], which should be handled by the user. | ||
| /// | ||
| /// [`Node`]: [`crate::Node`] | ||
|
|
@@ -128,29 +172,14 @@ pub enum Event { | |
| }, | ||
| /// A payment has been forwarded. | ||
| PaymentForwarded { | ||
| /// The channel id of the incoming channel between the previous node and us. | ||
| prev_channel_id: ChannelId, | ||
| /// The channel id of the outgoing channel between the next node and us. | ||
| next_channel_id: ChannelId, | ||
| /// The `user_channel_id` of the incoming channel between the previous node and us. | ||
| /// | ||
| /// Will only be `None` for events serialized with LDK Node v0.3.0 or prior. | ||
| prev_user_channel_id: Option<UserChannelId>, | ||
| /// 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. | ||
| next_user_channel_id: Option<UserChannelId>, | ||
| /// The node id of the previous node. | ||
| /// | ||
| /// This is only `None` for HTLCs received prior to LDK Node v0.5 or for events serialized by | ||
| /// versions prior to v0.5. | ||
| prev_node_id: Option<PublicKey>, | ||
| /// The node id of the next node. | ||
| /// | ||
| /// This is only `None` for HTLCs received prior to LDK Node v0.5 or for events serialized by | ||
| /// versions prior to v0.5. | ||
| next_node_id: Option<PublicKey>, | ||
| /// The set of incoming HTLCs that were forwarded to our node. Contains a single HTLC for | ||
| /// source-routed payments, and may contain multiple HTLCs when we acted as a trampoline | ||
| /// router. | ||
| prev_htlcs: Vec<HTLCLocator>, | ||
| /// The set of outgoing HTLCs forwarded by our node. Contains a single HTLC for regular | ||
| /// source-routed payments, and may contain multiple HTLCs when we acted as a trampoline | ||
| /// router. | ||
| next_htlcs: Vec<HTLCLocator>, | ||
| /// 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 | ||
|
|
@@ -323,16 +352,63 @@ impl_writeable_tlv_based_enum!(Event, | |
| (7, custom_records, optional_vec), | ||
| }, | ||
| (7, PaymentForwarded) => { | ||
| (0, prev_channel_id, required), | ||
| (1, prev_node_id, option), | ||
| (2, next_channel_id, required), | ||
| (3, next_node_id, option), | ||
| (4, prev_user_channel_id, option), | ||
| (6, next_user_channel_id, option), | ||
| // We don't write our legacy types because we don't need to support downgrades, but we do | ||
| // read them so that we can forwards compatibly fill prev/next_htlcs on upgrade. | ||
| (0, legacy_prev_channel_id, (legacy, ChannelId, |_| Ok(()), |_: &Event| None::<Option<ChannelId>>)), | ||
| (1, legacy_prev_node_id, (legacy, PublicKey, |_| Ok(()), |_: &Event| None::<Option<PublicKey>>)), | ||
| (2, legacy_next_channel_id, (legacy, ChannelId, |_| Ok(()), |_: &Event| None::<Option<ChannelId>>)), | ||
| (3, legacy_next_node_id, (legacy, PublicKey, |_| Ok(()), |_: &Event| None::<Option<PublicKey>>)), | ||
| (4, legacy_prev_user_channel_id, (legacy, u128, |_| Ok(()), |_: &Event| None::<Option<u128>>)), | ||
| (6, legacy_next_user_channel_id, (legacy, u128, |_| Ok(()), |_: &Event| None::<Option<u128>>)), | ||
| (8, total_fee_earned_msat, option), | ||
| (10, skimmed_fee_msat, option), | ||
| (12, claim_from_onchain_tx, required), | ||
| (14, outbound_amount_forwarded_msat, option), | ||
| // We cannot implement Readable/Writeable for Vec<HTLCLocator> because we do not own the | ||
| // trait or the type (Vec). To work around this, and prevent duplicating serialization code, | ||
| // we map to the underlying LdkHTLCLocator type for serialization. | ||
| (15, prev_htlcs, (custom, Vec<LdkHTLCLocator>, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we try to streamline this a bit? Maybe introduce a helper method to dedup the code? At the very least it seems we can drop a few lines: diff --git a/src/event.rs b/src/event.rs
index 417f5941..0505a870 100644
--- a/src/event.rs
+++ b/src/event.rs
@@ -369,16 +369,14 @@ impl_writeable_tlv_based_enum!(Event,
// we map to the underlying LdkHTLCLocator type for serialization.
(15, prev_htlcs, (custom, Vec<LdkHTLCLocator>,
|v: Option<Vec<LdkHTLCLocator>>| {
- let res: Result<Vec<HTLCLocator>, lightning::ln::msgs::DecodeError> =
- Ok(v.map(|ldk_vec| ldk_vec.into_iter().map(HTLCLocator::from).collect())
+ Ok(v.map(|ldk_vec| ldk_vec.into_iter().map(HTLCLocator::from).collect())
.unwrap_or_else(|| {
legacy_prev_channel_id.map(|ch| vec![HTLCLocator {
channel_id: ch,
user_channel_id: legacy_prev_user_channel_id.map(UserChannelId),
node_id: legacy_prev_node_id,
}]).unwrap_or_default()
- }));
- res
+ }))
},
|us: &Event| {
if let Event::PaymentForwarded { ref prev_htlcs, .. } = us {
@@ -390,7 +388,6 @@ impl_writeable_tlv_based_enum!(Event,
)),
(17, next_htlcs, (custom, Vec<LdkHTLCLocator>,
|v: Option<Vec<LdkHTLCLocator>>| {
- let res: Result<Vec<HTLCLocator>, lightning::ln::msgs::DecodeError> =
Ok(v.map(|ldk_vec| ldk_vec.into_iter().map(HTLCLocator::from).collect())
.unwrap_or_else(|| {
legacy_next_channel_id.map(|ch| vec![HTLCLocator {
@@ -398,8 +395,7 @@ impl_writeable_tlv_based_enum!(Event,
user_channel_id: legacy_next_user_channel_id.map(UserChannelId),
node_id: legacy_next_node_id,
}]).unwrap_or_default()
- }));
- res
+ }))
},
|us: &Event| {
if let Event::PaymentForwarded { ref next_htlcs, .. } = us { |
||
| |v: Option<Vec<LdkHTLCLocator>>| { | ||
| let res: Result<Vec<HTLCLocator>, lightning::ln::msgs::DecodeError> = | ||
| Ok(v.map(|ldk_vec| ldk_vec.into_iter().map(HTLCLocator::from).collect()) | ||
| .unwrap_or_else(|| { | ||
| legacy_prev_channel_id.map(|ch| vec![HTLCLocator { | ||
| channel_id: ch, | ||
| user_channel_id: legacy_prev_user_channel_id.map(UserChannelId), | ||
| node_id: legacy_prev_node_id, | ||
| }]).unwrap_or_default() | ||
| })); | ||
| res | ||
| }, | ||
| |us: &Event| { | ||
| if let Event::PaymentForwarded { ref prev_htlcs, .. } = us { | ||
| if !prev_htlcs.is_empty() { | ||
| Some(prev_htlcs.iter().cloned().map(LdkHTLCLocator::from).collect::<Vec<_>>()) | ||
| } else { None } | ||
| } else { unreachable!() } | ||
| } | ||
| )), | ||
| (17, next_htlcs, (custom, Vec<LdkHTLCLocator>, | ||
| |v: Option<Vec<LdkHTLCLocator>>| { | ||
| let res: Result<Vec<HTLCLocator>, lightning::ln::msgs::DecodeError> = | ||
| Ok(v.map(|ldk_vec| ldk_vec.into_iter().map(HTLCLocator::from).collect()) | ||
| .unwrap_or_else(|| { | ||
| legacy_next_channel_id.map(|ch| vec![HTLCLocator { | ||
| channel_id: ch, | ||
| user_channel_id: legacy_next_user_channel_id.map(UserChannelId), | ||
| node_id: legacy_next_node_id, | ||
| }]).unwrap_or_default() | ||
| })); | ||
| res | ||
| }, | ||
| |us: &Event| { | ||
| if let Event::PaymentForwarded { ref next_htlcs, .. } = us { | ||
| if !next_htlcs.is_empty() { | ||
| Some(next_htlcs.iter().cloned().map(LdkHTLCLocator::from).collect::<Vec<_>>()) | ||
| } else { None } | ||
| } else { unreachable!() } | ||
| } | ||
| )), | ||
| }, | ||
| (8, SplicePending) => { | ||
| (1, channel_id, required), | ||
|
|
@@ -1306,12 +1382,8 @@ where | |
| } | ||
| }, | ||
| LdkEvent::PaymentForwarded { | ||
| prev_channel_id, | ||
| next_channel_id, | ||
| prev_user_channel_id, | ||
| next_user_channel_id, | ||
| prev_node_id, | ||
| next_node_id, | ||
| prev_htlcs, | ||
| next_htlcs, | ||
| total_fee_earned_msat, | ||
| skimmed_fee_msat, | ||
| claim_from_onchain_tx, | ||
|
|
@@ -1322,11 +1394,10 @@ where | |
| let nodes = read_only_network_graph.nodes(); | ||
| let channels = self.channel_manager.list_channels(); | ||
|
|
||
| let node_str = |channel_id: &Option<ChannelId>| { | ||
| channel_id | ||
| .and_then(|channel_id| { | ||
| channels.iter().find(|c| c.channel_id == channel_id) | ||
| }) | ||
| let node_str = |channel_id: &ChannelId| { | ||
| channels | ||
| .iter() | ||
| .find(|c| c.channel_id == *channel_id) | ||
| .and_then(|channel| { | ||
| nodes.get(&NodeId::from_pubkey(&channel.counterparty.node_id)) | ||
| }) | ||
|
|
@@ -1338,21 +1409,21 @@ where | |
| }) | ||
| }) | ||
| }; | ||
| let channel_str = |channel_id: &Option<ChannelId>| { | ||
| channel_id | ||
| .map(|channel_id| format!(" with channel {}", channel_id)) | ||
| .unwrap_or_default() | ||
| }; | ||
| let from_prev_str = format!( | ||
| " from {}{}", | ||
| node_str(&prev_channel_id), | ||
| channel_str(&prev_channel_id) | ||
| ); | ||
| let to_next_str = format!( | ||
| " to {}{}", | ||
| node_str(&next_channel_id), | ||
| channel_str(&next_channel_id) | ||
| ); | ||
| let from_prev_str: String = prev_htlcs | ||
| .iter() | ||
| .map(|htlc| { | ||
| format!("with {} on {}", node_str(&htlc.channel_id), htlc.channel_id) | ||
| }) | ||
| .collect::<Vec<_>>() | ||
| .join(", "); | ||
|
|
||
| let to_next_str: String = next_htlcs | ||
| .iter() | ||
| .map(|htlc| { | ||
| format!("with {} on {}", node_str(&htlc.channel_id), htlc.channel_id) | ||
| }) | ||
| .collect::<Vec<_>>() | ||
| .join(", "); | ||
|
|
||
| let fee_earned = total_fee_earned_msat.unwrap_or(0); | ||
| if claim_from_onchain_tx { | ||
|
|
@@ -1367,8 +1438,10 @@ where | |
| } else { | ||
| log_info!( | ||
| self.logger, | ||
| "Forwarded payment{}{} of {}msat, earning {}msat in fees.", | ||
| "Forwarded payment with {} inbound HTLC(s) ({}) and {} outbound HTLC(s) ({}) of {}msat, earning {}msat in fees.", | ||
| prev_htlcs.len(), | ||
| from_prev_str, | ||
| next_htlcs.len(), | ||
| to_next_str, | ||
| outbound_amount_forwarded_msat.unwrap_or(0), | ||
| fee_earned, | ||
|
|
@@ -1378,18 +1451,16 @@ where | |
|
|
||
| if let Some(liquidity_source) = self.liquidity_source.as_ref() { | ||
| let skimmed_fee_msat = skimmed_fee_msat.unwrap_or(0); | ||
| liquidity_source | ||
| .handle_payment_forwarded(next_channel_id, skimmed_fee_msat) | ||
| .await; | ||
| for next_htlc in next_htlcs.iter() { | ||
| liquidity_source | ||
| .handle_payment_forwarded(Some(next_htlc.channel_id), skimmed_fee_msat) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, wouldn't this have us / the accounting logic believe that we withheld the skimmed amount multiple times? |
||
| .await; | ||
| } | ||
| } | ||
|
|
||
| let event = Event::PaymentForwarded { | ||
| prev_channel_id: prev_channel_id.expect("prev_channel_id expected for events generated by LDK versions greater than 0.0.107."), | ||
| next_channel_id: next_channel_id.expect("next_channel_id expected for events generated by LDK versions greater than 0.0.107."), | ||
| prev_user_channel_id: prev_user_channel_id.map(UserChannelId), | ||
| next_user_channel_id: next_user_channel_id.map(UserChannelId), | ||
| prev_node_id, | ||
| next_node_id, | ||
| prev_htlcs: prev_htlcs.into_iter().map(|h| h.into()).collect(), | ||
| next_htlcs: next_htlcs.into_iter().map(|h| h.into()).collect(), | ||
| total_fee_earned_msat, | ||
| skimmed_fee_msat, | ||
| claim_from_onchain_tx, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'd really like to avoid duplicating upstream's serialization logic, which always risks to get out-of-sync. Can we rather also
impl From<Vec<HTLCLocator>> for Vec<LdkHTLCLocator>and use upstream'swrite? I admit that is also not great, but maybe preferable to duplicating even more logic?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this a shot, it's a little unwieldy but I agree probably preferable to duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed unwieldy and not loving the reallocation either, but oh well...
But we can now remove the
impl_writeable_tlv_basedhere, right?