From 3cb61e979c21cd86b298202e6a716983c7349bab Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Fri, 20 Mar 2020 22:41:12 -0400 Subject: [PATCH] Access signed local commitment through OnchainTxHandler Implementing dynamic fee bumping implied to cache transaction material including its witness, to generate a bumped version if needed. ChannelMonitor is slowly rescoped to its parsing function with ongoing patchset and data duplicata are removed. If signed local commitment tx access is needed, it's done through OnchainTxHandler extended API For test framework purpose, we use the test-only method ChannelMonitor::unsafe_get_latest_local_commitment_txn to intentionally generate unsafe local commitment to exerce revocation logic. --- lightning/src/ln/channelmonitor.rs | 67 ++++++++++++++--------- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/onchaintx.rs | 18 ++++++ 3 files changed, 60 insertions(+), 27 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index ea95f229..4bdc5667 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -1687,7 +1687,7 @@ impl ChannelMonitor { (claimable_outpoints, Some((htlc_txid, tx.output.clone()))) } - fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx) -> (Vec, Vec, Option<(Script, SecretKey, Script)>) { + fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec, Vec, Option<(Script, SecretKey, Script)>) { let mut res = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); @@ -1730,7 +1730,7 @@ impl ChannelMonitor { res.push(htlc_success_tx); } } - watch_outputs.push(local_tx.tx.without_valid_witness().output[transaction_output_index as usize].clone()); + watch_outputs.push(commitment_tx.output[transaction_output_index as usize].clone()); } else { panic!("Should have sigs for non-dust local tx outputs!") } } } @@ -1784,7 +1784,7 @@ impl ChannelMonitor { if local_tx.txid == commitment_txid { is_local_tx = true; log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim"); - let mut res = self.broadcast_by_local_state(local_tx); + let mut res = self.broadcast_by_local_state(tx, local_tx); append_onchain_update!(res); } } @@ -1792,8 +1792,7 @@ impl ChannelMonitor { if local_tx.txid == commitment_txid { is_local_tx = true; log_trace!(self, "Got previous local commitment tx broadcast, searching for available HTLCs to claim"); - assert!(local_tx.tx.has_local_sig()); - let mut res = self.broadcast_by_local_state(local_tx); + let mut res = self.broadcast_by_local_state(tx, local_tx); append_onchain_update!(res); } } @@ -1832,22 +1831,35 @@ impl ChannelMonitor { /// out-of-band the other node operator to coordinate with him if option is available to you. /// In any-case, choice is up to the user. pub fn get_latest_local_commitment_txn(&mut self) -> Vec { - // TODO: We should likely move all of the logic in here into OnChainTxHandler and unify it - // to ensure add_local_sig is only ever called once no matter what. This likely includes - // tracking state and panic!()ing if we get an update after force-closure/local-tx signing. log_trace!(self, "Getting signed latest local commitment transaction!"); - if let &mut Some(ref mut local_tx) = &mut self.current_local_signed_commitment_tx { - self.onchain_detection.keys.sign_local_commitment(&mut local_tx.tx, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); + if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx(self.channel_value_satoshis.unwrap()) { + let mut res = vec![commitment_tx]; + if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { + let mut htlc_txn = self.broadcast_by_local_state(res.get(0).unwrap(), local_tx).0; + res.append(&mut htlc_txn); + // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. + // The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation. + } + return res } - if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { - let mut res = vec![local_tx.tx.with_valid_witness().clone()]; - res.append(&mut self.broadcast_by_local_state(local_tx).0); - // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. - // The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation. - res - } else { - Vec::new() + Vec::new() + } + + /// Unsafe test-only version of get_latest_local_commitment_txn used by our test framework + /// to bypass LocalCommitmentTransaction state update lockdown after signature and generate + /// revoked commitment transaction. + #[cfg(test)] + pub fn unsafe_get_latest_local_commitment_txn(&mut self) -> Vec { + log_trace!(self, "Getting signed copy of latest local commitment transaction!"); + if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_copy_local_tx(self.channel_value_satoshis.unwrap()) { + let mut res = vec![commitment_tx]; + if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { + let mut htlc_txn = self.broadcast_by_local_state(res.get(0).unwrap(), local_tx).0; + res.append(&mut htlc_txn); + } + return res } + Vec::new() } /// Called by SimpleManyChannelMonitor::block_connected, which implements @@ -1922,13 +1934,15 @@ impl ChannelMonitor { } if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx { if should_broadcast { - let (txs, new_outputs, _) = self.broadcast_by_local_state(&cur_local_tx); - if !new_outputs.is_empty() { - watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); - } - for tx in txs { - log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(&tx); + if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx(self.channel_value_satoshis.unwrap()) { + let (txs, new_outputs, _) = self.broadcast_by_local_state(&commitment_tx, cur_local_tx); + if !new_outputs.is_empty() { + watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); + } + for tx in txs { + log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); + broadcaster.broadcast_transaction(&tx); + } } } } @@ -2401,7 +2415,8 @@ impl ReadableArgs> for (Sha256dH LocalSignedTx { txid: tx.txid(), - tx, revocation_key, a_htlc_key, b_htlc_key, delayed_payment_key, per_commitment_point, feerate_per_kw, + tx, + revocation_key, a_htlc_key, b_htlc_key, delayed_payment_key, per_commitment_point, feerate_per_kw, htlc_outputs: htlcs } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 2e071c2b..a2f9a7c3 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -264,7 +264,7 @@ macro_rules! get_local_commitment_txn { let mut commitment_txn = None; for (funding_txo, monitor) in monitors.iter_mut() { if funding_txo.to_channel_id() == $channel_id { - commitment_txn = Some(monitor.get_latest_local_commitment_txn()); + commitment_txn = Some(monitor.unsafe_get_latest_local_commitment_txn()); break; } } diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 2270cdb7..68c71dd8 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -769,4 +769,22 @@ impl OnchainTxHandler { self.local_commitment = Some(tx); Ok(()) } + + pub(super) fn get_fully_signed_local_tx(&mut self, channel_value_satoshis: u64) -> Option { + if let Some(ref mut local_commitment) = self.local_commitment { + self.key_storage.sign_local_commitment(local_commitment, &self.funding_redeemscript, channel_value_satoshis, &self.secp_ctx); + return Some(local_commitment.with_valid_witness().clone()); + } + None + } + + #[cfg(test)] + pub(super) fn get_fully_signed_copy_local_tx(&mut self, channel_value_satoshis: u64) -> Option { + if let Some(ref mut local_commitment) = self.local_commitment { + let mut local_commitment = local_commitment.clone(); + self.key_storage.unsafe_sign_local_commitment(&mut local_commitment, &self.funding_redeemscript, channel_value_satoshis, &self.secp_ctx); + return Some(local_commitment.with_valid_witness().clone()); + } + None + } } -- 2.30.2