Skip to content
Open
17 changes: 15 additions & 2 deletions lightning-types/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@
//! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#channel-quiescence) for more information).
//! - `ZeroFeeCommitments` - A channel type which always uses zero transaction fee on commitment transactions.
//! (see [BOLT PR #1228](https://github.com/lightning/bolts/pull/1228) for more info).
//! - `HtlcHold` - requires/supports holding HTLCs and forwarding on receipt of an onion message
//! (see [BOLT-2](https://github.com/lightning/bolts/pull/989/files) for more information).
//!
//! LDK knows about the following features, but does not support them:
//! - `AnchorsNonzeroFeeHtlcTx` - the initial version of anchor outputs, which was later found to be
Expand Down Expand Up @@ -161,7 +163,7 @@ mod sealed {
// Byte 5
ProvideStorage | ChannelType | SCIDPrivacy | AnchorZeroFeeCommitments,
// Byte 6
ZeroConf,
ZeroConf | HtlcHold,
// Byte 7
Trampoline | SimpleClose,
]
Expand All @@ -182,7 +184,7 @@ mod sealed {
// Byte 5
ProvideStorage | ChannelType | SCIDPrivacy | AnchorZeroFeeCommitments,
// Byte 6
ZeroConf | Keysend,
ZeroConf | HtlcHold | Keysend,
// Byte 7
Trampoline | SimpleClose,
// Byte 8 - 31
Expand Down Expand Up @@ -640,6 +642,17 @@ mod sealed {
define_feature!(51, ZeroConf, [InitContext, NodeContext, ChannelTypeContext],
"Feature flags for accepting channels with zero confirmations. Called `option_zeroconf` in the BOLTs",
set_zero_conf_optional, set_zero_conf_required, supports_zero_conf, requires_zero_conf);
define_feature!(
53,
HtlcHold,
[InitContext, NodeContext],
"Feature flags for holding HTLCs and forwarding on receipt of an onion message",
set_htlc_hold_optional,
set_htlc_hold_required,
clear_htlc_hold,
supports_htlc_hold,
requires_htlc_hold
);
define_feature!(
55,
Keysend,
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/blinded_payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,7 @@ fn update_add_msg(
onion_routing_packet,
skimmed_fee_msat: None,
blinding_point,
hold_htlc: None,
}
}

Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9027,6 +9027,7 @@ where
onion_routing_packet: (**onion_packet).clone(),
skimmed_fee_msat: htlc.skimmed_fee_msat,
blinding_point: htlc.blinding_point,
hold_htlc: None, // Will be set by the async sender when support is added
});
}
}
Expand Down
36 changes: 36 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6406,6 +6406,32 @@ where
});
let shared_secret = next_hop.shared_secret().secret_bytes();

// Nodes shouldn't expect us to hold HTLCs for them if we don't advertise htlc_hold feature
// support.
//
// If we wanted to pretend to be a node that didn't understand the feature at all here, the
// correct behavior would've been to disconnect the sender when we first received the
// update_add message. However, this would make the `UserConfig::enable_htlc_hold` option
// unsafe -- if our node switched the config option from on to off just after the sender
// enqueued their update_add + CS, the sender would continue retransmitting those messages
// and we would keep disconnecting them until the HTLC timed out.
if update_add_htlc.hold_htlc.is_some()
&& !BaseMessageHandler::provided_node_features(self).supports_htlc_hold()
{
let reason = LocalHTLCFailureReason::TemporaryNodeFailure;
let htlc_fail = self.htlc_failure_from_update_add_err(
&update_add_htlc,
&incoming_counterparty_node_id,
reason,
is_intro_node_blinded_forward,
&shared_secret,
);
let failure_type =
get_htlc_failure_type(outgoing_scid_opt, update_add_htlc.payment_hash);
htlc_fails.push((htlc_fail, failure_type, reason.into()));
continue;
}
Comment on lines +6473 to +6497
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seeking conceptual feedback on this approach, including the failure reason

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it isn't better and simpler to just let the htlc through and log? Maybe the receiver is online or can be woken up and it just works. And otherwise that error will come back anyway.

It probably also depends on how the sender is going to interpret this failure. If it handles it like any other, it isn't necessary to be specific? Zooming out, I don't think any of this matters much in a client<->lsp setup, except for some debugging with can be done through logs too.


// Process the HTLC on the incoming channel.
match self.do_funded_channel_callback(
incoming_scid,
Expand Down Expand Up @@ -12983,6 +13009,8 @@ where
for (err, counterparty_node_id) in failed_channels.drain(..) {
let _ = handle_error!(self, err, counterparty_node_id);
}

let _ = self.flow.peer_disconnected(counterparty_node_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Log error? Or otherwise don't return anything from peer_disconnected as this is the only usage?

}

#[rustfmt::skip]
Expand Down Expand Up @@ -13092,6 +13120,7 @@ where
// until we have some peer connection(s) to receive onion messages over, so as a minor optimization
// refresh the cache when a peer connects.
self.check_refresh_async_receive_offer_cache(false);
let _ = self.flow.peer_connected(counterparty_node_id, &init_msg.features);
res
}

Expand Down Expand Up @@ -14789,6 +14818,13 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures {
features.set_anchor_zero_fee_commitments_optional();
}

// If we are configured to be an announced node, we are expected to be always-online and can
// advertise the htlc_hold feature.
#[cfg(test)]
if config.enable_htlc_hold {
features.set_htlc_hold_optional();
}

features
}

Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2288,6 +2288,7 @@ pub fn fail_backward_pending_htlc_upon_channel_failure() {
onion_routing_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};
nodes[0].node.handle_update_add_htlc(node_b_id, &update_add_htlc);
}
Expand Down
5 changes: 5 additions & 0 deletions lightning/src/ln/htlc_reserve_unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@ pub fn do_test_fee_spike_buffer(cfg: Option<UserConfig>, htlc_fails: bool) {
onion_routing_packet: onion_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};

nodes[1].node.handle_update_add_htlc(node_a_id, &msg);
Expand Down Expand Up @@ -1072,6 +1073,7 @@ pub fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
onion_routing_packet: onion_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};

nodes[0].node.handle_update_add_htlc(node_b_id, &msg);
Expand Down Expand Up @@ -1255,6 +1257,7 @@ pub fn test_chan_reserve_violation_inbound_htlc_inbound_chan() {
onion_routing_packet: onion_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};

nodes[1].node.handle_update_add_htlc(node_a_id, &msg);
Expand Down Expand Up @@ -1637,6 +1640,7 @@ pub fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
onion_routing_packet: onion_packet.clone(),
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};

for i in 0..50 {
Expand Down Expand Up @@ -2242,6 +2246,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) {
onion_routing_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};

nodes[1].node.handle_update_add_htlc(node_a_id, &msg);
Expand Down
11 changes: 10 additions & 1 deletion lightning/src/ln/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,11 @@ pub struct UpdateAddHTLC {
/// Provided if we are relaying or receiving a payment within a blinded path, to decrypt the onion
/// routing packet and the recipient-provided encrypted payload within.
pub blinding_point: Option<PublicKey>,
/// Set to `Some` if the sender wants the receiver of this message to hold onto this HTLC until
/// receipt of a [`ReleaseHeldHtlc`] onion message from the payment recipient.
///
/// [`ReleaseHeldHtlc`]: crate::onion_message::async_payments::ReleaseHeldHtlc
pub hold_htlc: Option<()>,
}

/// An onion message to be sent to or received from a peer.
Expand Down Expand Up @@ -3268,7 +3273,10 @@ impl_writeable_msg!(UpdateAddHTLC, {
onion_routing_packet,
}, {
(0, blinding_point, option),
(65537, skimmed_fee_msat, option)
(65537, skimmed_fee_msat, option),
// TODO: currently we may fail to read the `ChannelManager` if we write a new even TLV in this message
// and then downgrade. Once this is fixed, update the type here to match BOLTs PR 989.
(75537, hold_htlc, option),
});

impl LengthReadable for OnionMessage {
Expand Down Expand Up @@ -5701,6 +5709,7 @@ mod tests {
onion_routing_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
};
let encoded_value = update_add_htlc.encode();
let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034d32144668701144760101010101010101010101010101010101010101010101010101010101010101000c89d4ff031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010202020202020202020202020202020202020202020202020202020202020202").unwrap();
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/onion_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ mod tests {
onion_routing_packet,
skimmed_fee_msat: None,
blinding_point: None,
hold_htlc: None,
}
}

Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5017,6 +5017,7 @@ fn peel_payment_onion_custom_tlvs() {
skimmed_fee_msat: None,
onion_routing_packet,
blinding_point: None,
hold_htlc: None,
};
let peeled_onion = crate::ln::onion_payment::peel_payment_onion(
&update_add,
Expand Down
39 changes: 39 additions & 0 deletions lightning/src/offers/flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use crate::sign::{EntropySource, NodeSigner, ReceiveAuthKey};

use crate::offers::static_invoice::{StaticInvoice, StaticInvoiceBuilder};
use crate::sync::{Mutex, RwLock};
use crate::types::features::InitFeatures;
use crate::types::payment::{PaymentHash, PaymentSecret};
use crate::util::ser::Writeable;

Expand Down Expand Up @@ -98,6 +99,7 @@ where

pending_async_payments_messages: Mutex<Vec<(AsyncPaymentsMessage, MessageSendInstructions)>>,
async_receive_offer_cache: Mutex<AsyncReceiveOfferCache>,
peers_cache: Mutex<Vec<MessageForwardNode>>,

#[cfg(feature = "dnssec")]
pub(crate) hrn_resolver: OMNameResolver,
Expand Down Expand Up @@ -130,6 +132,7 @@ where

pending_offers_messages: Mutex::new(Vec::new()),
pending_async_payments_messages: Mutex::new(Vec::new()),
peers_cache: Mutex::new(Vec::new()),

#[cfg(feature = "dnssec")]
hrn_resolver: OMNameResolver::new(current_timestamp, best_block.height),
Expand Down Expand Up @@ -1627,4 +1630,40 @@ where
pub fn writeable_async_receive_offer_cache(&self) -> Vec<u8> {
self.async_receive_offer_cache.encode()
}

/// Indicates that a peer was connected to our node. Useful for the [`OffersMessageFlow`] to keep
/// track of which peers are connected, which allows for methods that can create blinded paths
/// without requiring a fresh set of [`MessageForwardNode`]s to be passed in.
///
/// MUST be called by always-online nodes that support holding HTLCs on behalf of often-offline
/// senders.
///
/// Errors if the peer does not support onion messages.
pub fn peer_connected(
&self, peer_node_id: PublicKey, features: &InitFeatures,
) -> Result<(), ()> {
if !features.supports_onion_messages() {
return Err(());
}

let mut peers_cache = self.peers_cache.lock().unwrap();
let peer = MessageForwardNode { node_id: peer_node_id, short_channel_id: None };
peers_cache.push(peer);

Ok(())
}

/// Indicates that a peer was disconnected from our node. See [`Self::peer_connected`].
///
/// Errors if the peer is unknown.
pub fn peer_disconnected(&self, peer_node_id: PublicKey) -> Result<(), ()> {
let mut peers_cache = self.peers_cache.lock().unwrap();
let peer_idx = match peers_cache.iter().position(|peer| peer.node_id == peer_node_id) {
Some(idx) => idx,
None => return Err(()),
};
peers_cache.swap_remove(peer_idx);

Ok(())
}
}
14 changes: 14 additions & 0 deletions lightning/src/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,18 @@ pub struct UserConfig {
///
/// Default value: `false`
pub enable_dual_funded_channels: bool,
/// LDK supports a feature for always-online nodes such that these nodes can hold onto an HTLC
/// from an often-offline channel peer until the often-offline payment recipient sends an onion
/// message telling the always-online node to release the HTLC. If this is set to `true`, our node
/// will carry out this feature for channel peers that request it.
///
/// This should only be set to `true` for nodes which expect to be online reliably.
///
/// Setting this to `true` may break backwards compatibility with LDK versions < 0.2.
///
/// Default value: `false`
#[cfg(test)]
pub enable_htlc_hold: bool,
}

impl Default for UserConfig {
Expand All @@ -949,6 +961,8 @@ impl Default for UserConfig {
accept_intercept_htlcs: false,
manually_handle_bolt12_invoices: false,
enable_dual_funded_channels: false,
#[cfg(test)]
enable_htlc_hold: false,
}
}
}
Expand Down