Wrote test to explicitly test BOLT 2 requirements for update_add_htlc 2019-01-291-rebased
authorPhilip Robinson <simian@tari.com>
Tue, 15 Jan 2019 09:52:02 +0000 (11:52 +0200)
committerMatt Corallo <git@bluematt.me>
Tue, 22 Jan 2019 17:22:14 +0000 (12:22 -0500)
(minor tweaks by @TheBlueMatt)

src/ln/channel.rs
src/ln/functional_tests.rs

index 237ccd840f561a86d89d915bf2c4c635157f37b7..98eb19fee032ea4aee96afb924d8ab6f8644e104 100644 (file)
@@ -307,6 +307,9 @@ pub(super) struct Channel {
        pub(super) our_dust_limit_satoshis: u64,
        #[cfg(not(test))]
        our_dust_limit_satoshis: u64,
+       #[cfg(test)]
+       pub(super) their_max_htlc_value_in_flight_msat: u64,
+       #[cfg(not(test))]
        their_max_htlc_value_in_flight_msat: u64,
        //get_our_max_htlc_value_in_flight_msat(): u64,
        /// minimum channel reserve for **self** to maintain - set by them.
@@ -316,6 +319,9 @@ pub(super) struct Channel {
        our_htlc_minimum_msat: u64,
        their_to_self_delay: u16,
        //implied by BREAKDOWN_TIMEOUT: our_to_self_delay: u16,
+       #[cfg(test)]
+       pub their_max_accepted_htlcs: u16,
+       #[cfg(not(test))]
        their_max_accepted_htlcs: u16,
        //implied by OUR_MAX_HTLCS: our_max_accepted_htlcs: u16,
        minimum_depth: u32,
@@ -337,7 +343,7 @@ pub(super) struct Channel {
        logger: Arc<Logger>,
 }
 
-const OUR_MAX_HTLCS: u16 = 50; //TODO
+pub const OUR_MAX_HTLCS: u16 = 50; //TODO
 /// Confirmation count threshold at which we close a channel. Ideally we'd keep the channel around
 /// on ice until the funding transaction gets more confirmations, but the LN protocol doesn't
 /// really allow for this, so instead we're stuck closing it out at that point.
@@ -1597,7 +1603,6 @@ impl Channel {
                        cltv_expiry: msg.cltv_expiry,
                        state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
                });
-
                Ok(())
        }
 
index de2b42d3fa372e32f1aa5fdf64f051b84a9acb01..eeff90e291a283e75b47ce40b55f039262b7089c 100644 (file)
@@ -21,6 +21,7 @@ use util::errors::APIError;
 use util::logger::Logger;
 use util::ser::{Writeable, Writer, ReadableArgs};
 use util::config::UserConfig;
+use util::rng;
 
 use bitcoin::util::hash::{BitcoinHash, Sha256dHash};
 use bitcoin::util::bip143;
@@ -1968,19 +1969,33 @@ fn get_announce_close_broadcast_events(nodes: &Vec<Node>, a: usize, b: usize) {
        }
 }
 
+macro_rules! expect_payment_received {
+       ($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => {
+               let events = $node.node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 1);
+               match events[0] {
+                       Event::PaymentReceived { ref payment_hash, amt } => {
+                               assert_eq!($expected_payment_hash, *payment_hash);
+                               assert_eq!($expected_recv_value, amt);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+       }
+}
+
+macro_rules! get_channel_value_stat {
+       ($node: expr, $channel_id: expr) => {{
+               let chan_lock = $node.node.channel_state.lock().unwrap();
+               let chan = chan_lock.by_id.get(&$channel_id).unwrap();
+               chan.get_value_stat()
+       }}
+}
+
 fn do_channel_reserve_test(test_recv: bool) {
        use util::rng;
        use std::sync::atomic::Ordering;
        use ln::msgs::HandleError;
 
-       macro_rules! get_channel_value_stat {
-               ($node: expr, $channel_id: expr) => {{
-                       let chan_lock = $node.node.channel_state.lock().unwrap();
-                       let chan = chan_lock.by_id.get(&$channel_id).unwrap();
-                       chan.get_value_stat()
-               }}
-       }
-
        let mut nodes = create_network(3);
        let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1900, 1001);
        let chan_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1900, 1001);
@@ -2009,20 +2024,6 @@ fn do_channel_reserve_test(test_recv: bool) {
                }}
        }
 
-       macro_rules! expect_payment_received {
-               ($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => {
-                       let events = $node.node.get_and_clear_pending_events();
-                       assert_eq!(events.len(), 1);
-                       match events[0] {
-                               Event::PaymentReceived { ref payment_hash, amt } => {
-                                       assert_eq!($expected_payment_hash, *payment_hash);
-                                       assert_eq!($expected_recv_value, amt);
-                               },
-                               _ => panic!("Unexpected event"),
-                       }
-               }
-       };
-
        let feemsat = 239; // somehow we know?
        let total_fee_msat = (nodes.len() - 2) as u64 * 239;
 
@@ -6602,3 +6603,306 @@ fn test_onion_failure() {
                msg.onion_routing_packet = onion_packet;
        }, ||{}, true, Some(21), None);
 }
+
+// BOLT 2 Requirements for the Sender when constructing and sending an update_add_htlc message.
+// BOLT 2 Requirement: MUST NOT offer amount_msat it cannot pay for in the remote commitment transaction at the current feerate_per_kw (see "Updating Fees") while maintaining its channel reserve.
+//TODO: I don't believe this is explicitly enforced when sending an HTLC but as the Fee aspect of the BOLT specs is in flux leaving this as a TODO.
+
+#[test]
+fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() {
+       //BOLT2 Requirement: MUST offer amount_msat greater than 0.
+       //BOLT2 Requirement: MUST NOT offer amount_msat below the receiving node's htlc_minimum_msat (same validation check catches both of these)
+       let mut nodes = create_network(2);
+       let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
+       let mut route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 100000, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+
+       route.hops[0].fee_msat = 0;
+
+       let err = nodes[0].node.send_payment(route, our_payment_hash);
+
+       if let Err(APIError::ChannelUnavailable{err}) = err {
+               assert_eq!(err, "Cannot send less than their minimum HTLC value");
+       } else {
+               assert!(false);
+       }
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() {
+       //BOLT 2 Requirement: MUST set cltv_expiry less than 500000000.
+       //It is enforced when constructing a route.
+       let mut nodes = create_network(2);
+       let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 0);
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 100000000, 500000001).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+
+       let err = nodes[0].node.send_payment(route, our_payment_hash);
+
+       if let Err(APIError::RouteError{err}) = err {
+               assert_eq!(err, "Channel CLTV overflowed?!");
+       } else {
+               assert!(false);
+       }
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment() {
+       //BOLT 2 Requirement: if result would be offering more than the remote's max_accepted_htlcs HTLCs, in the remote commitment transaction: MUST NOT add an HTLC.
+       //BOLT 2 Requirement: for the first HTLC it offers MUST set id to 0.
+       //BOLT 2 Requirement: MUST increase the value of id by 1 for each successive offer.
+       let mut nodes = create_network(2);
+       let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 0);
+       let max_accepted_htlcs = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().their_max_accepted_htlcs as u64;
+
+       for i in 0..max_accepted_htlcs {
+               let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 100000, TEST_FINAL_CLTV).unwrap();
+               let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+               let mut payment_event = {
+                       nodes[0].node.send_payment(route, our_payment_hash).unwrap();
+                       check_added_monitors!(nodes[0], 1);
+
+                       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+                       assert_eq!(events.len(), 1);
+                       if let MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate{ update_add_htlcs: ref htlcs, .. }, } = events[0] {
+                               assert_eq!(htlcs[0].htlc_id, i);
+                       } else {
+                               assert!(false);
+                       }
+                       SendEvent::from_event(events.remove(0))
+               };
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
+               check_added_monitors!(nodes[1], 0);
+               commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+
+               expect_pending_htlcs_forwardable!(nodes[1]);
+               expect_payment_received!(nodes[1], our_payment_hash, 100000);
+       }
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 100000, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       let err = nodes[0].node.send_payment(route, our_payment_hash);
+
+       if let Err(APIError::ChannelUnavailable{err}) = err {
+               assert_eq!(err, "Cannot push more than their max accepted HTLCs");
+       } else {
+               assert!(false);
+       }
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_value_in_flight() {
+       //BOLT 2 Requirement: if the sum of total offered HTLCs would exceed the remote's max_htlc_value_in_flight_msat: MUST NOT add an HTLC.
+       let mut nodes = create_network(2);
+       let channel_value = 100000;
+       let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, channel_value, 0);
+       let max_in_flight = get_channel_value_stat!(nodes[0], chan.2).their_max_htlc_value_in_flight_msat;
+
+       send_payment(&nodes[0], &vec!(&nodes[1])[..], max_in_flight);
+
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], max_in_flight+1, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       let err = nodes[0].node.send_payment(route, our_payment_hash);
+
+       if let Err(APIError::ChannelUnavailable{err}) = err {
+               assert_eq!(err, "Cannot send value that would put us over our max HTLC value in flight");
+       } else {
+               assert!(false);
+       }
+
+       send_payment(&nodes[0], &[&nodes[1]], max_in_flight);
+}
+
+// BOLT 2 Requirements for the Receiver when handling an update_add_htlc message.
+#[test]
+fn test_update_add_htlc_bolt2_receiver_check_amount_received_more_than_min() {
+       //BOLT2 Requirement: receiving an amount_msat equal to 0, OR less than its own htlc_minimum_msat -> SHOULD fail the channel.
+       let mut nodes = create_network(2);
+       let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
+       let htlc_minimum_msat: u64;
+       {
+               let chan_lock = nodes[0].node.channel_state.lock().unwrap();
+               let channel = chan_lock.by_id.get(&chan.2).unwrap();
+               htlc_minimum_msat = channel.get_our_htlc_minimum_msat();
+       }
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], htlc_minimum_msat+1, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       nodes[0].node.send_payment(route, our_payment_hash).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+       updates.update_add_htlcs[0].amount_msat = htlc_minimum_msat-1;
+       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+       if let Err(msgs::HandleError{err, action: Some(msgs::ErrorAction::SendErrorMessage {..})}) = err {
+               assert_eq!(err, "Remote side tried to send less than our minimum HTLC value");
+       } else {
+               assert!(false);
+       }
+       assert!(nodes[1].node.list_channels().is_empty());
+       check_closed_broadcast!(nodes[1]);
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_receiver_sender_can_afford_amount_sent() {
+       //BOLT2 Requirement: receiving an amount_msat that the sending node cannot afford at the current feerate_per_kw (while maintaining its channel reserve): SHOULD fail the channel
+       let mut nodes = create_network(2);
+       let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
+
+       let their_channel_reserve = get_channel_value_stat!(nodes[0], chan.2).channel_reserve_msat;
+
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 5000000-their_channel_reserve, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       nodes[0].node.send_payment(route, our_payment_hash).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+       updates.update_add_htlcs[0].amount_msat = 5000000-their_channel_reserve+1;
+       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+
+       if let Err(msgs::HandleError{err, action: Some(msgs::ErrorAction::SendErrorMessage {..})}) = err {
+               assert_eq!(err, "Remote HTLC add would put them over their reserve value");
+       } else {
+               assert!(false);
+       }
+
+       assert!(nodes[1].node.list_channels().is_empty());
+       check_closed_broadcast!(nodes[1]);
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
+       let secp_ctx = Secp256k1::new();
+
+       //BOLT 2 Requirement: if a sending node adds more than its max_accepted_htlcs HTLCs to its local commitment transaction: SHOULD fail the channel
+       //BOLT 2 Requirement: MUST allow multiple HTLCs with the same payment_hash.
+       let mut nodes = create_network(2);
+       let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 3999999, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+
+       let session_priv = SecretKey::from_slice(&secp_ctx, &{
+               let mut session_key = [0; 32];
+               rng::fill_bytes(&mut session_key);
+               session_key
+       }).expect("RNG is bad!");
+
+       let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+       let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route, &session_priv).unwrap();
+       let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route, cur_height).unwrap();
+       let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, &our_payment_hash);
+
+       let mut msg = msgs::UpdateAddHTLC {
+               channel_id: chan.2,
+               htlc_id: 0,
+               amount_msat: 1000,
+               payment_hash: our_payment_hash,
+               cltv_expiry: htlc_cltv,
+               onion_routing_packet: onion_packet.clone(),
+       };
+
+       for i in 0..super::channel::OUR_MAX_HTLCS {
+               msg.htlc_id = i as u64;
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg).unwrap();
+       }
+       msg.htlc_id = (super::channel::OUR_MAX_HTLCS) as u64;
+       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);
+
+       if let Err(msgs::HandleError{err, action: Some(msgs::ErrorAction::SendErrorMessage {..})}) = err {
+               assert_eq!(err, "Remote tried to push more than our max accepted HTLCs");
+       } else {
+               assert!(false);
+       }
+
+       assert!(nodes[1].node.list_channels().is_empty());
+       check_closed_broadcast!(nodes[1]);
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_receiver_check_max_in_flight_msat() {
+       //OR adds more than its max_htlc_value_in_flight_msat worth of offered HTLCs to its local commitment transaction: SHOULD fail the channel
+       let mut nodes = create_network(2);
+       let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000);
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 1000000, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       nodes[0].node.send_payment(route, our_payment_hash).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+       updates.update_add_htlcs[0].amount_msat = get_channel_value_stat!(nodes[1], chan.2).their_max_htlc_value_in_flight_msat + 1;
+       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+
+       if let Err(msgs::HandleError{err, action: Some(msgs::ErrorAction::SendErrorMessage {..})}) = err {
+               assert_eq!(err,"Remote HTLC add would put them over their max HTLC value in flight");
+       } else {
+               assert!(false);
+       }
+
+       assert!(nodes[1].node.list_channels().is_empty());
+       check_closed_broadcast!(nodes[1]);
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_receiver_check_cltv_expiry() {
+       //BOLT2 Requirement: if sending node sets cltv_expiry to greater or equal to 500000000: SHOULD fail the channel.
+       let mut nodes = create_network(2);
+       create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000);
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 3999999, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       nodes[0].node.send_payment(route, our_payment_hash).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+       updates.update_add_htlcs[0].cltv_expiry = 500000000;
+       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+
+       if let Err(msgs::HandleError{err, action: Some(msgs::ErrorAction::SendErrorMessage {..})}) = err {
+               assert_eq!(err,"Remote provided CLTV expiry in seconds instead of block height");
+       } else {
+               assert!(false);
+       }
+
+       assert!(nodes[1].node.list_channels().is_empty());
+       check_closed_broadcast!(nodes[1]);
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_receiver_check_repeated_id_ignore() {
+       //BOLT 2 requirement: if the sender did not previously acknowledge the commitment of that HTLC: MUST ignore a repeated id value after a reconnection.
+       // We test this by first testing that that repeated HTLCs pass commitment signature checks
+       // after disconnect and that non-sequential htlc_ids result in a channel failure.
+       let mut nodes = create_network(2);
+       create_announced_chan_between_nodes(&nodes, 0, 1);
+       let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 1000000, TEST_FINAL_CLTV).unwrap();
+       let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+       nodes[0].node.send_payment(route, our_payment_hash).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]).unwrap();
+
+       //Disconnect and Reconnect
+       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);
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
+       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());
+       let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+       assert_eq!(reestablish_2.len(), 1);
+       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
+       handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+       nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+       handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
+
+       //Resend HTLC
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]).unwrap();
+       assert_eq!(updates.commitment_signed.htlc_signatures.len(), 1);
+       nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &updates.commitment_signed).unwrap();
+       check_added_monitors!(nodes[1], 1);
+       let _bs_responses = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+       if let Err(msgs::HandleError{err, action: Some(msgs::ErrorAction::SendErrorMessage {..})}) = err {
+               assert_eq!(err, "Remote skipped HTLC ID");
+       } else {
+               assert!(false);
+       }
+
+       assert!(nodes[1].node.list_channels().is_empty());
+       check_closed_broadcast!(nodes[1]);
+}