]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Prevent any update of local commitment transaction once signed
authorAntoine Riard <ariard@student.42.fr>
Fri, 27 Mar 2020 21:53:52 +0000 (17:53 -0400)
committerAntoine Riard <ariard@student.42.fr>
Fri, 17 Apr 2020 21:43:50 +0000 (17:43 -0400)
To prevent any unsafe state discrepancy between offchain and onchain,
once local commitment transaction has been signed due to an event
(either block height for HTLC-timeout or channel force-closure), don't
allow any further update of local commitment transaction view
to avoid delivery of revocation secret to counterparty for the
aformentionned signed transaction.

lightning/src/ln/chan_utils.rs
lightning/src/ln/channelmonitor.rs
lightning/src/ln/onchaintx.rs

index 0ef269c8cb815121edf733982fc4ac3c574c54e0..a5e0c1460be71b2281fb46e1abb528ac5fbecec5 100644 (file)
@@ -522,9 +522,18 @@ pub struct LocalCommitmentTransaction {
 impl LocalCommitmentTransaction {
        #[cfg(test)]
        pub fn dummy() -> Self {
+               let dummy_input = TxIn {
+                       previous_output: OutPoint {
+                               txid: Default::default(),
+                               vout: 0,
+                       },
+                       script_sig: Default::default(),
+                       sequence: 0,
+                       witness: vec![vec![], vec![], vec![]]
+               };
                Self { tx: Transaction {
                        version: 2,
-                       input: Vec::new(),
+                       input: vec![dummy_input],
                        output: Vec::new(),
                        lock_time: 0,
                } }
index 70c92143af8b50d58eb06f9c10878fc6fe190e2d..2fa416edb2dd79f26c8c0f5433b31e60c135bc2b 100644 (file)
@@ -1241,10 +1241,18 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                if self.their_to_self_delay.is_none() {
                        return Err(MonitorUpdateError("Got a local commitment tx info update before we'd set basic information about the channel"));
                }
+               // Returning a monitor error before updating tracking points means in case of using
+               // a concurrent watchtower implementation for same channel, if this one doesn't
+               // reject update as we do, you MAY have the latest local valid commitment tx onchain
+               // for which you want to spend outputs. We're NOT robust again this scenario right
+               // now but we should consider it later.
+               if let Err(_) = self.onchain_tx_handler.provide_latest_local_tx(commitment_tx.clone()) {
+                       return Err(MonitorUpdateError("Local commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed"));
+               }
                self.prev_local_signed_commitment_tx = self.current_local_signed_commitment_tx.take();
                self.current_local_signed_commitment_tx = Some(LocalSignedTx {
                        txid: commitment_tx.txid(),
-                       tx: commitment_tx.clone(),
+                       tx: commitment_tx,
                        revocation_key: local_keys.revocation_key,
                        a_htlc_key: local_keys.a_htlc_key,
                        b_htlc_key: local_keys.b_htlc_key,
@@ -1253,7 +1261,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        feerate_per_kw,
                        htlc_outputs,
                });
-               self.onchain_tx_handler.provide_latest_local_tx(commitment_tx);
                Ok(())
        }
 
index 4c935ea654843ab66f6e5a36f0794652e06d3c01..d0d1f29e691f5ea5118d0a06b2e5bf4030fd73aa 100644 (file)
@@ -725,8 +725,17 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                }
        }
 
-       pub(super) fn provide_latest_local_tx(&mut self, tx: LocalCommitmentTransaction) {
+       pub(super) fn provide_latest_local_tx(&mut self, tx: LocalCommitmentTransaction) -> Result<(), ()> {
+               // To prevent any unsafe state discrepancy between offchain and onchain, once local
+               // commitment transaction has been signed due to an event (either block height for
+               // HTLC-timeout or channel force-closure), don't allow any further update of local
+               // commitment transaction view to avoid delivery of revocation secret to counterparty
+               // for the aformentionned signed transaction.
+               if let Some(ref local_commitment) = self.local_commitment {
+                       if local_commitment.has_local_sig() { return Err(()) }
+               }
                self.prev_local_commitment = self.local_commitment.take();
                self.local_commitment = Some(tx);
+               Ok(())
        }
 }