Store channel in monitor - simplified approach

lightning/Cargo.toml

@@ -22,6 +22,7 @@ _externalize_tests = ["inventory", "_test_utils"]
 # Allow signing of local transactions that may have been revoked or will be revoked, for functional testing (e.g. justice tx handling).
 # This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
 unsafe_revoked_tx_signing = []
+safe_channels = []
 
 std = []
 

lightning/src/chain/channelmonitor.rs

@@ -111,6 +111,9 @@ pub struct ChannelMonitorUpdate {
 	/// Will be `None` for `ChannelMonitorUpdate`s constructed on LDK versions prior to 0.0.121 and
 	/// always `Some` otherwise.
 	pub channel_id: Option<ChannelId>,
+
+	/// The encoded channel data associated with this ChannelMonitor, if any.
+	pub encoded_channel: Option<Vec<u8>>,
 }
 
 impl ChannelMonitorUpdate {
@@ -156,6 +159,13 @@ impl Writeable for ChannelMonitorUpdate {
 		for update_step in self.updates.iter() {
 			update_step.write(w)?;
 		}
+		#[cfg(feature = "safe_channels")]
+		write_tlv_fields!(w, {
+			// 1 was previously used to store `counterparty_node_id`
+			(3, self.channel_id, option),
+			(5, self.encoded_channel, option)
+		});
+		#[cfg(not(feature = "safe_channels"))]
 		write_tlv_fields!(w, {
 			// 1 was previously used to store `counterparty_node_id`
 			(3, self.channel_id, option),
@@ -176,11 +186,13 @@ impl Readable for ChannelMonitorUpdate {
 			}
 		}
 		let mut channel_id = None;
+		let mut encoded_channel = None;
 		read_tlv_fields!(r, {
 			// 1 was previously used to store `counterparty_node_id`
 			(3, channel_id, option),
+			(5, encoded_channel, option)
 		});
-		Ok(Self { update_id, updates, channel_id })
+		Ok(Self { update_id, updates, channel_id, encoded_channel })
 	}
 }
 
@@ -1402,6 +1414,8 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	/// make deciding whether to do so simple, here we track whether this monitor was last written
 	/// prior to 0.1.
 	written_by_0_1_or_later: bool,
+
+	encoded_channel: Option<Vec<u8>>,
 }
 
 // Returns a `&FundingScope` for the one we are currently observing/handling commitment transactions
@@ -1733,6 +1747,32 @@ pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
 			_ => channel_monitor.pending_monitor_events.clone(),
 		};
 
+	#[cfg(feature = "safe_channels")]
+	write_tlv_fields!(writer, {
+		(1, channel_monitor.funding_spend_confirmed, option),
+		(3, channel_monitor.htlcs_resolved_on_chain, required_vec),
+		(5, pending_monitor_events, required_vec),
+		(7, channel_monitor.funding_spend_seen, required),
+		(9, channel_monitor.counterparty_node_id, required),
+		(11, channel_monitor.confirmed_commitment_tx_counterparty_output, option),
+		(13, channel_monitor.spendable_txids_confirmed, required_vec),
+		(15, channel_monitor.counterparty_fulfilled_htlcs, required),
+		(17, channel_monitor.initial_counterparty_commitment_info, option),
+		(19, channel_monitor.channel_id, required),
+		(21, channel_monitor.balances_empty_height, option),
+		(23, channel_monitor.holder_pays_commitment_tx_fee, option),
+		(25, channel_monitor.payment_preimages, required),
+		(27, channel_monitor.first_negotiated_funding_txo, required),
+		(29, channel_monitor.initial_counterparty_commitment_tx, option),
+		(31, channel_monitor.funding.channel_parameters, required),
+		(32, channel_monitor.pending_funding, optional_vec),
+		(33, channel_monitor.htlcs_resolved_to_user, required),
+		(34, channel_monitor.alternative_funding_confirmed, option),
+		(35, channel_monitor.is_manual_broadcast, required),
+		(37, channel_monitor.funding_seen_onchain, required),
+		(39, channel_monitor.encoded_channel, option),
+	});
+	#[cfg(not(feature = "safe_channels"))]
 	write_tlv_fields!(writer, {
 		(1, channel_monitor.funding_spend_confirmed, option),
 		(3, channel_monitor.htlcs_resolved_on_chain, required_vec),
@@ -1994,6 +2034,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 			alternative_funding_confirmed: None,
 
 			written_by_0_1_or_later: true,
+			encoded_channel: None,
 		})
 	}
 
@@ -2114,6 +2155,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		inner.update_monitor(updates, broadcaster, fee_estimator, &logger)
 	}
 
+	/// Gets the encoded channel data, if any, associated with this ChannelMonitor.
+	pub fn get_encoded_channel(&self) -> Option<Vec<u8>> {
+		self.inner.lock().unwrap().encoded_channel.clone()
+	}
+
+	/// Updates the encoded channel data associated with this ChannelMonitor.
+	pub fn update_encoded_channel(&self, encoded: Vec<u8>) {
+		self.inner.lock().unwrap().encoded_channel = Some(encoded);
+	}
+
 	/// Gets the update_id from the latest ChannelMonitorUpdate which was applied to this
 	/// ChannelMonitor.
 	///
@@ -4405,9 +4456,18 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			}
 		}
 
-		if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
-			log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
-			Err(())
+		if ret.is_ok() {
+			if self.no_further_updates_allowed() && is_pre_close_update {
+				log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
+				Err(())
+			} else {
+				// Assume that if the updates contains no encoded channel, that the channel remained unchanged. We
+				// therefore do not update the monitor.
+				if let Some(encoded_channel) = updates.encoded_channel.as_ref() {
+					self.encoded_channel = Some(encoded_channel.clone());
+				}
+				Ok(())
+			}
 		} else { ret }
 	}
 
@@ -6645,6 +6705,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut alternative_funding_confirmed = None;
 		let mut is_manual_broadcast = RequiredWrapper(None);
 		let mut funding_seen_onchain = RequiredWrapper(None);
+		let mut encoded_channel = None;
 		read_tlv_fields!(reader, {
 			(1, funding_spend_confirmed, option),
 			(3, htlcs_resolved_on_chain, optional_vec),
@@ -6667,6 +6728,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(34, alternative_funding_confirmed, option),
 			(35, is_manual_broadcast, (default_value, false)),
 			(37, funding_seen_onchain, (default_value, true)),
+			(39, encoded_channel, option),
 		});
 		// Note that `payment_preimages_with_info` was added (and is always written) in LDK 0.1, so
 		// we can use it to determine if this monitor was last written by LDK 0.1 or later.
@@ -6844,6 +6906,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			alternative_funding_confirmed,
 
 			written_by_0_1_or_later,
+			encoded_channel,
 		});
 
 		if counterparty_node_id.is_none() {

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2839,6 +2839,8 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
 	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
 	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
+
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn channel_holding_cell_serialize() {
 	do_channel_holding_cell_serialize(true, true);
@@ -3320,6 +3322,7 @@ fn do_test_outbound_reload_without_init_mon(use_0conf: bool) {
 	assert!(nodes[0].node.list_channels().is_empty());
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn test_outbound_reload_without_init_mon() {
 	do_test_outbound_reload_without_init_mon(true);
@@ -3428,6 +3431,7 @@ fn do_test_inbound_reload_without_init_mon(use_0conf: bool, lock_commitment: boo
 	assert!(nodes[1].node.list_channels().is_empty());
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn test_inbound_reload_without_init_mon() {
 	do_test_inbound_reload_without_init_mon(true, true);
@@ -3767,6 +3771,7 @@ fn do_test_inverted_mon_completion_order(
 	expect_payment_sent(&nodes[0], payment_preimage, None, true, true);
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn test_inverted_mon_completion_order() {
 	do_test_inverted_mon_completion_order(true, true);
@@ -3969,6 +3974,7 @@ fn do_test_durable_preimages_on_closed_channel(
 	}
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn test_durable_preimages_on_closed_channel() {
 	do_test_durable_preimages_on_closed_channel(true, true, true);
@@ -4093,6 +4099,7 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) {
 	send_payment(&nodes[1], &[&nodes[2]], 100_000);
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn test_reload_mon_update_completion_actions() {
 	do_test_reload_mon_update_completion_actions(true);
@@ -4459,6 +4466,7 @@ fn do_test_partial_claim_mon_update_compl_actions(reload_a: bool, reload_b: bool
 	assert!(!get_monitor!(nodes[3], chan_4_id).get_stored_preimages().contains_key(&payment_hash));
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[test]
 fn test_partial_claim_mon_update_compl_actions() {
 	do_test_partial_claim_mon_update_compl_actions(true, true);

lightning/src/ln/channel.rs

@@ -2153,7 +2153,7 @@ where
 					// Not having a signing session implies they've already sent `splice_locked`,
 					// which must always come after the initial commitment signed is sent.
 					.unwrap_or(true);
-				let res = if has_negotiated_pending_splice && !session_received_commitment_signed {
+				let res: Result<(Option<ChannelMonitor<<<SP as Deref>::Target as SignerProvider>::EcdsaSigner>>, Option<ChannelMonitorUpdate>), ChannelError> = if has_negotiated_pending_splice && !session_received_commitment_signed {
 					funded_channel
 						.splice_initial_commitment_signed(msg, fee_estimator, logger)
 						.map(|monitor_update_opt| (None, monitor_update_opt))
@@ -6045,6 +6045,7 @@ where
 						should_broadcast: broadcast,
 					}],
 					channel_id: Some(self.channel_id()),
+					encoded_channel: None,
 				};
 				Some((self.get_counterparty_node_id(), funding_txo, self.channel_id(), update))
 			} else {
@@ -7276,6 +7277,7 @@ where
 				payment_info,
 			}],
 			channel_id: Some(self.context.channel_id()),
+			encoded_channel: None,
 		};
 
 		if !self.context.channel_state.can_generate_new_commitment() {
@@ -7417,6 +7419,7 @@ where
 					Vec::new(),
 					Vec::new(),
 				);
+				monitor_update.encoded_channel = Some(self.encode());
 				UpdateFulfillCommitFetch::NewClaim { monitor_update, htlc_value_msat }
 			},
 			UpdateFulfillFetch::DuplicateClaim {} => UpdateFulfillCommitFetch::DuplicateClaim {},
@@ -7892,14 +7895,15 @@ where
 			&self.context.channel_id(), pending_splice_funding.get_funding_txo().unwrap().txid);
 
 		self.context.latest_monitor_update_id += 1;
-		let monitor_update = ChannelMonitorUpdate {
+		let mut monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
 			updates: vec![ChannelMonitorUpdateStep::RenegotiatedFunding {
 				channel_parameters: pending_splice_funding.channel_transaction_parameters.clone(),
 				holder_commitment_tx,
 				counterparty_commitment_tx,
 			}],
 			channel_id: Some(self.context.channel_id()),
+			encoded_channel: None,
 		};
 
 		self.context
@@ -7909,6 +7913,7 @@ where
 			.received_commitment_signed();
 		self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
 
+		monitor_update.encoded_channel = Some(self.encode());
 		Ok(self.push_ret_blockable_mon_update(monitor_update))
 	}
 
@@ -8165,6 +8170,7 @@ where
 			update_id: self.context.latest_monitor_update_id,
 			updates: vec![update],
 			channel_id: Some(self.context.channel_id()),
+			encoded_channel: None,
 		};
 
 		self.context.expecting_peer_commitment_signed = false;
@@ -8217,6 +8223,7 @@ where
 			Vec::new(),
 			Vec::new(),
 		);
+		monitor_update.encoded_channel = Some(self.encode());
 		return Ok(self.push_ret_blockable_mon_update(monitor_update));
 	}
 
@@ -8270,6 +8277,7 @@ where
 				update_id: self.context.latest_monitor_update_id + 1, // We don't increment this yet!
 				updates: Vec::new(),
 				channel_id: Some(self.context.channel_id()),
+				encoded_channel: None,
 			};
 
 			let mut htlc_updates = Vec::new();
@@ -8346,7 +8354,7 @@ where
 						// `ChannelMonitorUpdate` to the user, making this one redundant, however
 						// there's no harm in including the extra `ChannelMonitorUpdateStep` here.
 						// We do not bother to track and include `payment_info` here, however.
-						let fulfill = self.get_update_fulfill_htlc(
+						let fulfill: UpdateFulfillFetch = self.get_update_fulfill_htlc(
 							htlc_id,
 							*payment_preimage,
 							None,
@@ -8360,6 +8368,8 @@ where
 								unreachable!()
 							};
 						update_fulfill_count += 1;
+
+						additional_monitor_update.encoded_channel = Some(self.encode());
 						monitor_update.updates.append(&mut additional_monitor_update.updates);
 						None
 					},
@@ -8418,6 +8428,8 @@ where
 				update_add_count, update_fulfill_count, update_fail_count);
 
 			self.monitor_updating_paused(false, true, false, Vec::new(), Vec::new(), Vec::new());
+
+			monitor_update.encoded_channel = Some(self.encode());
 			(self.push_ret_blockable_mon_update(monitor_update), htlcs_to_fail)
 		} else {
 			(None, Vec::new())
@@ -8534,6 +8546,7 @@ where
 				secret: msg.per_commitment_secret,
 			}],
 			channel_id: Some(self.context.channel_id()),
+			encoded_channel: None,
 		};
 
 		// Update state now that we've passed all the can-fail calls...
@@ -8759,6 +8772,7 @@ where
 		};
 		macro_rules! return_with_htlcs_to_fail {
 			($htlcs_to_fail: expr) => {
+				monitor_update.encoded_channel = Some(self.encode());
 				if !release_monitor {
 					self.context
 						.blocked_monitor_updates
@@ -10384,6 +10398,7 @@ where
 					scriptpubkey: self.get_closing_scriptpubkey(),
 				}],
 				channel_id: Some(self.context.channel_id()),
+				encoded_channel: Some(self.encode()),
 			};
 			self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
 			self.push_ret_blockable_mon_update(monitor_update)
@@ -11153,6 +11168,7 @@ where
 				funding_txid: funding_txo.txid,
 			}],
 			channel_id: Some(self.context.channel_id()),
+			encoded_channel: Some(self.encode()),
 		};
 		self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
 		let monitor_update = self.push_ret_blockable_mon_update(monitor_update);
@@ -12712,6 +12728,7 @@ where
 			update_id: self.context.latest_monitor_update_id,
 			updates: vec![update],
 			channel_id: Some(self.context.channel_id()),
+			encoded_channel: Some(self.encode()),
 		};
 		self.context.channel_state.set_awaiting_remote_revoke();
 		monitor_update
@@ -12958,6 +12975,7 @@ where
 					scriptpubkey: self.get_closing_scriptpubkey(),
 				}],
 				channel_id: Some(self.context.channel_id()),
+				encoded_channel: Some(self.encode()),
 			};
 			self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
 			self.push_ret_blockable_mon_update(monitor_update)

lightning/src/ln/channel_open_tests.rs

@@ -2095,6 +2095,7 @@ pub fn test_batch_channel_open() {
 	)));
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[xtest(feature = "_externalize_tests")]
 pub fn test_close_in_funding_batch() {
 	// This test ensures that if one of the channels
@@ -2183,6 +2184,7 @@ pub fn test_close_in_funding_batch() {
 	assert!(nodes[0].node.list_channels().is_empty());
 }
 
+#[cfg(not(feature = "safe_channels"))]
 #[xtest(feature = "_externalize_tests")]
 pub fn test_batch_funding_close_after_funding_signed() {
 	let chanmon_cfgs = create_chanmon_cfgs(3);