From: Antoine Riard Date: Sat, 21 Mar 2020 02:41:12 +0000 (-0400) Subject: Access signed local commitment through OnchainTxHandler X-Git-Tag: v0.0.12~84^2~11 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=3cb61e979c21cd86b298202e6a716983c7349bab;p=rust-lightning 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. --- diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index ea95f2293..4bdc56674 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 2e071c2b7..a2f9a7c32 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 2270cdb7c..68c71dd88 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 + } }