Skip to content

Commit 5093eba

Browse files
committed
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.
1 parent a3411d6 commit 5093eba

File tree

6 files changed

+72
-28
lines changed

6 files changed

+72
-28
lines changed

fuzz/src/full_stack.rs

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

lightning/src/ln/channel.rs

+32-7
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ pub(super) struct Channel<Signer: Sign> {
509509

510510
inbound_handshake_limits_override: Option<ChannelHandshakeLimits>,
511511

512-
user_id: u64,
512+
user_id: u128,
513513

514514
channel_id: [u8; 32],
515515
channel_state: u32,
@@ -902,7 +902,7 @@ impl<Signer: Sign> Channel<Signer> {
902902
// Constructors:
903903
pub fn new_outbound<K: Deref, F: Deref>(
904904
fee_estimator: &LowerBoundedFeeEstimator<F>, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
905-
channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig, current_chain_height: u32,
905+
channel_value_satoshis: u64, push_msat: u64, user_id: u128, config: &UserConfig, current_chain_height: u32,
906906
outbound_scid_alias: u64
907907
) -> Result<Channel<Signer>, APIError>
908908
where K::Target: KeysInterface<Signer = Signer>,
@@ -1102,7 +1102,7 @@ impl<Signer: Sign> Channel<Signer> {
11021102
/// Assumes chain_hash has already been checked and corresponds with what we expect!
11031103
pub fn new_from_req<K: Deref, F: Deref, L: Deref>(
11041104
fee_estimator: &LowerBoundedFeeEstimator<F>, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
1105-
msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32, logger: &L,
1105+
msg: &msgs::OpenChannel, user_id: u128, config: &UserConfig, current_chain_height: u32, logger: &L,
11061106
outbound_scid_alias: u64
11071107
) -> Result<Channel<Signer>, ChannelError>
11081108
where K::Target: KeysInterface<Signer = Signer>,
@@ -4482,7 +4482,7 @@ impl<Signer: Sign> Channel<Signer> {
44824482

44834483
/// Gets the "user_id" value passed into the construction of this channel. It has no special
44844484
/// meaning and exists only to allow users to have a persistent identifier of a channel.
4485-
pub fn get_user_id(&self) -> u64 {
4485+
pub fn get_user_id(&self) -> u128 {
44864486
self.user_id
44874487
}
44884488

@@ -5173,7 +5173,7 @@ impl<Signer: Sign> Channel<Signer> {
51735173
/// should be sent back to the counterparty node.
51745174
///
51755175
/// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
5176-
pub fn accept_inbound_channel(&mut self, user_id: u64) -> msgs::AcceptChannel {
5176+
pub fn accept_inbound_channel(&mut self, user_id: u128) -> msgs::AcceptChannel {
51775177
if self.is_outbound() {
51785178
panic!("Tried to send accept_channel for an outbound channel?");
51795179
}
@@ -6002,7 +6002,11 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
60026002

60036003
write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
60046004

6005-
self.user_id.write(writer)?;
6005+
// `user_id` used to be a single u64 value. In order to remain backwards compatible with
6006+
// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We write
6007+
// the low bytes now and the optional high bytes later.
6008+
let user_id_low = self.user_id as u64;
6009+
user_id_low.write(writer)?;
60066010

60076011
// Version 1 deserializers expected to read parts of the config object here. Version 2
60086012
// deserializers (0.0.99) now read config through TLVs, and as we now require them for
@@ -6249,6 +6253,11 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
62496253

62506254
let channel_ready_event_emitted = Some(self.channel_ready_event_emitted);
62516255

6256+
// `user_id` used to be a single u64 value. In order to remain backwards compatible with
6257+
// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. Therefore,
6258+
// we write the high bytes as an option here.
6259+
let user_id_high_opt = Some((self.user_id >> 64) as u64);
6260+
62526261
write_tlv_fields!(writer, {
62536262
(0, self.announcement_sigs, option),
62546263
// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -6272,6 +6281,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
62726281
(19, self.latest_inbound_scid_alias, option),
62736282
(21, self.outbound_scid_alias, required),
62746283
(23, channel_ready_event_emitted, option),
6284+
(25, user_id_high_opt, option),
62756285
});
62766286

62776287
Ok(())
@@ -6285,7 +6295,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
62856295
let (keys_source, serialized_height) = args;
62866296
let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
62876297

6288-
let user_id = Readable::read(reader)?;
6298+
// `user_id` used to be a single u64 value. In order to remain backwards compatible with
6299+
// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We read
6300+
// the low bytes now and the high bytes later.
6301+
let user_id_low: u64 = Readable::read(reader)?;
62896302

62906303
let mut config = Some(LegacyChannelConfig::default());
62916304
if ver == 1 {
@@ -6531,6 +6544,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
65316544
let mut outbound_scid_alias = None;
65326545
let mut channel_ready_event_emitted = None;
65336546

6547+
let mut user_id_high_opt: Option<u64> = None;
6548+
65346549
read_tlv_fields!(reader, {
65356550
(0, announcement_sigs, option),
65366551
(1, minimum_depth, option),
@@ -6548,6 +6563,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
65486563
(19, latest_inbound_scid_alias, option),
65496564
(21, outbound_scid_alias, option),
65506565
(23, channel_ready_event_emitted, option),
6566+
(25, user_id_high_opt, option),
65516567
});
65526568

65536569
if let Some(preimages) = preimages_opt {
@@ -6584,6 +6600,15 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
65846600
let mut secp_ctx = Secp256k1::new();
65856601
secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
65866602

6603+
// `user_id` used to be a single u64 value. In order to remain backwards
6604+
// compatible with versions prior to 0.0.113, the u128 is serialized as two
6605+
// separate u64 values.
6606+
let user_id = if let Some(user_id_high) = user_id_high_opt {
6607+
user_id_low as u128 + ((user_id_high as u128) << 64)
6608+
} else {
6609+
user_id_low as u128
6610+
};
6611+
65876612
Ok(Channel {
65886613
user_id,
65896614

lightning/src/ln/channelmanager.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource
289289
290290
struct MsgHandleErrInternal {
291291
err: msgs::LightningError,
292-
chan_id: Option<([u8; 32], u64)>, // If Some a channel of ours has been closed
292+
chan_id: Option<([u8; 32], u128)>, // If Some a channel of ours has been closed
293293
shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
294294
}
295295
impl MsgHandleErrInternal {
@@ -325,7 +325,7 @@ impl MsgHandleErrInternal {
325325
Self { err, chan_id: None, shutdown_finish: None }
326326
}
327327
#[inline]
328-
fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u64, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
328+
fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u128, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
329329
Self {
330330
err: LightningError {
331331
err: err.clone(),
@@ -1084,7 +1084,7 @@ pub struct ChannelDetails {
10841084
/// [`outbound_capacity_msat`]: ChannelDetails::outbound_capacity_msat
10851085
pub unspendable_punishment_reserve: Option<u64>,
10861086
/// The `user_channel_id` passed in to create_channel, or 0 if the channel was inbound.
1087-
pub user_channel_id: u64,
1087+
pub user_channel_id: u128,
10881088
/// Our total balance. This is the amount we would get if we close the channel.
10891089
/// This value is not exact. Due to various in-flight changes and feerate changes, exactly this
10901090
/// 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
17401740
/// [`Event::FundingGenerationReady::user_channel_id`]: events::Event::FundingGenerationReady::user_channel_id
17411741
/// [`Event::FundingGenerationReady::temporary_channel_id`]: events::Event::FundingGenerationReady::temporary_channel_id
17421742
/// [`Event::ChannelClosed::channel_id`]: events::Event::ChannelClosed::channel_id
1743-
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> {
1743+
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> {
17441744
if channel_value_satoshis < 1000 {
17451745
return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
17461746
}
@@ -4529,7 +4529,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
45294529
///
45304530
/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
45314531
/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
4532-
pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u64) -> Result<(), APIError> {
4532+
pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
45334533
self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id)
45344534
}
45354535

@@ -4551,11 +4551,11 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
45514551
///
45524552
/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
45534553
/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
4554-
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> {
4554+
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> {
45554555
self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id)
45564556
}
45574557

4558-
fn do_accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u64) -> Result<(), APIError> {
4558+
fn do_accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u128) -> Result<(), APIError> {
45594559
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
45604560

45614561
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
46034603
return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone()));
46044604
}
46054605

4606-
let mut random_bytes = [0u8; 8];
4607-
random_bytes.copy_from_slice(&self.keys_manager.get_secure_random_bytes()[..8]);
4608-
let user_channel_id = u64::from_be_bytes(random_bytes);
4606+
let mut random_bytes = [0u8; 16];
4607+
random_bytes.copy_from_slice(&self.keys_manager.get_secure_random_bytes()[..16]);
4608+
let user_channel_id = u128::from_be_bytes(random_bytes);
46094609

46104610
let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
46114611
let mut channel = match Channel::new_from_req(&self.fee_estimator, &self.keys_manager,

lightning/src/ln/functional_test_utils.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ macro_rules! check_added_monitors {
618618
}
619619
}
620620

621-
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) {
621+
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) {
622622
let chan_id = *node.network_chan_count.borrow();
623623

624624
let events = node.node.get_and_clear_pending_events();

lightning/src/util/events.rs

+27-9
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ pub enum Event {
322322
/// an inbound channel.
323323
///
324324
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
325-
user_channel_id: u64,
325+
user_channel_id: u128,
326326
},
327327
/// Indicates we've received (an offer of) money! Just gotta dig out that payment preimage and
328328
/// feed it to [`ChannelManager::claim_funds`] to get it....
@@ -617,7 +617,7 @@ pub enum Event {
617617
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
618618
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
619619
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
620-
user_channel_id: u64,
620+
user_channel_id: u128,
621621
/// The node_id of the channel counterparty.
622622
counterparty_node_id: PublicKey,
623623
/// The features that this channel will operate with.
@@ -638,7 +638,7 @@ pub enum Event {
638638
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
639639
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
640640
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
641-
user_channel_id: u64,
641+
user_channel_id: u128,
642642
/// The reason the channel was closed.
643643
reason: ClosureReason
644644
},
@@ -813,10 +813,16 @@ impl Writeable for Event {
813813
},
814814
&Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason } => {
815815
9u8.write(writer)?;
816+
// `user_channel_id` used to be a single u64 value. In order to remain backwards
817+
// compatible with versions prior to 0.0.113, the u128 is serialized as two
818+
// separate u64 values.
819+
let user_channel_id_low = *user_channel_id as u64;
820+
let user_channel_id_high = (*user_channel_id >> 64) as u64;
816821
write_tlv_fields!(writer, {
817822
(0, channel_id, required),
818-
(1, user_channel_id, required),
819-
(2, reason, required)
823+
(1, user_channel_id_low, required),
824+
(2, reason, required),
825+
(3, user_channel_id_high, required),
820826
});
821827
},
822828
&Event::DiscardFunding { ref channel_id, ref transaction } => {
@@ -1035,14 +1041,26 @@ impl MaybeReadable for Event {
10351041
let f = || {
10361042
let mut channel_id = [0; 32];
10371043
let mut reason = None;
1038-
let mut user_channel_id_opt = None;
1044+
let mut user_channel_id_low_opt: Option<u64> = None;
1045+
let mut user_channel_id_high_opt: Option<u64> = None;
10391046
read_tlv_fields!(reader, {
10401047
(0, channel_id, required),
1041-
(1, user_channel_id_opt, option),
1048+
(1, user_channel_id_low_opt, option),
10421049
(2, reason, ignorable),
1050+
(3, user_channel_id_high_opt, option),
10431051
});
10441052
if reason.is_none() { return Ok(None); }
1045-
let user_channel_id = if let Some(id) = user_channel_id_opt { id } else { 0 };
1053+
1054+
// `user_channel_id` used to be a single u64 value. In order to remain
1055+
// backwards compatible with versions prior to 0.0.113, the u128 is serialized
1056+
// as two separate u64 values.
1057+
let user_channel_id = if let Some(user_channel_id_low) = user_channel_id_low_opt {
1058+
if let Some(user_channel_id_high) = user_channel_id_high_opt {
1059+
user_channel_id_low as u128 + ((user_channel_id_high as u128) << 64)
1060+
} else {
1061+
user_channel_id_low as u128
1062+
}
1063+
} else { 0u128 };
10461064
Ok(Some(Event::ChannelClosed { channel_id, user_channel_id, reason: reason.unwrap() }))
10471065
};
10481066
f()
@@ -1180,7 +1198,7 @@ impl MaybeReadable for Event {
11801198
29u8 => {
11811199
let f = || {
11821200
let mut channel_id = [0; 32];
1183-
let mut user_channel_id: u64 = 0;
1201+
let mut user_channel_id: u128 = 0;
11841202
let mut counterparty_node_id = OptionDeserWrapper(None);
11851203
let mut channel_type = OptionDeserWrapper(None);
11861204
read_tlv_fields!(reader, {

lightning/src/util/ser.rs

+1
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ macro_rules! impl_writeable_primitive {
454454
}
455455
}
456456

457+
impl_writeable_primitive!(u128, 16);
457458
impl_writeable_primitive!(u64, 8);
458459
impl_writeable_primitive!(u32, 4);
459460
impl_writeable_primitive!(u16, 2);

0 commit comments

Comments
 (0)