From: Matt Corallo <git@bluematt.me>
Date: Sun, 19 Apr 2020 18:15:56 +0000 (-0400)
Subject: Track signing of local txn in channelmonitor and refuse updates
X-Git-Tag: v0.0.12~72^2~5
X-Git-Url: http://git.bitcoin.ninja/index.cgi?p=rust-lightning;a=commitdiff_plain;h=ddd85fb55023ac7fea1c1a7ae4748b2b795dff61

Track signing of local txn in channelmonitor and refuse updates

In e46e183084ed858f41aa304acd78503aea1a96ed we began tracking
whether a local commitment transaction had been signed and
broadcast in OnchainTxHandler, refusing to update the local
commitment transaction state in the ChannelMonitor on that basis.

This is fine, except that it doesn't make a lot of sense to store
the full local transaction state in OnchainTxHandler - we should be
providing it the unsigned local transaction at the time we wish to
broadcast and no more (just like we do all other transaction data).
---

diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs
index da8ba03a..5241d97d 100644
--- a/lightning/src/ln/channelmonitor.rs
+++ b/lightning/src/ln/channelmonitor.rs
@@ -784,9 +784,16 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
 	#[cfg(not(test))]
 	onchain_tx_handler: OnchainTxHandler<ChanSigner>,
 
-	// Used to detect programming bug due to unsafe monitor update sequence { ChannelForceClosed, LatestLocalCommitmentTXInfo }
+	// This is set when the Channel[Manager] generated a ChannelMonitorUpdate which indicated the
+	// channel has been force-closed. After this is set, no further local commitment transaction
+	// updates may occur, and we panic!() if one is provided.
 	lockdown_from_offchain: bool,
 
+	// Set once we've signed a local commitment transaction and handed it over to our
+	// OnchainTxHandler. After this is set, no future updates to our local commitment transactions
+	// may occur, and we fail any such monitor updates.
+	local_tx_signed: bool,
+
 	// We simply modify last_block_hash in Channel's block_connected so that serialization is
 	// consistent but hopefully the users' copy handles block_connected in a consistent way.
 	// (we do *not*, however, update them in update_monitor to ensure any local user copies keep
@@ -830,7 +837,9 @@ impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
 			self.pending_htlcs_updated != other.pending_htlcs_updated ||
 			self.pending_events.len() != other.pending_events.len() || // We trust events to round-trip properly
 			self.onchain_events_waiting_threshold_conf != other.onchain_events_waiting_threshold_conf ||
-			self.outputs_to_watch != other.outputs_to_watch
+			self.outputs_to_watch != other.outputs_to_watch ||
+			self.lockdown_from_offchain != other.lockdown_from_offchain ||
+			self.local_tx_signed != other.local_tx_signed
 		{
 			false
 		} else {
@@ -1031,6 +1040,7 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 		self.onchain_tx_handler.write(writer)?;
 
 		self.lockdown_from_offchain.write(writer)?;
+		self.local_tx_signed.write(writer)?;
 
 		Ok(())
 	}
@@ -1113,6 +1123,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			onchain_tx_handler,
 
 			lockdown_from_offchain: false,
+			local_tx_signed: false,
 
 			last_block_hash: Default::default(),
 			secp_ctx: Secp256k1::new(),
@@ -1229,6 +1240,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// up-to-date as our local commitment transaction is updated.
 	/// Panics if set_their_to_self_delay has never been called.
 	pub(super) fn provide_latest_local_commitment_tx_info(&mut self, commitment_tx: LocalCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+		if self.local_tx_signed {
+			return Err(MonitorUpdateError("A local commitment tx has already been signed, no new local commitment txn can be sent to our counterparty"));
+		}
 		let txid = commitment_tx.txid();
 		let sequence = commitment_tx.without_valid_witness().input[0].sequence as u64;
 		let locktime = commitment_tx.without_valid_witness().lock_time as u64;
@@ -1756,6 +1770,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// In any-case, choice is up to the user.
 	pub fn get_latest_local_commitment_txn(&mut self) -> Vec<Transaction> {
 		log_trace!(self, "Getting signed latest local commitment transaction!");
+		self.local_tx_signed = true;
 		if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx() {
 			let txid = commitment_tx.txid();
 			let mut res = vec![commitment_tx];
@@ -2415,6 +2430,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
 		let onchain_tx_handler = ReadableArgs::read(reader, logger.clone())?;
 
 		let lockdown_from_offchain = Readable::read(reader)?;
+		let local_tx_signed = Readable::read(reader)?;
 
 		Ok((last_block_hash.clone(), ChannelMonitor {
 			latest_update_id,
@@ -2459,6 +2475,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
 			onchain_tx_handler,
 
 			lockdown_from_offchain,
+			local_tx_signed,
 
 			last_block_hash,
 			secp_ctx: Secp256k1::new(),