Merge pull request #2485 from optout21/channel-id-4struct1

Introduce new ChannelId struct

Author: TheBlueMatt

fuzz/src/full_stack.rs

@@ -34,7 +34,7 @@ use lightning::chain::chainmonitor;
 use lightning::chain::transaction::OutPoint;
 use lightning::sign::{InMemorySigner, Recipient, KeyMaterial, EntropySource, NodeSigner, SignerProvider};
 use lightning::events::Event;
-use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
+use lightning::ln::{ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentId, RecipientOnionFields, Retry};
 use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,IgnoringMessageHandler};
 use lightning::ln::msgs::{self, DecodeError};
@@ -481,7 +481,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 	let mut should_forward = false;
 	let mut payments_received: Vec<PaymentHash> = Vec::new();
 	let mut payments_sent = 0;
-	let mut pending_funding_generation: Vec<([u8; 32], PublicKey, u64, Script)> = Vec::new();
+	let mut pending_funding_generation: Vec<(ChannelId, PublicKey, u64, Script)> = Vec::new();
 	let mut pending_funding_signatures = HashMap::new();
 
 	loop {

fuzz/src/router.rs

@@ -13,6 +13,7 @@ use bitcoin::hash_types::BlockHash;
 
 use lightning::blinded_path::{BlindedHop, BlindedPath};
 use lightning::chain::transaction::OutPoint;
+use lightning::ln::ChannelId;
 use lightning::ln::channelmanager::{self, ChannelDetails, ChannelCounterparty};
 use lightning::ln::features::{BlindedHopFeatures, Bolt12InvoiceFeatures};
 use lightning::ln::msgs;
@@ -210,7 +211,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 						let rnid = node_pks.iter().skip(u16::from_be_bytes(get_slice!(2).try_into().unwrap()) as usize % node_pks.len()).next().unwrap();
 						let capacity = u64::from_be_bytes(get_slice!(8).try_into().unwrap());
 						$first_hops_vec.push(ChannelDetails {
-							channel_id: [0; 32],
+							channel_id: ChannelId::new_zero(),
 							counterparty: ChannelCounterparty {
 								node_id: *rnid,
 								features: channelmanager::provided_init_features(&UserConfig::default()),

lightning-invoice/src/utils.rs

@@ -627,7 +627,7 @@ where
 	log_trace!(logger, "Considering {} channels for invoice route hints", channels.len());
 	for channel in channels.into_iter().filter(|chan| chan.is_channel_ready) {
 		if channel.get_inbound_payment_scid().is_none() || channel.counterparty.forwarding_info.is_none() {
-			log_trace!(logger, "Ignoring channel {} for invoice route hints", log_bytes!(channel.channel_id));
+			log_trace!(logger, "Ignoring channel {} for invoice route hints", &channel.channel_id);
 			continue;
 		}
 
@@ -641,7 +641,7 @@ where
 				// If any public channel exists, return no hints and let the sender
 				// look at the public channels instead.
 				log_trace!(logger, "Not including channels in invoice route hints on account of public channel {}",
-					log_bytes!(channel.channel_id));
+					&channel.channel_id);
 				return vec![].into_iter().take(MAX_CHANNEL_HINTS).map(route_hint_from_channel);
 			}
 		}
@@ -681,18 +681,18 @@ where
 					log_trace!(logger,
 						"Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",
 						log_pubkey!(channel.counterparty.node_id),
-						log_bytes!(channel.channel_id), channel.short_channel_id,
+						&channel.channel_id, channel.short_channel_id,
 						channel.inbound_capacity_msat,
-						log_bytes!(entry.get().channel_id), entry.get().short_channel_id,
+						&entry.get().channel_id, entry.get().short_channel_id,
 						current_max_capacity);
 					entry.insert(channel);
 				} else {
 					log_trace!(logger,
 						"Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",
 						log_pubkey!(channel.counterparty.node_id),
-						log_bytes!(entry.get().channel_id), entry.get().short_channel_id,
+						&entry.get().channel_id, entry.get().short_channel_id,
 						current_max_capacity,
-						log_bytes!(channel.channel_id), channel.short_channel_id,
+						&channel.channel_id, channel.short_channel_id,
 						channel.inbound_capacity_msat);
 				}
 			}
@@ -731,14 +731,14 @@ where
 
 			if include_channel {
 				log_trace!(logger, "Including channel {} in invoice route hints",
-					log_bytes!(channel.channel_id));
+					&channel.channel_id);
 			} else if !has_enough_capacity {
 				log_trace!(logger, "Ignoring channel {} without enough capacity for invoice route hints",
-					log_bytes!(channel.channel_id));
+					&channel.channel_id);
 			} else {
 				debug_assert!(!channel.is_usable || (has_pub_unconf_chan && !channel.is_public));
 				log_trace!(logger, "Ignoring channel {} with disconnected peer",
-					log_bytes!(channel.channel_id));
+					&channel.channel_id);
 			}
 
 			include_channel

lightning/src/chain/channelmonitor.rs

@@ -2642,7 +2642,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						}
 					} else if !self.holder_tx_signed {
 						log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast");
-						log_error!(logger, "    in channel monitor for channel {}!", log_bytes!(self.funding_info.0.to_channel_id()));
+						log_error!(logger, "    in channel monitor for channel {}!", &self.funding_info.0.to_channel_id());
 						log_error!(logger, "    Read the docs for ChannelMonitor::get_latest_holder_commitment_txn and take manual action!");
 					} else {
 						// If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
@@ -3389,7 +3389,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				if prevout.txid == self.funding_info.0.txid && prevout.vout == self.funding_info.0.index as u32 {
 					let mut balance_spendable_csv = None;
 					log_info!(logger, "Channel {} closed by funding output spend in txid {}.",
-						log_bytes!(self.funding_info.0.to_channel_id()), txid);
+						&self.funding_info.0.to_channel_id(), txid);
 					self.funding_spend_seen = true;
 					let mut commitment_tx_to_counterparty_output = None;
 					if (tx.input[0].sequence.0 >> 8*3) as u8 == 0x80 && (tx.lock_time.0 >> 8*3) as u8 == 0x20 {

lightning/src/chain/transaction.rs

@@ -9,7 +9,9 @@
 
 //! Types describing on-chain transactions.
 
+use crate::ln::ChannelId;
 use bitcoin::hash_types::Txid;
+use bitcoin::hashes::Hash;
 use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::transaction::Transaction;
 
@@ -57,12 +59,8 @@ pub struct OutPoint {
 
 impl OutPoint {
 	/// Convert an `OutPoint` to a lightning channel id.
-	pub fn to_channel_id(&self) -> [u8; 32] {
-		let mut res = [0; 32];
-		res[..].copy_from_slice(&self.txid[..]);
-		res[30] ^= ((self.index >> 8) & 0xff) as u8;
-		res[31] ^= ((self.index >> 0) & 0xff) as u8;
-		res
+	pub fn to_channel_id(&self) -> ChannelId {
+		ChannelId::v1_from_funding_txid(&self.txid.as_inner(), self.index)
 	}
 
 	/// Converts this OutPoint into the OutPoint field as used by rust-bitcoin
@@ -94,10 +92,10 @@ mod tests {
 		assert_eq!(&OutPoint {
 			txid: tx.txid(),
 			index: 0
-		}.to_channel_id(), &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25e").unwrap()[..]);
+		}.to_channel_id().0[..], &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25e").unwrap()[..]);
 		assert_eq!(&OutPoint {
 			txid: tx.txid(),
 			index: 1
-		}.to_channel_id(), &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25f").unwrap()[..]);
+		}.to_channel_id().0[..], &hex::decode("3e88dd7165faf7be58b3c5bb2c9c452aebef682807ea57080f62e6f6e113c25f").unwrap()[..]);
 	}
 }

lightning/src/events/mod.rs

@@ -23,7 +23,7 @@ use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields};
 use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
 use crate::ln::features::ChannelTypeFeatures;
 use crate::ln::msgs;
-use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+use crate::ln::{ChannelId, PaymentPreimage, PaymentHash, PaymentSecret};
 use crate::routing::gossip::NetworkUpdate;
 use crate::util::errors::APIError;
 use crate::util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, RequiredWrapper, UpgradableRequired, WithoutLength};
@@ -83,7 +83,7 @@ impl_writeable_tlv_based_enum!(PaymentPurpose,
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ClaimedHTLC {
 	/// The `channel_id` of the channel over which the HTLC was received.
-	pub channel_id: [u8; 32],
+	pub channel_id: ChannelId,
 	/// The `user_channel_id` of the channel over which the HTLC was received. This is the value
 	/// passed in to [`ChannelManager::create_channel`] for outbound channels, or to
 	/// [`ChannelManager::accept_inbound_channel`] for inbound channels if
@@ -246,7 +246,7 @@ pub enum HTLCDestination {
 		/// counterparty node information.
 		node_id: Option<PublicKey>,
 		/// The outgoing `channel_id` between us and the next node.
-		channel_id: [u8; 32],
+		channel_id: ChannelId,
 	},
 	/// Scenario where we are unsure of the next node to forward the HTLC to.
 	UnknownNextHop {
@@ -364,7 +364,7 @@ pub enum Event {
 		/// [`ChannelManager::funding_transaction_generated`].
 		///
 		/// [`ChannelManager::funding_transaction_generated`]: crate::ln::channelmanager::ChannelManager::funding_transaction_generated
-		temporary_channel_id: [u8; 32],
+		temporary_channel_id: ChannelId,
 		/// The counterparty's node_id, which you'll need to pass back into
 		/// [`ChannelManager::funding_transaction_generated`].
 		///
@@ -458,7 +458,7 @@ pub enum Event {
 		/// payment is to pay an invoice or to send a spontaneous payment.
 		purpose: PaymentPurpose,
 		/// The `channel_id` indicating over which channel we received the payment.
-		via_channel_id: Option<[u8; 32]>,
+		via_channel_id: Option<ChannelId>,
 		/// The `user_channel_id` indicating over which channel we received the payment.
 		via_user_channel_id: Option<u128>,
 		/// The block height at which this payment will be failed back and will no longer be
@@ -718,17 +718,17 @@ pub enum Event {
 		/// The `channel_id` indicating which channel the spendable outputs belong to.
 		///
 		/// This will always be `Some` for events generated by LDK versions 0.0.117 and above.
-		channel_id: Option<[u8; 32]>,
+		channel_id: Option<ChannelId>,
 	},
 	/// This event is generated when a payment has been successfully forwarded through us and a
 	/// forwarding fee earned.
 	PaymentForwarded {
 		/// The incoming channel between the previous node and us. This is only `None` for events
 		/// generated or serialized by versions prior to 0.0.107.
-		prev_channel_id: Option<[u8; 32]>,
+		prev_channel_id: Option<ChannelId>,
 		/// The outgoing channel between the next node and us. This is only `None` for events
 		/// generated or serialized by versions prior to 0.0.107.
-		next_channel_id: Option<[u8; 32]>,
+		next_channel_id: Option<ChannelId>,
 		/// The fee, in milli-satoshis, which was earned as a result of the payment.
 		///
 		/// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC
@@ -759,7 +759,7 @@ pub enum Event {
 	/// [`Event::ChannelReady`] event.
 	ChannelPending {
 		/// The `channel_id` of the channel that is pending confirmation.
-		channel_id: [u8; 32],
+		channel_id: ChannelId,
 		/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
 		/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
 		/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
@@ -772,7 +772,7 @@ pub enum Event {
 		/// The `temporary_channel_id` this channel used to be known by during channel establishment.
 		///
 		/// Will be `None` for channels created prior to LDK version 0.0.115.
-		former_temporary_channel_id: Option<[u8; 32]>,
+		former_temporary_channel_id: Option<ChannelId>,
 		/// The `node_id` of the channel counterparty.
 		counterparty_node_id: PublicKey,
 		/// The outpoint of the channel's funding transaction.
@@ -784,7 +784,7 @@ pub enum Event {
 	/// establishment.
 	ChannelReady {
 		/// The `channel_id` of the channel that is ready.
-		channel_id: [u8; 32],
+		channel_id: ChannelId,
 		/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
 		/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
 		/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
@@ -811,7 +811,7 @@ pub enum Event {
 	ChannelClosed  {
 		/// The `channel_id` of the channel which has been closed. Note that on-chain transactions
 		/// resolving the channel are likely still awaiting confirmation.
-		channel_id: [u8; 32],
+		channel_id: ChannelId,
 		/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
 		/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
 		/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
@@ -838,7 +838,7 @@ pub enum Event {
 	/// inputs for another purpose.
 	DiscardFunding {
 		/// The channel_id of the channel which has been closed.
-		channel_id: [u8; 32],
+		channel_id: ChannelId,
 		/// The full transaction received from the user
 		transaction: Transaction
 	},
@@ -863,7 +863,7 @@ pub enum Event {
 		///
 		/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
 		/// [`ChannelManager::force_close_without_broadcasting_txn`]: crate::ln::channelmanager::ChannelManager::force_close_without_broadcasting_txn
-		temporary_channel_id: [u8; 32],
+		temporary_channel_id: ChannelId,
 		/// The node_id of the counterparty requesting to open the channel.
 		///
 		/// When responding to the request, the `counterparty_node_id` should be passed
@@ -909,7 +909,7 @@ pub enum Event {
 	/// requirements (i.e. insufficient fees paid, or a CLTV that is too soon).
 	HTLCHandlingFailed {
 		/// The channel over which the HTLC was received.
-		prev_channel_id: [u8; 32],
+		prev_channel_id: ChannelId,
 		/// Destination of the HTLC that failed to be processed.
 		failed_next_destination: HTLCDestination,
 	},
@@ -1279,7 +1279,7 @@ impl MaybeReadable for Event {
 			5u8 => {
 				let f = || {
 					let mut outputs = WithoutLength(Vec::new());
-					let mut channel_id: Option<[u8; 32]> = None;
+					let mut channel_id: Option<ChannelId> = None;
 					read_tlv_fields!(reader, {
 						(0, outputs, required),
 						(1, channel_id, option),
@@ -1335,7 +1335,7 @@ impl MaybeReadable for Event {
 			},
 			9u8 => {
 				let f = || {
-					let mut channel_id = [0; 32];
+					let mut channel_id = ChannelId::new_zero();
 					let mut reason = UpgradableRequired(None);
 					let mut user_channel_id_low_opt: Option<u64> = None;
 					let mut user_channel_id_high_opt: Option<u64> = None;
@@ -1363,7 +1363,7 @@ impl MaybeReadable for Event {
 			},
 			11u8 => {
 				let f = || {
-					let mut channel_id = [0; 32];
+					let mut channel_id = ChannelId::new_zero();
 					let mut transaction = Transaction{ version: 2, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: Vec::new() };
 					read_tlv_fields!(reader, {
 						(0, channel_id, required),
@@ -1474,7 +1474,7 @@ impl MaybeReadable for Event {
 			},
 			25u8 => {
 				let f = || {
-					let mut prev_channel_id = [0; 32];
+					let mut prev_channel_id = ChannelId::new_zero();
 					let mut failed_next_destination_opt = UpgradableRequired(None);
 					read_tlv_fields!(reader, {
 						(0, prev_channel_id, required),
@@ -1490,7 +1490,7 @@ impl MaybeReadable for Event {
 			27u8 => Ok(None),
 			29u8 => {
 				let f = || {
-					let mut channel_id = [0; 32];
+					let mut channel_id = ChannelId::new_zero();
 					let mut user_channel_id: u128 = 0;
 					let mut counterparty_node_id = RequiredWrapper(None);
 					let mut channel_type = RequiredWrapper(None);
@@ -1512,7 +1512,7 @@ impl MaybeReadable for Event {
 			},
 			31u8 => {
 				let f = || {
-					let mut channel_id = [0; 32];
+					let mut channel_id = ChannelId::new_zero();
 					let mut user_channel_id: u128 = 0;
 					let mut former_temporary_channel_id = None;
 					let mut counterparty_node_id = RequiredWrapper(None);