Correctly handle new ChannelMonitorUpdates to old post-FC chans

In 0.1 (1481216) we started
setting `ChannelMonitorUpdate::update_id` to a non-`u64::MAX` value
for updates generated after a channel has been closed.

This is great, but in 71a364c we
then started calculating the next `update_id` by incrementing the
last `update_id` we saw when we started and were looking at the
`ChannelMonitor`. However, the last-applied `update_id` may well be
`u64::MAX` for old `ChannelMonitor`s which were closed prior to
0.1. In that case the increment would overflow.

Here we fix this naively by simply replacing the increment with a
`saturating_add`. While its possible this will result in a
`ChannelMonitorUpdate` being tracked as in-flight (only for the
`ReleasePaymentComplete` updates added in 71a364c)
at the same `update_id` as other updates already in-flight and
handling post-`ChannelMonitorUpdate` actions too early, this should
only apply to releasing payment complete updates, which have no
post-`ChannelMonitorUpdate` action.

Its also possible that this leads to a regression in the future,
where we have some new post-closure update that does have a
post-`ChannelMonitorUpdate` action and we run it too early, but by
then presumably its fairly rare to have a `ChannelMonitor` for a
channel closed pre-0.1 that still needs multiple updates.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -1601,6 +1601,8 @@ where
 	/// the highest `update_id` of all the pending in-flight updates (note that any pending updates
 	/// not yet applied sitting in [`ChannelManager::pending_background_events`] will also be
 	/// considered as they are also in [`Self::in_flight_monitor_updates`]).
+	///
+	/// Note that channels which were closed prior to LDK 0.1 may have a value here of `u64::MAX`.
 	closed_channel_monitor_update_ids: BTreeMap<ChannelId, u64>,
 	/// The peer is currently connected (i.e. we've seen a
 	/// [`BaseMessageHandler::peer_connected`] and no corresponding
@@ -13262,7 +13264,8 @@ where
 						.closed_channel_monitor_update_ids
 						.get_mut(&channel_id)
 						.expect("Channels originating a payment resolution must have a monitor");
-					*update_id += 1;
+					// Note that for channels closed pre-0.1, the latest update_id is `u64::MAX`.
+					*update_id = update_id.saturating_add(1);
 
 					let update = ChannelMonitorUpdate {
 						update_id: *update_id,
@@ -16523,7 +16526,9 @@ where
 					should_queue_fc_update = !monitor.no_further_updates_allowed();
 					let mut latest_update_id = monitor.get_latest_update_id();
 					if should_queue_fc_update {
-						latest_update_id += 1;
+						// Note that for channels closed pre-0.1, the latest update_id is
+						// `u64::MAX`.
+						latest_update_id = latest_update_id.saturating_add(1);
 					}
 					per_peer_state
 						.entry(counterparty_node_id)
@@ -17170,7 +17175,9 @@ where
 											.closed_channel_monitor_update_ids
 											.get_mut(channel_id)
 											.expect("Channels originating a preimage must have a monitor");
-										*update_id += 1;
+										// Note that for channels closed pre-0.1, the latest
+										// update_id is `u64::MAX`.
+										*update_id = update_id.saturating_add(1);
 
 										pending_background_events.push(BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
 											counterparty_node_id: monitor.get_counterparty_node_id(),