From ddd85fb55023ac7fea1c1a7ae4748b2b795dff61 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 19 Apr 2020 14:15:56 -0400 Subject: [PATCH] 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). --- lightning/src/ln/channelmonitor.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index da8ba03a7..5241d97d4 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -784,9 +784,16 @@ pub struct ChannelMonitor { #[cfg(not(test))] onchain_tx_handler: OnchainTxHandler, - // 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 PartialEq for ChannelMonitor { 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 ChannelMonitor { self.onchain_tx_handler.write(writer)?; self.lockdown_from_offchain.write(writer)?; + self.local_tx_signed.write(writer)?; Ok(()) } @@ -1113,6 +1123,7 @@ impl ChannelMonitor { 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 ChannelMonitor { /// 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, Option)>) -> 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 ChannelMonitor { /// In any-case, choice is up to the user. pub fn get_latest_local_commitment_txn(&mut self) -> Vec { 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 ReadableArgs> 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 ReadableArgs> for (Sha256dH onchain_tx_handler, lockdown_from_offchain, + local_tx_signed, last_block_hash, secp_ctx: Secp256k1::new(), -- 2.39.5