[pallet-revive] Update delegate_call to accept address and weight

prdoc/pr_6111.prdoc

@@ -0,0 +1,17 @@
+title: "[pallet-revive] Update delegate_call to accept address and weight"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Enhance the `delegate_call` function to accept an `address` target parameter instead of a `code_hash`.
+      This allows direct identification of the target contract using the provided address.
+      Additionally, introduce parameters for specifying a customizable `ref_time` limit and `proof_size` limit,
+      thereby improving flexibility and control during contract interactions.
+
+crates:
+  - name: pallet-revive
+    bump: major
+  - name: pallet-revive-fixtures
+    bump: major
+  - name: pallet-revive-uapi
+    bump: major

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

@@ -28,7 +28,11 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(code_hash: &[u8; 32],);
+	input!(
+		address: &[u8; 20],
+		ref_time: u64,
+		proof_size: u64,
+	);
 
 	let mut key = [0u8; 32];
 	key[0] = 1u8;
@@ -42,7 +46,7 @@ pub extern "C" fn call() {
 	assert!(value[0] == 2u8);
 
 	let input = [0u8; 0];
-	api::delegate_call(uapi::CallFlags::empty(), code_hash, &input, None).unwrap();
+	api::delegate_call(uapi::CallFlags::empty(), address, ref_time, proof_size, &input, None).unwrap();
 
 	api::get_storage(StorageFlags::empty(), &key, value).unwrap();
 	assert!(value[0] == 1u8);

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

@@ -28,9 +28,9 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(code_hash: &[u8; 32],);
+	input!(address: &[u8; 20],);
 
-	// Delegate call into passed code hash.
+	// Delegate call into passed address.
 	let input = [0u8; 0];
-	api::delegate_call(uapi::CallFlags::empty(), code_hash, &input, None).unwrap();
+	api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, &input, None).unwrap();
 }

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

@@ -30,6 +30,7 @@ const ETH_ALICE: [u8; 20] = [1u8; 20];
 fn load_input(delegate_call: bool) {
 	input!(
 		action: u32,
+		address: &[u8; 20],
 		code_hash: &[u8; 32],
 	);
 
@@ -51,7 +52,7 @@ fn load_input(delegate_call: bool) {
 	}
 
 	if delegate_call {
-		api::delegate_call(uapi::CallFlags::empty(), code_hash, &[], None).unwrap();
+		api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, &[], None).unwrap();
 	}
 }
 

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

@@ -1501,7 +1501,9 @@ mod benchmarks {
 			result = runtime.bench_delegate_call(
 				memory.as_mut_slice(),
 				0,        // flags
-				0,        // code_hash_ptr
+				0,        // address_ptr
+				0,        // ref_time_limit
+				0,        // proof_size_limit
 				0,        // input_data_ptr
 				0,        // input_data_len
 				SENTINEL, // output_ptr

substrate/frame/revive/src/exec.rs

@@ -198,7 +198,12 @@ pub trait Ext: sealing::Sealed {
 	/// Execute code in the current frame.
 	///
 	/// Returns the code size of the called contract.
-	fn delegate_call(&mut self, code: H256, input_data: Vec<u8>) -> Result<(), ExecError>;
+	fn delegate_call(
+		&mut self,
+		gas_limit: Weight,
+		address: H160,
+		input_data: Vec<u8>,
+	) -> Result<(), ExecError>;
 
 	/// Instantiate a contract from the given code.
 	///
@@ -1397,12 +1402,18 @@ where
 		result
 	}
 
-	fn delegate_call(&mut self, code_hash: H256, input_data: Vec<u8>) -> Result<(), ExecError> {
+	fn delegate_call(
+		&mut self,
+		gas_limit: Weight,
+		address: H160,
+		input_data: Vec<u8>,
+	) -> Result<(), ExecError> {
 		// We reset the return data now, so it is cleared out even if no new frame was executed.
 		// This is for example the case for unknown code hashes or creating the frame fails.
 		*self.last_frame_output_mut() = Default::default();
 
-		let executable = E::from_storage(code_hash, self.gas_meter_mut())?;
+		let contract_info = ContractInfoOf::<T>::get(&address).ok_or(Error::<T>::CodeNotFound)?;
+		let executable = E::from_storage(contract_info.code_hash, self.gas_meter_mut())?;
 		let top_frame = self.top_frame_mut();
 		let contract_info = top_frame.contract_info().clone();
 		let account_id = top_frame.account_id.clone();
@@ -1414,7 +1425,7 @@ where
 				delegated_call: Some(DelegatedCall { executable, caller: self.caller().clone() }),
 			},
 			value,
-			Weight::zero(),
+			gas_limit,
 			BalanceOf::<T>::zero(),
 			self.is_read_only(),
 		)?;
@@ -1839,7 +1850,7 @@ mod tests {
 		AddressMapper, Error,
 	};
 	use assert_matches::assert_matches;
-	use frame_support::{assert_err, assert_ok, parameter_types};
+	use frame_support::{assert_err, assert_noop, assert_ok, parameter_types};
 	use frame_system::{AccountInfo, EventRecord, Phase};
 	use pallet_revive_uapi::ReturnFlags;
 	use pretty_assertions::assert_eq;
@@ -2062,33 +2073,82 @@ mod tests {
 
 		let delegate_ch = MockLoader::insert(Call, move |ctx, _| {
 			assert_eq!(ctx.ext.value_transferred(), U256::from(value));
-			let _ = ctx.ext.delegate_call(success_ch, Vec::new())?;
+			let _ = ctx.ext.delegate_call(Weight::zero(), CHARLIE_ADDR, Vec::new())?;
 			Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
 		});
 
 		ExtBuilder::default().build().execute_with(|| {
 			place_contract(&BOB, delegate_ch);
+			place_contract(&CHARLIE, success_ch);
 			set_balance(&ALICE, 100);
 			let balance = get_balance(&BOB_CONTRACT_ID);
 			let origin = Origin::from_account_id(ALICE);
 			let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap();
 
-			let _ = MockStack::run_call(
+			assert_ok!(MockStack::run_call(
 				origin,
 				BOB_ADDR,
 				&mut GasMeter::<Test>::new(GAS_LIMIT),
 				&mut storage_meter,
 				value,
 				vec![],
 				None,
-			)
-			.unwrap();
+			));
 
 			assert_eq!(get_balance(&ALICE), 100 - value);
 			assert_eq!(get_balance(&BOB_CONTRACT_ID), balance + value);
 		});
 	}
 
+	#[test]
+	fn delegate_call_missing_contract() {
+		let missing_ch = MockLoader::insert(Call, move |_ctx, _| {
+			Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
+		});
+
+		let delegate_ch = MockLoader::insert(Call, move |ctx, _| {
+			let _ = ctx.ext.delegate_call(Weight::zero(), CHARLIE_ADDR, Vec::new())?;
+			Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
+		});
+
+		ExtBuilder::default().build().execute_with(|| {
+			place_contract(&BOB, delegate_ch);
+			set_balance(&ALICE, 100);
+
+			let origin = Origin::from_account_id(ALICE);
+			let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap();
+
+			// contract code missing
+			assert_noop!(
+				MockStack::run_call(
+					origin.clone(),
+					BOB_ADDR,
+					&mut GasMeter::<Test>::new(GAS_LIMIT),
+					&mut storage_meter,
+					0,
+					vec![],
+					None,
+				),
+				ExecError {
+					error: Error::<Test>::CodeNotFound.into(),
+					origin: ErrorOrigin::Callee,
+				}
+			);
+
+			// add missing contract code
+			place_contract(&CHARLIE, missing_ch);
+			assert_ok!(MockStack::run_call(
+				origin,
+				BOB_ADDR,
+				&mut GasMeter::<Test>::new(GAS_LIMIT),
+				&mut storage_meter,
+				0,
+				vec![],
+				None,
+			));
+		});
+	}
+
 	#[test]
 	fn changes_are_reverted_on_failing_call() {
 		// This test verifies that changes are reverted on a call which fails (or equally, returns
@@ -4410,7 +4470,7 @@ mod tests {
 			// An unknown code hash to fail the delegate_call on the first condition.
 			*ctx.ext.last_frame_output_mut() = output_revert();
 			assert_eq!(
-				ctx.ext.delegate_call(invalid_code_hash, Default::default()),
+				ctx.ext.delegate_call(Weight::zero(), H160([0xff; 20]), Default::default()),
 				Err(Error::<Test>::CodeNotFound.into())
 			);
 			assert_eq!(ctx.ext.last_frame_output(), &Default::default());
@@ -4529,7 +4589,7 @@ mod tests {
 
 			// In a delegate call, we should witness the caller immutable data
 			assert_eq!(
-				ctx.ext.delegate_call(charlie_ch, Vec::new()).map(|_| ctx
+				ctx.ext.delegate_call(Weight::zero(), CHARLIE_ADDR, Vec::new()).map(|_| ctx
 					.ext
 					.last_frame_output()
 					.data

substrate/frame/revive/src/tests.rs

@@ -1142,7 +1142,7 @@ mod run_tests {
 	#[test]
 	fn delegate_call() {
 		let (caller_wasm, _caller_code_hash) = compile_module("delegate_call").unwrap();
-		let (callee_wasm, callee_code_hash) = compile_module("delegate_call_lib").unwrap();
+		let (callee_wasm, _callee_code_hash) = compile_module("delegate_call_lib").unwrap();
 
 		ExtBuilder::default().existential_deposit(500).build().execute_with(|| {
 			let _ = <Test as Config>::Currency::set_balance(&ALICE, 1_000_000);
@@ -1152,12 +1152,53 @@ mod run_tests {
 				builder::bare_instantiate(Code::Upload(caller_wasm))
 					.value(300_000)
 					.build_and_unwrap_contract();
-			// Only upload 'callee' code
-			assert_ok!(Contracts::upload_code(RuntimeOrigin::signed(ALICE), callee_wasm, 100_000,));
+
+			// Instantiate the 'callee'
+			let Contract { addr: callee_addr, .. } =
+				builder::bare_instantiate(Code::Upload(callee_wasm))
+					.value(100_000)
+					.build_and_unwrap_contract();
 
 			assert_ok!(builder::call(caller_addr)
 				.value(1337)
-				.data(callee_code_hash.as_ref().to_vec())
+				.data((callee_addr, 0u64, 0u64).encode())
+				.build());
+		});
+	}
+
+	#[test]
+	fn delegate_call_with_limits() {
+		let (caller_wasm, _caller_code_hash) = compile_module("delegate_call").unwrap();
+		let (callee_wasm, _callee_code_hash) = compile_module("delegate_call_lib").unwrap();
+
+		ExtBuilder::default().existential_deposit(500).build().execute_with(|| {
+			let _ = <Test as Config>::Currency::set_balance(&ALICE, 1_000_000);
+
+			// Instantiate the 'caller'
+			let Contract { addr: caller_addr, .. } =
+				builder::bare_instantiate(Code::Upload(caller_wasm))
+					.value(300_000)
+					.build_and_unwrap_contract();
+
+			// Instantiate the 'callee'
+			let Contract { addr: callee_addr, .. } =
+				builder::bare_instantiate(Code::Upload(callee_wasm))
+					.value(100_000)
+					.build_and_unwrap_contract();
+
+			// fails, not enough weight
+			assert_err!(
+				builder::bare_call(caller_addr)
+					.value(1337)
+					.data((callee_addr, 100u64, 100u64).encode())
+					.build()
+					.result,
+				Error::<Test>::ContractTrapped,
+			);
+
+			assert_ok!(builder::call(caller_addr)
+				.value(1337)
+				.data((callee_addr, 500_000_000u64, 100_000u64).encode())
 				.build());
 		});
 	}
@@ -3722,6 +3763,12 @@ mod run_tests {
 			.map(|c| sp_core::H256(sp_io::hashing::keccak_256(c)))
 			.collect();
 
+		let hash2addr = |code_hash: &H256| {
+			let mut addr = H160::zero();
+			addr.as_bytes_mut().copy_from_slice(&code_hash.as_ref()[..20]);
+			addr
+		};
+
 		// Define inputs with various actions to test locking / unlocking delegate_dependencies.
 		// See the contract for more details.
 		let noop_input = (0u32, callee_hashes[0]);
@@ -3731,17 +3778,19 @@ mod run_tests {
 
 		// Instantiate the caller contract with the given input.
 		let instantiate = |input: &(u32, H256)| {
+			let (action, code_hash) = input;
 			builder::bare_instantiate(Code::Upload(wasm_caller.clone()))
 				.origin(RuntimeOrigin::signed(ETH_ALICE))
-				.data(input.encode())
+				.data((action, hash2addr(code_hash), code_hash).encode())
 				.build()
 		};
 
 		// Call contract with the given input.
 		let call = |addr_caller: &H160, input: &(u32, H256)| {
+			let (action, code_hash) = input;
 			builder::bare_call(*addr_caller)
 				.origin(RuntimeOrigin::signed(ETH_ALICE))
-				.data(input.encode())
+				.data((action, hash2addr(code_hash), code_hash).encode())
 				.build()
 		};
 		const ED: u64 = 2000;
@@ -3758,14 +3807,17 @@ mod run_tests {
 			// Upload all the delegated codes (they all have the same size)
 			let mut deposit = Default::default();
 			for code in callee_codes.iter() {
-				let CodeUploadReturnValue { deposit: deposit_per_code, .. } =
+				let CodeUploadReturnValue { deposit: deposit_per_code, code_hash } =
 					Contracts::bare_upload_code(
 						RuntimeOrigin::signed(ETH_ALICE),
 						code.clone(),
 						deposit_limit::<Test>(),
 					)
 					.unwrap();
 				deposit = deposit_per_code;
+				// Mock contract info by using first 20 bytes of code_hash as address.
+				let addr = hash2addr(&code_hash);
+				ContractInfoOf::<Test>::set(&addr, ContractInfo::new(&addr, 0, code_hash).ok());
 			}
 
 			// Instantiate should now work.
@@ -3802,7 +3854,11 @@ mod run_tests {
 
 			// Locking self should fail.
 			assert_err!(
-				call(&addr_caller, &(1u32, self_code_hash)).result,
+				builder::bare_call(addr_caller)
+					.origin(RuntimeOrigin::signed(ETH_ALICE))
+					.data((1u32, &addr_caller, self_code_hash).encode())
+					.build()
+					.result,
 				Error::<Test>::CannotAddSelfAsDelegateDependency
 			);
 
@@ -3841,7 +3897,7 @@ mod run_tests {
 			assert_err!(
 				builder::bare_call(addr_caller)
 					.storage_deposit_limit(dependency_deposit - 1)
-					.data(lock_delegate_dependency_input.encode())
+					.data((1u32, hash2addr(&callee_hashes[0]), callee_hashes[0]).encode())
 					.build()
 					.result,
 				Error::<Test>::StorageDepositLimitExhausted