f fix Matt review from 06/08 nits

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -1735,7 +1735,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_state: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
 	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
-		if !self.is_usable() {
+		// We can't accept HTLCs sent after we've sent a shutdown.
+		let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+		if local_sent_shutdown {
 			// TODO: Note that |20 is defined as "channel FROM the processing
 			// node has been disabled" (emphasis mine), which seems to imply
 			// that we can't return |20 for an inbound channel being disabled.
@@ -1815,6 +1817,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		if !self.channel_outbound {
 			// `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote (this deviates from the spec
 			// but should help protect us from stuck channels).
+			// Note that when we eventually remove support for fee updates and switch to anchor output fees,
+			// we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+			// as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+			// being sensitive to fee spikes.
 			let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
 			if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
 				// Note that if the pending_forward_state is not updated here, then it's because we're already failing

lightning/src/ln/channelmanager.rs

@@ -2482,17 +2482,14 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 				}
 
 				let create_pending_htlc_status = |chan: &Channel<ChanSigner>, pending_forward_info: PendingHTLCStatus, error_code: u16| {
+					// Ensure error_code has the UPDATE flag set, since by default we send a
+					// channel update along as part of failing the HTLC.
+					assert!((error_code & 0x1000) != 0);
 					// If the update_add is completely bogus, the call will Err and we will close,
 					// but if we've sent a shutdown and they haven't acknowledged it yet, we just
 					// want to reject the new HTLC and fail it backwards instead of forwarding.
 					match pending_forward_info {
 						PendingHTLCStatus::Forward(PendingHTLCInfo { ref incoming_shared_secret, .. }) => {
-							// The only case where we'd be unable to successfully get a channel
-							// update here is if the channel isn't in the fully-funded
-							// state yet, implying our counterparty is trying to route payments
-							// over the channel back to themselves (cause no one else should
-							// know the short_id is a lightning channel yet). We should have no
-							// problem just calling this unknown_next_peer, as above (0x4000|10).
 							let reason = if let Ok(upd) = self.get_channel_update(chan) {
 								onion_utils::build_first_hop_failure_packet(incoming_shared_secret, error_code, &{
 									let mut res = Vec::with_capacity(8 + 128);
@@ -2501,6 +2498,15 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 									res
 								}[..])
 							} else {
+								// The only case where we'd be unable to
+								// successfully get a channel update is if the
+								// channel isn't in the fully-funded state yet,
+								// implying our counterparty is trying to route
+								// payments over the channel back to themselves
+								// (cause no one else should know the short_id
+								// is a lightning channel yet). We should have
+								// no problem just calling this
+								// unknown_next_peer (0x4000|10).
 								onion_utils::build_first_hop_failure_packet(incoming_shared_secret, 0x4000|10, &[])
 							};
 							let msg = msgs::UpdateFailHTLC {