Merge pull request #841 from valentinewallace/207-replacement

Expose counterparty forwarding info in ChannelDetails

Author: TheBlueMatt

fuzz/src/router.rs

@@ -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[..])

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);

lightning/src/ln/channel.rs

@@ -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:

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<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);

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,
@@ -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);

lightning/src/ln/peer_handler.rs

@@ -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()); },

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<_>>()), &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,
 		}];
 
 		{

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) {}