f Enforce BOLT2 compat in new_op_return and test we fail if violated

Author: tnull

lightning/src/ln/script.rs

@@ -81,13 +81,18 @@ impl ShutdownScript {
 	/// This is only needed and valid for channels supporting `option_simple_close`. Please refer
 	/// to [BOLT-2] for more information.
 	///
-	/// Note this only supports creating a script with data of up to 76 bytes length via
-	/// [`PushBytes`].
+	/// Will return [`InvalidShutdownScript`] if the given data is not [BOLT-2] compliant based on
+	/// the given features.
 	///
 	/// [BOLT-2]: https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-negotiation-closing_complete-and-closing_sig
-	pub fn new_op_return<T: AsRef<PushBytes>>(data: T) -> Self {
+	pub fn new_op_return<T: AsRef<PushBytes>>(
+		data: T, features: &InitFeatures,
+	) -> Result<Self, InvalidShutdownScript> {
 		let script = ScriptBuf::new_op_return(data);
-		Self(ShutdownScriptImpl::Bolt2(script))
+		if !is_bolt2_compliant(&script, features) {
+			return Err(InvalidShutdownScript { script });
+		}
+		Ok(Self(ShutdownScriptImpl::Bolt2(script)))
 	}
 
 	/// Generates a witness script pubkey from the given segwit version and program.
@@ -157,9 +162,8 @@ pub(crate) fn is_bolt2_compliant(script: &Script, features: &InitFeatures) -> bo
 						return instruction_iter.next().is_none();
 					}
 
-					// While `rust-bitcoin` doesn't allow to construct `PushBytes` from arrays
-					// longer than 75 bytes, itself curiously interprets `OP_PUSHDATA1` as
-					// `Instruction::PushBytes`, having us land here in this case, too.
+					// `rust-bitcoin` interprets `OP_PUSHDATA1` as `Instruction::PushBytes`, having
+					// us land here in this case, too.
 					//
 					// * `76` followed by `76` to `80` followed by exactly that many bytes
 					if (76..=80).contains(&bytes.len()) {
@@ -228,13 +232,15 @@ mod shutdown_script_tests {
 	use super::ShutdownScript;
 
 	use bitcoin::opcodes;
-	use bitcoin::script::{Builder, ScriptBuf};
+	use bitcoin::script::{Builder, PushBytes, ScriptBuf};
 	use bitcoin::secp256k1::Secp256k1;
 	use bitcoin::secp256k1::{PublicKey, SecretKey};
 	use bitcoin::{WitnessProgram, WitnessVersion};
 
+	use crate::ln::channelmanager::provided_init_features;
 	use crate::prelude::*;
 	use crate::types::features::InitFeatures;
+	use crate::util::config::UserConfig;
 
 	fn pubkey() -> bitcoin::key::PublicKey {
 		let secp_ctx = Secp256k1::signing_only();
@@ -310,8 +316,9 @@ mod shutdown_script_tests {
 	#[test]
 	fn generates_op_return_from_data() {
 		let data = [6; 6];
+		let features = provided_init_features(&UserConfig::default());
 		let op_return_script = ScriptBuf::new_op_return(&data);
-		let shutdown_script = ShutdownScript::new_op_return(&data);
+		let shutdown_script = ShutdownScript::new_op_return(&data, &features).unwrap();
 		assert!(shutdown_script.is_compatible(&simple_close_features()));
 		assert!(!shutdown_script.is_compatible(&InitFeatures::empty()));
 		assert_eq!(shutdown_script.into_inner(), op_return_script);
@@ -370,5 +377,11 @@ mod shutdown_script_tests {
 		pushdata_vec.extend_from_slice(&[1u8; 81]);
 		let pushdata_script = ScriptBuf::from_bytes(pushdata_vec);
 		assert!(ShutdownScript::try_from(pushdata_script).is_err());
+
+		// - In `ShutdownScript::new_op_return` the OP_RETURN data is longer than 80 bytes.
+		let features = provided_init_features(&UserConfig::default());
+		let big_buffer = &[0xfau8; 81][..];
+		let push_bytes: &PushBytes = big_buffer.try_into().unwrap();
+		assert!(ShutdownScript::new_op_return(&push_bytes, &features).is_err());
 	}
 }