diff --git a/prdoc/pr_9455.prdoc b/prdoc/pr_9455.prdoc new file mode 100644 index 0000000000000..198445575b0e0 --- /dev/null +++ b/prdoc/pr_9455.prdoc @@ -0,0 +1,12 @@ +title: '[pallet-revive] Migrate `to_account_id` from host function to pre-compile' +doc: +- audience: Runtime Dev + description: Moves the unstable host function `to_account_id` to the + `System` pre-compile. +crates: +- name: pallet-revive + bump: patch +- name: pallet-revive-fixtures + bump: patch +- name: pallet-revive-uapi + bump: major diff --git a/substrate/frame/revive/fixtures/contracts/to_account_id.rs b/substrate/frame/revive/fixtures/contracts/to_account_id.rs deleted file mode 100644 index a0b359c628577..0000000000000 --- a/substrate/frame/revive/fixtures/contracts/to_account_id.rs +++ /dev/null @@ -1,40 +0,0 @@ -// 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] -include!("../panic_handler.rs"); - -use uapi::{input, 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.rs b/substrate/frame/revive/src/benchmarking.rs index f7cec55c28c7b..7e50932d66f4e 100644 --- a/substrate/frame/revive/src/benchmarking.rs +++ b/substrate/frame/revive/src/benchmarking.rs @@ -23,11 +23,14 @@ use crate::{ evm::runtime::GAS_PRICE, exec::{Key, MomentOf, PrecompileExt}, limits, - precompiles::{self, run::builtin as run_builtin_precompile}, + precompiles::{ + self, run::builtin as run_builtin_precompile, BenchmarkSystem, BuiltinPrecompile, ISystem, + }, storage::WriteOutcome, Pallet as Contracts, *, }; use alloc::{vec, vec::Vec}; +use alloy_core::sol_types::SolInterface; use codec::{Encode, MaxEncodedLen}; use frame_benchmarking::v2::*; use frame_support::{ @@ -552,35 +555,40 @@ mod benchmarks { } #[benchmark(pov_mode = Measured)] - fn seal_to_account_id() { + fn to_account_id() { // use a mapped address for the benchmark, to ensure that we bench the worst // case (and not the fallback case). + let account_id = account("precompile_to_account_id", 0, 0); let address = { - let caller = account("seal_to_account_id", 0, 0); - T::Currency::set_balance(&caller, caller_funding::()); - T::AddressMapper::map(&caller).unwrap(); - T::AddressMapper::to_address(&caller) + T::Currency::set_balance(&account_id, caller_funding::()); + T::AddressMapper::map(&account_id).unwrap(); + T::AddressMapper::to_address(&account_id) }; - let len = ::max_encoded_len(); - build_runtime!(runtime, memory: [vec![0u8; len], address.0, ]); + let input_bytes = ISystem::ISystemCalls::toAccountId(ISystem::toAccountIdCall { + input: address.0.into(), + }) + .abi_encode(); + + let mut call_setup = CallSetup::::default(); + let (mut ext, _) = call_setup.ext(); let result; #[block] { - result = runtime.bench_to_account_id(memory.as_mut_slice(), len as u32, 0); + result = run_builtin_precompile( + &mut ext, + H160(BenchmarkSystem::::MATCHER.base_address()).as_fixed_bytes(), + input_bytes, + ); } - - assert_ok!(result); + let data = result.unwrap().data; assert_ne!( - memory.as_slice()[20..32], + data.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)) - ); + assert_eq!(T::AccountId::decode(&mut data.as_slice()), Ok(account_id),); } #[benchmark(pov_mode = Measured)] @@ -1973,9 +1981,6 @@ mod benchmarks { // `n`: Input to hash in bytes #[benchmark(pov_mode = Measured)] fn hash_blake2_256(n: Linear<0, { limits::code::BLOB_BYTES }>) { - use crate::precompiles::{BenchmarkSystem, BuiltinPrecompile, ISystem}; - use alloy_core::sol_types::SolInterface; - let input = vec![0u8; n as usize]; let input_bytes = ISystem::ISystemCalls::hashBlake256(ISystem::hashBlake256Call { input: input.clone().into(), diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 3fca74ad82958..a191584ee4807 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -323,9 +323,6 @@ pub trait PrecompileExt: sealing::Sealed { /// Return the origin of the whole call stack. fn origin(&self) -> &Origin; - /// 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; @@ -1920,10 +1917,6 @@ where &self.origin } - fn to_account_id(&self, address: &H160) -> T::AccountId { - T::AddressMapper::to_account_id(address) - } - fn code_hash(&self, address: &H160) -> H256 { if let Some(code) = >::code(address.as_fixed_bytes()) { return sp_io::hashing::keccak_256(code).into() diff --git a/substrate/frame/revive/src/exec/tests.rs b/substrate/frame/revive/src/exec/tests.rs index eacf1353e8b32..381abc7c26117 100644 --- a/substrate/frame/revive/src/exec/tests.rs +++ b/substrate/frame/revive/src/exec/tests.rs @@ -793,40 +793,6 @@ fn origin_returns_proper_values() { assert_eq!(WitnessedCallerCharlie::get(), Some(ALICE_ADDR)); } -#[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(0); - let result = MockStack::run_call( - origin, - BOB_ADDR, - &mut GasMeter::::new(GAS_LIMIT), - &mut storage_meter, - U256::zero(), - vec![0], - false, - ); - 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/precompiles/builtin/system.rs b/substrate/frame/revive/src/precompiles/builtin/system.rs index 668e84a6f5e00..0766be69433fe 100644 --- a/substrate/frame/revive/src/precompiles/builtin/system.rs +++ b/substrate/frame/revive/src/precompiles/builtin/system.rs @@ -31,6 +31,16 @@ sol! { interface ISystem { /// Computes the BLAKE2 256-bit hash on the given input. function hashBlake256(bytes memory input) external pure returns (bytes32 digest); + /// Retrieve the account id for a specified `H160` address. + /// + /// Calling this function on a native `H160` chain (`type AccountId = H160`) + /// does not make sense, as it would just return the `address` that it was + /// called with. + /// + /// # Note + /// + /// If no mapping exists for `addr`, the fallback account id will be returned. + function toAccountId(address input) external view returns (bytes memory account_id); } } @@ -53,17 +63,95 @@ impl BuiltinPrecompile for System { let output = sp_io::hashing::blake2_256(input.as_bytes_ref()); Ok(output.to_vec()) }, + ISystemCalls::toAccountId(ISystem::toAccountIdCall { input }) => { + use crate::address::AddressMapper; + use codec::Encode; + env.gas_meter_mut().charge(RuntimeCosts::ToAccountId)?; + let account_id = + T::AddressMapper::to_account_id(&crate::H160::from_slice(input.as_slice())); + Ok(account_id.encode()) + }, } } } #[cfg(test)] mod tests { - use super::*; - use crate::{precompiles::tests::run_test_vectors, tests::Test}; + use super::{ISystem, *}; + use crate::{ + address::AddressMapper, + call_builder::{caller_funding, CallSetup}, + pallet, + precompiles::{tests::run_test_vectors, BuiltinPrecompile}, + tests::{ExtBuilder, Test}, + H160, + }; + use codec::Decode; + use frame_support::traits::fungible::Mutate; #[test] fn test_system_precompile() { run_test_vectors::>(include_str!("testdata/900-blake2_256.json")); + run_test_vectors::>(include_str!("testdata/900-to_account_id.json")); + } + + #[test] + fn test_system_precompile_unmapped_account() { + ExtBuilder::default().build().execute_with(|| { + // given + let mut call_setup = CallSetup::::default(); + let (mut ext, _) = call_setup.ext(); + let unmapped_address = H160::zero(); + + // when + let input = ISystem::ISystemCalls::toAccountId(ISystem::toAccountIdCall { + input: unmapped_address.0.into(), + }); + let expected_fallback_account_id = + >::call(&>::MATCHER.base_address(), &input, &mut ext) + .unwrap(); + + // then + assert_eq!( + expected_fallback_account_id[20..32], + [0xEE; 12], + "no fallback suffix found where one should be" + ); + }) + } + + #[test] + fn test_system_precompile_mapped_account() { + use crate::test_utils::EVE; + ExtBuilder::default().build().execute_with(|| { + // given + let mapped_address = { + ::Currency::set_balance(&EVE, caller_funding::()); + let _ = ::AddressMapper::map(&EVE); + ::AddressMapper::to_address(&EVE) + }; + + let mut call_setup = CallSetup::::default(); + let (mut ext, _) = call_setup.ext(); + + // when + let input = ISystem::ISystemCalls::toAccountId(ISystem::toAccountIdCall { + input: mapped_address.0.into(), + }); + let data = + >::call(&>::MATCHER.base_address(), &input, &mut ext) + .unwrap(); + + // then + assert_ne!( + data.as_slice()[20..32], + [0xEE; 12], + "fallback suffix found where none should be" + ); + assert_eq!( + ::AccountId::decode(&mut data.as_slice()), + Ok(EVE), + ); + }) } } diff --git a/substrate/frame/revive/src/precompiles/builtin/testdata/900-to_account_id.json b/substrate/frame/revive/src/precompiles/builtin/testdata/900-to_account_id.json new file mode 100644 index 0000000000000..b88909e923b33 --- /dev/null +++ b/substrate/frame/revive/src/precompiles/builtin/testdata/900-to_account_id.json @@ -0,0 +1,9 @@ +[ + { + "Input": "cf5231cc00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000", + "Expected": "0000000000000000000000000000000000000020eeeeeeeeeeeeeeeeeeeeeeee", + "Name": "vector 1", + "Gas": 8000000, + "NoBenchmark": false + } +] diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index de94596ed7e09..c4e82fac81299 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -4005,43 +4005,6 @@ 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() { use crate::precompiles::{Precompile, EVM_REVERT}; diff --git a/substrate/frame/revive/src/vm/runtime.rs b/substrate/frame/revive/src/vm/runtime.rs index 3ee2ebf852bc5..c97ba43cd1587 100644 --- a/substrate/frame/revive/src/vm/runtime.rs +++ b/substrate/frame/revive/src/vm/runtime.rs @@ -2095,25 +2095,4 @@ pub mod env { already_charged, )?) } - - /// 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 = self.ext.to_account_id(&address); - Ok(self.write_fixed_sandbox_output( - memory, - out_ptr, - &account_id.encode(), - false, - already_charged, - )?) - } } diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index f87fb1a54b887..c4c790e439619 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -433,19 +433,6 @@ pub trait HostFn: private::Sealed { /// - `output`: A reference to the output data buffer to write the block number. fn block_number(output: &mut [u8; 32]); - /// 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. - #[unstable_hostfn] - fn to_account_id(addr: &[u8; 20], output: &mut [u8]); - /// Stores the block hash of the given block number into the supplied buffer. /// /// # Parameters diff --git a/substrate/frame/revive/uapi/src/host/riscv64.rs b/substrate/frame/revive/uapi/src/host/riscv64.rs index adfd8f2789594..0ee14807d79e5 100644 --- a/substrate/frame/revive/uapi/src/host/riscv64.rs +++ b/substrate/frame/revive/uapi/src/host/riscv64.rs @@ -95,7 +95,6 @@ mod sys { pub fn seal_return(flags: u32, data_ptr: *const u8, data_len: u32); pub fn caller(out_ptr: *mut u8); pub fn origin(out_ptr: *mut u8); - 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); @@ -447,11 +446,6 @@ 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()) };