Skip to content

Commit b8c3a2f

Browse files
author
Antoine Riard
committed
Drop Result for ChannelMessageHandler methods
Simplify interfaces between ChannelMessageHandler and PeerManager, by switching all ChannelMessageHandler errors to HandleError sent internally instead of being return. With further refactors in Router and PeerChannelEncryptor, errors management on the PeerManager-side won't be splitted between try_potential_handleerror and HandleError processing. Inside ChannelManager, we now log MsgHandleErrInternal and send ErrorAction to PeerManager. On a high-level, it should allow client using API to be more flexible by polling events instead of waiting function call returns.
1 parent 126b514 commit b8c3a2f

7 files changed

+796
-786
lines changed

‎lightning/src/ln/chanmon_update_fail_tests.rs

+165-151
Large diffs are not rendered by default.

‎lightning/src/ln/channelmanager.rs

+51-116
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ pub struct ChannelDetails {
422422
}
423423

424424
macro_rules! handle_error {
425-
($self: ident, $internal: expr) => {
425+
($self: ident, $internal: expr, $their_node_id: expr) => {
426426
match $internal {
427427
Ok(msg) => Ok(msg),
428428
Err(MsgHandleErrInternal { err, shutdown_finish }) => {
@@ -435,6 +435,10 @@ macro_rules! handle_error {
435435
});
436436
}
437437
}
438+
let mut channel_state = $self.channel_state.lock().unwrap();
439+
log_error!($self, "Got Error because {}", err.err);
440+
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: $their_node_id, action: err.action.clone() });
441+
// Return error in case higher-API need one
438442
Err(err)
439443
},
440444
}
@@ -1140,20 +1144,9 @@ impl ChannelManager {
11401144
return Ok(());
11411145
};
11421146

1143-
match handle_error!(self, err) {
1147+
match handle_error!(self, err, route.hops.first().unwrap().pubkey) {
11441148
Ok(_) => unreachable!(),
1145-
Err(e) => {
1146-
if let msgs::ErrorAction::IgnoreError = e.action {
1147-
} else {
1148-
log_error!(self, "Got bad keys: {}!", e.err);
1149-
let mut channel_state = self.channel_state.lock().unwrap();
1150-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
1151-
node_id: route.hops.first().unwrap().pubkey,
1152-
action: e.action,
1153-
});
1154-
}
1155-
Err(APIError::ChannelUnavailable { err: e.err })
1156-
},
1149+
Err(e) => { Err(APIError::ChannelUnavailable { err: e.err }) }
11571150
}
11581151
}
11591152

@@ -1183,36 +1176,20 @@ impl ChannelManager {
11831176
None => return
11841177
}
11851178
};
1186-
match handle_error!(self, res) {
1179+
match handle_error!(self, res, chan.get_their_node_id()) {
11871180
Ok(funding_msg) => {
11881181
(chan, funding_msg.0, funding_msg.1)
11891182
},
1190-
Err(e) => {
1191-
log_error!(self, "Got bad signatures: {}!", e.err);
1192-
let mut channel_state = self.channel_state.lock().unwrap();
1193-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
1194-
node_id: chan.get_their_node_id(),
1195-
action: e.action,
1196-
});
1197-
return;
1198-
},
1183+
Err(_) => { return; }
11991184
}
12001185
};
12011186
// Because we have exclusive ownership of the channel here we can release the channel_state
12021187
// lock before add_update_monitor
12031188
if let Err(e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
12041189
match e {
12051190
ChannelMonitorUpdateErr::PermanentFailure => {
1206-
match handle_error!(self, Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", *temporary_channel_id, chan.force_shutdown(), None))) {
1207-
Err(e) => {
1208-
log_error!(self, "Failed to store ChannelMonitor update for funding tx generation");
1209-
let mut channel_state = self.channel_state.lock().unwrap();
1210-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
1211-
node_id: chan.get_their_node_id(),
1212-
action: e.action,
1213-
});
1214-
return;
1215-
},
1191+
match handle_error!(self, Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", *temporary_channel_id, chan.force_shutdown(), None)), chan.get_their_node_id()) {
1192+
Err(_) => { return; },
12161193
Ok(()) => unreachable!(),
12171194
}
12181195
},
@@ -1390,22 +1367,9 @@ impl ChannelManager {
13901367
},
13911368
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"); }
13921369
};
1393-
match handle_error!(self, err) {
1370+
match handle_error!(self, err, their_node_id) {
13941371
Ok(_) => unreachable!(),
1395-
Err(e) => {
1396-
match e.action {
1397-
msgs::ErrorAction::IgnoreError => {},
1398-
_ => {
1399-
log_error!(self, "Got bad keys: {}!", e.err);
1400-
let mut channel_state = self.channel_state.lock().unwrap();
1401-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
1402-
node_id: their_node_id,
1403-
action: e.action,
1404-
});
1405-
},
1406-
}
1407-
continue;
1408-
},
1372+
Err(_) => { continue; },
14091373
}
14101374
}
14111375
};
@@ -1463,18 +1427,9 @@ impl ChannelManager {
14631427
}
14641428

14651429
for (their_node_id, err) in handle_errors.drain(..) {
1466-
match handle_error!(self, err) {
1430+
match handle_error!(self, err, their_node_id) {
14671431
Ok(_) => {},
1468-
Err(e) => {
1469-
if let msgs::ErrorAction::IgnoreError = e.action {
1470-
} else {
1471-
let mut channel_state = self.channel_state.lock().unwrap();
1472-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
1473-
node_id: their_node_id,
1474-
action: e.action,
1475-
});
1476-
}
1477-
},
1432+
Err(_) => {}
14781433
}
14791434
}
14801435

@@ -1702,18 +1657,9 @@ impl ChannelManager {
17021657
return;
17031658
};
17041659

1705-
match handle_error!(self, err) {
1660+
match handle_error!(self, err, their_node_id) {
17061661
Ok(_) => {},
1707-
Err(e) => {
1708-
if let msgs::ErrorAction::IgnoreError = e.action {
1709-
} else {
1710-
let mut channel_state = self.channel_state.lock().unwrap();
1711-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
1712-
node_id: their_node_id,
1713-
action: e.action,
1714-
});
1715-
}
1716-
},
1662+
Err(_) => {},
17171663
}
17181664
}
17191665

@@ -2487,20 +2433,9 @@ impl ChannelManager {
24872433
return Ok(())
24882434
};
24892435

2490-
match handle_error!(self, err) {
2436+
match handle_error!(self, err, their_node_id) {
24912437
Ok(_) => unreachable!(),
2492-
Err(e) => {
2493-
if let msgs::ErrorAction::IgnoreError = e.action {
2494-
} else {
2495-
log_error!(self, "Got bad keys: {}!", e.err);
2496-
let mut channel_state = self.channel_state.lock().unwrap();
2497-
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
2498-
node_id: their_node_id,
2499-
action: e.action,
2500-
});
2501-
}
2502-
Err(APIError::APIMisuseError { err: e.err })
2503-
},
2438+
Err(e) => { Err(APIError::APIMisuseError { err: e.err })}
25042439
}
25052440
}
25062441
}
@@ -2671,84 +2606,84 @@ impl ChainListener for ChannelManager {
26712606

26722607
impl ChannelMessageHandler for ChannelManager {
26732608
//TODO: Handle errors and close channel (or so)
2674-
fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) -> Result<(), LightningError> {
2609+
fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) {
26752610
let _ = self.total_consistency_lock.read().unwrap();
2676-
handle_error!(self, self.internal_open_channel(their_node_id, their_local_features, msg))
2611+
if let Err(_) = handle_error!(self, self.internal_open_channel(their_node_id, their_local_features, msg), *their_node_id) {}
26772612
}
26782613

2679-
fn handle_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::AcceptChannel) -> Result<(), LightningError> {
2614+
fn handle_accept_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::AcceptChannel) {
26802615
let _ = self.total_consistency_lock.read().unwrap();
2681-
handle_error!(self, self.internal_accept_channel(their_node_id, their_local_features, msg))
2616+
if let Err(_) = handle_error!(self, self.internal_accept_channel(their_node_id, their_local_features, msg), *their_node_id) {}
26822617
}
26832618

2684-
fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), LightningError> {
2619+
fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) {
26852620
let _ = self.total_consistency_lock.read().unwrap();
2686-
handle_error!(self, self.internal_funding_created(their_node_id, msg))
2621+
if let Err(_) = handle_error!(self, self.internal_funding_created(their_node_id, msg), *their_node_id) {}
26872622
}
26882623

2689-
fn handle_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), LightningError> {
2624+
fn handle_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) {
26902625
let _ = self.total_consistency_lock.read().unwrap();
2691-
handle_error!(self, self.internal_funding_signed(their_node_id, msg))
2626+
if let Err(_) = handle_error!(self, self.internal_funding_signed(their_node_id, msg), *their_node_id) {}
26922627
}
26932628

2694-
fn handle_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) -> Result<(), LightningError> {
2629+
fn handle_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) {
26952630
let _ = self.total_consistency_lock.read().unwrap();
2696-
handle_error!(self, self.internal_funding_locked(their_node_id, msg))
2631+
if let Err(_) = handle_error!(self, self.internal_funding_locked(their_node_id, msg), *their_node_id) {}
26972632
}
26982633

2699-
fn handle_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), LightningError> {
2634+
fn handle_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) {
27002635
let _ = self.total_consistency_lock.read().unwrap();
2701-
handle_error!(self, self.internal_shutdown(their_node_id, msg))
2636+
if let Err(_) = handle_error!(self, self.internal_shutdown(their_node_id, msg), *their_node_id) {}
27022637
}
27032638

2704-
fn handle_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), LightningError> {
2639+
fn handle_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) {
27052640
let _ = self.total_consistency_lock.read().unwrap();
2706-
handle_error!(self, self.internal_closing_signed(their_node_id, msg))
2641+
if let Err(_) = handle_error!(self, self.internal_closing_signed(their_node_id, msg), *their_node_id) {}
27072642
}
27082643

2709-
fn handle_update_add_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateAddHTLC) -> Result<(), LightningError> {
2644+
fn handle_update_add_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateAddHTLC) {
27102645
let _ = self.total_consistency_lock.read().unwrap();
2711-
handle_error!(self, self.internal_update_add_htlc(their_node_id, msg))
2646+
if let Err(_) = handle_error!(self, self.internal_update_add_htlc(their_node_id, msg), *their_node_id) {}
27122647
}
27132648

2714-
fn handle_update_fulfill_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), LightningError> {
2649+
fn handle_update_fulfill_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) {
27152650
let _ = self.total_consistency_lock.read().unwrap();
2716-
handle_error!(self, self.internal_update_fulfill_htlc(their_node_id, msg))
2651+
if let Err(_) = handle_error!(self, self.internal_update_fulfill_htlc(their_node_id, msg), *their_node_id) {}
27172652
}
27182653

2719-
fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), LightningError> {
2654+
fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) {
27202655
let _ = self.total_consistency_lock.read().unwrap();
2721-
handle_error!(self, self.internal_update_fail_htlc(their_node_id, msg))
2656+
if let Err(_) = handle_error!(self, self.internal_update_fail_htlc(their_node_id, msg), *their_node_id) {}
27222657
}
27232658

2724-
fn handle_update_fail_malformed_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailMalformedHTLC) -> Result<(), LightningError> {
2659+
fn handle_update_fail_malformed_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailMalformedHTLC) {
27252660
let _ = self.total_consistency_lock.read().unwrap();
2726-
handle_error!(self, self.internal_update_fail_malformed_htlc(their_node_id, msg))
2661+
if let Err(_) = handle_error!(self, self.internal_update_fail_malformed_htlc(their_node_id, msg), *their_node_id) {}
27272662
}
27282663

2729-
fn handle_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(), LightningError> {
2664+
fn handle_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) {
27302665
let _ = self.total_consistency_lock.read().unwrap();
2731-
handle_error!(self, self.internal_commitment_signed(their_node_id, msg))
2666+
if let Err(_) = handle_error!(self, self.internal_commitment_signed(their_node_id, msg), *their_node_id) {}
27322667
}
27332668

2734-
fn handle_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<(), LightningError> {
2669+
fn handle_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) {
27352670
let _ = self.total_consistency_lock.read().unwrap();
2736-
handle_error!(self, self.internal_revoke_and_ack(their_node_id, msg))
2671+
if let Err(_) = handle_error!(self, self.internal_revoke_and_ack(their_node_id, msg), *their_node_id) {}
27372672
}
27382673

2739-
fn handle_update_fee(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFee) -> Result<(), LightningError> {
2674+
fn handle_update_fee(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFee) {
27402675
let _ = self.total_consistency_lock.read().unwrap();
2741-
handle_error!(self, self.internal_update_fee(their_node_id, msg))
2676+
if let Err(_) = handle_error!(self, self.internal_update_fee(their_node_id, msg), *their_node_id) {}
27422677
}
27432678

2744-
fn handle_announcement_signatures(&self, their_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) -> Result<(), LightningError> {
2679+
fn handle_announcement_signatures(&self, their_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) {
27452680
let _ = self.total_consistency_lock.read().unwrap();
2746-
handle_error!(self, self.internal_announcement_signatures(their_node_id, msg))
2681+
if let Err(_) = handle_error!(self, self.internal_announcement_signatures(their_node_id, msg), *their_node_id) {}
27472682
}
27482683

2749-
fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), LightningError> {
2684+
fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) {
27502685
let _ = self.total_consistency_lock.read().unwrap();
2751-
handle_error!(self, self.internal_channel_reestablish(their_node_id, msg))
2686+
if let Err(_) = handle_error!(self, self.internal_channel_reestablish(their_node_id, msg), *their_node_id) {}
27522687
}
27532688

27542689
fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool) {

0 commit comments

Comments
 (0)