f fix fee spike buffer + update to latest features API

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -1768,20 +1768,22 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		if self.channel_outbound {
 			// Check that they won't violate our channel reserve by adding this HTLC.
 
-			// One of the +1's is for the HTLC that is currently being added to the commitment tx
-			// and the other is a fee spike buffer we're keeping for the remote (this deviates
-			// from the spec but should help protect us from stuck channels).
-			// TODO: is the reason this added HTLC's amount can't count toward's the receiver's
-			// fee spike buffer because that doesn't work with existing HTLC output spend scripts?
-			let local_fee_cost_msat = self.commit_tx_fee_msat(self.htlc_count_next_local_commit_tx() + 1 + 1);
+			// +1 for this HTLC
+			let local_fee_cost_msat = self.commit_tx_fee_msat(self.htlc_count_next_local_commit_tx() + 1);
 			if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + local_fee_cost_msat {
+				// TODO: finish this snippet so failing here doesn't cause channel close
+				// pending_forward_state = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC{
+				// 	channel_id: msg.channel_id,
+				// 	htlc_id: 
+				// }));
 				return Err(ChannelError::Ignore("Cannot receive value that would put us over their reserve value"));
 			}
 		}
 
-		// The +1 is for the HTLC that is currently being added to the commitment tx.
 		let remote_fee_cost_msat = if self.channel_outbound { 0 } else {
-			self.commit_tx_fee_msat(self.htlc_count_next_remote_commit_tx() + 1)
+			// +1 for this HTLC, +1 fee spike buffer we keep for the remote (this deviates from the spec but
+			// should protect us from stuck channels).
+			self.commit_tx_fee_msat(self.htlc_count_next_remote_commit_tx() + 1 + 1)
 		};
 		if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat + remote_fee_cost_msat
 			> (self.channel_value_satoshis - Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
@@ -3625,10 +3627,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 			let their_balance = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
 			let chan_reserve_we_require_msat = Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis);
-			// +1 for this HTLC, +2 for their fee spike buffer
-			// TODO: is the reason this added HTLC's amount can't count toward's the receiver's
-			// fee spike buffer because that doesn't work with existing HTLC output spend scripts?
-			let remote_commit_tx_fee_msat = self.commit_tx_fee_msat(self.htlc_count_next_remote_commit_tx() + 1 + 2);
+			// +1 for this HTLC
+			let remote_commit_tx_fee_msat = self.commit_tx_fee_msat(self.htlc_count_next_remote_commit_tx() + 1);
 			if their_balance < chan_reserve_we_require_msat + remote_commit_tx_fee_msat {
 				return Err(ChannelError::Ignore("Cannot send value that would put them over our reserve value"));
 			}

lightning/src/ln/functional_tests.rs

@@ -1506,7 +1506,7 @@ fn test_basic_channel_reserve() {
 	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 chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
 
 	let chan_stat = get_channel_value_stat!(nodes[0], chan.2);
 	let their_channel_reserve = chan_stat.channel_reserve_msat;
@@ -1538,8 +1538,8 @@ fn test_channel_reserve_violation_close_pending_htlc() {
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
-	let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::supported(), InitFeatures::supported());
-	let _ = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 95000000, InitFeatures::supported(), InitFeatures::supported());
+	let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
+	let _ = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
 
 	macro_rules! get_route_and_payment_hash {
 		($recv_value: expr) => {{
@@ -5859,7 +5859,7 @@ fn test_holding_cell_htlc_with_pending_fee_update() {
 	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 chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
 
 	// First nodes[0] generates an update_fee, setting the channel's
 	// pending_update_fee.
@@ -6193,7 +6193,7 @@ fn test_update_add_htlc_bolt2_receiver_check_cltv_expiry() {
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known());
-	let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 3999999, TEST_FINAL_CLTV).unwrap();
+	let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 399999, TEST_FINAL_CLTV).unwrap();
 	let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
 	nodes[0].node.send_payment(&route, our_payment_hash, &None).unwrap();
 	check_added_monitors!(nodes[0], 1);