contracts: Don't put unstable functions in special module (#12781)

* Don't put unstable functions in special module

* Apply suggestions from code review

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

* cargo fmt

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

substrate/frame/contracts/Cargo.toml

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

substrate/frame/contracts/README.md

@@ -142,7 +142,7 @@ 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
+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

substrate/frame/contracts/fixtures/account_reentrance_count_call.wat

@@ -4,7 +4,7 @@
 	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
 	(import "seal0" "seal_caller" (func $seal_caller (param i32 i32)))
 	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
-	(import "__unstable__" "account_reentrance_count" (func $account_reentrance_count (param i32) (result i32)))
+	(import "seal0" "account_reentrance_count" (func $account_reentrance_count (param i32) (result i32)))
 	(import "env" "memory" (memory 1 1))
 
 	;; [0, 32) buffer where input is copied

substrate/frame/contracts/fixtures/call_runtime.wat

@@ -1,6 +1,6 @@
 ;; This passes its input to `seal_call_runtime` and returns the return value to its caller.
 (module
-	(import "__unstable__" "call_runtime" (func $call_runtime (param i32 i32) (result i32)))
+	(import "seal0" "call_runtime" (func $call_runtime (param i32 i32) (result i32)))
 	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
 	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
 	(import "env" "memory" (memory 1 1))

substrate/frame/contracts/fixtures/reentrance_count_call.wat

@@ -4,7 +4,7 @@
 	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
 	(import "seal0" "seal_address" (func $seal_address (param i32 i32)))
 	(import "seal1" "seal_call" (func $seal_call (param i32 i32 i64 i32 i32 i32 i32 i32) (result i32)))
-	(import "__unstable__" "reentrance_count" (func $reentrance_count (result i32)))
+	(import "seal0" "reentrance_count" (func $reentrance_count (result i32)))
 	(import "env" "memory" (memory 1 1))
 
 	;; [0, 32) reserved for $seal_address output

substrate/frame/contracts/fixtures/reentrance_count_delegated_call.wat

@@ -4,7 +4,7 @@
 	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
 	(import "seal0" "seal_set_storage" (func $seal_set_storage (param i32 i32 i32)))
 	(import "seal0" "seal_delegate_call" (func $seal_delegate_call (param i32 i32 i32 i32 i32 i32) (result i32)))
-	(import "__unstable__" "reentrance_count" (func $reentrance_count (result i32)))
+	(import "seal0" "reentrance_count" (func $reentrance_count (result i32)))
 	(import "env" "memory" (memory 1 1))
 
 	;; [0, 32) buffer where code hash is copied

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

@@ -157,6 +157,7 @@ struct HostFn {
 	module: String,
 	name: String,
 	returns: HostFnReturn,
+	is_unstable: bool,
 }
 
 enum HostFnReturn {
@@ -191,27 +192,34 @@ impl HostFn {
 		};
 
 		// process attributes
-		let msg = "only #[version(<u8>)] or #[unstable] attribute is allowed.";
+		let msg =
+			"only #[version(<u8>)], #[unstable] and #[prefixed_alias] attributes are allowed.";
 		let span = item.span();
 		let mut attrs = item.attrs.clone();
 		attrs.retain(|a| !(a.path.is_ident("doc") || a.path.is_ident("prefixed_alias")));
 		let name = item.sig.ident.to_string();
-		let module = match attrs.len() {
-			0 => Ok("seal0".to_string()),
-			1 => {
-				let attr = &attrs[0];
-				let ident = attr.path.get_ident().ok_or(err(span, msg))?.to_string();
-				match ident.as_str() {
-					"version" => {
-						let ver: syn::LitInt = attr.parse_args()?;
-						Ok(format!("seal{}", ver.base10_parse::<u8>().map_err(|_| err(span, msg))?))
-					},
-					"unstable" => Ok("__unstable__".to_string()),
-					_ => Err(err(span, msg)),
-				}
-			},
-			_ => Err(err(span, msg)),
-		}?;
+		let mut maybe_module = None;
+		let mut is_unstable = false;
+		while let Some(attr) = attrs.pop() {
+			let ident = attr.path.get_ident().ok_or(err(span, msg))?.to_string();
+			match ident.as_str() {
+				"version" => {
+					if maybe_module.is_some() {
+						return Err(err(span, "#[version] can only be specified once"))
+					}
+					let ver: u8 =
+						attr.parse_args::<syn::LitInt>().and_then(|lit| lit.base10_parse())?;
+					maybe_module = Some(format!("seal{}", ver));
+				},
+				"unstable" => {
+					if is_unstable {
+						return Err(err(span, "#[unstable] can only be specified once"))
+					}
+					is_unstable = true;
+				},
+				_ => return Err(err(span, msg)),
+			}
+		}
 
 		// process arguments: The first and second arg are treated differently (ctx, memory)
 		// they must exist and be `ctx: _` and `memory: _`.
@@ -299,7 +307,13 @@ impl HostFn {
 							_ => Err(err(arg1.span(), &msg)),
 						}?;
 
-						Ok(Self { item, module, name, returns })
+						Ok(Self {
+							item,
+							module: maybe_module.unwrap_or_else(|| "seal0".to_string()),
+							name,
+							returns,
+							is_unstable,
+						})
 					},
 					_ => Err(err(span, &msg)),
 				}
@@ -423,9 +437,9 @@ fn expand_functions(
 			f.returns.to_wasm_sig(),
 			&f.item.sig.output
 		);
-		let unstable_feat = match module.as_str() {
-			"__unstable__" => quote! { #[cfg(feature = "unstable-interface")] },
-			_ => quote! {},
+		let unstable_feat = match f.is_unstable {
+			true => quote! { #[cfg(feature = "unstable-interface")] },
+			false => quote! {},
 		};
 
 		// If we don't expand blocks (implementing for `()`) we change a few things:
@@ -496,13 +510,15 @@ fn expand_functions(
 /// ```nocompile
 /// #[define_env]
 /// pub mod some_env {
-/// 	fn some_host_fn(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<(), TrapReason> {
+/// 	fn foo(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<(), TrapReason> {
 /// 		ctx.some_host_fn(KeyType::Fix, key_ptr, value_ptr, value_len).map(|_| ())
 /// 	}
 /// }
 /// ```
-/// This example will expand to the `some_host_fn()` defined in the wasm module named `seal0`.
-/// To define a host function in `seal1` and `__unstable__` modules, it should be annotated with the
+/// This example will expand to the `foo()` defined in the wasm module named `seal0`. This is
+/// because the module `seal0` is the default when no module is specified.
+///
+/// To define a host function in `seal2` and `seal3` modules, it should be annotated with the
 /// appropriate attribute as follows:
 ///
 /// ## Example
@@ -510,17 +526,20 @@ fn expand_functions(
 /// ```nocompile
 /// #[define_env]
 /// pub mod some_env {
-/// 	#[version(1)]
-/// 	fn some_host_fn(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<ReturnCode, TrapReason> {
+/// 	#[version(2)]
+/// 	fn foo(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<ReturnCode, TrapReason> {
 /// 		ctx.some_host_fn(KeyType::Fix, key_ptr, value_ptr, value_len).map(|_| ())
 /// 	}
 ///
+/// 	#[version(3)]
 /// 	#[unstable]
-/// 	fn some_host_fn(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<u32, TrapReason> {
+/// 	fn bar(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<u32, TrapReason> {
 /// 		ctx.some_host_fn(KeyType::Fix, key_ptr, value_ptr, value_len).map(|_| ())
 /// 	}
 /// }
 /// ```
+/// The function `bar` is additionally annotated with `unstable` which removes it from the stable
+/// interface. Check out the README to learn about unstable functions.
 ///
 /// In legacy versions of pallet_contracts, it was a naming convention that all host functions had
 /// to be named with the `seal_` prefix. For the sake of backwards compatibility, each host function
@@ -534,21 +553,21 @@ fn expand_functions(
 /// pub mod some_env {
 /// 	#[version(1)]
 /// 	#[prefixed_alias]
-/// 	fn some_host_fn(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<ReturnCode, TrapReason> {
+/// 	fn foo(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<ReturnCode, TrapReason> {
 /// 		ctx.some_host_fn(KeyType::Fix, key_ptr, value_ptr, value_len).map(|_| ())
 /// 	}
 ///
-/// 	#[unstable]
-/// 	fn some_host_fn(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<u32, TrapReason> {
+/// 	#[version(42)]
+/// 	fn bar(ctx: _, memory: _, key_ptr: u32, value_ptr: u32, value_len: u32) -> Result<u32, TrapReason> {
 /// 		ctx.some_host_fn(KeyType::Fix, key_ptr, value_ptr, value_len).map(|_| ())
 /// 	}
 /// }
 /// ```
 ///
 /// In this example, the following host functions will be generated by the macro:
-/// - `some_host_fn()` in module `seal1`,
-/// - `seal_some_host_fn()` in module `seal1`,
-/// - `some_host_fn()` in module `__unstable__`.
+/// - `foo()` in module `seal1`,
+/// - `seal_foo()` in module `seal1`,
+/// - `bar()` in module `seal42`.
 ///
 /// Only following return types are allowed for the host functions defined with the macro:
 /// - `Result<(), TrapReason>`,

substrate/frame/contracts/src/benchmarking/mod.rs

@@ -1366,7 +1366,7 @@ benchmarks! {
 		let code = WasmModule::<T>::from(ModuleDefinition {
 			memory: Some(ImportedMemory::max::<T>()),
 			imported_functions: vec![ImportedFunction {
-				module: "__unstable__",
+				module: "seal0",
 				name: "take_storage",
 				params: vec![ValueType::I32, ValueType::I32, ValueType::I32, ValueType::I32],
 				return_type: Some(ValueType::I32),
@@ -1420,7 +1420,7 @@ benchmarks! {
 		let code = WasmModule::<T>::from(ModuleDefinition {
 			memory: Some(ImportedMemory::max::<T>()),
 			imported_functions: vec![ImportedFunction {
-				module: "__unstable__",
+				module: "seal0",
 				name: "take_storage",
 				params: vec![ValueType::I32, ValueType::I32, ValueType::I32, ValueType::I32],
 				return_type: Some(ValueType::I32),
@@ -2090,7 +2090,7 @@ benchmarks! {
 		let code = WasmModule::<T>::from(ModuleDefinition {
 			memory: Some(ImportedMemory::max::<T>()),
 			imported_functions: vec![ImportedFunction {
-				module: "__unstable__",
+				module: "seal0",
 				name: "reentrance_count",
 				params: vec![],
 				return_type: Some(ValueType::I32),
@@ -2116,7 +2116,7 @@ benchmarks! {
 		let code = WasmModule::<T>::from(ModuleDefinition {
 			memory: Some(ImportedMemory::max::<T>()),
 			imported_functions: vec![ImportedFunction {
-				module: "__unstable__",
+				module: "seal0",
 				name: "account_reentrance_count",
 				params: vec![ValueType::I32],
 				return_type: Some(ValueType::I32),

substrate/frame/contracts/src/wasm/mod.rs

@@ -2277,7 +2277,7 @@ mod tests {
 	#[cfg(feature = "unstable-interface")]
 	const CODE_CALL_RUNTIME: &str = r#"
 (module
-	(import "__unstable__" "call_runtime" (func $call_runtime (param i32 i32) (result i32)))
+	(import "seal0" "call_runtime" (func $call_runtime (param i32 i32) (result i32)))
 	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
 	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
 	(import "env" "memory" (memory 1 1))
@@ -2592,7 +2592,7 @@ mod tests {
 (module
 	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
 	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
-	(import "__unstable__" "take_storage" (func $take_storage (param i32 i32 i32 i32) (result i32)))
+	(import "seal0" "take_storage" (func $take_storage (param i32 i32 i32 i32) (result i32)))
 	(import "env" "memory" (memory 1 1))
 
 	;; [0, 4) size of input buffer (160 bytes as we copy the key+len here)
@@ -2898,7 +2898,7 @@ mod tests {
 	fn reentrance_count_works() {
 		const CODE: &str = r#"
 (module
-	(import "__unstable__" "reentrance_count" (func $reentrance_count (result i32)))
+	(import "seal0" "reentrance_count" (func $reentrance_count (result i32)))
 	(import "env" "memory" (memory 1 1))
 	(func $assert (param i32)
 		(block $ok
@@ -2931,7 +2931,7 @@ mod tests {
 	fn account_reentrance_count_works() {
 		const CODE: &str = r#"
 (module
-	(import "__unstable__" "account_reentrance_count" (func $account_reentrance_count (param i32) (result i32)))
+	(import "seal0" "account_reentrance_count" (func $account_reentrance_count (param i32) (result i32)))
 	(import "env" "memory" (memory 1 1))
 	(func $assert (param i32)
 		(block $ok