Skip to content

Commit 57bebe7

Browse files
committed
Store + process pending ChannelMonitorUpdates in Channel
The previous commits set up the ability for us to hold `ChannelMonitorUpdate`s which are pending until we're ready to pass them to users and have them be applied. However, if the `ChannelManager` is persisted while we're waiting to give the user a `ChannelMonitorUpdate` we'll be confused on restart - seeing our latest `ChannelMonitor` state as stale compared to our `ChannelManager` - a critical error. Luckily the solution is trivial, we simply need to store the pending `ChannelMonitorUpdate` state and load it with the `ChannelManager` data, allowing stale monitors on load as long as we have the missing pending updates between where we are and the latest `ChannelMonitor` state.
1 parent ede80d8 commit 57bebe7

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

lightning/src/ln/channel.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,11 @@ struct PendingChannelMonitorUpdate {
490490
flown: bool,
491491
}
492492

493+
impl_writeable_tlv_based!(PendingChannelMonitorUpdate, {
494+
(0, update, required),
495+
(2, flown, required),
496+
});
497+
493498
// TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
494499
// has been completed, and then turn into a Channel to get compiler-time enforcement of things like
495500
// calling channel_id() before we're set up or things like get_outbound_funding_signed on an
@@ -4952,6 +4957,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
49524957
(self.channel_state & ChannelState::MonitorUpdateInProgress as u32) != 0
49534958
}
49544959

4960+
pub fn get_latest_complete_monitor_update_id(&self) -> u64 {
4961+
if self.pending_monitor_updates.is_empty() { return self.get_latest_monitor_update_id(); }
4962+
self.pending_monitor_updates[0].update.update_id - 1
4963+
}
4964+
49554965
/// Returns the next unflown monitor update, if one exists, and a bool which indicates a
49564966
/// further unflown monitor update exists after the next.
49574967
pub fn fly_next_unflown_monitor_update(&mut self) -> Option<(&ChannelMonitorUpdate, bool)> {
@@ -6539,6 +6549,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> {
65396549
(23, channel_ready_event_emitted, option),
65406550
(25, user_id_high_opt, option),
65416551
(27, self.channel_keys_id, required),
6552+
(29, self.pending_monitor_updates, vec_type),
65426553
});
65436554

65446555
Ok(())
@@ -6811,6 +6822,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
68116822
let mut user_id_high_opt: Option<u64> = None;
68126823
let mut channel_keys_id: Option<[u8; 32]> = None;
68136824

6825+
let mut pending_monitor_updates = Some(Vec::new());
6826+
68146827
read_tlv_fields!(reader, {
68156828
(0, announcement_sigs, option),
68166829
(1, minimum_depth, option),
@@ -6830,6 +6843,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
68306843
(23, channel_ready_event_emitted, option),
68316844
(25, user_id_high_opt, option),
68326845
(27, channel_keys_id, option),
6846+
(29, pending_monitor_updates, vec_type),
68336847
});
68346848

68356849
let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id {
@@ -6994,7 +7008,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
69947008
channel_type: channel_type.unwrap(),
69957009
channel_keys_id,
69967010

6997-
pending_monitor_updates: Vec::new(),
7011+
pending_monitor_updates: pending_monitor_updates.unwrap(),
69987012
})
69997013
}
70007014
}

lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7506,14 +7506,11 @@ where
75067506
let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?;
75077507
funding_txo_set.insert(funding_txo.clone());
75087508
if let Some(ref mut monitor) = args.channel_monitors.get_mut(&funding_txo) {
7509-
if channel.get_cur_holder_commitment_transaction_number() < monitor.get_cur_holder_commitment_number() ||
7510-
channel.get_revoked_counterparty_commitment_transaction_number() < monitor.get_min_seen_secret() ||
7511-
channel.get_cur_counterparty_commitment_transaction_number() < monitor.get_cur_counterparty_commitment_number() ||
7512-
channel.get_latest_monitor_update_id() > monitor.get_latest_update_id() {
7509+
if channel.get_latest_complete_monitor_update_id() > monitor.get_latest_update_id() {
75137510
// If the channel is ahead of the monitor, return InvalidValue:
75147511
log_error!(args.logger, "A ChannelMonitor is stale compared to the current ChannelManager! This indicates a potentially-critical violation of the chain::Watch API!");
75157512
log_error!(args.logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.",
7516-
log_bytes!(channel.channel_id()), monitor.get_latest_update_id(), channel.get_latest_monitor_update_id());
7513+
log_bytes!(channel.channel_id()), monitor.get_latest_update_id(), channel.get_latest_complete_monitor_update_id());
75177514
log_error!(args.logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,");
75187515
log_error!(args.logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!");
75197516
log_error!(args.logger, " Without the latest ChannelMonitor we cannot continue without risking funds.");

0 commit comments

Comments
 (0)