Add a test for HTLC freeing on monitor update restoration
authorMatt Corallo <git@bluematt.me>
Mon, 1 Mar 2021 02:00:46 +0000 (21:00 -0500)
committerMatt Corallo <git@bluematt.me>
Fri, 21 May 2021 15:10:45 +0000 (15:10 +0000)
lightning/src/ln/chanmon_update_fail_tests.rs

index d1da9f7ef3090300284152820d037b27ab8e8ea0..79bbfadc7687372e63d9c738f45c55b18c8aa64d 100644 (file)
@@ -20,11 +20,12 @@ use chain::transaction::OutPoint;
 use chain::Listen;
 use chain::Watch;
 use ln::{PaymentPreimage, PaymentHash};
-use ln::channelmanager::{RAACommitmentOrder, PaymentSendFailure};
+use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
 use ln::features::{InitFeatures, InvoiceFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler, ErrorAction, RoutingMessageHandler};
 use routing::router::get_route;
+use util::config::UserConfig;
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
 use util::errors::APIError;
@@ -37,6 +38,8 @@ use ln::functional_test_utils::*;
 
 use util::test_utils;
 
+use std::collections::HashMap;
+
 // If persister_fail is true, we have the persister return a PermanentFailure
 // instead of the higher-level ChainMonitor.
 fn do_test_simple_monitor_permanent_update_fail(persister_fail: bool) {
@@ -1971,3 +1974,202 @@ fn test_path_paused_mpp() {
 
        claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
 }
+
+fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
+       // Tests that, when we serialize a channel with AddHTLC entries in the holding cell, we
+       // properly free them on reconnect. We previously failed such HTLCs upon serialization, but
+       // that behavior was both somewhat unexpected and also broken (there was a debug assertion
+       // which failed in such a case).
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let persister: test_utils::TestPersister;
+       let new_chain_monitor: test_utils::TestChainMonitor;
+       let nodes_0_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       let chan_id = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 15_000_000, 7_000_000_000, InitFeatures::known(), InitFeatures::known()).2;
+       let (payment_preimage_1, payment_hash_1, payment_secret_1) = get_payment_preimage_hash!(&nodes[1]);
+       let (payment_preimage_2, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(&nodes[1]);
+
+       // Do a really complicated dance to get an HTLC into the holding cell, with MonitorUpdateFailed
+       // set but AwaitingRemoteRevoke unset. When this test was written, any attempts to send an HTLC
+       // while MonitorUpdateFailed is set are immediately failed-backwards. Thus, the only way to get
+       // an AddHTLC into the holding cell is to add it while AwaitingRemoteRevoke is set but
+       // MonitorUpdateFailed is unset, and then swap the flags.
+       //
+       // We do this by:
+       //  a) routing a payment from node B to node A,
+       //  b) sending a payment from node A to node B without delivering any of the generated messages,
+       //     putting node A in AwaitingRemoteRevoke,
+       //  c) sending a second payment from node A to node B, which is immediately placed in the
+       //     holding cell,
+       //  d) claiming the first payment from B, allowing us to fail the monitor update which occurs
+       //     when we try to persist the payment preimage,
+       //  e) delivering A's commitment_signed from (b) and the resulting B revoke_and_ack message,
+       //     clearing AwaitingRemoteRevoke on node A.
+       //
+       // Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
+       // will not be freed from the holding cell.
+       let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
+
+       let route = {
+               let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+               get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), None, None, &Vec::new(), 100000, TEST_FINAL_CLTV, nodes[0].logger).unwrap()
+       };
+
+       nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let send = SendEvent::from_node(&nodes[0]);
+       assert_eq!(send.msgs.len(), 1);
+
+       nodes[0].node.send_payment(&route, payment_hash_2, &Some(payment_secret_2)).unwrap();
+       check_added_monitors!(nodes[0], 0);
+
+       *nodes[0].chain_monitor.update_ret.lock().unwrap() = Some(Err(ChannelMonitorUpdateErr::TemporaryFailure));
+       assert!(nodes[0].node.claim_funds(payment_preimage_0));
+       check_added_monitors!(nodes[0], 1);
+
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
+       nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
+       check_added_monitors!(nodes[1], 1);
+
+       let (raa, cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+       nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &raa);
+       check_added_monitors!(nodes[0], 1);
+
+       if disconnect {
+               // Optionally reload nodes[0] entirely through a serialization roundtrip, otherwise just
+               // disconnect the peers. Note that the fuzzer originally found this issue because
+               // deserializing a ChannelManager in this state causes an assertion failure.
+               if reload_a {
+                       let nodes_0_serialized = nodes[0].node.encode();
+                       let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
+                       nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
+
+                       persister = test_utils::TestPersister::new();
+                       let keys_manager = &chanmon_cfgs[0].keys_manager;
+                       new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), nodes[0].logger, node_cfgs[0].fee_estimator, &persister, keys_manager);
+                       nodes[0].chain_monitor = &new_chain_monitor;
+                       let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..];
+                       let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
+                               &mut chan_0_monitor_read, keys_manager).unwrap();
+                       assert!(chan_0_monitor_read.is_empty());
+
+                       let mut nodes_0_read = &nodes_0_serialized[..];
+                       let config = UserConfig::default();
+                       nodes_0_deserialized = {
+                               let mut channel_monitors = HashMap::new();
+                               channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor);
+                               <(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+                                       default_config: config,
+                                       keys_manager,
+                                       fee_estimator: node_cfgs[0].fee_estimator,
+                                       chain_monitor: nodes[0].chain_monitor,
+                                       tx_broadcaster: nodes[0].tx_broadcaster.clone(),
+                                       logger: nodes[0].logger,
+                                       channel_monitors,
+                               }).unwrap().1
+                       };
+                       nodes[0].node = &nodes_0_deserialized;
+                       assert!(nodes_0_read.is_empty());
+
+                       nodes[0].chain_monitor.watch_channel(chan_0_monitor.get_funding_txo().0.clone(), chan_0_monitor).unwrap();
+                       check_added_monitors!(nodes[0], 1);
+               } else {
+                       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);
+
+               // Now reconnect the two
+               nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+               let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
+               assert_eq!(reestablish_1.len(), 1);
+               nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+               let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+               assert_eq!(reestablish_2.len(), 1);
+
+               nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
+               let resp_1 = handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
+               check_added_monitors!(nodes[1], 0);
+
+               nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]);
+               let resp_0 = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+
+               assert!(resp_0.0.is_none());
+               assert!(resp_0.1.is_none());
+               assert!(resp_0.2.is_none());
+               assert!(resp_1.0.is_none());
+               assert!(resp_1.1.is_none());
+
+               // Check that the freshly-generated cs is equal to the original (which we will deliver in a
+               // moment).
+               if let Some(pending_cs) = resp_1.2 {
+                       assert!(pending_cs.update_add_htlcs.is_empty());
+                       assert!(pending_cs.update_fail_htlcs.is_empty());
+                       assert!(pending_cs.update_fulfill_htlcs.is_empty());
+                       assert_eq!(pending_cs.commitment_signed, cs);
+               } else { panic!(); }
+
+               // There should be no monitor updates as we are still pending awaiting a failed one.
+               check_added_monitors!(nodes[0], 0);
+               check_added_monitors!(nodes[1], 0);
+       }
+
+       // If we finish updating the monitor, we should free the holding cell right away (this did
+       // not occur prior to #756).
+       *nodes[0].chain_monitor.update_ret.lock().unwrap() = None;
+       let (funding_txo, mon_id) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id).unwrap().clone();
+       nodes[0].node.channel_monitor_updated(&funding_txo, mon_id);
+
+       // New outbound messages should be generated immediately upon a call to
+       // get_and_clear_pending_msg_events (but not before).
+       check_added_monitors!(nodes[0], 0);
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       check_added_monitors!(nodes[0], 1);
+       assert_eq!(events.len(), 1);
+
+       // Deliver the pending in-flight CS
+       nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &cs);
+       check_added_monitors!(nodes[0], 1);
+
+       let commitment_msg = match events.pop().unwrap() {
+               MessageSendEvent::UpdateHTLCs { node_id, updates } => {
+                       assert_eq!(node_id, nodes[1].node.get_our_node_id());
+                       assert!(updates.update_fail_htlcs.is_empty());
+                       assert!(updates.update_fail_malformed_htlcs.is_empty());
+                       assert!(updates.update_fee.is_none());
+                       assert_eq!(updates.update_fulfill_htlcs.len(), 1);
+                       nodes[1].node.handle_update_fulfill_htlc(&nodes[0].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
+                       expect_payment_sent!(nodes[1], payment_preimage_0);
+                       assert_eq!(updates.update_add_htlcs.len(), 1);
+                       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+                       updates.commitment_signed
+               },
+               _ => panic!("Unexpected event type!"),
+       };
+
+       nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commitment_msg);
+       check_added_monitors!(nodes[1], 1);
+
+       let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack);
+       expect_pending_htlcs_forwardable!(nodes[1]);
+       expect_payment_received!(nodes[1], payment_hash_1, payment_secret_1, 100000);
+       check_added_monitors!(nodes[1], 1);
+
+       commitment_signed_dance!(nodes[1], nodes[0], (), false, true, false);
+
+       expect_pending_htlcs_forwardable!(nodes[1]);
+       expect_payment_received!(nodes[1], payment_hash_2, payment_secret_2, 100000);
+
+       claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
+       claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
+}
+#[test]
+fn channel_holding_cell_serialize() {
+       do_channel_holding_cell_serialize(true, true);
+       do_channel_holding_cell_serialize(true, false);
+       do_channel_holding_cell_serialize(false, true); // last arg doesn't matter
+}