Merge pull request #513 from ariard/2020-02-fix-zero-msat-htlc

BOLT2: Check we don't send and accept 0-msat HTLC

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 8b5d9120babb2e98eccdcccb3e472443e5b070d8..86074627523ca3c7b7ea4f2c5f55b43a756ffbfd 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -433,10 +433,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                cmp::max(at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO
        }
 
-       fn derive_our_htlc_minimum_msat(_at_open_channel_feerate_per_kw: u64) -> u64 {
-               1000 // TODO
-       }
-
        // Constructors:
        pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, their_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel<ChanSigner>, APIError>
        where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
@@ -519,7 +515,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_max_htlc_value_in_flight_msat: 0,
                        their_channel_reserve_satoshis: 0,
                        their_htlc_minimum_msat: 0,
-                       our_htlc_minimum_msat: Channel::<ChanSigner>::derive_our_htlc_minimum_msat(feerate),
+                       our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        their_to_self_delay: 0,
                        our_to_self_delay: config.own_channel_config.our_to_self_delay,
                        their_max_accepted_htlcs: 0,
@@ -744,7 +740,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
                        their_channel_reserve_satoshis: msg.channel_reserve_satoshis,
                        their_htlc_minimum_msat: msg.htlc_minimum_msat,
-                       our_htlc_minimum_msat: Channel::<ChanSigner>::derive_our_htlc_minimum_msat(msg.feerate_per_kw as u64),
+                       our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        their_to_self_delay: msg.to_self_delay,
                        our_to_self_delay: config.own_channel_config.our_to_self_delay,
                        their_max_accepted_htlcs: msg.max_accepted_htlcs,
@@ -1656,6 +1652,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if msg.amount_msat > self.channel_value_satoshis * 1000 {
                        return Err(ChannelError::Close("Remote side tried to send more than the total value of the channel"));
                }
+               if msg.amount_msat == 0 {
+                       return Err(ChannelError::Close("Remote side tried to send a 0-msat HTLC"));
+               }
                if msg.amount_msat < self.our_htlc_minimum_msat {
                        return Err(ChannelError::Close("Remote side tried to send less than our minimum HTLC value"));
                }
@@ -3493,6 +3492,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if amount_msat > self.channel_value_satoshis * 1000 {
                        return Err(ChannelError::Ignore("Cannot send more than the total value of the channel"));
                }
+
+               if amount_msat == 0 {
+                       return Err(ChannelError::Ignore("Cannot send 0-msat HTLC"));
+               }
+
                if amount_msat < self.their_htlc_minimum_msat {
                        return Err(ChannelError::Ignore("Cannot send less than their minimum HTLC value"));
                }

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 27908ca54623a64c07bffbd3fac9815e3d703e02..ba918f82e860bf572b16dd886875743c315b1cd0 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1005,6 +1005,7 @@ pub fn create_node_chanmgrs<'a, 'b>(node_count: usize, cfgs: &'a Vec<NodeCfg<'b>
                let mut default_config = UserConfig::default();
                default_config.channel_options.announced_channel = true;
                default_config.peer_channel_config_limits.force_announced_channel_preference = false;
+               default_config.own_channel_config.our_htlc_minimum_msat = 1000; // sanitization being done by the sender, to exerce receiver logic we need to lift of limit
                let node = ChannelManager::new(Network::Testnet, cfgs[i].fee_estimator, &cfgs[i].chan_monitor, cfgs[i].tx_broadcaster, cfgs[i].logger.clone(), &cfgs[i].keys_manager, if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, 0).unwrap();
                chanmgrs.push(node);
        }

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index a41ae11af2c2d19cb95060ef72d230b1e1cc4ca8..d12de937e54cfe93cd3b8b7cc1f9a2134ac646ad 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -5486,7 +5486,6 @@ fn bolt2_open_channel_sending_node_checks_part2() {
 
 #[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 chanmon_cfgs = create_chanmon_cfgs(2);
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
@@ -5496,7 +5495,7 @@ fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() {
        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;
+       route.hops[0].fee_msat = 100;
 
        let err = nodes[0].node.send_payment(route, our_payment_hash);
 
@@ -5509,6 +5508,51 @@ fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() {
        nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send less than their minimum HTLC value".to_string(), 1);
 }
 
+#[test]
+fn test_update_add_htlc_bolt2_sender_zero_value_msat() {
+       //BOLT2 Requirement: MUST offer amount_msat greater than 0.
+       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 mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::supported(), InitFeatures::supported());
+       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 0-msat HTLC");
+       } else {
+               assert!(false);
+       }
+       assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+       nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send 0-msat HTLC".to_string(), 1);
+}
+
+#[test]
+fn test_update_add_htlc_bolt2_receiver_zero_value_msat() {
+       //BOLT2 Requirement: MUST offer amount_msat greater than 0.
+       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 mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::supported(), InitFeatures::supported());
+       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]);
+
+       nodes[0].node.send_payment(route, our_payment_hash);
+       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 = 0;
+
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+       nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Remote side tried to send a 0-msat HTLC".to_string(), 1);
+       check_closed_broadcast!(nodes[1], true).unwrap();
+}
+
 #[test]
 fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() {
        //BOLT 2 Requirement: MUST set cltv_expiry less than 500000000.
@@ -7271,3 +7315,21 @@ fn test_override_channel_config() {
        assert_eq!(res.channel_flags, 0);
        assert_eq!(res.to_self_delay, 200);
 }
+
+#[test]
+fn test_override_0msat_htlc_minimum() {
+       let mut zero_config = UserConfig::default();
+       zero_config.own_channel_config.our_htlc_minimum_msat = 0;
+       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, Some(zero_config.clone())]);
+       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 16_000_000, 12_000_000, 42, Some(zero_config)).unwrap();
+       let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
+       assert_eq!(res.htlc_minimum_msat, 1);
+
+       nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::supported(), &res);
+       let res = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
+       assert_eq!(res.htlc_minimum_msat, 1);
+}

diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs
index 3c4ab77c16bc1d921d6057c21bb620fe410dce68..c76747cbbf8e1de83ab1ac381ae09c8c266de5d3 100644 (file)
--- a/lightning/src/util/config.rs
+++ b/lightning/src/util/config.rs
@@ -51,6 +51,14 @@ pub struct ChannelHandshakeConfig {
        /// Default value: BREAKDOWN_TIMEOUT (currently 144), we enforce it as a minimum at channel
        /// opening so you can tweak config to ask for more security, not less.
        pub our_to_self_delay: u16,
+       /// Set to the smallest value HTLC we will accept to process.
+       ///
+       /// This value is sent to our counterparty on channel-open and we close the channel any time
+       /// our counterparty misbehaves by sending us an HTLC with a value smaller than this.
+       ///
+       /// Default value: 1. If the value is less than 1, it is ignored and set to 1, as is required
+       /// by the protocol.
+       pub our_htlc_minimum_msat: u64,
 }
 
 impl Default for ChannelHandshakeConfig {
@@ -58,6 +66,7 @@ impl Default for ChannelHandshakeConfig {
                ChannelHandshakeConfig {
                        minimum_depth: 6,
                        our_to_self_delay: BREAKDOWN_TIMEOUT,
+                       our_htlc_minimum_msat: 1,
                }
        }
 }