revive: Include immutable storage deposit into the contracts storage_base_deposit

prdoc/pr_7230.prdoc

@@ -0,0 +1,46 @@
+title: 'revive: Include immutable storage deposit into the contracts `storage_base_deposit`'
+doc:
+- audience: Runtime Dev
+  description: |-
+    This PR is centered around a main fix regarding the base deposit and a bunch of drive by or related fixtures that make sense to resolve in one go. It could be broken down more but I am constantly rebasing this PR and would appreciate getting those fixes in as-one.
+
+    ## Record the deposit for immutable data into the `storage_base_deposit`
+
+    The `storage_base_deposit` are all the deposit a contract has to pay for existing. It included the deposit for its own metadata and a deposit proportional (< 1.0x) to the size of its code. However, the immutable code size was not recorded there. This would lead to the situation where on terminate this portion wouldn't be refunded staying locked into the contract. It would also make the calculation of the deposit changes on `set_code_hash` more complicated when it updates the immutable data (to be done in #6985). Reason is because it didn't know how much was payed before since the storage prices could have changed in the mean time.
+
+    In order for this solution to work I needed to delay the deposit calculation for a new contract for after the contract is done executing  is constructor as only then we know the immutable data size. Before, we just charged this eagerly in `charge_instantiate` before we execute the constructor. Now, we merely send the ED as free balance before the constructor in order to create the account. After the constructor is done we calculate the contract base deposit and charge it. This will make `set_code_hash` much easier to implement.
+
+    As a side effect it is now legal to call `set_immutable_data` multiple times per constructor (even though I see no reason to do so). It simply overrides the immutable data with the new value. The deposit accounting will be done after the constructor returns (as mentioned above) instead of when setting the immutable data.
+
+    ## Don't pre-charge for reading immutable data
+
+    I noticed that we were pre-charging weight for the max allowable immutable data when reading those values and then refunding after read. This is not necessary as we know its length without reading the storage as we store it out of band in contract metadata. This makes reading it free. Less pre-charging less problems.
+
+    ## Remove delegate locking
+
+    Fixes #7092
+
+    This is also in the spirit of making #6985 easier to implement. The locking complicates `set_code_hash` as we might need to block settings the code hash when locks exist. Check #7092 for further rationale.
+
+    ## Enforce "no terminate in constructor" eagerly
+
+    We used to enforce this rule after the contract execution returned. Now we error out early in the host call. This makes it easier to be sure to argue that a contract info still exists (wasn't terminated) when a constructor successfully returns. All around this his just much simpler than dealing this check.
+
+    ## Moved refcount functions to `CodeInfo`
+
+    They never really made sense to exist on `Stack`. But now with the locking gone this makes even less sense. The refcount is stored inside `CodeInfo` to lets just move them there.
+
+    ## Set `CodeHashLockupDepositPercent` for test runtime
+
+    The test runtime was setting `CodeHashLockupDepositPercent` to zero. This was trivializing many code paths and excluded them from testing. I set it to `30%` which is our default value and fixed up all the tests that broke. This should give us confidence that the lockup doeposit collections properly works.
+
+    ## Reworked the `MockExecutable` to have both a `deploy` and a `call` entry point
+
+    This type used for testing could only have either entry points but not both. In order to fix the `immutable_data_set_overrides` I needed to a new function `add_both` to `MockExecutable` that allows to have both entry points. Make sure to make use of it in the future :)
+crates:
+- name: pallet-revive-fixtures
+  bump: major
+- name: pallet-revive
+  bump: major
+- name: pallet-revive-uapi
+  bump: major

substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs

@@ -20,7 +20,9 @@
 #![no_main]
 
 use common::{input, u256_bytes};
-use uapi::{HostFn, HostFnImpl as api};
+use uapi::{HostFn, HostFnImpl as api, StorageFlags};
+
+static BUFFER: [u8; 16 * 1024 + 1] = [0u8; 16 * 1024 + 1];
 
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
@@ -30,11 +32,16 @@ pub extern "C" fn deploy() {}
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
 	input!(
-		input: [u8; 4],
+		len: u32,
 		code_hash: &[u8; 32],
 		deposit_limit: &[u8; 32],
 	);
 
+	let data = &BUFFER[..len as usize];
+	let mut key = [0u8; 32];
+	key[0] = 1;
+	api::set_storage(StorageFlags::empty(), &key, data);
+
 	let value = u256_bytes(10_000u64);
 	let salt = [0u8; 32];
 	let mut address = [0u8; 20];
@@ -45,7 +52,7 @@ pub extern "C" fn call() {
 		u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		deposit_limit,
 		&value,
-		input,
+		&len.to_le_bytes(),
 		Some(&mut address),
 		None,
 		Some(&salt),

substrate/frame/revive/fixtures/contracts/self_destruct.rs

@@ -25,7 +25,10 @@ const DJANGO_FALLBACK: [u8; 20] = [4u8; 20];
 
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
-pub extern "C" fn deploy() {}
+pub extern "C" fn deploy() {
+	// make sure that the deposit for the immutable data is refunded
+	api::set_immutable_data(&[1, 2, 3, 4, 5])
+}
 
 #[no_mangle]
 #[polkavm_derive::polkavm_export]

substrate/frame/revive/src/benchmarking/mod.rs

@@ -24,7 +24,7 @@ mod code;
 use self::{call_builder::CallSetup, code::WasmModule};
 use crate::{
 	evm::runtime::GAS_PRICE,
-	exec::{Key, MomentOf},
+	exec::{Ext, Key, MomentOf},
 	limits,
 	storage::WriteOutcome,
 	Pallet as Contracts, *,
@@ -1011,24 +1011,11 @@ mod benchmarks {
 	}
 
 	#[benchmark(pov_mode = Measured)]
-	fn seal_terminate(
-		n: Linear<0, { limits::DELEGATE_DEPENDENCIES }>,
-	) -> Result<(), BenchmarkError> {
+	fn seal_terminate() -> Result<(), BenchmarkError> {
 		let beneficiary = account::<T::AccountId>("beneficiary", 0, 0);
-		let caller = whitelisted_caller();
-		T::Currency::set_balance(&caller, caller_funding::<T>());
-		let origin = RawOrigin::Signed(caller);
-		let storage_deposit = default_deposit_limit::<T>();
 
 		build_runtime!(runtime, memory: [beneficiary.encode(),]);
 
-		(0..n).for_each(|i| {
-			let new_code = WasmModule::dummy_unique(65 + i);
-			Contracts::<T>::bare_upload_code(origin.clone().into(), new_code.code, storage_deposit)
-				.unwrap();
-			runtime.ext().lock_delegate_dependency(new_code.hash).unwrap();
-		});
-
 		let result;
 		#[block]
 		{
@@ -1930,43 +1917,6 @@ mod benchmarks {
 		Ok(())
 	}
 
-	#[benchmark(pov_mode = Measured)]
-	fn lock_delegate_dependency() -> Result<(), BenchmarkError> {
-		let code_hash = Contract::<T>::with_index(1, WasmModule::dummy_unique(1), vec![])?
-			.info()?
-			.code_hash;
-
-		build_runtime!(runtime, memory: [ code_hash.encode(),]);
-
-		let result;
-		#[block]
-		{
-			result = runtime.bench_lock_delegate_dependency(memory.as_mut_slice(), 0);
-		}
-
-		assert_ok!(result);
-		Ok(())
-	}
-
-	#[benchmark]
-	fn unlock_delegate_dependency() -> Result<(), BenchmarkError> {
-		let code_hash = Contract::<T>::with_index(1, WasmModule::dummy_unique(1), vec![])?
-			.info()?
-			.code_hash;
-
-		build_runtime!(runtime, memory: [ code_hash.encode(),]);
-		runtime.bench_lock_delegate_dependency(memory.as_mut_slice(), 0).unwrap();
-
-		let result;
-		#[block]
-		{
-			result = runtime.bench_unlock_delegate_dependency(memory.as_mut_slice(), 0);
-		}
-
-		assert_ok!(result);
-		Ok(())
-	}
-
 	// Benchmark the execution of instructions.
 	#[benchmark(pov_mode = Ignored)]
 	fn instr(r: Linear<0, INSTR_BENCHMARK_RUNS>) {