mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-25 15:20:24 +01:00
Code Issues Projects Releases Packages Wiki Activity

Swap handle_monitor_update_fail for a macro ala try_chan_entry

Browse source 
This resolves an API bug where send_payment may return a
MonitorUpdateFailed Err both when the payment will not be sent and
when the HTLC will be retried automatically when monitor updating
is restored. This makes it impossible for a client to know when
they should retry a payment and when they should not.
This commit is contained in:
Matt Corallo 2018-11-26 16:40:15 -05:00
parent 16df97d988
commit 788dd738a8
1 changed files with 65 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

84
src/ln/channelmanager.rs
View file

@ -442,6 +442,24 @@ macro_rules! try_chan_entry {
}
}
// Does not break in case of TemporaryFailure!
macro_rules! maybe_break_monitor_err {
($self: expr, $err: expr, $channel_state: expr, $entry: expr, $action_type: path) => {
match $err {
ChannelMonitorUpdateErr::PermanentFailure => {
let (channel_id, mut chan) = $entry.remove_entry();
if let Some(short_id) = chan.get_short_channel_id() {
$channel_state.short_to_id.remove(&short_id);
}
break Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok()))
},
ChannelMonitorUpdateErr::TemporaryFailure => {
$entry.get_mut().monitor_update_failed($action_type);
},
}
}
}
impl ChannelManager {
/// Constructs a new ChannelManager to hold several channels and route between them.
///
@ -667,33 +685,6 @@ impl ChannelManager {
}
}
fn handle_monitor_update_fail(&self, mut channel_state_lock: MutexGuard<ChannelHolder>, channel_id: &[u8; 32], err: ChannelMonitorUpdateErr, reason: RAACommitmentOrder) {
match err {
ChannelMonitorUpdateErr::PermanentFailure => {
let mut chan = {
let channel_state = channel_state_lock.borrow_parts();
let chan = channel_state.by_id.remove(channel_id).expect("monitor_update_failed must be called within the same lock as the channel get!");
if let Some(short_id) = chan.get_short_channel_id() {
channel_state.short_to_id.remove(&short_id);
}
chan
};
mem::drop(channel_state_lock);
self.finish_force_close_channel(chan.force_shutdown());
if let Ok(update) = self.get_channel_update(&chan) {
let mut channel_state = self.channel_state.lock().unwrap();
channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
msg: update
});
}
},
ChannelMonitorUpdateErr::TemporaryFailure => {
let channel = channel_state_lock.by_id.get_mut(channel_id).expect("monitor_update_failed must be called within the same lock as the channel get!");
channel.monitor_update_failed(reason);
},
}
}
#[inline]
fn gen_rho_mu_from_shared_secret(shared_secret: &[u8]) -> ([u8; 32], [u8; 32]) {
assert_eq!(shared_secret.len(), 32);
@ -1193,7 +1184,17 @@ impl ChannelManager {
/// May generate a SendHTLCs message event on success, which should be relayed.
///
/// Raises APIError::RoutError when invalid route or forward parameter
/// (cltv_delta, fee, node public key) is specified
/// (cltv_delta, fee, node public key) is specified.
/// Raises APIError::ChannelUnavailable if the next-hop channel is not available for updates
/// (including due to previous monitor update failure or new permanent monitor update failure).
/// Raised APIError::MonitorUpdateFailed if a new monitor update failure prevented sending the
/// relevant updates.
///
/// In case of APIError::RouteError/APIError::ChannelUnavailable, the payment send has failed
/// and you may wish to retry via a different route immediately.
/// In case of APIError::MonitorUpdateFailed, the commitment update has been irrevocably
/// committed on our end and we're just waiting for a monitor update to send it. Do NOT retry
/// the payment via a different route unless you intend to pay twice!
pub fn send_payment(&self, route: Route, payment_hash: [u8; 32]) -> Result<(), APIError> {
if route.hops.len() < 1 || route.hops.len() > 20 {
return Err(APIError::RouteError{err: "Route didn't go anywhere/had bogus size"});
@ -1224,32 +1225,32 @@ impl ChannelManager {
Some(id) => id.clone(),
};
match {
let channel_state = channel_lock.borrow_parts();
if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) {
match {
if chan.get().get_their_node_id() != route.hops.first().unwrap().pubkey {
return Err(APIError::RouteError{err: "Node ID mismatch on first hop!"});
}
if chan.get().is_awaiting_monitor_update() {
return Err(APIError::MonitorUpdateFailed);
}
if !chan.get().is_live() {
return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected!"});
return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!"});
}
break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
route: route.clone(),
session_priv: session_priv.clone(),
first_hop_htlc_msat: htlc_msat,
}, onion_packet), channel_state, chan)
} else { unreachable!(); }
} {
Some((update_add, commitment_signed, chan_monitor)) => {
if let Err(e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
self.handle_monitor_update_fail(channel_lock, &id, e, RAACommitmentOrder::CommitmentFirst);
maybe_break_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst);
// Note that MonitorUpdateFailed here indicates (per function docs)
// that we will resent the commitment update once we unfree monitor
// updating, so we have to take special care that we don't return
// something else in case we will resend later!
return Err(APIError::MonitorUpdateFailed);
}
channel_lock.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
node_id: route.hops.first().unwrap().pubkey,
updates: msgs::CommitmentUpdate {
update_add_htlcs: vec![update_add],
@ -1263,6 +1264,7 @@ impl ChannelManager {
},
None => {},
}
} else { unreachable!(); }
return Ok(());
};
@ -1441,7 +1443,7 @@ impl ChannelManager {
},
};
if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
unimplemented!();// but def dont push the event...
unimplemented!();
}
channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
node_id: forward_chan.get_their_node_id(),
@ -6946,15 +6948,19 @@ mod tests {
let (_, payment_hash_1) = get_payment_preimage_hash!(nodes[0]);
*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
if let Err(APIError::MonitorUpdateFailed) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
check_added_monitors!(nodes[0], 1);
let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events_1.len(), 1);
assert_eq!(events_1.len(), 2);
match events_1[0] {
MessageSendEvent::BroadcastChannelUpdate { .. } => {},
_ => panic!("Unexpected event"),
};
match events_1[1] {
MessageSendEvent::HandleError { node_id, .. } => assert_eq!(node_id, nodes[1].node.get_our_node_id()),
_ => panic!("Unexpected event"),
};
// TODO: Once we hit the chain with the failure transaction we should check that we get a
// PaymentFailed event