Skip to content

Commit

Permalink
Merge pull request #1121 from TheBlueMatt/2021-10-return-temp-id
Browse files Browse the repository at this point in the history 
Expose temporary channel ID and user channel ID pre-funding
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Oct 16, 2021
2 parents 2398f17 + bb7ef6c commit 001bc71
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 35 deletions.
2 changes: 1 addition & 1 deletion fuzz/src/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
short_channel_id: Some(scid),
channel_value_satoshis: slice_to_be64(get_slice!(8)),
user_id: 0, inbound_capacity_msat: 0,
user_channel_id: 0, inbound_capacity_msat: 0,
unspendable_punishment_reserve: None,
confirmations_required: None,
force_close_spend_delay: None,
Expand Down
78 changes: 49 additions & 29 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource

struct MsgHandleErrInternal {
err: msgs::LightningError,
chan_id: Option<[u8; 32]>, // If Some a channel of ours has been closed
chan_id: Option<([u8; 32], u64)>, // If Some a channel of ours has been closed
shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
}
impl MsgHandleErrInternal {
Expand Down Expand Up @@ -278,7 +278,7 @@ impl MsgHandleErrInternal {
Self { err, chan_id: None, shutdown_finish: None }
}
#[inline]
fn from_finish_shutdown(err: String, channel_id: [u8; 32], shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u64, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
Self {
err: LightningError {
err: err.clone(),
Expand All @@ -289,7 +289,7 @@ impl MsgHandleErrInternal {
},
},
},
chan_id: Some(channel_id),
chan_id: Some((channel_id, user_channel_id)),
shutdown_finish: Some((shutdown_res, channel_update)),
}
}
Expand Down Expand Up @@ -776,8 +776,8 @@ pub struct ChannelDetails {
///
/// [`outbound_capacity_msat`]: ChannelDetails::outbound_capacity_msat
pub unspendable_punishment_reserve: Option<u64>,
/// The user_id passed in to create_channel, or 0 if the channel was inbound.
pub user_id: u64,
/// The `user_channel_id` passed in to create_channel, or 0 if the channel was inbound.
pub user_channel_id: u64,
/// The available outbound capacity for sending HTLCs to the remote peer. This does not include
/// any pending HTLCs which are not yet fully resolved (and, thus, who's balance is not
/// available for inclusion in new outbound HTLCs). This further does not include any pending
Expand Down Expand Up @@ -894,8 +894,11 @@ macro_rules! handle_error {
msg: update
});
}
if let Some(channel_id) = chan_id {
$self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id, reason: ClosureReason::ProcessingError { err: err.err.clone() } });
if let Some((channel_id, user_channel_id)) = chan_id {
$self.pending_events.lock().unwrap().push(events::Event::ChannelClosed {
channel_id, user_channel_id,
reason: ClosureReason::ProcessingError { err: err.err.clone() }
});
}
}

Expand Down Expand Up @@ -937,15 +940,17 @@ macro_rules! convert_chan_err {
$short_to_id.remove(&short_id);
}
let shutdown_res = $channel.force_shutdown(true);
(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
},
ChannelError::CloseDelayBroadcast(msg) => {
log_error!($self.logger, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($channel_id[..]), msg);
if let Some(short_id) = $channel.get_short_channel_id() {
$short_to_id.remove(&short_id);
}
let shutdown_res = $channel.force_shutdown(false);
(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
}
}
}
Expand Down Expand Up @@ -1013,7 +1018,7 @@ macro_rules! handle_monitor_err {
// splitting hairs we'd prefer to claim payments that were to us, but we haven't
// given up the preimage yet, so might as well just wait until the payment is
// retried, avoiding the on-chain fees.
let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id,
let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id, $chan.get_user_id(),
$chan.force_shutdown(true), $self.get_channel_update_for_broadcast(&$chan).ok() ));
(res, true)
},
Expand Down Expand Up @@ -1267,22 +1272,31 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana

/// Creates a new outbound channel to the given remote node and with the given value.
///
/// user_id will be provided back as user_channel_id in FundingGenerationReady events to allow
/// tracking of which events correspond with which create_channel call. Note that the
/// user_channel_id defaults to 0 for inbound channels, so you may wish to avoid using 0 for
/// user_id here. user_id has no meaning inside of LDK, it is simply copied to events and
/// otherwise ignored.
///
/// If successful, will generate a SendOpenChannel message event, so you should probably poll
/// PeerManager::process_events afterwards.
/// `user_channel_id` will be provided back as in
/// [`Event::FundingGenerationReady::user_channel_id`] to allow tracking of which events
/// correspond with which `create_channel` call. Note that the `user_channel_id` defaults to 0
/// for inbound channels, so you may wish to avoid using 0 for `user_channel_id` here.
/// `user_channel_id` has no meaning inside of LDK, it is simply copied to events and otherwise
/// ignored.
///
/// Raises APIError::APIMisuseError when channel_value_satoshis > 2**24 or push_msat is
/// greater than channel_value_satoshis * 1k or channel_value_satoshis is < 1000.
/// Raises [`APIError::APIMisuseError`] when `channel_value_satoshis` > 2**24 or `push_msat` is
/// greater than `channel_value_satoshis * 1k` or `channel_value_satoshis < 1000`.
///
/// Note that we do not check if you are currently connected to the given peer. If no
/// connection is available, the outbound `open_channel` message may fail to send, resulting in
/// the channel eventually being silently forgotten.
pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, override_config: Option<UserConfig>) -> Result<(), APIError> {
/// the channel eventually being silently forgotten (dropped on reload).
///
/// Returns the new Channel's temporary `channel_id`. This ID will appear as
/// [`Event::FundingGenerationReady::temporary_channel_id`] and in
/// [`ChannelDetails::channel_id`] until after
/// [`ChannelManager::funding_transaction_generated`] is called, swapping the Channel's ID for
/// one derived from the funding transaction's TXID. If the counterparty rejects the channel
/// immediately, this temporary ID will appear in [`Event::ChannelClosed::channel_id`].
///
/// [`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> {
if channel_value_satoshis < 1000 {
return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
}
Expand All @@ -1294,7 +1308,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
let peer_state = peer_state.lock().unwrap();
let their_features = &peer_state.latest_features;
let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_id, config)?
Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_channel_id, config)?
},
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
}
Expand All @@ -1305,8 +1319,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
// We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
debug_assert!(&self.total_consistency_lock.try_write().is_err());

let temporary_channel_id = channel.channel_id();
let mut channel_state = self.channel_state.lock().unwrap();
match channel_state.by_id.entry(channel.channel_id()) {
match channel_state.by_id.entry(temporary_channel_id) {
hash_map::Entry::Occupied(_) => {
if cfg!(feature = "fuzztarget") {
return Err(APIError::APIMisuseError { err: "Fuzzy bad RNG".to_owned() });
Expand All @@ -1320,7 +1335,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
node_id: their_network_key,
msg: res,
});
Ok(())
Ok(temporary_channel_id)
}

fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<Signer>)) -> bool>(&self, f: Fn) -> Vec<ChannelDetails> {
Expand All @@ -1346,7 +1361,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
unspendable_punishment_reserve: to_self_reserve_satoshis,
inbound_capacity_msat,
outbound_capacity_msat,
user_id: channel.get_user_id(),
user_channel_id: channel.get_user_id(),
confirmations_required: channel.minimum_depth(),
force_close_spend_delay: channel.get_counterparty_selected_contest_delay(),
is_outbound: channel.is_outbound(),
Expand Down Expand Up @@ -1393,7 +1408,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
},
None => {},
}
pending_events_lock.push(events::Event::ChannelClosed { channel_id: channel.channel_id(), reason: closure_reason });
pending_events_lock.push(events::Event::ChannelClosed {
channel_id: channel.channel_id(),
user_channel_id: channel.get_user_id(),
reason: closure_reason
});
}

fn close_channel_internal(&self, channel_id: &[u8; 32], target_feerate_sats_per_1000_weight: Option<u32>) -> Result<(), APIError> {
Expand Down Expand Up @@ -2228,7 +2247,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana

(chan.get_outbound_funding_created(funding_transaction, funding_txo, &self.logger)
.map_err(|e| if let ChannelError::Close(msg) = e {
MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.force_shutdown(true), None)
MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.get_user_id(), chan.force_shutdown(true), None)
} else { unreachable!(); })
, chan)
},
Expand Down Expand Up @@ -2570,7 +2589,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
channel_state.short_to_id.remove(&short_id);
}
// ChannelClosed event is generated by handle_error for us.
Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
},
ChannelError::CloseDelayBroadcast(_) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); }
};
Expand Down Expand Up @@ -5609,6 +5628,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
channel_closures.push(events::Event::ChannelClosed {
channel_id: channel.channel_id(),
user_channel_id: channel.get_user_id(),
reason: ClosureReason::OutdatedChannelManager
});
} else {
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/ln/functional_test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,11 +527,12 @@ pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_
}

pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> Transaction {
node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), a_flags, &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id()));
node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), b_flags, &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id()));

let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, channel_value, 42);
assert_eq!(temporary_channel_id, create_chan_id);

node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
check_added_monitors!(node_a, 0);
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/routing/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,7 @@ mod tests {
funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
short_channel_id,
channel_value_satoshis: 0,
user_id: 0,
user_channel_id: 0,
outbound_capacity_msat,
inbound_capacity_msat: 42,
unspendable_punishment_reserve: None,
Expand Down
19 changes: 16 additions & 3 deletions lightning/src/util/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ pub enum Event {
channel_value_satoshis: u64,
/// The script which should be used in the transaction output.
output_script: Script,
/// The value passed in to ChannelManager::create_channel
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or 0 for
/// an inbound channel.
///
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
user_channel_id: u64,
},
/// Indicates we've received money! Just gotta dig out that payment preimage and feed it to
Expand Down Expand Up @@ -262,6 +265,12 @@ pub enum Event {
/// 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],
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or 0 for
/// an inbound channel. This will always be zero for objects serialized with LDK versions
/// prior to 0.0.102.
///
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
user_channel_id: u64,
/// The reason the channel was closed.
reason: ClosureReason
},
Expand Down Expand Up @@ -352,10 +361,11 @@ impl Writeable for Event {
(2, claim_from_onchain_tx, required),
});
},
&Event::ChannelClosed { ref channel_id, ref reason } => {
&Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason } => {
9u8.write(writer)?;
write_tlv_fields!(writer, {
(0, channel_id, required),
(1, user_channel_id, required),
(2, reason, required)
});
},
Expand Down Expand Up @@ -492,12 +502,15 @@ impl MaybeReadable for Event {
let f = || {
let mut channel_id = [0; 32];
let mut reason = None;
let mut user_channel_id_opt = None;
read_tlv_fields!(reader, {
(0, channel_id, required),
(1, user_channel_id_opt, option),
(2, reason, ignorable),
});
if reason.is_none() { return Ok(None); }
Ok(Some(Event::ChannelClosed { channel_id, reason: reason.unwrap() }))
let user_channel_id = if let Some(id) = user_channel_id_opt { id } else { 0 };
Ok(Some(Event::ChannelClosed { channel_id, user_channel_id, reason: reason.unwrap() }))
};
f()
},
Expand Down

0 comments on commit 001bc71

Please sign in to comment.