Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

contracts: Get rid of #[pallet::without_storage_info] #11414

Merged
paritytech-processbot merged 7 commits into from
May 18, 2022
Merged

contracts: Get rid of #[pallet::without_storage_info] #11414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
395149c
Implement `MaxEncodeLen` for pallet-contracts storage
athei May 12, 2022
52dfda8
Merge branch 'master' into at/contracts-bounded
athei May 15, 2022
a132c08
Remove redundant debug println
athei May 17, 2022
c670604
Merge remote-tracking branch 'origin/master' into at/contracts-bounded
athei May 17, 2022
cb9208e
Move code len check to PrefabWasmModule::from_code
athei May 17, 2022
6016cfe
Merge remote-tracking branch 'origin/master' into at/contracts-bounded
athei May 17, 2022
7dc9166
Merge remote-tracking branch 'origin/master' into at/contracts-bounded
athei May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bin/node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,8 @@ impl pallet_contracts::Config for Runtime {
type Schedule = Schedule;
type AddressGenerator = pallet_contracts::DefaultAddressGenerator;
type ContractAccessWeight = pallet_contracts::DefaultContractAccessWeight<RuntimeBlockWeights>;
type MaxCodeLen = ConstU32<{ 128 * 1024 }>;
type RelaxedMaxCodeLen = ConstU32<{ 256 * 1024 }>;
}

impl pallet_sudo::Config for Runtime {
Expand Down
8 changes: 4 additions & 4 deletions frame/contracts/src/benchmarking/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ benchmarks! {
// first time after a new schedule was deployed: For every new schedule a contract needs
// to re-run the instrumentation once.
reinstrument {
let c in 0 .. T::Schedule::get().limits.code_len;
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
Contracts::<T>::store_code_raw(code, whitelisted_caller())?;
let schedule = T::Schedule::get();
Expand All @@ -247,7 +247,7 @@ benchmarks! {
// which is in the wasm module but not executed on `call`.
// The results are supposed to be used as `call_with_code_kb(c) - call_with_code_kb(0)`.
call_with_code_per_byte {
let c in 0 .. T::Schedule::get().limits.code_len;
let c in 0 .. T::MaxCodeLen::get();
let instance = Contract::<T>::with_caller(
whitelisted_caller(), WasmModule::sized(c, Location::Deploy), vec![],
)?;
Expand All @@ -271,7 +271,7 @@ benchmarks! {
// We cannot let `c` grow to the maximum code size because the code is not allowed
// to be larger than the maximum size **after instrumentation**.
instantiate_with_code {
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::Schedule::get().limits.code_len);
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
let s in 0 .. code::max_pages::<T>() * 64 * 1024;
let salt = vec![42u8; s as usize];
let value = T::Currency::minimum_balance();
Expand Down Expand Up @@ -360,7 +360,7 @@ benchmarks! {
// We cannot let `c` grow to the maximum code size because the code is not allowed
// to be larger than the maximum size **after instrumentation**.
upload_code {
let c in 0 .. Perbill::from_percent(50).mul_ceil(T::Schedule::get().limits.code_len);
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
let caller = whitelisted_caller();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
Expand Down
43 changes: 26 additions & 17 deletions frame/contracts/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ use codec::{Encode, HasCompact};
use frame_support::{
dispatch::Dispatchable,
ensure,
traits::{Contains, Currency, Get, Randomness, ReservableCurrency, Time},
traits::{ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time},
weights::{DispatchClass, GetDispatchInfo, Pays, PostDispatchInfo, Weight},
BoundedVec,
};
use frame_system::{limits::BlockWeights, Pallet as System};
use pallet_contracts_primitives::{
Expand All @@ -129,9 +130,11 @@ use sp_runtime::traits::{Convert, Hash, Saturating, StaticLookup};
use sp_std::{fmt::Debug, marker::PhantomData, prelude::*};

type CodeHash<T> = <T as frame_system::Config>::Hash;
type TrieId = Vec<u8>;
type TrieId = BoundedVec<u8, ConstU32<128>>;
type BalanceOf<T> =
<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
type CodeVec<T> = BoundedVec<u8, <T as Config>::MaxCodeLen>;
type RelaxedCodeVec<T> = BoundedVec<u8, <T as Config>::RelaxedMaxCodeLen>;

/// Used as a sentinel value when reading and writing contract memory.
///
Expand Down Expand Up @@ -224,7 +227,6 @@ pub mod pallet {

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
#[pallet::without_storage_info]
pub struct Pallet<T>(PhantomData<T>);

#[pallet::config]
Expand Down Expand Up @@ -356,6 +358,20 @@ pub mod pallet {

/// The address generator used to generate the addresses of contracts.
type AddressGenerator: AddressGenerator<Self>;

/// The maximum length of a contract code in bytes. This limit applies to the instrumented
/// version of the code. Therefore `instantiate_with_code` can fail even when supplying
/// a wasm binary below this maximum size.
type MaxCodeLen: Get<u32>;

/// The maximum length of a contract code after reinstrumentation.
///
/// When uploading a new contract the size defined by [`Self::MaxCodeLen`] is used for both
/// the pristine **and** the instrumented version. When a existing contract needs to be
/// reinstrumented after a runtime upgrade we apply this bound. The reason is that if the
/// new instrumentation increases the size beyond the limit it would make that contract
/// inaccessible until rectified by another runtime upgrade.
type RelaxedMaxCodeLen: Get<u32>;
}

#[pallet::hooks]
Expand Down Expand Up @@ -698,7 +714,7 @@ pub mod pallet {

/// A mapping from an original code hash to the original code, untouched by instrumentation.
#[pallet::storage]
pub(crate) type PristineCode<T: Config> = StorageMap<_, Identity, CodeHash<T>, Vec<u8>>;
pub(crate) type PristineCode<T: Config> = StorageMap<_, Identity, CodeHash<T>, CodeVec<T>>;

/// A mapping between an original code hash and instrumented wasm code, ready for execution.
#[pallet::storage]
Expand Down Expand Up @@ -746,7 +762,8 @@ pub mod pallet {
/// Child trie deletion is a heavy operation depending on the amount of storage items
/// stored in said trie. Therefore this operation is performed lazily in `on_initialize`.
#[pallet::storage]
pub(crate) type DeletionQueue<T: Config> = StorageValue<_, Vec<DeletedContract>, ValueQuery>;
pub(crate) type DeletionQueue<T: Config> =
StorageValue<_, BoundedVec<DeletedContract, T::DeletionQueueDepth>, ValueQuery>;
}

/// Return type of the private [`Pallet::internal_call`] function.
Expand Down Expand Up @@ -864,8 +881,8 @@ where
storage_deposit_limit: Option<BalanceOf<T>>,
) -> CodeUploadResult<CodeHash<T>, BalanceOf<T>> {
let schedule = T::Schedule::get();
let module = PrefabWasmModule::from_code(code, &schedule, origin)
.map_err(|_| <Error<T>>::CodeRejected)?;
let module =
PrefabWasmModule::from_code(code, &schedule, origin).map_err(|(err, _)| err)?;
let deposit = module.open_deposit();
if let Some(storage_deposit_limit) = storage_deposit_limit {
ensure!(storage_deposit_limit >= deposit, <Error<T>>::StorageDepositLimitExhausted);
Expand Down Expand Up @@ -971,19 +988,11 @@ where
let schedule = T::Schedule::get();
let (extra_deposit, executable) = match code {
Code::Upload(Bytes(binary)) => {
ensure!(
binary.len() as u32 <= schedule.limits.code_len,
<Error<T>>::CodeTooLarge
);
let executable = PrefabWasmModule::from_code(binary, &schedule, origin.clone())
.map_err(|msg| {
.map_err(|(err, msg)| {
debug_message.as_mut().map(|buffer| buffer.extend(msg.as_bytes()));
<Error<T>>::CodeRejected
err
})?;
ensure!(
executable.code_len() <= schedule.limits.code_len,
<Error<T>>::CodeTooLarge
);
// The open deposit will be charged during execution when the
// uploaded module does not already exist. This deposit is not part of the
// storage meter because it is not transfered to the contract but
Expand Down
8 changes: 1 addition & 7 deletions frame/contracts/src/schedule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use wasm_instrument::{gas_metering, parity_wasm::elements};
/// How many API calls are executed in a single batch. The reason for increasing the amount
/// of API calls in batches (per benchmark component increase) is so that the linear regression
/// has an easier time determining the contribution of that component.
pub const API_BENCHMARK_BATCH_SIZE: u32 = 100;
pub const API_BENCHMARK_BATCH_SIZE: u32 = 80;

/// How many instructions are executed in a single batch. The reasoning is the same
/// as for `API_BENCHMARK_BATCH_SIZE`.
Expand Down Expand Up @@ -147,11 +147,6 @@ pub struct Limits {

/// The maximum size of a storage value and event payload in bytes.
pub payload_len: u32,

/// The maximum length of a contract code in bytes. This limit applies to the instrumented
/// version of the code. Therefore `instantiate_with_code` can fail even when supplying
/// a wasm binary below this maximum size.
pub code_len: u32,
}

impl Limits {
Expand Down Expand Up @@ -522,7 +517,6 @@ impl Default for Limits {
subject_len: 32,
call_depth: 32,
payload_len: 16 * 1024,
code_len: 128 * 1024,
}
}
}
Expand Down
29 changes: 15 additions & 14 deletions frame/contracts/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ use crate::{
weights::WeightInfo,
BalanceOf, CodeHash, Config, ContractInfoOf, DeletionQueue, Error, TrieId, SENTINEL,
};
use codec::{Decode, Encode};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
dispatch::{DispatchError, DispatchResult},
storage::child::{self, ChildInfo, KillStorageResult},
traits::Get,
weights::Weight,
};
use scale_info::TypeInfo;
Expand All @@ -44,7 +43,7 @@ pub type ContractInfo<T> = RawContractInfo<CodeHash<T>, BalanceOf<T>>;

/// Information for managing an account and its sub trie abstraction.
/// This is the required info to cache for an account.
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)]
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub struct RawContractInfo<CodeHash, Balance> {
/// Unique ID for the subtree encoded as a bytes vector.
pub trie_id: TrieId,
Expand All @@ -67,7 +66,7 @@ fn child_trie_info(trie_id: &[u8]) -> ChildInfo {
ChildInfo::new_default(trie_id)
}

#[derive(Encode, Decode, TypeInfo)]
#[derive(Encode, Decode, TypeInfo, MaxEncodedLen)]
pub struct DeletedContract {
pub(crate) trie_id: TrieId,
}
Expand Down Expand Up @@ -217,12 +216,8 @@ where
///
/// You must make sure that the contract is also removed when queuing the trie for deletion.
pub fn queue_trie_for_deletion(contract: &ContractInfo<T>) -> DispatchResult {
if <DeletionQueue<T>>::decode_len().unwrap_or(0) >= T::DeletionQueueDepth::get() as usize {
Err(Error::<T>::DeletionQueueFull.into())
} else {
<DeletionQueue<T>>::append(DeletedContract { trie_id: contract.trie_id.clone() });
Ok(())
}
<DeletionQueue<T>>::try_append(DeletedContract { trie_id: contract.trie_id.clone() })
.map_err(|_| <Error<T>>::DeletionQueueFull.into())
}

/// Calculates the weight that is necessary to remove one key from the trie and how many
Expand Down Expand Up @@ -293,7 +288,11 @@ where
/// Generates a unique trie id by returning `hash(account_id ++ nonce)`.
pub fn generate_trie_id(account_id: &AccountIdOf<T>, nonce: u64) -> TrieId {
let buf: Vec<_> = account_id.as_ref().iter().chain(&nonce.to_le_bytes()).cloned().collect();
T::Hashing::hash(&buf).as_ref().into()
T::Hashing::hash(&buf)
.as_ref()
.to_vec()
.try_into()
.expect("Runtime uses a reasonable hash size. Hence sizeof(T::Hash) <= 128; qed")
}

/// Returns the code hash of the contract specified by `account` ID.
Expand All @@ -305,9 +304,11 @@ where
/// Fill up the queue in order to exercise the limits during testing.
#[cfg(test)]
pub fn fill_queue_with_dummies() {
let queue: Vec<_> = (0..T::DeletionQueueDepth::get())
.map(|_| DeletedContract { trie_id: vec![] })
use frame_support::{traits::Get, BoundedVec};
let queue: Vec<DeletedContract> = (0..T::DeletionQueueDepth::get())
.map(|_| DeletedContract { trie_id: TrieId::default() })
.collect();
<DeletionQueue<T>>::put(queue);
let bounded: BoundedVec<_, _> = queue.try_into().unwrap();
<DeletionQueue<T>>::put(bounded);
}
}
2 changes: 2 additions & 0 deletions frame/contracts/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ impl Config for Test {
type DepositPerItem = DepositPerItem;
type AddressGenerator = DefaultAddressGenerator;
type ContractAccessWeight = DefaultContractAccessWeight<BlockWeights>;
type MaxCodeLen = ConstU32<{ 128 * 1024 }>;
type RelaxedMaxCodeLen = ConstU32<{ 256 * 1024 }>;
}

pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);
Expand Down
10 changes: 7 additions & 3 deletions frame/contracts/src/wasm/code_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ pub fn load<T: Config>(
where
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
let charged = gas_meter.charge(CodeToken::Load(schedule.limits.code_len))?;
let max_code_len = T::MaxCodeLen::get();
let charged = gas_meter.charge(CodeToken::Load(max_code_len))?;

let mut prefab_module = <CodeStorage<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
gas_meter.adjust_gas(charged, CodeToken::Load(prefab_module.code.len() as u32));
Expand All @@ -172,7 +173,7 @@ where
if prefab_module.instruction_weights_version < schedule.instruction_weights.version {
// The instruction weights have changed.
// We need to re-instrument the code with the new instruction weights.
let charged = gas_meter.charge(CodeToken::Reinstrument(schedule.limits.code_len))?;
let charged = gas_meter.charge(CodeToken::Reinstrument(max_code_len))?;
let code_size = reinstrument(&mut prefab_module, schedule)?;
gas_meter.adjust_gas(charged, CodeToken::Reinstrument(code_size));
}
Expand All @@ -190,7 +191,10 @@ pub fn reinstrument<T: Config>(
let original_code =
<PristineCode<T>>::get(&prefab_module.code_hash).ok_or(Error::<T>::CodeNotFound)?;
let original_code_len = original_code.len();
prefab_module.code = prepare::reinstrument_contract::<T>(original_code, schedule)?;
prefab_module.code = prepare::reinstrument_contract::<T>(&original_code, schedule)
.map_err(|_| <Error<T>>::CodeRejected)?
.try_into()
.map_err(|_| <Error<T>>::CodeTooLarge)?;
prefab_module.instruction_weights_version = schedule.instruction_weights.version;
<CodeStorage<T>>::insert(&prefab_module.code_hash, &*prefab_module);
Ok(original_code_len as u32)
Expand Down
31 changes: 24 additions & 7 deletions frame/contracts/src/wasm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,15 @@ use crate::{
exec::{ExecResult, Executable, ExportedFunction, Ext},
gas::GasMeter,
wasm::env_def::FunctionImplProvider,
AccountIdOf, BalanceOf, CodeHash, CodeStorage, Config, Schedule,
AccountIdOf, BalanceOf, CodeHash, CodeStorage, CodeVec, Config, Error, RelaxedCodeVec,
Schedule,
};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::dispatch::{DispatchError, DispatchResult};
use frame_support::{
dispatch::{DispatchError, DispatchResult},
ensure,
traits::Get,
};
use sp_core::crypto::UncheckedFrom;
use sp_sandbox::{SandboxEnvironmentBuilder, SandboxInstance, SandboxMemory};
use sp_std::prelude::*;
Expand All @@ -50,7 +55,8 @@ pub use tests::MockExt;
/// `instruction_weights_version` and `code` change when a contract with an outdated instrumentation
/// is called. Therefore one must be careful when holding any in-memory representation of this
/// type while calling into a contract as those fields can get out of date.
#[derive(Clone, Encode, Decode, scale_info::TypeInfo)]
#[derive(Clone, Encode, Decode, scale_info::TypeInfo, MaxEncodedLen)]
#[codec(mel_bound())]
#[scale_info(skip_type_params(T))]
pub struct PrefabWasmModule<T: Config> {
/// Version of the instruction weights with which the code was instrumented.
Expand All @@ -63,14 +69,14 @@ pub struct PrefabWasmModule<T: Config> {
#[codec(compact)]
maximum: u32,
/// Code instrumented with the latest schedule.
code: Vec<u8>,
code: RelaxedCodeVec<T>,
/// The uninstrumented, pristine version of the code.
///
/// It is not stored because the pristine code has its own storage item. The value
/// is only `Some` when this module was created from an `original_code` and `None` if
/// it was loaded from storage.
#[codec(skip)]
original_code: Option<Vec<u8>>,
original_code: Option<CodeVec<T>>,
/// The code hash of the stored code which is defined as the hash over the `original_code`.
///
/// As the map key there is no need to store the hash in the value, too. It is set manually
Expand Down Expand Up @@ -122,8 +128,19 @@ where
original_code: Vec<u8>,
schedule: &Schedule<T>,
owner: AccountIdOf<T>,
) -> Result<Self, &'static str> {
prepare::prepare_contract(original_code, schedule, owner)
) -> Result<Self, (DispatchError, &'static str)> {
let module = prepare::prepare_contract(
original_code.try_into().map_err(|_| (<Error<T>>::CodeTooLarge.into(), ""))?,
schedule,
owner,
)?;
// When instrumenting a new code we apply a stricter limit than enforced by the
// `RelaxedCodeVec` in order to leave some headroom for reinstrumentation.
ensure!(
module.code.len() as u32 <= T::MaxCodeLen::get(),
(<Error<T>>::CodeTooLarge.into(), ""),
);
Ok(module)
}

/// Store the code without instantiating it.
Expand Down
Loading