contracts: Refactor instantiate with code (#14503)

* wip

* fixes

* rm comment

* join fns

* clippy

* Fix limits

* reduce diff

* fix

* fix

* fix typo

* refactor store to  use self

* refactor run to take self by value

* pass tests

* rm comment

* fixes

* fix typo

* rm

* fix fmt

* clippy

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

* Update frame/contracts/src/lib.rs

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

* Update frame/contracts/src/wasm/mod.rs

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

* Update frame/contracts/src/wasm/mod.rs

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

* PR review, rm duplicate increment_refcount

* PR review

* Update frame/contracts/src/wasm/prepare.rs

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

* Add test for failing storage_deposit

* fix lint

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

---------

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

substrate/frame/contracts/primitives/src/lib.rs

@@ -161,12 +161,6 @@ pub enum Code<Hash> {
 	Existing(Hash),
 }
 
-impl<T: Into<Vec<u8>>, Hash> From<T> for Code<Hash> {
-	fn from(from: T) -> Self {
-		Code::Upload(from.into())
-	}
-}
-
 /// The amount of balance that was either charged or refunded in order to pay for storage.
 #[derive(Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug, Clone, TypeInfo)]
 pub enum StorageDeposit<Balance> {

substrate/frame/contracts/src/lib.rs

@@ -75,8 +75,8 @@
 //!   allowed to code owner.
 //! * [`Pallet::set_code`] - Changes the code of an existing contract. Only allowed to `Root`
 //!   origin.
-//! * [`Pallet::migrate`] - Runs migration steps of curent multi-block migration in priority, before
+//! * [`Pallet::migrate`] - Runs migration steps of current multi-block migration in priority,
-//!   [`Hooks::on_idle`][frame_support::traits::Hooks::on_idle] activates.
+//!   before [`Hooks::on_idle`][frame_support::traits::Hooks::on_idle] activates.
 //!
 //! ## Usage
 //!
@@ -105,7 +105,7 @@ use crate::{
 	exec::{AccountIdOf, ErrorOrigin, ExecError, Executable, Key, Stack as ExecStack},
 	gas::GasMeter,
 	storage::{meter::Meter as StorageMeter, ContractInfo, DeletionQueueManager},
-	wasm::{CodeInfo, TryInstantiate, WasmBlob},
+	wasm::{CodeInfo, WasmBlob},
 };
 use codec::{Codec, Decode, Encode, HasCompact};
 use environmental::*;
@@ -695,24 +695,40 @@ pub mod pallet {
 			salt: Vec<u8>,
 		) -> DispatchResultWithPostInfo {
 			Migration::<T>::ensure_migrated()?;
+			let origin = ensure_signed(origin)?;
 			let code_len = code.len() as u32;
+
+			let (module, upload_deposit) = Self::try_upload_code(
+				origin.clone(),
+				code,
+				storage_deposit_limit.clone().map(Into::into),
+				Determinism::Enforced,
+				None,
+			)?;
+
+			// Reduces the storage deposit limit by the amount that was reserved for the upload.
+			let storage_deposit_limit =
+				storage_deposit_limit.map(|limit| limit.into().saturating_sub(upload_deposit));
+
 			let data_len = data.len() as u32;
 			let salt_len = salt.len() as u32;
 			let common = CommonInput {
-				origin: Origin::from_runtime_origin(origin)?,
+				origin: Origin::from_account_id(origin),
 				value,
 				data,
 				gas_limit,
-				storage_deposit_limit: storage_deposit_limit.map(Into::into),
+				storage_deposit_limit,
 				debug_message: None,
 			};
+
 			let mut output =
-				InstantiateInput::<T> { code: Code::Upload(code), salt }.run_guarded(common);
+				InstantiateInput::<T> { code: WasmCode::Wasm(module), salt }.run_guarded(common);
 			if let Ok(retval) = &output.result {
 				if retval.1.did_revert() {
 					output.result = Err(<Error<T>>::ContractReverted.into());
 				}
 			}
+
 			output.gas_meter.into_dispatch_result(
 				output.result.map(|(_address, result)| result),
 				T::WeightInfo::instantiate_with_code(code_len, data_len, salt_len),
@@ -748,8 +764,8 @@ pub mod pallet {
 				storage_deposit_limit: storage_deposit_limit.map(Into::into),
 				debug_message: None,
 			};
-			let mut output =
+			let mut output = InstantiateInput::<T> { code: WasmCode::CodeHash(code_hash), salt }
-				InstantiateInput::<T> { code: Code::Existing(code_hash), salt }.run_guarded(common);
+				.run_guarded(common);
 			if let Ok(retval) = &output.result {
 				if retval.1.did_revert() {
 					output.result = Err(<Error<T>>::ContractReverted.into());
@@ -1052,9 +1068,15 @@ struct CallInput<T: Config> {
 	determinism: Determinism,
 }
 
+/// Reference to an existing code hash or a new wasm module.
+enum WasmCode<T: Config> {
+	Wasm(WasmBlob<T>),
+	CodeHash(CodeHash<T>),
+}
+
 /// Input specific to a contract instantiation invocation.
 struct InstantiateInput<T: Config> {
-	code: Code<CodeHash<T>>,
+	code: WasmCode<T>,
 	salt: Vec<u8>,
 }
 
@@ -1099,7 +1121,7 @@ struct InternalOutput<T: Config, O> {
 
 /// Helper trait to wrap contract execution entry points into a single function
 /// [`Invokable::run_guarded`].
-trait Invokable<T: Config> {
+trait Invokable<T: Config>: Sized {
 	/// What is returned as a result of a successful invocation.
 	type Output;
 
@@ -1111,7 +1133,7 @@ trait Invokable<T: Config> {
 	///
 	/// We enforce a re-entrancy guard here by initializing and checking a boolean flag through a
 	/// global reference.
-	fn run_guarded(&self, common: CommonInput<T>) -> InternalOutput<T, Self::Output> {
+	fn run_guarded(self, common: CommonInput<T>) -> InternalOutput<T, Self::Output> {
 		// Set up a global reference to the boolean flag used for the re-entrancy guard.
 		environmental!(executing_contract: bool);
 
@@ -1159,11 +1181,8 @@ trait Invokable<T: Config> {
 	/// contract or a instantiation of a new one.
 	///
 	/// Called by dispatchables and public functions through the [`Invokable::run_guarded`].
-	fn run(
+	fn run(self, common: CommonInput<T>, gas_meter: GasMeter<T>)
-		&self,
+		-> InternalOutput<T, Self::Output>;
-		common: CommonInput<T>,
-		gas_meter: GasMeter<T>,
-	) -> InternalOutput<T, Self::Output>;
 
 	/// This method ensures that the given `origin` is allowed to invoke the current `Invokable`.
 	///
@@ -1175,7 +1194,7 @@ impl<T: Config> Invokable<T> for CallInput<T> {
 	type Output = ExecReturnValue;
 
 	fn run(
-		&self,
+		self,
 		common: CommonInput<T>,
 		mut gas_meter: GasMeter<T>,
 	) -> InternalOutput<T, Self::Output> {
@@ -1201,7 +1220,7 @@ impl<T: Config> Invokable<T> for CallInput<T> {
 			value,
 			data.clone(),
 			debug_message,
-			*determinism,
+			determinism,
 		);
 
 		match storage_meter.try_into_deposit(&origin) {
@@ -1223,8 +1242,8 @@ impl<T: Config> Invokable<T> for InstantiateInput<T> {
 	type Output = (AccountIdOf<T>, ExecReturnValue);
 
 	fn run(
-		&self,
+		self,
-		mut common: CommonInput<T>,
+		common: CommonInput<T>,
 		mut gas_meter: GasMeter<T>,
 	) -> InternalOutput<T, Self::Output> {
 		let mut storage_deposit = Default::default();
@@ -1233,37 +1252,15 @@ impl<T: Config> Invokable<T> for InstantiateInput<T> {
 			let InstantiateInput { salt, .. } = self;
 			let CommonInput { origin: contract_origin, .. } = common;
 			let origin = contract_origin.account_id()?;
-			let (extra_deposit, executable) = match &self.code {
+
-				Code::Upload(binary) => {
+			let executable = match self.code {
-					let executable = WasmBlob::from_code(
+				WasmCode::Wasm(module) => module,
-						binary.clone(),
+				WasmCode::CodeHash(code_hash) => WasmBlob::from_storage(code_hash, &mut gas_meter)?,
-						&schedule,
-						origin.clone(),
-						Determinism::Enforced,
-						TryInstantiate::Skip,
-					)
-					.map_err(|(err, msg)| {
-						common
-							.debug_message
-							.as_mut()
-							.map(|buffer| buffer.try_extend(&mut msg.bytes()));
-						err
-					})?;
-					// The open deposit will be charged during execution when the
-					// uploaded module does not already exist. This deposit is not part of the
-					// storage meter because it is not transferred to the contract but
-					// reserved on the uploading account.
-					(executable.open_deposit(&executable.code_info()), executable)
-				},
-				Code::Existing(hash) =>
-					(Default::default(), WasmBlob::from_storage(*hash, &mut gas_meter)?),
 			};
+
 			let contract_origin = Origin::from_account_id(origin.clone());
-			let mut storage_meter = StorageMeter::new(
+			let mut storage_meter =
-				&contract_origin,
+				StorageMeter::new(&contract_origin, common.storage_deposit_limit, common.value)?;
-				common.storage_deposit_limit,
-				common.value.saturating_add(extra_deposit),
-			)?;
 			let CommonInput { value, data, debug_message, .. } = common;
 			let result = ExecStack::<T, WasmBlob<T>>::run_instantiate(
 				origin.clone(),
@@ -1277,9 +1274,7 @@ impl<T: Config> Invokable<T> for InstantiateInput<T> {
 				debug_message,
 			);
 
-			storage_deposit = storage_meter
+			storage_deposit = storage_meter.try_into_deposit(&contract_origin)?;
-				.try_into_deposit(&contract_origin)?
-				.saturating_add(&StorageDeposit::Charge(extra_deposit));
 			result
 		};
 		InternalOutput { result: try_exec(), gas_meter, storage_deposit }
@@ -1383,7 +1378,7 @@ impl<T: Config> Pallet<T> {
 		origin: T::AccountId,
 		value: BalanceOf<T>,
 		gas_limit: Weight,
-		storage_deposit_limit: Option<BalanceOf<T>>,
+		mut storage_deposit_limit: Option<BalanceOf<T>>,
 		code: Code<CodeHash<T>>,
 		data: Vec<u8>,
 		salt: Vec<u8>,
@@ -1397,6 +1392,45 @@ impl<T: Config> Pallet<T> {
 		} else {
 			None
 		};
+		// collect events if CollectEvents is UnsafeCollect
+		let events = || {
+			if collect_events == CollectEvents::UnsafeCollect {
+				Some(System::<T>::read_events_no_consensus().map(|e| *e).collect())
+			} else {
+				None
+			}
+		};
+
+		let (code, upload_deposit): (WasmCode<T>, BalanceOf<T>) = match code {
+			Code::Upload(code) => {
+				let result = Self::try_upload_code(
+					origin.clone(),
+					code,
+					storage_deposit_limit.map(Into::into),
+					Determinism::Enforced,
+					debug_message.as_mut(),
+				);
+
+				let (module, deposit) = match result {
+					Ok(result) => result,
+					Err(error) =>
+						return ContractResult {
+							gas_consumed: Zero::zero(),
+							gas_required: Zero::zero(),
+							storage_deposit: Default::default(),
+							debug_message: debug_message.unwrap_or(Default::default()).into(),
+							result: Err(error),
+							events: events(),
+						},
+				};
+
+				storage_deposit_limit =
+					storage_deposit_limit.map(|l| l.saturating_sub(deposit.into()));
+				(WasmCode::Wasm(module), deposit)
+			},
+			Code::Existing(hash) => (WasmCode::CodeHash(hash), Default::default()),
+		};
+
 		let common = CommonInput {
 			origin: Origin::from_account_id(origin),
 			value,
@@ -1405,13 +1439,8 @@ impl<T: Config> Pallet<T> {
 			storage_deposit_limit,
 			debug_message: debug_message.as_mut(),
 		};
+
 		let output = InstantiateInput::<T> { code, salt }.run_guarded(common);
-		// collect events if CollectEvents is UnsafeCollect
-		let events = if collect_events == CollectEvents::UnsafeCollect {
-			Some(System::<T>::read_events_no_consensus().map(|e| *e).collect())
-		} else {
-			None
-		};
 		ContractInstantiateResult {
 			result: output
 				.result
@@ -1419,9 +1448,11 @@ impl<T: Config> Pallet<T> {
 				.map_err(|e| e.error),
 			gas_consumed: output.gas_meter.gas_consumed(),
 			gas_required: output.gas_meter.gas_required(),
-			storage_deposit: output.storage_deposit,
+			storage_deposit: output
+				.storage_deposit
+				.saturating_add(&StorageDeposit::Charge(upload_deposit)),
 			debug_message: debug_message.unwrap_or_default().to_vec(),
-			events,
+			events: events(),
 		}
 	}
 
@@ -1436,17 +1467,31 @@ impl<T: Config> Pallet<T> {
 		determinism: Determinism,
 	) -> CodeUploadResult<CodeHash<T>, BalanceOf<T>> {
 		Migration::<T>::ensure_migrated()?;
+		let (module, deposit) =
+			Self::try_upload_code(origin, code, storage_deposit_limit, determinism, None)?;
+		Ok(CodeUploadReturnValue { code_hash: *module.code_hash(), deposit })
+	}
+
+	/// Uploads new code and returns the Wasm blob and deposit amount collected.
+	fn try_upload_code(
+		origin: T::AccountId,
+		code: Vec<u8>,
+		storage_deposit_limit: Option<BalanceOf<T>>,
+		determinism: Determinism,
+		mut debug_message: Option<&mut DebugBufferVec<T>>,
+	) -> Result<(WasmBlob<T>, BalanceOf<T>), DispatchError> {
 		let schedule = T::Schedule::get();
-		let module =
+		let mut module =
-			WasmBlob::from_code(code, &schedule, origin, determinism, TryInstantiate::Instantiate)
+			WasmBlob::from_code(code, &schedule, origin, determinism).map_err(|(err, msg)| {
-				.map_err(|(err, _)| err)?;
+				debug_message.as_mut().map(|d| d.try_extend(msg.bytes()));
-		let deposit = module.open_deposit(&module.code_info());
+				err
+			})?;
+		let deposit = module.store_code()?;
 		if let Some(storage_deposit_limit) = storage_deposit_limit {
 			ensure!(storage_deposit_limit >= deposit, <Error<T>>::StorageDepositLimitExhausted);
 		}
-		let result = CodeUploadReturnValue { code_hash: *module.code_hash(), deposit };
+
-		module.store()?;
+		Ok((module, deposit))
-		Ok(result)
 	}
 
 	/// Query storage of a specified contract under a specified key.
@@ -1490,7 +1535,7 @@ impl<T: Config> Pallet<T> {
 		owner: T::AccountId,
 	) -> frame_support::dispatch::DispatchResult {
 		let schedule = T::Schedule::get();
-		WasmBlob::store_code_unchecked(code, &schedule, owner)?;
+		WasmBlob::<T>::from_code_unchecked(code, &schedule, owner)?.store_code()?;
 		Ok(())
 	}
 

substrate/frame/contracts/src/migration/v11.rs

@@ -80,7 +80,9 @@ impl<T: Config> MigrationStep for Migration<T> {
 	}
 
 	fn step(&mut self) -> (IsFinished, Weight) {
-		let Some(old_queue) = old::DeletionQueue::<T>::take() else { return (IsFinished::Yes, Weight::zero()) };
+		let Some(old_queue) = old::DeletionQueue::<T>::take() else {
+			return (IsFinished::Yes, Weight::zero())
+		};
 		let len = old_queue.len();
 
 		log::debug!(

substrate/frame/contracts/src/tests.rs

@@ -3753,6 +3753,19 @@ fn instantiate_with_zero_balance_works() {
 		assert_eq!(
 			System::events(),
 			vec![
+				EventRecord {
+					phase: Phase::Initialization,
+					event: RuntimeEvent::Balances(pallet_balances::Event::Reserved {
+						who: ALICE,
+						amount: deposit_expected,
+					}),
+					topics: vec![],
+				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: RuntimeEvent::Contracts(crate::Event::CodeStored { code_hash }),
+					topics: vec![code_hash],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: RuntimeEvent::System(frame_system::Event::NewAccount {
@@ -3801,19 +3814,6 @@ fn instantiate_with_zero_balance_works() {
 					}),
 					topics: vec![],
 				},
-				EventRecord {
-					phase: Phase::Initialization,
-					event: RuntimeEvent::Balances(pallet_balances::Event::Reserved {
-						who: ALICE,
-						amount: deposit_expected,
-					}),
-					topics: vec![],
-				},
-				EventRecord {
-					phase: Phase::Initialization,
-					event: RuntimeEvent::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![code_hash],
-				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: RuntimeEvent::Contracts(crate::Event::Instantiated {
@@ -3865,6 +3865,19 @@ fn instantiate_with_below_existential_deposit_works() {
 		assert_eq!(
 			System::events(),
 			vec![
+				EventRecord {
+					phase: Phase::Initialization,
+					event: RuntimeEvent::Balances(pallet_balances::Event::Reserved {
+						who: ALICE,
+						amount: deposit_expected,
+					}),
+					topics: vec![],
+				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: RuntimeEvent::Contracts(crate::Event::CodeStored { code_hash }),
+					topics: vec![code_hash],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: RuntimeEvent::System(frame_system::Event::NewAccount {
@@ -3922,19 +3935,6 @@ fn instantiate_with_below_existential_deposit_works() {
 					}),
 					topics: vec![],
 				},
-				EventRecord {
-					phase: Phase::Initialization,
-					event: RuntimeEvent::Balances(pallet_balances::Event::Reserved {
-						who: ALICE,
-						amount: deposit_expected,
-					}),
-					topics: vec![],
-				},
-				EventRecord {
-					phase: Phase::Initialization,
-					event: RuntimeEvent::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![code_hash],
-				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: RuntimeEvent::Contracts(crate::Event::Instantiated {
@@ -4587,11 +4587,29 @@ fn set_code_hash() {
 
 #[test]
 fn storage_deposit_limit_is_enforced() {
+	let ed = 200;
 	let (wasm, _code_hash) = compile_module::<Test>("store_call").unwrap();
-	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+	ExtBuilder::default().existential_deposit(ed).build().execute_with(|| {
 		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
 		let min_balance = <Test as Config>::Currency::minimum_balance();
 
+		// Setting insufficient storage_deposit should fail.
+		assert_err!(
+			Contracts::bare_instantiate(
+				ALICE,
+				0,
+				GAS_LIMIT,
+				Some((2 * ed + 3 - 1).into()), // expected deposit is 2 * ed + 3 for the call
+				Code::Upload(wasm.clone()),
+				vec![],
+				vec![],
+				DebugInfo::Skip,
+				CollectEvents::Skip,
+			)
+			.result,
+			<Error<Test>>::StorageDepositLimitExhausted,
+		);
+
 		// Instantiate the BOB contract.
 		let addr = Contracts::bare_instantiate(
 			ALICE,
@@ -5591,7 +5609,7 @@ fn root_cannot_instantiate_with_code() {
 				vec![],
 				vec![],
 			),
-			DispatchError::RootNotAllowed,
+			DispatchError::BadOrigin
 		);
 	});
 }

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

@@ -27,12 +27,9 @@ pub use crate::wasm::runtime::api_doc;
 #[cfg(test)]
 pub use tests::MockExt;
 
-pub use crate::wasm::{
+pub use crate::wasm::runtime::{
-	prepare::TryInstantiate,
+	AllowDeprecatedInterface, AllowUnstableInterface, CallFlags, Environment, ReturnCode, Runtime,
-	runtime::{
+	RuntimeCosts,
-		AllowDeprecatedInterface, AllowUnstableInterface, CallFlags, Environment, ReturnCode,
-		Runtime, RuntimeCosts,
-	},
 };
 
 use crate::{
@@ -147,33 +144,20 @@ impl<T: Config> Token<T> for CodeLoadToken {
 
 impl<T: Config> WasmBlob<T> {
 	/// Create the module by checking the `code`.
-	///
-	/// This does **not** store the module. For this one need to either call [`Self::store`]
-	/// or [`<Self as Executable>::execute`][`Executable::execute`].
 	pub fn from_code(
 		code: Vec<u8>,
 		schedule: &Schedule<T>,
 		owner: AccountIdOf<T>,
 		determinism: Determinism,
-		try_instantiate: TryInstantiate,
 	) -> Result<Self, (DispatchError, &'static str)> {
 		prepare::prepare::<runtime::Env, T>(
 			code.try_into().map_err(|_| (<Error<T>>::CodeTooLarge.into(), ""))?,
 			schedule,
 			owner,
 			determinism,
-			try_instantiate,
 		)
 	}
 
-	/// Store the code without instantiating it.
-	///
-	/// Otherwise the code is stored when [`<Self as Executable>::execute`][`Executable::execute`]
-	/// is called.
-	pub fn store(self) -> DispatchResult {
-		Self::store_code(self, false)
-	}
-
 	/// Remove the code from storage and refund the deposit to its owner.
 	///
 	/// Applies all necessary checks before removing the code.
@@ -181,18 +165,6 @@ impl<T: Config> WasmBlob<T> {
 		Self::try_remove_code(origin, code_hash)
 	}
 
-	/// Returns whether there is a deposit to be paid for this module.
-	///
-	/// Returns `0` if the module is already in storage and hence no deposit will
-	/// be charged for storing it.
-	pub fn open_deposit(&self, code_info: &CodeInfo<T>) -> BalanceOf<T> {
-		if <CodeInfoOf<T>>::contains_key(self.code_hash()) {
-			0u32.into()
-		} else {
-			code_info.deposit
-		}
-	}
-
 	/// Creates and returns an instance of the supplied code.
 	///
 	/// This is either used for later executing a contract or for validation of a contract.
@@ -227,7 +199,7 @@ impl<T: Config> WasmBlob<T> {
 		// Query wasmi for memory limits specified in the module's import entry.
 		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.
+		// to grow up to maximum number of memory pages, if necessary.
 		let qed = "We checked the limits versus our Schedule,
 					 which specifies the max amount of memory pages
 					 well below u16::MAX; qed";
@@ -250,50 +222,26 @@ impl<T: Config> WasmBlob<T> {
 		Ok((store, memory, instance))
 	}
 
-	/// Getter method for the code_info.
+	/// Puts the module blob into storage, and returns the deposit collected for the storage.
-	pub fn code_info(&self) -> &CodeInfo<T> {
+	pub fn store_code(&mut self) -> Result<BalanceOf<T>, Error<T>> {
-		&self.code_info
+		let code_hash = *self.code_hash();
-	}
-
-	/// Put the module blob into storage.
-	///
-	/// Increments the reference count of the in-storage `WasmBlob`, if it already exists in
-	/// storage.
-	fn store_code(mut module: Self, instantiated: bool) -> DispatchResult {
-		let code_hash = &module.code_hash().clone();
 		<CodeInfoOf<T>>::mutate(code_hash, |stored_code_info| {
 			match stored_code_info {
-				// Instantiate existing contract.
-				Some(stored_code_info) if instantiated => {
-					stored_code_info.refcount = stored_code_info.refcount.checked_add(1).expect(
-						"
-					 refcount is 64bit. Generating this overflow would require to store
-					 _at least_ 18 exabyte of data assuming that a contract consumes only
-					 one byte of data. Any node would run out of storage space before hitting
-					 this overflow;
-					 qed
-					",
-					);
-					Ok(())
-				},
 				// Contract code is already stored in storage. Nothing to be done here.
-				Some(_) => Ok(()),
+				Some(_) => Ok(Default::default()),
 				// Upload a new contract code.
-				//
 				// We need to store the code and its code_info, and collect the deposit.
+				// This `None` case happens only with freshly uploaded modules. This means that
+				// the `owner` is always the origin of the current transaction.
 				None => {
-					// This `None` case happens only in freshly uploaded modules. This means that
+					let deposit = self.code_info.deposit;
-					// the `owner` is always the origin of the current transaction.
+					T::Currency::reserve(&self.code_info.owner, deposit)
-					T::Currency::reserve(&module.code_info.owner, module.code_info.deposit)
 						.map_err(|_| <Error<T>>::StorageDepositNotEnoughFunds)?;
-					module.code_info.refcount = if instantiated { 1 } else { 0 };
+					self.code_info.refcount = 0;
-					<PristineCode<T>>::insert(code_hash, module.code);
+					<PristineCode<T>>::insert(code_hash, &self.code);
-					*stored_code_info = Some(module.code_info);
+					*stored_code_info = Some(self.code_info.clone());
-					<Pallet<T>>::deposit_event(
+					<Pallet<T>>::deposit_event(vec![code_hash], Event::CodeStored { code_hash });
-						vec![*code_hash],
+					Ok(deposit)
-						Event::CodeStored { code_hash: *code_hash },
-					);
-					Ok(())
 				},
 			}
 		})
@@ -331,17 +279,6 @@ impl<T: Config> WasmBlob<T> {
 		Ok(code)
 	}
 
-	/// See [`Self::from_code_unchecked`].
-	#[cfg(feature = "runtime-benchmarks")]
-	pub fn store_code_unchecked(
-		code: Vec<u8>,
-		schedule: &Schedule<T>,
-		owner: T::AccountId,
-	) -> DispatchResult {
-		let executable = Self::from_code_unchecked(code, schedule, owner)?;
-		Self::store_code(executable, false)
-	}
-
 	/// Create the module without checking the passed code.
 	///
 	/// # Note
@@ -350,7 +287,7 @@ impl<T: Config> WasmBlob<T> {
 	/// our results. This also does not collect any deposit from the `owner`. Also useful
 	/// during testing when we want to deploy codes that do not pass the instantiation checks.
 	#[cfg(any(test, feature = "runtime-benchmarks"))]
-	fn from_code_unchecked(
+	pub fn from_code_unchecked(
 		code: Vec<u8>,
 		schedule: &Schedule<T>,
 		owner: T::AccountId,
@@ -450,9 +387,8 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 				Error::<T>::CodeRejected
 			})?;
 
-		// We store before executing so that the code hash is available in the constructor.
 		if let &ExportedFunction::Constructor = function {
-			Self::store_code(self, true)?;
+			WasmBlob::<T>::increment_refcount(self.code_hash)?;
 		}
 
 		let result = exported_func.call(&mut store, &[], &mut []);
@@ -790,7 +726,6 @@ mod tests {
 				ext.borrow_mut().schedule(),
 				ALICE,
 				Determinism::Enforced,
-				TryInstantiate::Instantiate,
 			)
 			.map_err(|err| err.0)?
 		};

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

@@ -41,21 +41,6 @@ use wasmi::{
 /// compiler toolchains might not support specifying other modules than "env" for memory imports.
 pub const IMPORT_MODULE_MEMORY: &str = "env";
 
-/// Determines whether a module should be instantiated during preparation.
-pub enum TryInstantiate {
-	/// Do the instantiation to make sure that the module is valid.
-	///
-	/// This should be used if a module is only uploaded but not executed. We need
-	/// to make sure that it can be actually instantiated.
-	Instantiate,
-	/// Skip the instantiation during preparation.
-	///
-	/// This makes sense when the preparation takes place as part of an instantiation. Then
-	/// this instantiation would fail the whole transaction and an extra check is not
-	/// necessary.
-	Skip,
-}
-
 /// The inner deserialized module is valid and contains only allowed WebAssembly features.
 /// This is checked by loading it into wasmi interpreter `engine`.
 pub struct LoadedModule {
@@ -237,7 +222,6 @@ fn validate<E, T>(
 	code: &[u8],
 	schedule: &Schedule<T>,
 	determinism: Determinism,
-	try_instantiate: TryInstantiate,
 ) -> Result<(), (DispatchError, &'static str)>
 where
 	E: Environment<()>,
@@ -261,23 +245,22 @@ where
 	//
 	// - It doesn't use any unknown imports.
 	// - It doesn't explode the wasmi bytecode generation.
-	if matches!(try_instantiate, TryInstantiate::Instantiate) {
+	//
-		// We don't actually ever run any code so we can get away with a minimal stack which
+	// We don't actually ever execute this instance so we can get away with a minimal stack which
-		// reduces the amount of memory that needs to be zeroed.
+	// reduces the amount of memory that needs to be zeroed.
-		let stack_limits = StackLimits::new(1, 1, 0).expect("initial <= max; qed");
+	let stack_limits = StackLimits::new(1, 1, 0).expect("initial <= max; qed");
-		WasmBlob::<T>::instantiate::<E, _>(
+	WasmBlob::<T>::instantiate::<E, _>(
-			&code,
+		&code,
-			(),
+		(),
-			schedule,
+		schedule,
-			determinism,
+		determinism,
-			stack_limits,
+		stack_limits,
-			AllowDeprecatedInterface::No,
+		AllowDeprecatedInterface::No,
-		)
+	)
-		.map_err(|err| {
+	.map_err(|err| {
-			log::debug!(target: LOG_TARGET, "{}", err);
+		log::debug!(target: LOG_TARGET, "{}", err);
-			(Error::<T>::CodeRejected.into(), "New code rejected on wasmi instantiation!")
+		(Error::<T>::CodeRejected.into(), "New code rejected on wasmi instantiation!")
-		})?;
+	})?;
-	}
 
 	Ok(())
 }
@@ -295,13 +278,12 @@ pub fn prepare<E, T>(
 	schedule: &Schedule<T>,
 	owner: AccountIdOf<T>,
 	determinism: Determinism,
-	try_instantiate: TryInstantiate,
 ) -> Result<WasmBlob<T>, (DispatchError, &'static str)>
 where
 	E: Environment<()>,
 	T: Config,
 {
-	validate::<E, T>(code.as_ref(), schedule, determinism, try_instantiate)?;
+	validate::<E, T>(code.as_ref(), schedule, determinism)?;
 
 	// Calculate deposit for storing contract code and `code_info` in two different storage items.
 	let bytes_added = code.len().saturating_add(<CodeInfo<T>>::max_encoded_len()) as u32;
@@ -416,7 +398,6 @@ mod tests {
 					&schedule,
 					ALICE,
 					Determinism::Enforced,
-					TryInstantiate::Instantiate,
 				);
 				assert_matches::assert_matches!(r.map_err(|(_, msg)| msg), $($expected)*);
 			}