Merge pull request #1429 from TheBlueMatt/2022-04-drop-no-conn-possible
[rust-lightning] / lightning / src / ln / functional_tests.rs
index 6cc0c45bd0180b74d86b11d20c348d196d323117..3961b70773bf3c15c8d51c5335c16fbd18489250 100644 (file)
@@ -26,7 +26,7 @@ use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputI
 use routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, find_route, get_route};
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
+use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, OptionalField, ErrorAction};
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::{byte_utils, test_utils};
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason};
@@ -42,7 +42,7 @@ use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::network::constants::Network;
 
 use bitcoin::secp256k1::Secp256k1;
-use bitcoin::secp256k1::key::{PublicKey,SecretKey};
+use bitcoin::secp256k1::{PublicKey,SecretKey};
 
 use regex;
 
@@ -58,9 +58,12 @@ use ln::chan_utils::CommitmentTransaction;
 #[test]
 fn test_insane_channel_opens() {
        // Stand up a network of 2 nodes
+       use ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS;
+       let mut cfg = UserConfig::default();
+       cfg.peer_channel_config_limits.max_funding_satoshis = TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1;
        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 node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(cfg)]);
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
        // Instantiate channel parameters where we push the maximum msats given our
@@ -92,15 +95,15 @@ fn test_insane_channel_opens() {
                } else { assert!(false); }
        };
 
-       use ln::channel::MAX_FUNDING_SATOSHIS;
        use ln::channelmanager::MAX_LOCAL_BREAKDOWN_TIMEOUT;
 
        // Test all mutations that would make the channel open message insane
-       insane_open_helper(format!("Funding must be smaller than {}. It was {}", MAX_FUNDING_SATOSHIS, MAX_FUNDING_SATOSHIS).as_str(), |mut msg| { msg.funding_satoshis = MAX_FUNDING_SATOSHIS; msg });
+       insane_open_helper(format!("Per our config, funding must be at most {}. It was {}", TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, TOTAL_BITCOIN_SUPPLY_SATOSHIS + 2).as_str(), |mut msg| { msg.funding_satoshis = TOTAL_BITCOIN_SUPPLY_SATOSHIS + 2; msg });
+       insane_open_helper(format!("Funding must be smaller than the total bitcoin supply. It was {}", TOTAL_BITCOIN_SUPPLY_SATOSHIS).as_str(), |mut msg| { msg.funding_satoshis = TOTAL_BITCOIN_SUPPLY_SATOSHIS; msg });
 
        insane_open_helper("Bogus channel_reserve_satoshis", |mut msg| { msg.channel_reserve_satoshis = msg.funding_satoshis + 1; msg });
 
-       insane_open_helper(r"push_msat \d+ was larger than funding value \d+", |mut msg| { msg.push_msat = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 + 1; msg });
+       insane_open_helper(r"push_msat \d+ was larger than channel amount minus reserve \(\d+\)", |mut msg| { msg.push_msat = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 + 1; msg });
 
        insane_open_helper("Peer never wants payout outputs?", |mut msg| { msg.dust_limit_satoshis = msg.funding_satoshis + 1 ; msg });
 
@@ -113,6 +116,25 @@ fn test_insane_channel_opens() {
        insane_open_helper("max_accepted_htlcs was 484. It must not be larger than 483", |mut msg| { msg.max_accepted_htlcs = 484; msg });
 }
 
+#[test]
+fn test_funding_exceeds_no_wumbo_limit() {
+       // Test that if a peer does not support wumbo channels, we'll refuse to open a wumbo channel to
+       // them.
+       use ln::channel::MAX_FUNDING_SATOSHIS_NO_WUMBO;
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       node_cfgs[1].features = InitFeatures::known().clear_wumbo();
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       match nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), MAX_FUNDING_SATOSHIS_NO_WUMBO + 1, 0, 42, None) {
+               Err(APIError::APIMisuseError { err }) => {
+                       assert_eq!(format!("funding_value must not exceed {}, it was {}", MAX_FUNDING_SATOSHIS_NO_WUMBO, MAX_FUNDING_SATOSHIS_NO_WUMBO + 1), err);
+               },
+               _ => panic!()
+       }
+}
+
 fn do_test_counterparty_no_reserve(send_from_initiator: bool) {
        // A peer providing a channel_reserve_satoshis of 0 (or less than our dust limit) is insecure,
        // but only for them. Because some LSPs do it with some level of trust of the clients (for a
@@ -3467,6 +3489,47 @@ fn test_dup_events_on_peer_disconnect() {
        expect_payment_path_successful!(nodes[0]);
 }
 
+#[test]
+fn test_peer_disconnected_before_funding_broadcasted() {
+       // Test that channels are closed with `ClosureReason::DisconnectedPeer` if the peer disconnects
+       // before the funding transaction has been broadcasted.
+       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 nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       // Open a channel between `nodes[0]` and `nodes[1]`, for which the funding transaction is never
+       // broadcasted, even though it's created by `nodes[0]`.
+       let expected_temporary_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
+       let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel);
+       let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel);
+
+       let (temporary_channel_id, tx, _funding_output) = create_funding_transaction(&nodes[0], 1_000_000, 42);
+       assert_eq!(temporary_channel_id, expected_temporary_channel_id);
+
+       assert!(nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).is_ok());
+
+       let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
+       assert_eq!(funding_created_msg.temporary_channel_id, expected_temporary_channel_id);
+
+       // Even though the funding transaction is created by `nodes[0]`, the `FundingCreated` msg is
+       // never sent to `nodes[1]`, and therefore the tx is never signed by either party nor
+       // broadcasted.
+       {
+               assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 0);
+       }
+
+       // Ensure that the channel is closed with `ClosureReason::DisconnectedPeer` when the peers are
+       // disconnected before the funding transaction was broadcasted.
+       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);
+
+       check_closed_event!(nodes[0], 1, ClosureReason::DisconnectedPeer);
+       check_closed_event!(nodes[1], 1, ClosureReason::DisconnectedPeer);
+}
+
 #[test]
 fn test_simple_peer_disconnect() {
        // Test that we can reconnect when there are no lost messages
@@ -5724,8 +5787,8 @@ fn test_key_derivation_params() {
        check_spends!(local_txn_1[0], chan_1.3);
 
        // We check funding pubkey are unique
-       let (from_0_funding_key_0, from_0_funding_key_1) = (PublicKey::from_slice(&local_txn_0[0].input[0].witness[3][2..35]), PublicKey::from_slice(&local_txn_0[0].input[0].witness[3][36..69]));
-       let (from_1_funding_key_0, from_1_funding_key_1) = (PublicKey::from_slice(&local_txn_1[0].input[0].witness[3][2..35]), PublicKey::from_slice(&local_txn_1[0].input[0].witness[3][36..69]));
+       let (from_0_funding_key_0, from_0_funding_key_1) = (PublicKey::from_slice(&local_txn_0[0].input[0].witness.to_vec()[3][2..35]), PublicKey::from_slice(&local_txn_0[0].input[0].witness.to_vec()[3][36..69]));
+       let (from_1_funding_key_0, from_1_funding_key_1) = (PublicKey::from_slice(&local_txn_1[0].input[0].witness.to_vec()[3][2..35]), PublicKey::from_slice(&local_txn_1[0].input[0].witness.to_vec()[3][36..69]));
        if from_0_funding_key_0 == from_1_funding_key_0
            || from_0_funding_key_0 == from_1_funding_key_1
            || from_0_funding_key_1 == from_1_funding_key_0
@@ -7323,7 +7386,7 @@ fn test_data_loss_protect() {
        logger = test_utils::TestLogger::with_id(format!("node {}", 0));
        let mut chain_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut io::Cursor::new(previous_chain_monitor_state.0), keys_manager).unwrap().1;
        chain_source = test_utils::TestChainSource::new(Network::Testnet);
-       tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
+       tx_broadcaster = test_utils::TestBroadcaster { txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new())) };
        fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
        persister = test_utils::TestPersister::new();
        monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &tx_broadcaster, &logger, &fee_estimator, &persister, keys_manager);
@@ -7380,22 +7443,48 @@ fn test_data_loss_protect() {
        }
 
        // Check we close channel detecting A is fallen-behind
+       // Check that we sent the warning message when we detected that A has fallen behind,
+       // and give the possibility for A to recover from the warning.
        nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
-       check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "Peer attempted to reestablish channel with a very old local commitment transaction".to_string() });
-       assert_eq!(check_closed_broadcast!(nodes[1], true).unwrap().data, "Peer attempted to reestablish channel with a very old local commitment transaction");
-       check_added_monitors!(nodes[1], 1);
+       let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction".to_owned();
+       assert!(check_warn_msg!(nodes[1], nodes[0].node.get_our_node_id(), chan.2).contains(&warn_msg));
 
        // Check A is able to claim to_remote output
-       let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
-       assert_eq!(node_txn.len(), 1);
-       check_spends!(node_txn[0], chan.3);
-       assert_eq!(node_txn[0].output.len(), 2);
-       mine_transaction(&nodes[0], &node_txn[0]);
-       connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
-       check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can\'t do any automated broadcasting".to_string() });
-       let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
-       assert_eq!(spend_txn.len(), 1);
-       check_spends!(spend_txn[0], node_txn[0]);
+       let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
+       // The node B should not broadcast the transaction to force close the channel!
+       assert!(node_txn.is_empty());
+       // B should now detect that there is something wrong and should force close the channel.
+       let exp_err = "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can\'t do any automated broadcasting";
+       check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: exp_err.to_string() });
+
+       // after the warning message sent by B, we should not able to
+       // use the channel, or reconnect with success to the channel.
+       assert!(nodes[0].node.list_usable_channels().is_empty());
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
+       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
+       let retry_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+
+       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &retry_reestablish[0]);
+       let mut err_msgs_0 = Vec::with_capacity(1);
+       for msg in nodes[0].node.get_and_clear_pending_msg_events() {
+               if let MessageSendEvent::HandleError { ref action, .. } = msg {
+                       match action {
+                               &ErrorAction::SendErrorMessage { ref msg } => {
+                                       assert_eq!(msg.data, "Failed to find corresponding channel");
+                                       err_msgs_0.push(msg.clone());
+                               },
+                               _ => panic!("Unexpected event!"),
+                       }
+               } else {
+                       panic!("Unexpected event!");
+               }
+       }
+       assert_eq!(err_msgs_0.len(), 1);
+       nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &err_msgs_0[0]);
+       assert!(nodes[1].node.list_usable_channels().is_empty());
+       check_added_monitors!(nodes[1], 1);
+       check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: "Failed to find corresponding channel".to_owned() });
+       check_closed_broadcast!(nodes[1], false);
 }
 
 #[test]
@@ -7593,7 +7682,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() {
                assert_eq!(node_txn[0].output.len(), 1);
                check_spends!(node_txn[0], revoked_txn[0]);
                let fee_1 = penalty_sum - node_txn[0].output[0].value;
-               feerate_1 = fee_1 * 1000 / node_txn[0].get_weight() as u64;
+               feerate_1 = fee_1 * 1000 / node_txn[0].weight() as u64;
                penalty_1 = node_txn[0].txid();
                node_txn.clear();
        };
@@ -7613,7 +7702,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() {
                        // Verify new bumped tx is different from last claiming transaction, we don't want spurrious rebroadcast
                        assert_ne!(penalty_2, penalty_1);
                        let fee_2 = penalty_sum - node_txn[0].output[0].value;
-                       feerate_2 = fee_2 * 1000 / node_txn[0].get_weight() as u64;
+                       feerate_2 = fee_2 * 1000 / node_txn[0].weight() as u64;
                        // Verify 25% bump heuristic
                        assert!(feerate_2 * 100 >= feerate_1 * 125);
                        node_txn.clear();
@@ -7636,7 +7725,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() {
                        // Verify new bumped tx is different from last claiming transaction, we don't want spurrious rebroadcast
                        assert_ne!(penalty_3, penalty_2);
                        let fee_3 = penalty_sum - node_txn[0].output[0].value;
-                       feerate_3 = fee_3 * 1000 / node_txn[0].get_weight() as u64;
+                       feerate_3 = fee_3 * 1000 / node_txn[0].weight() as u64;
                        // Verify 25% bump heuristic
                        assert!(feerate_3 * 100 >= feerate_2 * 125);
                        node_txn.clear();
@@ -7755,7 +7844,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                first = node_txn[4].txid();
                // Store both feerates for later comparison
                let fee_1 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[2].output[0].value - node_txn[4].output[0].value;
-               feerate_1 = fee_1 * 1000 / node_txn[4].get_weight() as u64;
+               feerate_1 = fee_1 * 1000 / node_txn[4].weight() as u64;
                penalty_txn = vec![node_txn[2].clone()];
                node_txn.clear();
        }
@@ -7795,7 +7884,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                // Verify bumped tx is different and 25% bump heuristic
                assert_ne!(first, node_txn[0].txid());
                let fee_2 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[2].output[0].value - node_txn[0].output[0].value;
-               let feerate_2 = fee_2 * 1000 / node_txn[0].get_weight() as u64;
+               let feerate_2 = fee_2 * 1000 / node_txn[0].weight() as u64;
                assert!(feerate_2 * 100 > feerate_1 * 125);
                let txn = vec![node_txn[0].clone()];
                node_txn.clear();
@@ -7879,12 +7968,12 @@ fn test_bump_penalty_txn_on_remote_commitment() {
                timeout = node_txn[6].txid();
                let index = node_txn[6].input[0].previous_output.vout;
                let fee = remote_txn[0].output[index as usize].value - node_txn[6].output[0].value;
-               feerate_timeout = fee * 1000 / node_txn[6].get_weight() as u64;
+               feerate_timeout = fee * 1000 / node_txn[6].weight() as u64;
 
                preimage = node_txn[0].txid();
                let index = node_txn[0].input[0].previous_output.vout;
                let fee = remote_txn[0].output[index as usize].value - node_txn[0].output[0].value;
-               feerate_preimage = fee * 1000 / node_txn[0].get_weight() as u64;
+               feerate_preimage = fee * 1000 / node_txn[0].weight() as u64;
 
                node_txn.clear();
        };
@@ -7903,13 +7992,13 @@ fn test_bump_penalty_txn_on_remote_commitment() {
 
                let index = preimage_bump.input[0].previous_output.vout;
                let fee = remote_txn[0].output[index as usize].value - preimage_bump.output[0].value;
-               let new_feerate = fee * 1000 / preimage_bump.get_weight() as u64;
+               let new_feerate = fee * 1000 / preimage_bump.weight() as u64;
                assert!(new_feerate * 100 > feerate_timeout * 125);
                assert_ne!(timeout, preimage_bump.txid());
 
                let index = node_txn[0].input[0].previous_output.vout;
                let fee = remote_txn[0].output[index as usize].value - node_txn[0].output[0].value;
-               let new_feerate = fee * 1000 / node_txn[0].get_weight() as u64;
+               let new_feerate = fee * 1000 / node_txn[0].weight() as u64;
                assert!(new_feerate * 100 > feerate_preimage * 125);
                assert_ne!(preimage, node_txn[0].txid());
 
@@ -8121,6 +8210,58 @@ fn test_override_0msat_htlc_minimum() {
        assert_eq!(res.htlc_minimum_msat, 1);
 }
 
+#[test]
+fn test_channel_update_has_correct_htlc_maximum_msat() {
+       // Tests that the `ChannelUpdate` message has the correct values for `htlc_maximum_msat` set.
+       // Bolt 7 specifies that if present `htlc_maximum_msat`:
+       // 1. MUST be set to less than or equal to the channel capacity. In LDK, this is capped to
+       // 90% of the `channel_value`.
+       // 2. MUST be set to less than or equal to the `max_htlc_value_in_flight_msat` received from the peer.
+
+       let mut config_30_percent = UserConfig::default();
+       config_30_percent.channel_options.announced_channel = true;
+       config_30_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 30;
+       let mut config_50_percent = UserConfig::default();
+       config_50_percent.channel_options.announced_channel = true;
+       config_50_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 50;
+       let mut config_95_percent = UserConfig::default();
+       config_95_percent.channel_options.announced_channel = true;
+       config_95_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 95;
+       let mut config_100_percent = UserConfig::default();
+       config_100_percent.channel_options.announced_channel = true;
+       config_100_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
+
+       let chanmon_cfgs = create_chanmon_cfgs(4);
+       let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[Some(config_30_percent), Some(config_50_percent), Some(config_95_percent), Some(config_100_percent)]);
+       let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+       let channel_value_satoshis = 100000;
+       let channel_value_msat = channel_value_satoshis * 1000;
+       let channel_value_30_percent_msat = (channel_value_msat as f64 * 0.3) as u64;
+       let channel_value_50_percent_msat = (channel_value_msat as f64 * 0.5) as u64;
+       let channel_value_90_percent_msat = (channel_value_msat as f64 * 0.9) as u64;
+
+       let (node_0_chan_update, node_1_chan_update, _, _)  = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, channel_value_satoshis, 10001, InitFeatures::known(), InitFeatures::known());
+       let (node_2_chan_update, node_3_chan_update, _, _)  = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, channel_value_satoshis, 10001, InitFeatures::known(), InitFeatures::known());
+
+       // Assert that `node[0]`'s `ChannelUpdate` is capped at 50 percent of the `channel_value`, as
+       // that's the value of `node[1]`'s `holder_max_htlc_value_in_flight_msat`.
+       assert_eq!(node_0_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_50_percent_msat));
+       // Assert that `node[1]`'s `ChannelUpdate` is capped at 30 percent of the `channel_value`, as
+       // that's the value of `node[0]`'s `holder_max_htlc_value_in_flight_msat`.
+       assert_eq!(node_1_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_30_percent_msat));
+
+       // Assert that `node[2]`'s `ChannelUpdate` is capped at 90 percent of the `channel_value`, as
+       // the value of `node[3]`'s `holder_max_htlc_value_in_flight_msat` (100%), exceeds 90% of the
+       // `channel_value`.
+       assert_eq!(node_2_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_90_percent_msat));
+       // Assert that `node[3]`'s `ChannelUpdate` is capped at 90 percent of the `channel_value`, as
+       // the value of `node[2]`'s `holder_max_htlc_value_in_flight_msat` (95%), exceeds 90% of the
+       // `channel_value`.
+       assert_eq!(node_3_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_90_percent_msat));
+}
+
 #[test]
 fn test_manually_accept_inbound_channel_request() {
        let mut manually_accept_conf = UserConfig::default();
@@ -9435,12 +9576,7 @@ fn test_forwardable_regen() {
        claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage_2);
 }
 
-#[test]
-fn test_dup_htlc_second_fail_panic() {
-       // Previously, if we received two HTLCs back-to-back, where the second overran the expected
-       // value for the payment, we'd fail back both HTLCs after generating a `PaymentReceived` event.
-       // Then, if the user failed the second payment, they'd hit a "tried to fail an already failed
-       // HTLC" debug panic. This tests for this behavior, checking that only one HTLC is auto-failed.
+fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) {
        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]);
@@ -9450,14 +9586,9 @@ fn test_dup_htlc_second_fail_panic() {
 
        let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
                .with_features(InvoiceFeatures::known());
-       let scorer = test_utils::TestScorer::with_penalty(0);
-       let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
-       let route = get_route(
-               &nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(),
-               Some(&nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
-               10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
+       let route = get_route!(nodes[0], payment_params, 10_000, TEST_FINAL_CLTV).unwrap();
 
-       let (_, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[1]);
+       let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[1]);
 
        {
                nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
@@ -9485,26 +9616,153 @@ fn test_dup_htlc_second_fail_panic() {
                // the first HTLC delivered above.
        }
 
-       // Now we go fail back the first HTLC from the user end.
        expect_pending_htlcs_forwardable_ignore!(nodes[1]);
        nodes[1].node.process_pending_htlc_forwards();
-       nodes[1].node.fail_htlc_backwards(&our_payment_hash);
 
-       expect_pending_htlcs_forwardable_ignore!(nodes[1]);
-       nodes[1].node.process_pending_htlc_forwards();
+       if test_for_second_fail_panic {
+               // Now we go fail back the first HTLC from the user end.
+               nodes[1].node.fail_htlc_backwards(&our_payment_hash);
 
-       check_added_monitors!(nodes[1], 1);
-       let fail_updates_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
-       assert_eq!(fail_updates_1.update_fail_htlcs.len(), 2);
+               expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+               nodes[1].node.process_pending_htlc_forwards();
+
+               check_added_monitors!(nodes[1], 1);
+               let fail_updates_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               assert_eq!(fail_updates_1.update_fail_htlcs.len(), 2);
+
+               nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
+               nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[1]);
+               commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);
 
-       nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
-       nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[1]);
-       commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);
+               let failure_events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(failure_events.len(), 2);
+               if let Event::PaymentPathFailed { .. } = failure_events[0] {} else { panic!(); }
+               if let Event::PaymentPathFailed { .. } = failure_events[1] {} else { panic!(); }
+       } else {
+               // Let the second HTLC fail and claim the first
+               expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+               nodes[1].node.process_pending_htlc_forwards();
+
+               check_added_monitors!(nodes[1], 1);
+               let fail_updates_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
+               commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);
+
+               expect_payment_failed_conditions!(nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());
+
+               claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage);
+       }
+}
+
+#[test]
+fn test_dup_htlc_second_fail_panic() {
+       // Previously, if we received two HTLCs back-to-back, where the second overran the expected
+       // value for the payment, we'd fail back both HTLCs after generating a `PaymentReceived` event.
+       // Then, if the user failed the second payment, they'd hit a "tried to fail an already failed
+       // HTLC" debug panic. This tests for this behavior, checking that only one HTLC is auto-failed.
+       do_test_dup_htlc_second_rejected(true);
+}
+
+#[test]
+fn test_dup_htlc_second_rejected() {
+       // Test that if we receive a second HTLC for an MPP payment that overruns the payment amount we
+       // simply reject the second HTLC but are still able to claim the first HTLC.
+       do_test_dup_htlc_second_rejected(false);
+}
+
+#[test]
+fn test_inconsistent_mpp_params() {
+       // Test that if we recieve two HTLCs with different payment parameters we fail back the first
+       // such HTLC and allow the second to stay.
+       let chanmon_cfgs = create_chanmon_cfgs(4);
+       let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+       let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+       create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0, InitFeatures::known(), InitFeatures::known());
+       create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 100_000, 0, InitFeatures::known(), InitFeatures::known());
+       create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0, InitFeatures::known(), InitFeatures::known());
+       create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0, InitFeatures::known(), InitFeatures::known());
+
+       let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id())
+               .with_features(InvoiceFeatures::known());
+       let mut route = get_route!(nodes[0], payment_params, 15_000_000, TEST_FINAL_CLTV).unwrap();
+       assert_eq!(route.paths.len(), 2);
+       route.paths.sort_by(|path_a, _| {
+               // Sort the path so that the path through nodes[1] comes first
+               if path_a[0].pubkey == nodes[1].node.get_our_node_id() {
+                       core::cmp::Ordering::Less } else { core::cmp::Ordering::Greater }
+       });
+       let payment_params_opt = Some(payment_params);
+
+       let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[3]);
+
+       let cur_height = nodes[0].best_block_info().1;
+       let payment_id = PaymentId([42; 32]);
+       {
+               nodes[0].node.send_payment_along_path(&route.paths[0], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None).unwrap();
+               check_added_monitors!(nodes[0], 1);
+
+               let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+               assert_eq!(events.len(), 1);
+               pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), false, None);
+       }
+       assert!(nodes[3].node.get_and_clear_pending_events().is_empty());
+
+       {
+               nodes[0].node.send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 14_000_000, cur_height, payment_id, &None).unwrap();
+               check_added_monitors!(nodes[0], 1);
+
+               let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+               assert_eq!(events.len(), 1);
+               let payment_event = SendEvent::from_event(events.pop().unwrap());
+
+               nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+               commitment_signed_dance!(nodes[2], nodes[0], payment_event.commitment_msg, false);
+
+               expect_pending_htlcs_forwardable!(nodes[2]);
+               check_added_monitors!(nodes[2], 1);
+
+               let mut events = nodes[2].node.get_and_clear_pending_msg_events();
+               assert_eq!(events.len(), 1);
+               let payment_event = SendEvent::from_event(events.pop().unwrap());
+
+               nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &payment_event.msgs[0]);
+               check_added_monitors!(nodes[3], 0);
+               commitment_signed_dance!(nodes[3], nodes[2], payment_event.commitment_msg, true, true);
+
+               // At this point, nodes[3] should notice the two HTLCs don't contain the same total payment
+               // amount. It will assume the second is a privacy attack (no longer particularly relevant
+               // post-payment_secrets) and fail back the new HTLC.
+       }
+       expect_pending_htlcs_forwardable_ignore!(nodes[3]);
+       nodes[3].node.process_pending_htlc_forwards();
+       expect_pending_htlcs_forwardable_ignore!(nodes[3]);
+       nodes[3].node.process_pending_htlc_forwards();
+
+       check_added_monitors!(nodes[3], 1);
+
+       let fail_updates_1 = get_htlc_update_msgs!(nodes[3], nodes[2].node.get_our_node_id());
+       nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[2], nodes[3], fail_updates_1.commitment_signed, false);
+
+       expect_pending_htlcs_forwardable!(nodes[2]);
+       check_added_monitors!(nodes[2], 1);
+
+       let fail_updates_2 = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &fail_updates_2.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[2], fail_updates_2.commitment_signed, false);
+
+       expect_payment_failed_conditions!(nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());
+
+       nodes[0].node.send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None).unwrap();
+       check_added_monitors!(nodes[0], 1);
+
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);
 
-       let failure_events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(failure_events.len(), 2);
-       if let Event::PaymentPathFailed { .. } = failure_events[0] {} else { panic!(); }
-       if let Event::PaymentPathFailed { .. } = failure_events[1] {} else { panic!(); }
+       claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
 }
 
 #[test]