Coalesce all shutdown script checks into a single function

Author: galderz

lightning/src/ln/channel.rs

@@ -694,19 +694,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
 			match &msg.shutdown_scriptpubkey {
 				&OptionalField::Present(ref script) => {
-					if unsupported_witness_shutdown_script(&their_features, script) {
-						return Err(ChannelError::Close("Peer is signaling upfront_shutdown but not support for any segwit, and the script is a witness program".to_owned()));
+					if is_unsupported_shutdown_script(&their_features, script) {
+						return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex())));
 					}
 
-					// Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. We enforce it while receiving shutdown msg
-					if script.is_p2pkh() || script.is_p2sh() || script.is_v0_p2wsh() || script.is_v0_p2wpkh() {
-						Some(script.clone())
-					// Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
-					} else if script.len() == 0 {
+					if script.len() == 0 {
 						None
-					// Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel
 					} else {
-						return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex())));
+						Some(script.clone())
 					}
 				},
 				// Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
@@ -1395,19 +1390,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
 			match &msg.shutdown_scriptpubkey {
 				&OptionalField::Present(ref script) => {
-					if unsupported_witness_shutdown_script(&their_features, script) {
-						return Err(ChannelError::Close("Peer is signaling upfront_shutdown but not support for any segwit, and the script is a witness program".to_owned()));
+					if is_unsupported_shutdown_script(&their_features, script) {
+						return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex())));
 					}
 
-					// Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. We enforce it while receiving shutdown msg
-					if script.is_p2pkh() || script.is_p2sh() || script.is_v0_p2wsh() || script.is_v0_p2wpkh() {
-						Some(script.clone())
-					// Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
-					} else if script.len() == 0 {
+					if script.len() == 0 {
 						None
-					// Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel
 					} else {
-						return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. scriptpubkey: ({})", script.to_bytes().to_hex())));
+						Some(script.clone())
 					}
 				},
 				// Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
@@ -2921,7 +2911,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		}
 
 		//Check counterparty_shutdown_scriptpubkey form as BOLT says we must
-		if unsupported_witness_shutdown_script(&their_features, &msg.scriptpubkey) && !msg.scriptpubkey.is_p2pkh() && !msg.scriptpubkey.is_p2sh() && !msg.scriptpubkey.is_v0_p2wpkh() && !msg.scriptpubkey.is_v0_p2wsh() {
+		if is_unsupported_shutdown_script(&their_features, &msg.scriptpubkey) {
 			return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
 		}
 
@@ -4029,7 +4019,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 	}
 }
 
-fn unsupported_witness_shutdown_script(their_features: &InitFeatures, scriptpubkey: &Script) -> bool {
+fn is_unsupported_shutdown_script(their_features: &InitFeatures, scriptpubkey: &Script) -> bool {
+	return is_unsupported_witness_shutdown_script(their_features, scriptpubkey) && !scriptpubkey.is_p2pkh() && !scriptpubkey.is_p2sh() && !scriptpubkey.is_v0_p2wpkh() && !scriptpubkey.is_v0_p2wsh()
+}
+
+fn is_unsupported_witness_shutdown_script(their_features: &InitFeatures, scriptpubkey: &Script) -> bool {
 	return !their_features.supports_shutdown_anysegwit()
 		&& scriptpubkey.is_witness_program()
 }

lightning/src/ln/functional_tests.rs

@@ -7339,7 +7339,7 @@ fn test_upfront_shutdown_script_unsupport_segwit() {
 	match events[0] {
 		MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id } => {
 			assert_eq!(node_id, nodes[0].node.get_our_node_id());
-			assert_eq!(msg.data, "Peer is signaling upfront_shutdown but not support for any segwit, and the script is a witness program".to_owned())
+			assert!(regex::Regex::new(r"Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: (\([A-Fa-f0-9]+\))").unwrap().is_match(&*msg.data));
 		},
 		_ => panic!("Unexpected event"),
 	}