Skip to content

Commit 971f7a7

Browse files
TheBlueMattTheBlueMatt
committed
Drop error handling in handle_new_monitor_update
Now that `handle_new_monitor_update` can no longer return an error, it similarly no longer needs any code to handle errors. Here we remove that code, substantially reducing macro variants.
1 parent 341163e commit 971f7a7

File tree

1 file changed

+22
-54
lines changed

1 file changed

+22
-54
lines changed

‎lightning/src/ln/channelmanager.rs

+22-54
Original file line numberDiff line numberDiff line change
@@ -2042,10 +2042,7 @@ macro_rules! handle_monitor_update_completion {
20422042
}
20432043

20442044
macro_rules! handle_new_monitor_update {
2045-
($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, _internal, $remove: expr, $completed: expr) => { {
2046-
// update_maps_on_chan_removal needs to be able to take id_to_peer, so make sure we can in
2047-
// any case so that it won't deadlock.
2048-
debug_assert_ne!($self.id_to_peer.held_by_thread(), LockHeldState::HeldByThread);
2045+
($self: ident, $update_res: expr, $chan: expr, _internal, $completed: expr) => { {
20492046
debug_assert!($self.background_events_processed_since_startup.load(Ordering::Acquire));
20502047
match $update_res {
20512048
ChannelMonitorUpdateStatus::InProgress => {
@@ -2059,23 +2056,11 @@ macro_rules! handle_new_monitor_update {
20592056
},
20602057
}
20612058
} };
2062-
($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, MANUALLY_REMOVING_INITIAL_MONITOR, $remove: expr) => {
2063-
handle_new_monitor_update!($self, $update_res, $peer_state_lock, $peer_state,
2064-
$per_peer_state_lock, $chan, _internal, $remove,
2059+
($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, INITIAL_MONITOR) => {
2060+
handle_new_monitor_update!($self, $update_res, $chan, _internal,
20652061
handle_monitor_update_completion!($self, $peer_state_lock, $peer_state, $per_peer_state_lock, $chan))
20662062
};
2067-
($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan_entry: expr, INITIAL_MONITOR) => {
2068-
if let ChannelPhase::Funded(chan) = $chan_entry.get_mut() {
2069-
handle_new_monitor_update!($self, $update_res, $peer_state_lock, $peer_state,
2070-
$per_peer_state_lock, chan, MANUALLY_REMOVING_INITIAL_MONITOR, { $chan_entry.remove() })
2071-
} else {
2072-
// We're not supposed to handle monitor updates for unfunded channels (they have no monitors to
2073-
// update). Throwing away a monitor update could be dangerous, so we assert even in
2074-
// release builds.
2075-
panic!("Initial Monitors should not exist for non-funded channels");
2076-
}
2077-
};
2078-
($self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, MANUALLY_REMOVING, $remove: expr) => { {
2063+
($self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr) => { {
20792064
let in_flight_updates = $peer_state.in_flight_monitor_updates.entry($funding_txo)
20802065
.or_insert_with(Vec::new);
20812066
// During startup, we push monitor updates as background events through to here in
@@ -2087,26 +2072,14 @@ macro_rules! handle_new_monitor_update {
20872072
in_flight_updates.len() - 1
20882073
});
20892074
let update_res = $self.chain_monitor.update_channel($funding_txo, &in_flight_updates[idx]);
2090-
handle_new_monitor_update!($self, update_res, $peer_state_lock, $peer_state,
2091-
$per_peer_state_lock, $chan, _internal, $remove,
2075+
handle_new_monitor_update!($self, update_res, $chan, _internal,
20922076
{
20932077
let _ = in_flight_updates.remove(idx);
20942078
if in_flight_updates.is_empty() && $chan.blocked_monitor_updates_pending() == 0 {
20952079
handle_monitor_update_completion!($self, $peer_state_lock, $peer_state, $per_peer_state_lock, $chan);
20962080
}
20972081
})
20982082
} };
2099-
($self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan_entry: expr) => {
2100-
if let ChannelPhase::Funded(chan) = $chan_entry.get_mut() {
2101-
handle_new_monitor_update!($self, $funding_txo, $update, $peer_state_lock, $peer_state,
2102-
$per_peer_state_lock, chan, MANUALLY_REMOVING, { $chan_entry.remove() })
2103-
} else {
2104-
// We're not supposed to handle monitor updates for unfunded channels (they have no monitors to
2105-
// update). Throwing away a monitor update could be dangerous, so we assert even in
2106-
// release builds.
2107-
panic!("Monitor updates should not exist for non-funded channels");
2108-
}
2109-
}
21102083
}
21112084

21122085
macro_rules! process_events_body {
@@ -2551,7 +2524,7 @@ where
25512524
// Update the monitor with the shutdown script if necessary.
25522525
if let Some(monitor_update) = monitor_update_opt.take() {
25532526
handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
2554-
peer_state_lock, peer_state, per_peer_state, chan_phase_entry);
2527+
peer_state_lock, peer_state, per_peer_state, chan);
25552528
break;
25562529
}
25572530

@@ -3325,7 +3298,7 @@ where
33253298
}, onion_packet, None, &self.fee_estimator, &self.logger);
33263299
match break_chan_phase_entry!(self, send_res, chan_phase_entry) {
33273300
Some(monitor_update) => {
3328-
match handle_new_monitor_update!(self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state, chan_phase_entry) {
3301+
match handle_new_monitor_update!(self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state, chan) {
33293302
false => {
33303303
// Note that MonitorUpdateInProgress here indicates (per function
33313304
// docs) that we will resend the commitment update once monitor
@@ -4524,9 +4497,13 @@ where
45244497
let peer_state = &mut *peer_state_lock;
45254498
match peer_state.channel_by_id.entry(funding_txo.to_channel_id()) {
45264499
hash_map::Entry::Occupied(mut chan_phase) => {
4527-
updated_chan = true;
4528-
handle_new_monitor_update!(self, funding_txo, update.clone(),
4529-
peer_state_lock, peer_state, per_peer_state, chan_phase);
4500+
if let ChannelPhase::Funded(chan) = chan_phase.get_mut() {
4501+
updated_chan = true;
4502+
handle_new_monitor_update!(self, funding_txo, update.clone(),
4503+
peer_state_lock, peer_state, per_peer_state, chan);
4504+
} else {
4505+
debug_assert!(false, "We shouldn't have an update for a non-funded channel");
4506+
}
45304507
},
45314508
hash_map::Entry::Vacant(_) => {},
45324509
}
@@ -5259,7 +5236,7 @@ where
52595236
}
52605237
if !during_init {
52615238
handle_new_monitor_update!(self, prev_hop.outpoint, monitor_update, peer_state_lock,
5262-
peer_state, per_peer_state, chan_phase_entry);
5239+
peer_state, per_peer_state, chan);
52635240
} else {
52645241
// If we're running during init we cannot update a monitor directly -
52655242
// they probably haven't actually been loaded yet. Instead, push the
@@ -5903,16 +5880,14 @@ where
59035880
// hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
59045881
// accepted payment from yet. We do, however, need to wait to send our channel_ready
59055882
// until we have persisted our monitor.
5906-
let new_channel_id = funding_msg.channel_id;
59075883
peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
59085884
node_id: counterparty_node_id.clone(),
59095885
msg: funding_msg,
59105886
});
59115887

59125888
if let ChannelPhase::Funded(chan) = e.insert(ChannelPhase::Funded(chan)) {
59135889
handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
5914-
per_peer_state, chan, MANUALLY_REMOVING_INITIAL_MONITOR,
5915-
{ peer_state.channel_by_id.remove(&new_channel_id) });
5890+
per_peer_state, chan, INITIAL_MONITOR);
59165891
} else {
59175892
unreachable!("This must be a funded channel as we just inserted it.");
59185893
}
@@ -5947,7 +5922,7 @@ where
59475922
let monitor = try_chan_phase_entry!(self,
59485923
chan.funding_signed(&msg, best_block, &self.signer_provider, &self.logger), chan_phase_entry);
59495924
if let Ok(persist_status) = self.chain_monitor.watch_channel(chan.context.get_funding_txo().unwrap(), monitor) {
5950-
handle_new_monitor_update!(self, persist_status, peer_state_lock, peer_state, per_peer_state, chan_phase_entry, INITIAL_MONITOR);
5925+
handle_new_monitor_update!(self, persist_status, peer_state_lock, peer_state, per_peer_state, chan, INITIAL_MONITOR);
59515926
Ok(())
59525927
} else {
59535928
try_chan_phase_entry!(self, Err(ChannelError::Close("Channel funding outpoint was a duplicate".to_owned())), chan_phase_entry)
@@ -6054,7 +6029,7 @@ where
60546029
// Update the monitor with the shutdown script if necessary.
60556030
if let Some(monitor_update) = monitor_update_opt {
60566031
handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
6057-
peer_state_lock, peer_state, per_peer_state, chan_phase_entry);
6032+
peer_state_lock, peer_state, per_peer_state, chan);
60586033
}
60596034
},
60606035
ChannelPhase::UnfundedInboundV1(_) | ChannelPhase::UnfundedOutboundV1(_) => {
@@ -6306,7 +6281,7 @@ where
63066281
let monitor_update_opt = try_chan_phase_entry!(self, chan.commitment_signed(&msg, &self.logger), chan_phase_entry);
63076282
if let Some(monitor_update) = monitor_update_opt {
63086283
handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
6309-
peer_state, per_peer_state, chan_phase_entry);
6284+
peer_state, per_peer_state, chan);
63106285
}
63116286
Ok(())
63126287
} else {
@@ -6480,7 +6455,7 @@ where
64806455
let funding_txo = funding_txo_opt
64816456
.expect("Funding outpoint must have been set for RAA handling to succeed");
64826457
handle_new_monitor_update!(self, funding_txo, monitor_update,
6483-
peer_state_lock, peer_state, per_peer_state, chan_phase_entry);
6458+
peer_state_lock, peer_state, per_peer_state, chan);
64846459
}
64856460
htlcs_to_fail
64866461
} else {
@@ -6777,10 +6752,8 @@ where
67776752
if let Some(monitor_update) = monitor_opt {
67786753
has_monitor_update = true;
67796754

6780-
let channel_id: ChannelId = *channel_id;
67816755
handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update,
6782-
peer_state_lock, peer_state, per_peer_state, chan, MANUALLY_REMOVING,
6783-
peer_state.channel_by_id.remove(&channel_id));
6756+
peer_state_lock, peer_state, per_peer_state, chan);
67846757
continue 'peer_loop;
67856758
}
67866759
}
@@ -7089,7 +7062,6 @@ where
70897062
/// operation. It will double-check that nothing *else* is also blocking the same channel from
70907063
/// making progress and then let any blocked [`ChannelMonitorUpdate`]s fly.
70917064
fn handle_monitor_update_release(&self, counterparty_node_id: PublicKey, channel_funding_outpoint: OutPoint, mut completed_blocker: Option<RAAMonitorUpdateBlockingAction>) {
7092-
let mut errors = Vec::new();
70937065
loop {
70947066
let per_peer_state = self.per_peer_state.read().unwrap();
70957067
if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) {
@@ -7122,7 +7094,7 @@ where
71227094
log_debug!(self.logger, "Unlocking monitor updating for channel {} and updating monitor",
71237095
channel_funding_outpoint.to_channel_id());
71247096
handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,
7125-
peer_state_lck, peer_state, per_peer_state, chan_phase_entry);
7097+
peer_state_lck, peer_state, per_peer_state, chan);
71267098
if further_update_exists {
71277099
// If there are more `ChannelMonitorUpdate`s to process, restart at the
71287100
// top of the loop.
@@ -7141,10 +7113,6 @@ where
71417113
}
71427114
break;
71437115
}
7144-
for (err, counterparty_node_id) in errors {
7145-
let res = Err::<(), _>(err);
7146-
let _ = handle_error!(self, res, counterparty_node_id);
7147-
}
71487116
}
71497117

71507118
fn handle_post_event_actions(&self, actions: Vec<EventCompletionAction>) {

0 commit comments

Comments
 (0)