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

Fix a minor memory leak on PermanentFailure mon errs when sending

Browse source 
If we send a payment and fail to update the first-hop channel state
with a `PermanentFailure` ChannelMonitorUpdateErr, we would have an
entry in our pending payments map, but possibly not return the
PaymentId back to the user to retry the payment, leading to a (rare
and relatively minor) memory leak.
This commit is contained in:
Matt Corallo 2021-10-26 21:40:14 +00:00
parent 6f002ea93a
commit 0ec13f611b
1 changed files with 29 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
lightning/src/ln/channelmanager.rs
View file

@ -2075,26 +2075,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
Some(id) => id.clone(),
};
let channel_state = &mut *channel_lock;
if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) {
match {
if chan.get().get_counterparty_node_id() != path.first().unwrap().pubkey {
return Err(APIError::RouteError{err: "Node ID mismatch on first hop!"});
}
if !chan.get().is_live() {
return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()});
}
let send_res = break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(
htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
path: path.clone(),
session_priv: session_priv.clone(),
first_hop_htlc_msat: htlc_msat,
payment_id,
payment_secret: payment_secret.clone(),
payee: payee.clone(),
}, onion_packet, &self.logger),
channel_state, chan);
macro_rules! insert_outbound_payment {
() => {
let payment = payment_entry.or_insert_with(|| PendingOutboundPayment::Retryable {
session_privs: HashSet::new(),
pending_amt_msat: 0,
@ -2105,8 +2087,28 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
total_msat: total_value,
});
assert!(payment.insert(session_priv_bytes, path));
}
}
send_res
let channel_state = &mut *channel_lock;
if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) {
match {
if chan.get().get_counterparty_node_id() != path.first().unwrap().pubkey {
return Err(APIError::RouteError{err: "Node ID mismatch on first hop!"});
}
if !chan.get().is_live() {
return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()});
}
break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(
htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
path: path.clone(),
session_priv: session_priv.clone(),
first_hop_htlc_msat: htlc_msat,
payment_id,
payment_secret: payment_secret.clone(),
payee: payee.clone(),
}, onion_packet, &self.logger),
channel_state, chan)
} {
Some((update_add, commitment_signed, monitor_update)) => {
if let Err(e) = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
@ -2116,8 +2118,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
// is restored. Therefore, we must return an error indicating that
// it is unsafe to retry the payment wholesale, which we do in the
// send_payment check for MonitorUpdateFailed, below.
insert_outbound_payment!(); // Only do this after possibly break'ing on Perm failure above.
return Err(APIError::MonitorUpdateFailed);
}
insert_outbound_payment!();
log_debug!(self.logger, "Sending payment along path resulted in a commitment_signed for channel {}", log_bytes!(chan.get().channel_id()));
channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
@ -2132,7 +2136,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
},
});
},
None => {},
None => { insert_outbound_payment!(); },
}
} else { unreachable!(); }
return Ok(());
@ -2249,6 +2253,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
if has_err && has_ok {
Err(PaymentSendFailure::PartialFailure(results))
} else if has_err {
// If we failed to send any paths, we shouldn't have inserted the new PaymentId into
// our `pending_outbound_payments` map at all.
debug_assert!(self.pending_outbound_payments.lock().unwrap().get(&payment_id).is_none());
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
} else {
Ok(payment_id)