Refactor: move channel checks for HTLC adds into Channel

This also includes adding a closure that creates a new pending HTLC status
as a parameter for Channel's update_add_htlc. This will later be useful
when we add the check for fee spike buffer violations, which will also result
in changing an HTLC's pending status to failing.

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -1665,8 +1665,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		cmp::min(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64)
 	}
 
-	pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), ChannelError> {
-		if (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32) {
+	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() {
+			// 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.
+			// This probably needs a spec update but should definitely be
+			// allowed.
+			pending_forward_state = create_pending_htlc_status(self, pending_forward_state, 0x1000|20);
+		}
+		// If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+		let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+		if remote_sent_shutdown {
 			return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
 		}
 		if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {

lightning/src/ln/channelmanager.rs

@@ -2472,47 +2472,48 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		//encrypted with the same key. It's not immediately obvious how to usefully exploit that,
 		//but we should prevent it anyway.
 
-		let (mut pending_forward_info, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg);
+		let (pending_forward_info, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg);
 		let channel_state = &mut *channel_state_lock;
 
 		match channel_state.by_id.entry(msg.channel_id) {
 			hash_map::Entry::Occupied(mut chan) => {
 				if chan.get().get_their_node_id() != *their_node_id {
 					return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
 				}
-				if !chan.get().is_usable() {
+
+				let create_pending_htlc_status = |chan: &Channel<ChanSigner>, pending_forward_info: PendingHTLCStatus, error_code: u16| {
 					// 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.
-					if let PendingHTLCStatus::Forward(PendingHTLCInfo { incoming_shared_secret, .. }) = pending_forward_info {
-						let chan_update = self.get_channel_update(chan.get());
-						pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
-							channel_id: msg.channel_id,
-							htlc_id: msg.htlc_id,
-							reason: if let Ok(update) = chan_update {
-								// 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.
-								// This probably needs a spec update but should definitely be
-								// allowed.
-								onion_utils::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &{
+					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);
-									res.extend_from_slice(&byte_utils::be16_to_array(update.contents.flags));
-									res.extend_from_slice(&update.encode_with_len()[..]);
+									res.extend_from_slice(&byte_utils::be16_to_array(upd.contents.flags));
+									res.extend_from_slice(&upd.encode_with_len()[..]);
 									res
 								}[..])
 							} else {
-								// This can only happen 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
-								onion_utils::build_first_hop_failure_packet(&incoming_shared_secret, 0x4000|10, &[])
-							},
-						}));
+								onion_utils::build_first_hop_failure_packet(incoming_shared_secret, 0x4000|10, &[])
+							};
+							let msg = msgs::UpdateFailHTLC {
+								channel_id: msg.channel_id,
+								htlc_id: msg.htlc_id,
+								reason
+							};
+							PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msg))
+						},
+						_ => pending_forward_info
 					}
-				}
-				try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info), channel_state, chan);
+				};
+				try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status), channel_state, chan);
 			},
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
 		}