Make Executable::execute consume and store itself

frame/contracts/src/exec.rs

@@ -199,11 +199,8 @@ pub trait Executable<T: Config>: Sized {
 	/// queried for purposes other than execution.
 	fn from_storage_noinstr(code_hash: CodeHash<T>) -> Result<Self, DispatchError>;
 
-	/// Put the executable into storage. If it already exists there the refcount is incremented.
-	fn store(self);
-
 	/// Decrements the refcount by one and deletes the code if it drops to zero.
-	fn store_decremented(self);
+	fn drop_from_storage(self);
 
 	/// Increment the refcount by one. Fails if the code does not exist on-chain.
 	fn add_user(code_hash: CodeHash<T>) -> DispatchResult;
@@ -212,8 +209,16 @@ pub trait Executable<T: Config>: Sized {
 	fn remove_user(code_hash: CodeHash<T>);
 
 	/// Execute the specified exported function and return the result.
+	///
+	/// When the specified function is `Constructor` the executable is stored and its
+	/// refcount incremented.
+	///
+	/// # Note
+	///
+	/// This functions expects to be executed in a storage transaction that rolls back
+	/// all of its emitted storage changes.
 	fn execute<E: Ext<T = T>>(
-		&self,
+		self,
 		ext: E,
 		function: &ExportedFunction,
 		input_data: Vec<u8>,
@@ -333,7 +338,7 @@ where
 		&mut self,
 		endowment: BalanceOf<T>,
 		gas_meter: &mut GasMeter<T>,
-		executable: &E,
+		executable: E,
 		input_data: Vec<u8>,
 		salt: &[u8],
 	) -> Result<(T::AccountId, ExecReturnValue), ExecError> {
@@ -370,6 +375,12 @@ where
 					nested,
 				)?;
 
+				// Cache the value before calling into the constructor because that
+				// consumes the value. If the constructor creates additional contracts using
+				// the same code hash we still charge the "1 block rent" as if they weren't
+				// spawned. This is OK as overcharging is always safe.
+				let occupied_storage = executable.occupied_storage();
+
 				let output = executable.execute(
 					nested.new_call_context(caller.clone(), endowment),
 					&ExportedFunction::Constructor,
@@ -388,7 +399,7 @@ where
 				// This also makes sure that it is above the subsistence threshold
 				// in order to keep up the guarantuee that we always leave a tombstone behind
 				// with the exception of a contract that called `seal_terminate`.
-				Rent::<T, E>::charge(&dest, contract, executable.occupied_storage())?
+				Rent::<T, E>::charge(&dest, contract, occupied_storage)?
 					.ok_or(Error::<T>::NewContractNotFunded)?;
 
 				// Deposit an instantiation event.
@@ -570,8 +581,7 @@ where
 		salt: &[u8],
 	) -> Result<(AccountIdOf<T>, ExecReturnValue), ExecError> {
 		let executable = E::from_storage(code_hash, &self.ctx.config.schedule)?;
-		let result = self.ctx.instantiate(endowment, gas_meter, &executable, input_data, salt)?;
-		executable.store();
+		let result = self.ctx.instantiate(endowment, gas_meter, executable, input_data, salt)?;
 		Ok(result)
 	}
 
@@ -833,9 +843,7 @@ mod tests {
 			})
 		}
 
-		fn store(self) {}
-
-		fn store_decremented(self) {}
+		fn drop_from_storage(self) {}
 
 		fn add_user(_code_hash: CodeHash<Test>) -> DispatchResult {
 			Ok(())
@@ -844,7 +852,7 @@ mod tests {
 		fn remove_user(_code_hash: CodeHash<Test>) {}
 
 		fn execute<E: Ext<T = Test>>(
-			&self,
+			self,
 			mut ext: E,
 			_function: &ExportedFunction,
 			input_data: Vec<u8>,
@@ -1083,7 +1091,7 @@ mod tests {
 			let result = ctx.instantiate(
 				cfg.subsistence_threshold() * 3,
 				&mut GasMeter::<Test>::new(GAS_LIMIT),
-				&MockExecutable::from_storage(input_data_ch, &cfg.schedule).unwrap(),
+				MockExecutable::from_storage(input_data_ch, &cfg.schedule).unwrap(),
 				vec![1, 2, 3, 4],
 				&[],
 			);
@@ -1238,7 +1246,7 @@ mod tests {
 				ctx.instantiate(
 					0, // <- zero endowment
 					&mut GasMeter::<Test>::new(GAS_LIMIT),
-					&MockExecutable::from_storage(dummy_ch, &cfg.schedule).unwrap(),
+					MockExecutable::from_storage(dummy_ch, &cfg.schedule).unwrap(),
 					vec![],
 					&[],
 				),
@@ -1262,7 +1270,7 @@ mod tests {
 				ctx.instantiate(
 					100,
 					&mut GasMeter::<Test>::new(GAS_LIMIT),
-					&MockExecutable::from_storage(dummy_ch, &cfg.schedule).unwrap(),
+					MockExecutable::from_storage(dummy_ch, &cfg.schedule).unwrap(),
 					vec![],
 					&[],
 				),
@@ -1293,7 +1301,7 @@ mod tests {
 				ctx.instantiate(
 					100,
 					&mut GasMeter::<Test>::new(GAS_LIMIT),
-					&MockExecutable::from_storage(dummy_ch, &cfg.schedule).unwrap(),
+					MockExecutable::from_storage(dummy_ch, &cfg.schedule).unwrap(),
 					vec![],
 					&[],
 				),
@@ -1414,7 +1422,7 @@ mod tests {
 					ctx.instantiate(
 						100,
 						&mut GasMeter::<Test>::new(GAS_LIMIT),
-						&MockExecutable::from_storage(terminate_ch, &cfg.schedule).unwrap(),
+						MockExecutable::from_storage(terminate_ch, &cfg.schedule).unwrap(),
 						vec![],
 						&[],
 					),
@@ -1446,7 +1454,7 @@ mod tests {
 			let result = ctx.instantiate(
 				cfg.subsistence_threshold() * 5,
 				&mut GasMeter::<Test>::new(GAS_LIMIT),
-				&MockExecutable::from_storage(rent_allowance_ch, &cfg.schedule).unwrap(),
+				MockExecutable::from_storage(rent_allowance_ch, &cfg.schedule).unwrap(),
 				vec![],
 				&[],
 			);

frame/contracts/src/lib.rs

@@ -598,9 +598,8 @@ decl_module! {
 			let mut gas_meter = GasMeter::new(gas_limit);
 			let result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| {
 				let executable = PrefabWasmModule::from_code(code, &schedule)?;
-				let result = ctx.instantiate(endowment, gas_meter, &executable, data, &salt)
+				let result = ctx.instantiate(endowment, gas_meter, executable, data, &salt)
 					.map(|(_address, output)| output)?;
-				executable.store();
 				Ok(result)
 			});
 			gas_meter.into_dispatch_result(
@@ -630,9 +629,8 @@ decl_module! {
 			let mut gas_meter = GasMeter::new(gas_limit);
 			let result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| {
 				let executable = PrefabWasmModule::from_storage(code_hash, &ctx.config.schedule)?;
-				let result = ctx.instantiate(endowment, gas_meter, &executable, data, &salt)
+				let result = ctx.instantiate(endowment, gas_meter, executable, data, &salt)
 					.map(|(_address, output)| output)?;
-				executable.store();
 				Ok(result)
 			});
 			gas_meter.into_dispatch_result(
@@ -741,8 +739,7 @@ where
 	#[cfg(feature = "runtime-benchmarks")]
 	pub fn store_code_raw(code: Vec<u8>) -> DispatchResult {
 		let schedule = <Module<T>>::current_schedule();
-		let executable = PrefabWasmModule::from_code_unchecked(code, &schedule)?;
-		executable.store();
+		PrefabWasmModule::store_code_unchecked(code, &schedule)?;
 		Ok(())
 	}
 

frame/contracts/src/rent.rs

@@ -267,7 +267,7 @@ where
 				);
 				let tombstone_info = ContractInfo::Tombstone(tombstone);
 				<ContractInfoOf<T>>::insert(account, &tombstone_info);
-				code.store_decremented();
+				code.drop_from_storage();
 				<Module<T>>::deposit_event(RawEvent::Evicted(account.clone()));
 				Ok(None)
 			}

frame/contracts/src/tests.rs

@@ -514,19 +514,19 @@ fn instantiate_and_call_and_deposit_event() {
 				},
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::contracts(
-						RawEvent::ContractEmitted(addr.clone(), vec![1, 2, 3, 4])
-					),
+					event: MetaEvent::contracts(RawEvent::CodeStored(code_hash.into())),
 					topics: vec![],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::contracts(RawEvent::Instantiated(ALICE, addr.clone())),
+					event: MetaEvent::contracts(
+						RawEvent::ContractEmitted(addr.clone(), vec![1, 2, 3, 4])
+					),
 					topics: vec![],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::contracts(RawEvent::CodeStored(code_hash.into())),
+					event: MetaEvent::contracts(RawEvent::Instantiated(ALICE, addr.clone())),
 					topics: vec![],
 				},
 			]);
@@ -1243,12 +1243,12 @@ fn restoration(
 				},
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::contracts(RawEvent::Instantiated(ALICE, addr_bob.clone())),
+					event: MetaEvent::contracts(RawEvent::CodeStored(set_rent_code_hash.into())),
 					topics: vec![],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::contracts(RawEvent::CodeStored(set_rent_code_hash.into())),
+					event: MetaEvent::contracts(RawEvent::Instantiated(ALICE, addr_bob.clone())),
 					topics: vec![],
 				},
 			];
@@ -1446,14 +1446,15 @@ fn restoration(
 							},
 							EventRecord {
 								phase: Phase::Initialization,
-								event: MetaEvent::contracts(RawEvent::Instantiated(CHARLIE, addr_django.clone())),
+								event: MetaEvent::contracts(RawEvent::CodeStored(restoration_code_hash)),
 								topics: vec![],
 							},
 							EventRecord {
 								phase: Phase::Initialization,
-								event: MetaEvent::contracts(RawEvent::CodeStored(restoration_code_hash)),
+								event: MetaEvent::contracts(RawEvent::Instantiated(CHARLIE, addr_django.clone())),
 								topics: vec![],
 							},
+
 						]);
 					},
 					(false, false, true) => {

frame/contracts/src/wasm/mod.rs

@@ -106,18 +106,21 @@ where
 		prepare::prepare_contract(original_code, schedule).map_err(Into::into)
 	}
 
-	/// Create the module without checking nor instrumenting the passed code.
+	/// Create and store the module without checking nor instrumenting the passed code.
 	///
 	/// # Note
 	///
-	/// This is useful for benchmarking where we don't want instrumentation to skwe
+	/// This is useful for benchmarking where we don't want instrumentation to skew
 	/// our results.
 	#[cfg(feature = "runtime-benchmarks")]
-	pub fn from_code_unchecked(
+	pub fn store_code_unchecked(
 		original_code: Vec<u8>,
 		schedule: &Schedule<T>
-	) -> Result<Self, DispatchError> {
-		prepare::benchmarking::prepare_contract(original_code, schedule).map_err(Into::into)
+	) -> DispatchResult {
+		let executable = prepare::benchmarking::prepare_contract(original_code, schedule)
+			.map_err::<DispatchError, _>(Into::into)?;
+		code_cache::store(executable);
+		Ok(())
 	}
 
 	/// Return the refcount of the module.
@@ -142,11 +145,7 @@ where
 		code_cache::load(code_hash, None)
 	}
 
-	fn store(self) {
-		code_cache::store(self)
-	}
-
-	fn store_decremented(self) {
+	fn drop_from_storage(self) {
 		code_cache::store_decremented(self);
 	}
 
@@ -159,7 +158,7 @@ where
 	}
 
 	fn execute<E: Ext<T = T>>(
-		&self,
+		self,
 		mut ext: E,
 		function: &ExportedFunction,
 		input_data: Vec<u8>,
@@ -190,10 +189,17 @@ where
 			gas_meter,
 		);
 
+		// We store before executing so that the code hash is available in the constructor.
+		let code = self.code.clone();
+		if let &ExportedFunction::Constructor = function {
+			code_cache::store(self)
+		}
+
 		// Instantiate the instance from the instrumented module code and invoke the contract
 		// entrypoint.
-		let result = sp_sandbox::Instance::new(&self.code, &imports, &mut runtime)
+		let result = sp_sandbox::Instance::new(&code, &imports, &mut runtime)
 			.and_then(|mut instance| instance.invoke(function.identifier(), &[], &mut runtime));
+
 		runtime.to_execution_result(result)
 	}