Make user_channel_id a u128

We increase the `user_channel_id` type from `u64` to `u128`. In order to
maintain backwards compatibility, we have to de-/serialize it as two
separate `u64`s in `Event` as well as in the `Channel` itself.

Author: tnull

fuzz/src/full_stack.rs

@@ -403,7 +403,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 	// Adding new calls to `KeysInterface::get_secure_random_bytes` during startup can change all the
 	// keys subsequently generated in this test. Rather than regenerating all the messages manually,
 	// it's easier to just increment the counter here so the keys don't change.
-	keys_manager.counter.fetch_sub(2, Ordering::AcqRel);
+	keys_manager.counter.fetch_sub(3, Ordering::AcqRel);
 	let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap());
 	let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash(), Arc::clone(&logger)));
 	let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger)));

lightning/src/ln/channel.rs

@@ -509,7 +509,7 @@ pub(super) struct Channel<Signer: Sign> {
 
 	inbound_handshake_limits_override: Option<ChannelHandshakeLimits>,
 
-	user_id: u64,
+	user_id: u128,
 
 	channel_id: [u8; 32],
 	channel_state: u32,
@@ -902,7 +902,7 @@ impl<Signer: Sign> Channel<Signer> {
 	// Constructors:
 	pub fn new_outbound<K: Deref, F: Deref>(
 		fee_estimator: &LowerBoundedFeeEstimator<F>, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
-		channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig, current_chain_height: u32,
+		channel_value_satoshis: u64, push_msat: u64, user_id: u128, config: &UserConfig, current_chain_height: u32,
 		outbound_scid_alias: u64
 	) -> Result<Channel<Signer>, APIError>
 	where K::Target: KeysInterface<Signer = Signer>,
@@ -1102,7 +1102,7 @@ impl<Signer: Sign> Channel<Signer> {
 	/// Assumes chain_hash has already been checked and corresponds with what we expect!
 	pub fn new_from_req<K: Deref, F: Deref, L: Deref>(
 		fee_estimator: &LowerBoundedFeeEstimator<F>, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
-		msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32, logger: &L,
+		msg: &msgs::OpenChannel, user_id: u128, config: &UserConfig, current_chain_height: u32, logger: &L,
 		outbound_scid_alias: u64
 	) -> Result<Channel<Signer>, ChannelError>
 		where K::Target: KeysInterface<Signer = Signer>,
@@ -4482,7 +4482,7 @@ impl<Signer: Sign> Channel<Signer> {
 
 	/// Gets the "user_id" value passed into the construction of this channel. It has no special
 	/// meaning and exists only to allow users to have a persistent identifier of a channel.
-	pub fn get_user_id(&self) -> u64 {
+	pub fn get_user_id(&self) -> u128 {
 		self.user_id
 	}
 
@@ -5173,7 +5173,7 @@ impl<Signer: Sign> Channel<Signer> {
 	/// should be sent back to the counterparty node.
 	///
 	/// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
-	pub fn accept_inbound_channel(&mut self, user_id: u64) -> msgs::AcceptChannel {
+	pub fn accept_inbound_channel(&mut self, user_id: u128) -> msgs::AcceptChannel {
 		if self.is_outbound() {
 			panic!("Tried to send accept_channel for an outbound channel?");
 		}
@@ -6002,7 +6002,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
-		self.user_id.write(writer)?;
+		// `user_id` used to be a single u64 value. In order to remain backwards compatible with
+		// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We write
+		// the low bytes now and the optional high bytes later.
+		let mut low_bytes = [0u8; 8];
+		low_bytes.copy_from_slice(&self.user_id.to_be_bytes()[8..16]);
+		let user_id_low = u64::from_be_bytes(low_bytes);
+		user_id_low.write(writer)?;
 
 		// Version 1 deserializers expected to read parts of the config object here. Version 2
 		// deserializers (0.0.99) now read config through TLVs, and as we now require them for
@@ -6249,6 +6255,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		let channel_ready_event_emitted = Some(self.channel_ready_event_emitted);
 
+		// `user_id` used to be a single u64 value. In order to remain backwards compatible with
+		// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. Therefore,
+		// we write the high bytes as an option here.
+		let mut high_bytes = [0u8; 8];
+		high_bytes.copy_from_slice(&self.user_id.to_be_bytes()[0..8]);
+		let user_id_high_opt = Some(u64::from_be_bytes(high_bytes));
+
 		write_tlv_fields!(writer, {
 			(0, self.announcement_sigs, option),
 			// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -6272,6 +6285,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(19, self.latest_inbound_scid_alias, option),
 			(21, self.outbound_scid_alias, required),
 			(23, channel_ready_event_emitted, option),
+			(25, user_id_high_opt, option),
 		});
 
 		Ok(())
@@ -6285,7 +6299,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		let (keys_source, serialized_height) = args;
 		let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
-		let user_id = Readable::read(reader)?;
+		// `user_id` used to be a single u64 value. In order to remain backwards compatible with
+		// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We read
+		// the low bytes now and the high bytes later.
+		let user_id_low: u64 = Readable::read(reader)?;
 
 		let mut config = Some(LegacyChannelConfig::default());
 		if ver == 1 {
@@ -6531,6 +6548,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		let mut outbound_scid_alias = None;
 		let mut channel_ready_event_emitted = None;
 
+		let mut user_id_high_opt: Option<u64> = None;
+
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
@@ -6548,6 +6567,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			(19, latest_inbound_scid_alias, option),
 			(21, outbound_scid_alias, option),
 			(23, channel_ready_event_emitted, option),
+			(25, user_id_high_opt, option),
 		});
 
 		if let Some(preimages) = preimages_opt {
@@ -6584,6 +6604,16 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
 
+		// `user_id` used to be a single u64 value. In order to remain backwards
+		// compatible with versions prior to 0.0.113, the u128 is serialized as two
+		// separate u64 values.
+		let mut user_id_bytes = [0u8; 16];
+		user_id_bytes[8..16].copy_from_slice(&user_id_low.to_be_bytes());
+		if let Some(high_bytes) = user_id_high_opt {
+			user_id_bytes[0..8].copy_from_slice(&high_bytes.to_be_bytes());
+		}
+		let user_id = u128::from_be_bytes(user_id_bytes);
+
 		Ok(Channel {
 			user_id,
 

lightning/src/ln/channelmanager.rs

@@ -289,7 +289,7 @@ type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource
 
 struct MsgHandleErrInternal {
 	err: msgs::LightningError,
-	chan_id: Option<([u8; 32], u64)>, // If Some a channel of ours has been closed
+	chan_id: Option<([u8; 32], u128)>, // If Some a channel of ours has been closed
 	shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
 }
 impl MsgHandleErrInternal {
@@ -325,7 +325,7 @@ impl MsgHandleErrInternal {
 		Self { err, chan_id: None, shutdown_finish: None }
 	}
 	#[inline]
-	fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u64, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
+	fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u128, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
 		Self {
 			err: LightningError {
 				err: err.clone(),
@@ -1084,7 +1084,7 @@ pub struct ChannelDetails {
 	/// [`outbound_capacity_msat`]: ChannelDetails::outbound_capacity_msat
 	pub unspendable_punishment_reserve: Option<u64>,
 	/// The `user_channel_id` passed in to create_channel, or 0 if the channel was inbound.
-	pub user_channel_id: u64,
+	pub user_channel_id: u128,
 	/// Our total balance.  This is the amount we would get if we close the channel.
 	/// This value is not exact. Due to various in-flight changes and feerate changes, exactly this
 	/// amount is not likely to be recoverable on close.
@@ -1740,7 +1740,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	/// [`Event::FundingGenerationReady::user_channel_id`]: events::Event::FundingGenerationReady::user_channel_id
 	/// [`Event::FundingGenerationReady::temporary_channel_id`]: events::Event::FundingGenerationReady::temporary_channel_id
 	/// [`Event::ChannelClosed::channel_id`]: events::Event::ChannelClosed::channel_id
-	pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_channel_id: u64, override_config: Option<UserConfig>) -> Result<[u8; 32], APIError> {
+	pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_channel_id: u128, override_config: Option<UserConfig>) -> Result<[u8; 32], APIError> {
 		if channel_value_satoshis < 1000 {
 			return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
 		}
@@ -4529,7 +4529,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	///
 	/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
 	/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
-	pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u64) -> Result<(), APIError> {
+	pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
 		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id)
 	}
 
@@ -4551,11 +4551,11 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	///
 	/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
 	/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
-	pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u64) -> Result<(), APIError> {
+	pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
 		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id)
 	}
 
-	fn do_accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u64) -> Result<(), APIError> {
+	fn do_accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u128) -> 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();
@@ -4603,9 +4603,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 			return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone()));
 		}
 
-		let mut random_bytes = [0u8; 8];
-		random_bytes.copy_from_slice(&self.keys_manager.get_secure_random_bytes()[..8]);
-		let user_channel_id = u64::from_be_bytes(random_bytes);
+		let mut random_bytes = [0u8; 16];
+		random_bytes.copy_from_slice(&self.keys_manager.get_secure_random_bytes()[..16]);
+		let user_channel_id = u128::from_be_bytes(random_bytes);
 
 		let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
 		let mut channel = match Channel::new_from_req(&self.fee_estimator, &self.keys_manager,

lightning/src/ln/functional_test_utils.rs

@@ -618,7 +618,7 @@ macro_rules! check_added_monitors {
 	}
 }
 
-pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u64) -> ([u8; 32], Transaction, OutPoint) {
+pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128) -> ([u8; 32], Transaction, OutPoint) {
 	let chan_id = *node.network_chan_count.borrow();
 
 	let events = node.node.get_and_clear_pending_events();

lightning/src/util/events.rs

@@ -322,7 +322,7 @@ pub enum Event {
 		/// an inbound channel.
 		///
 		/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
-		user_channel_id: u64,
+		user_channel_id: u128,
 	},
 	/// Indicates we've received (an offer of) money! Just gotta dig out that payment preimage and
 	/// feed it to [`ChannelManager::claim_funds`] to get it....
@@ -638,7 +638,7 @@ pub enum Event {
 		/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
 		/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
 		/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
-		user_channel_id: u64,
+		user_channel_id: u128,
 		/// The reason the channel was closed.
 		reason: ClosureReason
 	},
@@ -813,10 +813,20 @@ impl Writeable for Event {
 			},
 			&Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason } => {
 				9u8.write(writer)?;
+				// `user_channel_id` used to be a single u64 value. In order to remain backwards
+				// compatible with versions prior to 0.0.113, the u128 is serialized as two
+				// separate u64 values.
+				let mut low_bytes = [0u8; 8];
+				low_bytes.copy_from_slice(&user_channel_id.to_be_bytes()[8..16]);
+				let user_channel_id_low = u64::from_be_bytes(low_bytes);
+				let mut high_bytes = [0u8; 8];
+				high_bytes.copy_from_slice(&user_channel_id.to_be_bytes()[0..8]);
+				let user_channel_id_high = u64::from_be_bytes(high_bytes);
 				write_tlv_fields!(writer, {
 					(0, channel_id, required),
-					(1, user_channel_id, required),
-					(2, reason, required)
+					(1, user_channel_id_low, required),
+					(2, reason, required),
+					(3, user_channel_id_high, required),
 				});
 			},
 			&Event::DiscardFunding { ref channel_id, ref transaction } => {
@@ -1035,14 +1045,28 @@ impl MaybeReadable for Event {
 				let f = || {
 					let mut channel_id = [0; 32];
 					let mut reason = None;
-					let mut user_channel_id_opt = None;
+					let mut user_channel_id_low_opt: Option<u64> = None;
+					let mut user_channel_id_high_opt: Option<u64> = None;
 					read_tlv_fields!(reader, {
 						(0, channel_id, required),
-						(1, user_channel_id_opt, option),
+						(1, user_channel_id_low_opt, option),
 						(2, reason, ignorable),
+						(3, user_channel_id_high_opt, option),
 					});
 					if reason.is_none() { return Ok(None); }
-					let user_channel_id = if let Some(id) = user_channel_id_opt { id } else { 0 };
+
+					// `user_channel_id` used to be a single u64 value. In order to remain
+					// backwards compatible with versions prior to 0.0.113, the u128 is serialized
+					// as two separate u64 values.
+					let user_channel_id = if let Some(user_channel_id_low) = user_channel_id_low_opt {
+						let mut user_channel_id_bytes = [0u8; 16];
+						if let Some(user_channel_id_high) = user_channel_id_high_opt {
+							user_channel_id_bytes[0..8].copy_from_slice(&user_channel_id_high.to_be_bytes());
+						}
+
+						user_channel_id_bytes[8..16].copy_from_slice(&user_channel_id_low.to_be_bytes());
+						u128::from_be_bytes(user_channel_id_bytes)
+					} else { 0u128 };
 					Ok(Some(Event::ChannelClosed { channel_id, user_channel_id, reason: reason.unwrap() }))
 				};
 				f()

lightning/src/util/ser.rs

@@ -454,6 +454,7 @@ macro_rules! impl_writeable_primitive {
 	}
 }
 
+impl_writeable_primitive!(u128, 16);
 impl_writeable_primitive!(u64, 8);
 impl_writeable_primitive!(u32, 4);
 impl_writeable_primitive!(u16, 2);