Fix infinite loop when closing a pre-funding channel #2760
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tnull
previously approved these changes
Nov 29, 2023
wpaulino
reviewed
Nov 29, 2023
shaavan
approved these changes
Nov 29, 2023
TheBlueMatt
force-pushed
the
2023-11-chan-close-loop
branch
from
cf63465 to
25da171
Compare
|
Lol I was paying no attention and built this on 0.0.118, rather than git HEAD, rebased now. |
TheBlueMatt
force-pushed
the
2023-11-chan-close-loop
branch
from
25da171 to
5db34c5
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2760 +/- ##
==========================================
- Coverage 88.52% 88.52% -0.01%
==========================================
Files 115 115
Lines 90996 90999 +3
Branches 90996 90999 +3
==========================================
+ Hits 80557 80558 +1
- Misses 8012 8014 +2
Partials 2427 2427 ☔ View full report in Codecov by Sentry. |
|
Okay, pushed two commits to start to redo the line between funded and unfunded channels yet again, but I think everyone should be happy with this (it was the original idea, and now no longer has the temporary-vs-post-funding-channel-id map key problem). |
dunxen
previously approved these changes
Nov 30, 2023
|
|
||
| /// Handles a funding_signed message from the remote end. | ||
| /// If this call is successful, broadcast the funding transaction (and not before!) | ||
| pub fn funding_signed<L: Deref>( |
There was a problem hiding this comment.
Ok this makes sense for unfunded V1 channels since there's no need to persist the channel before a funding_signed. Also it's great that this is only in outbound now.
It has given me a little more to think about in the V2 case.
There was a problem hiding this comment.
Yea, sorry for the back-and-forth on it, I know we'd discussed this originally but I think I pushed back cause I was annoyed about the different channel id keys.
shaavan
reviewed
Dec 3, 2023
There was a problem hiding this comment.
ACK modulo nit
(Adding to my previous review)
Thanks for this great update to the codebase! 🚀
- Handling funding signed from the remote end, is a behavior specific to
OutboundV1Channel, and until the funding sign is handled, theOutboundV1Channelis not yet technically aChannel. - So it makes sense to make this handler part of the
OutboundV1Channelstruct. - Updates to the code are clean and clear, and a few added comments are helpful.
- All the tests of the codebase pass, and hence, there is no broken behavioral change.
TheBlueMatt
force-pushed
the
2023-11-chan-close-loop
branch
from
5db34c5 to
3f6e29b
Compare
|
Rebased and addressed feedback: $ git range-diff 4fc3a171152140860440311743178d922c6b7c4b..5db34c58542b06ae8f92d186aab45b43b6d28956 37150b4d697f723f278b6cf3f63892df272b9aa5..3f6e29bd0519dba925992b24455118f1a4e3400d
1: 1de0f5f45 ! 1: c8124e4fb Move to `Funded` after `funding_signed` rather than on funding
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
- funding_redeemscript.clone(), self.context.channel_value_satoshis,
- obscure_factor,
- holder_commitment_tx, best_block, self.context.counterparty_node_id);
--
+- let logger_with_chan_monitor = WithChannelMonitor::from(logger, &channel_monitor);
- channel_monitor.provide_initial_counterparty_commitment_tx(
- counterparty_initial_bitcoin_tx.txid, Vec::new(),
- self.context.cur_counterparty_commitment_transaction_number,
- self.context.counterparty_cur_commitment_point.unwrap(),
- counterparty_initial_commitment_tx.feerate_per_kw(),
- counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
-- counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
+- counterparty_initial_commitment_tx.to_countersignatory_value_sat(), &&logger_with_chan_monitor);
-
- assert_eq!(self.context.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update!
- if self.context.is_batch_funding() {
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Tar
+ Ok(funding_created)
}
-+
fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) -> ChannelTypeFeatures {
- // The default channel type (ie the first one we try) depends on whether the channel is
- // public - if it is, we just go with `only_static_remotekey` as it's the only option
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
Ok(())
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Tar
+ funding_redeemscript.clone(), self.context.channel_value_satoshis,
+ obscure_factor,
+ holder_commitment_tx, best_block, self.context.counterparty_node_id);
-+
++ let logger_with_chan_monitor = WithChannelMonitor::from(logger, &channel_monitor);
+ channel_monitor.provide_initial_counterparty_commitment_tx(
+ counterparty_initial_bitcoin_tx.txid, Vec::new(),
+ self.context.cur_counterparty_commitment_transaction_number,
+ self.context.counterparty_cur_commitment_point.unwrap(),
+ counterparty_initial_commitment_tx.feerate_per_kw(),
+ counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
-+ counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
++ counterparty_initial_commitment_tx.to_countersignatory_value_sat(), &&logger_with_chan_monitor);
+
+ assert_eq!(self.context.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update!
+ if self.context.is_batch_funding() {
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Tar
}
/// A not-yet-funded inbound (from counterparty) channel using V1 channel establishment.
-@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
+@@ lightning/src/ln/channel.rs: mod tests {
value: 10000000, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, P
// Put some inbound and outbound HTLCs in A's channel.
let htlc_amount_msat = 11_092_000; // put an amount below A's effective dust limit but above B's.
-@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
+@@ lightning/src/ln/channel.rs: mod tests {
value: 10000000, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, P
// Now disconnect the two nodes and check that the commitment point in
// Node B's channel_reestablish message is sane.
-@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
+@@ lightning/src/ln/channel.rs: mod tests {
value: 10000000, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, P
// Make sure that receiving a channel update will update the Channel as expected.
let update = ChannelUpdate {
-@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
+@@ lightning/src/ln/channel.rs: mod tests {
},
]};
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, P
).map_err(|_| ()).unwrap();
let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(
&funding_created_msg.unwrap(),
-@@ lightning/src/ln/channel.rs: use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
+@@ lightning/src/ln/channel.rs: mod tests {
// Receive funding_signed, but the channel will be configured to hold sending channel_ready and
// broadcasting the funding transaction until the batch is ready.
@@ lightning/src/ln/channelmanager.rs: where
+ Some(ChannelPhase::UnfundedOutboundV1(mut chan)) => {
let funding_txo = find_funding_output(&chan, &funding_transaction)?;
- let funding_res = chan.get_funding_created(funding_transaction, funding_txo, is_batch_funding, &self.logger)
+ let logger = WithChannelContext::from(&self.logger, &chan.context);
@@ lightning/src/ln/channelmanager.rs: where
(chan, MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, user_id, shutdown_res, None, channel_capacity))
} else { unreachable!(); });
@@ lightning/src/ln/channelmanager.rs: where
}
}
Ok(())
+@@ lightning/src/ln/channelmanager.rs: where
+ // We got a temporary failure updating monitor, but will claim the
+ // HTLC when the monitor updating is restored (or on chain).
+ let logger = WithContext::from(&self.logger, None, Some(prev_hop_chan_id));
+- log_error!(self.logger, "Temporary failure claiming HTLC, treating as success: {}", err.err.err);
++ log_error!(logger, "Temporary failure claiming HTLC, treating as success: {}", err.err.err);
+ } else { errs.push((pk, err)); }
+ }
+ }
@@ lightning/src/ln/channelmanager.rs: where
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
let peer_state = &mut *peer_state_lock;
@@ lightning/src/ln/channelmanager.rs: where
- hash_map::Entry::Occupied(mut chan_phase_entry) => {
- match chan_phase_entry.get_mut() {
- ChannelPhase::Funded(ref mut chan) => {
+- let logger = WithChannelContext::from(&self.logger, &chan.context);
- let monitor = try_chan_phase_entry!(self,
-- chan.funding_signed(&msg, best_block, &self.signer_provider, &self.logger), chan_phase_entry);
+- chan.funding_signed(&msg, best_block, &self.signer_provider, &&logger), chan_phase_entry);
- if let Ok(persist_status) = self.chain_monitor.watch_channel(chan.context.get_funding_txo().unwrap(), monitor) {
- handle_new_monitor_update!(self, persist_status, peer_state_lock, peer_state, per_peer_state, chan, INITIAL_MONITOR);
- Ok(())
@@ lightning/src/ln/channelmanager.rs: where
+ hash_map::Entry::Occupied(chan_phase_entry) => {
+ if matches!(chan_phase_entry.get(), ChannelPhase::UnfundedOutboundV1(_)) {
+ let chan = if let ChannelPhase::UnfundedOutboundV1(chan) = chan_phase_entry.remove() { chan } else { unreachable!() };
++ let logger = WithContext::from(
++ &self.logger,
++ Some(chan.context.get_counterparty_node_id()),
++ Some(chan.context.channel_id())
++ );
+ let res =
-+ chan.funding_signed(&msg, best_block, &self.signer_provider, &self.logger);
++ chan.funding_signed(&msg, best_block, &self.signer_provider, &&logger);
+ match res {
+ Ok((chan, monitor)) => {
+ if let Ok(persist_status) = self.chain_monitor.watch_channel(chan.context.get_funding_txo().unwrap(), monitor) {
@@ lightning/src/ln/channelmanager.rs: where
+ Err((chan, e)) => {
+ debug_assert!(matches!(e, ChannelError::Close(_)),
+ "We don't have a channel anymore, so the error better have expected close");
-+ // We've already removed this inbound channel from the map in `PeerState`
-+ // above so at this point we just need to clean up any lingering entries
-+ // concerning this channel as it is safe to do so.
++ // We've already removed this outbound channel from the map in
++ // `PeerState` above so at this point we just need to clean up any
++ // lingering entries concerning this channel as it is safe to do so.
+ return Err(convert_chan_phase_err!(self, e, &mut ChannelPhase::UnfundedOutboundV1(chan), &msg.channel_id).1);
}
- },
2: fc8daa30e ! 2: 973de6f4e Limit the scope of `get_funding_created_msg` to outbound channels
@@ lightning/src/ln/channel.rs: impl<SP: Deref> ChannelContext<SP> where SP::Target
- let signature = match &self.holder_signer {
- // TODO (taproot|arik): move match into calling method for Taproot
- ChannelSignerType::Ecdsa(ecdsa) => {
-- ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
+- ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), Vec::new(), &self.secp_ctx)
- .map(|(sig, _)| sig).ok()?
- },
- // TODO (taproot|arik)
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Tar
})
}
-+ /// Only allowed after [`Self::channel_transaction_parameters`] is set.
++ /// Only allowed after [`ChannelContext::channel_transaction_parameters`] is set.
+ fn get_funding_created_msg<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> 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;
+ let signature = match &self.context.holder_signer {
+ // TODO (taproot|arik): move match into calling method for Taproot
+ ChannelSignerType::Ecdsa(ecdsa) => {
-+ ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
++ ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), Vec::new(), &self.context.secp_ctx)
+ .map(|(sig, _)| sig).ok()?
+ },
+ // TODO (taproot|arik)
3: 0e264f9df ! 3: bfb503eec Drop unreachable shutdown code in `Channel::get_shutdown`
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
- monitor_update: None,
- dropped_outbound_htlcs: Vec::new(),
- unbroadcasted_batch_funding_txid: self.context.unbroadcasted_batch_funding_txid(),
+- channel_id: self.context.channel_id,
+- counterparty_node_id: self.context.counterparty_node_id,
- };
- self.context.channel_state = ChannelState::ShutdownComplete as u32;
- Some(shutdown_result)
4: 80f9d505a = 4: 0d3000520 Move pre-funded-channel immediate shutdown logic to the right place
5: 5db34c585 = 5: 3f6e29bd0 Immediately error in `close_channel_internal` if there is no chan |
wpaulino
previously approved these changes
Dec 5, 2023
|
Needs rebase |
Previously, channels were stored in different maps in `PeerState` based on whether the funding had been set, keeping the keys across the maps consistent (pre-funding temporary_channel_ids vs funding-outpoint-based channel_ids). However, channels are now stored in a single `channel_by_id` map, making that point moot. Instead, here, we convert the `ChannelPhase` state transition boundary to "once we have a `ChannelMonitor`", which makes more sense now, and was actually the original proposed boundary. This also requires calling `signer_maybe_unblocked` on a pre-funded outbound channel, but that nicely also lets us limit the scope of `FundingCreated` message generation, which we do in the next commit.
Since we no longer use `ChannelContext::get_funding_created_msg`, it can be moved back into `UnfundedOutboundV1` channels only, where it realistically belongs.
`Channel` is only a thing for funded channels. Thus, checking if a channel has not yet been funded is dead code and can simply be elided.
Because a `Funded` `Channel` cannot possibly be pre-funding, the
logic in `ChannelManager::close_channel_internal` to handle
pre-funding channels is in the wrong place.
Rather than being handled inside the `Funded` branch, it should be
in an `else` following it, handling either of the two
`ChannelPhases` outside of `Funded`.
Sadly, because of a previous control flow management `loop {}`, the
existing code will infinite loop, which is fixed here.
Previously, unfunded channels would be stored outside of `PeerState::channel_by_id`, and thus if there is no channel when we look in `PeerState::channel_by_id`, `close_channel_internal` called `force_close_channel_with_peer` to hunt for unfunded channels. However, that is no longer the case, so the call is redundant, and we can simply return an error instead.
TheBlueMatt
force-pushed
the
2023-11-chan-close-loop
branch
from
3f6e29b to
5ba4c07
Compare
|
Rebased: $ git range-diff 37150b4d697f723f278b6cf3f63892df272b9aa5..3f6e29bd0519dba925992b24455118f1a4e3400d 2d266794c2133782938ce62a96538ef138c85781..5ba4c079bb242f5a0279a36d24ba87e3d8d1d528
1: c8124e4fb ! 1: 6ab56a40b Move to `Funded` after `funding_signed` rather than on funding
@@ lightning/src/ln/channel.rs: pub(super) struct MonitorRestoreUpdates {
}
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
+ }
// Message handlers:
-
+-
- /// Handles a funding_signed message from the remote end.
- /// If this call is successful, broadcast the funding transaction (and not before!)
- pub fn funding_signed<L: Deref>(
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
- if !self.context.is_outbound() {
- return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
- }
-- if self.context.channel_state & !(ChannelState::MonitorUpdateInProgress as u32) != ChannelState::FundingCreated as u32 {
+- if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
- return Err(ChannelError::Close("Received funding_signed in strange state!".to_owned()));
- }
- if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
- funding_redeemscript.clone(), self.context.channel_value_satoshis,
- obscure_factor,
- holder_commitment_tx, best_block, self.context.counterparty_node_id);
-- let logger_with_chan_monitor = WithChannelMonitor::from(logger, &channel_monitor);
- channel_monitor.provide_initial_counterparty_commitment_tx(
- counterparty_initial_bitcoin_tx.txid, Vec::new(),
- self.context.cur_counterparty_commitment_transaction_number,
- self.context.counterparty_cur_commitment_point.unwrap(),
- counterparty_initial_commitment_tx.feerate_per_kw(),
- counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
-- counterparty_initial_commitment_tx.to_countersignatory_value_sat(), &&logger_with_chan_monitor);
+- counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
-
-- assert_eq!(self.context.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update!
+- assert!(!self.context.channel_state.is_monitor_update_in_progress()); // We have no had any monitor(s) yet to fail update!
- if self.context.is_batch_funding() {
-- self.context.channel_state = ChannelState::FundingSent as u32 | ChannelState::WaitingForBatch as u32;
+- self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
- } else {
-- self.context.channel_state = ChannelState::FundingSent as u32;
+- self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
- }
- self.context.cur_holder_commitment_transaction_number -= 1;
- self.context.cur_counterparty_commitment_transaction_number -= 1;
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Tar
+ if !self.context.is_outbound() {
+ return Err((self, ChannelError::Close("Received funding_signed for an inbound channel?".to_owned())));
+ }
-+ if self.context.channel_state & !(ChannelState::MonitorUpdateInProgress as u32) != ChannelState::FundingCreated as u32 {
++ if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
+ return Err((self, ChannelError::Close("Received funding_signed in strange state!".to_owned())));
+ }
+ if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
@@ lightning/src/ln/channel.rs: impl<SP: Deref> OutboundV1Channel<SP> where SP::Tar
+ funding_redeemscript.clone(), self.context.channel_value_satoshis,
+ obscure_factor,
+ holder_commitment_tx, best_block, self.context.counterparty_node_id);
-+ let logger_with_chan_monitor = WithChannelMonitor::from(logger, &channel_monitor);
+ channel_monitor.provide_initial_counterparty_commitment_tx(
+ counterparty_initial_bitcoin_tx.txid, Vec::new(),
+ self.context.cur_counterparty_commitment_transaction_number,
+ self.context.counterparty_cur_commitment_point.unwrap(),
+ counterparty_initial_commitment_tx.feerate_per_kw(),
+ counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
-+ counterparty_initial_commitment_tx.to_countersignatory_value_sat(), &&logger_with_chan_monitor);
++ counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
+
-+ assert_eq!(self.context.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update!
++ assert!(!self.context.channel_state.is_monitor_update_in_progress()); // We have no had any monitor(s) yet to fail update!
+ if self.context.is_batch_funding() {
-+ self.context.channel_state = ChannelState::FundingSent as u32 | ChannelState::WaitingForBatch as u32;
++ self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
+ } else {
-+ self.context.channel_state = ChannelState::FundingSent as u32;
++ self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+ }
+ self.context.cur_holder_commitment_transaction_number -= 1;
+ self.context.cur_counterparty_commitment_transaction_number -= 1;
2: 973de6f4e = 2: 8e519b56d Limit the scope of `get_funding_created_msg` to outbound channels
3: bfb503eec ! 3: 1667b4d20 Drop unreachable shutdown code in `Channel::get_shutdown`
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
- // If we haven't funded the channel yet, we don't need to bother ensuring the shutdown
- // script is set, we just force-close and call it a day.
- let mut chan_closed = false;
-- if self.context.channel_state & !STATE_FLAGS < ChannelState::FundingSent as u32 {
+- if self.context.channel_state.is_pre_funded_state() {
- chan_closed = true;
- }
-
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
// From here on out, we may not fail!
self.context.target_closing_feerate_sats_per_kw = target_feerate_sats_per_kw;
-- let shutdown_result = if self.context.channel_state & !STATE_FLAGS < ChannelState::FundingSent as u32 {
+- let shutdown_result = if self.context.channel_state.is_pre_funded_state() {
- let shutdown_result = ShutdownResult {
- monitor_update: None,
- dropped_outbound_htlcs: Vec::new(),
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
- channel_id: self.context.channel_id,
- counterparty_node_id: self.context.counterparty_node_id,
- };
-- self.context.channel_state = ChannelState::ShutdownComplete as u32;
+- self.context.channel_state = ChannelState::ShutdownComplete;
- Some(shutdown_result)
- } else {
-- self.context.channel_state |= ChannelState::LocalShutdownSent as u32;
+- self.context.channel_state.set_local_shutdown_sent();
- None
- };
-+ self.context.channel_state |= ChannelState::LocalShutdownSent as u32;
++ self.context.channel_state.set_local_shutdown_sent();
self.context.update_time_counter += 1;
let monitor_update = if update_shutdown_script {
4: 0d3000520 = 4: 3a2690c8a Move pre-funded-channel immediate shutdown logic to the right place
5: 3f6e29bd0 = 5: 5ba4c079b Immediately error in `close_channel_internal` if there is no chan |
shaavan
approved these changes
Dec 8, 2023
wpaulino
approved these changes
Dec 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because a
FundedChannelcannot possibly be pre-funding, thelogic in
ChannelManager::close_channel_internalto handlepre-funding channels is in the wrong place.
Rather than being handled inside the
Fundedbranch, it should bein an
elsefollowing it, handling either of the twoChannelPhasesoutside ofFunded.Sadly, because of a previous control flow management
loop {}, theexisting code will infinite loop, which is fixed here.