contracts: Replace cargo feature unstable-interface with config (#1…

…2787)

* Replace cargo feature with config

* Update frame/contracts/proc-macro/src/lib.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

bin/node/runtime/Cargo.toml

@@ -312,9 +312,6 @@ try-runtime = [
 	"pallet-vesting/try-runtime",
 	"pallet-whitelist/try-runtime",
 ]
-# Make contract callable functions marked as __unstable__ available. Do not enable
-# on live chains as those are subject to change.
-contracts-unstable-interface = ["pallet-contracts/unstable-interface"]
 # Force `sp-sandbox` to call into the host resident executor. One still need to make sure
 # that `sc-executor` gets the `wasmer-sandbox` feature which happens automatically when
 # specified on the command line.

bin/node/runtime/src/lib.rs

@@ -32,7 +32,7 @@ use frame_support::{
 	pallet_prelude::Get,
 	parameter_types,
 	traits::{
-		AsEnsureOriginWithArg, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse,
+		AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse,
 		EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem,
 		LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, WithdrawReasons,
 	},
@@ -1191,6 +1191,7 @@ impl pallet_contracts::Config for Runtime {
 	type AddressGenerator = pallet_contracts::DefaultAddressGenerator;
 	type MaxCodeLen = ConstU32<{ 128 * 1024 }>;
 	type MaxStorageKeyLen = ConstU32<128>;
+	type UnsafeUnstableInterface = ConstBool<false>;
 }
 
 impl pallet_sudo::Config for Runtime {

frame/contracts/Cargo.toml

@@ -84,9 +84,5 @@ runtime-benchmarks = [
 	"frame-benchmarking/runtime-benchmarks",
 	"rand",
 	"rand_pcg",
-	"unstable-interface",
 ]
 try-runtime = ["frame-support/try-runtime"]
-# Make contract callable functions marked as unstable available. Do not enable
-# on live chains as those are subject to change.
-unstable-interface = []

frame/contracts/README.md

@@ -142,18 +142,11 @@ this pallet contains the concept of an unstable interface. Akin to the rust nigh
 it allows us to add new interfaces but mark them as unstable so that contract languages can
 experiment with them and give feedback before we stabilize those.
 
-In order to access interfaces marked as `#[unstable]` in `runtime.rs` one need to compile
-this crate with the `unstable-interface` feature enabled. It should be obvious that any
-live runtime should never be compiled with this feature: In addition to be subject to
-change or removal those interfaces do not have proper weights associated with them and
-are therefore considered unsafe.
-
-The substrate runtime exposes this feature as `contracts-unstable-interface`. Example
-commandline for running the substrate node with unstable contracts interfaces:
-
-```bash
-cargo run --release --features contracts-unstable-interface -- --dev
-```
+In order to access interfaces marked as `#[unstable]` in `runtime.rs` one need to set
+`pallet_contracts::Config::UnsafeUnstableInterface` to `ConstU32<true>`. It should be obvious
+that any production runtime should never be compiled with this feature: In addition to be
+subject to change or removal those interfaces might not have proper weights associated with
+them and are therefore considered unsafe.
 
 New interfaces are generally added as unstable and might go through several iterations
 before they are promoted to a stable interface.

frame/contracts/proc-macro/src/lib.rs

@@ -157,7 +157,7 @@ struct HostFn {
 	module: String,
 	name: String,
 	returns: HostFnReturn,
-	is_unstable: bool,
+	is_stable: bool,
 }
 
 enum HostFnReturn {
@@ -199,7 +199,7 @@ impl HostFn {
 		attrs.retain(|a| !(a.path.is_ident("doc") || a.path.is_ident("prefixed_alias")));
 		let name = item.sig.ident.to_string();
 		let mut maybe_module = None;
-		let mut is_unstable = false;
+		let mut is_stable = true;
 		while let Some(attr) = attrs.pop() {
 			let ident = attr.path.get_ident().ok_or(err(span, msg))?.to_string();
 			match ident.as_str() {
@@ -212,10 +212,10 @@ impl HostFn {
 					maybe_module = Some(format!("seal{}", ver));
 				},
 				"unstable" => {
-					if is_unstable {
+					if !is_stable {
 						return Err(err(span, "#[unstable] can only be specified once"))
 					}
-					is_unstable = true;
+					is_stable = false;
 				},
 				_ => return Err(err(span, msg)),
 			}
@@ -312,7 +312,7 @@ impl HostFn {
 							module: maybe_module.unwrap_or_else(|| "seal0".to_string()),
 							name,
 							returns,
-							is_unstable,
+							is_stable,
 						})
 					},
 					_ => Err(err(span, &msg)),
@@ -406,15 +406,15 @@ fn expand_impls(def: &mut EnvDef) -> TokenStream2 {
 			<E::T as ::frame_system::Config>::AccountId:
 				::sp_core::crypto::UncheckedFrom<<E::T as ::frame_system::Config>::Hash> + ::core::convert::AsRef<[::core::primitive::u8]>,
 		{
-			fn define(store: &mut ::wasmi::Store<crate::wasm::Runtime<E>>, linker: &mut ::wasmi::Linker<crate::wasm::Runtime<E>>) -> Result<(), ::wasmi::errors::LinkerError> {
+			fn define(store: &mut ::wasmi::Store<crate::wasm::Runtime<E>>, linker: &mut ::wasmi::Linker<crate::wasm::Runtime<E>>, allow_unstable: bool) -> Result<(), ::wasmi::errors::LinkerError> {
 				#impls
 				Ok(())
 			}
 		}
 
 		impl crate::wasm::Environment<()> for Env
 		{
-			fn define(store: &mut ::wasmi::Store<()>, linker: &mut ::wasmi::Linker<()>) -> Result<(), ::wasmi::errors::LinkerError> {
+			fn define(store: &mut ::wasmi::Store<()>, linker: &mut ::wasmi::Linker<()>, allow_unstable: bool) -> Result<(), ::wasmi::errors::LinkerError> {
 				#dummy_impls
 				Ok(())
 			}
@@ -437,10 +437,7 @@ fn expand_functions(
 			f.returns.to_wasm_sig(),
 			&f.item.sig.output
 		);
-		let unstable_feat = match f.is_unstable {
-			true => quote! { #[cfg(feature = "unstable-interface")] },
-			false => quote! {},
-		};
+		let is_stable = f.is_stable;
 
 		// If we don't expand blocks (implementing for `()`) we change a few things:
 		// - We replace any code by unreachable!
@@ -480,16 +477,18 @@ fn expand_functions(
 			quote! { #[allow(unused_variables)] }
 		};
 
-
 		quote! {
-			#unstable_feat
-			#allow_unused
-			linker.define(#module, #name, ::wasmi::Func::wrap(&mut*store, |mut __caller__: ::wasmi::Caller<#host_state>, #( #params, )*| -> #wasm_output {
-				let mut func = #inner;
-				func()
-					.map_err(#map_err)
-					.map(::core::convert::Into::into)
-			}))?;
+			// We need to allow unstable functions when runtime benchmarks are performed because
+			// we generate the weights even when those interfaces are not enabled.
+			if ::core::cfg!(feature = "runtime-benchmarks") || #is_stable || allow_unstable {
+				#allow_unused
+				linker.define(#module, #name, ::wasmi::Func::wrap(&mut*store, |mut __caller__: ::wasmi::Caller<#host_state>, #( #params, )*| -> #wasm_output {
+					let mut func = #inner;
+					func()
+						.map_err(#map_err)
+						.map(::core::convert::Into::into)
+				}))?;
+			}
 		}
 	});
 	quote! {

frame/contracts/src/benchmarking/sandbox.rs

@@ -64,7 +64,7 @@ where
 struct EmptyEnv;
 
 impl Environment<()> for EmptyEnv {
-	fn define(_store: &mut Store<()>, _linker: &mut Linker<()>) -> Result<(), LinkerError> {
+	fn define(_: &mut Store<()>, _: &mut Linker<()>, _: bool) -> Result<(), LinkerError> {
 		Ok(())
 	}
 }

frame/contracts/src/lib.rs

@@ -326,10 +326,24 @@ pub mod pallet {
 		/// The maximum length of a contract code in bytes. This limit applies to the instrumented
 		/// version of the code. Therefore `instantiate_with_code` can fail even when supplying
 		/// a wasm binary below this maximum size.
+		#[pallet::constant]
 		type MaxCodeLen: Get<u32>;
 
 		/// The maximum allowable length in bytes for storage keys.
+		#[pallet::constant]
 		type MaxStorageKeyLen: Get<u32>;
+
+		/// Make contract callable functions marked as `#[unstable]` available.
+		///
+		/// Contracts that use `#[unstable]` functions won't be able to be uploaded unless
+		/// this is set to `true`. This is only meant for testnets and dev nodes in order to
+		/// experiment with new features.
+		///
+		/// # Warning
+		///
+		/// Do **not** set to `true` on productions chains.
+		#[pallet::constant]
+		type UnsafeUnstableInterface: Get<bool>;
 	}
 
 	#[pallet::hooks]

frame/contracts/src/tests.rs

@@ -122,6 +122,12 @@ pub mod test_utils {
 	}
 }
 
+impl Test {
+	pub fn set_unstable_interface(unstable_interface: bool) {
+		UNSTABLE_INTERFACE.with(|v| *v.borrow_mut() = unstable_interface);
+	}
+}
+
 parameter_types! {
 	static TestExtensionTestValue: TestExtension = Default::default();
 }
@@ -385,6 +391,7 @@ impl Contains<RuntimeCall> for TestFilter {
 
 parameter_types! {
 	pub const DeletionWeightLimit: Weight = Weight::from_ref_time(500_000_000_000);
+	pub static UnstableInterface: bool = true;
 }
 
 impl Config for Test {
@@ -407,6 +414,7 @@ impl Config for Test {
 	type AddressGenerator = DefaultAddressGenerator;
 	type MaxCodeLen = ConstU32<{ 128 * 1024 }>;
 	type MaxStorageKeyLen = ConstU32<128>;
+	type UnsafeUnstableInterface = UnstableInterface;
 }
 
 pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);
@@ -2687,7 +2695,6 @@ fn gas_estimation_nested_call_fixed_limit() {
 }
 
 #[test]
-#[cfg(feature = "unstable-interface")]
 fn gas_estimation_call_runtime() {
 	use codec::Decode;
 	let (caller_code, caller_hash) = compile_module::<Test>("call_runtime").unwrap();
@@ -4411,7 +4418,6 @@ fn delegate_call_indeterministic_code() {
 }
 
 #[test]
-#[cfg(feature = "unstable-interface")]
 fn reentrance_count_works_with_call() {
 	let (wasm, code_hash) = compile_module::<Test>("reentrance_count_call").unwrap();
 	let contract_addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
@@ -4448,7 +4454,6 @@ fn reentrance_count_works_with_call() {
 }
 
 #[test]
-#[cfg(feature = "unstable-interface")]
 fn reentrance_count_works_with_delegated_call() {
 	let (wasm, code_hash) = compile_module::<Test>("reentrance_count_delegated_call").unwrap();
 	let contract_addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
@@ -4485,7 +4490,6 @@ fn reentrance_count_works_with_delegated_call() {
 }
 
 #[test]
-#[cfg(feature = "unstable-interface")]
 fn account_reentrance_count_works() {
 	let (wasm, code_hash) = compile_module::<Test>("account_reentrance_count_call").unwrap();
 	let (wasm_reentrance_count, code_hash_reentrance_count) =