Skip to content

Handle receiving payments via Trampoline #3670

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Mar 28, 2025
Merged

Handle receiving payments via Trampoline #3670

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4741892
Remove Trampoline config flag
arik-so Mar 26, 2025
0c82cc2
Make Trampoline inbound MPP data optional
arik-so Mar 17, 2025
a293d21
Fix blinded Trampoline TLV IDs
arik-so Mar 17, 2025
313f344
De-macro `return_malformed_err`
arik-so Mar 26, 2025
23b4c80
Propagate Trampoline blinding errors
arik-so Mar 26, 2025
dc14f26
Implement `Writeable` for `TrampolineForwardTlvs`
arik-so Mar 17, 2025
35ff142
Handle Trampoline receives
arik-so Mar 17, 2025
a6f4c7f
Test unblinded Trampoline receives
arik-so Mar 25, 2025
3970f1d
Test rejection of Trampoline forwards
arik-so Mar 26, 2025
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
1 change: 0 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ check-cfg = [
"cfg(ldk_bench)",
"cfg(ldk_test_vectors)",
"cfg(taproot)",
"cfg(trampoline)",
"cfg(require_route_graph_test)",
"cfg(splicing)",
"cfg(async_payments)",
Expand Down
2 changes: 0 additions & 2 deletions ci/ci-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=splicing" cargo test --verbose --color always -p lightning
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=trampoline" cargo test --verbose --color always -p lightning
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not following the commit message here in 908c6bf since we don't support trampoline forwarding yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, it ensures that receiving forwarding onions results in a rejection, but not a panic or some undefined behavior. I'll rephrase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok Joost explained that we test that we don't support forwarding basically

[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=async_payments" cargo test --verbose --color always -p lightning
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=lsps1_service" cargo test --verbose --color always -p lightning-liquidity
5 changes: 1 addition & 4 deletions lightning/src/blinded_path/payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ pub struct ForwardTlvs {
}

/// Data to construct a [`BlindedHop`] for forwarding a Trampoline payment.
#[cfg(trampoline)]
#[derive(Clone, Debug)]
pub struct TrampolineForwardTlvs {
/// The node id to which the trampoline node must find a route.
Expand Down Expand Up @@ -371,7 +370,6 @@ pub(crate) enum BlindedPaymentTlvs {
/// Data to construct a [`BlindedHop`] for sending a Trampoline payment over.
///
/// [`BlindedHop`]: crate::blinded_path::BlindedHop
#[cfg(trampoline)]
pub(crate) enum BlindedTrampolineTlvs {
/// This blinded payment data is for a forwarding node.
Forward(TrampolineForwardTlvs),
Expand Down Expand Up @@ -591,14 +589,13 @@ impl Readable for BlindedPaymentTlvs {
}
}

#[cfg(trampoline)]
impl Readable for BlindedTrampolineTlvs {
fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
_init_and_read_tlv_stream!(r, {
(4, next_trampoline, option),
(8, next_blinding_override, option),
(10, payment_relay, option),
(12, payment_constraints, required),
(14, next_trampoline, option),
(14, features, (option, encoding: (BlindedHopFeatures, WithoutLength))),
(65536, payment_secret, option),
(65537, payment_context, option),
Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/blinded_payment_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1816,7 +1816,6 @@ fn test_combined_trampoline_onion_creation_vectors() {
}

#[test]
#[cfg(trampoline)]
fn test_trampoline_inbound_payment_decoding() {
let secp_ctx = Secp256k1::new();
let session_priv = secret_from_hex("0303030303030303030303030303030303030303030303030303030303030303");
Expand Down
54 changes: 1 addition & 53 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ pub enum PendingHTLCRouting {
incoming_cltv_expiry: Option<u32>,
},
/// An HTLC which should be forwarded on to another Trampoline node.
#[cfg(trampoline)]
TrampolineForward {
/// The onion shared secret we build with the sender (or the preceding Trampoline node) used
/// to decrypt the onion.
Expand Down Expand Up @@ -288,7 +287,6 @@ impl PendingHTLCRouting {
fn blinded_failure(&self) -> Option<BlindedFailure> {
match self {
Self::Forward { blinded: Some(BlindedForward { failure, .. }), .. } => Some(*failure),
#[cfg(trampoline)]
Self::TrampolineForward { blinded: Some(BlindedForward { failure, .. }), .. } => Some(*failure),
Self::Receive { requires_blinded_error: true, .. } => Some(BlindedFailure::FromBlindedNode),
Self::ReceiveKeysend { requires_blinded_error: true, .. } => Some(BlindedFailure::FromBlindedNode),
Expand All @@ -299,7 +297,6 @@ impl PendingHTLCRouting {
fn incoming_cltv_expiry(&self) -> Option<u32> {
match self {
Self::Forward { incoming_cltv_expiry, .. } => *incoming_cltv_expiry,
#[cfg(trampoline)]
Self::TrampolineForward { incoming_cltv_expiry, .. } => Some(*incoming_cltv_expiry),
Self::Receive { incoming_cltv_expiry, .. } => Some(*incoming_cltv_expiry),
Self::ReceiveKeysend { incoming_cltv_expiry, .. } => Some(*incoming_cltv_expiry),
Expand Down Expand Up @@ -4510,24 +4507,7 @@ where
}
}
match decoded_hop {
onion_utils::Hop::Receive { .. } | onion_utils::Hop::BlindedReceive { .. } => {
// OUR PAYMENT!
let current_height: u32 = self.best_block.read().unwrap().height;
match create_recv_pending_htlc_info(decoded_hop, shared_secret, msg.payment_hash,
msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat,
current_height)
{
Ok(info) => {
// Note that we could obviously respond immediately with an update_fulfill_htlc
// message, however that would leak that we are the recipient of this payment, so
// instead we stay symmetric with the forwarding case, only responding (after a
// delay) once they've sent us a commitment_signed!
PendingHTLCStatus::Forward(info)
},
Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
}
},
#[cfg(trampoline)]
onion_utils::Hop::Receive { .. } | onion_utils::Hop::BlindedReceive { .. } |
onion_utils::Hop::TrampolineReceive { .. } | onion_utils::Hop::TrampolineBlindedReceive { .. } => {
// OUR PAYMENT!
let current_height: u32 = self.best_block.read().unwrap().height;
Expand All @@ -4551,7 +4531,6 @@ where
Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
}
},
#[cfg(trampoline)]
onion_utils::Hop::TrampolineForward { .. } | onion_utils::Hop::TrampolineBlindedForward { .. } => {
match create_fwd_pending_htlc_info(msg, decoded_hop, shared_secret, next_packet_pubkey_opt) {
Ok(info) => PendingHTLCStatus::Forward(info),
Expand Down Expand Up @@ -9067,7 +9046,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
for (forward_info, prev_htlc_id) in pending_forwards.drain(..) {
let scid = match forward_info.routing {
PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id,
#[cfg(trampoline)]
PendingHTLCRouting::TrampolineForward { .. } => 0,
PendingHTLCRouting::Receive { .. } => 0,
PendingHTLCRouting::ReceiveKeysend { .. } => 0,
Expand Down Expand Up @@ -12888,36 +12866,6 @@ impl_writeable_tlv_based!(BlindedForward, {
(3, next_blinding_override, option),
});

#[cfg(not(trampoline))]
impl_writeable_tlv_based_enum!(PendingHTLCRouting,
(0, Forward) => {
(0, onion_packet, required),
(1, blinded, option),
(2, short_channel_id, required),
(3, incoming_cltv_expiry, option),
},
(1, Receive) => {
(0, payment_data, required),
(1, phantom_shared_secret, option),
(2, incoming_cltv_expiry, required),
(3, payment_metadata, option),
(5, custom_tlvs, optional_vec),
(7, requires_blinded_error, (default_value, false)),
(9, payment_context, option),
},
(2, ReceiveKeysend) => {
(0, payment_preimage, required),
(1, requires_blinded_error, (default_value, false)),
(2, incoming_cltv_expiry, required),
(3, payment_metadata, option),
(4, payment_data, option), // Added in 0.0.116
(5, custom_tlvs, optional_vec),
(7, has_recipient_created_payment_secret, (default_value, false)),
(9, payment_context, option),
(11, invoice_request, option),
},
);
#[cfg(trampoline)]
impl_writeable_tlv_based_enum!(PendingHTLCRouting,
(0, Forward) => {
(0, onion_packet, required),
Expand Down
37 changes: 3 additions & 34 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use bitcoin::script::ScriptBuf;
use bitcoin::hash_types::Txid;

use crate::blinded_path::payment::{BlindedPaymentTlvs, ForwardTlvs, ReceiveTlvs, UnauthenticatedReceiveTlvs};
#[cfg(trampoline)]
use crate::blinded_path::payment::{BlindedTrampolineTlvs, TrampolineForwardTlvs};
use crate::ln::channelmanager::Verification;
use crate::ln::types::ChannelId;
Expand Down Expand Up @@ -2075,12 +2074,11 @@ mod fuzzy_internal_msgs {
pub outgoing_cltv_value: u32,
}

#[cfg(trampoline)]
#[cfg_attr(trampoline, allow(unused))]
#[allow(unused)]
pub struct InboundTrampolineEntrypointPayload {
pub amt_to_forward: u64,
pub outgoing_cltv_value: u32,
pub multipath_trampoline_data: FinalOnionHopData,
pub multipath_trampoline_data: Option<FinalOnionHopData>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the discussion you're referring to in the commit message? lightning/bolts#836 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, exactly! @t-bast and I discussed it a bit offline, and I believe they're assessing Eclair code viability for not mandating it outside of MPP scenarios. Alternatively, I can also make it mandatory on the sending side (right now we have an internal mismatch), but I think this approach makes more sense for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd think that for simplicity, it may be better to just always set this field? One code path.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd agree, although it's all cfg-gated for now anyway I suppose... @arik-so was the reason for doing this now for ease of testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Partially. The other option would require us deciding on a mechanism to convert the prng_seed into a pseudo-random payment secret in the outgoing onion construction method, that's about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

But generating that secret is something to be looked at when implementing the forwarding part isn't it? Isn't it best for now to stick with the proposed spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The spec did actually get updated this morning to make it optional for the outer onion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe should update the commit message then?

pub trampoline_packet: TrampolineOnionPacket,
/// The blinding point this hop needs to decrypt its Trampoline onion.
/// This is used for Trampoline hops that are not the blinded path intro hop.
Expand Down Expand Up @@ -2118,23 +2116,19 @@ mod fuzzy_internal_msgs {

pub enum InboundOnionPayload {
Forward(InboundOnionForwardPayload),
#[cfg(trampoline)]
#[cfg_attr(trampoline, allow(unused))]
TrampolineEntrypoint(InboundTrampolineEntrypointPayload),
Receive(InboundOnionReceivePayload),
BlindedForward(InboundOnionBlindedForwardPayload),
BlindedReceive(InboundOnionBlindedReceivePayload),
}

#[cfg(trampoline)]
pub struct InboundTrampolineForwardPayload {
pub next_trampoline: PublicKey,
/// The value, in msat, of the payment after this hop's fee is deducted.
pub amt_to_forward: u64,
pub outgoing_cltv_value: u32,
}

#[cfg(trampoline)]
pub struct InboundTrampolineBlindedForwardPayload {
pub next_trampoline: PublicKey,
pub payment_relay: PaymentRelay,
Expand All @@ -2144,7 +2138,6 @@ mod fuzzy_internal_msgs {
pub next_blinding_override: Option<PublicKey>,
}

#[cfg(trampoline)]
pub enum InboundTrampolinePayload {
Forward(InboundTrampolineForwardPayload),
BlindedForward(InboundTrampolineBlindedForwardPayload),
Expand Down Expand Up @@ -3239,15 +3232,13 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, NS)> for InboundOnionPayload wh
let mut payment_metadata: Option<WithoutLength<Vec<u8>>> = None;
let mut total_msat = None;
let mut keysend_preimage: Option<PaymentPreimage> = None;
#[cfg(trampoline)]
let mut trampoline_onion_packet: Option<TrampolineOnionPacket> = None;
let mut invoice_request: Option<InvoiceRequest> = None;
let mut custom_tlvs = Vec::new();

let tlv_len = BigSize::read(r)?;
let mut rd = FixedLengthReader::new(r, tlv_len.0);

#[cfg(trampoline)]
decode_tlv_stream_with_custom_tlv_decode!(&mut rd, {
(2, amt, (option, encoding: (u64, HighZeroBytesDroppedBigSize))),
(4, cltv_value, (option, encoding: (u32, HighZeroBytesDroppedBigSize))),
Expand All @@ -3268,41 +3259,20 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, NS)> for InboundOnionPayload wh
custom_tlvs.push((msg_type, value));
Ok(true)
});
#[cfg(not(trampoline))]
decode_tlv_stream_with_custom_tlv_decode!(&mut rd, {
(2, amt, (option, encoding: (u64, HighZeroBytesDroppedBigSize))),
(4, cltv_value, (option, encoding: (u32, HighZeroBytesDroppedBigSize))),
(6, short_id, option),
(8, payment_data, option),
(10, encrypted_tlvs_opt, option),
(12, intro_node_blinding_point, option),
(16, payment_metadata, option),
(18, total_msat, (option, encoding: (u64, HighZeroBytesDroppedBigSize))),
(77_777, invoice_request, option),
// See https://github.com/lightning/blips/blob/master/blip-0003.md
(5482373484, keysend_preimage, option)
}, |msg_type: u64, msg_reader: &mut FixedLengthReader<_>| -> Result<bool, DecodeError> {
if msg_type < 1 << 16 { return Ok(false) }
let mut value = Vec::new();
msg_reader.read_to_limit(&mut value, u64::MAX)?;
custom_tlvs.push((msg_type, value));
Ok(true)
});

if amt.unwrap_or(0) > MAX_VALUE_MSAT { return Err(DecodeError::InvalidValue) }
if intro_node_blinding_point.is_some() && update_add_blinding_point.is_some() {
return Err(DecodeError::InvalidValue)
}

#[cfg(trampoline)]
if let Some(trampoline_onion_packet) = trampoline_onion_packet {
if payment_metadata.is_some() || encrypted_tlvs_opt.is_some() ||
total_msat.is_some()
{ return Err(DecodeError::InvalidValue) }
return Ok(Self::TrampolineEntrypoint(InboundTrampolineEntrypointPayload {
amt_to_forward: amt.ok_or(DecodeError::InvalidValue)?,
outgoing_cltv_value: cltv_value.ok_or(DecodeError::InvalidValue)?,
multipath_trampoline_data: payment_data.ok_or(DecodeError::InvalidValue)?,
multipath_trampoline_data: payment_data,
trampoline_packet: trampoline_onion_packet,
current_path_key: intro_node_blinding_point,
}))
Expand Down Expand Up @@ -3391,7 +3361,6 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, NS)> for InboundOnionPayload wh
}
}

#[cfg(trampoline)]
impl<NS: Deref> ReadableArgs<(Option<PublicKey>, NS)> for InboundTrampolinePayload where NS::Target: NodeSigner {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's kind of a shame to keep this duplicated with InboundOnionPayload since the code is complex and ~identical, but might be best to save this consideration for follow-up so we can get this in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, that one saw a lot of back and forth, and I agree, but the alternative is clunky, too, and loses the benefit of semantic distinction

fn read<R: Read>(r: &mut R, args: (Option<PublicKey>, NS)) -> Result<Self, DecodeError> {
let (update_add_blinding_point, node_signer) = args;
Expand Down
Loading