Return ClosureReason from Channel chain update methods

This fixes a few `ClosureReason`s and allows us to have
finer-grained user-visible errors when a channel closes due to an
on-chain event.

lightning/src/ln/channel.rs

@@ -35,6 +35,7 @@ use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{Sign, KeysInterface};
+use util::events::ClosureReason;
 use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use util::logger::Logger;
 use util::errors::APIError;
@@ -4018,7 +4019,8 @@ impl<Signer: Sign> Channel<Signer> {
 	/// In the first case, we store the confirmation height and calculating the short channel id.
 	/// In the second, we simply return an Err indicating we need to be force-closed now.
 	pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, logger: &L)
-			-> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> where L::Target: Logger {
+	-> Result<Option<msgs::FundingLocked>, (msgs::ErrorMessage, ClosureReason)>
+	where L::Target: Logger {
 		let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
 		for &(index_in_block, tx) in txdata.iter() {
 			if let Some(funding_txo) = self.get_funding_txo() {
@@ -4039,10 +4041,11 @@ impl<Signer: Sign> Channel<Signer> {
 								panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
 							}
 							self.update_time_counter += 1;
-							return Err(msgs::ErrorMessage {
+							let err_reason = "funding tx had wrong script/value or output index";
+							return Err((msgs::ErrorMessage {
 								channel_id: self.channel_id(),
-								data: "funding tx had wrong script/value or output index".to_owned()
-							});
+								data: err_reason.to_owned()
+							}, ClosureReason::ProcessingError { err: err_reason.to_owned() }));
 						} else {
 							if self.is_outbound() {
 								for input in tx.input.iter() {
@@ -4073,10 +4076,10 @@ impl<Signer: Sign> Channel<Signer> {
 				for inp in tx.input.iter() {
 					if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
 						log_info!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(self.channel_id()));
-						return Err(msgs::ErrorMessage {
+						return Err((msgs::ErrorMessage {
 							channel_id: self.channel_id(),
 							data: "Commitment or closing transaction was confirmed on chain.".to_owned()
-						});
+						}, ClosureReason::CommitmentTxConfirmed));
 					}
 				}
 			}
@@ -4096,7 +4099,8 @@ impl<Signer: Sign> Channel<Signer> {
 	/// May return some HTLCs (and their payment_hash) which have timed out and should be failed
 	/// back.
 	pub fn best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, logger: &L)
-			-> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> where L::Target: Logger {
+	-> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), (msgs::ErrorMessage, ClosureReason)>
+	where L::Target: Logger {
 		let mut timed_out_htlcs = Vec::new();
 		let unforwarded_htlc_cltv_limit = height + HTLC_FAIL_BACK_BUFFER;
 		self.holding_cell_htlc_updates.retain(|htlc_update| {
@@ -4134,10 +4138,12 @@ impl<Signer: Sign> Channel<Signer> {
 			// close the channel and hope we can get the latest state on chain (because presumably
 			// the funding transaction is at least still in the mempool of most nodes).
 			if funding_tx_confirmations < self.minimum_depth.unwrap() as i64 / 2 {
-				return Err(msgs::ErrorMessage {
+				let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.",
+					self.minimum_depth.unwrap(), funding_tx_confirmations);
+				return Err((msgs::ErrorMessage {
 					channel_id: self.channel_id(),
-					data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth.unwrap(), funding_tx_confirmations),
-				});
+					data: err_reason.clone(),
+				}, ClosureReason::ProcessingError { err: err_reason }));
 			}
 		}
 
@@ -4147,7 +4153,8 @@ impl<Signer: Sign> Channel<Signer> {
 	/// Indicates the funding transaction is no longer confirmed in the main chain. This may
 	/// force-close the channel, but may also indicate a harmless reorganization of a block or two
 	/// before the channel has reached funding_locked and we can just wait for more blocks.
-	pub fn funding_transaction_unconfirmed<L: Deref>(&mut self, logger: &L) -> Result<(), msgs::ErrorMessage> where L::Target: Logger {
+	pub fn funding_transaction_unconfirmed<L: Deref>(&mut self, logger: &L)
+	-> Result<(), (msgs::ErrorMessage, ClosureReason)> where L::Target: Logger {
 		if self.funding_tx_confirmation_height != 0 {
 			// We handle the funding disconnection by calling best_block_updated with a height one
 			// below where our funding was connected, implying a reorg back to conf_height - 1.

lightning/src/ln/channelmanager.rs

@@ -4472,7 +4472,7 @@ where
 	/// Calls a function which handles an on-chain event (blocks dis/connected, transactions
 	/// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by
 	/// the function.
-	fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage>>
+	fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), (msgs::ErrorMessage, ClosureReason)>>
 			(&self, height_opt: Option<u32>, f: FN) {
 		// Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
 		// during initialization prior to the chain_monitor being fully configured in some cases.
@@ -4517,7 +4517,7 @@ where
 						}
 						short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
 					}
-				} else if let Err(e) = res {
+				} else if let Err((e, reason)) = res {
 					if let Some(short_id) = channel.get_short_channel_id() {
 						short_to_id.remove(&short_id);
 					}
@@ -4529,7 +4529,7 @@ where
 							msg: update
 						});
 					}
-					self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(),  reason: ClosureReason::CommitmentTxConfirmed });
+					self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(),  reason });
 					pending_msg_events.push(events::MessageSendEvent::HandleError {
 						node_id: channel.get_counterparty_node_id(),
 						action: msgs::ErrorAction::SendErrorMessage { msg: e },

lightning/src/ln/functional_tests.rs

@@ -9110,15 +9110,16 @@ fn test_invalid_funding_tx() {
 	assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0], tx);
 	nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
 
+	let expected_err = "funding tx had wrong script/value or output index";
 	confirm_transaction_at(&nodes[1], &tx, 1);
-	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
+	check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: expected_err.to_string() });
 	check_added_monitors!(nodes[1], 1);
 	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
 	assert_eq!(events_2.len(), 1);
 	if let MessageSendEvent::HandleError { node_id, action } = &events_2[0] {
 		assert_eq!(*node_id, nodes[0].node.get_our_node_id());
 		if let msgs::ErrorAction::SendErrorMessage { msg } = action {
-			assert_eq!(msg.data, "funding tx had wrong script/value or output index");
+			assert_eq!(msg.data, expected_err);
 		} else { panic!(); }
 	} else { panic!(); }
 	assert_eq!(nodes[1].node.list_channels().len(), 0);

lightning/src/ln/reorg_tests.rs

@@ -300,12 +300,13 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_
 	*nodes[0].chain_monitor.expect_channel_force_closed.lock().unwrap() = Some((chan.2, true));
 	nodes[0].node.test_process_background_events(); // Required to free the pending background monitor update
 	check_added_monitors!(nodes[0], 1);
-	check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed);
-	if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed {
-		check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: "Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs.".to_string() });
+	let expected_err = if connect_style == ConnectStyle::FullBlockViaListen && !use_funding_unconfirmed {
+		"Funding transaction was un-confirmed. Locked at 6 confs, now have 2 confs.".to_string()
 	} else {
-		check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: "Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.".to_string() });
-	}
+		"Funding transaction was un-confirmed. Locked at 6 confs, now have 0 confs.".to_string()
+	};
+	check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: expected_err.clone() });
+	check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: expected_err });
 	assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
 	nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();