contracts: switch from parity-wasm-based to wasmi-based module validation (paritytech#14449)

agryaznov · web-flow · commit 60aa1d747d7a · 2023-07-04T21:38:47.000Z
* upgrade to wasmi 0.29

* prepare cleanup

* sync ref_time w engine from the stack frame

* proc_macro: sync gas in host funcs

save: compiles, only gas pushing left to macro

WIP proc macro

proc macro: done

* clean benchmarks &amp; schedule: w_base = w_i64const

* scale gas values btw engine and gas meter

* (re)instrumentation &amp; code_cache removed

* remove gas() host fn, continue clean-up

save

* address review comments

* move from CodeStorage&amp;PrefabWasmModule to PristineCode&amp;WasmBlob

* refactor: no reftime_limit&amp;schedule passes, no CodeStorage

* bugs fixing

* fix tests: expected deposit amount

* fix prepare::tests

* update tests and fix bugs

tests::run_out_of_gas_engine, need 2 more

save: 2 bugs with gas syncs: 1 of 2 tests done

gas_syncs_no_overcharge bug fixed, test passes!

cleaned out debug prints

second bug is not a bug

disabled_chain_extension test fix (err msg)

tests run_out_of_fuel_host, chain_extension pass

all tests pass

* update docs

* bump wasmi 0.30.0

* benchmarks updated, tests pass

* refactoring

* s/OwnerInfo/CodeInfo/g;

* migration: draft, compiles

* migration: draft, runs

* migration: draft, runs (fixing)

* deposits repaid non pro rata

* deposits repaid pro rata

* better try-runtime output

* even better try-runtime output

* benchmark migration

* fix merge leftover

* add forgotten fixtures, fix docs

* address review comments

* ci fixes

* cleanup

* benchmarks::prepare to return DispatchError

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* store memory limits to CodeInfo

* ci: roll back weights

* ".git/.scripts/commands/bench-vm/bench-vm.sh" pallet dev pallet_contracts

* drive-by: update Readme and pallet rustdoc

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* use wasmi 0.29

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* use wasmi 0.30 again

* query memory limits from wasmi

* save: scan_exports ported, compiles

* save (wip, not compiles)

* query memory limits from wasmi

* better migration types

* ci: pull weights from master

* refactoring

* ".git/.scripts/commands/bench-vm/bench-vm.sh" pallet dev pallet_contracts

* scan_imports ported

* scan_export ported, other checks removed

* tests fixed

tests fixed

* drop wasmparser and parity-wasm dependencies

* typo fix

* addressing review comments

* refactor

* address review comments

* optimize migration

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* another review round comments addressed

* ci fix one

* clippy fix

* ci fix two

* allow stored modules to have no memory imports

* rollback: allow stored modules to have no memory imports

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* address review comments

---------

Co-authored-by: command-bot &lt;&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml
@@ -21,16 +21,15 @@ codec = { package = "parity-scale-codec", version = "3.6.1", default-features =
 ] }
 scale-info = { version = "2.5.0", default-features = false, features = ["derive"] }
 log = { version = "0.4", default-features = false }
-wasm-instrument = { version = "0.4", default-features = false }
 serde = { version = "1", optional = true, features = ["derive"] }
 smallvec = { version = "1", default-features = false, features = [
 	"const_generics",
 ] }
 wasmi = { version = "0.30", default-features = false }
-wasmparser = { package = "wasmparser-nostd", version = "0.100", default-features = false }
 impl-trait-for-tuples = "0.2"
 
-# Only used in benchmarking to generate random contract code
+# Only used in benchmarking to generate contract code
+wasm-instrument = { version = "0.4", optional = true, default-features = false }
 rand = { version = "0.8", optional = true, default-features = false }
 rand_pcg = { version = "0.3", optional = true }
 
@@ -81,12 +80,12 @@ std = [
 	"pallet-contracts-proc-macro/full",
 	"log/std",
 	"rand/std",
-	"wasmparser/std",
 	"environmental/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking/runtime-benchmarks",
 	"rand",
 	"rand_pcg",
+	"wasm-instrument",
 ]
 try-runtime = ["frame-support/try-runtime"]
diff --git a/frame/contracts/fixtures/dummy.wat b/frame/contracts/fixtures/dummy.wat
@@ -1,5 +1,6 @@
 ;; A valid contract which does nothing at all
 (module
+	(import "env" "memory" (memory 1 1))
 	(func (export "deploy"))
 	(func (export "call"))
 )
diff --git a/frame/contracts/fixtures/float_instruction.wat b/frame/contracts/fixtures/float_instruction.wat
@@ -1,5 +1,6 @@
 ;; Module that contains a float instruction which is illegal in deterministic mode
 (module
+	(import "env" "memory" (memory 1 1))
 	(func (export "call")
 		f32.const 1
 		drop
diff --git a/frame/contracts/fixtures/invalid_contract_no_call.wat b/frame/contracts/fixtures/invalid_contract_no_call.wat
@@ -1,4 +1,5 @@
 ;; Valid module but missing the call function
 (module
+	(import "env" "memory" (memory 1 1))
 	(func (export "deploy"))
 )
diff --git a/frame/contracts/fixtures/invalid_contract_no_memory.wat b/frame/contracts/fixtures/invalid_contract_no_memory.wat
@@ -0,0 +1,5 @@
+;; A valid contract which does nothing at all
+(module
+	(func (export "deploy"))
+	(func (export "call"))
+)
diff --git a/frame/contracts/fixtures/run_out_of_gas.wat b/frame/contracts/fixtures/run_out_of_gas.wat
@@ -1,4 +1,5 @@
 (module
+	(import "env" "memory" (memory 1 1))
 	(func (export "call")
 		(loop $inf (br $inf)) ;; just run out of gas
 		(unreachable)
diff --git a/frame/contracts/src/benchmarking/code.rs b/frame/contracts/src/benchmarking/code.rs
@@ -122,7 +122,7 @@ impl<T: Config> From<ModuleDefinition> for WasmModule<T> {
 		// internal functions start at that offset.
 		let func_offset = u32::try_from(def.imported_functions.len()).unwrap();
 
-		// Every contract must export "deploy" and "call" functions
+		// Every contract must export "deploy" and "call" functions.
 		let mut contract = builder::module()
 			// deploy function (first internal function)
 			.function()
@@ -163,15 +163,16 @@ impl<T: Config> From<ModuleDefinition> for WasmModule<T> {
 		}
 
 		// Grant access to linear memory.
-		if let Some(memory) = &def.memory {
-			contract = contract
-				.import()
-				.module("env")
-				.field("memory")
-				.external()
-				.memory(memory.min_pages, Some(memory.max_pages))
-				.build();
-		}
+		// Every contract module is required to have an imported memory.
+		// If no memory is specified in the passed ModuleDefenition, then
+		// default to (1, 1).
+		let (init, max) = if let Some(memory) = &def.memory {
+			(memory.min_pages, Some(memory.max_pages))
+		} else {
+			(1, Some(1))
+		};
+
+		contract = contract.import().path("env", "memory").external().memory(init, max).build();
 
 		// Import supervisor functions. They start with idx 0.
 		for func in def.imported_functions {
diff --git a/frame/contracts/src/benchmarking/sandbox.rs b/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::{AllowDeprecatedInterface, AllowUnstableInterface, Environment, WasmBlob};
+use crate::wasm::{
+	AllowDeprecatedInterface, AllowUnstableInterface, Determinism, Environment, WasmBlob,
+};
 use sp_core::Get;
 use wasmi::{errors::LinkerError, Func, Linker, StackLimits, Store};
 
@@ -44,6 +46,7 @@ impl<T: Config> From<&WasmModule<T>> for Sandbox {
 			&module.code,
 			(),
 			&<T>::Schedule::get(),
+			Determinism::Relaxed,
 			StackLimits::default(),
 			// We are testing with an empty environment anyways
 			AllowDeprecatedInterface::No,
diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs
@@ -885,6 +885,8 @@ pub mod pallet {
 		CodeTooLarge,
 		/// No code could be found at the supplied code hash.
 		CodeNotFound,
+		/// No code info could be found at the supplied code hash.
+		CodeInfoNotFound,
 		/// A buffer outside of sandbox memory was passed to a contract API function.
 		OutOfBounds,
 		/// Input passed to a contract API function failed to decode as expected type.
diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs
@@ -2055,7 +2055,7 @@ fn disabled_chain_extension_errors_on_call() {
 		TestExtension::disable();
 		assert_err_ignore_postinfo!(
 			Contracts::call(RuntimeOrigin::signed(ALICE), addr.clone(), 0, GAS_LIMIT, None, vec![],),
-			Error::<Test>::NoChainExtension,
+			Error::<Test>::CodeRejected,
 		);
 	});
 }
@@ -4419,10 +4419,10 @@ fn code_rejected_error_works() {
 		assert_err!(result.result, <Error<Test>>::CodeRejected);
 		assert_eq!(
 			std::str::from_utf8(&result.debug_message).unwrap(),
-			"Validation of new code failed!"
+			"Can't load the module into wasmi!"
 		);
 
-		let (wasm, _) = compile_module::<Test>("invalid_contract").unwrap();
+		let (wasm, _) = compile_module::<Test>("invalid_contract_no_call").unwrap();
 		assert_noop!(
 			Contracts::upload_code(
 				RuntimeOrigin::signed(ALICE),
@@ -4449,6 +4449,34 @@ fn code_rejected_error_works() {
 			std::str::from_utf8(&result.debug_message).unwrap(),
 			"call function isn't exported"
 		);
+
+		let (wasm, _) = compile_module::<Test>("invalid_contract_no_memory").unwrap();
+		assert_noop!(
+			Contracts::upload_code(
+				RuntimeOrigin::signed(ALICE),
+				wasm.clone(),
+				None,
+				Determinism::Enforced
+			),
+			<Error<Test>>::CodeRejected,
+		);
+
+		let result = Contracts::bare_instantiate(
+			ALICE,
+			0,
+			GAS_LIMIT,
+			None,
+			Code::Upload(wasm),
+			vec![],
+			vec![],
+			DebugInfo::UnsafeDebug,
+			CollectEvents::Skip,
+		);
+		assert_err!(result.result, <Error<Test>>::CodeRejected);
+		assert_eq!(
+			std::str::from_utf8(&result.debug_message).unwrap(),
+			"No memory import found in the module"
+		);
 	});
 }
 
@@ -5117,6 +5145,7 @@ fn cannot_instantiate_indeterministic_code() {
 			None,
 			Determinism::Relaxed,
 		));
+
 		assert_err_ignore_postinfo!(
 			Contracts::instantiate(
 				RuntimeOrigin::signed(ALICE),
diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs
@@ -38,7 +38,7 @@ pub use crate::wasm::{
 use crate::{
 	exec::{ExecResult, Executable, ExportedFunction, Ext},
 	gas::{GasMeter, Token},
-	wasm::prepare::IMPORT_MODULE_MEMORY,
+	wasm::prepare::LoadedModule,
 	weights::WeightInfo,
 	AccountIdOf, BadOrigin, BalanceOf, CodeHash, CodeInfoOf, CodeVec, Config, Error, Event, Pallet,
 	PristineCode, Schedule, Weight, LOG_TARGET,
@@ -52,10 +52,8 @@ use frame_support::{
 use sp_core::Get;
 use sp_runtime::RuntimeDebug;
 use sp_std::prelude::*;
-use wasmi::{
-	Config as WasmiConfig, Engine, ExternType, FuelConsumptionMode, Instance, Linker, Memory,
-	MemoryType, Module, StackLimits, Store,
-};
+use wasmi::{Instance, Linker, Memory, MemoryType, StackLimits, Store};
+
 const BYTES_PER_PAGE: usize = 64 * 1024;
 
 /// Validated Wasm module ready for execution.
@@ -204,26 +202,16 @@ impl<T: Config> WasmBlob<T> {
 		code: &[u8],
 		host_state: H,
 		schedule: &Schedule<T>,
+		determinism: Determinism,
 		stack_limits: StackLimits,
 		allow_deprecated: AllowDeprecatedInterface,
 	) -> Result<(Store<H>, Memory, Instance), &'static str>
 	where
 		E: Environment<H>,
 	{
-		let mut config = WasmiConfig::default();
-		config
-			.set_stack_limits(stack_limits)
-			.wasm_multi_value(false)
-			.wasm_mutable_global(false)
-			.wasm_sign_extension(false)
-			.wasm_saturating_float_to_int(false)
-			.consume_fuel(true)
-			.fuel_consumption_mode(FuelConsumptionMode::Eager);
-
-		let engine = Engine::new(&config);
-		let module = Module::new(&engine, code.clone()).map_err(|_| "can't decode Wasm module")?;
-		let mut store = Store::new(&engine, host_state);
-		let mut linker = Linker::new(&engine);
+		let contract = LoadedModule::new::<T>(&code, determinism, Some(stack_limits))?;
+		let mut store = Store::new(&contract.engine, host_state);
+		let mut linker = Linker::new(&contract.engine);
 		E::define(
 			&mut store,
 			&mut linker,
@@ -235,8 +223,9 @@ impl<T: Config> WasmBlob<T> {
 			allow_deprecated,
 		)
 		.map_err(|_| "can't define host functions to Linker")?;
+
 		// Query wasmi for memory limits specified in the module's import entry.
-		let memory_limits = Self::get_memory_limits(module.imports(), schedule)?;
+		let memory_limits = contract.scan_imports::<T>(schedule)?;
 		// Here we allocate this memory in the _store_. It allocates _inital_ value, but allows it
 		// to grow up to maximum number of memory pages, if neccesary.
 		let qed = "We checked the limits versus our Schedule,
@@ -247,63 +236,20 @@ impl<T: Config> WasmBlob<T> {
 			MemoryType::new(memory_limits.0, Some(memory_limits.1)).expect(qed),
 		)
 		.expect(qed);
+
 		linker
 			.define("env", "memory", memory)
 			.expect("We just created the Linker. It has no definitions with this name; qed");
 
 		let instance = linker
-			.instantiate(&mut store, &module)
+			.instantiate(&mut store, &contract.module)
 			.map_err(|_| "can't instantiate module with provided definitions")?
 			.ensure_no_start(&mut store)
 			.map_err(|_| "start function is forbidden but found in the module")?;
 
 		Ok((store, memory, instance))
 	}
 
-	/// Query wasmi for memory limits specified for the import in Wasm module.
-	fn get_memory_limits(
-		imports: wasmi::ModuleImportsIter,
-		schedule: &Schedule<T>,
-	) -> Result<(u32, u32), &'static str> {
-		let mut mem_type = None;
-		for import in imports {
-			match *import.ty() {
-				ExternType::Memory(mt) => {
-					if import.module() != IMPORT_MODULE_MEMORY {
-						return Err("Invalid module for imported memory")
-					}
-					if import.name() != "memory" {
-						return Err("Memory import must have the field name 'memory'")
-					}
-					mem_type = Some(mt);
-					break
-				},
-				_ => continue,
-			}
-		}
-		// We don't need to check here if module memory limits satisfy the schedule,
-		// as this was already done during the code uploading.
-		// If none memory imported then set its limits to (0,0).
-		// Any access to it will then lead to out of bounds trap.
-		let (initial, maximum) = mem_type.map_or(Default::default(), |mt| {
-			(
-				mt.initial_pages().to_bytes().unwrap_or(0).saturating_div(BYTES_PER_PAGE) as u32,
-				mt.maximum_pages().map_or(schedule.limits.memory_pages, |p| {
-					p.to_bytes().unwrap_or(0).saturating_div(BYTES_PER_PAGE) as u32
-				}),
-			)
-		});
-		if initial > maximum {
-			return Err(
-				"Requested initial number of memory pages should not exceed the requested maximum",
-			)
-		}
-		if maximum > schedule.limits.memory_pages {
-			return Err("Maximum number of memory pages should not exceed the maximum configured in the Schedule.")
-		}
-		Ok((initial, maximum))
-	}
-
 	/// Getter method for the code_info.
 	pub fn code_info(&self) -> &CodeInfo<T> {
 		&self.code_info
@@ -469,6 +415,7 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 			code,
 			runtime,
 			&schedule,
+			self.code_info.determinism,
 			StackLimits::default(),
 			match function {
 				ExportedFunction::Call => AllowDeprecatedInterface::Yes,
@@ -3314,6 +3261,8 @@ mod tests {
 		const CODE: &str = r#"
 (module
 	(import "seal0" "instantiation_nonce" (func $nonce (result i64)))
+	(import "env" "memory" (memory 1 1))
+
 	(func $assert (param i32)
 		(block $ok
 			(br_if $ok
@@ -3344,6 +3293,8 @@ mod tests {
 		const CANNOT_DEPLOY_UNSTABLE: &str = r#"
 (module
 	(import "seal0" "reentrance_count" (func $reentrance_count (result i32)))
+	(import "env" "memory" (memory 1 1))
+
 	(func (export "call"))
 	(func (export "deploy"))
 )
@@ -3364,27 +3315,35 @@ mod tests {
 		const CODE_RANDOM_0: &str = r#"
 (module
 	(import "seal0" "seal_random" (func $seal_random (param i32 i32 i32 i32)))
+	(import "env" "memory" (memory 1 1))
+
 	(func (export "call"))
 	(func (export "deploy"))
 )
 	"#;
 		const CODE_RANDOM_1: &str = r#"
 (module
 	(import "seal1" "seal_random" (func $seal_random (param i32 i32 i32 i32)))
+	(import "env" "memory" (memory 1 1))
+
 	(func (export "call"))
 	(func (export "deploy"))
 )
 	"#;
 		const CODE_RANDOM_2: &str = r#"
 (module
 	(import "seal0" "random" (func $seal_random (param i32 i32 i32 i32)))
+	(import "env" "memory" (memory 1 1))
+
 	(func (export "call"))
 	(func (export "deploy"))
 )
 	"#;
 		const CODE_RANDOM_3: &str = r#"
 (module
 	(import "seal1" "random" (func $seal_random (param i32 i32 i32 i32)))
+	(import "env" "memory" (memory 1 1))
+
 	(func (export "call"))
 	(func (export "deploy"))
 )
diff --git a/frame/contracts/src/wasm/prepare.rs b/frame/contracts/src/wasm/prepare.rs
diff --git a/frame/contracts/src/weights.rs b/frame/contracts/src/weights.rs

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`;; A valid contract which does nothing at all`
`2`	`2`	`(module`
	`3`	`+ (import "env" "memory" (memory 1 1))`
`3`	`4`	`(func (export "deploy"))`
`4`	`5`	`(func (export "call"))`
`5`	`6`	`)`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`;; Valid module but missing the call function`
`2`	`2`	`(module`
	`3`	`+ (import "env" "memory" (memory 1 1))`
`3`	`4`	`(func (export "deploy"))`
`4`	`5`	`)`