Merge pull request #266 from TheBlueMatt/2018-12-closing_signed-3-leg-commitment

Remove check which makes us sometimes never send closing_signed

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index c2ec53fb353d8fad52d1e6d8d5074ed361546752..cbcec942d80661dafb83e13f98826dfc2cceb2d1 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -2410,8 +2410,7 @@ impl Channel {
        fn maybe_propose_first_closing_signed(&mut self, fee_estimator: &FeeEstimator) -> Option<msgs::ClosingSigned> {
                if !self.channel_outbound || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() ||
                                self.channel_state & (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32) != BOTH_SIDES_SHUTDOWN_MASK ||
-                               self.last_sent_closing_fee.is_some() ||
-                               self.cur_remote_commitment_transaction_number != self.cur_local_commitment_transaction_number{
+                               self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() {
                        return None;
                }
 

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 44dee8d4bb4c65af5804ac3a9c3ea46ac7a8c628..e0d2b9aaaa37222a17f5b134a994bf1080ed4f2b 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -5001,52 +5001,6 @@ mod tests {
                assert!(nodes[2].node.list_channels().is_empty());
        }
 
-       #[test]
-       fn update_fee_async_shutdown() {
-               // Test update_fee works after shutdown start if messages are delivered out-of-order
-               let nodes = create_network(2);
-               let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
-
-               let starting_feerate = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().get_feerate();
-               nodes[0].node.update_fee(chan_1.2.clone(), starting_feerate + 20).unwrap();
-               check_added_monitors!(nodes[0], 1);
-               let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
-               assert!(updates.update_add_htlcs.is_empty());
-               assert!(updates.update_fulfill_htlcs.is_empty());
-               assert!(updates.update_fail_htlcs.is_empty());
-               assert!(updates.update_fail_malformed_htlcs.is_empty());
-               assert!(updates.update_fee.is_some());
-
-               nodes[1].node.close_channel(&chan_1.2).unwrap();
-               let node_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id());
-               nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &node_1_shutdown).unwrap();
-               // Note that we don't actually test normative behavior here. The spec indicates we could
-               // actually send a closing_signed here, but is kinda unclear and could possibly be amended
-               // to require waiting on the full commitment dance before doing so (see
-               // https://github.com/lightningnetwork/lightning-rfc/issues/499). In any case, to avoid
-               // ambiguity, we should wait until after the full commitment dance to send closing_signed.
-               let node_0_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
-
-               nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &updates.update_fee.unwrap()).unwrap();
-               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &updates.commitment_signed).unwrap();
-               check_added_monitors!(nodes[1], 1);
-               nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &node_0_shutdown).unwrap();
-               let node_0_closing_signed = commitment_signed_dance!(nodes[1], nodes[0], (), false, true, true);
-
-               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
-               nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), match node_0_closing_signed.unwrap() {
-                       MessageSendEvent::SendClosingSigned { ref node_id, ref msg } => {
-                               assert_eq!(*node_id, nodes[1].node.get_our_node_id());
-                               msg
-                       },
-                       _ => panic!("Unexpected event"),
-               }).unwrap();
-               let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-               nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed.unwrap()).unwrap();
-               let (_, node_0_none) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
-               assert!(node_0_none.is_none());
-       }
-
        fn do_test_shutdown_rebroadcast(recv_count: u8) {
                // Test that shutdown/closing_signed is re-sent on reconnect with a variable number of
                // messages delivered prior to disconnect