From 034c377e24ac6c00a3834b03fa4d5d1f2401a93e Mon Sep 17 00:00:00 2001 From: FreeOnlineUser Date: Sun, 1 Mar 2026 16:57:27 +1000 Subject: [PATCH] Add ChannelMonitor justice tx API for simplified watchtower integration Adds sign_initial_justice_tx() and sign_justice_txs_from_update() to ChannelMonitor, allowing Persist implementors to obtain signed justice transactions without maintaining external state. Storage uses cur/prev counterparty commitment fields on FundingScope, matching the existing pattern and supporting splicing. Simplifies WatchtowerPersister in test_utils by removing the manual queue and signing logic. Addresses feedback from lightningdevkit/ldk-node#813 and picks up the intent of #2552. --- lightning/src/chain/channelmonitor.rs | 135 ++++++++++++++++++++++++++ lightning/src/util/test_utils.rs | 103 +++++--------------- 2 files changed, 158 insertions(+), 80 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a8d055a9c5b..b54322139df 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -262,6 +262,17 @@ impl_writeable_tlv_based!(HTLCUpdate, { (4, payment_preimage, option), }); +/// A signed justice transaction ready for broadcast or watchtower submission. +#[derive(Clone, Debug)] +pub struct JusticeTransaction { + /// The fully signed justice transaction. + pub tx: Transaction, + /// The txid of the revoked counterparty commitment transaction. + pub revoked_commitment_txid: Txid, + /// The commitment number of the revoked commitment transaction. + pub commitment_number: u64, +} + /// If an output goes from claimable only by us to claimable by us or our counterparty within this /// many blocks, we consider it pinnable for the purposes of aggregating claims in a single /// transaction. @@ -1166,6 +1177,11 @@ struct FundingScope { // transaction for which we have deleted claim information on some watchtowers. current_holder_commitment_tx: HolderCommitmentTransaction, prev_holder_commitment_tx: Option, + + /// The current counterparty commitment transaction, stored for justice tx signing. + cur_counterparty_commitment_tx: Option, + /// The previous counterparty commitment transaction, stored for justice tx signing. + prev_counterparty_commitment_tx: Option, } impl FundingScope { @@ -1194,6 +1210,8 @@ impl_writeable_tlv_based!(FundingScope, { (7, current_holder_commitment_tx, required), (9, prev_holder_commitment_tx, option), (11, counterparty_claimable_outpoints, required), + (13, cur_counterparty_commitment_tx, option), + (15, prev_counterparty_commitment_tx, option), }); #[derive(Clone, PartialEq)] @@ -1755,6 +1773,8 @@ pub(crate) fn write_chanmon_internal( (34, channel_monitor.alternative_funding_confirmed, option), (35, channel_monitor.is_manual_broadcast, required), (37, channel_monitor.funding_seen_onchain, required), + (39, channel_monitor.funding.cur_counterparty_commitment_tx, option), + (41, channel_monitor.funding.prev_counterparty_commitment_tx, option), }); Ok(()) @@ -1904,6 +1924,9 @@ impl ChannelMonitor { current_holder_commitment_tx: initial_holder_commitment_tx, prev_holder_commitment_tx: None, + + cur_counterparty_commitment_tx: None, + prev_counterparty_commitment_tx: None, }, pending_funding: vec![], @@ -2271,6 +2294,37 @@ impl ChannelMonitor { self.inner.lock().unwrap().sign_to_local_justice_tx(justice_tx, input_idx, value, commitment_number) } + /// Stores the initial counterparty commitment and returns a signed justice transaction + /// if the commitment has already been revoked, or `None` otherwise. + /// + /// Intended to be called during [`Persist::persist_new_channel`]. + /// + /// [`Persist::persist_new_channel`]: crate::chain::chainmonitor::Persist::persist_new_channel + pub fn sign_initial_justice_tx( + &self, feerate_per_kw: u64, destination_script: ScriptBuf, + ) -> Option { + self.inner.lock().unwrap().sign_initial_justice_tx(feerate_per_kw, destination_script) + } + + /// Returns signed justice transactions for any counterparty commitments that + /// became revoked as a result of applying the given update. + /// + /// Stores new counterparty commitment(s) from the update and signs any + /// previously-stored commitments whose revocation secrets are now available. + /// + /// Intended to be called during [`Persist::update_persisted_channel`]. + /// + /// [`Persist::update_persisted_channel`]: crate::chain::chainmonitor::Persist::update_persisted_channel + pub fn sign_justice_txs_from_update( + &self, update: &ChannelMonitorUpdate, feerate_per_kw: u64, destination_script: ScriptBuf, + ) -> Vec { + self.inner.lock().unwrap().sign_justice_txs_from_update( + update, + feerate_per_kw, + destination_script, + ) + } + pub(crate) fn get_min_seen_secret(&self) -> u64 { self.inner.lock().unwrap().get_min_seen_secret() } @@ -3482,6 +3536,7 @@ impl ChannelMonitorImpl { self.provide_latest_counterparty_commitment_tx(commitment_tx.trust().txid(), Vec::new(), commitment_tx.commitment_number(), commitment_tx.per_commitment_point()); + self.funding.cur_counterparty_commitment_tx = Some(commitment_tx.clone()); // Soon, we will only populate this field self.initial_counterparty_commitment_tx = Some(commitment_tx); } @@ -4022,6 +4077,8 @@ impl ChannelMonitorImpl { counterparty_claimable_outpoints, current_holder_commitment_tx: alternative_holder_commitment_tx.clone(), prev_holder_commitment_tx: None, + cur_counterparty_commitment_tx: None, + prev_counterparty_commitment_tx: None, }; let alternative_funding_outpoint = alternative_funding.funding_outpoint(); @@ -4563,6 +4620,77 @@ impl ChannelMonitorImpl { Ok(justice_tx) } + fn sign_initial_justice_tx( + &mut self, feerate_per_kw: u64, destination_script: ScriptBuf, + ) -> Option { + let commitment_tx = self.initial_counterparty_commitment_tx()?; + self.funding.cur_counterparty_commitment_tx = Some(commitment_tx.clone()); + self.try_sign_justice_tx(&commitment_tx, feerate_per_kw, destination_script) + } + + fn sign_justice_txs_from_update( + &mut self, update: &ChannelMonitorUpdate, feerate_per_kw: u64, + destination_script: ScriptBuf, + ) -> Vec { + let new_commitment_txs = self.counterparty_commitment_txs_from_update(update); + + // Store new commitments, rotating cur -> prev per funding scope. + for commitment_tx in new_commitment_txs { + let txid = commitment_tx.trust().built_transaction().txid; + let funding = core::iter::once(&mut self.funding) + .chain(self.pending_funding.iter_mut()) + .find(|f| f.current_counterparty_commitment_txid == Some(txid)); + if let Some(funding) = funding { + funding.prev_counterparty_commitment_tx = + funding.cur_counterparty_commitment_tx.take(); + funding.cur_counterparty_commitment_tx = Some(commitment_tx); + } + } + + // Collect prev commitments that have revocation secrets available, clearing them + // from storage so they aren't signed again on subsequent calls. + let mut to_sign = Vec::new(); + for funding in core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) { + let should_take = + funding.prev_counterparty_commitment_tx.as_ref().is_some_and(|prev| { + self.commitment_secrets.get_secret(prev.commitment_number()).is_some() + }); + if should_take { + to_sign.push(funding.prev_counterparty_commitment_tx.take().unwrap()); + } + } + + let mut result = Vec::new(); + for commitment_tx in &to_sign { + if let Some(jtx) = + self.try_sign_justice_tx(commitment_tx, feerate_per_kw, destination_script.clone()) + { + result.push(jtx); + } + } + result + } + + fn try_sign_justice_tx( + &self, commitment_tx: &CommitmentTransaction, feerate_per_kw: u64, + destination_script: ScriptBuf, + ) -> Option { + let commitment_number = commitment_tx.commitment_number(); + self.get_secret(commitment_number)?; + + let trusted = commitment_tx.trust(); + let output_idx = trusted.revokeable_output_index()?; + let built = trusted.built_transaction(); + let value = built.transaction.output[output_idx].value; + let txid = built.txid; + + let justice_tx = + trusted.build_to_local_justice_tx(feerate_per_kw, destination_script).ok()?; + let signed = + self.sign_to_local_justice_tx(justice_tx, 0, value.to_sat(), commitment_number).ok()?; + Some(JusticeTransaction { tx: signed, revoked_commitment_txid: txid, commitment_number }) + } + /// Can only fail if idx is < get_min_seen_secret fn get_secret(&self, idx: u64) -> Option<[u8; 32]> { self.commitment_secrets.get_secret(idx) @@ -6521,6 +6649,8 @@ 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 cur_counterparty_commitment_tx: Option = None; + let mut prev_counterparty_commitment_tx_deser: Option = None; read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, optional_vec), @@ -6543,6 +6673,8 @@ 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, cur_counterparty_commitment_tx, option), + (41, prev_counterparty_commitment_tx_deser, 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. @@ -6657,6 +6789,9 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP current_holder_commitment_tx, prev_holder_commitment_tx, + + cur_counterparty_commitment_tx, + prev_counterparty_commitment_tx: prev_counterparty_commitment_tx_deser, }, pending_funding: pending_funding.unwrap_or(vec![]), is_manual_broadcast: is_manual_broadcast.0.unwrap(), diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 22be4367c7a..62b8c5fc79b 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -21,8 +21,6 @@ use crate::chain::channelmonitor::{ }; use crate::chain::transaction::OutPoint; use crate::chain::WatchedOutput; -#[cfg(any(test, feature = "_externalize_tests"))] -use crate::ln::chan_utils::CommitmentTransaction; use crate::ln::channel_state::ChannelDetails; use crate::ln::channelmanager; use crate::ln::inbound_payment::ExpandedKey; @@ -679,21 +677,9 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { } } -#[cfg(any(test, feature = "_externalize_tests"))] -struct JusticeTxData { - justice_tx: Transaction, - value: Amount, - commitment_number: u64, -} - #[cfg(any(test, feature = "_externalize_tests"))] pub(crate) struct WatchtowerPersister { persister: TestPersister, - /// Upon a new commitment_signed, we'll get a - /// ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTxInfo. We'll store the justice tx - /// amount, and commitment number so we can build the justice tx after our counterparty - /// revokes it. - unsigned_justice_tx_data: Mutex>>, /// After receiving a revoke_and_ack for a commitment number, we'll form and store the justice /// tx which would be used to provide a watchtower with the data it needs. watchtower_state: Mutex>>, @@ -703,12 +689,9 @@ pub(crate) struct WatchtowerPersister { #[cfg(any(test, feature = "_externalize_tests"))] impl WatchtowerPersister { pub(crate) fn new(destination_script: ScriptBuf) -> Self { - let unsigned_justice_tx_data = Mutex::new(new_hash_map()); - let watchtower_state = Mutex::new(new_hash_map()); WatchtowerPersister { persister: TestPersister::new(), - unsigned_justice_tx_data, - watchtower_state, + watchtower_state: Mutex::new(new_hash_map()), destination_script, } } @@ -724,23 +707,6 @@ impl WatchtowerPersister { .get(commitment_txid) .cloned() } - - fn form_justice_data_from_commitment( - &self, counterparty_commitment_tx: &CommitmentTransaction, - ) -> Option { - let trusted_tx = counterparty_commitment_tx.trust(); - let output_idx = trusted_tx.revokeable_output_index()?; - let built_tx = trusted_tx.built_transaction(); - let value = built_tx.transaction.output[output_idx as usize].value; - let justice_tx = trusted_tx - .build_to_local_justice_tx( - FEERATE_FLOOR_SATS_PER_KW as u64, - self.destination_script.clone(), - ) - .ok()?; - let commitment_number = counterparty_commitment_tx.commitment_number(); - Some(JusticeTxData { justice_tx, value, commitment_number }) - } } #[cfg(any(test, feature = "_externalize_tests"))] @@ -750,12 +716,6 @@ impl Persist for WatchtowerPers ) -> chain::ChannelMonitorUpdateStatus { let res = self.persister.persist_new_channel(monitor_name, data); - assert!(self - .unsigned_justice_tx_data - .lock() - .unwrap() - .insert(data.channel_id(), VecDeque::new()) - .is_none()); assert!(self .watchtower_state .lock() @@ -763,18 +723,18 @@ impl Persist for WatchtowerPers .insert(data.channel_id(), new_hash_map()) .is_none()); - let initial_counterparty_commitment_tx = - data.initial_counterparty_commitment_tx().expect("First and only call expects Some"); - if let Some(justice_data) = - self.form_justice_data_from_commitment(&initial_counterparty_commitment_tx) - { - self.unsigned_justice_tx_data + if let Some(jtx) = data.sign_initial_justice_tx( + FEERATE_FLOOR_SATS_PER_KW as u64, + self.destination_script.clone(), + ) { + self.watchtower_state .lock() .unwrap() .get_mut(&data.channel_id()) .unwrap() - .push_back(justice_data); + .insert(jtx.revoked_commitment_txid, jtx.tx); } + res } @@ -785,40 +745,23 @@ impl Persist for WatchtowerPers let res = self.persister.update_persisted_channel(monitor_name, update, data); if let Some(update) = update { - let commitment_txs = data.counterparty_commitment_txs_from_update(update); - let justice_datas = commitment_txs - .into_iter() - .filter_map(|commitment_tx| self.form_justice_data_from_commitment(&commitment_tx)); - let mut channels_justice_txs = self.unsigned_justice_tx_data.lock().unwrap(); - let channel_state = channels_justice_txs.get_mut(&data.channel_id()).unwrap(); - channel_state.extend(justice_datas); - - while let Some(JusticeTxData { justice_tx, value, commitment_number }) = - channel_state.front() - { - let input_idx = 0; - let commitment_txid = justice_tx.input[input_idx].previous_output.txid; - match data.sign_to_local_justice_tx( - justice_tx.clone(), - input_idx, - value.to_sat(), - *commitment_number, - ) { - Ok(signed_justice_tx) => { - let dup = self - .watchtower_state - .lock() - .unwrap() - .get_mut(&data.channel_id()) - .unwrap() - .insert(commitment_txid, signed_justice_tx); - assert!(dup.is_none()); - channel_state.pop_front(); - }, - Err(_) => break, - } + let justice_txs = data.sign_justice_txs_from_update( + update, + FEERATE_FLOOR_SATS_PER_KW as u64, + self.destination_script.clone(), + ); + for jtx in justice_txs { + let dup = self + .watchtower_state + .lock() + .unwrap() + .get_mut(&data.channel_id()) + .unwrap() + .insert(jtx.revoked_commitment_txid, jtx.tx); + assert!(dup.is_none()); } } + res }