Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pallet-revive] implement the block hash API #6246

Merged
merged 12 commits into from
Oct 30, 2024
13 changes: 13 additions & 0 deletions prdoc/pr_6246.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
title: '[pallet-revive] implement the block hash API'
doc:
- audience: Runtime Dev
description: |-
- Bound T::Hash to H256
- Implement the block hash API
crates:
- name: pallet-revive
bump: major
- name: pallet-revive-fixtures
bump: major
- name: pallet-revive-uapi
bump: major
37 changes: 37 additions & 0 deletions substrate/frame/revive/fixtures/contracts/block_hash.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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!(block_number: &[u8; 32], block_hash: &[u8; 32],);

let mut buf = [0; 32];
api::block_hash(block_number, &mut &mut buf);

assert_eq!(&buf[..], block_hash);
}
4 changes: 3 additions & 1 deletion substrate/frame/revive/src/benchmarking/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
};
use alloc::{vec, vec::Vec};
use frame_benchmarking::benchmarking;
use sp_core::U256;
use sp_core::{H256, U256};

type StackExt<'a, T> = Stack<'a, T, WasmBlob<T>>;

Expand All @@ -48,6 +48,7 @@ where
T: Config + pallet_balances::Config,
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
Copy link
Contributor

@pgherveou pgherveou Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't T::Hash: Into<H256> suffice for all these added constraints?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically yes. But I think we are splitting hairs here :) I don't see where a runtime would fullfill T::Hash: Into<H256> but T::Hash isn't actually H256.

{
fn default() -> Self {
Self::new(WasmModule::dummy())
Expand All @@ -59,6 +60,7 @@ where
T: Config + pallet_balances::Config,
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
{
/// Setup a new call for the given module.
pub fn new(module: WasmModule) -> Self {
Expand Down
27 changes: 27 additions & 0 deletions substrate/frame/revive/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ where
T: Config + pallet_balances::Config,
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
{
/// Create new contract and use a default account id as instantiator.
fn new(module: WasmModule, data: Vec<u8>) -> Result<Contract<T>, &'static str> {
Expand Down Expand Up @@ -224,6 +225,7 @@ fn default_deposit_limit<T: Config>() -> BalanceOf<T> {
<T as frame_system::Config>::RuntimeEvent: From<pallet::Event<T>>,
<T as Config>::RuntimeCall: From<frame_system::Call<T>>,
<pallet_balances::Pallet<T> as Currency<T::AccountId>>::Balance: From<BalanceOf<T>>,
<T as frame_system::Config>::Hash: frame_support::traits::IsType<H256>,
)]
mod benchmarks {
use super::*;
Expand Down Expand Up @@ -747,6 +749,31 @@ mod benchmarks {
assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().block_number());
}

#[benchmark(pov_mode = Measured)]
fn seal_block_hash() {
let mut memory = vec![0u8; 64];
let mut setup = CallSetup::<T>::default();
let input = setup.data();
let (mut ext, _) = setup.ext();
ext.set_block_number(BlockNumberFor::<T>::from(1u32));

let mut runtime = crate::wasm::Runtime::<_, [u8]>::new(&mut ext, input);

let block_hash = H256::from([1; 32]);
frame_system::BlockHash::<T>::insert(
&BlockNumberFor::<T>::from(0u32),
T::Hash::from(block_hash),
);

let result;
#[block]
{
result = runtime.bench_block_hash(memory.as_mut_slice(), 32, 0);
}
assert_ok!(result);
assert_eq!(&memory[..32], &block_hash.0);
}

#[benchmark(pov_mode = Measured)]
fn seal_now() {
build_runtime!(runtime, memory: [[0u8;32], ]);
Expand Down
4 changes: 3 additions & 1 deletion substrate/frame/revive/src/evm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use frame_support::{
use pallet_transaction_payment::OnChargeTransaction;
use scale_info::TypeInfo;
use sp_arithmetic::Percent;
use sp_core::{Get, U256};
use sp_core::{Get, H256, U256};
use sp_runtime::{
generic::{self, CheckedExtrinsic, ExtrinsicFormat},
traits::{
Expand Down Expand Up @@ -109,6 +109,7 @@ where
BalanceOf<E::Config>: Into<U256> + TryFrom<U256>,
MomentOf<E::Config>: Into<U256>,
CallOf<E::Config>: From<crate::Call<E::Config>> + TryInto<crate::Call<E::Config>>,
<E::Config as frame_system::Config>::Hash: frame_support::traits::IsType<H256>,

// required by Checkable for `generic::UncheckedExtrinsic`
LookupSource: Member + MaybeDisplay,
Expand Down Expand Up @@ -278,6 +279,7 @@ pub trait EthExtra {
<Self::Config as frame_system::Config>::RuntimeCall: Dispatchable<Info = DispatchInfo>,
OnChargeTransactionBalanceOf<Self::Config>: Into<BalanceOf<Self::Config>>,
CallOf<Self::Config>: From<crate::Call<Self::Config>>,
<Self::Config as frame_system::Config>::Hash: frame_support::traits::IsType<H256>,
{
let tx = rlp::decode::<TransactionLegacySigned>(&payload).map_err(|err| {
log::debug!(target: LOG_TARGET, "Failed to decode transaction: {err:?}");
Expand Down
86 changes: 85 additions & 1 deletion substrate/frame/revive/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use sp_core::{
};
use sp_io::{crypto::secp256k1_ecdsa_recover_compressed, hashing::blake2_256};
use sp_runtime::{
traits::{BadOrigin, Convert, Dispatchable, Zero},
traits::{BadOrigin, Convert, Dispatchable, Saturating, Zero},
DispatchError, SaturatedConversion,
};

Expand Down Expand Up @@ -350,6 +350,10 @@ pub trait Ext: sealing::Sealed {
/// Returns the current block number.
fn block_number(&self) -> U256;

/// Returns the block hash at the given `block_number` or `None` if
/// `block_number` isn't within the range of the previous 256 blocks.
fn block_hash(&self, block_number: U256) -> Option<H256>;

/// Returns the maximum allowed size of a storage item.
fn max_value_size(&self) -> u32;

Expand Down Expand Up @@ -733,6 +737,7 @@ where
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
E: Executable<T>,
T::Hash: frame_support::traits::IsType<H256>,
{
/// Create and run a new call stack by calling into `dest`.
///
Expand Down Expand Up @@ -1323,6 +1328,24 @@ where
pub(crate) fn override_export(&mut self, export: ExportedFunction) {
self.top_frame_mut().entry_point = export;
}

#[cfg(all(feature = "runtime-benchmarks", feature = "riscv"))]
pub(crate) fn set_block_number(&mut self, block_number: BlockNumberFor<T>) {
self.block_number = block_number;
}

fn block_hash(&self, block_number: U256) -> Option<H256> {
let Ok(block_number) = BlockNumberFor::<T>::try_from(block_number) else {
return None;
};
if block_number >= self.block_number {
return None;
}
if block_number < self.block_number.saturating_sub(256u32.into()) {
return None;
}
Comment on lines +1350 to +1352
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this ? why don't we just try t get it with System::::block_hash

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented it to resemble the semantics of the corresponding EVM opcode

Some(System::<T>::block_hash(&block_number).into())
}
}

impl<'a, T, E> Ext for Stack<'a, T, E>
Expand All @@ -1331,6 +1354,7 @@ where
E: Executable<T>,
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
{
type T = T;

Expand Down Expand Up @@ -1631,6 +1655,10 @@ where
self.block_number.into()
}

fn block_hash(&self, block_number: U256) -> Option<H256> {
self.block_hash(block_number)
}

fn max_value_size(&self) -> u32 {
limits::PAYLOAD_BYTES
}
Expand Down Expand Up @@ -4671,4 +4699,60 @@ mod tests {
.unwrap()
});
}

#[test]
fn block_hash_returns_proper_values() {
let bob_code_hash = MockLoader::insert(Call, |ctx, _| {
ctx.ext.block_number = 1u32.into();
assert_eq!(ctx.ext.block_hash(U256::from(1)), None);
assert_eq!(ctx.ext.block_hash(U256::from(0)), Some(H256::from([1; 32])));

ctx.ext.block_number = 300u32.into();
assert_eq!(ctx.ext.block_hash(U256::from(300)), None);
assert_eq!(ctx.ext.block_hash(U256::from(43)), None);
assert_eq!(ctx.ext.block_hash(U256::from(44)), Some(H256::from([2; 32])));

exec_success()
});

ExtBuilder::default().build().execute_with(|| {
frame_system::BlockHash::<Test>::insert(
&BlockNumberFor::<Test>::from(0u32),
<tests::Test as frame_system::Config>::Hash::from([1; 32]),
);
frame_system::BlockHash::<Test>::insert(
&BlockNumberFor::<Test>::from(1u32),
<tests::Test as frame_system::Config>::Hash::default(),
);
frame_system::BlockHash::<Test>::insert(
&BlockNumberFor::<Test>::from(43u32),
<tests::Test as frame_system::Config>::Hash::default(),
);
frame_system::BlockHash::<Test>::insert(
&BlockNumberFor::<Test>::from(44u32),
<tests::Test as frame_system::Config>::Hash::from([2; 32]),
);
frame_system::BlockHash::<Test>::insert(
&BlockNumberFor::<Test>::from(300u32),
<tests::Test as frame_system::Config>::Hash::default(),
);

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();
assert_matches!(
MockStack::run_call(
origin,
BOB_ADDR,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&mut storage_meter,
0,
vec![0],
None,
),
Ok(_)
);
});
}
}
3 changes: 3 additions & 0 deletions substrate/frame/revive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,7 @@ pub mod pallet {
where
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
{
/// A raw EVM transaction, typically dispatched by an Ethereum JSON-RPC server.
///
Expand Down Expand Up @@ -1077,6 +1078,7 @@ impl<T: Config> Pallet<T>
where
BalanceOf<T>: Into<U256> + TryFrom<U256>,
MomentOf<T>: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
{
/// A generalized version of [`Self::call`].
///
Expand Down Expand Up @@ -1236,6 +1238,7 @@ where
<T as Config>::RuntimeCall: Encode,
OnChargeTransactionBalanceOf<T>: Into<BalanceOf<T>>,
T::Nonce: Into<U256>,
T::Hash: frame_support::traits::IsType<H256>,
{
// Get the nonce to encode in the tx.
let nonce: T::Nonce = <System<T>>::account_nonce(&origin);
Expand Down
25 changes: 25 additions & 0 deletions substrate/frame/revive/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4577,4 +4577,29 @@ mod run_tests {
assert_eq!(<Test as Config>::Currency::total_balance(&EVE), 1_100);
});
}

#[test]
fn block_hash_works() {
let (code, _) = compile_module("block_number").unwrap();

ExtBuilder::default().existential_deposit(1).build().execute_with(|| {
let _ = <Test as Config>::Currency::set_balance(&ALICE, 1_000_000);

let Contract { addr, .. } =
builder::bare_instantiate(Code::Upload(code)).build_and_unwrap_contract();

// The genesis config sets to the block number to 1
let block_hash = [1; 32];
frame_system::BlockHash::<Test>::insert(
&crate::BlockNumberFor::<Test>::from(0u32),
<Test as frame_system::Config>::Hash::from(&block_hash),
);
assert_ok!(builder::call(addr)
.data((U256::zero(), H256::from(block_hash)).encode())
.build());

// A block number out of range returns the zero value
assert_ok!(builder::call(addr).data((U256::from(1), H256::zero()).encode()).build());
});
}
}
15 changes: 6 additions & 9 deletions substrate/frame/revive/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use frame_support::{
ensure,
traits::{fungible::MutateHold, tokens::Precision::BestEffort},
};
use sp_core::{Get, U256};
use sp_core::{Get, H256, U256};
use sp_runtime::DispatchError;

/// Validated Wasm module ready for execution.
Expand All @@ -63,7 +63,7 @@ pub struct WasmBlob<T: Config> {
code_info: CodeInfo<T>,
// This is for not calculating the hash every time we need it.
#[codec(skip)]
code_hash: sp_core::H256,
code_hash: H256,
}

/// Contract code related data, such as:
Expand Down Expand Up @@ -147,14 +147,14 @@ where
api_version: API_VERSION,
behaviour_version: Default::default(),
};
let code_hash = sp_core::H256(sp_io::hashing::keccak_256(&code));
let code_hash = H256(sp_io::hashing::keccak_256(&code));
Ok(WasmBlob { code, code_info, code_hash })
}

/// Remove the code from storage and refund the deposit to its owner.
///
/// Applies all necessary checks before removing the code.
pub fn remove(origin: &T::AccountId, code_hash: sp_core::H256) -> DispatchResult {
pub fn remove(origin: &T::AccountId, code_hash: H256) -> DispatchResult {
<CodeInfoOf<T>>::try_mutate_exists(&code_hash, |existing| {
if let Some(code_info) = existing {
ensure!(code_info.refcount == 0, <Error<T>>::CodeInUse);
Expand Down Expand Up @@ -330,10 +330,7 @@ impl<T: Config> Executable<T> for WasmBlob<T>
where
BalanceOf<T>: Into<U256> + TryFrom<U256>,
{
fn from_storage(
code_hash: sp_core::H256,
gas_meter: &mut GasMeter<T>,
) -> Result<Self, DispatchError> {
fn from_storage(code_hash: H256, gas_meter: &mut GasMeter<T>) -> Result<Self, DispatchError> {
let code_info = <CodeInfoOf<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
gas_meter.charge(CodeLoadToken(code_info.code_len))?;
let code = <PristineCode<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
Expand All @@ -360,7 +357,7 @@ where
self.code.as_ref()
}

fn code_hash(&self) -> &sp_core::H256 {
fn code_hash(&self) -> &H256 {
&self.code_hash
}

Expand Down
Loading
Loading