Merge pull request #841 from valentinewallace/207-replacement
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 17 Mar 2021 22:41:30 +0000 (22:41 +0000)
committerGitHub <noreply@github.com>
Wed, 17 Mar 2021 22:41:30 +0000 (22:41 +0000)
Expose counterparty forwarding info in ChannelDetails

fuzz/src/router.rs
lightning-net-tokio/src/lib.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/msgs.rs
lightning/src/ln/peer_handler.rs
lightning/src/routing/router.rs
lightning/src/util/test_utils.rs

index 44c0b209c2cdd0d9cebcb9e413d1a7e763a3f519..e93618ca7137905ab7677a775e3697e390b6cd02 100644 (file)
@@ -212,6 +212,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                                                inbound_capacity_msat: 0,
                                                                is_live: true,
                                                                outbound_capacity_msat: 0,
+                                                               counterparty_forwarding_info: None,
                                                        });
                                                }
                                                Some(&first_hops_vec[..])
index 8c98333080f941b5102bf415e9cb4e54a17a8a76..9dc99e4776da7bf281e0295b2cdb6cc1ff074239 100644 (file)
@@ -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);
index 13b2dbb1a1c6a7b9f54b996e03ab6acc6bdcbf11..0cde1aa3e6e9af4374261be045c76d74cc8e9a7d 100644 (file)
@@ -282,6 +282,19 @@ 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,
+       /// 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
@@ -392,6 +405,8 @@ pub(super) struct Channel<Signer: Sign> {
        //implied by OUR_MAX_HTLCS: max_accepted_htlcs: u16,
        minimum_depth: u32,
 
+       counterparty_forwarding_info: Option<CounterpartyForwardingInfo>,
+
        pub(crate) channel_transaction_parameters: ChannelTransactionParameters,
 
        counterparty_cur_commitment_point: Option<PublicKey>,
@@ -578,6 +593,8 @@ impl<Signer: Sign> Channel<Signer> {
                        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,
@@ -814,6 +831,8 @@ impl<Signer: Sign> Channel<Signer> {
                        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,
@@ -4112,6 +4131,25 @@ impl<Signer: Sign> Channel<Signer> {
                }
        }
 
+       /// Get forwarding information for the counterparty.
+       pub fn counterparty_forwarding_info(&self) -> Option<CounterpartyForwardingInfo> {
+               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 {
+                       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> {
@@ -4435,6 +4473,16 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                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)?;
 
@@ -4595,6 +4643,16 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                let counterparty_max_accepted_htlcs = Readable::read(reader)?;
                let minimum_depth = Readable::read(reader)?;
 
+               let counterparty_forwarding_info = match <u8 as Readable>::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)?;
 
@@ -4665,6 +4723,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        counterparty_max_accepted_htlcs,
                        minimum_depth,
 
+                       counterparty_forwarding_info,
+
                        channel_transaction_parameters: channel_parameters,
                        counterparty_cur_commitment_point,
 
@@ -4700,7 +4760,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};
@@ -4711,6 +4771,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;
@@ -4967,6 +5028,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::<EnforcingSigner>::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:
index 970ad2d18a8542620daf54a7b6b24be1f9202cb2..45849c920219edb74cae08e360262bc3eca19532 100644 (file)
@@ -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<CounterpartyForwardingInfo>,
 }
 
 /// 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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        outbound_capacity_msat,
                                        user_id: channel.get_user_id(),
                                        is_live: channel.is_live(),
+                                       counterparty_forwarding_info: channel.counterparty_forwarding_info(),
                                });
                        }
                }
@@ -2994,6 +3002,29 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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 +3548,11 @@ impl<Signer: Sign, M: Deref + Sync + Send, T: Deref + Sync + Send, K: Deref + Sy
                let _ = handle_error!(self, self.internal_announcement_signatures(counterparty_node_id, msg), *counterparty_node_id);
        }
 
+       fn handle_channel_update(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelUpdate) {
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+               let _ = handle_error!(self, self.internal_channel_update(counterparty_node_id, msg), *counterparty_node_id);
+       }
+
        fn handle_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) {
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
                let _ = handle_error!(self, self.internal_channel_reestablish(counterparty_node_id, msg), *counterparty_node_id);
index a9c4bc3d6f545c3ceda25ce4795d11aac9ecde91..01a4378456498d7c20706784da60ea13790e3d26 100644 (file)
@@ -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,
@@ -789,6 +796,9 @@ pub trait ChannelMessageHandler : MessageSendEventsProvider + Send + Sync {
        /// Handle an incoming channel_reestablish message from the given peer.
        fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &ChannelReestablish);
 
+       /// Handle an incoming channel update from the given peer.
+       fn handle_channel_update(&self, their_node_id: &PublicKey, msg: &ChannelUpdate);
+
        // Error:
        /// Handle an incoming error message from the given peer.
        fn handle_error(&self, their_node_id: &PublicKey, msg: &ErrorMessage);
index 0e3f0ed4602a164a30189bc5ba0617f9be3451e1..9488a34db0d7e04d322b787bc47501be5cba0c80 100644 (file)
@@ -142,6 +142,8 @@ impl ChannelMessageHandler for ErroringMessageHandler {
        fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) {
                ErroringMessageHandler::push_error(self, their_node_id, msg.channel_id);
        }
+       // msgs::ChannelUpdate does not contain the channel_id field, so we just drop them.
+       fn handle_channel_update(&self, _their_node_id: &PublicKey, _msg: &msgs::ChannelUpdate) {}
        fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {}
        fn peer_connected(&self, _their_node_id: &PublicKey, _msg: &msgs::Init) {}
        fn handle_error(&self, _their_node_id: &PublicKey, _msg: &msgs::ErrorMessage) {}
@@ -970,6 +972,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
                                }
                        },
                        wire::Message::ChannelUpdate(msg) => {
+                               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()); },
index 682302481b653f47b1a5485d50dcbd015050f1ec..c9d7cc253217e28578ad0b26e58cef4665f7dbbe 100644 (file)
@@ -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<_>>()), &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<_>>()),  &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<_>>()), &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<_>>()), &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::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 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::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 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,
                }];
 
                {
index ea110023823242c0e3ae5999e353db8c7c61f1b1..a0ccf4f816ea991e31a019351018d43a95633324 100644 (file)
@@ -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) {}