Implement reorg-safety for SpendableOutputDescriptor detection
authorAntoine Riard <ariard@student.42.fr>
Thu, 19 Mar 2020 00:58:05 +0000 (20:58 -0400)
committerAntoine Riard <ariard@student.42.fr>
Fri, 20 Mar 2020 02:31:48 +0000 (22:31 -0400)
We delay SpendableOutputDescriptor until reaching ANTI_REORG_DELAY
to avoid misleading user wallet in case of reorg and alternative
settlement on a channel output.

Fix tests in consequence.

lightning/src/chain/keysinterface.rs
lightning/src/ln/channelmonitor.rs
lightning/src/ln/functional_tests.rs

index 43a044524e4d1713fa6da0cac1ce7c8eb713fa85..d023c31de239c1127cf62c63ed1fabc39a6010da 100644 (file)
@@ -37,6 +37,7 @@ use ln::msgs::DecodeError;
 /// spend on-chain. The information needed to do this is provided in this enum, including the
 /// outpoint describing which txid and output index is available, the full output which exists at
 /// that txid/index, and any keys or other information required to sign.
+#[derive(Clone, PartialEq)]
 pub enum SpendableOutputDescriptor {
        /// An output to a script which was provided via KeysInterface, thus you should already know
        /// how to spend it. No keys are provided as rust-lightning was never given any keys - only the
index 28ca6714acafaf51c16af07722af0b977fb8300e..9b25da5b4c9fd23ce938f551f798021a17efd592 100644 (file)
@@ -594,6 +594,9 @@ enum OnchainEvent {
        HTLCUpdate {
                htlc_update: (HTLCSource, PaymentHash),
        },
+       MaturingOutput {
+               descriptor: SpendableOutputDescriptor,
+       },
 }
 
 const SERIALIZATION_VERSION: u8 = 1;
@@ -1065,6 +1068,10 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
                                                htlc_update.0.write(writer)?;
                                                htlc_update.1.write(writer)?;
                                        },
+                                       OnchainEvent::MaturingOutput { ref descriptor } => {
+                                               1u8.write(writer)?;
+                                               descriptor.write(writer)?;
+                                       },
                                }
                        }
                }
@@ -1561,6 +1568,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                                                        OnchainEvent::HTLCUpdate { ref htlc_update } => {
                                                                                                                return htlc_update.0 != **source
                                                                                                        },
+                                                                                                       _ => true
                                                                                                }
                                                                                        });
                                                                                        e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())});
@@ -1625,6 +1633,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                                                OnchainEvent::HTLCUpdate { ref htlc_update } => {
                                                                                                        return htlc_update.0 != **source
                                                                                                },
+                                                                                               _ => true
                                                                                        }
                                                                                });
                                                                                e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())});
@@ -1808,6 +1817,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                OnchainEvent::HTLCUpdate { ref htlc_update } => {
                                                                        return htlc_update.0 != $source
                                                                },
+                                                               _ => true
                                                        }
                                                });
                                                e.push(OnchainEvent::HTLCUpdate { htlc_update: ($source, $payment_hash)});
@@ -1961,7 +1971,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
                log_trace!(self, "Block {} at height {} connected with {} txn matched", block_hash, height, txn_matched.len());
                let mut watch_outputs = Vec::new();
-               let mut spendable_outputs = Vec::new();
                let mut claimable_outpoints = Vec::new();
                for tx in txn_matched {
                        if tx.input.len() == 1 {
@@ -2011,9 +2020,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        // we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check.
                        self.is_resolving_htlc_output(&tx, height);
 
-                       if let Some(spendable_output) = self.is_paying_spendable_output(&tx) {
-                               spendable_outputs.push(spendable_output);
-                       }
+                       self.is_paying_spendable_output(&tx, height);
                }
                let should_broadcast = if let Some(_) = self.current_local_signed_commitment_tx {
                        self.would_broadcast_at_height(height)
@@ -2058,6 +2065,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                        source: htlc_update.0,
                                                });
                                        },
+                                       OnchainEvent::MaturingOutput { descriptor } => {
+                                               log_trace!(self, "Descriptor {} has got enough confirmations to be passed upstream", log_spendable!(descriptor));
+                                               self.pending_events.push(events::Event::SpendableOutputs {
+                                                       outputs: vec![descriptor]
+                                               });
+                                       }
                                }
                        }
                }
@@ -2068,16 +2081,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        self.outputs_to_watch.insert(txid.clone(), output_scripts.iter().map(|o| o.script_pubkey.clone()).collect());
                }
 
-               for spend in spendable_outputs.iter() {
-                       log_trace!(self, "Announcing spendable output to user: {}", log_spendable!(spend));
-               }
-
-               if spendable_outputs.len() > 0 {
-                       self.pending_events.push(events::Event::SpendableOutputs {
-                               outputs: spendable_outputs,
-                       });
-               }
-
                watch_outputs
        }
 
@@ -2089,6 +2092,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + ANTI_REORG_DELAY - 1)) {
                        //We may discard:
                        //- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected
+                       //- maturing spendable output has transaction paying us has been disconnected
                }
 
                self.onchain_tx_handler.block_disconnected(height, broadcaster, fee_estimator);
@@ -2291,6 +2295,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                                        OnchainEvent::HTLCUpdate { ref htlc_update } => {
                                                                                return htlc_update.0 != source
                                                                        },
+                                                                       _ => true
                                                                }
                                                        });
                                                        e.push(OnchainEvent::HTLCUpdate { htlc_update: (source, payment_hash)});
@@ -2305,39 +2310,54 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        }
 
        /// Check if any transaction broadcasted is paying fund back to some address we can assume to own
-       fn is_paying_spendable_output(&self, tx: &Transaction) -> Option<SpendableOutputDescriptor> {
+       fn is_paying_spendable_output(&mut self, tx: &Transaction, height: u32) {
+               let mut spendable_output = None;
                for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us
                        if outp.script_pubkey == self.destination_script {
-                               return Some(SpendableOutputDescriptor::StaticOutput {
+                               spendable_output =  Some(SpendableOutputDescriptor::StaticOutput {
                                        outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
                                        output: outp.clone(),
                                });
+                               break;
                        } else if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script {
                                if broadcasted_local_revokable_script.0 == outp.script_pubkey {
-                                       return Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
+                                       spendable_output =  Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
                                                outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
                                                key: broadcasted_local_revokable_script.1,
                                                witness_script: broadcasted_local_revokable_script.2.clone(),
                                                to_self_delay: self.their_to_self_delay.unwrap(),
                                                output: outp.clone(),
                                        });
+                                       break;
                                }
                        } else if let Some(ref broadcasted_remote_payment_script) = self.broadcasted_remote_payment_script {
                                if broadcasted_remote_payment_script.0 == outp.script_pubkey {
-                                       return Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
+                                       spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
                                                outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
                                                key: broadcasted_remote_payment_script.1,
                                                output: outp.clone(),
                                        });
+                                       break;
                                }
                        } else if outp.script_pubkey == self.shutdown_script {
-                               return Some(SpendableOutputDescriptor::StaticOutput {
-                                               outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
-                                               output: outp.clone(),
+                               spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
+                                       outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
+                                       output: outp.clone(),
                                });
                        }
                }
-               None
+               if let Some(spendable_output) = spendable_output {
+                       log_trace!(self, "Maturing {} until {}", log_spendable!(spendable_output), height + ANTI_REORG_DELAY - 1);
+                       match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) {
+                               hash_map::Entry::Occupied(mut entry) => {
+                                       let e = entry.get_mut();
+                                       e.push(OnchainEvent::MaturingOutput { descriptor: spendable_output });
+                               }
+                               hash_map::Entry::Vacant(entry) => {
+                                       entry.insert(vec![OnchainEvent::MaturingOutput { descriptor: spendable_output }]);
+                               }
+                       }
+               }
        }
 }
 
@@ -2588,6 +2608,12 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
                                                        htlc_update: (htlc_source, hash)
                                                }
                                        },
+                                       1 => {
+                                               let descriptor = Readable::read(reader)?;
+                                               OnchainEvent::MaturingOutput {
+                                                       descriptor
+                                               }
+                                       },
                                        _ => return Err(DecodeError::InvalidValue),
                                };
                                events.push(ev);
index 275ecb082023117927be61f3882520a4ea51d95b..5cd98d2f76028b74e03624f60b5f95e2e99e4662 100644 (file)
@@ -4049,6 +4049,8 @@ fn test_claim_sizeable_push_msat() {
 
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
+
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
@@ -4077,6 +4079,8 @@ fn test_claim_on_remote_sizeable_push_msat() {
        nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
        check_closed_broadcast!(nodes[1], false);
        check_added_monitors!(nodes[1], 1);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
+
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
        assert_eq!(spend_txn.len(), 2);
        assert_eq!(spend_txn[0], spend_txn[1]);
@@ -4101,13 +4105,14 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
 
        claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+       nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 0);
        check_closed_broadcast!(nodes[1], false);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
        let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 2);
+       nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
        assert_eq!(spend_txn.len(), 3);
@@ -4158,6 +4163,7 @@ fn test_static_spendable_outputs_preimage_tx() {
 
        let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
        assert_eq!(spend_txn.len(), 1);
@@ -4182,7 +4188,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
        claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
 
        let  header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+       nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 0);
        check_closed_broadcast!(nodes[1], false);
        check_added_monitors!(nodes[1], 1);
 
@@ -4192,7 +4198,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
        check_spends!(node_txn[0], revoked_local_txn[0]);
 
        let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 2);
+       nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
        assert_eq!(spend_txn.len(), 1);
@@ -4247,6 +4254,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
 
        let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone(), node_txn[2].clone()] }, 1);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
        // Check B's ChannelMonitor was able to generate the right spendable output descriptor
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
@@ -4297,6 +4305,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
 
        let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[0].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone(), node_txn[2].clone()] }, 1);
+       connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
        // Check A's ChannelMonitor was able to generate the right spendable output descriptor
        let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -4573,6 +4582,7 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
 
        let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[1].block_notifier.block_connected(&Block { header: header_201, txdata: node_txn.clone() }, 201);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
 
        // Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
        let spend_txn = check_spendable_outputs!(nodes[1], 1);
@@ -4844,7 +4854,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
        // Create some initial channels
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::supported(), InitFeatures::supported());
 
-       route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000).0;
+       let (_, our_payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000);
        let local_txn = get_local_commitment_txn!(nodes[0], chan_1.2);
        assert_eq!(local_txn[0].input.len(), 1);
        check_spends!(local_txn[0], chan_1.3);
@@ -4865,6 +4875,15 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 
        let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        nodes[0].block_notifier.block_connected(&Block { header: header_201, txdata: vec![htlc_timeout.clone()] }, 201);
+       connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
+               Event::PaymentFailed { payment_hash, .. } => {
+                       assert_eq!(payment_hash, our_payment_hash);
+               },
+               _ => panic!("Unexpected event"),
+       }
 
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
        let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -4887,12 +4906,16 @@ fn test_static_output_closing_tx() {
        let closing_tx = close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true).2;
 
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 1);
+       nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
+       connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
+
        let spend_txn = check_spendable_outputs!(nodes[0], 2);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], closing_tx);
 
-       nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 1);
+       nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
+       connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
+
        let spend_txn = check_spendable_outputs!(nodes[1], 2);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], closing_tx);
@@ -6744,7 +6767,8 @@ fn test_data_loss_protect() {
        check_spends!(node_txn[0], chan.3);
        assert_eq!(node_txn[0].output.len(), 2);
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
-       nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 1);
+       nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 0);
+       connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
        let spend_txn = check_spendable_outputs!(nodes[0], 1);
        assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);