From fbd9c7839698157556ffd84bb28054869a032536 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 8 Jan 2025 12:54:43 +0100 Subject: [PATCH 1/6] Add host function `to_account_id` --- prdoc/pr_7091.prdoc | 12 ++++++ .../fixtures/contracts/to_account_id.rs | 40 +++++++++++++++++++ .../frame/revive/src/benchmarking/mod.rs | 18 +++++++++ substrate/frame/revive/src/exec.rs | 39 ++++++++++++++++++ substrate/frame/revive/src/tests.rs | 32 +++++++++++++++ substrate/frame/revive/src/wasm/runtime.rs | 19 +++++++++ substrate/frame/revive/src/weights.rs | 21 ++++++++++ substrate/frame/revive/uapi/src/host.rs | 12 ++++++ .../frame/revive/uapi/src/host/riscv64.rs | 5 +++ 9 files changed, 198 insertions(+) create mode 100644 prdoc/pr_7091.prdoc create mode 100644 substrate/frame/revive/fixtures/contracts/to_account_id.rs diff --git a/prdoc/pr_7091.prdoc b/prdoc/pr_7091.prdoc new file mode 100644 index 000000000000..adb6a0055ab8 --- /dev/null +++ b/prdoc/pr_7091.prdoc @@ -0,0 +1,12 @@ +title: '[pallet-revive] Add new host function `to_account_id` +doc: +- audience: Runtime Dev + description: A new host function `to_account_id` is added. It allows retrieving + the account id for a `H160` address. +crates: +- name: pallet-revive-fixtures + bump: minor +- name: pallet-revive + bump: minor +- name: pallet-revive-uapi + bump: minor diff --git a/substrate/frame/revive/fixtures/contracts/to_account_id.rs b/substrate/frame/revive/fixtures/contracts/to_account_id.rs new file mode 100644 index 000000000000..c2a8fce3ec99 --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/to_account_id.rs @@ -0,0 +1,40 @@ +// 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; +use uapi::{HostFn, HostFnImpl as api}; + +#[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], + expected_account_id: &[u8; 32], + ); + + let mut account_id = [0u8; 32]; + api::to_account_id(address, &mut account_id); + + assert!(&account_id == expected_account_id); +} diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index e67c39ec0899..f5657f4fe49b 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -558,6 +558,24 @@ mod benchmarks { assert_eq!(result.unwrap(), 1); } + #[benchmark(pov_mode = Measured)] + fn seal_to_account_id() { + let len = ::max_encoded_len(); + let account = account::("target", 0, 0); + let address = T::AddressMapper::to_address(&account); + + build_runtime!(runtime, memory: [vec![0u8; len], address.0, ]); + + let result; + #[block] + { + result = runtime.bench_to_account_id(memory.as_mut_slice(), len as u32, 0); + } + + assert_ok!(result); + assert_eq!(T::AccountId::decode(&mut memory.as_slice()), Ok(runtime.ext().to_account_id(&address))); + } + #[benchmark(pov_mode = Measured)] fn seal_code_hash() { let contract = Contract::::with_index(1, WasmModule::dummy(), vec![]).unwrap(); diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index a6a259149768..35e0e57c5122 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -293,6 +293,9 @@ pub trait Ext: sealing::Sealed { /// Check if a contract lives at the specified `address`. fn is_contract(&self, address: &H160) -> bool; + /// Returns the account id for the given `address`. + fn to_account_id(&self, address: &H160) -> AccountIdOf; + /// Returns the code hash of the contract for the given `address`. /// If not a contract but account exists then `keccak_256([])` is returned, otherwise `zero`. fn code_hash(&self, address: &H160) -> H256; @@ -1652,6 +1655,10 @@ where ContractInfoOf::::contains_key(&address) } + fn to_account_id(&self, address: &H160) -> T::AccountId { + T::AddressMapper::to_account_id(address) + } + fn code_hash(&self, address: &H160) -> H256 { >::get(&address) .map(|contract| contract.code_hash) @@ -2703,6 +2710,38 @@ mod tests { }); } + #[test] + fn to_account_id_returns_proper_values() { + let bob_code_hash = MockLoader::insert(Call, |ctx, _| { + let alice_account_id = ::AddressMapper::to_account_id(&ALICE_ADDR); + assert_eq!(ctx.ext.to_account_id(&ALICE_ADDR), alice_account_id); + + const UNMAPPED_ADDR: H160 = H160([99u8; 20]); + let mut unmapped_fallback_account_id = [0xEE; 32]; + unmapped_fallback_account_id[..20].copy_from_slice(UNMAPPED_ADDR.as_bytes()); + assert_eq!(ctx.ext.to_account_id(&UNMAPPED_ADDR), AccountId32::new(unmapped_fallback_account_id)); + + exec_success() + }); + + ExtBuilder::default().build().execute_with(|| { + place_contract(&BOB, bob_code_hash); + let origin = Origin::from_account_id(ALICE); + let mut storage_meter = storage::meter::Meter::new(&origin, 0, 0).unwrap(); + let result = MockStack::run_call( + origin, + BOB_ADDR, + &mut GasMeter::::new(GAS_LIMIT), + &mut storage_meter, + U256::zero(), + vec![0], + false, + None, + ); + assert_matches!(result, Ok(_)); + }); + } + #[test] fn code_hash_returns_proper_values() { let bob_code_hash = MockLoader::insert(Call, |ctx, _| { diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 664578bf7672..1e4a184adfd5 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -4664,6 +4664,38 @@ fn origin_api_works() { }); } +#[test] +fn to_account_id_works() { + let (code_hash_code, _) = compile_module("to_account_id").unwrap(); + + ExtBuilder::default().existential_deposit(1).build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 1_000_000); + let _ = ::Currency::set_balance(&EVE, 1_000_000); + + let Contract { addr, .. } = + builder::bare_instantiate(Code::Upload(code_hash_code)).build_and_unwrap_contract(); + + // mapped account + >::map_account(RuntimeOrigin::signed(EVE)).unwrap(); + let expected_mapped_account_id = &::AddressMapper::to_account_id(&EVE_ADDR); + assert_ne!( + expected_mapped_account_id.encode()[20..32], + [0xEE; 12], + "fallback suffix found where none should be" + ); + assert_ok!(builder::call(addr).data((EVE_ADDR, expected_mapped_account_id).encode()).build()); + + // fallback for unmapped accounts + let expected_fallback_account_id = &::AddressMapper::to_account_id(&BOB_ADDR); + assert_eq!( + expected_fallback_account_id.encode()[20..32], + [0xEE; 12], + "no fallback suffix found where one should be" + ); + assert_ok!(builder::call(addr).data((BOB_ADDR, expected_fallback_account_id).encode()).build()); + }); +} + #[test] fn code_hash_works() { let (code_hash_code, self_code_hash) = compile_module("code_hash").unwrap(); diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 52f79f2eb55a..8549037973fb 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -293,6 +293,8 @@ pub enum RuntimeCosts { CallDataSize, /// Weight of calling `seal_return_data_size`. ReturnDataSize, + /// Weight of calling `seal_to_account_id`. + ToAccountId, /// Weight of calling `seal_origin`. Origin, /// Weight of calling `seal_is_contract`. @@ -468,6 +470,7 @@ impl Token for RuntimeCosts { Caller => T::WeightInfo::seal_caller(), Origin => T::WeightInfo::seal_origin(), IsContract => T::WeightInfo::seal_is_contract(), + ToAccountId => T::WeightInfo::seal_to_account_id(), CodeHash => T::WeightInfo::seal_code_hash(), CodeSize => T::WeightInfo::seal_code_size(), OwnCodeHash => T::WeightInfo::seal_own_code_hash(), @@ -1396,6 +1399,22 @@ pub mod env { )?) } + /// Retrieves the account id for a specified contract address. + /// + /// See [`pallet_revive_uapi::HostFn::to_account_id`]. + fn to_account_id(&mut self, memory: &mut M, addr_ptr: u32, out_ptr: u32) -> Result<(), TrapReason> { + self.charge_gas(RuntimeCosts::ToAccountId)?; + let address = memory.read_h160(addr_ptr)?; + let account_id = ::AddressMapper::to_account_id(&address); + Ok(self.write_fixed_sandbox_output( + memory, + out_ptr, + &account_id.encode(), + false, + already_charged, + )?) + } + /// Stores the address of the call stack origin into the supplied buffer. /// See [`pallet_revive_uapi::HostFn::origin`]. #[stable] diff --git a/substrate/frame/revive/src/weights.rs b/substrate/frame/revive/src/weights.rs index e35ba5ca0766..250e81b46d72 100644 --- a/substrate/frame/revive/src/weights.rs +++ b/substrate/frame/revive/src/weights.rs @@ -67,6 +67,7 @@ pub trait WeightInfo { fn seal_caller() -> Weight; fn seal_origin() -> Weight; fn seal_is_contract() -> Weight; + fn seal_to_account_id() -> Weight; fn seal_code_hash() -> Weight; fn seal_own_code_hash() -> Weight; fn seal_code_size() -> Weight; @@ -378,6 +379,16 @@ impl WeightInfo for SubstrateWeight { Weight::from_parts(10_336_000, 3771) .saturating_add(T::DbWeight::get().reads(1_u64)) } + /// Storage: `Revive::AddressSuffix` (r:1 w:0) + /// Proof: `Revive::AddressSuffix` (`max_values`: None, `max_size`: Some(32), added: 2507, mode: `Measured`) + fn seal_to_account_id() -> Weight { + // Proof Size summary in bytes: + // Measured: `212` + // Estimated: `3677` + // Minimum execution time: 4_000_000 picoseconds. + Weight::from_parts(4_000_000, 3677) + .saturating_add(T::DbWeight::get().reads(1_u64)) + } /// Storage: `Revive::ContractInfoOf` (r:1 w:0) /// Proof: `Revive::ContractInfoOf` (`max_values`: None, `max_size`: Some(1779), added: 4254, mode: `Measured`) fn seal_code_hash() -> Weight { @@ -1274,6 +1285,16 @@ impl WeightInfo for () { Weight::from_parts(10_336_000, 3771) .saturating_add(RocksDbWeight::get().reads(1_u64)) } + /// Storage: `Revive::AddressSuffix` (r:1 w:0) + /// Proof: `Revive::AddressSuffix` (`max_values`: None, `max_size`: Some(32), added: 2507, mode: `Measured`) + fn seal_to_account_id() -> Weight { + // Proof Size summary in bytes: + // Measured: `212` + // Estimated: `3677` + // Minimum execution time: 4_000_000 picoseconds. + Weight::from_parts(4_000_000, 3677) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + } /// Storage: `Revive::ContractInfoOf` (r:1 w:0) /// Proof: `Revive::ContractInfoOf` (`max_values`: None, `max_size`: Some(1779), added: 4254, mode: `Measured`) fn seal_code_hash() -> Weight { diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index eced4843b552..541b9dec802a 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -144,6 +144,18 @@ pub trait HostFn: private::Sealed { /// - `output`: A reference to the output data buffer to write the origin's address. fn origin(output: &mut [u8; 20]); + /// Retrieve the account id for a specified address. + /// + /// # Parameters + /// + /// - `addr`: A `H160` address. + /// - `output`: A reference to the output data buffer to write the account id. + /// + /// # Note + /// + /// If no mapping exists for `addr`, the fallback account id will be returned. + fn to_account_id(addr: &[u8; 20], output: &mut [u8]); + /// Retrieve the code hash for a specified contract address. /// /// # Parameters diff --git a/substrate/frame/revive/uapi/src/host/riscv64.rs b/substrate/frame/revive/uapi/src/host/riscv64.rs index 6fdda86892d5..1a90839dcaa8 100644 --- a/substrate/frame/revive/uapi/src/host/riscv64.rs +++ b/substrate/frame/revive/uapi/src/host/riscv64.rs @@ -69,6 +69,7 @@ mod sys { pub fn caller(out_ptr: *mut u8); pub fn origin(out_ptr: *mut u8); pub fn is_contract(account_ptr: *const u8) -> ReturnCode; + pub fn to_account_id(address_ptr: *const u8, out_ptr: *mut u8); pub fn code_hash(address_ptr: *const u8, out_ptr: *mut u8); pub fn code_size(address_ptr: *const u8) -> u64; pub fn own_code_hash(out_ptr: *mut u8); @@ -433,6 +434,10 @@ impl HostFn for HostFnImpl { unsafe { sys::balance_of(address.as_ptr(), output.as_mut_ptr()) }; } + fn to_account_id(address: &[u8; 20], output: &mut [u8]) { + unsafe { sys::to_account_id(address.as_ptr(), output.as_mut_ptr()) } + } + fn code_hash(address: &[u8; 20], output: &mut [u8; 32]) { unsafe { sys::code_hash(address.as_ptr(), output.as_mut_ptr()) } } From dd980f72b5a9b3701d6c3a2c8d68a0eba58acccd Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 8 Jan 2025 15:44:52 +0100 Subject: [PATCH 2/6] Fix `prdoc` syntax --- prdoc/pr_7091.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_7091.prdoc b/prdoc/pr_7091.prdoc index adb6a0055ab8..badea4e82fdb 100644 --- a/prdoc/pr_7091.prdoc +++ b/prdoc/pr_7091.prdoc @@ -1,4 +1,4 @@ -title: '[pallet-revive] Add new host function `to_account_id` +title: '[pallet-revive] Add new host function `to_account_id`' doc: - audience: Runtime Dev description: A new host function `to_account_id` is added. It allows retrieving From bde4b5547319867be8a90fed04fa4c1a206f8ffb Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 8 Jan 2025 17:50:40 +0100 Subject: [PATCH 3/6] Mark `to_account_id` unstable + move it to unstable section --- substrate/frame/revive/uapi/src/host/riscv64.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/substrate/frame/revive/uapi/src/host/riscv64.rs b/substrate/frame/revive/uapi/src/host/riscv64.rs index 1a90839dcaa8..02e7a2013cf1 100644 --- a/substrate/frame/revive/uapi/src/host/riscv64.rs +++ b/substrate/frame/revive/uapi/src/host/riscv64.rs @@ -434,10 +434,6 @@ impl HostFn for HostFnImpl { unsafe { sys::balance_of(address.as_ptr(), output.as_mut_ptr()) }; } - fn to_account_id(address: &[u8; 20], output: &mut [u8]) { - unsafe { sys::to_account_id(address.as_ptr(), output.as_mut_ptr()) } - } - fn code_hash(address: &[u8; 20], output: &mut [u8; 32]) { unsafe { sys::code_hash(address.as_ptr(), output.as_mut_ptr()) } } @@ -462,6 +458,11 @@ impl HostFn for HostFnImpl { unsafe { sys::ref_time_left() } } + #[unstable_hostfn] + fn to_account_id(address: &[u8; 20], output: &mut [u8]) { + unsafe { sys::to_account_id(address.as_ptr(), output.as_mut_ptr()) } + } + #[unstable_hostfn] fn block_hash(block_number_ptr: &[u8; 32], output: &mut [u8; 32]) { unsafe { sys::block_hash(block_number_ptr.as_ptr(), output.as_mut_ptr()) }; From 84ac7679a1c2f53bb970b90bb8781608b500f1ce Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 8 Jan 2025 17:53:43 +0100 Subject: [PATCH 4/6] Use `to_account_id` from `exec.rs` instead --- substrate/frame/revive/src/wasm/runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 8549037973fb..24a0d95b4af1 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -1405,7 +1405,7 @@ pub mod env { fn to_account_id(&mut self, memory: &mut M, addr_ptr: u32, out_ptr: u32) -> Result<(), TrapReason> { self.charge_gas(RuntimeCosts::ToAccountId)?; let address = memory.read_h160(addr_ptr)?; - let account_id = ::AddressMapper::to_account_id(&address); + let account_id = self.ext.to_account_id(&address); Ok(self.write_fixed_sandbox_output( memory, out_ptr, From 304d1a95bec24a3d6c6c2d5f08beb58bd649ee85 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 8 Jan 2025 18:43:29 +0100 Subject: [PATCH 5/6] Benchmark worst case of `to_account_id` --- substrate/frame/revive/src/benchmarking/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index f5657f4fe49b..d1e469b72d4d 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -560,10 +560,14 @@ mod benchmarks { #[benchmark(pov_mode = Measured)] fn seal_to_account_id() { - let len = ::max_encoded_len(); - let account = account::("target", 0, 0); - let address = T::AddressMapper::to_address(&account); + // use a mapped address for the benchmark, to ensure that we bench the worst + // case (and not the fallback case). + let caller = whitelisted_caller(); + let origin: T::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); + Contracts::::map_account(origin.clone()).unwrap(); + let address = T::AddressMapper::to_address(&caller); + let len = ::max_encoded_len(); build_runtime!(runtime, memory: [vec![0u8; len], address.0, ]); let result; From b901e4334871fc2d2b20941f4f3495de3dd61530 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 8 Jan 2025 18:50:20 +0100 Subject: [PATCH 6/6] Asser that worst case is benchmarked --- substrate/frame/revive/src/benchmarking/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index d1e469b72d4d..6e7f69c56e48 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -577,6 +577,11 @@ mod benchmarks { } assert_ok!(result); + assert_ne!( + memory.as_slice()[20..32], + [0xEE; 12], + "fallback suffix found where none should be" + ); assert_eq!(T::AccountId::decode(&mut memory.as_slice()), Ok(runtime.ext().to_account_id(&address))); }