Expose API to update HTLC relay policy

A new `update_htlc_relay_policy` method is exposed on the
`ChannelManger` to update the HTLC relay policy for a set of channels
atomically. New ChannelUpdate events are generated for each eligible
channel.

Note that as currently implemented, a buggy and/or
auto-policy-management client could spam the network with updates as
there is no rate-limiting in place. This could already be done with
`broadcast_node_announcement`, though users are less inclined to update
that as frequently as its data is mostly static.

Signed-off-by: Wilmer Paulino <git@w.paulino.io>

Author: Wilmer Paulino

lightning/src/ln/channel.rs

@@ -39,7 +39,7 @@ use util::events::ClosureReason;
 use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use util::logger::Logger;
 use util::errors::APIError;
-use util::config::{UserConfig, ChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits};
+use util::config::{UserConfig, ChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits, HTLCRelayPolicyUpdate};
 use util::scid_utils::scid_from_parts;
 
 use io;
@@ -4482,6 +4482,36 @@ impl<Signer: Sign> Channel<Signer> {
 		self.config.max_dust_htlc_exposure_msat
 	}
 
+	/// Updates the channel's relay policy for forwarding HTLCs. A bool is returned indicating
+	/// whether the updates applied resulted in a policy change.
+	pub fn update_htlc_relay_policy(&mut self, updates: &[HTLCRelayPolicyUpdate]) -> bool {
+		let default = ChannelConfig::default();
+		let prev_base_fee = self.get_outbound_forwarding_fee_base_msat();
+		let prev_proportional_fee = self.get_fee_proportional_millionths();
+		let prev_cltv_expiry_delta = self.get_cltv_expiry_delta();
+		for update in updates {
+			match update {
+				HTLCRelayPolicyUpdate::ForwardingFeeProportionalMillionths(val) =>
+					self.config.forwarding_fee_proportional_millionths =
+						val.unwrap_or(default.forwarding_fee_proportional_millionths),
+				HTLCRelayPolicyUpdate::ForwardingFeeBaseMsat(val) =>
+					self.config.forwarding_fee_base_msat =
+						val.unwrap_or(default.forwarding_fee_base_msat),
+				HTLCRelayPolicyUpdate::CltvExpiryDelta(val) =>
+					self.config.cltv_expiry_delta = val.unwrap_or(default.cltv_expiry_delta),
+			}
+		}
+		let did_change = prev_base_fee != self.get_outbound_forwarding_fee_base_msat() ||
+			prev_proportional_fee != self.get_fee_proportional_millionths() ||
+			prev_cltv_expiry_delta != self.get_cltv_expiry_delta();
+		if did_change {
+			// Update the counter, which backs the ChannelUpdate timestamp, to allow the relay
+			// policy change to propagate throughout the network.
+			self.update_time_counter += 1;
+		}
+		did_change
+	}
+
 	pub fn get_feerate(&self) -> u32 {
 		self.feerate_per_kw
 	}

lightning/src/ln/channelmanager.rs

@@ -51,7 +51,7 @@ use ln::onion_utils;
 use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT, OptionalField};
 use ln::wire::Encode;
 use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner, Recipient};
-use util::config::UserConfig;
+use util::config::{UserConfig, HTLCRelayPolicyUpdate};
 use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
 use util::{byte_utils, events};
 use util::scid_utils::fake_scid;
@@ -2920,6 +2920,65 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		}
 	}
 
+	/// Atomically updates the relay policy for forwarding HTLCs on the given channels.
+	///
+	/// Once the updates are applied, each eligible channel (advertised with a known short channel
+	/// ID and a change in relay policy) has a [`BroadcastChannelUpdate`] event message generated
+	/// containing the new [`ChannelUpdate`] message which should be broadcast to the network.
+	///
+	/// Returns [`ChannelUnavailable`] when a channel is not found. None of the updates should be
+	/// considered applied.
+	///
+	/// Returns [`APIMisuseError`] when a [`CltvExpiryDelta`] update is to be applied with a value
+	/// below [`MIN_CLTV_EXPIRY_DELTA`].
+	///
+	/// [`BroadcastChannelUpdate`]: events::MessageSendEvent::BroadcastChannelUpdate
+	/// [`ChannelUpdate`]: msgs::ChannelUpdate
+	/// [`ChannelUnavailable`]: APIError::ChannelUnavailable
+	/// [`APIMisuseError`]: APIError::APIMisuseError
+	/// [`CltvExpiryDelta`]: HTLCRelayPolicyUpdate::CltvExpiryDelta
+	pub fn update_htlc_relay_policy(
+		&self, channel_ids: &[[u8; 32]], updates: &[HTLCRelayPolicyUpdate]
+	) -> Result<(), APIError> {
+		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(
+			&self.total_consistency_lock, &self.persistence_notifier,
+		);
+		{
+			let mut channel_state_lock = self.channel_state.lock().unwrap();
+			let channel_state = &mut *channel_state_lock;
+			for channel_id in channel_ids {
+				if !channel_state.by_id.contains_key(channel_id) {
+					return Err(APIError::ChannelUnavailable {
+						err: format!("Channel {} not found", log_bytes!(*channel_id)),
+					});
+				}
+			}
+			for update in updates {
+				match update {
+					HTLCRelayPolicyUpdate::CltvExpiryDelta(Some(val)) =>
+						if *val < MIN_CLTV_EXPIRY_DELTA {
+							return Err(APIError::APIMisuseError { err:
+								format!("cltv expiry delta below minimum {}", MIN_CLTV_EXPIRY_DELTA),
+							});
+						},
+					_ => continue,
+				}
+			}
+			for channel_id in channel_ids {
+				let channel = channel_state.by_id.get_mut(channel_id).unwrap();
+				if !channel.update_htlc_relay_policy(updates) {
+					continue;
+				}
+				if let Ok(msg) = self.get_channel_update_for_broadcast(channel) {
+					channel_state.pending_msg_events.push(
+						events::MessageSendEvent::BroadcastChannelUpdate { msg },
+					);
+				}
+			}
+		}
+		Ok(())
+	}
+
 	/// Processes HTLCs which are pending waiting on random forward delay.
 	///
 	/// Should only really ever be called in response to a PendingHTLCsForwardable event.

lightning/src/ln/onion_route_tests.rs

@@ -25,7 +25,8 @@ use ln::wire::Encode;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
 use util::ser::{Writeable, Writer};
 use util::{byte_utils, test_utils};
-use util::config::UserConfig;
+use util::config::{UserConfig, HTLCRelayPolicyUpdate};
+use util::errors::APIError;
 
 use bitcoin::hash_types::BlockHash;
 
@@ -506,8 +507,6 @@ fn test_onion_failure() {
 	let preimage = send_along_route(&nodes[0], bogus_route, &[&nodes[1], &nodes[2]], amt_to_forward+1).0;
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], preimage);
 
-	//TODO: with new config API, we will be able to generate both valid and
-	//invalid channel_update cases.
 	let short_channel_id = channels[0].0.contents.short_channel_id;
 	run_onion_failure_test("fee_insufficient", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
 		msg.amount_msat -= 1;
@@ -594,6 +593,93 @@ fn test_onion_failure() {
 	}, true, Some(23), None, None);
 }
 
+#[test]
+fn test_onion_failure_stale_channel_update() {
+    // Create a network of three nodes and two channels connecting them. We'll be updating the
+    // second channel with insane HTLC relay parameters, causing forwarding failures at the first
+    // hop.
+    let chanmon_cfgs = create_chanmon_cfgs(3);
+    let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+    let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+    let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+    create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+    let channel_to_update = create_announced_chan_between_nodes(
+        &nodes, 1, 2, InitFeatures::known(), InitFeatures::known(),
+    );
+
+    // A test payment should succeed as the HTLC relay paramters have not been changed yet.
+    const PAYMENT_AMT: u64 = 40000;
+    send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], PAYMENT_AMT);
+
+    // Closure to update and retrieve the latest ChannelUpdate.
+    let update_and_get_channel_update =
+		|updates: &[HTLCRelayPolicyUpdate], expect_new_update: bool,
+		prev_update: Option<&msgs::ChannelUpdate>| -> Option<msgs::ChannelUpdate> {
+			nodes[1].node.update_htlc_relay_policy(&[channel_to_update.2], updates).unwrap();
+			let events = nodes[1].node.get_and_clear_pending_msg_events();
+			assert_eq!(events.len(), expect_new_update as usize);
+			if !expect_new_update {
+				return None;
+			}
+			let new_update = match &events[0] {
+				MessageSendEvent::BroadcastChannelUpdate { msg, .. } => msg.clone(),
+				_ => panic!("expected BroadcastChannelUpdate event"),
+			};
+			if prev_update.is_some() {
+				assert!(new_update.contents.timestamp > prev_update.unwrap().contents.timestamp)
+			}
+			Some(new_update)
+		};
+
+	// We'll be attempting to route payments using the default ChannelUpdate for channels. This will
+	// lead to onion failures at the first hop once we update the HTLC relay parameters for the
+	// second hop.
+	let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(
+		nodes[0], nodes[2], PAYMENT_AMT
+	);
+	let expect_onion_failure = |name: &str, error_code: u16, channel_update: &msgs::ChannelUpdate| {
+		let short_channel_id = channel_to_update.0.contents.short_channel_id;
+		let network_update = NetworkUpdate::ChannelUpdateMessage { msg: channel_update.clone() };
+		run_onion_failure_test(
+			name, 0, &nodes, &route, &payment_hash, &payment_secret, |_| {}, || {}, true,
+			Some(error_code), Some(network_update), Some(short_channel_id),
+		);
+	};
+
+	// Updates to cltv_expiry_delta below MIN_CLTV_EXPIRY_DELTA should fail with APIMisuseError.
+	let updates = &[HTLCRelayPolicyUpdate::CltvExpiryDelta(Some(MIN_CLTV_EXPIRY_DELTA - 1))];
+	match nodes[1].node.update_htlc_relay_policy(&[channel_to_update.2], updates) {
+		Err(APIError::APIMisuseError{ .. }) => {},
+		_ => panic!("unexpected result applying invalid cltv_expiry_delta"),
+	}
+
+	// Increase the base fee which should trigger a new ChannelUpdate.
+	let updates = &[HTLCRelayPolicyUpdate::ForwardingFeeBaseMsat(Some(u32::MAX))];
+	let msg = update_and_get_channel_update(updates, true, None).unwrap();
+	expect_onion_failure("fee_insufficient", UPDATE|12, &msg);
+
+	// Redundant updates should not trigger a new ChannelUpdate.
+	assert!(update_and_get_channel_update(updates, false, None).is_none());
+
+	// Reset the base fee to the default and increase the proportional fee which should trigger a
+	// new ChannelUpdate.
+	let updates = &[
+		HTLCRelayPolicyUpdate::ForwardingFeeBaseMsat(None),
+		HTLCRelayPolicyUpdate::CltvExpiryDelta(Some(u16::MAX)),
+	];
+	let msg = update_and_get_channel_update(updates, true, Some(&msg)).unwrap();
+	expect_onion_failure("incorrect_cltv_expiry", UPDATE|13, &msg);
+
+	// Reset the proportional fee and increase the CLTV expiry delta which should trigger a new
+	// ChannelUpdate.
+	let updates = &[
+		HTLCRelayPolicyUpdate::CltvExpiryDelta(None),
+		HTLCRelayPolicyUpdate::ForwardingFeeProportionalMillionths(Some(u32::MAX)),
+	];
+	let msg = update_and_get_channel_update(updates, true, Some(&msg)).unwrap();
+	expect_onion_failure("fee_insufficient", UPDATE|12, &msg);
+}
+
 #[test]
 fn test_default_to_onion_payload_tlv_format() {
 	// Tests that we default to creating tlv format onion payloads when no `NodeAnnouncementInfo`

lightning/src/util/config.rs

@@ -355,6 +355,45 @@ impl_writeable_tlv_based!(ChannelConfig, {
 	(8, forwarding_fee_base_msat, required),
 });
 
+/// Updates that can be made to a channel's relay policy for forwarding HTLCs. Each variant is
+/// represented as an Option, and when None is used, the variant's default value will be applied.
+pub enum HTLCRelayPolicyUpdate {
+	/// Amount (in millionths of a satoshi) charged per satoshi for payments forwarded outbound
+	/// over the channel.
+	///
+	/// Default value: 0.
+	ForwardingFeeProportionalMillionths(Option<u32>),
+	/// Amount (in milli-satoshi) charged for payments forwarded outbound over the channel, in
+	/// excess of [`forwarding_fee_proportional_millionths`].
+	/// This may be allowed to change at runtime in a later update, however doing so must result in
+	/// update messages sent to notify all nodes of our updated relay fee.
+	///
+	/// Default value: 1000.
+	///
+	/// [`forwarding_fee_proportional_millionths`]: ChannelConfig::forwarding_fee_proportional_millionths
+	ForwardingFeeBaseMsat(Option<u32>),
+	/// The difference in the CLTV value between incoming HTLCs and an outbound HTLC forwarded over
+	/// the channel this config applies to.
+	///
+	/// This is analogous to [`ChannelHandshakeConfig::our_to_self_delay`] but applies to in-flight
+	/// HTLC balance when a channel appears on-chain whereas
+	/// [`ChannelHandshakeConfig::our_to_self_delay`] applies to the remaining
+	/// (non-HTLC-encumbered) balance.
+	///
+	/// Thus, for HTLC-encumbered balances to be enforced on-chain when a channel is force-closed,
+	/// we (or one of our watchtowers) MUST be online to check for broadcast of the current
+	/// commitment transaction at least once per this many blocks (minus some margin to allow us
+	/// enough time to broadcast and confirm a transaction, possibly with time in between to RBF
+	/// the spending transaction).
+	///
+	/// Default value: 72 (12 hours at an average of 6 blocks/hour).
+	/// Minimum value: [`MIN_CLTV_EXPIRY_DELTA`], any values less than this will be treated as
+	///                [`MIN_CLTV_EXPIRY_DELTA`] instead.
+	///
+	/// [`MIN_CLTV_EXPIRY_DELTA`]: crate::ln::channelmanager::MIN_CLTV_EXPIRY_DELTA
+	CltvExpiryDelta(Option<u16>),
+}
+
 /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig.
 ///
 /// Default::default() provides sane defaults for most configurations