From a55355e64145a45ae3bdc6784c0bfd0095ba3d7f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 13 Sep 2018 23:54:15 -0400 Subject: [PATCH 1/1] Ignore HTLC txn we dont know how to claim instead of unwrap()ing This fixes a crash introduced in 3e149b1fb6624eef99b055bde772842f36 and introduces a test which will tickle the bug. --- src/ln/channelmanager.rs | 42 ++++++++++++++++++++++++++++++++++++++++ src/ln/channelmonitor.rs | 3 +-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index a2b3b242..576c4f0f 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -3380,6 +3380,48 @@ mod tests { } } + #[test] + fn test_htlc_ignore_latest_remote_commitment() { + // Test that HTLC transactions spending the latest remote commitment transaction are simply + // ignored if we cannot claim them. This originally tickled an invalid unwrap(). + let nodes = create_network(2); + create_announced_chan_between_nodes(&nodes, 0, 1); + + route_payment(&nodes[0], &[&nodes[1]], 10000000); + nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id); + { + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::BroadcastChannelUpdate { msg: msgs::ChannelUpdate { contents: msgs::UnsignedChannelUpdate { flags, .. }, .. } } => { + assert_eq!(flags & 0b10, 0b10); + }, + _ => panic!("Unexpected event"), + } + } + + let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); + assert_eq!(node_txn.len(), 2); + + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + nodes[1].chain_monitor.block_connected_checked(&header, 1, &[&node_txn[0], &node_txn[1]], &[1; 2]); + + { + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::BroadcastChannelUpdate { msg: msgs::ChannelUpdate { contents: msgs::UnsignedChannelUpdate { flags, .. }, .. } } => { + assert_eq!(flags & 0b10, 0b10); + }, + _ => panic!("Unexpected event"), + } + } + + // Duplicate the block_connected call since this may happen due to other listeners + // registering new transactions + nodes[1].chain_monitor.block_connected_checked(&header, 1, &[&node_txn[0], &node_txn[1]], &[1; 2]); + } + #[test] fn test_unconf_chan() { // After creating a chan between nodes, we disconnect all blocks previously seen to force a channel close on nodes[0] side diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 12c26322..55e534c8 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -1222,7 +1222,7 @@ impl ChannelMonitor { }; } - let secret = self.get_secret(commitment_number).unwrap(); + let secret = ignore_error!(self.get_secret(commitment_number)); let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret)); let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); let revocation_pubkey = match self.key_storage { @@ -1269,7 +1269,6 @@ impl ChannelMonitor { output: outputs, }; - let sighash_parts = bip143::SighashComponents::new(&spend_tx); let sig = match self.key_storage { -- 2.30.2