From b7407b219d41f727f6967bd0cd046f15795367ab Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 18 Mar 2020 20:58:05 -0400 Subject: [PATCH] Implement reorg-safety for SpendableOutputDescriptor detection 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 | 1 + lightning/src/ln/channelmonitor.rs | 70 +++++++++++++++++++--------- lightning/src/ln/functional_tests.rs | 40 ++++++++++++---- 3 files changed, 81 insertions(+), 30 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 43a04452..d023c31d 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -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 diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 28ca6714..9b25da5b 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -594,6 +594,9 @@ enum OnchainEvent { HTLCUpdate { htlc_update: (HTLCSource, PaymentHash), }, + MaturingOutput { + descriptor: SpendableOutputDescriptor, + }, } const SERIALIZATION_VERSION: u8 = 1; @@ -1065,6 +1068,10 @@ impl ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { // 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 ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { 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 ChannelMonitor { } /// 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 { + 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 ReadableArgs> for (Sha256dH htlc_update: (htlc_source, hash) } }, + 1 => { + let descriptor = Readable::read(reader)?; + OnchainEvent::MaturingOutput { + descriptor + } + }, _ => return Err(DecodeError::InvalidValue), }; events.push(ev); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 275ecb08..5cd98d2f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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]); -- 2.30.2