Add support for opt_shutdown_anysegwit feature lightningdevkit#780

* Implemented protocol.
* Made feature required for all.

lightning/src/ln/channel.rs

@@ -691,11 +691,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			return Err(ChannelError::Close("Insufficient funding amount for initial commitment".to_owned()));
 		}
 
+		// TODO duplicate code new_from_req / accept_channel
+		if !their_features.requires_shutdown_anysegwit() {
+			return Err(ChannelError::Close("Peer opt_shutdown_anysegwit is required".to_owned()));
+		}
+
 		let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
 			match &msg.shutdown_scriptpubkey {
 				&OptionalField::Present(ref script) => {
 					// 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() {
+					// TODO duplicate code new_from_req / accept_channel
+					if script.is_witness_program() || 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 {
@@ -1388,11 +1394,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth)));
 		}
 
+		// TODO duplicate code new_from_req / accept_channel
+		if !their_features.requires_shutdown_anysegwit() {
+			return Err(ChannelError::Close("Peer opt_shutdown_anysegwit is required".to_owned()));
+		}
+
 		let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
 			match &msg.shutdown_scriptpubkey {
 				&OptionalField::Present(ref script) => {
 					// 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() {
+					// TODO duplicate code new_from_req / accept_channel
+					if script.is_witness_program() || 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 {
@@ -2906,13 +2918,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0);
 
 		// BOLT 2 says we must only send a scriptpubkey of certain standard forms, which are up to
-		// 34 bytes in length, so don't let the remote peer feed us some super fee-heavy script.
-		if self.is_outbound() && msg.scriptpubkey.len() > 34 {
+		// 42 bytes in length, so don't let the remote peer feed us some super fee-heavy script.
+		if self.is_outbound() && msg.scriptpubkey.len() > 42 {
 			return Err(ChannelError::Close(format!("Got counterparty shutdown_scriptpubkey ({}) of absurd length from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
 		}
 
 		//Check counterparty_shutdown_scriptpubkey form as BOLT says we must
-		if !msg.scriptpubkey.is_p2pkh() && !msg.scriptpubkey.is_p2sh() && !msg.scriptpubkey.is_v0_p2wpkh() && !msg.scriptpubkey.is_v0_p2wsh() {
+		if !msg.scriptpubkey.is_witness_program() && !msg.scriptpubkey.is_p2pkh() && !msg.scriptpubkey.is_p2sh() && !msg.scriptpubkey.is_v0_p2wpkh() && !msg.scriptpubkey.is_v0_p2wsh() {
 			return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
 		}
 
@@ -4487,7 +4499,7 @@ mod tests {
 	use bitcoin::hashes::hex::FromHex;
 	use hex;
 	use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
-	use ln::channel::{Channel,ChannelKeys,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,TxCreationKeys};
+	use ln::channel::{Channel, ChannelKeys, InboundHTLCOutput, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCOutputInCommitment, TxCreationKeys, ChannelError};
 	use ln::channel::MAX_FUNDING_SATOSHIS;
 	use ln::features::InitFeatures;
 	use ln::msgs::{OptionalField, DataLossProtect, DecodeError};
@@ -5235,4 +5247,29 @@ mod tests {
 		assert_eq!(chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_secret, &base_secret).unwrap(),
 				SecretKey::from_slice(&hex::decode("d09ffff62ddb2297ab000cc85bcb4283fdeb6aa052affbc9dddcf33b61078110").unwrap()[..]).unwrap());
 	}
+
+	#[test]
+	fn test_mandatory_shutdown_anysegwit() {
+		let feeest = TestFeeEstimator{fee_est: 15000};
+		let secp_ctx = Secp256k1::new();
+		let seed = [42; 32];
+		let network = Network::Testnet;
+		let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
+
+		// Go through the flow of opening a channel between two nodes.
+
+		// Create Node A's channel pointing to Node B's pubkey
+		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
+		let config = UserConfig::default();
+		let node_a_chan = Channel::<EnforcingChannelKeys>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap();
+
+		// Create Node B's channel by receiving Node A's open_channel message
+		let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
+		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
+		let err = Channel::<EnforcingChannelKeys>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known().clear_shutdown_anysegwit(), &open_channel_msg, 7, &config).err().unwrap();
+		match err {
+			ChannelError::Close(msg) => assert_eq!(msg, "Peer opt_shutdown_anysegwit is required".to_owned()),
+			_ => panic!("Unexpected error message")
+		}
+	}
 }

lightning/src/ln/features.rs

@@ -101,6 +101,8 @@ mod sealed {
 			StaticRemoteKey,
 			// Byte 2
 			,
+			// Byte 3
+			ShutdownAnySegwit,
 		],
 		optional_features: [
 			// Byte 0
@@ -109,6 +111,8 @@ mod sealed {
 			VariableLengthOnion | PaymentSecret,
 			// Byte 2
 			BasicMPP,
+			// Byte 3
+			,
 		],
 	});
 	define_context!(NodeContext {
@@ -119,6 +123,8 @@ mod sealed {
 			StaticRemoteKey,
 			// Byte 2
 			,
+			// Byte 3
+			ShutdownAnySegwit,
 		],
 		optional_features: [
 			// Byte 0
@@ -127,6 +133,8 @@ mod sealed {
 			VariableLengthOnion | PaymentSecret,
 			// Byte 2
 			BasicMPP,
+			// Byte 3
+			,
 		],
 	});
 	define_context!(ChannelContext {
@@ -253,6 +261,8 @@ mod sealed {
 		"Feature flags for `payment_secret`.");
 	define_feature!(17, BasicMPP, [InitContext, NodeContext],
 		"Feature flags for `basic_mpp`.");
+	define_feature!(25, ShutdownAnySegwit, [InitContext, NodeContext],
+		"Feature flags for `opt_shutdown_anysegwit`.");
 
 	#[cfg(test)]
 	define_context!(TestingContext {
@@ -550,6 +560,17 @@ impl<T: sealed::BasicMPP> Features<T> {
 	}
 }
 
+impl<T: sealed::ShutdownAnySegwit> Features<T> {
+	pub(crate) fn requires_shutdown_anysegwit(&self) -> bool {
+		<T as sealed::ShutdownAnySegwit>::requires_feature(&self.flags)
+	}
+	#[cfg(test)]
+	pub(crate) fn clear_shutdown_anysegwit(mut self) -> Self {
+		<T as sealed::ShutdownAnySegwit>::clear_bits(&mut self.flags);
+		self
+	}
+}
+
 impl<T: sealed::Context> Writeable for Features<T> {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
 		w.size_hint(self.flags.len() + 2);
@@ -620,6 +641,9 @@ mod tests {
 		assert!(!InitFeatures::known().requires_basic_mpp());
 		assert!(!NodeFeatures::known().requires_basic_mpp());
 
+		assert!(InitFeatures::known().requires_shutdown_anysegwit());
+		assert!(NodeFeatures::known().requires_shutdown_anysegwit());
+
 		let mut init_features = InitFeatures::known();
 		assert!(init_features.initial_routing_sync());
 		init_features.clear_initial_routing_sync();
@@ -658,10 +682,12 @@ mod tests {
 			// - option_data_loss_protect
 			// - var_onion_optin | static_remote_key (req) | payment_secret
 			// - basic_mpp
-			assert_eq!(node_features.flags.len(), 3);
+			// - opt_shutdown_anysegwit
+			assert_eq!(node_features.flags.len(), 4);
 			assert_eq!(node_features.flags[0], 0b00000010);
 			assert_eq!(node_features.flags[1], 0b10010010);
 			assert_eq!(node_features.flags[2], 0b00000010);
+			assert_eq!(node_features.flags[3], 0b00000001);
 		}
 
 		// Check that cleared flags are kept blank when converting back:
@@ -673,4 +699,10 @@ mod tests {
 		assert!(!features.supports_upfront_shutdown_script());
 		assert!(!init_features.supports_gossip_queries());
 	}
+
+	#[test]
+	fn sanity_test_shutdown_anysegwit() {
+		let features = InitFeatures::known().clear_shutdown_anysegwit();
+		assert!(!features.requires_shutdown_anysegwit());
+	}
 }