fixes

frame/contracts/src/lib.rs

@@ -1065,6 +1065,7 @@ 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>),
@@ -1468,6 +1469,7 @@ impl<T: Config> Pallet<T> {
 		Ok(CodeUploadReturnValue { code_hash: *module.code_hash(), deposit })
 	}
 
+	/// Upload new code and returns the wasm blob and deposit amount paid.
 	fn try_upload_code(
 		origin: T::AccountId,
 		code: Vec<u8>,
@@ -1481,11 +1483,11 @@ impl<T: Config> Pallet<T> {
 				debug_message.as_mut().map(|d| d.try_extend(msg.bytes()));
 				err
 			})?;
-		let deposit = module.open_deposit(&module.code_info());
+		let deposit = module.store_code()?;
 		if let Some(storage_deposit_limit) = storage_deposit_limit {
 			ensure!(storage_deposit_limit >= deposit, <Error<T>>::StorageDepositLimitExhausted);
 		}
-		module.store()?;
+
 		Ok((module, deposit))
 	}
 
@@ -1530,7 +1532,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(())
 	}
 

frame/contracts/src/wasm/mod.rs

@@ -161,33 +161,13 @@ impl<T: Config> WasmBlob<T> {
 		)
 	}
 
-	/// Store the code without instantiating it.
-	///
-	/// Otherwise the code is stored when [`<Self as Executable>::execute`][`Executable::execute`]
-	/// is called.
-	pub fn store(&mut self) -> DispatchResult {
-		self.store_code(false)
-	}
-
 	/// Remove the code from storage and refund the deposit to its owner.
 	///
 	/// Applies all necessary checks before removing the code.
 	pub fn remove(origin: &T::AccountId, code_hash: CodeHash<T>) -> DispatchResult {
 		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.
@@ -222,7 +202,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,47 +230,51 @@ impl<T: Config> WasmBlob<T> {
 		&self.code_info
 	}
 
-	/// 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 self, instantiated: bool) -> DispatchResult {
+	/// Put the module blob into storage, and returns the deposit paid for the storage.
+	pub fn store_code(&mut self) -> Result<BalanceOf<T>, Error<T>> {
 		let code_hash = self.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 in 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
-					// the `owner` is always the origin of the current transaction.
-					T::Currency::reserve(&self.code_info.owner, self.code_info.deposit)
+					let deposit = self.code_info.deposit;
+					T::Currency::reserve(&self.code_info.owner, deposit)
 						.map_err(|_| <Error<T>>::StorageDepositNotEnoughFunds)?;
-					self.code_info.refcount = if instantiated { 1 } else { 0 };
+					self.code_info.refcount = 0;
 					<PristineCode<T>>::insert(code_hash, &self.code);
 					*stored_code_info = Some(self.code_info.clone());
 					<Pallet<T>>::deposit_event(vec![code_hash], Event::CodeStored { code_hash });
-					Ok(())
+					Ok(deposit)
 				},
 			}
 		})
 	}
 
+	/// Increments the reference count of the in-storage `WasmBlob`
+	fn increment_refcount(&mut self) -> DispatchResult {
+		let code_hash = self.code_hash().clone();
+		<CodeInfoOf<T>>::mutate(code_hash, |stored_code_info| match stored_code_info {
+			Some(stored_code_info) => {
+				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(())
+			},
+			None => Err(<Error<T>>::CodeNotFound.into()),
+		})
+	}
+
 	/// Try to remove code together with all associated information.
 	fn try_remove_code(origin: &T::AccountId, code_hash: CodeHash<T>) -> DispatchResult {
 		<CodeInfoOf<T>>::try_mutate_exists(&code_hash, |existing| {
@@ -323,17 +307,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
@@ -342,7 +315,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,
@@ -442,9 +415,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(true)?;
+			self.increment_refcount()?;
 		}
 
 		let result = exported_func.call(&mut store, &[], &mut []);