Don't unwrap() get_channel_update result in HTLC router
[rust-lightning] / src / ln / channelmanager.rs
index 4808bd832a66d4ea85af8f38f0e9b78a8bfa271a..c3d39d8f85498508468c771d21a690a26891a351 100644 (file)
@@ -12,8 +12,7 @@ use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::transaction::Transaction;
 use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::network::constants::Network;
-use bitcoin::network::serialize::BitcoinHash;
-use bitcoin::util::hash::Sha256dHash;
+use bitcoin::util::hash::{BitcoinHash, Sha256dHash};
 
 use secp256k1::key::{SecretKey,PublicKey};
 use secp256k1::{Secp256k1,Message};
@@ -1943,10 +1942,20 @@ impl ChannelManager {
                                        // but if we've sent a shutdown and they haven't acknowledged it yet, we just
                                        // want to reject the new HTLC and fail it backwards instead of forwarding.
                                        if let PendingHTLCStatus::Forward(PendingForwardHTLCInfo { incoming_shared_secret, .. }) = pending_forward_info {
+                                               let chan_update = self.get_channel_update(chan);
                                                pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
                                                        channel_id: msg.channel_id,
                                                        htlc_id: msg.htlc_id,
-                                                       reason: ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &self.get_channel_update(chan).unwrap().encode_with_len()[..]),
+                                                       reason: if let Ok(update) = chan_update {
+                                                               ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &update.encode_with_len()[..])
+                                                       } else {
+                                                               // This can only happen if the channel isn't in the fully-funded
+                                                               // state yet, implying our counterparty is trying to route payments
+                                                               // over the channel back to themselves (cause no one else should
+                                                               // know the short_id is a lightning channel yet). We should have no
+                                                               // problem just calling this unknown_next_peer
+                                                               ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x4000|10, &[])
+                                                       },
                                                }));
                                        }
                                }
@@ -2194,8 +2203,8 @@ impl ChannelManager {
                                        //TODO: here and below MsgHandleErrInternal, #153 case
                                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                }
-                               if (msg.failure_code & 0x8000) != 0 {
-                                       return Err(MsgHandleErrInternal::send_err_msg_close_chan("Got update_fail_malformed_htlc with BADONION set", msg.channel_id));
+                               if (msg.failure_code & 0x8000) == 0 {
+                                       return Err(MsgHandleErrInternal::send_err_msg_close_chan("Got update_fail_malformed_htlc with BADONION not set", msg.channel_id));
                                }
                                chan.update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() })
                                        .map_err(|e| MsgHandleErrInternal::from_chan_maybe_close(e, msg.channel_id))?;
@@ -2733,6 +2742,7 @@ impl ChannelMessageHandler for ChannelManager {
                        let short_to_id = channel_state.short_to_id;
                        let pending_msg_events = channel_state.pending_msg_events;
                        if no_connection_possible {
+                               log_debug!(self, "Failing all channels with {} due to no_connection_possible", log_pubkey!(their_node_id));
                                channel_state.by_id.retain(|_, chan| {
                                        if chan.get_their_node_id() == *their_node_id {
                                                if let Some(short_id) = chan.get_short_channel_id() {
@@ -2750,6 +2760,7 @@ impl ChannelMessageHandler for ChannelManager {
                                        }
                                });
                        } else {
+                               log_debug!(self, "Marking channels with {} disconnected and generating channel_updates", log_pubkey!(their_node_id));
                                channel_state.by_id.retain(|_, chan| {
                                        if chan.get_their_node_id() == *their_node_id {
                                                //TODO: mark channel disabled (and maybe announce such after a timeout).
@@ -2780,6 +2791,8 @@ impl ChannelMessageHandler for ChannelManager {
        }
 
        fn peer_connected(&self, their_node_id: &PublicKey) {
+               log_debug!(self, "Generating channel_reestablish events for {}", log_pubkey!(their_node_id));
+
                let _ = self.total_consistency_lock.read().unwrap();
                let mut channel_state_lock = self.channel_state.lock().unwrap();
                let channel_state = channel_state_lock.borrow_parts();
@@ -3220,13 +3233,11 @@ mod tests {
        use util::ser::{Writeable, Writer, ReadableArgs};
        use util::config::UserConfig;
 
-       use bitcoin::util::hash::Sha256dHash;
+       use bitcoin::util::hash::{BitcoinHash, Sha256dHash};
        use bitcoin::blockdata::block::{Block, BlockHeader};
        use bitcoin::blockdata::transaction::{Transaction, TxOut};
        use bitcoin::blockdata::constants::genesis_block;
        use bitcoin::network::constants::Network;
-       use bitcoin::network::serialize::serialize;
-       use bitcoin::network::serialize::BitcoinHash;
 
        use hex;
 
@@ -3516,7 +3527,7 @@ mod tests {
                                tx = Transaction { version: chan_id as u32, lock_time: 0, input: Vec::new(), output: vec![TxOut {
                                        value: *channel_value_satoshis, script_pubkey: output_script.clone(),
                                }]};
-                               funding_output = OutPoint::new(Sha256dHash::from_data(&serialize(&tx).unwrap()[..]), 0);
+                               funding_output = OutPoint::new(tx.txid(), 0);
 
                                node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
                                let mut added_monitors = node_a.chan_monitor.added_monitors.lock().unwrap();
@@ -5704,7 +5715,13 @@ mod tests {
                get_announce_close_broadcast_events(&nodes, 3, 4);
                assert_eq!(nodes[3].node.list_channels().len(), 0);
                assert_eq!(nodes[4].node.list_channels().len(), 0);
+       }
+
+       #[test]
+       fn test_justice_tx() {
+               // Test justice txn built on revoked HTLC-Success tx, against both sides
 
+               let nodes = create_network(2);
                // Create some new channels:
                let chan_5 = create_announced_chan_between_nodes(&nodes, 0, 1);
 
@@ -5743,6 +5760,45 @@ mod tests {
                        test_revoked_htlc_claim_txn_broadcast(&nodes[1], node_txn[1].clone());
                }
                get_announce_close_broadcast_events(&nodes, 0, 1);
+
+               assert_eq!(nodes[0].node.list_channels().len(), 0);
+               assert_eq!(nodes[1].node.list_channels().len(), 0);
+
+               // We test justice_tx build by A on B's revoked HTLC-Success tx
+               // Create some new channels:
+               let chan_6 = create_announced_chan_between_nodes(&nodes, 0, 1);
+
+               // A pending HTLC which will be revoked:
+               let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
+               // Get the will-be-revoked local txn from B
+               let revoked_local_txn = nodes[1].node.channel_state.lock().unwrap().by_id.iter().next().unwrap().1.last_local_commitment_txn.clone();
+               assert_eq!(revoked_local_txn.len(), 1); // Only commitment tx
+               assert_eq!(revoked_local_txn[0].input.len(), 1);
+               assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_6.3.txid());
+               assert_eq!(revoked_local_txn[0].output.len(), 2); // Only HTLC and output back to A are present
+               // Revoke the old state
+               claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_4);
+               {
+                       let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+                       nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+                       {
+                               let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+                               assert_eq!(node_txn.len(), 3);
+                               assert_eq!(node_txn.pop().unwrap(), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
+                               assert_eq!(node_txn[0].input.len(), 1); // We claim the received HTLC output
+
+                               check_spends!(node_txn[0], revoked_local_txn[0].clone());
+                               node_txn.swap_remove(0);
+                       }
+                       test_txn_broadcast(&nodes[0], &chan_6, None, HTLCType::NONE);
+
+                       nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+                       let node_txn = test_txn_broadcast(&nodes[1], &chan_6, Some(revoked_local_txn[0].clone()), HTLCType::SUCCESS);
+                       header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+                       nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![node_txn[1].clone()] }, 1);
+                       test_revoked_htlc_claim_txn_broadcast(&nodes[0], node_txn[1].clone());
+               }
+               get_announce_close_broadcast_events(&nodes, 0, 1);
                assert_eq!(nodes[0].node.list_channels().len(), 0);
                assert_eq!(nodes[1].node.list_channels().len(), 0);
        }
@@ -6163,7 +6219,7 @@ mod tests {
 
        /// pending_htlc_adds includes both the holding cell and in-flight update_add_htlcs, whereas
        /// for claims/fails they are separated out.
-       fn reconnect_nodes(node_a: &Node, node_b: &Node, pre_all_htlcs: bool, pending_htlc_adds: (i64, i64), pending_htlc_claims: (usize, usize), pending_cell_htlc_claims: (usize, usize), pending_cell_htlc_fails: (usize, usize), pending_raa: (bool, bool)) {
+       fn reconnect_nodes(node_a: &Node, node_b: &Node, send_funding_locked: (bool, bool), pending_htlc_adds: (i64, i64), pending_htlc_claims: (usize, usize), pending_cell_htlc_claims: (usize, usize), pending_cell_htlc_fails: (usize, usize), pending_raa: (bool, bool)) {
                node_a.node.peer_connected(&node_b.node.get_our_node_id());
                let reestablish_1 = get_chan_reestablish_msgs!(node_a, node_b);
                node_b.node.peer_connected(&node_a.node.get_our_node_id());
@@ -6196,7 +6252,7 @@ mod tests {
                        (pending_htlc_adds.1 == 0 && pending_htlc_claims.1 == 0 && pending_cell_htlc_claims.1 == 0 && pending_cell_htlc_fails.1 == 0));
 
                for chan_msgs in resp_1.drain(..) {
-                       if pre_all_htlcs {
+                       if send_funding_locked.0 {
                                node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), &chan_msgs.0.unwrap()).unwrap();
                                let announcement_event = node_a.node.get_and_clear_pending_msg_events();
                                if !announcement_event.is_empty() {
@@ -6253,7 +6309,7 @@ mod tests {
                }
 
                for chan_msgs in resp_2.drain(..) {
-                       if pre_all_htlcs {
+                       if send_funding_locked.1 {
                                node_b.node.handle_funding_locked(&node_a.node.get_our_node_id(), &chan_msgs.0.unwrap()).unwrap();
                                let announcement_event = node_b.node.get_and_clear_pending_msg_events();
                                if !announcement_event.is_empty() {
@@ -6317,7 +6373,7 @@ mod tests {
 
                nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-               reconnect_nodes(&nodes[0], &nodes[1], true, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
                let payment_preimage_1 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
                let payment_hash_2 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).1;
@@ -6326,7 +6382,7 @@ mod tests {
 
                nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-               reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
                let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
                let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
@@ -6339,7 +6395,7 @@ mod tests {
                claim_payment_along_route(&nodes[0], &vec!(&nodes[1], &nodes[2]), true, payment_preimage_3);
                fail_payment_along_route(&nodes[0], &[&nodes[1], &nodes[2]], true, payment_hash_5);
 
-               reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (1, 0), (1, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (1, 0), (1, 0), (false, false));
                {
                        let events = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 2);
@@ -6420,19 +6476,19 @@ mod tests {
                if messages_delivered < 3 {
                        // Even if the funding_locked messages get exchanged, as long as nothing further was
                        // received on either side, both sides will need to resend them.
-                       reconnect_nodes(&nodes[0], &nodes[1], true, (0, 1), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (true, true), (0, 1), (0, 0), (0, 0), (0, 0), (false, false));
                } else if messages_delivered == 3 {
                        // nodes[0] still wants its RAA + commitment_signed
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (-1, 0), (0, 0), (0, 0), (0, 0), (true, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (-1, 0), (0, 0), (0, 0), (0, 0), (true, false));
                } else if messages_delivered == 4 {
                        // nodes[0] still wants its commitment_signed
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (-1, 0), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (-1, 0), (0, 0), (0, 0), (0, 0), (false, false));
                } else if messages_delivered == 5 {
                        // nodes[1] still wants its final RAA
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, true));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, true));
                } else if messages_delivered == 6 {
                        // Everything was delivered...
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
                }
 
                let events_1 = nodes[1].node.get_and_clear_pending_events();
@@ -6444,7 +6500,7 @@ mod tests {
 
                nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-               reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
                nodes[1].node.channel_state.lock().unwrap().next_forward = Instant::now();
                nodes[1].node.process_pending_htlc_forwards();
@@ -6518,7 +6574,7 @@ mod tests {
                nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
                if messages_delivered < 2 {
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (1, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (1, 0), (0, 0), (0, 0), (false, false));
                        //TODO: Deduplicate PaymentSent events, then enable this if:
                        //if messages_delivered < 1 {
                                let events_4 = nodes[0].node.get_and_clear_pending_events();
@@ -6532,21 +6588,21 @@ mod tests {
                        //}
                } else if messages_delivered == 2 {
                        // nodes[0] still wants its RAA + commitment_signed
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, -1), (0, 0), (0, 0), (0, 0), (false, true));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, -1), (0, 0), (0, 0), (0, 0), (false, true));
                } else if messages_delivered == 3 {
                        // nodes[0] still wants its commitment_signed
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, -1), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, -1), (0, 0), (0, 0), (0, 0), (false, false));
                } else if messages_delivered == 4 {
                        // nodes[1] still wants its final RAA
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (true, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (true, false));
                } else if messages_delivered == 5 {
                        // Everything was delivered...
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
                }
 
                nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-               reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
                // Channel should still work fine...
                let payment_preimage_2 = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0;
@@ -6587,20 +6643,28 @@ mod tests {
                        _ => panic!("Unexpected event"),
                }
 
+               reconnect_nodes(&nodes[0], &nodes[1], (false, true), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+
+               nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+               nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
+
                confirm_transaction(&nodes[1].chain_monitor, &tx, tx.version);
                let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
-               assert_eq!(events_2.len(), 1);
+               assert_eq!(events_2.len(), 2);
                match events_2[0] {
                        MessageSendEvent::SendFundingLocked { ref node_id, msg: _ } => {
                                assert_eq!(*node_id, nodes[0].node.get_our_node_id());
                        },
                        _ => panic!("Unexpected event"),
                }
+               match events_2[1] {
+                       MessageSendEvent::SendAnnouncementSignatures { ref node_id, msg: _ } => {
+                               assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
 
-               reconnect_nodes(&nodes[0], &nodes[1], true, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
-               nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
-               nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-               reconnect_nodes(&nodes[0], &nodes[1], true, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
                // TODO: We shouldn't need to manually pass list_usable_chanels here once we support
                // rebroadcasting announcement_signatures upon reconnect.
@@ -6803,7 +6867,7 @@ mod tests {
                if disconnect {
                        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-                       reconnect_nodes(&nodes[0], &nodes[1], true, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
                }
 
                *nodes[0].chan_monitor.update_ret.lock().unwrap() = Ok(());
@@ -6844,7 +6908,7 @@ mod tests {
                if disconnect {
                        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-                       reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+                       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
                }
 
                // ...and make sure we can force-close a TemporaryFailure channel with a PermanentFailure
@@ -7404,7 +7468,7 @@ mod tests {
                nodes[0].node = Arc::new(nodes_0_deserialized);
                check_added_monitors!(nodes[0], 1);
 
-               reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
                fail_payment(&nodes[0], &[&nodes[1]], our_payment_hash);
                claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage);
@@ -7474,8 +7538,8 @@ mod tests {
                nodes[0].node = Arc::new(nodes_0_deserialized);
 
                // nodes[1] and nodes[2] have no lost state with nodes[0]...
-               reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
-               reconnect_nodes(&nodes[0], &nodes[2], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+               reconnect_nodes(&nodes[0], &nodes[2], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
                //... and we can even still claim the payment!
                claim_payment(&nodes[2], &[&nodes[0], &nodes[1]], our_payment_preimage);