Merge branch 'master' into ib-dev-flag-with-manual-seal

.github/workflows/command-backport.yml

@@ -16,6 +16,7 @@ jobs:
   backport:
     name: Backport pull request
     runs-on: ubuntu-latest
+    environment: release
 
     # The 'github.event.pull_request.merged' ensures that it got into master:
     if: >

.gitlab/pipeline/zombienet/parachain-template.yml

@@ -43,4 +43,4 @@ zombienet-parachain-template-smoke:
     - ls -ltr $(pwd)/artifacts
     - cargo test -p template-zombienet-tests  --features zombienet --tests  minimal_template_block_production_test
     - cargo test -p template-zombienet-tests  --features zombienet --tests  parachain_template_block_production_test
-    # - cargo test -p template-zombienet-tests  --features zombienet --tests  solochain_template_block_production_test
+    - cargo test -p template-zombienet-tests  --features zombienet --tests  solochain_template_block_production_test

prdoc/pr_6741.prdoc

@@ -0,0 +1,16 @@
+title: 'pallet-revive: Adjust error handling of sub calls'
+doc:
+- audience: Runtime Dev
+  description: |-
+    We were trapping the host context in case a sub call was exhausting the storage deposit limit set for this sub call. This prevents the caller from handling this error. In this PR we added a new error code that is returned when either gas or storage deposit limit is exhausted by the sub call.
+
+    We also remove the longer used `NotCallable` error. No longer used because this is no longer an error: It will just be a balance transfer.
+
+    We also make `set_code_hash` infallible to be consistent with other host functions which just trap on any error condition.
+crates:
+- name: pallet-revive
+  bump: major
+- name: pallet-revive-uapi
+  bump: major
+- name: pallet-revive-fixtures
+  bump: major

prdoc/pr_6760.prdoc

@@ -0,0 +1,9 @@
+title: 'chainHead: Always report discarded items for storage operations'
+doc:
+- audience: [Node Dev, Node Operator]
+  description: |-
+    This PR ensures that substrate always reports discarded items as zero.
+    This is needed to align with the rpc-v2 spec
+crates:
+- name: sc-rpc-spec-v2
+  bump: patch

substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs

@@ -318,7 +318,7 @@ where
 				}),
 			};
 
-			let (rp, rp_fut) = method_started_response(operation_id);
+			let (rp, rp_fut) = method_started_response(operation_id, None);
 			let fut = async move {
 				// Wait for the server to send out the response and if it produces an error no event
 				// should be generated.
@@ -432,7 +432,8 @@ where
 
 		let mut storage_client = ChainHeadStorage::<Client, Block, BE>::new(self.client.clone());
 
-		let (rp, rp_fut) = method_started_response(block_guard.operation().operation_id());
+		// Storage items are never discarded.
+		let (rp, rp_fut) = method_started_response(block_guard.operation().operation_id(), Some(0));
 
 		let fut = async move {
 			// Wait for the server to send out the response and if it produces an error no event
@@ -507,7 +508,7 @@ where
 		let operation_id = block_guard.operation().operation_id();
 		let client = self.client.clone();
 
-		let (rp, rp_fut) = method_started_response(operation_id.clone());
+		let (rp, rp_fut) = method_started_response(operation_id.clone(), None);
 		let fut = async move {
 			// Wait for the server to send out the response and if it produces an error no event
 			// should be generated.
@@ -630,8 +631,9 @@ where
 
 fn method_started_response(
 	operation_id: String,
+	discarded_items: Option<usize>,
 ) -> (ResponsePayload<'static, MethodResponse>, MethodResponseFuture) {
-	let rp = MethodResponse::Started(MethodResponseStarted { operation_id, discarded_items: None });
+	let rp = MethodResponse::Started(MethodResponseStarted { operation_id, discarded_items });
 	ResponsePayload::success(rp).notify_on_completion()
 }
 

substrate/client/rpc-spec-v2/src/chain_head/tests.rs

@@ -2871,7 +2871,7 @@ async fn ensure_operation_limits_works() {
 	let operation_id = match response {
 		MethodResponse::Started(started) => {
 			// Check discarded items.
-			assert!(started.discarded_items.is_none());
+			assert_eq!(started.discarded_items, Some(0));
 			started.operation_id
 		},
 		MethodResponse::LimitReached => panic!("Expected started response"),
@@ -3228,7 +3228,10 @@ async fn storage_closest_merkle_value() {
 			.await
 			.unwrap();
 		let operation_id = match response {
-			MethodResponse::Started(started) => started.operation_id,
+			MethodResponse::Started(started) => {
+				assert_eq!(started.discarded_items, Some(0));
+				started.operation_id
+			},
 			MethodResponse::LimitReached => panic!("Expected started response"),
 		};
 

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

@@ -65,7 +65,7 @@ pub extern "C" fn call() {
 		None,
 		Some(&salt),
 	);
-	assert!(matches!(res, Err(ReturnErrorCode::CalleeTrapped)));
+	assert!(matches!(res, Err(ReturnErrorCode::OutOfResources)));
 
 	// Fail to deploy the contract due to insufficient proof_size weight.
 	let res = api::instantiate(
@@ -79,7 +79,7 @@ pub extern "C" fn call() {
 		None,
 		Some(&salt),
 	);
-	assert!(matches!(res, Err(ReturnErrorCode::CalleeTrapped)));
+	assert!(matches!(res, Err(ReturnErrorCode::OutOfResources)));
 
 	// Deploy the contract successfully.
 	let mut callee = [0u8; 20];
@@ -121,7 +121,7 @@ pub extern "C" fn call() {
 		&input,
 		None,
 	);
-	assert!(matches!(res, Err(ReturnErrorCode::CalleeTrapped)));
+	assert!(matches!(res, Err(ReturnErrorCode::OutOfResources)));
 
 	// Fail to call the contract due to insufficient proof_size weight.
 	let res = api::call(
@@ -134,7 +134,7 @@ pub extern "C" fn call() {
 		&input,
 		None,
 	);
-	assert!(matches!(res, Err(ReturnErrorCode::CalleeTrapped)));
+	assert!(matches!(res, Err(ReturnErrorCode::OutOfResources)));
 
 	// Call the contract successfully.
 	let mut output = [0u8; 4];

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

@@ -40,7 +40,7 @@ pub extern "C" fn call() {
 	api::set_storage(StorageFlags::empty(), buffer, &[1u8; 4]);
 
 	// Call the callee
-	api::call(
+	let ret = api::call(
 		uapi::CallFlags::empty(),
 		callee,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
@@ -49,8 +49,10 @@ pub extern "C" fn call() {
 		&[0u8; 32], // Value transferred to the contract.
 		input,
 		None,
-	)
-	.unwrap();
+	);
+	if let Err(code) = ret {
+		api::return_value(uapi::ReturnFlags::REVERT, &(code as u32).to_le_bytes());
+	};
 
 	// create 8 byte of storage after calling
 	// item of 12 bytes because we override 4 bytes

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

@@ -39,7 +39,7 @@ pub extern "C" fn call() {
 	let salt = [0u8; 32];
 	let mut address = [0u8; 20];
 
-	api::instantiate(
+	let ret = api::instantiate(
 		code_hash,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
 		0u64, // How much proof_size weight to devote for the execution. 0 = all.
@@ -49,8 +49,10 @@ pub extern "C" fn call() {
 		Some(&mut address),
 		None,
 		Some(&salt),
-	)
-	.unwrap();
+	);
+	if let Err(code) = ret {
+		api::return_value(uapi::ReturnFlags::REVERT, &(code as u32).to_le_bytes());
+	};
 
 	// Return the deployed contract address.
 	api::return_value(uapi::ReturnFlags::empty(), &address);

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

@@ -34,7 +34,11 @@ pub extern "C" fn call() {
 	);
 
 	let input = [0u8; 0];
-	api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, Some(&u256_bytes(deposit_limit)), &input, None).unwrap();
+	let ret = api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, Some(&u256_bytes(deposit_limit)), &input, None);
+
+	if let Err(code) = ret {
+		api::return_value(uapi::ReturnFlags::REVERT, &(code as u32).to_le_bytes());
+	};
 
 	let mut key = [0u8; 32];
 	key[0] = 1u8;

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

@@ -29,7 +29,7 @@ pub extern "C" fn deploy() {}
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
 	input!(addr: &[u8; 32],);
-	api::set_code_hash(addr).unwrap();
+	api::set_code_hash(addr);
 
 	// we return 1 after setting new code_hash
 	// next `call` will NOT return this value, because contract code has been changed

substrate/frame/revive/src/exec.rs

@@ -1068,7 +1068,7 @@ where
 
 		self.transient_storage.start_transaction();
 
-		let do_transaction = || {
+		let do_transaction = || -> ExecResult {
 			let caller = self.caller();
 			let frame = top_frame_mut!(self);
 
@@ -1107,11 +1107,8 @@ where
 			let call_span = T::Debug::new_call_span(&contract_address, entry_point, &input_data);
 
 			let output = T::Debug::intercept_call(&contract_address, entry_point, &input_data)
-				.unwrap_or_else(|| {
-					executable
-						.execute(self, entry_point, input_data)
-						.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })
-				})?;
+				.unwrap_or_else(|| executable.execute(self, entry_point, input_data))
+				.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;
 
 			call_span.after_call(&output);
 
@@ -1136,7 +1133,10 @@ where
 			// If a special limit was set for the sub-call, we enforce it here.
 			// The sub-call will be rolled back in case the limit is exhausted.
 			let contract = frame.contract_info.as_contract();
-			frame.nested_storage.enforce_subcall_limit(contract)?;
+			frame
+				.nested_storage
+				.enforce_subcall_limit(contract)
+				.map_err(|e| ExecError { error: e, origin: ErrorOrigin::Callee })?;
 
 			let account_id = T::AddressMapper::to_address(&frame.account_id);
 			match (entry_point, delegated_code_hash) {
@@ -3412,7 +3412,7 @@ mod tests {
 					true,
 					false
 				),
-				<Error<Test>>::TransferFailed
+				<Error<Test>>::TransferFailed,
 			);
 			exec_success()
 		});

substrate/frame/revive/src/tests.rs

@@ -38,8 +38,8 @@ use crate::{
 	wasm::Memory,
 	weights::WeightInfo,
 	AccountId32Mapper, BalanceOf, Code, CodeInfoOf, CollectEvents, Config, ContractInfo,
-	ContractInfoOf, DebugInfo, DeletionQueueCounter, Error, HoldReason, Origin, Pallet,
-	PristineCode, H160,
+	ContractInfoOf, DebugInfo, DeletionQueueCounter, DepositLimit, Error, HoldReason, Origin,
+	Pallet, PristineCode, H160,
 };
 
 use crate::test_utils::builder::Contract;
@@ -1211,14 +1211,11 @@ fn delegate_call_with_deposit_limit() {
 
 		// Delegate call will write 1 storage and deposit of 2 (1 item) + 32 (bytes) is required.
 		// Fails, not enough deposit
-		assert_err!(
-			builder::bare_call(caller_addr)
-				.value(1337)
-				.data((callee_addr, 33u64).encode())
-				.build()
-				.result,
-			Error::<Test>::StorageDepositLimitExhausted,
-		);
+		let ret = builder::bare_call(caller_addr)
+			.value(1337)
+			.data((callee_addr, 33u64).encode())
+			.build_and_unwrap_result();
+		assert_return_code!(ret, RuntimeReturnCode::OutOfResources);
 
 		assert_ok!(builder::call(caller_addr)
 			.value(1337)
@@ -1678,8 +1675,8 @@ fn instantiate_return_code() {
 
 		// Contract has enough balance but the passed code hash is invalid
 		<Test as Config>::Currency::set_balance(&contract.account_id, min_balance + 10_000);
-		let result = builder::bare_call(contract.addr).data(vec![0; 33]).build_and_unwrap_result();
-		assert_return_code!(result, RuntimeReturnCode::CodeNotFound);
+		let result = builder::bare_call(contract.addr).data(vec![0; 33]).build();
+		assert_err!(result.result, <Error<Test>>::CodeNotFound);
 
 		// Contract has enough balance but callee reverts because "1" is passed.
 		let result = builder::bare_call(contract.addr)
@@ -3463,13 +3460,11 @@ fn deposit_limit_in_nested_calls() {
 		// nested call. This should fail as callee adds up 2 bytes to the storage, meaning
 		// that the nested call should have a deposit limit of at least 2 Balance. The
 		// sub-call should be rolled back, which is covered by the next test case.
-		assert_err_ignore_postinfo!(
-			builder::call(addr_caller)
-				.storage_deposit_limit(16)
-				.data((102u32, &addr_callee, U256::from(1u64)).encode())
-				.build(),
-			<Error<Test>>::StorageDepositLimitExhausted,
-		);
+		let ret = builder::bare_call(addr_caller)
+			.storage_deposit_limit(DepositLimit::Balance(16))
+			.data((102u32, &addr_callee, U256::from(1u64)).encode())
+			.build_and_unwrap_result();
+		assert_return_code!(ret, RuntimeReturnCode::OutOfResources);
 
 		// Refund in the callee contract but not enough to cover the 14 Balance required by the
 		// caller. Note that if previous sub-call wouldn't roll back, this call would pass
@@ -3485,13 +3480,11 @@ fn deposit_limit_in_nested_calls() {
 		let _ = <Test as Config>::Currency::set_balance(&ALICE, 511);
 
 		// Require more than the sender's balance.
-		// We don't set a special limit for the nested call.
-		assert_err_ignore_postinfo!(
-			builder::call(addr_caller)
-				.data((512u32, &addr_callee, U256::from(1u64)).encode())
-				.build(),
-			<Error<Test>>::StorageDepositLimitExhausted,
-		);
+		// Limit the sub call to little balance so it should fail in there
+		let ret = builder::bare_call(addr_caller)
+			.data((512u32, &addr_callee, U256::from(1u64)).encode())
+			.build_and_unwrap_result();
+		assert_return_code!(ret, RuntimeReturnCode::OutOfResources);
 
 		// Same as above but allow for the additional deposit of 1 Balance in parent.
 		// We set the special deposit limit of 1 Balance for the nested call, which isn't
@@ -3563,29 +3556,25 @@ fn deposit_limit_in_nested_instantiate() {
 		// Now we set enough limit in parent call, but an insufficient limit for child
 		// instantiate. This should fail during the charging for the instantiation in
 		// `RawMeter::charge_instantiate()`
-		assert_err_ignore_postinfo!(
-			builder::call(addr_caller)
-				.origin(RuntimeOrigin::signed(BOB))
-				.storage_deposit_limit(callee_info_len + 2 + ED + 2)
-				.data((0u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 1)).encode())
-				.build(),
-			<Error<Test>>::StorageDepositLimitExhausted,
-		);
+		let ret = builder::bare_call(addr_caller)
+			.origin(RuntimeOrigin::signed(BOB))
+			.storage_deposit_limit(DepositLimit::Balance(callee_info_len + 2 + ED + 2))
+			.data((0u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 1)).encode())
+			.build_and_unwrap_result();
+		assert_return_code!(ret, RuntimeReturnCode::OutOfResources);
 		// The charges made on the instantiation should be rolled back.
 		assert_eq!(<Test as Config>::Currency::free_balance(&BOB), 1_000_000);
 
 		// Same as above but requires for single added storage
 		// item of 1 byte to be covered by the limit, which implies 3 more Balance.
 		// Now we set enough limit for the parent call, but insufficient limit for child
 		// instantiate. This should fail right after the constructor execution.
-		assert_err_ignore_postinfo!(
-			builder::call(addr_caller)
-				.origin(RuntimeOrigin::signed(BOB))
-				.storage_deposit_limit(callee_info_len + 2 + ED + 3) // enough parent limit
-				.data((1u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 2)).encode())
-				.build(),
-			<Error<Test>>::StorageDepositLimitExhausted,
-		);
+		let ret = builder::bare_call(addr_caller)
+			.origin(RuntimeOrigin::signed(BOB))
+			.storage_deposit_limit(DepositLimit::Balance(callee_info_len + 2 + ED + 3)) // enough parent limit
+			.data((1u32, &code_hash_callee, U256::from(callee_info_len + 2 + ED + 2)).encode())
+			.build_and_unwrap_result();
+		assert_return_code!(ret, RuntimeReturnCode::OutOfResources);
 		// The charges made on the instantiation should be rolled back.
 		assert_eq!(<Test as Config>::Currency::free_balance(&BOB), 1_000_000);
 
@@ -3890,7 +3879,7 @@ fn locking_delegate_dependency_works() {
 		assert_ok!(Contracts::remove_code(RuntimeOrigin::signed(ALICE_FALLBACK), callee_hashes[0]));
 
 		// Calling should fail since the delegated contract is not on chain anymore.
-		assert_err!(call(&addr_caller, &noop_input).result, Error::<Test>::ContractTrapped);
+		assert_err!(call(&addr_caller, &noop_input).result, Error::<Test>::CodeNotFound);
 
 		// Add the dependency back.
 		Contracts::upload_code(