From 74ae05142849bdf2a816df38e80f419741b10dc6 Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Fri, 14 Jul 2023 14:49:47 +0200 Subject: [PATCH] Contracts add code_len to ContractsInfo (#14523) * add code_len to v12 * fix * Update frame/contracts/src/wasm/mod.rs * fix * fixes * rm test * add test back * fix * update test * Fix comments * fix build * del * fix clippy * fix * re-rename --- frame/contracts/src/migration/v12.rs | 3 +++ frame/contracts/src/tests.rs | 14 +++++----- frame/contracts/src/tests/pallet_dummy.rs | 33 +++++++++++++++++++++++ frame/contracts/src/wasm/mod.rs | 22 +++++---------- frame/contracts/src/wasm/prepare.rs | 6 +++-- 5 files changed, 54 insertions(+), 24 deletions(-) create mode 100644 frame/contracts/src/tests/pallet_dummy.rs diff --git a/frame/contracts/src/migration/v12.rs b/frame/contracts/src/migration/v12.rs index c7987075f54b4..4fb7ca76fea92 100644 --- a/frame/contracts/src/migration/v12.rs +++ b/frame/contracts/src/migration/v12.rs @@ -80,6 +80,7 @@ pub struct CodeInfo { #[codec(compact)] refcount: u64, determinism: Determinism, + code_len: u32, } #[storage_alias] @@ -177,6 +178,7 @@ impl MigrationStep for Migration { owner: old_info.owner, deposit, refcount: old_info.refcount, + code_len: code_len as u32, }; let amount = old_info.deposit.saturating_sub(info.deposit); @@ -221,6 +223,7 @@ impl MigrationStep for Migration { deposit: v.deposit, refcount: v.refcount, owner: v.owner, + code_len: module.code.len() as u32, }; (k, info) }) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index b6755a69cc76f..b25487469ba0e 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1,4 +1,5 @@ // This file is part of Substrate. +mod pallet_dummy; // Copyright (C) Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 @@ -68,6 +69,7 @@ frame_support::construct_runtime!( Utility: pallet_utility::{Pallet, Call, Storage, Event}, Contracts: pallet_contracts::{Pallet, Call, Storage, Event}, Proxy: pallet_proxy::{Pallet, Call, Storage, Event}, + Dummy: pallet_dummy } ); @@ -373,6 +375,8 @@ impl pallet_proxy::Config for Test { type AnnouncementDepositFactor = ConstU64<1>; } +impl pallet_dummy::Config for Test {} + parameter_types! { pub MySchedule: Schedule = { let schedule = >::default(); @@ -2999,7 +3003,7 @@ fn gas_estimation_call_runtime() { .unwrap() .account_id; - let addr_callee = Contracts::bare_instantiate( + Contracts::bare_instantiate( ALICE, min_balance * 100, GAS_LIMIT, @@ -3011,15 +3015,11 @@ fn gas_estimation_call_runtime() { CollectEvents::Skip, ) .result - .unwrap() - .account_id; + .unwrap(); // Call something trivial with a huge gas limit so that we can observe the effects // of pre-charging. This should create a difference between consumed and required. - let call = RuntimeCall::Balances(pallet_balances::Call::transfer_allow_death { - dest: addr_callee, - value: min_balance * 10, - }); + let call = RuntimeCall::Dummy(pallet_dummy::Call::overestimate_pre_charge {}); let result = Contracts::bare_call( ALICE, addr_caller.clone(), diff --git a/frame/contracts/src/tests/pallet_dummy.rs b/frame/contracts/src/tests/pallet_dummy.rs new file mode 100644 index 0000000000000..d84e34de06c8d --- /dev/null +++ b/frame/contracts/src/tests/pallet_dummy.rs @@ -0,0 +1,33 @@ +pub use pallet::*; + +#[frame_support::pallet(dev_mode)] +pub mod pallet { + use frame_support::{ + dispatch::{Pays, PostDispatchInfo}, + pallet_prelude::DispatchResultWithPostInfo, + weights::Weight, + }; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::call] + impl Pallet { + /// Dummy function that overcharges the predispatch weight, allowing us to test the correct + /// values of [`ContractResult::gas_consumed`] and [`ContractResult::gas_required`] in + /// tests. + #[pallet::call_index(1)] + #[pallet::weight(Weight::from_parts(10_000_000, 0))] + pub fn overestimate_pre_charge(origin: OriginFor) -> DispatchResultWithPostInfo { + ensure_signed(origin)?; + Ok(PostDispatchInfo { + actual_weight: Some(Weight::from_parts(100, 0)), + pays_fee: Pays::Yes, + }) + } + } +} diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index aa37b6fea2c73..b975a39a81052 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -92,6 +92,8 @@ pub struct CodeInfo { /// to be run on-chain. Specifically, such a code can never be instantiated into a contract /// and can just be used through a delegate call. determinism: Determinism, + /// length of the code in bytes. + code_len: u32, } /// Defines the required determinism level of a wasm blob when either running or uploading code. @@ -268,15 +270,11 @@ impl WasmBlob { fn load_code( code_hash: CodeHash, gas_meter: &mut GasMeter, - ) -> Result, DispatchError> { - let max_code_len = T::MaxCodeLen::get(); - let charged = gas_meter.charge(CodeLoadToken(max_code_len))?; - + ) -> Result<(CodeVec, CodeInfo), DispatchError> { + let code_info = >::get(code_hash).ok_or(Error::::CodeNotFound)?; + gas_meter.charge(CodeLoadToken(code_info.code_len))?; let code = >::get(code_hash).ok_or(Error::::CodeNotFound)?; - let code_len = code.len() as u32; - gas_meter.adjust_gas(charged, CodeLoadToken(code_len)); - - Ok(code) + Ok((code, code_info)) } /// Create the module without checking the passed code. @@ -309,13 +307,7 @@ impl Executable for WasmBlob { code_hash: CodeHash, gas_meter: &mut GasMeter, ) -> Result { - let code = Self::load_code(code_hash, gas_meter)?; - // We store `code_info` at the same time as contract code, - // therefore this query shouldn't really fail. - // We consider its failure equal to `CodeNotFound`, as contract code without - // `code_info` is unusable in this pallet. - let code_info = >::get(code_hash).ok_or(Error::::CodeNotFound)?; - + let (code, code_info) = Self::load_code(code_hash, gas_meter)?; Ok(Self { code, code_info, code_hash }) } diff --git a/frame/contracts/src/wasm/prepare.rs b/frame/contracts/src/wasm/prepare.rs index ee89aae642b4a..7deccdde7a069 100644 --- a/frame/contracts/src/wasm/prepare.rs +++ b/frame/contracts/src/wasm/prepare.rs @@ -286,11 +286,12 @@ where validate::(code.as_ref(), schedule, determinism)?; // Calculate deposit for storing contract code and `code_info` in two different storage items. - let bytes_added = code.len().saturating_add(>::max_encoded_len()) as u32; + let code_len = code.len() as u32; + let bytes_added = code_len.saturating_add(>::max_encoded_len() as u32); let deposit = Diff { bytes_added, items_added: 2, ..Default::default() } .update_contract::(None) .charge_or_zero(); - let code_info = CodeInfo { owner, deposit, determinism, refcount: 0 }; + let code_info = CodeInfo { owner, deposit, determinism, refcount: 0, code_len }; let code_hash = T::Hashing::hash(&code); Ok(WasmBlob { code, code_info, code_hash }) @@ -320,6 +321,7 @@ pub mod benchmarking { // this is a helper function for benchmarking which skips deposit collection deposit: Default::default(), refcount: 0, + code_len: code.len() as u32, determinism, }; let code_hash = T::Hashing::hash(&code);