Move forwarding_fee_proportional_millionths, forwarding_fee_base_msat to ChannelHandshakeConfig

fuzz/src/chanmon_consistency.rs

@@ -354,7 +354,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 				Arc::new(TestPersister { update_ret: Mutex::new(Ok(())) }), Arc::clone(&keys_manager)));
 
 			let mut config = UserConfig::default();
-			config.channel_options.forwarding_fee_proportional_millionths = 0;
+			config.own_channel_config.forwarding_fee_proportional_millionths = 0;
 			config.channel_options.announced_channel = true;
 			let network = Network::Bitcoin;
 			let params = ChainParameters {
@@ -374,7 +374,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 				Arc::new(TestPersister { update_ret: Mutex::new(Ok(())) }), Arc::clone(& $keys_manager)));
 
 			let mut config = UserConfig::default();
-			config.channel_options.forwarding_fee_proportional_millionths = 0;
+			config.own_channel_config.forwarding_fee_proportional_millionths = 0;
 			config.channel_options.announced_channel = true;
 
 			let mut monitors = HashMap::new();

fuzz/src/full_stack.rs

@@ -379,7 +379,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 
 	let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()), counter: AtomicU64::new(0) });
 	let mut config = UserConfig::default();
-	config.channel_options.forwarding_fee_proportional_millionths =  slice_to_be32(get_slice!(4));
+	config.own_channel_config.forwarding_fee_proportional_millionths =  slice_to_be32(get_slice!(4));
 	config.channel_options.announced_channel = get_slice!(1)[0] != 0;
 	let network = Network::Bitcoin;
 	let params = ChainParameters {

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};
+use util::config::{UserConfig,ChannelConfig,ChannelHandshakeConfig};
 use util::scid_utils::scid_from_parts;
 
 use io;
@@ -426,6 +426,13 @@ pub(super) struct Channel<Signer: Sign> {
 	#[cfg(not(any(test, feature = "_test_utils")))]
 	config: ChannelConfig,
 
+	#[cfg(any(test, feature = "_test_utils"))]
+	pub(crate) forwarding_fee_base_msat: u32,
+	#[cfg(not(any(test, feature = "_test_utils")))]
+	forwarding_fee_base_msat: u32,
+	forwarding_fee_proportional_millionths: u32,
+	cltv_expiry_delta: u16,
+
 	user_id: u64,
 
 	channel_id: [u8; 32],
@@ -760,11 +767,16 @@ impl<Signer: Sign> Channel<Signer> {
 				return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() });
 			}
 		}
+		let handshake_config = config.own_channel_config.clone();
 
 		Ok(Channel {
 			user_id,
 			config: config.channel_options.clone(),
 
+			forwarding_fee_base_msat: handshake_config.forwarding_fee_base_msat,
+			forwarding_fee_proportional_millionths: handshake_config.forwarding_fee_proportional_millionths,
+			cltv_expiry_delta: handshake_config.cltv_expiry_delta,
+
 			channel_id: keys_provider.get_secure_random_bytes(),
 			channel_state: ChannelState::OurInitSent as u32,
 			secp_ctx,
@@ -929,6 +941,7 @@ impl<Signer: Sign> Channel<Signer> {
 			htlc_basepoint: msg.htlc_basepoint
 		};
 		let mut local_config = (*config).channel_options.clone();
+		let handshake_config = (*config).own_channel_config.clone();
 
 		if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
 			return Err(ChannelError::Close(format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks. It must be greater than {}", config.own_channel_config.our_to_self_delay, BREAKDOWN_TIMEOUT)));
@@ -1063,6 +1076,10 @@ impl<Signer: Sign> Channel<Signer> {
 			user_id,
 			config: local_config,
 
+			forwarding_fee_base_msat: handshake_config.forwarding_fee_base_msat,
+			forwarding_fee_proportional_millionths: handshake_config.forwarding_fee_proportional_millionths,
+			cltv_expiry_delta: handshake_config.cltv_expiry_delta,
+
 			channel_id: msg.temporary_channel_id,
 			channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
 			secp_ctx,
@@ -4052,11 +4069,11 @@ impl<Signer: Sign> Channel<Signer> {
 	}
 
 	pub fn get_fee_proportional_millionths(&self) -> u32 {
-		self.config.forwarding_fee_proportional_millionths
+		self.forwarding_fee_proportional_millionths
 	}
 
 	pub fn get_cltv_expiry_delta(&self) -> u16 {
-		cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA)
+		cmp::max(self.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA)
 	}
 
 	pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 {
@@ -4147,7 +4164,7 @@ impl<Signer: Sign> Channel<Signer> {
 	/// Gets the fee we'd want to charge for adding an HTLC output to this Channel
 	/// Allowed in any state (including after shutdown)
 	pub fn get_outbound_forwarding_fee_base_msat(&self) -> u32 {
-		self.config.forwarding_fee_base_msat
+		self.forwarding_fee_base_msat
 	}
 
 	/// Returns true if we've ever received a message from the remote end for this Channel
@@ -5188,8 +5205,8 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		// Write out the old serialization for the config object. This is read by version-1
 		// deserializers, but we will read the version in the TLV at the end instead.
-		self.config.forwarding_fee_proportional_millionths.write(writer)?;
-		self.config.cltv_expiry_delta.write(writer)?;
+		self.forwarding_fee_proportional_millionths.write(writer)?;
+		self.cltv_expiry_delta.write(writer)?;
 		self.config.announced_channel.write(writer)?;
 		self.config.commit_upfront_shutdown_pubkey.write(writer)?;
 
@@ -5450,17 +5467,18 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		let user_id = Readable::read(reader)?;
 
 		let mut config = Some(ChannelConfig::default());
+		let handshake_config = Some(ChannelHandshakeConfig::default());
+
 		if ver == 1 {
-			// Read the old serialization of the ChannelConfig from version 0.0.98.
-			config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?;
-			config.as_mut().unwrap().cltv_expiry_delta = Readable::read(reader)?;
+			// Read the old serialization of the ChannelConfig from version 0.0.98.handshake_config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?;
 			config.as_mut().unwrap().announced_channel = Readable::read(reader)?;
 			config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?;
 		} else {
 			// Read the 8 bytes of backwards-compatibility ChannelConfig data.
 			let mut _val: u64 = Readable::read(reader)?;
 		}
-
+
+		let cltv_expiry_delta = Readable::read(reader)?;
 		let channel_id = Readable::read(reader)?;
 		let channel_state = Readable::read(reader)?;
 		let channel_value_satoshis = Readable::read(reader)?;
@@ -5708,6 +5726,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			user_id,
 
 			config: config.unwrap(),
+			forwarding_fee_base_msat: handshake_config.unwrap().forwarding_fee_base_msat,
+			forwarding_fee_proportional_millionths: handshake_config.unwrap().forwarding_fee_proportional_millionths,
+			cltv_expiry_delta: handshake_config.unwrap().cltv_expiry_delta,
+
 			channel_id,
 			channel_state,
 			secp_ctx,

lightning/src/ln/functional_test_utils.rs

@@ -1468,7 +1468,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
 			($node: expr, $prev_node: expr, $new_msgs: expr) => {
 				{
 					$node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
-					let fee = $node.node.channel_state.lock().unwrap().by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap().config.forwarding_fee_base_msat;
+					let fee = $node.node.channel_state.lock().unwrap().by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap().forwarding_fee_base_msat;
 					expect_payment_forwarded!($node, Some(fee as u64), false);
 					expected_total_fee_msat += fee as u64;
 					check_added_monitors!($node, 1);
@@ -1731,7 +1731,7 @@ pub fn test_default_channel_config() -> UserConfig {
 	let mut default_config = UserConfig::default();
 	// Set cltv_expiry_delta slightly lower to keep the final CLTV values inside one byte in our
 	// tests so that our script-length checks don't fail (see ACCEPTED_HTLC_SCRIPT_WEIGHT).
-	default_config.channel_options.cltv_expiry_delta = 6*6;
+	default_config.own_channel_config.cltv_expiry_delta = 6*6;
 	default_config.channel_options.announced_channel = true;
 	default_config.peer_channel_config_limits.force_announced_channel_preference = false;
 	// When most of our tests were written, the default HTLC minimum was fixed at 1000.

lightning/src/ln/functional_tests.rs

@@ -1787,7 +1787,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
 	// When this test was written, the default base fee floated based on the HTLC count.
 	// It is now fixed, so we simply set the fee to the expected value here.
 	let mut config = test_default_channel_config();
-	config.channel_options.forwarding_fee_base_msat = 239;
+	config.own_channel_config.forwarding_fee_base_msat = 239;
 	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config.clone())]);
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 	let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 190000, 1001, InitFeatures::known(), InitFeatures::known());
@@ -5135,7 +5135,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 	// When this test was written, the default base fee floated based on the HTLC count.
 	// It is now fixed, so we simply set the fee to the expected value here.
 	let mut config = test_default_channel_config();
-	config.channel_options.forwarding_fee_base_msat = 196;
+	config.own_channel_config.forwarding_fee_base_msat = 196;
 	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs,
 		&[Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone())]);
 	let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);
@@ -5332,7 +5332,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 	// When this test was written, the default base fee floated based on the HTLC count.
 	// It is now fixed, so we simply set the fee to the expected value here.
 	let mut config = test_default_channel_config();
-	config.channel_options.forwarding_fee_base_msat = 196;
+	config.own_channel_config.forwarding_fee_base_msat = 196;
 	let node_chanmgrs = create_node_chanmgrs(6, &node_cfgs,
 		&[Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone()), Some(config.clone())]);
 	let nodes = create_network(6, &node_cfgs, &node_chanmgrs);
@@ -6208,7 +6208,7 @@ fn test_fail_holding_cell_htlc_upon_free_multihop() {
 	// When this test was written, the default base fee floated based on the HTLC count.
 	// It is now fixed, so we simply set the fee to the expected value here.
 	let mut config = test_default_channel_config();
-	config.channel_options.forwarding_fee_base_msat = 196;
+	config.own_channel_config.forwarding_fee_base_msat = 196;
 	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config.clone())]);
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 	let chan_0_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());

lightning/src/ln/onion_route_tests.rs

@@ -258,7 +258,7 @@ fn test_fee_failures() {
 	// When this test was written, the default base fee floated based on the HTLC count.
 	// It is now fixed, so we simply set the fee to the expected value here.
 	let mut config = test_default_channel_config();
-	config.channel_options.forwarding_fee_base_msat = 196;
+	config.own_channel_config.forwarding_fee_base_msat = 196;
 
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
@@ -310,7 +310,7 @@ fn test_onion_failure() {
 	// When this test was written, the default base fee floated based on the HTLC count.
 	// It is now fixed, so we simply set the fee to the expected value here.
 	let mut config = test_default_channel_config();
-	config.channel_options.forwarding_fee_base_msat = 196;
+	config.own_channel_config.forwarding_fee_base_msat = 196;
 
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);

lightning/src/ln/payment_tests.rs

@@ -448,7 +448,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
 	// Update the fee on the middle hop to ensure PaymentSent events have the correct (retried) fee
 	// and not the original fee. We also update node[1]'s relevant config as
 	// do_claim_payment_along_route expects us to never overpay.
-	nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&chan_id_2).unwrap().config.forwarding_fee_base_msat += 100_000;
+	nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&chan_id_2).unwrap().forwarding_fee_base_msat += 100_000;
 	new_route.paths[0][0].fee_msat += 100_000;
 
 	assert!(nodes[0].node.retry_payment(&new_route, payment_id_1).is_err()); // Shouldn't be allowed to retry a fulfilled payment

lightning/src/util/config.rs

@@ -47,6 +47,46 @@ pub struct ChannelHandshakeConfig {
 	/// Default value: 1. If the value is less than 1, it is ignored and set to 1, as is required
 	/// by the protocol.
 	pub our_htlc_minimum_msat: u64,
+	/// Amount (in millionths of a satoshi) charged per satoshi for payments forwarded outbound
+	/// over the channel.
+	/// 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: 0.
+	pub forwarding_fee_proportional_millionths: 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.
+	///
+	/// The default value of a single satoshi roughly matches the market rate on many routing nodes
+	/// as of July 2021. Adjusting it upwards or downwards may change whether nodes route through
+	/// this node.
+	///
+	/// Default value: 1000.
+	///
+	/// [`forwarding_fee_proportional_millionths`]: ChannelHandshakeConfig::forwarding_fee_proportional_millionths
+	pub forwarding_fee_base_msat: 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
+	pub cltv_expiry_delta: u16,
 }
 
 impl Default for ChannelHandshakeConfig {
@@ -55,6 +95,9 @@ impl Default for ChannelHandshakeConfig {
 			minimum_depth: 6,
 			our_to_self_delay: BREAKDOWN_TIMEOUT,
 			our_htlc_minimum_msat: 1,
+			forwarding_fee_base_msat: 1000,
+			forwarding_fee_proportional_millionths: 0,
+			cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours
 		}
 	}
 }
@@ -143,46 +186,6 @@ impl Default for ChannelHandshakeLimits {
 /// with our counterparty.
 #[derive(Copy, Clone, Debug)]
 pub struct ChannelConfig {
-	/// Amount (in millionths of a satoshi) charged per satoshi for payments forwarded outbound
-	/// over the channel.
-	/// 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: 0.
-	pub forwarding_fee_proportional_millionths: 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.
-	///
-	/// The default value of a single satoshi roughly matches the market rate on many routing nodes
-	/// as of July 2021. Adjusting it upwards or downwards may change whether nodes route through
-	/// this node.
-	///
-	/// Default value: 1000.
-	///
-	/// [`forwarding_fee_proportional_millionths`]: ChannelConfig::forwarding_fee_proportional_millionths
-	pub forwarding_fee_base_msat: 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
-	pub cltv_expiry_delta: u16,
 	/// Set to announce the channel publicly and notify all nodes that they can route via this
 	/// channel.
 	///
@@ -252,9 +255,6 @@ impl Default for ChannelConfig {
 	/// Provides sane defaults for most configurations (but with zero relay fees!).
 	fn default() -> Self {
 		ChannelConfig {
-			forwarding_fee_proportional_millionths: 0,
-			forwarding_fee_base_msat: 1000,
-			cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours
 			announced_channel: false,
 			commit_upfront_shutdown_pubkey: true,
 			max_dust_htlc_exposure_msat: 5_000_000,
@@ -264,13 +264,10 @@ impl Default for ChannelConfig {
 }
 
 impl_writeable_tlv_based!(ChannelConfig, {
-	(0, forwarding_fee_proportional_millionths, required),
 	(1, max_dust_htlc_exposure_msat, (default_value, 5_000_000)),
-	(2, cltv_expiry_delta, required),
 	(3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)),
 	(4, announced_channel, required),
 	(6, commit_upfront_shutdown_pubkey, required),
-	(8, forwarding_fee_base_msat, required),
 });
 
 /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig.