contracts: Deprecate random interface (paritytech#13204)

* Deprecate random interface

* Revert change to runtime file

* Fix docs

* Fix tests

* Rename to not_deprecated

* Apply suggestions from code review

Co-authored-by: Sasha Gryaznov <[email protected]>

* Deprecate `set_rent_allowance`

Co-authored-by: Sasha Gryaznov <[email protected]>

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

@@ -162,6 +162,8 @@ struct HostFn {
 	returns: HostFnReturn,
 	is_stable: bool,
 	alias_to: Option<String>,
+	/// Formulating the predicate inverted makes the expression using it simpler.
+	not_deprecated: bool,
 }
 
 enum HostFnReturn {
@@ -199,13 +201,14 @@ impl HostFn {
 
 		// process attributes
 		let msg =
-			"only #[version(<u8>)], #[unstable] and #[prefixed_alias] attributes are allowed.";
+			"only #[version(<u8>)], #[unstable], #[prefixed_alias] and #[deprecated] attributes are allowed.";
 		let span = item.span();
 		let mut attrs = item.attrs.clone();
 		attrs.retain(|a| !a.path.is_ident("doc"));
 		let mut maybe_module = None;
 		let mut is_stable = true;
 		let mut alias_to = None;
+		let mut not_deprecated = true;
 		while let Some(attr) = attrs.pop() {
 			let ident = attr.path.get_ident().ok_or(err(span, msg))?.to_string();
 			match ident.as_str() {
@@ -230,12 +233,22 @@ impl HostFn {
 						item.sig.ident.span(),
 					);
 				},
+				"deprecated" => {
+					if !not_deprecated {
+						return Err(err(span, "#[deprecated] can only be specified once"))
+					}
+					not_deprecated = false;
+				},
 				_ => return Err(err(span, msg)),
 			}
 		}
 		let name = item.sig.ident.to_string();
 
-		// process arguments: The first and second arg are treated differently (ctx, memory)
+		if !(is_stable || not_deprecated) {
+			return Err(err(span, "#[deprecated] is mutually exclusive with #[unstable]"))
+		}
+
+		// process arguments: The first and second args are treated differently (ctx, memory)
 		// they must exist and be `ctx: _` and `memory: _`.
 		let msg = "Every function must start with two inferred parameters: ctx: _ and memory: _";
 		let special_args = item
@@ -330,6 +343,7 @@ impl HostFn {
 							returns,
 							is_stable,
 							alias_to,
+							not_deprecated,
 						})
 					},
 					_ => Err(err(span, &msg)),
@@ -510,15 +524,25 @@ fn expand_impls(def: &mut EnvDef) -> TokenStream2 {
 	quote! {
 		impl<'a, E: Ext> crate::wasm::Environment<crate::wasm::runtime::Runtime<'a, E>> for Env
 		{
-			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> {
+			fn define(
+				store: &mut ::wasmi::Store<crate::wasm::Runtime<E>>,
+				linker: &mut ::wasmi::Linker<crate::wasm::Runtime<E>>,
+				allow_unstable: AllowUnstableInterface,
+				allow_deprecated: AllowDeprecatedInterface,
+			) -> Result<(),::wasmi::errors::LinkerError> {
 				#impls
 				Ok(())
 			}
 		}
 
 		impl crate::wasm::Environment<()> for Env
 		{
-			fn define(store: &mut ::wasmi::Store<()>, linker: &mut ::wasmi::Linker<()>, allow_unstable: bool) -> Result<(), ::wasmi::errors::LinkerError> {
+			fn define(
+				store: &mut ::wasmi::Store<()>,
+				linker: &mut ::wasmi::Linker<()>,
+				allow_unstable: AllowUnstableInterface,
+				allow_deprecated: AllowDeprecatedInterface,
+			) -> Result<(), ::wasmi::errors::LinkerError> {
 				#dummy_impls
 				Ok(())
 			}
@@ -542,6 +566,7 @@ fn expand_functions(
 			&f.item.sig.output
 		);
 		let is_stable = f.is_stable;
+		let not_deprecated = f.not_deprecated;
 
 		// If we don't expand blocks (implementing for `()`) we change a few things:
 		// - We replace any code by unreachable!
@@ -582,9 +607,13 @@ fn expand_functions(
 		};
 
 		quote! {
-			// 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 {
+			// We need to allow all interfaces when runtime benchmarks are performed because
+			// we generate the weights even when those interfaces are not enabled. This
+			// is necessary as the decision whether we allow unstable or deprecated functions
+			// is a decision made at runtime. Generation of the weights happens statically.
+			if ::core::cfg!(feature = "runtime-benchmarks") ||
+				((#is_stable || __allow_unstable__) && (#not_deprecated || __allow_deprecated__))
+			{
 				#allow_unused
 				linker.define(#module, #name, ::wasmi::Func::wrap(&mut*store, |mut __caller__: ::wasmi::Caller<#host_state>, #( #params, )*| -> #wasm_output {
 					let mut func = #inner;
@@ -596,6 +625,8 @@ fn expand_functions(
 		}
 	});
 	quote! {
+		let __allow_unstable__ = matches!(allow_unstable, AllowUnstableInterface::Yes);
+		let __allow_deprecated__ = matches!(allow_deprecated, AllowDeprecatedInterface::Yes);
 		#( #impls )*
 	}
 }
@@ -691,6 +722,16 @@ fn expand_functions(
 /// `...` modules each having its `Api` trait containing functions holding documentation for every
 /// host function defined by the macro.
 ///
+/// # Deprecated Interfaces
+///
+/// An interface can be annotated with `#[deprecated]`. It is mutually exclusive with `#[unstable]`.
+/// Deprecated interfaces have the following properties:
+/// 	- New contract codes utilizing those interfaces cannot be uploaded.
+/// 	- New contracts from existing codes utilizing those interfaces cannot be instantiated.
+/// - Existing contracts containing those interfaces still work.
+///
+/// Those interfaces will eventually be removed.
+///
 /// To build up these docs, run:
 ///
 /// ```nocompile

frame/contracts/src/benchmarking/sandbox.rs

@@ -19,7 +19,9 @@
 /// ! sandbox to execute the wasm code. This is because we do not need the full
 /// ! environment that provides the seal interface as imported functions.
 use super::{code::WasmModule, Config};
-use crate::wasm::{Environment, PrefabWasmModule};
+use crate::wasm::{
+	AllowDeprecatedInterface, AllowUnstableInterface, Environment, PrefabWasmModule,
+};
 use wasmi::{errors::LinkerError, Func, Linker, StackLimits, Store};
 
 /// Minimal execution environment without any imported functions.
@@ -49,6 +51,8 @@ impl<T: Config> From<&WasmModule<T>> for Sandbox {
 			(),
 			memory,
 			StackLimits::default(),
+			// We are testing with an empty environment anyways
+			AllowDeprecatedInterface::No,
 		)
 		.expect("Failed to create benchmarking Sandbox instance");
 		let entry_point = instance.get_export(&store, "call").unwrap().into_func().unwrap();
@@ -59,7 +63,12 @@ impl<T: Config> From<&WasmModule<T>> for Sandbox {
 struct EmptyEnv;
 
 impl Environment<()> for EmptyEnv {
-	fn define(_: &mut Store<()>, _: &mut Linker<()>, _: bool) -> Result<(), LinkerError> {
+	fn define(
+		_: &mut Store<()>,
+		_: &mut Linker<()>,
+		_: AllowUnstableInterface,
+		_: AllowDeprecatedInterface,
+	) -> Result<(), LinkerError> {
 		Ok(())
 	}
 }

frame/contracts/src/exec.rs

@@ -3440,4 +3440,35 @@ mod tests {
 			));
 		});
 	}
+
+	/// This works even though random interface is deprecated, as the check to ban deprecated
+	/// functions happens in the wasm stack which is mocked for exec tests.
+	#[test]
+	fn randomness_works() {
+		let subject = b"nice subject".as_ref();
+		let code_hash = MockLoader::insert(Call, move |ctx, _| {
+			let rand = <Test as Config>::Randomness::random(subject);
+			assert_eq!(rand, ctx.ext.random(subject));
+			exec_success()
+		});
+
+		ExtBuilder::default().build().execute_with(|| {
+			let schedule = <Test as Config>::Schedule::get();
+			place_contract(&BOB, code_hash);
+
+			let mut storage_meter = storage::meter::Meter::new(&ALICE, Some(0), 0).unwrap();
+			let result = MockStack::run_call(
+				ALICE,
+				BOB,
+				&mut GasMeter::<Test>::new(GAS_LIMIT),
+				&mut storage_meter,
+				&schedule,
+				0,
+				vec![],
+				None,
+				Determinism::Deterministic,
+			);
+			assert_matches!(result, Ok(_));
+		});
+	}
 }

frame/contracts/src/lib.rs

@@ -216,7 +216,14 @@ pub mod pallet {
 		/// The time implementation used to supply timestamps to contracts through `seal_now`.
 		type Time: Time;
 
-		/// The generator used to supply randomness to contracts through `seal_random`
+		/// The generator used to supply randomness to contracts through `seal_random`.
+		///
+		/// # Deprecated
+		///
+		/// Codes using the randomness functionality cannot be uploaded. Neither can contracts
+		/// be instantiated from existing codes that use this deprecated functionality. It will
+		/// be removed eventually. Hence for new `pallet-contracts` deployments it is okay
+		/// to supply a dummy implementation for this type (because it is never used).
 		type Randomness: Randomness<Self::Hash, Self::BlockNumber>;
 
 		/// The currency in which fees are paid and contract balances are held.