diff --git a/prdoc/pr_6111.prdoc b/prdoc/pr_6111.prdoc new file mode 100644 index 000000000000..4ada3031c805 --- /dev/null +++ b/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: patch + - name: pallet-revive-uapi + bump: major diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call.rs b/substrate/frame/revive/fixtures/contracts/delegate_call.rs index 9fd155408af3..3cf74acf1321 100644 --- a/substrate/frame/revive/fixtures/contracts/delegate_call.rs +++ b/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, None, &input, None).unwrap(); api::get_storage(StorageFlags::empty(), &key, value).unwrap(); assert!(value[0] == 1u8); diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs new file mode 100644 index 000000000000..55203d534c9b --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs @@ -0,0 +1,46 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#![no_std] +#![no_main] + +use common::{input, u256_bytes}; +use uapi::{HostFn, HostFnImpl as api, StorageFlags}; + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn deploy() {} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn call() { + input!( + address: &[u8; 20], + deposit_limit: u64, + ); + + let input = [0u8; 0]; + api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, Some(&u256_bytes(deposit_limit)), &input, None).unwrap(); + + let mut key = [0u8; 32]; + key[0] = 1u8; + + let mut value = [0u8; 32]; + + api::get_storage(StorageFlags::empty(), &key, &mut &mut value[..]).unwrap(); + assert!(value[0] == 1u8); +} diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs index 20f8ec3364ee..a8501dad4692 100644 --- a/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs +++ b/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, None, &input, None).unwrap(); } diff --git a/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs b/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs index 54c7c7f3d5e2..3d7702c6537a 100644 --- a/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs +++ b/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs @@ -30,6 +30,7 @@ const ALICE_FALLBACK: [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, &[], None).unwrap(); } } diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 40ad3a3aed17..9c4d817a07de 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -1555,25 +1555,36 @@ mod benchmarks { #[benchmark(pov_mode = Measured)] fn seal_delegate_call() -> Result<(), BenchmarkError> { - let hash = Contract::::with_index(1, WasmModule::dummy(), vec![])?.info()?.code_hash; + let Contract { account_id: address, .. } = + Contract::::with_index(1, WasmModule::dummy(), vec![]).unwrap(); + + let address_bytes = address.encode(); + let address_len = address_bytes.len() as u32; + + let deposit: BalanceOf = (u32::MAX - 100).into(); + let deposit_bytes = Into::::into(deposit).encode(); let mut setup = CallSetup::::default(); + setup.set_storage_deposit_limit(deposit); setup.set_origin(Origin::from_account_id(setup.contract().account_id.clone())); let (mut ext, _) = setup.ext(); let mut runtime = crate::wasm::Runtime::<_, [u8]>::new(&mut ext, vec![]); - let mut memory = memory!(hash.encode(),); + let mut memory = memory!(address_bytes, deposit_bytes,); let result; #[block] { result = runtime.bench_delegate_call( memory.as_mut_slice(), - 0, // flags - 0, // code_hash_ptr - 0, // input_data_ptr - 0, // input_data_len - SENTINEL, // output_ptr + 0, // flags + 0, // address_ptr + 0, // ref_time_limit + 0, // proof_size_limit + address_len, // deposit_ptr + 0, // input_data_ptr + 0, // input_data_len + SENTINEL, // output_ptr 0, ); } diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index fdb45f045bbc..49c08166483e 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -210,7 +210,13 @@ 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) -> Result<(), ExecError>; + fn delegate_call( + &mut self, + gas_limit: Weight, + deposit_limit: U256, + address: H160, + input_data: Vec, + ) -> Result<(), ExecError>; /// Instantiate a contract from the given code. /// @@ -581,18 +587,30 @@ struct Frame { allows_reentry: bool, /// If `true` subsequent calls cannot modify storage. read_only: bool, - /// The caller of the currently executing frame which was spawned by `delegate_call`. - delegate_caller: Option>, + /// The delegate call info of the currently executing frame which was spawned by + /// `delegate_call`. + delegate: Option>, /// The output of the last executed call frame. last_frame_output: ExecReturnValue, } +/// This structure is used to represent the arguments in a delegate call frame in order to +/// distinguish who delegated the call and where it was delegated to. +struct DelegateInfo { + /// The caller of the contract. + pub caller: Origin, + /// The address of the contract the call was delegated to. + pub callee: H160, +} + /// Used in a delegate call frame arguments in order to override the executable and caller. struct DelegatedCall { /// The executable which is run instead of the contracts own `executable`. executable: E, /// The caller of the contract. caller: Origin, + /// The address of the contract the call was delegated to. + callee: H160, } /// Parameter passed in when creating a new `Frame`. @@ -898,8 +916,7 @@ where read_only: bool, origin_is_caller: bool, ) -> Result, E)>, ExecError> { - let (account_id, contract_info, executable, delegate_caller, entry_point) = match frame_args - { + let (account_id, contract_info, executable, delegate, entry_point) = match frame_args { FrameArgs::Call { dest, cached_info, delegated_call } => { let contract = if let Some(contract) = cached_info { contract @@ -914,8 +931,8 @@ where }; let (executable, delegate_caller) = - if let Some(DelegatedCall { executable, caller }) = delegated_call { - (executable, Some(caller)) + if let Some(DelegatedCall { executable, caller, callee }) = delegated_call { + (executable, Some(DelegateInfo { caller, callee })) } else { (E::from_storage(contract.code_hash, gas_meter)?, None) }; @@ -931,8 +948,8 @@ where use sp_runtime::Saturating; address::create1( &deployer, - // the Nonce from the origin has been incremented pre-dispatch, so we need - // to subtract 1 to get the nonce at the time of the call. + // the Nonce from the origin has been incremented pre-dispatch, so we + // need to subtract 1 to get the nonce at the time of the call. if origin_is_caller { account_nonce.saturating_sub(1u32.into()).saturated_into() } else { @@ -956,7 +973,7 @@ where }; let frame = Frame { - delegate_caller, + delegate, value_transferred, contract_info: CachedContract::Cached(contract_info), account_id, @@ -1025,7 +1042,7 @@ where let frame = self.top_frame(); let entry_point = frame.entry_point; let delegated_code_hash = - if frame.delegate_caller.is_some() { Some(*executable.code_hash()) } else { None }; + if frame.delegate.is_some() { Some(*executable.code_hash()) } else { None }; // The output of the caller frame will be replaced by the output of this run. // It is also not accessible from nested frames. @@ -1103,7 +1120,13 @@ where frame.nested_storage.enforce_limit(contract)?; } - let frame = self.top_frame(); + let frame = self.top_frame_mut(); + + // 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)?; + let account_id = T::AddressMapper::to_address(&frame.account_id); match (entry_point, delegated_code_hash) { (ExportedFunction::Constructor, _) => { @@ -1112,15 +1135,7 @@ where return Err(Error::::TerminatedInConstructor.into()); } - // If a special limit was set for the sub-call, we enforce it here. - // This is needed because contract constructor might write to storage. - // The sub-call will be rolled back in case the limit is exhausted. - let frame = self.top_frame_mut(); - let contract = frame.contract_info.as_contract(); - frame.nested_storage.enforce_subcall_limit(contract)?; - let caller = T::AddressMapper::to_address(self.caller().account_id()?); - // Deposit an instantiation event. Contracts::::deposit_event(Event::Instantiated { deployer: caller, @@ -1134,12 +1149,6 @@ where }); }, (ExportedFunction::Call, None) => { - // 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 frame = self.top_frame_mut(); - let contract = frame.contract_info.as_contract(); - frame.nested_storage.enforce_subcall_limit(contract)?; - let caller = self.caller(); Contracts::::deposit_event(Event::Called { caller: caller.clone(), @@ -1468,11 +1477,20 @@ where result } - fn delegate_call(&mut self, code_hash: H256, input_data: Vec) -> Result<(), ExecError> { + fn delegate_call( + &mut self, + gas_limit: Weight, + deposit_limit: U256, + address: H160, + input_data: Vec, + ) -> 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 code_hash = ContractInfoOf::::get(&address) + .ok_or(Error::::CodeNotFound) + .map(|c| c.code_hash)?; let executable = E::from_storage(code_hash, self.gas_meter_mut())?; let top_frame = self.top_frame_mut(); let contract_info = top_frame.contract_info().clone(); @@ -1482,11 +1500,15 @@ where FrameArgs::Call { dest: account_id, cached_info: Some(contract_info), - delegated_call: Some(DelegatedCall { executable, caller: self.caller().clone() }), + delegated_call: Some(DelegatedCall { + executable, + caller: self.caller().clone(), + callee: address, + }), }, value, - Weight::zero(), - BalanceOf::::zero(), + gas_limit, + deposit_limit.try_into().map_err(|_| Error::::BalanceConversionFailed)?, self.is_read_only(), )?; self.run(executable.expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE), input_data) @@ -1601,7 +1623,7 @@ where } fn caller(&self) -> Origin { - if let Some(caller) = &self.top_frame().delegate_caller { + if let Some(DelegateInfo { caller, .. }) = &self.top_frame().delegate { caller.clone() } else { self.frames() @@ -1655,7 +1677,13 @@ where return Err(Error::::InvalidImmutableAccess.into()); } - let address = T::AddressMapper::to_address(self.account_id()); + // Immutable is read from contract code being executed + let address = self + .top_frame() + .delegate + .as_ref() + .map(|d| d.callee) + .unwrap_or(T::AddressMapper::to_address(self.account_id())); Ok(>::get(address).ok_or_else(|| Error::::InvalidImmutableAccess)?) } @@ -1917,7 +1945,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; @@ -2185,18 +2213,20 @@ 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(), U256::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_FALLBACK); 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::::new(GAS_LIMIT), @@ -2204,14 +2234,63 @@ mod tests { value.into(), vec![], None, - ) - .unwrap(); + )); assert_eq!(get_balance(&ALICE), 100 - value); assert_eq!(get_balance(&BOB_FALLBACK), 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(), U256::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::::new(GAS_LIMIT), + &mut storage_meter, + U256::zero(), + vec![], + None, + ), + ExecError { + error: Error::::CodeNotFound.into(), + origin: ErrorOrigin::Callee, + } + ); + + // add missing contract code + place_contract(&CHARLIE, missing_ch); + assert_ok!(MockStack::run_call( + origin, + BOB_ADDR, + &mut GasMeter::::new(GAS_LIMIT), + &mut storage_meter, + U256::zero(), + 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 @@ -4615,7 +4694,12 @@ 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(), + U256::zero(), + H160([0xff; 20]), + Default::default() + ), Err(Error::::CodeNotFound.into()) ); assert_eq!(ctx.ext.last_frame_output(), &Default::default()); @@ -4732,14 +4816,12 @@ mod tests { Ok(vec![2]), ); - // In a delegate call, we should witness the caller immutable data + // Also in a delegate call, we should witness the callee immutable data assert_eq!( - ctx.ext.delegate_call(charlie_ch, Vec::new()).map(|_| ctx - .ext - .last_frame_output() - .data - .clone()), - Ok(vec![1]) + ctx.ext + .delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new()) + .map(|_| ctx.ext.last_frame_output().data.clone()), + Ok(vec![2]) ); exec_success() diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index a35e4d908601..177b8dff706b 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -1127,7 +1127,7 @@ fn deploy_and_call_other_contract() { #[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 _ = ::Currency::set_balance(&ALICE, 1_000_000); @@ -1137,12 +1137,91 @@ fn delegate_call() { 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_weight_limit() { + 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 _ = ::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::::ContractTrapped, + ); + + assert_ok!(builder::call(caller_addr) + .value(1337) + .data((callee_addr, 500_000_000u64, 100_000u64).encode()) + .build()); + }); +} + +#[test] +fn delegate_call_with_deposit_limit() { + let (caller_pvm, _caller_code_hash) = compile_module("delegate_call_deposit_limit").unwrap(); + let (callee_pvm, _callee_code_hash) = compile_module("delegate_call_lib").unwrap(); + + ExtBuilder::default().existential_deposit(500).build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 1_000_000); + + // Instantiate the 'caller' + let Contract { addr: caller_addr, .. } = + builder::bare_instantiate(Code::Upload(caller_pvm)) + .value(300_000) + .build_and_unwrap_contract(); + + // Instantiate the 'callee' + let Contract { addr: callee_addr, .. } = + builder::bare_instantiate(Code::Upload(callee_pvm)) + .value(100_000) + .build_and_unwrap_contract(); + + // 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::::StorageDepositLimitExhausted, + ); + + assert_ok!(builder::call(caller_addr) + .value(1337) + .data((callee_addr, 34u64).encode()) .build()); }); } @@ -3666,6 +3745,12 @@ fn locking_delegate_dependency_works() { .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]); @@ -3675,17 +3760,19 @@ fn locking_delegate_dependency_works() { // 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(ALICE_FALLBACK)) - .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(ALICE_FALLBACK)) - .data(input.encode()) + .data((action, hash2addr(code_hash), code_hash).encode()) .build() }; const ED: u64 = 2000; @@ -3702,7 +3789,7 @@ fn locking_delegate_dependency_works() { // 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(ALICE_FALLBACK), code.clone(), @@ -3710,6 +3797,9 @@ fn locking_delegate_dependency_works() { ) .unwrap(); deposit = deposit_per_code; + // Mock contract info by using first 20 bytes of code_hash as address. + let addr = hash2addr(&code_hash); + ContractInfoOf::::set(&addr, ContractInfo::new(&addr, 0, code_hash).ok()); } // Instantiate should now work. @@ -3746,7 +3836,11 @@ fn locking_delegate_dependency_works() { // Locking self should fail. assert_err!( - call(&addr_caller, &(1u32, self_code_hash)).result, + builder::bare_call(addr_caller) + .origin(RuntimeOrigin::signed(ALICE_FALLBACK)) + .data((1u32, &addr_caller, self_code_hash).encode()) + .build() + .result, Error::::CannotAddSelfAsDelegateDependency ); @@ -3785,7 +3879,7 @@ fn locking_delegate_dependency_works() { 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::::StorageDepositLimitExhausted diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 8310fe701013..3e2c83db1ebd 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -536,16 +536,17 @@ macro_rules! charge_gas { /// The kind of call that should be performed. enum CallType { /// Execute another instantiated contract - Call { callee_ptr: u32, value_ptr: u32, deposit_ptr: u32, weight: Weight }, - /// Execute deployed code in the context (storage, account ID, value) of the caller contract - DelegateCall { code_hash_ptr: u32 }, + Call { value_ptr: u32 }, + /// Execute another contract code in the context (storage, account ID, value) of the caller + /// contract + DelegateCall, } impl CallType { fn cost(&self) -> RuntimeCosts { match self { CallType::Call { .. } => RuntimeCosts::CallBase, - CallType::DelegateCall { .. } => RuntimeCosts::DelegateCallBase, + CallType::DelegateCall => RuntimeCosts::DelegateCallBase, } } } @@ -987,6 +988,9 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { memory: &mut M, flags: CallFlags, call_type: CallType, + callee_ptr: u32, + deposit_ptr: u32, + weight: Weight, input_data_ptr: u32, input_data_len: u32, output_ptr: u32, @@ -994,6 +998,10 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { ) -> Result { self.charge_gas(call_type.cost())?; + let callee = memory.read_h160(callee_ptr)?; + let deposit_limit = + if deposit_ptr == SENTINEL { U256::zero() } else { memory.read_u256(deposit_ptr)? }; + let input_data = if flags.contains(CallFlags::CLONE_INPUT) { let input = self.input_data.as_ref().ok_or(Error::::InputForwarded)?; charge_gas!(self, RuntimeCosts::CallInputCloned(input.len() as u32))?; @@ -1006,13 +1014,7 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { }; let call_outcome = match call_type { - CallType::Call { callee_ptr, value_ptr, deposit_ptr, weight } => { - let callee = memory.read_h160(callee_ptr)?; - let deposit_limit = if deposit_ptr == SENTINEL { - U256::zero() - } else { - memory.read_u256(deposit_ptr)? - }; + CallType::Call { value_ptr } => { let read_only = flags.contains(CallFlags::READ_ONLY); let value = memory.read_u256(value_ptr)?; if value > 0u32.into() { @@ -1033,13 +1035,11 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { read_only, ) }, - CallType::DelegateCall { code_hash_ptr } => { + CallType::DelegateCall => { if flags.intersects(CallFlags::ALLOW_REENTRY | CallFlags::READ_ONLY) { return Err(Error::::InvalidCallFlags.into()); } - - let code_hash = memory.read_h256(code_hash_ptr)?; - self.ext.delegate_call(code_hash, input_data) + self.ext.delegate_call(weight, deposit_limit, callee, input_data) }, }; @@ -1252,12 +1252,10 @@ pub mod env { self.call( memory, CallFlags::from_bits(flags).ok_or(Error::::InvalidCallFlags)?, - CallType::Call { - callee_ptr, - value_ptr, - deposit_ptr, - weight: Weight::from_parts(ref_time_limit, proof_size_limit), - }, + CallType::Call { value_ptr }, + callee_ptr, + deposit_ptr, + Weight::from_parts(ref_time_limit, proof_size_limit), input_data_ptr, input_data_len, output_ptr, @@ -1272,7 +1270,10 @@ pub mod env { &mut self, memory: &mut M, flags: u32, - code_hash_ptr: u32, + address_ptr: u32, + ref_time_limit: u64, + proof_size_limit: u64, + deposit_ptr: u32, input_data_ptr: u32, input_data_len: u32, output_ptr: u32, @@ -1281,7 +1282,10 @@ pub mod env { self.call( memory, CallFlags::from_bits(flags).ok_or(Error::::InvalidCallFlags)?, - CallType::DelegateCall { code_hash_ptr }, + CallType::DelegateCall, + address_ptr, + deposit_ptr, + Weight::from_parts(ref_time_limit, proof_size_limit), input_data_ptr, input_data_len, output_ptr, diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index cb52cf93540b..6b3a8b07f040 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -323,7 +323,13 @@ pub trait HostFn: private::Sealed { /// # Parameters /// /// - `flags`: See [`CallFlags`] for a documentation of the supported flags. - /// - `code_hash`: The hash of the code to be executed. + /// - `address`: The address of the code to be executed. Should be decodable as an + /// `T::AccountId`. Traps otherwise. + /// - `ref_time_limit`: how much *ref_time* Weight to devote to the execution. + /// - `proof_size_limit`: how much *proof_size* Weight to devote to the execution. + /// - `deposit_limit`: The storage deposit limit for delegate call. Passing `None` means setting + /// no specific limit for the call, which implies storage usage up to the limit of the parent + /// call. /// - `input`: The input data buffer used to call the contract. /// - `output`: A reference to the output data buffer to write the call output buffer. If `None` /// is provided then the output buffer is not copied. @@ -338,7 +344,10 @@ pub trait HostFn: private::Sealed { /// - [CodeNotFound][`crate::ReturnErrorCode::CodeNotFound] fn delegate_call( flags: CallFlags, - code_hash: &[u8; 32], + address: &[u8; 20], + ref_time_limit: u64, + proof_size_limit: u64, + deposit_limit: Option<&[u8; 32]>, input_data: &[u8], output: Option<&mut &mut [u8]>, ) -> Result; diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index 199a0abc3ddc..e8b27057ed18 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/substrate/frame/revive/uapi/src/host/riscv32.rs @@ -59,14 +59,7 @@ mod sys { out_len_ptr: *mut u32, ) -> ReturnCode; pub fn call(ptr: *const u8) -> ReturnCode; - pub fn delegate_call( - flags: u32, - code_hash_ptr: *const u8, - input_data_ptr: *const u8, - input_data_len: u32, - out_ptr: *mut u8, - out_len_ptr: *mut u32, - ) -> ReturnCode; + pub fn delegate_call(ptr: *const u8) -> ReturnCode; pub fn instantiate(ptr: *const u8) -> ReturnCode; pub fn terminate(beneficiary_ptr: *const u8); pub fn input(out_ptr: *mut u8, out_len_ptr: *mut u32); @@ -306,24 +299,42 @@ impl HostFn for HostFnImpl { fn delegate_call( flags: CallFlags, - code_hash: &[u8; 32], + address: &[u8; 20], + ref_time_limit: u64, + proof_size_limit: u64, + deposit_limit: Option<&[u8; 32]>, input: &[u8], mut output: Option<&mut &mut [u8]>, ) -> Result { let (output_ptr, mut output_len) = ptr_len_or_sentinel(&mut output); - let ret_code = { - unsafe { - sys::delegate_call( - flags.bits(), - code_hash.as_ptr(), - input.as_ptr(), - input.len() as u32, - output_ptr, - &mut output_len, - ) - } + let deposit_limit_ptr = ptr_or_sentinel(&deposit_limit); + #[repr(packed)] + #[allow(dead_code)] + struct Args { + flags: u32, + address: *const u8, + ref_time_limit: u64, + proof_size_limit: u64, + deposit_limit: *const u8, + input: *const u8, + input_len: u32, + output: *mut u8, + output_len: *mut u32, + } + let args = Args { + flags: flags.bits(), + address: address.as_ptr(), + ref_time_limit, + proof_size_limit, + deposit_limit: deposit_limit_ptr, + input: input.as_ptr(), + input_len: input.len() as _, + output: output_ptr, + output_len: &mut output_len as *mut _, }; + let ret_code = { unsafe { sys::delegate_call(&args as *const Args as *const _) } }; + if let Some(ref mut output) = output { extract_from_slice(output, output_len as usize); }