Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ default = []
#lightning-liquidity = { version = "0.2.0", features = ["std"] }
#lightning-macros = { version = "0.2.0" }

lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["std"] }
lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204" }
lightning-invoice = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["std"] }
lightning-net-tokio = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204" }
lightning-persister = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["tokio"] }
lightning-background-processor = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204" }
lightning-rapid-gossip-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204" }
lightning-block-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["rest-client", "rpc-client", "tokio"] }
lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["std"] }
lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204" }
lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["std"] }
lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a" }
lightning-invoice = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["std"] }
lightning-net-tokio = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a" }
lightning-persister = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["tokio"] }
lightning-background-processor = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a" }
lightning-rapid-gossip-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a" }
lightning-block-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["rest-client", "rpc-client", "tokio"] }
lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["std"] }
lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a" }

bdk_chain = { version = "0.23.0", default-features = false, features = ["std"] }
bdk_esplora = { version = "0.22.0", default-features = false, features = ["async-https-rustls", "tokio"]}
Expand Down Expand Up @@ -79,13 +79,13 @@ async-trait = { version = "0.1", default-features = false }
vss-client = { package = "vss-client-ng", version = "0.5" }
prost = { version = "0.11.6", default-features = false}
#bitcoin-payment-instructions = { version = "0.6" }
bitcoin-payment-instructions = { git = "https://github.com/jkczyz/bitcoin-payment-instructions", rev = "0138feb7acefb1e49102a6fb46d7b776bf43265e" }
bitcoin-payment-instructions = { git = "https://github.com/carlaKC/bitcoin-payment-instructions", rev = "c22c9b836b70d4c915dd28701e11083a8b558d56" }

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3", features = ["winbase"] }

[dev-dependencies]
lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "98393b3de3d8aec897e9ab783cb2418da504e204", features = ["std", "_test_utils"] }
lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "128ead25e8d62b0262b71711631220983c48756a", features = ["std", "_test_utils"] }
rand = { version = "0.9.2", default-features = false, features = ["std", "thread_rng", "os_rng"] }
proptest = "1.0.0"
regex = "1.5.6"
Expand Down
207 changes: 139 additions & 68 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};

Expand Down Expand Up @@ -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, {
Copy link
Collaborator

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's write? I admit that is also not great, but maybe preferable to duplicating even more logic?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit that is also not great, but maybe preferable to duplicating even more logic?

Given this a shot, it's a little unwieldy but I agree probably preferable to duplication.

Copy link
Collaborator

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_based here, right?

(1, channel_id, required),
Copy link
Collaborator

Choose a reason for hiding this comment

The 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`]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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),
Expand Down Expand Up @@ -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,
Expand All @@ -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))
})
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The 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,
Expand Down
Loading