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

Cache code size/hash in storage #893

Merged
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions frame/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]
environmental = { workspace = true, optional = true }
evm = { workspace = true, features = ["with-codec"] }
hex = { version = "0.4.3", default-features = false, features = ["alloc"] }
hex-literal = { version = "0.3.4" }
impl-trait-for-tuples = "0.2.2"
log = { workspace = true }
rlp = { workspace = true }
Expand Down
54 changes: 53 additions & 1 deletion frame/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ use frame_support::{
};
use frame_system::RawOrigin;
use impl_trait_for_tuples::impl_for_tuples;
use sp_core::{Hasher, H160, H256, U256};
use scale_info::TypeInfo;
use sp_core::{Decode, Encode, Hasher, H160, H256, U256};
use sp_runtime::{
traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero},
AccountId32, DispatchErrorWithPostInfo,
Expand Down Expand Up @@ -512,6 +513,10 @@ pub mod pallet {
#[pallet::storage]
pub type AccountCodes<T: Config> = StorageMap<_, Blake2_128Concat, H160, Vec<u8>, ValueQuery>;

#[pallet::storage]
pub type AccountCodesMetadata<T: Config> =
nanocryk marked this conversation as resolved.
Show resolved Hide resolved
StorageMap<_, Blake2_128Concat, H160, CodeMetadata, OptionQuery>;

#[pallet::storage]
pub type AccountStorages<T: Config> =
StorageDoubleMap<_, Blake2_128Concat, H160, Blake2_128Concat, H256, H256, ValueQuery>;
Expand All @@ -525,6 +530,21 @@ pub type BalanceOf<T> =
type NegativeImbalanceOf<C, T> =
<C as Currency<<T as frame_system::Config>::AccountId>>::NegativeImbalance;

#[derive(Debug, Clone, Copy, Eq, PartialEq, Encode, Decode, TypeInfo)]
pub struct CodeMetadata {
pub size: u64,
pub hash: H256,
}

impl CodeMetadata {
fn from_code(code: &[u8]) -> Self {
let size = code.len() as u64;
let hash = H256::from(sp_io::hashing::keccak_256(code));

Self { size, hash }
}
}

pub trait EnsureAddressOrigin<OuterOrigin> {
/// Success return type.
type Success;
Expand Down Expand Up @@ -720,6 +740,7 @@ impl<T: Config> Pallet<T> {
}

<AccountCodes<T>>::remove(address);
<AccountCodesMetadata<T>>::remove(address);
#[allow(deprecated)]
let _ = <AccountStorages<T>>::remove_prefix(address, None);
}
Expand All @@ -735,9 +756,40 @@ impl<T: Config> Pallet<T> {
let _ = frame_system::Pallet::<T>::inc_sufficients(&account_id);
}

// Update metadata.
let meta = CodeMetadata::from_code(&code);
<AccountCodesMetadata<T>>::insert(address, meta);

<AccountCodes<T>>::insert(address, code);
}

/// Get the account metadata (hash and size) from storage if it exists,
/// or compute it from code and store it if it doesn't exist.
pub fn account_code_metadata(address: H160) -> CodeMetadata {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function should return an option to express the case where the account not have any code.

if let Some(meta) = <AccountCodesMetadata<T>>::get(address) {
return meta;
}

let code = <AccountCodes<T>>::get(address);

// If code is empty we return precomputed hash for empty code.
// We don't store it as this address could get code deployed in the future.
if code.is_empty() {
const EMPTY_CODE_HASH: [u8; 32] = hex_literal::hex!(
"c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"
);
return CodeMetadata {
size: 0,
hash: EMPTY_CODE_HASH.into(),
};
}

let meta = CodeMetadata::from_code(&code);

<AccountCodesMetadata<T>>::insert(address, meta);
meta
}

/// Get the account basic in EVM format.
pub fn account_basic(address: &H160) -> (Account, frame_support::weights::Weight) {
let account_id = T::AddressMapping::into_account_id(*address);
Expand Down
8 changes: 8 additions & 0 deletions frame/evm/src/runner/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,14 @@ where
self.substate
.recursive_is_cold(&|a: &Accessed| a.accessed_storage.contains(&(address, key)))
}

fn code_size(&self, address: H160) -> U256 {
U256::from(<Pallet<T>>::account_code_metadata(address).size)
}

fn code_hash(&self, address: H160) -> H256 {
<Pallet<T>>::account_code_metadata(address).hash
}
}

#[cfg(feature = "forbid-evm-reentrancy")]
Expand Down
37 changes: 37 additions & 0 deletions frame/evm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,3 +651,40 @@ fn eip3607_transaction_from_precompile() {
.is_ok());
});
}

#[test]
fn metadata_code_gets_cached() {
new_test_ext().execute_with(|| {
let address = H160::repeat_byte(0xaa);

crate::Pallet::<Test>::create_account(address, b"Exemple".to_vec());

let metadata = crate::Pallet::<Test>::account_code_metadata(address);
assert_eq!(metadata.size, 7);
assert_eq!(
metadata.hash,
hex_literal::hex!("e8396a990fe08f2402e64a00647e41dadf360ba078a59ba79f55e876e67ed4bc")
.into()
);

let metadata2 = <AccountCodesMetadata<Test>>::get(&address).expect("to have metadata set");
assert_eq!(metadata, metadata2);
});
}

#[test]
fn metadata_empty_dont_code_gets_cached() {
new_test_ext().execute_with(|| {
let address = H160::repeat_byte(0xaa);

let metadata = crate::Pallet::<Test>::account_code_metadata(address);
assert_eq!(metadata.size, 0);
assert_eq!(
metadata.hash,
hex_literal::hex!("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")
.into()
);

assert!(<AccountCodesMetadata<Test>>::get(&address).is_none());
});
}