Fix plain channel interoperability with stock LDK

lightning/src/ln/chan_utils.rs

@@ -277,17 +277,15 @@ impl HTLCClaim {
 }
 
 #[cfg(not(any(test, feature = "_test_utils")))]
-const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
+pub(crate) const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
 #[cfg(any(test, feature = "_test_utils"))]
 pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
 
 #[rustfmt::skip]
 pub(crate) fn commitment_tx_base_weight(channel_type_features: &ChannelTypeFeatures) -> u64 {
 	const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
 	const COMMITMENT_TX_BASE_ANCHOR_WEIGHT: u64 = 1124;
-	let base_weight = if channel_type_features.supports_anchors_zero_fee_htlc_tx() { COMMITMENT_TX_BASE_ANCHOR_WEIGHT } else { COMMITMENT_TX_BASE_WEIGHT };
-	// add OP_RETURN weight (RGB coloring)
-	base_weight + 172
+	if channel_type_features.supports_anchors_zero_fee_htlc_tx() { COMMITMENT_TX_BASE_ANCHOR_WEIGHT } else { COMMITMENT_TX_BASE_WEIGHT }
 }
 
 /// Get the fee cost of a commitment tx with a given number of HTLC outputs.

lightning/src/ln/channel.rs

@@ -3534,9 +3534,9 @@ where
 		// check if the funder's amount for the initial commitment tx is sufficient
 		// for full fee payment plus a few HTLCs to ensure the channel will be useful.
 		let funders_amount_msat = open_channel_fields.funding_satoshis * 1000 - msg_push_msat;
-		let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
+		let commit_tx_fee_sat = SpecTxBuilder::new(rgb_asset.is_some()).commit_tx_fee_sat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
 		// Subtract any non-HTLC outputs from the remote balance
-		let (_, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs(false, value_to_self_msat, funders_amount_msat, &channel_type);
+		let (_, remote_balance_before_fee_msat) = SpecTxBuilder::new(rgb_asset.is_some()).subtract_non_htlc_outputs(false, value_to_self_msat, funders_amount_msat, &channel_type);
 		if remote_balance_before_fee_msat / 1000 < commit_tx_fee_sat {
 			return Err(ChannelError::close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", funders_amount_msat / 1000, commit_tx_fee_sat)));
 		}
@@ -3808,9 +3808,9 @@ where
 		);
 
 		let value_to_self_msat = channel_value_satoshis * 1000 - push_msat;
-		let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
+		let commit_tx_fee_sat = SpecTxBuilder::new(rgb_asset.is_some()).commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
 		// Subtract any non-HTLC outputs from the local balance
-		let (local_balance_before_fee_msat, _) = SpecTxBuilder {}.subtract_non_htlc_outputs(
+		let (local_balance_before_fee_msat, _) = SpecTxBuilder::new(rgb_asset.is_some()).subtract_non_htlc_outputs(
 			true,
 			value_to_self_msat,
 			push_msat,
@@ -4724,7 +4724,7 @@ where
 		);
 		let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(true, funding);
 
-		let ret = SpecTxBuilder {}.get_next_commitment_stats(
+		let ret = SpecTxBuilder::new(funding.is_colored()).get_next_commitment_stats(
 			true,
 			funding.is_outbound(),
 			funding.get_value_satoshis(),
@@ -4746,7 +4746,7 @@ where
 					predicted_fee_sat: ret.commit_tx_fee_sat,
 				};
 			} else {
-				let predicted_stats = SpecTxBuilder {}
+				let predicted_stats = SpecTxBuilder::new(funding.is_colored())
 					.get_next_commitment_stats(
 						true,
 						funding.is_outbound(),
@@ -4783,7 +4783,7 @@ where
 		);
 		let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(false, funding);
 
-		let ret = SpecTxBuilder {}.get_next_commitment_stats(
+		let ret = SpecTxBuilder::new(funding.is_colored()).get_next_commitment_stats(
 			false,
 			funding.is_outbound(),
 			funding.get_value_satoshis(),
@@ -4805,7 +4805,7 @@ where
 					predicted_fee_sat: ret.commit_tx_fee_sat,
 				};
 			} else {
-				let predicted_stats = SpecTxBuilder {}
+				let predicted_stats = SpecTxBuilder::new(funding.is_colored())
 					.get_next_commitment_stats(
 						false,
 						funding.is_outbound(),
@@ -5453,7 +5453,7 @@ where
 
 		let value_to_self_msat = (funding.value_to_self_msat + value_to_self_claimed_msat).checked_sub(value_to_remote_claimed_msat).unwrap();
 
-		let (tx, stats) = SpecTxBuilder {}.build_commitment_transaction(
+		let (tx, stats) = SpecTxBuilder::new(funding.is_colored()).build_commitment_transaction(
 			local,
 			commitment_number,
 			per_commitment_point,
@@ -5639,10 +5639,10 @@ where
 		}
 
 		let extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat = excess_feerate_opt.map(|excess_feerate| {
-			let extra_htlc_commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1 + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type());
+			let extra_htlc_commit_tx_fee_sat = SpecTxBuilder::new(funding.is_colored()).commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1 + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type());
 			let extra_htlc_htlc_tx_fees_sat = chan_utils::htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type());
 
-			let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type());
+			let commit_tx_fee_sat = SpecTxBuilder::new(funding.is_colored()).commit_tx_fee_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type());
 			let htlc_tx_fees_sat = chan_utils::htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type());
 
 			let extra_htlc_dust_exposure = on_counterparty_tx_dust_exposure_msat + (extra_htlc_commit_tx_fee_sat + extra_htlc_htlc_tx_fees_sat) * 1000;
@@ -5770,7 +5770,7 @@ where
 		let htlc_stats = context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate);
 
 		// Subtract any non-HTLC outputs from the local and remote balances
-		let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = SpecTxBuilder {}.subtract_non_htlc_outputs(
+		let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = SpecTxBuilder::new(funding.is_colored()).subtract_non_htlc_outputs(
 			funding.is_outbound(),
 			funding.value_to_self_msat.saturating_sub(htlc_stats.pending_outbound_htlcs_value_msat),
 			(funding.get_value_satoshis() * 1000).checked_sub(funding.value_to_self_msat).unwrap().saturating_sub(htlc_stats.pending_inbound_htlcs_value_msat),
@@ -5987,7 +5987,7 @@ where
 		}
 
 		let num_htlcs = included_htlcs + addl_htlcs;
-		SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
+		SpecTxBuilder::new(funding.is_colored()).commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
 	}
 
 	/// Get the commitment tx fee for the remote's next commitment transaction based on the number of
@@ -6064,7 +6064,7 @@ where
 		}
 
 		let num_htlcs = included_htlcs + addl_htlcs;
-		SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
+		SpecTxBuilder::new(funding.is_colored()).commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
 	}
 
 	#[rustfmt::skip]

lightning/src/ln/htlc_reserve_unit_tests.rs

@@ -896,7 +896,7 @@ pub fn do_test_fee_spike_buffer(cfg: Option<UserConfig>, htlc_fails: bool) {
 		let channel = get_channel_ref!(nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan.2);
 		let chan_signer = channel.as_funded().unwrap().get_signer();
 
-		let (commitment_tx, _stats) = SpecTxBuilder {}.build_commitment_transaction(
+		let (commitment_tx, _stats) = SpecTxBuilder::new(false).build_commitment_transaction(
 			false,
 			commitment_number,
 			&remote_point,

lightning/src/ln/msgs.rs

@@ -3397,14 +3397,15 @@ impl_writeable_msg!(UpdateAddHTLC, {
 	amount_msat,
 	payment_hash,
 	cltv_expiry,
-	onion_routing_packet,
-	rgb_payment
+	onion_routing_packet
 }, {
 	(0, blinding_point, option),
 	(65537, skimmed_fee_msat, option),
 	// TODO: currently we may fail to read the `ChannelManager` if we write a new even TLV in this message
 	// and then downgrade. Once this is fixed, update the type here to match BOLTs PR 989.
 	(75537, hold_htlc, option),
+	// odd type outside the range the BOLTs may assign, so that peers not supporting RGB ignore it
+	(85537, rgb_payment, option),
 });
 
 impl LengthReadable for OnionMessage {
@@ -4025,7 +4026,9 @@ impl Writeable for UnsignedChannelAnnouncement {
 		self.node_id_2.write(w)?;
 		self.bitcoin_key_1.write(w)?;
 		self.bitcoin_key_2.write(w)?;
-		self.contract_id.write(w)?;
+		if self.contract_id.is_some() {
+			self.contract_id.write(w)?;
+		}
 		w.write_all(&self.excess_data[..])?;
 		Ok(())
 	}
@@ -4041,7 +4044,7 @@ impl LengthReadable for UnsignedChannelAnnouncement {
 			node_id_2: Readable::read(r)?,
 			bitcoin_key_1: Readable::read(r)?,
 			bitcoin_key_2: Readable::read(r)?,
-			contract_id: Readable::read(r)?,
+			contract_id: if r.remaining_bytes() > 0 { Readable::read(r)? } else { None },
 			excess_data: read_to_end(r)?,
 		})
 	}
@@ -4084,7 +4087,9 @@ impl Writeable for UnsignedChannelUpdate {
 		self.fee_base_msat.write(w)?;
 		self.fee_proportional_millionths.write(w)?;
 		self.htlc_maximum_msat.write(w)?;
-		self.htlc_maximum_rgb.write(w)?;
+		if self.htlc_maximum_rgb > 0 {
+			self.htlc_maximum_rgb.write(w)?;
+		}
 		w.write_all(&self.excess_data[..])?;
 		Ok(())
 	}
@@ -4103,7 +4108,7 @@ impl LengthReadable for UnsignedChannelUpdate {
 			fee_base_msat: Readable::read(r)?,
 			fee_proportional_millionths: Readable::read(r)?,
 			htlc_maximum_msat: Readable::read(r)?,
-			htlc_maximum_rgb: Readable::read(r)?,
+			htlc_maximum_rgb: if r.remaining_bytes() >= 8 { Readable::read(r)? } else { 0 },
 			excess_data: read_to_end(r)?,
 		};
 		if res.message_flags & 1 != 1 {
@@ -4715,6 +4720,7 @@ mod tests {
 			node_id_2: NodeId::from_pubkey(&pubkey_2),
 			bitcoin_key_1: NodeId::from_pubkey(&pubkey_3),
 			bitcoin_key_2: NodeId::from_pubkey(&pubkey_4),
+			contract_id: None,
 			excess_data: if excess_data {
 				vec![10, 0, 0, 20, 0, 0, 30, 0, 0, 40]
 			} else {
@@ -4913,6 +4919,7 @@ mod tests {
 			cltv_expiry_delta: 144,
 			htlc_minimum_msat: 1000000,
 			htlc_maximum_msat: 131355275467161,
+			htlc_maximum_rgb: 0x0000777788889999,
 			fee_base_msat: 10000,
 			fee_proportional_millionths: 20,
 			excess_data: if excess_data { vec![0, 0, 0, 0, 59, 154, 202, 0] } else { Vec::new() },

lightning/src/rgb_utils/mod.rs

@@ -6,8 +6,9 @@
 compile_error!("at least one of the `electrum` and `esplora` features needs to be enabled");
 
 use crate::ln::chan_utils::{
-	get_countersigner_payment_script, BuiltCommitmentTransaction, ClosingTransaction,
-	CommitmentTransaction, HTLCOutputInCommitment,
+	commitment_tx_base_weight, get_countersigner_payment_script, BuiltCommitmentTransaction,
+	ClosingTransaction, CommitmentTransaction, HTLCOutputInCommitment,
+	COMMITMENT_TX_WEIGHT_PER_HTLC,
 };
 use crate::ln::channel::{ChannelContext, ChannelError, FundingScope};
 use crate::ln::channel_state::ChannelDetails;
@@ -221,6 +222,23 @@ pub fn is_tx_colored(tx: &Transaction) -> bool {
 	op_return_position(tx).is_some()
 }
 
+/// Weight of the OP_RETURN output coloring a commitment transaction: 8-byte value, 1-byte script
+/// length and 34-byte `OP_RETURN OP_PUSHBYTES_32 <commitment>` script
+const COMMITMENT_TX_OP_RETURN_WEIGHT: u64 = 172;
+
+/// Get the fee cost of a colored commitment tx with a given number of HTLC outputs, which includes
+/// the weight of the OP_RETURN output.
+/// Note that num_htlcs should not include dust HTLCs.
+pub(crate) fn colored_commit_tx_fee_sat(
+	feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures,
+) -> u64 {
+	feerate_per_kw as u64
+		* (commitment_tx_base_weight(channel_type_features)
+			+ COMMITMENT_TX_OP_RETURN_WEIGHT
+			+ num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC)
+		/ 1000
+}
+
 /// Color commitment transaction
 pub(crate) fn color_commitment<SP: Deref>(
 	channel_context: &ChannelContext<SP>, funding_scope: &FundingScope,

lightning/src/sign/tx_builder.rs

@@ -13,6 +13,7 @@ use crate::ln::chan_utils::{
 };
 use crate::ln::channel::{CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI};
 use crate::prelude::*;
+use crate::rgb_utils::colored_commit_tx_fee_sat;
 use crate::types::features::ChannelTypeFeatures;
 use crate::util::logger::Logger;
 
@@ -67,8 +68,9 @@ impl NextCommitmentStats {
 }
 
 fn commit_plus_htlc_tx_fees_msat(
-	local: bool, next_commitment_htlcs: &[HTLCAmountDirection], dust_buffer_feerate: u32,
-	feerate: u32, broadcaster_dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures,
+	tx_builder: &SpecTxBuilder, local: bool, next_commitment_htlcs: &[HTLCAmountDirection],
+	dust_buffer_feerate: u32, feerate: u32, broadcaster_dust_limit_satoshis: u64,
+	channel_type: &ChannelTypeFeatures,
 ) -> (u64, u64) {
 	let accepted_nondust_htlcs = next_commitment_htlcs
 		.iter()
@@ -95,13 +97,16 @@ fn commit_plus_htlc_tx_fees_msat(
 		})
 		.count();
 
-	let commitment_fee_sat =
-		commit_tx_fee_sat(feerate, accepted_nondust_htlcs + offered_nondust_htlcs, channel_type);
+	let commitment_fee_sat = tx_builder.commit_tx_fee_sat(
+		feerate,
+		accepted_nondust_htlcs + offered_nondust_htlcs,
+		channel_type,
+	);
 	let second_stage_fees_sat =
 		htlc_tx_fees_sat(feerate, accepted_nondust_htlcs, offered_nondust_htlcs, channel_type);
 	let total_fees_msat = (commitment_fee_sat + second_stage_fees_sat) * 1000;
 
-	let extra_accepted_htlc_commitment_fee_sat = commit_tx_fee_sat(
+	let extra_accepted_htlc_commitment_fee_sat = tx_builder.commit_tx_fee_sat(
 		feerate,
 		accepted_nondust_htlcs + 1 + offered_nondust_htlcs,
 		channel_type,
@@ -179,7 +184,15 @@ pub(crate) trait TxBuilder {
 		L::Target: Logger;
 }
 
-pub(crate) struct SpecTxBuilder {}
+pub(crate) struct SpecTxBuilder {
+	is_colored: bool,
+}
+
+impl SpecTxBuilder {
+	pub(crate) fn new(is_colored: bool) -> Self {
+		Self { is_colored }
+	}
+}
 
 impl TxBuilder for SpecTxBuilder {
 	fn get_next_commitment_stats(
@@ -236,7 +249,7 @@ impl TxBuilder for SpecTxBuilder {
 				!htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type)
 			})
 			.count();
-		let commit_tx_fee_sat = commit_tx_fee_sat(
+		let commit_tx_fee_sat = self.commit_tx_fee_sat(
 			feerate_per_kw,
 			nondust_htlc_count + addl_nondust_htlc_count,
 			channel_type,
@@ -262,6 +275,7 @@ impl TxBuilder for SpecTxBuilder {
 		} else {
 			let (excess_fees_msat, extra_accepted_htlc_excess_fees_msat) =
 				commit_plus_htlc_tx_fees_msat(
+					self,
 					local,
 					&next_commitment_htlcs,
 					dust_buffer_feerate,
@@ -290,7 +304,11 @@ impl TxBuilder for SpecTxBuilder {
 	fn commit_tx_fee_sat(
 		&self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures,
 	) -> u64 {
-		commit_tx_fee_sat(feerate_per_kw, nondust_htlc_count, channel_type)
+		if self.is_colored {
+			colored_commit_tx_fee_sat(feerate_per_kw, nondust_htlc_count, channel_type)
+		} else {
+			commit_tx_fee_sat(feerate_per_kw, nondust_htlc_count, channel_type)
+		}
 	}
 	fn subtract_non_htlc_outputs(
 		&self, is_outbound_from_holder: bool, value_to_self_after_htlcs: u64,