X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannelmanager.rs;h=465256d21ea99bfa136062c8272c4dc34f65316b;hb=605d30eb034a879db4d5719730c88074b9347026;hp=570f04b628a7a5590d93de0b37edc43b5a5af32f;hpb=c82a65a1f695c76dfd78f2a07ee120f8a84b8dc7;p=rust-lightning

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 570f04b6..465256d2 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1348,7 +1348,7 @@ macro_rules! convert_chan_err {
 }
 
 macro_rules! break_chan_entry {
-	($self: ident, $res: expr, $channel_state: expr, $entry: expr) => {
+	($self: ident, $res: expr, $entry: expr) => {
 		match $res {
 			Ok(res) => res,
 			Err(e) => {
@@ -1363,7 +1363,7 @@ macro_rules! break_chan_entry {
 }
 
 macro_rules! try_chan_entry {
-	($self: ident, $res: expr, $channel_state: expr, $entry: expr) => {
+	($self: ident, $res: expr, $entry: expr) => {
 		match $res {
 			Ok(res) => res,
 			Err(e) => {
@@ -1378,7 +1378,7 @@ macro_rules! try_chan_entry {
 }
 
 macro_rules! remove_channel {
-	($self: expr, $channel_state: expr, $entry: expr) => {
+	($self: expr, $entry: expr) => {
 		{
 			let channel = $entry.remove_entry().1;
 			update_maps_on_chan_removal!($self, channel);
@@ -1919,7 +1919,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						let (result, is_permanent) =
 							handle_monitor_update_res!(self, update_res, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
 						if is_permanent {
-							remove_channel!(self, channel_state, chan_entry);
+							remove_channel!(self, chan_entry);
 							break result;
 						}
 					}
@@ -1930,7 +1930,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					});
 
 					if chan_entry.get().is_shutdown() {
-						let channel = remove_channel!(self, channel_state, chan_entry);
+						let channel = remove_channel!(self, chan_entry);
 						if let Ok(channel_update) = self.get_channel_update_for_broadcast(&channel) {
 							channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
 								msg: channel_update
@@ -2031,7 +2031,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				} else {
 					self.issue_channel_close_events(chan.get(),ClosureReason::HolderForceClosed);
 				}
-				remove_channel!(self, channel_state, chan)
+				remove_channel!(self, chan)
 			} else {
 				return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
 			}
@@ -2317,7 +2317,14 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						Some((_cp_id, chan_id)) => Some(chan_id.clone()),
 					};
 					let chan_update_opt = if let Some(forwarding_id) = forwarding_id_opt {
-						let chan = channel_state.by_id.get_mut(&forwarding_id).unwrap();
+						let chan = match channel_state.by_id.get_mut(&forwarding_id) {
+							None => {
+								// Channel was removed. The short_to_chan_info and by_id maps have
+								// no consistency guarantees.
+								break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+							},
+							Some(chan) => chan
+						};
 						if !chan.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels {
 							// Note that the behavior here should be identical to the above block - we
 							// should NOT reveal the existence or non-existence of a private channel if
@@ -2506,7 +2513,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 							payment_secret: payment_secret.clone(),
 							payment_params: payment_params.clone(),
 						}, onion_packet, &self.logger),
-					channel_state, chan)
+						chan)
 				} {
 					Some((update_add, commitment_signed, monitor_update)) => {
 						let update_err = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update);
@@ -2544,7 +2551,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					},
 					None => { },
 				}
-			} else { unreachable!(); }
+			} else {
+				// The channel was likely removed after we fetched the id from the
+				// `short_to_chan_info` map, but before we successfully locked the `by_id` map.
+				// This can occur as no consistency guarantees exists between the two maps.
+				return Err(APIError::ChannelUnavailable{err: "No channel available with first hop!".to_owned()});
+			}
 			return Ok(());
 		};
 
@@ -3133,9 +3145,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				let mut channel_state_lock = self.channel_state.lock().unwrap();
 				let channel_state = &mut *channel_state_lock;
 				if short_chan_id != 0 {
-					let forward_chan_id = match self.short_to_chan_info.read().unwrap().get(&short_chan_id) {
-						Some((_cp_id, chan_id)) => chan_id.clone(),
-						None => {
+					macro_rules! forwarding_channel_not_found {
+						() => {
 							for forward_info in pending_forwards.drain(..) {
 								match forward_info {
 									HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
@@ -3222,136 +3233,146 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 									}
 								}
 							}
+						}
+					}
+					let forward_chan_id = match self.short_to_chan_info.read().unwrap().get(&short_chan_id) {
+						Some((_cp_id, chan_id)) => chan_id.clone(),
+						None => {
+							forwarding_channel_not_found!();
 							continue;
 						}
 					};
-					if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(forward_chan_id) {
-						let mut add_htlc_msgs = Vec::new();
-						let mut fail_htlc_msgs = Vec::new();
-						for forward_info in pending_forwards.drain(..) {
-							match forward_info {
-								HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
-										routing: PendingHTLCRouting::Forward {
-											onion_packet, ..
-										}, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value },
-										prev_funding_outpoint } => {
-									log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
-									let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
-										short_channel_id: prev_short_channel_id,
-										outpoint: prev_funding_outpoint,
-										htlc_id: prev_htlc_id,
-										incoming_packet_shared_secret: incoming_shared_secret,
-										// Phantom payments are only PendingHTLCRouting::Receive.
-										phantom_shared_secret: None,
-									});
-									match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet, &self.logger) {
-										Err(e) => {
-											if let ChannelError::Ignore(msg) = e {
-												log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
-											} else {
-												panic!("Stated return value requirements in send_htlc() were not met");
-											}
-											let (failure_code, data) = self.get_htlc_temp_fail_err_and_data(0x1000|7, short_chan_id, chan.get());
-											failed_forwards.push((htlc_source, payment_hash,
-												HTLCFailReason::Reason { failure_code, data },
-												HTLCDestination::NextHopChannel { node_id: Some(chan.get().get_counterparty_node_id()), channel_id: forward_chan_id }
-											));
-											continue;
-										},
-										Ok(update_add) => {
-											match update_add {
-												Some(msg) => { add_htlc_msgs.push(msg); },
-												None => {
-													// Nothing to do here...we're waiting on a remote
-													// revoke_and_ack before we can add anymore HTLCs. The Channel
-													// will automatically handle building the update_add_htlc and
-													// commitment_signed messages when we can.
-													// TODO: Do some kind of timer to set the channel as !is_live()
-													// as we don't really want others relying on us relaying through
-													// this channel currently :/.
+					match channel_state.by_id.entry(forward_chan_id) {
+						hash_map::Entry::Vacant(_) => {
+							forwarding_channel_not_found!();
+							continue;
+						},
+						hash_map::Entry::Occupied(mut chan) => {
+							let mut add_htlc_msgs = Vec::new();
+							let mut fail_htlc_msgs = Vec::new();
+							for forward_info in pending_forwards.drain(..) {
+								match forward_info {
+									HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
+											routing: PendingHTLCRouting::Forward {
+												onion_packet, ..
+											}, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value },
+											prev_funding_outpoint } => {
+										log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
+										let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
+											short_channel_id: prev_short_channel_id,
+											outpoint: prev_funding_outpoint,
+											htlc_id: prev_htlc_id,
+											incoming_packet_shared_secret: incoming_shared_secret,
+											// Phantom payments are only PendingHTLCRouting::Receive.
+											phantom_shared_secret: None,
+										});
+										match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet, &self.logger) {
+											Err(e) => {
+												if let ChannelError::Ignore(msg) = e {
+													log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
+												} else {
+													panic!("Stated return value requirements in send_htlc() were not met");
+												}
+												let (failure_code, data) = self.get_htlc_temp_fail_err_and_data(0x1000|7, short_chan_id, chan.get());
+												failed_forwards.push((htlc_source, payment_hash,
+													HTLCFailReason::Reason { failure_code, data },
+													HTLCDestination::NextHopChannel { node_id: Some(chan.get().get_counterparty_node_id()), channel_id: forward_chan_id }
+												));
+												continue;
+											},
+											Ok(update_add) => {
+												match update_add {
+													Some(msg) => { add_htlc_msgs.push(msg); },
+													None => {
+														// Nothing to do here...we're waiting on a remote
+														// revoke_and_ack before we can add anymore HTLCs. The Channel
+														// will automatically handle building the update_add_htlc and
+														// commitment_signed messages when we can.
+														// TODO: Do some kind of timer to set the channel as !is_live()
+														// as we don't really want others relying on us relaying through
+														// this channel currently :/.
+													}
 												}
 											}
 										}
-									}
-								},
-								HTLCForwardInfo::AddHTLC { .. } => {
-									panic!("short_channel_id != 0 should imply any pending_forward entries are of type Forward");
-								},
-								HTLCForwardInfo::FailHTLC { htlc_id, err_packet } => {
-									log_trace!(self.logger, "Failing HTLC back to channel with short id {} (backward HTLC ID {}) after delay", short_chan_id, htlc_id);
-									match chan.get_mut().get_update_fail_htlc(htlc_id, err_packet, &self.logger) {
-										Err(e) => {
-											if let ChannelError::Ignore(msg) = e {
-												log_trace!(self.logger, "Failed to fail HTLC with ID {} backwards to short_id {}: {}", htlc_id, short_chan_id, msg);
-											} else {
-												panic!("Stated return value requirements in get_update_fail_htlc() were not met");
+									},
+									HTLCForwardInfo::AddHTLC { .. } => {
+										panic!("short_channel_id != 0 should imply any pending_forward entries are of type Forward");
+									},
+									HTLCForwardInfo::FailHTLC { htlc_id, err_packet } => {
+										log_trace!(self.logger, "Failing HTLC back to channel with short id {} (backward HTLC ID {}) after delay", short_chan_id, htlc_id);
+										match chan.get_mut().get_update_fail_htlc(htlc_id, err_packet, &self.logger) {
+											Err(e) => {
+												if let ChannelError::Ignore(msg) = e {
+													log_trace!(self.logger, "Failed to fail HTLC with ID {} backwards to short_id {}: {}", htlc_id, short_chan_id, msg);
+												} else {
+													panic!("Stated return value requirements in get_update_fail_htlc() were not met");
+												}
+												// fail-backs are best-effort, we probably already have one
+												// pending, and if not that's OK, if not, the channel is on
+												// the chain and sending the HTLC-Timeout is their problem.
+												continue;
+											},
+											Ok(Some(msg)) => { fail_htlc_msgs.push(msg); },
+											Ok(None) => {
+												// Nothing to do here...we're waiting on a remote
+												// revoke_and_ack before we can update the commitment
+												// transaction. The Channel will automatically handle
+												// building the update_fail_htlc and commitment_signed
+												// messages when we can.
+												// We don't need any kind of timer here as they should fail
+												// the channel onto the chain if they can't get our
+												// update_fail_htlc in time, it's not our problem.
 											}
-											// fail-backs are best-effort, we probably already have one
-											// pending, and if not that's OK, if not, the channel is on
-											// the chain and sending the HTLC-Timeout is their problem.
-											continue;
-										},
-										Ok(Some(msg)) => { fail_htlc_msgs.push(msg); },
-										Ok(None) => {
-											// Nothing to do here...we're waiting on a remote
-											// revoke_and_ack before we can update the commitment
-											// transaction. The Channel will automatically handle
-											// building the update_fail_htlc and commitment_signed
-											// messages when we can.
-											// We don't need any kind of timer here as they should fail
-											// the channel onto the chain if they can't get our
-											// update_fail_htlc in time, it's not our problem.
 										}
-									}
-								},
+									},
+								}
 							}
-						}
 
-						if !add_htlc_msgs.is_empty() || !fail_htlc_msgs.is_empty() {
-							let (commitment_msg, monitor_update) = match chan.get_mut().send_commitment(&self.logger) {
-								Ok(res) => res,
-								Err(e) => {
-									// We surely failed send_commitment due to bad keys, in that case
-									// close channel and then send error message to peer.
-									let counterparty_node_id = chan.get().get_counterparty_node_id();
-									let err: Result<(), _>  = match e {
-										ChannelError::Ignore(_) | ChannelError::Warn(_) => {
-											panic!("Stated return value requirements in send_commitment() were not met");
-										}
-										ChannelError::Close(msg) => {
-											log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
-											let mut channel = remove_channel!(self, channel_state, chan);
-											// ChannelClosed event is generated by handle_error for us.
-											Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
-										},
-									};
-									handle_errors.push((counterparty_node_id, err));
-									continue;
-								}
-							};
-							match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
-								ChannelMonitorUpdateStatus::Completed => {},
-								e => {
-									handle_errors.push((chan.get().get_counterparty_node_id(), handle_monitor_update_res!(self, e, chan, RAACommitmentOrder::CommitmentFirst, false, true)));
-									continue;
+							if !add_htlc_msgs.is_empty() || !fail_htlc_msgs.is_empty() {
+								let (commitment_msg, monitor_update) = match chan.get_mut().send_commitment(&self.logger) {
+									Ok(res) => res,
+									Err(e) => {
+										// We surely failed send_commitment due to bad keys, in that case
+										// close channel and then send error message to peer.
+										let counterparty_node_id = chan.get().get_counterparty_node_id();
+										let err: Result<(), _>  = match e {
+											ChannelError::Ignore(_) | ChannelError::Warn(_) => {
+												panic!("Stated return value requirements in send_commitment() were not met");
+											}
+											ChannelError::Close(msg) => {
+												log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
+												let mut channel = remove_channel!(self, chan);
+												// ChannelClosed event is generated by handle_error for us.
+												Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
+											},
+										};
+										handle_errors.push((counterparty_node_id, err));
+										continue;
+									}
+								};
+								match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
+									ChannelMonitorUpdateStatus::Completed => {},
+									e => {
+										handle_errors.push((chan.get().get_counterparty_node_id(), handle_monitor_update_res!(self, e, chan, RAACommitmentOrder::CommitmentFirst, false, true)));
+										continue;
+									}
 								}
+								log_debug!(self.logger, "Forwarding HTLCs resulted in a commitment update with {} HTLCs added and {} HTLCs failed for channel {}",
+									add_htlc_msgs.len(), fail_htlc_msgs.len(), log_bytes!(chan.get().channel_id()));
+								channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
+									node_id: chan.get().get_counterparty_node_id(),
+									updates: msgs::CommitmentUpdate {
+										update_add_htlcs: add_htlc_msgs,
+										update_fulfill_htlcs: Vec::new(),
+										update_fail_htlcs: fail_htlc_msgs,
+										update_fail_malformed_htlcs: Vec::new(),
+										update_fee: None,
+										commitment_signed: commitment_msg,
+									},
+								});
 							}
-							log_debug!(self.logger, "Forwarding HTLCs resulted in a commitment update with {} HTLCs added and {} HTLCs failed for channel {}",
-								add_htlc_msgs.len(), fail_htlc_msgs.len(), log_bytes!(chan.get().channel_id()));
-							channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
-								node_id: chan.get().get_counterparty_node_id(),
-								updates: msgs::CommitmentUpdate {
-									update_add_htlcs: add_htlc_msgs,
-									update_fulfill_htlcs: Vec::new(),
-									update_fail_htlcs: fail_htlc_msgs,
-									update_fail_malformed_htlcs: Vec::new(),
-									update_fee: None,
-									commitment_signed: commitment_msg,
-								},
-							});
 						}
-					} else {
-						unreachable!();
 					}
 				} else {
 					for forward_info in pending_forwards.drain(..) {
@@ -4310,7 +4331,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					return ClaimFundsFromHop::MonitorUpdateFail(counterparty_node_id, res, None);
 				},
 			}
-		} else { unreachable!(); }
+		} else { return ClaimFundsFromHop::PrevHopForceClosed }
 	}
 
 	fn finalize_claims(&self, mut sources: Vec<HTLCSource>) {
@@ -4558,7 +4579,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						}
 					};
 					channel_state.pending_msg_events.push(send_msg_err_event);
-					let _ = remove_channel!(self, channel_state, channel);
+					let _ = remove_channel!(self, channel);
 					return Err(APIError::APIMisuseError { err: "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned() });
 				}
 
@@ -4638,7 +4659,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					if chan.get().get_counterparty_node_id() != *counterparty_node_id {
 						return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
 					}
-					try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration.channel_handshake_limits, &their_features), channel_state, chan);
+					try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration.channel_handshake_limits, &their_features), chan);
 					(chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id())
 				},
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))
@@ -4665,7 +4686,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					if chan.get().get_counterparty_node_id() != *counterparty_node_id {
 						return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
 					}
-					(try_chan_entry!(self, chan.get_mut().funding_created(msg, best_block, &self.logger), channel_state, chan), chan.remove())
+					(try_chan_entry!(self, chan.get_mut().funding_created(msg, best_block, &self.logger), chan), chan.remove())
 				},
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))
 			}
@@ -4738,7 +4759,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					}
 					let (monitor, funding_tx, channel_ready) = match chan.get_mut().funding_signed(&msg, best_block, &self.logger) {
 						Ok(update) => update,
-						Err(e) => try_chan_entry!(self, Err(e), channel_state, chan),
+						Err(e) => try_chan_entry!(self, Err(e), chan),
 					};
 					match self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor) {
 						ChannelMonitorUpdateStatus::Completed => {},
@@ -4777,7 +4798,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
 				}
 				let announcement_sigs_opt = try_chan_entry!(self, chan.get_mut().channel_ready(&msg, self.get_our_node_id(),
-					self.genesis_hash.clone(), &self.best_block.read().unwrap(), &self.logger), channel_state, chan);
+					self.genesis_hash.clone(), &self.best_block.read().unwrap(), &self.logger), chan);
 				if let Some(announcement_sigs) = announcement_sigs_opt {
 					log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(chan.get().channel_id()));
 					channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
@@ -4825,7 +4846,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 							if chan_entry.get().sent_shutdown() { " after we initiated shutdown" } else { "" });
 					}
 
-					let (shutdown, monitor_update, htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.keys_manager, &their_features, &msg), channel_state, chan_entry);
+					let (shutdown, monitor_update, htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.keys_manager, &their_features, &msg), chan_entry);
 					dropped_htlcs = htlcs;
 
 					// Update the monitor with the shutdown script if necessary.
@@ -4834,7 +4855,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						let (result, is_permanent) =
 							handle_monitor_update_res!(self, update_res, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
 						if is_permanent {
-							remove_channel!(self, channel_state, chan_entry);
+							remove_channel!(self, chan_entry);
 							break result;
 						}
 					}
@@ -4869,7 +4890,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					if chan_entry.get().get_counterparty_node_id() != *counterparty_node_id {
 						return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
 					}
-					let (closing_signed, tx) = try_chan_entry!(self, chan_entry.get_mut().closing_signed(&self.fee_estimator, &msg), channel_state, chan_entry);
+					let (closing_signed, tx) = try_chan_entry!(self, chan_entry.get_mut().closing_signed(&self.fee_estimator, &msg), chan_entry);
 					if let Some(msg) = closing_signed {
 						channel_state.pending_msg_events.push(events::MessageSendEvent::SendClosingSigned {
 							node_id: counterparty_node_id.clone(),
@@ -4882,7 +4903,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						// also implies there are no pending HTLCs left on the channel, so we can
 						// fully delete it from tracking (the channel monitor is still around to
 						// watch for old state broadcasts)!
-						(tx, Some(remove_channel!(self, channel_state, chan_entry)))
+						(tx, Some(remove_channel!(self, chan_entry)))
 					} else { (tx, None) }
 				},
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
@@ -4946,7 +4967,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						_ => pending_forward_info
 					}
 				};
-				try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status, &self.logger), channel_state, chan);
+				try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status, &self.logger), chan);
 			},
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
 		}
@@ -4962,7 +4983,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					if chan.get().get_counterparty_node_id() != *counterparty_node_id {
 						return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
 					}
-					try_chan_entry!(self, chan.get_mut().update_fulfill_htlc(&msg), channel_state, chan)
+					try_chan_entry!(self, chan.get_mut().update_fulfill_htlc(&msg), chan)
 				},
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
 			}
@@ -4979,7 +5000,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				if chan.get().get_counterparty_node_id() != *counterparty_node_id {
 					return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
 				}
-				try_chan_entry!(self, chan.get_mut().update_fail_htlc(&msg, HTLCFailReason::LightningError { err: msg.reason.clone() }), channel_state, chan);
+				try_chan_entry!(self, chan.get_mut().update_fail_htlc(&msg, HTLCFailReason::LightningError { err: msg.reason.clone() }), chan);
 			},
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
 		}
@@ -4996,9 +5017,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				}
 				if (msg.failure_code & 0x8000) == 0 {
 					let chan_err: ChannelError = ChannelError::Close("Got update_fail_malformed_htlc with BADONION not set".to_owned());
-					try_chan_entry!(self, Err(chan_err), channel_state, chan);
+					try_chan_entry!(self, Err(chan_err), chan);
 				}
-				try_chan_entry!(self, chan.get_mut().update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() }), channel_state, chan);
+				try_chan_entry!(self, chan.get_mut().update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() }), chan);
 				Ok(())
 			},
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
@@ -5015,11 +5036,11 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				}
 				let (revoke_and_ack, commitment_signed, monitor_update) =
 					match chan.get_mut().commitment_signed(&msg, &self.logger) {
-						Err((None, e)) => try_chan_entry!(self, Err(e), channel_state, chan),
+						Err((None, e)) => try_chan_entry!(self, Err(e), chan),
 						Err((Some(update), e)) => {
 							assert!(chan.get().is_awaiting_monitor_update());
 							let _ = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), update);
-							try_chan_entry!(self, Err(e), channel_state, chan);
+							try_chan_entry!(self, Err(e), chan);
 							unreachable!();
 						},
 						Ok(res) => res
@@ -5102,7 +5123,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					}
 					let was_paused_for_mon_update = chan.get().is_awaiting_monitor_update();
 					let raa_updates = break_chan_entry!(self,
-						chan.get_mut().revoke_and_ack(&msg, &self.logger), channel_state, chan);
+						chan.get_mut().revoke_and_ack(&msg, &self.logger), chan);
 					htlcs_to_fail = raa_updates.holding_cell_failed_htlcs;
 					let update_res = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), raa_updates.monitor_update);
 					if was_paused_for_mon_update {
@@ -5162,7 +5183,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 				if chan.get().get_counterparty_node_id() != *counterparty_node_id {
 					return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
 				}
-				try_chan_entry!(self, chan.get_mut().update_fee(&self.fee_estimator, &msg), channel_state, chan);
+				try_chan_entry!(self, chan.get_mut().update_fee(&self.fee_estimator, &msg), chan);
 			},
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
 		}
@@ -5184,7 +5205,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
 				channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement {
 					msg: try_chan_entry!(self, chan.get_mut().announcement_signatures(
-						self.get_our_node_id(), self.genesis_hash.clone(), self.best_block.read().unwrap().height(), msg), channel_state, chan),
+						self.get_our_node_id(), self.genesis_hash.clone(), self.best_block.read().unwrap().height(), msg), chan),
 					// Note that announcement_signatures fails if the channel cannot be announced,
 					// so get_channel_update_for_broadcast will never fail by the time we get here.
 					update_msg: self.get_channel_update_for_broadcast(chan.get()).unwrap(),
@@ -5223,10 +5244,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					return Ok(NotifyOption::SkipPersist);
 				} else {
 					log_debug!(self.logger, "Received channel_update for channel {}.", log_bytes!(chan_id));
-					try_chan_entry!(self, chan.get_mut().channel_update(&msg), channel_state, chan);
+					try_chan_entry!(self, chan.get_mut().channel_update(&msg), chan);
 				}
 			},
-			hash_map::Entry::Vacant(_) => unreachable!()
+			hash_map::Entry::Vacant(_) => return Ok(NotifyOption::SkipPersist)
 		}
 		Ok(NotifyOption::DoPersist)
 	}
@@ -5248,7 +5269,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 					// add-HTLCs on disconnect, we may be handed HTLCs to fail backwards here.
 					let responses = try_chan_entry!(self, chan.get_mut().channel_reestablish(
 						msg, &self.logger, self.our_network_pubkey.clone(), self.genesis_hash,
-						&*self.best_block.read().unwrap()), channel_state, chan);
+						&*self.best_block.read().unwrap()), chan);
 					let mut channel_update = None;
 					if let Some(msg) = responses.shutdown_msg {
 						channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
@@ -5312,7 +5333,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 						let by_id = &mut channel_state.by_id;
 						let pending_msg_events = &mut channel_state.pending_msg_events;
 						if let hash_map::Entry::Occupied(chan_entry) = by_id.entry(funding_outpoint.to_channel_id()) {
-							let mut chan = remove_channel!(self, channel_state, chan_entry);
+							let mut chan = remove_channel!(self, chan_entry);
 							failed_channels.push(chan.force_shutdown(false));
 							if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 								pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {