Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address outstanding 2077 feedback #2382

Merged
merged 7 commits into from
Jul 20, 2023
2 changes: 1 addition & 1 deletion lightning/src/chain/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
}

//TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may
// have empty holder commitment transaction if a ChannelMonitor is asked to force-close just after Channel::get_outbound_funding_created,
// have empty holder commitment transaction if a ChannelMonitor is asked to force-close just after OutboundV1Channel::get_funding_created,
// before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
// to monitor before.
pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
Expand Down
9 changes: 7 additions & 2 deletions lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl_writeable_tlv_based_enum_upgradable!(PathFailure,
);

#[derive(Clone, Debug, PartialEq, Eq)]
/// The reason the channel was closed. See individual variants more details.
/// The reason the channel was closed. See individual variants for more details.
pub enum ClosureReason {
/// Closure generated from receiving a peer error message.
///
Expand Down Expand Up @@ -164,7 +164,10 @@ pub enum ClosureReason {
///
/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
OutdatedChannelManager
OutdatedChannelManager,
/// The counterparty requested a cooperative close of a channel that had not been funded yet.
/// The channel has been immediately closed.
CounterpartyCoopClosedUnfundedChannel,
}

impl core::fmt::Display for ClosureReason {
Expand All @@ -184,6 +187,7 @@ impl core::fmt::Display for ClosureReason {
},
ClosureReason::DisconnectedPeer => f.write_str("the peer disconnected prior to the channel being funded"),
ClosureReason::OutdatedChannelManager => f.write_str("the ChannelManager read from disk was stale compared to ChannelMonitor(s)"),
ClosureReason::CounterpartyCoopClosedUnfundedChannel => f.write_str("the peer requested the unfunded channel be closed"),
}
}
}
Expand All @@ -197,6 +201,7 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
(8, ProcessingError) => { (1, err, required) },
(10, DisconnectedPeer) => {},
(12, OutdatedChannelManager) => {},
(13, CounterpartyCoopClosedUnfundedChannel) => {},
);

/// Intended destination of a failed HTLC as indicated in [`Event::HTLCHandlingFailed`].
Expand Down
55 changes: 43 additions & 12 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,11 @@ pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5;
/// See [`ChannelContext::sent_message_awaiting_response`] for more information.
pub(crate) const DISCONNECT_PEER_AWAITING_RESPONSE_TICKS: usize = 2;

/// The number of ticks that may elapse while we're waiting for an unfunded outbound/inbound channel
/// to be promoted to a [`Channel`] since the unfunded channel was created. An unfunded channel
/// exceeding this age limit will be force-closed and purged from memory.
pub(crate) const UNFUNDED_CHANNEL_AGE_LIMIT_TICKS: usize = 60;

struct PendingChannelMonitorUpdate {
update: ChannelMonitorUpdate,
}
Expand All @@ -598,6 +603,28 @@ impl_writeable_tlv_based!(PendingChannelMonitorUpdate, {
(0, update, required),
});

/// Contains all state common to unfunded inbound/outbound channels.
pub(super) struct UnfundedChannelContext {
/// A counter tracking how many ticks have elapsed since this unfunded channel was
/// created. If this unfunded channel reaches peer has yet to respond after reaching
/// `UNFUNDED_CHANNEL_AGE_LIMIT_TICKS`, it will be force-closed and purged from memory.
///
/// This is so that we don't keep channels around that haven't progressed to a funded state
/// in a timely manner.
unfunded_channel_age_ticks: usize,
}

impl UnfundedChannelContext {
/// Determines whether we should force-close and purge this unfunded channel from memory due to it
/// having reached the unfunded channel age limit.
///
/// This should be called on every [`super::channelmanager::ChannelManager::timer_tick_occurred`].
pub fn should_expire_unfunded_channel(&mut self) -> bool {
self.unfunded_channel_age_ticks += 1;
self.unfunded_channel_age_ticks >= UNFUNDED_CHANNEL_AGE_LIMIT_TICKS
}
}

/// Contains everything about the channel including state, and various flags.
pub(super) struct ChannelContext<Signer: ChannelSigner> {
config: LegacyChannelConfig,
Expand Down Expand Up @@ -995,7 +1022,7 @@ impl<Signer: ChannelSigner> ChannelContext<Signer> {
}

/// Returns the funding_txo we either got from our peer, or were given by
/// get_outbound_funding_created.
/// get_funding_created.
pub fn get_funding_txo(&self) -> Option<OutPoint> {
self.channel_transaction_parameters.funding_outpoint
}
Expand Down Expand Up @@ -1440,7 +1467,7 @@ impl<Signer: ChannelSigner> ChannelContext<Signer> {
#[inline]
/// Creates a set of keys for build_commitment_transaction to generate a transaction which we
/// will sign and send to our counterparty.
/// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
/// If an Err is returned, it is a ChannelError::Close (for get_funding_created)
fn build_remote_transaction_keys(&self) -> TxCreationKeys {
//TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
//may see payments to it!
Expand Down Expand Up @@ -2026,7 +2053,7 @@ fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_type_featur

// TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
// has been completed, and then turn into a Channel to get compiler-time enforcement of things like
// calling channel_id() before we're set up or things like get_outbound_funding_signed on an
// calling channel_id() before we're set up or things like get_funding_signed on an
// inbound channel.
//
// Holder designates channel data owned for the benefit of the user client.
Expand Down Expand Up @@ -5477,6 +5504,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
/// A not-yet-funded outbound (from holder) channel using V1 channel establishment.
pub(super) struct OutboundV1Channel<Signer: ChannelSigner> {
pub context: ChannelContext<Signer>,
pub unfunded_context: UnfundedChannelContext,
}

impl<Signer: WriteableEcdsaChannelSigner> OutboundV1Channel<Signer> {
Expand Down Expand Up @@ -5678,12 +5706,13 @@ impl<Signer: WriteableEcdsaChannelSigner> OutboundV1Channel<Signer> {
channel_keys_id,

blocked_monitor_updates: Vec::new(),
}
},
unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 }
})
}

/// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
/// If an Err is returned, it is a ChannelError::Close (for get_funding_created)
fn get_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
let counterparty_keys = self.context.build_remote_transaction_keys();
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
Ok(self.context.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
Expand All @@ -5697,7 +5726,7 @@ impl<Signer: WriteableEcdsaChannelSigner> OutboundV1Channel<Signer> {
/// Note that channel_id changes during this call!
/// Do NOT broadcast the funding transaction until after a successful funding_signed call!
/// If an Err is returned, it is a ChannelError::Close.
pub fn get_outbound_funding_created<L: Deref>(mut self, funding_transaction: Transaction, funding_txo: OutPoint, logger: &L)
pub fn get_funding_created<L: Deref>(mut self, funding_transaction: Transaction, funding_txo: OutPoint, logger: &L)
-> Result<(Channel<Signer>, msgs::FundingCreated), (Self, ChannelError)> where L::Target: Logger {
if !self.context.is_outbound() {
panic!("Tried to create outbound funding_created message on an inbound channel!");
Expand All @@ -5714,7 +5743,7 @@ impl<Signer: WriteableEcdsaChannelSigner> OutboundV1Channel<Signer> {
self.context.channel_transaction_parameters.funding_outpoint = Some(funding_txo);
self.context.holder_signer.provide_channel_parameters(&self.context.channel_transaction_parameters);

let signature = match self.get_outbound_funding_created_signature(logger) {
let signature = match self.get_funding_created_signature(logger) {
Ok(res) => res,
Err(e) => {
log_error!(logger, "Got bad signatures: {:?}!", e);
Expand Down Expand Up @@ -5983,6 +6012,7 @@ impl<Signer: WriteableEcdsaChannelSigner> OutboundV1Channel<Signer> {
/// A not-yet-funded inbound (from counterparty) channel using V1 channel establishment.
pub(super) struct InboundV1Channel<Signer: ChannelSigner> {
pub context: ChannelContext<Signer>,
pub unfunded_context: UnfundedChannelContext,
}

impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
Expand Down Expand Up @@ -6310,7 +6340,8 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
channel_keys_id,

blocked_monitor_updates: Vec::new(),
}
},
unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 }
};

Ok(chan)
Expand Down Expand Up @@ -7598,7 +7629,7 @@ mod tests {
value: 10000000, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();

// Node B --> Node A: funding signed
Expand Down Expand Up @@ -7725,7 +7756,7 @@ mod tests {
value: 10000000, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();

// Node B --> Node A: funding signed
Expand Down Expand Up @@ -7913,7 +7944,7 @@ mod tests {
value: 10000000, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, &&logger).map_err(|_| ()).unwrap();
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();

// Node B --> Node A: funding signed
Expand Down
Loading