Skip to content

Commit dee2d73

Browse files
TheBlueMattTheBlueMatt
committed
Use new monitor persistence flow in funding_signed handling
In the previous commit, we moved all our `ChannelMonitorUpdate` pipelines to use a new async path via the `handle_new_monitor_update` macro. This avoids having two message sending pathways and simply sends messages in the "monitor update completed" flow, which is shared between sync and async monitor updates. Here we reuse the new macro for handling `funding_signed` messages when doing an initial `ChannelMonitor` persistence. This provides a similar benefit, simplifying the code a trivial amount, but importantly allows us to fully remove the original `handle_monitor_update_res` macro.
1 parent af22755 commit dee2d73

File tree

2 files changed

+33
-112
lines changed

2 files changed

+33
-112
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2320,11 +2320,8 @@ impl<Signer: Sign> Channel<Signer> {
23202320
/// If this call is successful, broadcast the funding transaction (and not before!)
23212321
pub fn funding_signed<K: Deref, L: Deref>(
23222322
&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, keys_source: &K, logger: &L
2323-
) -> Result<(ChannelMonitor<<K::Target as SignerProvider>::Signer>, Transaction, Option<msgs::ChannelReady>), ChannelError>
2324-
where
2325-
K::Target: KeysInterface,
2326-
L::Target: Logger
2327-
{
2323+
) -> Result<ChannelMonitor<Signer>, ChannelError>
2324+
where K::Target: KeysInterface<Signer = Signer>, L::Target: Logger {
23282325
if !self.is_outbound() {
23292326
return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
23302327
}
@@ -2395,7 +2392,9 @@ impl<Signer: Sign> Channel<Signer> {
23952392

23962393
log_info!(logger, "Received funding_signed from peer for channel {}", log_bytes!(self.channel_id()));
23972394

2398-
Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap(), self.check_get_channel_ready(0)))
2395+
let need_channel_ready = self.check_get_channel_ready(0).is_some();
2396+
self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
2397+
Ok(channel_monitor)
23992398
}
24002399

24012400
/// Handles a channel_ready message from our peer. If we've already sent our channel_ready

‎lightning/src/ln/channelmanager.rs

Lines changed: 28 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,69 +1302,6 @@ macro_rules! remove_channel {
13021302
}
13031303
}
13041304

1305-
macro_rules! handle_monitor_update_res {
1306-
($self: ident, $err: expr, $chan: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $resend_channel_ready: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr, $chan_id: expr) => {
1307-
match $err {
1308-
ChannelMonitorUpdateStatus::PermanentFailure => {
1309-
log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateStatus::PermanentFailure", log_bytes!($chan_id[..]));
1310-
update_maps_on_chan_removal!($self, $chan);
1311-
let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id, $chan.get_user_id(),
1312-
$chan.force_shutdown(false), $self.get_channel_update_for_broadcast(&$chan).ok() ));
1313-
(res, true)
1314-
},
1315-
ChannelMonitorUpdateStatus::InProgress => {
1316-
log_info!($self.logger, "Disabling channel {} due to monitor update in progress. On restore will send {} and process {} forwards, {} fails, and {} fulfill finalizations",
1317-
log_bytes!($chan_id[..]),
1318-
if $resend_commitment && $resend_raa {
1319-
match $action_type {
1320-
RAACommitmentOrder::CommitmentFirst => { "commitment then RAA" },
1321-
RAACommitmentOrder::RevokeAndACKFirst => { "RAA then commitment" },
1322-
}
1323-
} else if $resend_commitment { "commitment" }
1324-
else if $resend_raa { "RAA" }
1325-
else { "nothing" },
1326-
(&$failed_forwards as &Vec<(PendingHTLCInfo, u64)>).len(),
1327-
(&$failed_fails as &Vec<(HTLCSource, PaymentHash, HTLCFailReason)>).len(),
1328-
(&$failed_finalized_fulfills as &Vec<HTLCSource>).len());
1329-
if !$resend_commitment {
1330-
debug_assert!($action_type == RAACommitmentOrder::RevokeAndACKFirst || !$resend_raa);
1331-
}
1332-
if !$resend_raa {
1333-
debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst || !$resend_commitment);
1334-
}
1335-
$chan.monitor_updating_paused($resend_raa, $resend_commitment, $resend_channel_ready, $failed_forwards, $failed_fails, $failed_finalized_fulfills);
1336-
(Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore("Failed to update ChannelMonitor".to_owned()), *$chan_id)), false)
1337-
},
1338-
ChannelMonitorUpdateStatus::Completed => {
1339-
(Ok(()), false)
1340-
},
1341-
}
1342-
};
1343-
($self: ident, $err: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $resend_channel_ready: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr) => { {
1344-
let (res, drop) = handle_monitor_update_res!($self, $err, $entry.get_mut(), $action_type, $resend_raa, $resend_commitment, $resend_channel_ready, $failed_forwards, $failed_fails, $failed_finalized_fulfills, $entry.key());
1345-
if drop {
1346-
$entry.remove_entry();
1347-
}
1348-
res
1349-
} };
1350-
($self: ident, $err: expr, $entry: expr, $action_type: path, $chan_id: expr, COMMITMENT_UPDATE_ONLY) => { {
1351-
debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst);
1352-
handle_monitor_update_res!($self, $err, $entry, $action_type, false, true, false, Vec::new(), Vec::new(), Vec::new(), $chan_id)
1353-
} };
1354-
($self: ident, $err: expr, $entry: expr, $action_type: path, $chan_id: expr, NO_UPDATE) => {
1355-
handle_monitor_update_res!($self, $err, $entry, $action_type, false, false, false, Vec::new(), Vec::new(), Vec::new(), $chan_id)
1356-
};
1357-
($self: ident, $err: expr, $entry: expr, $action_type: path, $resend_channel_ready: expr, OPTIONALLY_RESEND_FUNDING_LOCKED) => {
1358-
handle_monitor_update_res!($self, $err, $entry, $action_type, false, false, $resend_channel_ready, Vec::new(), Vec::new(), Vec::new())
1359-
};
1360-
($self: ident, $err: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr) => {
1361-
handle_monitor_update_res!($self, $err, $entry, $action_type, $resend_raa, $resend_commitment, false, Vec::new(), Vec::new(), Vec::new())
1362-
};
1363-
($self: ident, $err: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => {
1364-
handle_monitor_update_res!($self, $err, $entry, $action_type, $resend_raa, $resend_commitment, false, $failed_forwards, $failed_fails, Vec::new())
1365-
};
1366-
}
1367-
13681305
macro_rules! send_channel_ready {
13691306
($self: ident, $pending_msg_events: expr, $channel: expr, $channel_ready_msg: expr) => {{
13701307
$pending_msg_events.push(events::MessageSendEvent::SendChannelReady {
@@ -1422,9 +1359,9 @@ macro_rules! handle_new_monitor_update {
14221359
res
14231360
},
14241361
ChannelMonitorUpdateStatus::Completed => {
1425-
if $chan.get_next_monitor_update()
1426-
.expect("We can't be processing a monitor udpate if it isn't queued")
1427-
.update_id == $update_id &&
1362+
if ($update_id == 0 || $chan.get_next_monitor_update()
1363+
.expect("We can't be processing a monitor update if it isn't queued")
1364+
.update_id == $update_id) &&
14281365
$chan.get_latest_monitor_update_id() == $update_id
14291366
{
14301367
let mut updates = $chan.monitor_updating_restored(&$self.logger,
@@ -4368,48 +4305,33 @@ where
43684305
}
43694306

43704307
fn internal_funding_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> {
4371-
let funding_tx = {
4372-
let best_block = *self.best_block.read().unwrap();
4373-
let per_peer_state = self.per_peer_state.read().unwrap();
4374-
let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
4375-
if let None = peer_state_mutex_opt {
4376-
return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
4377-
}
4308+
let best_block = *self.best_block.read().unwrap();
4309+
let per_peer_state = self.per_peer_state.read().unwrap();
4310+
let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
4311+
if let None = peer_state_mutex_opt {
4312+
return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
4313+
}
43784314

4379-
let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
4380-
let peer_state = &mut *peer_state_lock;
4381-
match peer_state.channel_by_id.entry(msg.channel_id) {
4382-
hash_map::Entry::Occupied(mut chan) => {
4383-
let (monitor, funding_tx, channel_ready) = match chan.get_mut().funding_signed(&msg, best_block, &self.keys_manager, &self.logger) {
4384-
Ok(update) => update,
4385-
Err(e) => try_chan_entry!(self, Err(e), chan),
4386-
};
4387-
match self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor) {
4388-
ChannelMonitorUpdateStatus::Completed => {},
4389-
e => {
4390-
let mut res = handle_monitor_update_res!(self, e, chan, RAACommitmentOrder::RevokeAndACKFirst, channel_ready.is_some(), OPTIONALLY_RESEND_FUNDING_LOCKED);
4391-
if let Err(MsgHandleErrInternal { ref mut shutdown_finish, .. }) = res {
4392-
// We weren't able to watch the channel to begin with, so no updates should be made on
4393-
// it. Previously, full_stack_target found an (unreachable) panic when the
4394-
// monitor update contained within `shutdown_finish` was applied.
4395-
if let Some((ref mut shutdown_finish, _)) = shutdown_finish {
4396-
shutdown_finish.0.take();
4397-
}
4398-
}
4399-
return res
4400-
},
4401-
}
4402-
if let Some(msg) = channel_ready {
4403-
send_channel_ready!(self, peer_state.pending_msg_events, chan.get(), msg);
4315+
let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
4316+
let peer_state = &mut *peer_state_lock;
4317+
match peer_state.channel_by_id.entry(msg.channel_id) {
4318+
hash_map::Entry::Occupied(mut chan) => {
4319+
let monitor = try_chan_entry!(self,
4320+
chan.get_mut().funding_signed(&msg, best_block, &self.keys_manager, &self.logger), chan);
4321+
let update_res = self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor);
4322+
let mut res = handle_new_monitor_update!(self, update_res, 0, peer_state_lock, peer_state, chan);
4323+
if let Err(MsgHandleErrInternal { ref mut shutdown_finish, .. }) = res {
4324+
// We weren't able to watch the channel to begin with, so no updates should be made on
4325+
// it. Previously, full_stack_target found an (unreachable) panic when the
4326+
// monitor update contained within `shutdown_finish` was applied.
4327+
if let Some((ref mut shutdown_finish, _)) = shutdown_finish {
4328+
shutdown_finish.0.take();
44044329
}
4405-
funding_tx
4406-
},
4407-
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
4408-
}
4409-
};
4410-
log_info!(self.logger, "Broadcasting funding transaction with txid {}", funding_tx.txid());
4411-
self.tx_broadcaster.broadcast_transaction(&funding_tx);
4412-
Ok(())
4330+
}
4331+
res
4332+
},
4333+
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
4334+
}
44134335
}
44144336

44154337
fn internal_channel_ready(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReady) -> Result<(), MsgHandleErrInternal> {

0 commit comments

Comments
 (0)