From 1d6c09a94a52ec5a09074de15ab54cbbf74911ce Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 13 Feb 2019 20:44:14 -0500 Subject: [PATCH] Clarify policy applied in send htlc error msgs max_htlc_value_in_flight_msat is applied per-direction --- src/ln/channel.rs | 8 +++----- src/ln/functional_test_utils.rs | 2 +- src/ln/functional_tests.rs | 12 ++++++------ 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/ln/channel.rs b/src/ln/channel.rs index beab7ffc6..07fb3474a 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -1590,10 +1590,9 @@ impl Channel { if inbound_htlc_count + 1 > OUR_MAX_HTLCS as u32 { return Err(ChannelError::Close("Remote tried to push more than our max accepted HTLCs")); } - //TODO: Spec is unclear if this is per-direction or in total (I assume per direction): // Check our_max_htlc_value_in_flight_msat if htlc_inbound_value_msat + msg.amount_msat > Channel::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis) { - return Err(ChannelError::Close("Remote HTLC add would put them over their max HTLC value in flight")); + return Err(ChannelError::Close("Remote HTLC add would put them over our max HTLC value")); } // Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet // the reserve_satoshis we told them to always have as direct payment so that they lose @@ -3214,16 +3213,15 @@ impl Channel { if outbound_htlc_count + 1 > self.their_max_accepted_htlcs as u32 { return Err(ChannelError::Ignore("Cannot push more than their max accepted HTLCs")); } - //TODO: Spec is unclear if this is per-direction or in total (I assume per direction): // Check their_max_htlc_value_in_flight_msat if htlc_outbound_value_msat + amount_msat > self.their_max_htlc_value_in_flight_msat { - return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight")); + return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept")); } // Check self.their_channel_reserve_satoshis (the amount we must keep as // reserve for them to have something to claim if we misbehave) if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat { - return Err(ChannelError::Ignore("Cannot send value that would put us over the reserve value")); + return Err(ChannelError::Ignore("Cannot send value that would put us over their reserve value")); } //TODO: Check cltv_expiry? Do this in channel manager? diff --git a/src/ln/functional_test_utils.rs b/src/ln/functional_test_utils.rs index 1f11fe74b..36a909873 100644 --- a/src/ln/functional_test_utils.rs +++ b/src/ln/functional_test_utils.rs @@ -702,7 +702,7 @@ pub fn route_over_limit(origin_node: &Node, expected_route: &[&Node], recv_value let err = origin_node.node.send_payment(route, our_payment_hash).err().unwrap(); match err { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight"), + APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept"), _ => panic!("Unknown error variants"), }; } diff --git a/src/ln/functional_tests.rs b/src/ln/functional_tests.rs index 58c8cd629..ff6169144 100644 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@ -1235,7 +1235,7 @@ fn do_channel_reserve_test(test_recv: bool) { assert!(route.hops.iter().rev().skip(1).all(|h| h.fee_msat == feemsat)); let err = nodes[0].node.send_payment(route, our_payment_hash).err().unwrap(); match err { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight"), + APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept"), _ => panic!("Unknown error variants"), } } @@ -1271,7 +1271,7 @@ fn do_channel_reserve_test(test_recv: bool) { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value + 1); let err = nodes[0].node.send_payment(route.clone(), our_payment_hash).err().unwrap(); match err { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the reserve value"), + APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"), _ => panic!("Unknown error variants"), } } @@ -1296,7 +1296,7 @@ fn do_channel_reserve_test(test_recv: bool) { { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1); match nodes[0].node.send_payment(route, our_payment_hash).err().unwrap() { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the reserve value"), + APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"), _ => panic!("Unknown error variants"), } } @@ -1359,7 +1359,7 @@ fn do_channel_reserve_test(test_recv: bool) { { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_22+1); match nodes[0].node.send_payment(route, our_payment_hash).err().unwrap() { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the reserve value"), + APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"), _ => panic!("Unknown error variants"), } } @@ -4852,7 +4852,7 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_value_in_flight() { 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 the max HTLC value in flight"); + assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept"); } else { assert!(false); } @@ -4975,7 +4975,7 @@ fn test_update_add_htlc_bolt2_receiver_check_max_in_flight_msat() { 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"); + assert_eq!(err,"Remote HTLC add would put them over our max HTLC value"); } else { assert!(false); } -- 2.39.5