From 5b28744755a4a5ef0303c9cbf927502495194055 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 12 Mar 2021 14:23:20 -0500 Subject: [PATCH 1/3] Add CounterpartyForwardingInfo field to channel. This will be filled in in upcoming commits, then exposed in ChannelDetails to allow constructing route hints for invoices. Also update the cltv_expiry_deta comment in msgs::ChannelUpdate Co-authored-by: Valentine Wallace Co-authored-by: Antoine Riard --- lightning/src/ln/channel.rs | 40 +++++++++++++++++++++++++++++++++++++ lightning/src/ln/msgs.rs | 9 ++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9e4bb081c1a..426083dd832 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -281,6 +281,18 @@ impl HTLCCandidate { } } +/// Information needed for constructing an invoice route hint for this channel. +pub struct CounterpartyForwardingInfo { + /// Base routing fee in millisatoshis. + pub fee_base_msat: u32, + /// Amount in millionths of a satoshi the channel will charge per transferred satoshi. + pub fee_proportional_millionths: u32, + /// The minimum difference in cltv_expiry between an ingoing HTLC and its outgoing counterpart, + /// such that the outgoing HTLC is forwardable to this counterparty. See `msgs::ChannelUpdate`'s + /// `cltv_expiry_delta` for more details. + pub cltv_expiry_delta: u16, +} + // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking // has been completed, and then turn into a Channel to get compiler-time enforcement of things like // calling channel_id() before we're set up or things like get_outbound_funding_signed on an @@ -391,6 +403,8 @@ pub(super) struct Channel { //implied by OUR_MAX_HTLCS: max_accepted_htlcs: u16, minimum_depth: u32, + counterparty_forwarding_info: Option, + pub(crate) channel_transaction_parameters: ChannelTransactionParameters, counterparty_cur_commitment_point: Option, @@ -577,6 +591,8 @@ impl Channel { counterparty_max_accepted_htlcs: 0, minimum_depth: 0, // Filled in in accept_channel + counterparty_forwarding_info: None, + channel_transaction_parameters: ChannelTransactionParameters { holder_pubkeys: pubkeys, holder_selected_contest_delay: config.own_channel_config.our_to_self_delay, @@ -813,6 +829,8 @@ impl Channel { counterparty_max_accepted_htlcs: msg.max_accepted_htlcs, minimum_depth: config.own_channel_config.minimum_depth, + counterparty_forwarding_info: None, + channel_transaction_parameters: ChannelTransactionParameters { holder_pubkeys: pubkeys, holder_selected_contest_delay: config.own_channel_config.our_to_self_delay, @@ -4437,6 +4455,16 @@ impl Writeable for Channel { self.counterparty_max_accepted_htlcs.write(writer)?; self.minimum_depth.write(writer)?; + match &self.counterparty_forwarding_info { + Some(info) => { + 1u8.write(writer)?; + info.fee_base_msat.write(writer)?; + info.fee_proportional_millionths.write(writer)?; + info.cltv_expiry_delta.write(writer)?; + }, + None => 0u8.write(writer)? + } + self.channel_transaction_parameters.write(writer)?; self.counterparty_cur_commitment_point.write(writer)?; @@ -4597,6 +4625,16 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let counterparty_max_accepted_htlcs = Readable::read(reader)?; let minimum_depth = Readable::read(reader)?; + let counterparty_forwarding_info = match ::read(reader)? { + 0 => None, + 1 => Some(CounterpartyForwardingInfo { + fee_base_msat: Readable::read(reader)?, + fee_proportional_millionths: Readable::read(reader)?, + cltv_expiry_delta: Readable::read(reader)?, + }), + _ => return Err(DecodeError::InvalidValue), + }; + let channel_parameters = Readable::read(reader)?; let counterparty_cur_commitment_point = Readable::read(reader)?; @@ -4667,6 +4705,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel counterparty_max_accepted_htlcs, minimum_depth, + counterparty_forwarding_info, + channel_transaction_parameters: channel_parameters, counterparty_cur_commitment_point, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 3042b4d32e0..004bd3d99d3 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -543,7 +543,14 @@ pub struct UnsignedChannelUpdate { pub timestamp: u32, /// Channel flags pub flags: u8, - /// The number of blocks to subtract from incoming HTLC cltv_expiry values + /// The number of blocks such that if: + /// `incoming_htlc.cltv_expiry < outgoing_htlc.cltv_expiry + cltv_expiry_delta` + /// then we need to fail the HTLC backwards. When forwarding an HTLC, cltv_expiry_delta determines + /// the outgoing HTLC's minimum cltv_expiry value -- so, if an incoming HTLC comes in with a + /// cltv_expiry of 100000, and the node we're forwarding to has a cltv_expiry_delta value of 10, + /// then we'll check that the outgoing HTLC's cltv_expiry value is at least 100010 before + /// forwarding. Note that the HTLC sender is the one who originally sets this value when + /// constructing the route. pub cltv_expiry_delta: u16, /// The minimum HTLC size incoming to sender, in milli-satoshi pub htlc_minimum_msat: u64, From e8a0824dd4b62288805f0e6cf435e52fd022b7c2 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 12 Mar 2021 15:25:56 -0500 Subject: [PATCH 2/3] Pass channel updates to ChannelManager and Channel. This will be used to expose forwarding info for route hints in the next commit. Co-authored-by: Valentine Wallace Co-authored-by: Antoine Riard --- lightning-net-tokio/src/lib.rs | 1 + lightning/src/ln/channel.rs | 14 ++++++++++++++ lightning/src/ln/channelmanager.rs | 28 ++++++++++++++++++++++++++++ lightning/src/ln/msgs.rs | 3 +++ lightning/src/ln/peer_handler.rs | 3 +++ lightning/src/util/test_utils.rs | 1 + 6 files changed, 50 insertions(+) diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index 8c98333080f..9dc99e4776d 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -561,6 +561,7 @@ mod tests { fn handle_revoke_and_ack(&self, _their_node_id: &PublicKey, _msg: &RevokeAndACK) {} fn handle_update_fee(&self, _their_node_id: &PublicKey, _msg: &UpdateFee) {} fn handle_announcement_signatures(&self, _their_node_id: &PublicKey, _msg: &AnnouncementSignatures) {} + fn handle_channel_update(&self, _their_node_id: &PublicKey, _msg: &ChannelUpdate) {} fn peer_disconnected(&self, their_node_id: &PublicKey, _no_connection_possible: bool) { if *their_node_id == self.expected_pubkey { self.disconnected_flag.store(true, Ordering::SeqCst); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 426083dd832..ce048f0eeb9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4132,6 +4132,20 @@ impl Channel { } } + pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> { + let usable_channel_value_msat = (self.channel_value_satoshis - self.counterparty_selected_channel_reserve_satoshis) * 1000; + if msg.contents.htlc_minimum_msat >= usable_channel_value_msat { + return Err(ChannelError::Close("Minimum htlc value is greater than channel value".to_string())); + } + self.counterparty_forwarding_info = Some(CounterpartyForwardingInfo { + fee_base_msat: msg.contents.fee_base_msat, + fee_proportional_millionths: msg.contents.fee_proportional_millionths, + cltv_expiry_delta: msg.contents.cltv_expiry_delta + }); + + Ok(()) + } + /// Begins the shutdown process, getting a message for the remote peer and returning all /// holding cell HTLCs for payment failure. pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index faae2c51726..6702db0bd9a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2994,6 +2994,29 @@ impl ChannelMana Ok(()) } + fn internal_channel_update(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelUpdate) -> Result<(), MsgHandleErrInternal> { + let mut channel_state_lock = self.channel_state.lock().unwrap(); + let channel_state = &mut *channel_state_lock; + let chan_id = match channel_state.short_to_id.get(&msg.contents.short_channel_id) { + Some(chan_id) => chan_id.clone(), + None => { + // It's not a local channel + return Ok(()) + } + }; + match channel_state.by_id.entry(chan_id) { + hash_map::Entry::Occupied(mut chan) => { + if chan.get().get_counterparty_node_id() != *counterparty_node_id { + // TODO: see issue #153, need a consistent behavior on obnoxious behavior from random node + return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), chan_id)); + } + try_chan_entry!(self, chan.get_mut().channel_update(&msg), channel_state, chan); + }, + hash_map::Entry::Vacant(_) => unreachable!() + } + Ok(()) + } + fn internal_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), MsgHandleErrInternal> { let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_state_lock; @@ -3517,6 +3540,11 @@ impl PeerManager { + self.message_handler.chan_handler.handle_channel_update(&peer.their_node_id.unwrap(), &msg); let should_forward = match self.message_handler.route_handler.handle_channel_update(&msg) { Ok(v) => v, Err(e) => { return Err(e.into()); }, diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index ea110023823..a0ccf4f816e 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -231,6 +231,7 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler { fn handle_commitment_signed(&self, _their_node_id: &PublicKey, _msg: &msgs::CommitmentSigned) {} fn handle_revoke_and_ack(&self, _their_node_id: &PublicKey, _msg: &msgs::RevokeAndACK) {} fn handle_update_fee(&self, _their_node_id: &PublicKey, _msg: &msgs::UpdateFee) {} + fn handle_channel_update(&self, _their_node_id: &PublicKey, _msg: &msgs::ChannelUpdate) {} fn handle_announcement_signatures(&self, _their_node_id: &PublicKey, _msg: &msgs::AnnouncementSignatures) {} fn handle_channel_reestablish(&self, _their_node_id: &PublicKey, _msg: &msgs::ChannelReestablish) {} fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {} From c318ad87e004b7082124239d3b70d77787a55c77 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 12 Mar 2021 16:02:17 -0500 Subject: [PATCH 3/3] Expose counterparty forwarding info in ChannelDetails. Useful for constructing route hints for private channels in invoices. Co-authored-by: Valentine Wallace Co-authored-by: Antoine Riard --- fuzz/src/router.rs | 1 + lightning/src/ln/channel.rs | 57 +++++++++++++++++++++++++++++- lightning/src/ln/channelmanager.rs | 8 +++++ lightning/src/routing/router.rs | 7 ++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 44c0b209c2c..e93618ca713 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -212,6 +212,7 @@ pub fn do_test(data: &[u8], out: Out) { inbound_capacity_msat: 0, is_live: true, outbound_capacity_msat: 0, + counterparty_forwarding_info: None, }); } Some(&first_hops_vec[..]) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ce048f0eeb9..a6049e915e9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -282,6 +282,7 @@ impl HTLCCandidate { } /// Information needed for constructing an invoice route hint for this channel. +#[derive(Clone)] pub struct CounterpartyForwardingInfo { /// Base routing fee in millisatoshis. pub fee_base_msat: u32, @@ -4132,6 +4133,11 @@ impl Channel { } } + /// Get forwarding information for the counterparty. + pub fn counterparty_forwarding_info(&self) -> Option { + self.counterparty_forwarding_info.clone() + } + pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> { let usable_channel_value_msat = (self.channel_value_satoshis - self.counterparty_selected_channel_reserve_satoshis) * 1000; if msg.contents.htlc_minimum_msat >= usable_channel_value_msat { @@ -4756,7 +4762,7 @@ mod tests { use ln::channel::{Channel,Sign,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys}; use ln::channel::MAX_FUNDING_SATOSHIS; use ln::features::InitFeatures; - use ln::msgs::{OptionalField, DataLossProtect, DecodeError}; + use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate}; use ln::chan_utils; use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT}; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; @@ -4767,6 +4773,7 @@ mod tests { use util::test_utils; use util::logger::Logger; use bitcoin::secp256k1::{Secp256k1, Message, Signature, All}; + use bitcoin::secp256k1::ffi::Signature as FFISignature; use bitcoin::secp256k1::key::{SecretKey,PublicKey}; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; @@ -5023,6 +5030,54 @@ mod tests { } } + #[test] + fn channel_update() { + let feeest = TestFeeEstimator{fee_est: 15000}; + let secp_ctx = Secp256k1::new(); + let seed = [42; 32]; + let network = Network::Testnet; + let chain_hash = genesis_block(network).header.block_hash(); + let keys_provider = test_utils::TestKeysInterface::new(&seed, network); + + // Create a channel. + let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let config = UserConfig::default(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + assert!(node_a_chan.counterparty_forwarding_info.is_none()); + assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default + assert!(node_a_chan.counterparty_forwarding_info().is_none()); + + // Make sure that receiving a channel update will update the Channel as expected. + let update = ChannelUpdate { + contents: UnsignedChannelUpdate { + chain_hash, + short_channel_id: 0, + timestamp: 0, + flags: 0, + cltv_expiry_delta: 100, + htlc_minimum_msat: 5, + htlc_maximum_msat: OptionalField::Absent, + fee_base_msat: 110, + fee_proportional_millionths: 11, + excess_data: Vec::new(), + }, + signature: Signature::from(unsafe { FFISignature::new() }) + }; + node_a_chan.channel_update(&update).unwrap(); + + // The counterparty can send an update with a higher minimum HTLC, but that shouldn't + // change our official htlc_minimum_msat. + assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); + match node_a_chan.counterparty_forwarding_info() { + Some(info) => { + assert_eq!(info.cltv_expiry_delta, 100); + assert_eq!(info.fee_base_msat, 110); + assert_eq!(info.fee_proportional_millionths, 11); + }, + None => panic!("expected counterparty forwarding info to be Some") + } + } + #[test] fn outbound_commitment_test() { // Test vectors from BOLT 3 Appendix C: diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6702db0bd9a..02ac8641372 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -39,6 +39,9 @@ use chain::Watch; use chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID}; use chain::transaction::{OutPoint, TransactionData}; +// Since this struct is returned in `list_channels` methods, expose it here in case users want to +// construct one themselves. +pub use ln::channel::CounterpartyForwardingInfo; use ln::channel::{Channel, ChannelError}; use ln::features::{InitFeatures, NodeFeatures}; use routing::router::{Route, RouteHop}; @@ -574,6 +577,10 @@ pub struct ChannelDetails { /// True if the channel is (a) confirmed and funding_locked messages have been exchanged, (b) /// the peer is connected, and (c) no monitor update failure is pending resolution. pub is_live: bool, + + /// Information on the fees and requirements that the counterparty requires when forwarding + /// payments to us through this channel. + pub counterparty_forwarding_info: Option, } /// If a payment fails to send, it can be in one of several states. This enum is returned as the @@ -892,6 +899,7 @@ impl ChannelMana outbound_capacity_msat, user_id: channel.get_user_id(), is_live: channel.is_live(), + counterparty_forwarding_info: channel.counterparty_forwarding_info(), }); } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 682302481b6..c9d7cc25321 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1473,6 +1473,7 @@ mod tests { outbound_capacity_msat: 100000, inbound_capacity_msat: 100000, is_live: true, + counterparty_forwarding_info: None, }]; if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)) { @@ -1790,6 +1791,7 @@ mod tests { outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, is_live: true, + counterparty_forwarding_info: None, }]; let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -1837,6 +1839,7 @@ mod tests { outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, is_live: true, + counterparty_forwarding_info: None, }]; let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -1901,6 +1904,7 @@ mod tests { outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, is_live: true, + counterparty_forwarding_info: None, }]; let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -2037,6 +2041,7 @@ mod tests { outbound_capacity_msat: 250_000_000, inbound_capacity_msat: 0, is_live: true, + counterparty_forwarding_info: None, }]; let mut last_hops = last_hops(&nodes); let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, Some(&our_chans.iter().collect::>()), &last_hops.iter().collect::>(), 100, 42, Arc::clone(&logger)).unwrap(); @@ -2165,6 +2170,7 @@ mod tests { outbound_capacity_msat: 100000, inbound_capacity_msat: 100000, is_live: true, + counterparty_forwarding_info: None, }]; let route = get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, None, Some(&our_chans.iter().collect::>()), &last_hops.iter().collect::>(), 100, 42, Arc::new(test_utils::TestLogger::new())).unwrap(); @@ -2296,6 +2302,7 @@ mod tests { outbound_capacity_msat: 200_000_000, inbound_capacity_msat: 0, is_live: true, + counterparty_forwarding_info: None, }]; {