From: Valentine Wallace <vwallace@protonmail.com>
Date: Thu, 1 Dec 2022 05:12:29 +0000 (-0500)
Subject: Clean up HTLC intercept errors
X-Git-Tag: v0.0.113~19^2~4
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=6791d2c30725832214ed374197cd1c328fd954db;p=rust-lightning

Clean up HTLC intercept errors

ChannelUnavailable is a better fit for errors regarding unavailable channels
than APIMisuseError.

Also log bytes in errors as hex instead of decimal.
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 20014f799..35b240e40 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3080,20 +3080,20 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 		let next_hop_scid = match self.channel_state.lock().unwrap().by_id.get(next_hop_channel_id) {
 			Some(chan) => {
 				if !chan.is_usable() {
-					return Err(APIError::APIMisuseError {
-						err: format!("Channel with id {:?} not fully established", next_hop_channel_id)
+					return Err(APIError::ChannelUnavailable {
+						err: format!("Channel with id {} not fully established", log_bytes!(*next_hop_channel_id))
 					})
 				}
 				chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias())
 			},
-			None => return Err(APIError::APIMisuseError {
-				err: format!("Channel with id {:?} not found", next_hop_channel_id)
+			None => return Err(APIError::ChannelUnavailable {
+				err: format!("Channel with id {} not found", log_bytes!(*next_hop_channel_id))
 			})
 		};
 
 		let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id)
 			.ok_or_else(|| APIError::APIMisuseError {
-				err: format!("Payment with intercept id {:?} not found", intercept_id.0)
+				err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0))
 			})?;
 
 		let routing = match payment.forward_info.routing {
@@ -3128,7 +3128,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
 		let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id)
 			.ok_or_else(|| APIError::APIMisuseError {
-				err: format!("Payment with InterceptId {:?} not found", intercept_id)
+				err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0))
 			})?;
 
 		if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing {
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 2da80ae3f..95bfae58b 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -1466,7 +1466,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
 
 	// Check for unknown channel id error.
 	let unknown_chan_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &[42; 32], nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
-	assert_eq!(unknown_chan_id_err , APIError::APIMisuseError { err: format!("Channel with id {:?} not found", [42; 32]) });
+	assert_eq!(unknown_chan_id_err , APIError::ChannelUnavailable  { err: format!("Channel with id {} not found", log_bytes!([42; 32])) });
 
 	if test == InterceptTest::Fail {
 		// Ensure we can fail the intercepted payment back.
@@ -1490,7 +1490,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
 		// Check that we'll fail as expected when sending to a channel that isn't in `ChannelReady` yet.
 		let temp_chan_id = nodes[1].node.create_channel(nodes[2].node.get_our_node_id(), 100_000, 0, 42, None).unwrap();
 		let unusable_chan_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &temp_chan_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
-		assert_eq!(unusable_chan_err , APIError::APIMisuseError { err: format!("Channel with id {:?} not fully established", temp_chan_id) });
+		assert_eq!(unusable_chan_err , APIError::ChannelUnavailable { err: format!("Channel with id {} not fully established", log_bytes!(temp_chan_id)) });
 		assert_eq!(nodes[1].node.get_and_clear_pending_msg_events().len(), 1);
 
 		// Open the just-in-time channel so the payment can then be forwarded.
@@ -1561,6 +1561,6 @@ fn do_test_intercepted_payment(test: InterceptTest) {
 		// Check for unknown intercept id error.
 		let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None);
 		let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
-		assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", intercept_id.0) });
+		assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0)) });
 	}
 }