Skip to content

Commit 10df89d

Browse files
committed
Stop using RAA-unblocking post event action chan funding outpoints
Historically we indexed channels by `(counterparty_node_id, funding outpoint)` in several pipelines, especially the `ChannelMonitorUpdate` pipeline. This ended up complexifying quite a few things as we always needed to store the full `(counterparty_node_id, funding outpoint, channel_id)` tuple to ensure we can always access a channel no matter its state. Over time we want to move to only the `(counterparty_node_id, channel_id)` tuple as *the* channel index, especially as we move towards V2 channels that have a globally-unique `channel_id` anyway. Here we take one small step towards this, avoiding using the channel funding outpoint in the `EventCompletionAction` pipeline.
1 parent 7c735d9 commit 10df89d

File tree

2 files changed

+56
-67
lines changed

2 files changed

+56
-67
lines changed

lightning/src/ln/channel.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6401,7 +6401,7 @@ where
64016401
Ok((closing_transaction, total_fee_satoshis))
64026402
}
64036403

6404-
fn funding_outpoint(&self) -> OutPoint {
6404+
pub fn funding_outpoint(&self) -> OutPoint {
64056405
self.funding.channel_transaction_parameters.funding_outpoint.unwrap()
64066406
}
64076407

lightning/src/ln/channelmanager.rs

Lines changed: 55 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,7 +1202,6 @@ pub(crate) enum MonitorUpdateCompletionAction {
12021202
/// stored for later processing.
12031203
FreeOtherChannelImmediately {
12041204
downstream_counterparty_node_id: PublicKey,
1205-
downstream_funding_outpoint: OutPoint,
12061205
blocking_action: RAAMonitorUpdateBlockingAction,
12071206
downstream_channel_id: ChannelId,
12081207
},
@@ -1217,11 +1216,8 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
12171216
// *immediately*. However, for simplicity we implement read/write here.
12181217
(1, FreeOtherChannelImmediately) => {
12191218
(0, downstream_counterparty_node_id, required),
1220-
(2, downstream_funding_outpoint, required),
12211219
(4, blocking_action, upgradable_required),
1222-
// Note that by the time we get past the required read above, downstream_funding_outpoint will be
1223-
// filled in, so we can safely unwrap it here.
1224-
(5, downstream_channel_id, (default_value, ChannelId::v1_from_funding_outpoint(downstream_funding_outpoint.0.unwrap()))),
1220+
(5, downstream_channel_id, required),
12251221
},
12261222
(2, EmitEventAndFreeOtherChannel) => {
12271223
(0, event, upgradable_required),
@@ -1238,17 +1234,21 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
12381234
pub(crate) enum EventCompletionAction {
12391235
ReleaseRAAChannelMonitorUpdate {
12401236
counterparty_node_id: PublicKey,
1241-
channel_funding_outpoint: OutPoint,
1237+
// Was required until LDK 0.2. Always filled in as `Some`.
1238+
channel_funding_outpoint: Option<OutPoint>,
12421239
channel_id: ChannelId,
12431240
},
12441241
}
12451242
impl_writeable_tlv_based_enum!(EventCompletionAction,
12461243
(0, ReleaseRAAChannelMonitorUpdate) => {
1247-
(0, channel_funding_outpoint, required),
1244+
(0, channel_funding_outpoint, option),
12481245
(2, counterparty_node_id, required),
1249-
// Note that by the time we get past the required read above, channel_funding_outpoint will be
1250-
// filled in, so we can safely unwrap it here.
1251-
(3, channel_id, (default_value, ChannelId::v1_from_funding_outpoint(channel_funding_outpoint.0.unwrap()))),
1246+
(3, channel_id, (default_value, {
1247+
if channel_funding_outpoint.is_none() {
1248+
Err(DecodeError::InvalidValue)?
1249+
}
1250+
ChannelId::v1_from_funding_outpoint(channel_funding_outpoint.unwrap())
1251+
})),
12521252
}
12531253
);
12541254

@@ -1278,8 +1278,8 @@ impl From<&MPPClaimHTLCSource> for HTLCClaimSource {
12781278

12791279
#[derive(Debug)]
12801280
pub(crate) struct PendingMPPClaim {
1281-
channels_without_preimage: Vec<(PublicKey, OutPoint, ChannelId)>,
1282-
channels_with_preimage: Vec<(PublicKey, OutPoint, ChannelId)>,
1281+
channels_without_preimage: Vec<(PublicKey, ChannelId)>,
1282+
channels_with_preimage: Vec<(PublicKey, ChannelId)>,
12831283
}
12841284

12851285
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
@@ -8066,7 +8066,7 @@ where
80668066
let pending_mpp_claim_ptr_opt = if sources.len() > 1 {
80678067
let mut channels_without_preimage = Vec::with_capacity(mpp_parts.len());
80688068
for part in mpp_parts.iter() {
8069-
let chan = (part.counterparty_node_id, part.funding_txo, part.channel_id);
8069+
let chan = (part.counterparty_node_id, part.channel_id);
80708070
if !channels_without_preimage.contains(&chan) {
80718071
channels_without_preimage.push(chan);
80728072
}
@@ -8321,7 +8321,6 @@ where
83218321
chan_id, action);
83228322
if let MonitorUpdateCompletionAction::FreeOtherChannelImmediately {
83238323
downstream_counterparty_node_id: node_id,
8324-
downstream_funding_outpoint: _,
83258324
blocking_action: blocker,
83268325
downstream_channel_id: channel_id,
83278326
} = action
@@ -8494,7 +8493,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
84948493
"We don't support claim_htlc claims during startup - monitors may not be available yet");
84958494
debug_assert_eq!(next_channel_counterparty_node_id, path.hops[0].pubkey);
84968495
let ev_completion_action = EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
8497-
channel_funding_outpoint: next_channel_outpoint,
8496+
channel_funding_outpoint: Some(next_channel_outpoint),
84988497
channel_id: next_channel_id,
84998498
counterparty_node_id: path.hops[0].pubkey,
85008499
};
@@ -8598,7 +8597,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
85988597
if let Some(other_chan) = chan_to_release {
85998598
(Some(MonitorUpdateCompletionAction::FreeOtherChannelImmediately {
86008599
downstream_counterparty_node_id: other_chan.counterparty_node_id,
8601-
downstream_funding_outpoint: other_chan.funding_txo,
86028600
downstream_channel_id: other_chan.channel_id,
86038601
blocking_action: other_chan.blocking_action,
86048602
}), None)
@@ -8672,17 +8670,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
86728670
if *pending_claim == claim_ptr {
86738671
let mut pending_claim_state_lock = pending_claim.0.lock().unwrap();
86748672
let pending_claim_state = &mut *pending_claim_state_lock;
8675-
pending_claim_state.channels_without_preimage.retain(|(cp, op, cid)| {
8673+
pending_claim_state.channels_without_preimage.retain(|(cp, cid)| {
86768674
let this_claim =
86778675
*cp == counterparty_node_id && *cid == chan_id;
86788676
if this_claim {
8679-
pending_claim_state.channels_with_preimage.push((*cp, *op, *cid));
8677+
pending_claim_state.channels_with_preimage.push((*cp, *cid));
86808678
false
86818679
} else { true }
86828680
});
86838681
if pending_claim_state.channels_without_preimage.is_empty() {
8684-
for (cp, op, cid) in pending_claim_state.channels_with_preimage.iter() {
8685-
let freed_chan = (*cp, *op, *cid, blocker.clone());
8682+
for (cp, cid) in pending_claim_state.channels_with_preimage.iter() {
8683+
let freed_chan = (*cp, *cid, blocker.clone());
86868684
freed_channels.push(freed_chan);
86878685
}
86888686
}
@@ -8734,26 +8732,26 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
87348732
self.pending_events.lock().unwrap().push_back((event, None));
87358733
if let Some(unblocked) = downstream_counterparty_and_funding_outpoint {
87368734
self.handle_monitor_update_release(
8737-
unblocked.counterparty_node_id, unblocked.funding_txo,
8738-
unblocked.channel_id, Some(unblocked.blocking_action),
8735+
unblocked.counterparty_node_id,
8736+
unblocked.channel_id,
8737+
Some(unblocked.blocking_action),
87398738
);
87408739
}
87418740
},
87428741
MonitorUpdateCompletionAction::FreeOtherChannelImmediately {
8743-
downstream_counterparty_node_id, downstream_funding_outpoint, downstream_channel_id, blocking_action,
8742+
downstream_counterparty_node_id, downstream_channel_id, blocking_action,
87448743
} => {
87458744
self.handle_monitor_update_release(
87468745
downstream_counterparty_node_id,
8747-
downstream_funding_outpoint,
87488746
downstream_channel_id,
87498747
Some(blocking_action),
87508748
);
87518749
},
87528750
}
87538751
}
87548752

8755-
for (node_id, funding_outpoint, channel_id, blocker) in freed_channels {
8756-
self.handle_monitor_update_release(node_id, funding_outpoint, channel_id, Some(blocker));
8753+
for (node_id, channel_id, blocker) in freed_channels {
8754+
self.handle_monitor_update_release(node_id, channel_id, Some(blocker));
87578755
}
87588756
}
87598757

@@ -10535,16 +10533,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1053510533
#[rustfmt::skip]
1053610534
fn raa_monitor_updates_held(&self,
1053710535
actions_blocking_raa_monitor_updates: &BTreeMap<ChannelId, Vec<RAAMonitorUpdateBlockingAction>>,
10538-
channel_funding_outpoint: OutPoint, channel_id: ChannelId, counterparty_node_id: PublicKey
10536+
channel_id: ChannelId, counterparty_node_id: PublicKey,
1053910537
) -> bool {
1054010538
actions_blocking_raa_monitor_updates
1054110539
.get(&channel_id).map(|v| !v.is_empty()).unwrap_or(false)
1054210540
|| self.pending_events.lock().unwrap().iter().any(|(_, action)| {
10543-
action == &Some(EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
10544-
channel_funding_outpoint,
10545-
channel_id,
10546-
counterparty_node_id,
10547-
})
10541+
if let Some(EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
10542+
channel_funding_outpoint: _,
10543+
channel_id: ev_channel_id,
10544+
counterparty_node_id: ev_counterparty_node_id
10545+
}) = action {
10546+
*ev_channel_id == channel_id && *ev_counterparty_node_id == counterparty_node_id
10547+
} else {
10548+
false
10549+
}
1054810550
})
1054910551
}
1055010552

@@ -10557,14 +10559,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1055710559
let mut peer_state_lck = peer_state_mtx.lock().unwrap();
1055810560
let peer_state = &mut *peer_state_lck;
1055910561

10560-
if let Some(chan) = peer_state.channel_by_id.get(&channel_id) {
10561-
return self.raa_monitor_updates_held(
10562-
&peer_state.actions_blocking_raa_monitor_updates,
10563-
chan.funding().get_funding_txo().unwrap(),
10564-
channel_id,
10565-
counterparty_node_id,
10566-
);
10567-
}
10562+
assert!(peer_state.channel_by_id.contains_key(&channel_id));
10563+
return self.raa_monitor_updates_held(
10564+
&peer_state.actions_blocking_raa_monitor_updates,
10565+
channel_id,
10566+
counterparty_node_id,
10567+
);
1056810568
}
1056910569
false
1057010570
}
@@ -10584,11 +10584,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1058410584
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
1058510585
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
1058610586
let funding_txo_opt = chan.funding.get_funding_txo();
10587-
let mon_update_blocked = if let Some(funding_txo) = funding_txo_opt {
10588-
self.raa_monitor_updates_held(
10589-
&peer_state.actions_blocking_raa_monitor_updates, funding_txo, msg.channel_id,
10590-
*counterparty_node_id)
10591-
} else { false };
10587+
let mon_update_blocked = self.raa_monitor_updates_held(
10588+
&peer_state.actions_blocking_raa_monitor_updates, msg.channel_id,
10589+
*counterparty_node_id);
1059210590
let (htlcs_to_fail, monitor_update_opt) = try_channel_entry!(self, peer_state,
1059310591
chan.revoke_and_ack(&msg, &self.fee_estimator, &&logger, mon_update_blocked), chan_entry);
1059410592
if let Some(monitor_update) = monitor_update_opt {
@@ -12573,10 +12571,10 @@ where
1257312571
/// operation. It will double-check that nothing *else* is also blocking the same channel from
1257412572
/// making progress and then let any blocked [`ChannelMonitorUpdate`]s fly.
1257512573
#[rustfmt::skip]
12576-
fn handle_monitor_update_release(&self, counterparty_node_id: PublicKey,
12577-
channel_funding_outpoint: OutPoint, channel_id: ChannelId,
12578-
mut completed_blocker: Option<RAAMonitorUpdateBlockingAction>) {
12579-
12574+
fn handle_monitor_update_release(
12575+
&self, counterparty_node_id: PublicKey, channel_id: ChannelId,
12576+
mut completed_blocker: Option<RAAMonitorUpdateBlockingAction>,
12577+
) {
1258012578
let logger = WithContext::from(
1258112579
&self.logger, Some(counterparty_node_id), Some(channel_id), None
1258212580
);
@@ -12595,7 +12593,7 @@ where
1259512593
}
1259612594

1259712595
if self.raa_monitor_updates_held(&peer_state.actions_blocking_raa_monitor_updates,
12598-
channel_funding_outpoint, channel_id, counterparty_node_id) {
12596+
channel_id, counterparty_node_id) {
1259912597
// Check that, while holding the peer lock, we don't have anything else
1260012598
// blocking monitor updates for this channel. If we do, release the monitor
1260112599
// update(s) when those blockers complete.
@@ -12607,7 +12605,7 @@ where
1260712605
if let hash_map::Entry::Occupied(mut chan_entry) = peer_state.channel_by_id.entry(
1260812606
channel_id) {
1260912607
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
12610-
debug_assert_eq!(chan.funding.get_funding_txo().unwrap(), channel_funding_outpoint);
12608+
let channel_funding_outpoint = chan.funding_outpoint();
1261112609
if let Some((monitor_update, further_update_exists)) = chan.unblock_next_blocked_monitor_update() {
1261212610
log_debug!(logger, "Unlocking monitor updating for channel {} and updating monitor",
1261312611
channel_id);
@@ -12637,16 +12635,11 @@ where
1263712635
for action in actions {
1263812636
match action {
1263912637
EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
12640-
channel_funding_outpoint,
12638+
channel_funding_outpoint: _,
1264112639
channel_id,
1264212640
counterparty_node_id,
1264312641
} => {
12644-
self.handle_monitor_update_release(
12645-
counterparty_node_id,
12646-
channel_funding_outpoint,
12647-
channel_id,
12648-
None,
12649-
);
12642+
self.handle_monitor_update_release(counterparty_node_id, channel_id, None);
1265012643
},
1265112644
}
1265212645
}
@@ -16420,7 +16413,9 @@ where
1642016413
// `ChannelMonitor` is removed.
1642116414
let compl_action =
1642216415
EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
16423-
channel_funding_outpoint: monitor.get_funding_txo(),
16416+
channel_funding_outpoint: Some(
16417+
monitor.get_funding_txo(),
16418+
),
1642416419
channel_id: monitor.channel_id(),
1642516420
counterparty_node_id: path.hops[0].pubkey,
1642616421
};
@@ -16923,13 +16918,7 @@ where
1692316918
let mut channels_without_preimage = payment_claim
1692416919
.mpp_parts
1692516920
.iter()
16926-
.map(|htlc_info| {
16927-
(
16928-
htlc_info.counterparty_node_id,
16929-
htlc_info.funding_txo,
16930-
htlc_info.channel_id,
16931-
)
16932-
})
16921+
.map(|htlc_info| (htlc_info.counterparty_node_id, htlc_info.channel_id))
1693316922
.collect::<Vec<_>>();
1693416923
// If we have multiple MPP parts which were received over the same channel,
1693516924
// we only track it once as once we get a preimage durably in the

0 commit comments

Comments
 (0)