Skip to content

Commit 1dec1f9

Browse files
ViktorTigerstromViktorTigerstrom
committed
f - Move creation of CommitmentUpdate structs
As we now may forward HTLCS over other channels than the channels specified in channel_state.forward_htlcs, we move the creation of CommitmentUpdate outside of the loop för the channel_state.forward_htlcs to ensure that we don't create multiple CommitmentUpdate structs for the same channels.
1 parent 1d44945 commit 1dec1f9

File tree

1 file changed

+79
-75
lines changed

1 file changed

+79
-75
lines changed

‎lightning/src/ln/channelmanager.rs

Lines changed: 79 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -3005,6 +3005,36 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
30053005
let channel_state = &mut *channel_state_lock;
30063006
let per_peer_state = self.per_peer_state.read().unwrap();
30073007

3008+
let mut htlcs_msgs_by_id: HashMap<[u8; 32], (Vec<msgs::UpdateAddHTLC>, Vec<msgs::UpdateFailHTLC>, PublicKey)> = HashMap::new();
3009+
3010+
macro_rules! add_channel_key {
3011+
($channel_id: expr, $counterparty_node_id: expr) => {{
3012+
if !htlcs_msgs_by_id.contains_key(&$channel_id){
3013+
htlcs_msgs_by_id.insert($channel_id, (Vec::new(), Vec::new(), $counterparty_node_id));
3014+
}
3015+
}}
3016+
}
3017+
3018+
macro_rules! add_update_add_htlc {
3019+
($add_htlc_msg: expr, $channel_id: expr, $counterparty_node_id: expr) => {{
3020+
add_channel_key!($channel_id, $counterparty_node_id);
3021+
if let hash_map::Entry::Occupied(mut entry) = htlcs_msgs_by_id.entry($channel_id) {
3022+
let msgs_entry = entry.get_mut();
3023+
msgs_entry.0.push($add_htlc_msg);
3024+
}
3025+
}}
3026+
}
3027+
3028+
macro_rules! add_update_fail_htlc {
3029+
($fail_htlc_msg: expr, $channel_id: expr, $counterparty_node_id: expr) => {{
3030+
add_channel_key!($channel_id, $counterparty_node_id);
3031+
if let hash_map::Entry::Occupied(mut entry) = htlcs_msgs_by_id.entry($channel_id) {
3032+
let msgs_entry = entry.get_mut();
3033+
msgs_entry.1.push($fail_htlc_msg);
3034+
}
3035+
}}
3036+
}
3037+
30083038
for (short_chan_id, mut pending_forwards) in channel_state.forward_htlcs.drain() {
30093039
if short_chan_id != 0 {
30103040
let (counterparty_node_id, forward_chan_id) = match channel_state.short_to_chan_info.get(&short_chan_id) {
@@ -3082,35 +3112,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
30823112
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
30833113
let peer_state = &mut *peer_state_lock;
30843114
if peer_state.channel_by_id.contains_key(&forward_chan_id) {
3085-
let mut htlcs_msgs_by_id: HashMap<[u8; 32], (Vec<msgs::UpdateAddHTLC>, Vec<msgs::UpdateFailHTLC>)> = HashMap::new();
3086-
3087-
macro_rules! add_channel_key {
3088-
($channel_id: expr) => {{
3089-
if !htlcs_msgs_by_id.contains_key(&$channel_id){
3090-
htlcs_msgs_by_id.insert($channel_id, (Vec::new(), Vec::new()));
3091-
}
3092-
}}
3093-
}
3094-
3095-
macro_rules! add_update_add_htlc {
3096-
($add_htlc_msg: expr, $channel_id: expr) => {{
3097-
add_channel_key!($channel_id);
3098-
if let hash_map::Entry::Occupied(mut entry) = htlcs_msgs_by_id.entry($channel_id) {
3099-
let msgs_entry = entry.get_mut();
3100-
msgs_entry.0.push($add_htlc_msg);
3101-
}
3102-
}}
3103-
}
3104-
3105-
macro_rules! add_update_fail_htlc {
3106-
($fail_htlc_msg: expr, $channel_id: expr) => {{
3107-
add_channel_key!($channel_id);
3108-
if let hash_map::Entry::Occupied(mut entry) = htlcs_msgs_by_id.entry($channel_id) {
3109-
let msgs_entry = entry.get_mut();
3110-
msgs_entry.1.push($fail_htlc_msg);
3111-
}
3112-
}}
3113-
}
31143115
for forward_info in pending_forwards.drain(..) {
31153116
match forward_info {
31163117
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
@@ -3153,7 +3154,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31533154
match update_add {
31543155
Some(msg) => {
31553156
log_info!(self.logger, "Will forward HTLC with payment_hash {}, over channel {}", log_bytes!(payment_hash.0), log_bytes!(chan_id));
3156-
add_update_add_htlc!(msg, chan_id);
3157+
add_update_add_htlc!(msg, chan_id, counterparty_node_id);
31573158
},
31583159
None => {
31593160
// Nothing to do here...we're waiting on a remote
@@ -3196,7 +3197,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31963197
// the chain and sending the HTLC-Timeout is their problem.
31973198
continue;
31983199
},
3199-
Ok(Some(msg)) => { add_update_fail_htlc!(msg, forward_chan_id); },
3200+
Ok(Some(msg)) => { add_update_fail_htlc!(msg, forward_chan_id, counterparty_node_id); },
32003201
Ok(None) => {
32013202
// Nothing to do here...we're waiting on a remote
32023203
// revoke_and_ack before we can update the commitment
@@ -3211,50 +3212,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
32113212
},
32123213
}
32133214
}
3214-
3215-
for (chan_id, (add_htlc_msgs, fail_htlc_msgs)) in htlcs_msgs_by_id.into_iter() {
3216-
if let hash_map::Entry::Occupied(mut chan) = peer_state.channel_by_id.entry(chan_id) {
3217-
let (commitment_msg, monitor_update) = match chan.get_mut().send_commitment(&self.logger) {
3218-
Ok(res) => res,
3219-
Err(e) => {
3220-
// We surely failed send_commitment due to bad keys, in that case
3221-
// close channel and then send error message to peer.
3222-
let counterparty_node_id = chan.get().get_counterparty_node_id();
3223-
let err: Result<(), _> = match e {
3224-
ChannelError::Ignore(_) | ChannelError::Warn(_) => {
3225-
panic!("Stated return value requirements in send_commitment() were not met");
3226-
}
3227-
ChannelError::Close(msg) => {
3228-
log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
3229-
let mut channel = remove_channel!(self, channel_state, chan);
3230-
// ChannelClosed event is generated by handle_error for us.
3231-
Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
3232-
},
3233-
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"); }
3234-
};
3235-
handle_errors.push((counterparty_node_id, err));
3236-
continue;
3237-
}
3238-
};
3239-
if let Err(e) = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
3240-
handle_errors.push((chan.get().get_counterparty_node_id(), handle_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, true)));
3241-
continue;
3242-
}
3243-
log_debug!(self.logger, "Forwarding HTLCs resulted in a commitment update with {} HTLCs added and {} HTLCs failed for channel {}",
3244-
add_htlc_msgs.len(), fail_htlc_msgs.len(), log_bytes!(chan.get().channel_id()));
3245-
channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
3246-
node_id: chan.get().get_counterparty_node_id(),
3247-
updates: msgs::CommitmentUpdate {
3248-
update_add_htlcs: add_htlc_msgs,
3249-
update_fulfill_htlcs: Vec::new(),
3250-
update_fail_htlcs: fail_htlc_msgs,
3251-
update_fail_malformed_htlcs: Vec::new(),
3252-
update_fee: None,
3253-
commitment_signed: commitment_msg,
3254-
},
3255-
});
3256-
}
3257-
}
32583215
} else {
32593216
let err = Err(MsgHandleErrInternal::send_err_msg_no_close(format!("No such channel for the counterparty_node_id {}, as indicated by the short_to_id map", counterparty_node_id), forward_chan_id));
32603217
handle_errors.push((counterparty_node_id, err));
@@ -3438,6 +3395,53 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
34383395
}
34393396
}
34403397
}
3398+
for (chan_id, (add_htlc_msgs, fail_htlc_msgs, counterparty_node_id)) in htlcs_msgs_by_id.into_iter() {
3399+
if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
3400+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
3401+
let peer_state = &mut *peer_state_lock;
3402+
if let hash_map::Entry::Occupied(mut chan) = peer_state.channel_by_id.entry(chan_id) {
3403+
let (commitment_msg, monitor_update) = match chan.get_mut().send_commitment(&self.logger) {
3404+
Ok(res) => res,
3405+
Err(e) => {
3406+
// We surely failed send_commitment due to bad keys, in that case
3407+
// close channel and then send error message to peer.
3408+
let counterparty_node_id = chan.get().get_counterparty_node_id();
3409+
let err: Result<(), _> = match e {
3410+
ChannelError::Ignore(_) | ChannelError::Warn(_) => {
3411+
panic!("Stated return value requirements in send_commitment() were not met");
3412+
}
3413+
ChannelError::Close(msg) => {
3414+
log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
3415+
let mut channel = remove_channel!(self, channel_state, chan);
3416+
// ChannelClosed event is generated by handle_error for us.
3417+
Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
3418+
},
3419+
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"); }
3420+
};
3421+
handle_errors.push((counterparty_node_id, err));
3422+
continue;
3423+
}
3424+
};
3425+
if let Err(e) = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
3426+
handle_errors.push((chan.get().get_counterparty_node_id(), handle_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, true)));
3427+
continue;
3428+
}
3429+
log_debug!(self.logger, "Forwarding HTLCs resulted in a commitment update with {} HTLCs added and {} HTLCs failed for channel {}",
3430+
add_htlc_msgs.len(), fail_htlc_msgs.len(), log_bytes!(chan.get().channel_id()));
3431+
channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
3432+
node_id: chan.get().get_counterparty_node_id(),
3433+
updates: msgs::CommitmentUpdate {
3434+
update_add_htlcs: add_htlc_msgs,
3435+
update_fulfill_htlcs: Vec::new(),
3436+
update_fail_htlcs: fail_htlc_msgs,
3437+
update_fail_malformed_htlcs: Vec::new(),
3438+
update_fee: None,
3439+
commitment_signed: commitment_msg,
3440+
},
3441+
});
3442+
}
3443+
}
3444+
}
34413445
}
34423446

34433447
for (htlc_source, payment_hash, failure_reason) in failed_forwards.drain(..) {

0 commit comments

Comments
 (0)