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

Commit

Permalink
refactor: no reftime_limit&schedule passes, no CodeStorage
Browse files Browse the repository at this point in the history
  • Loading branch information
agryaznov committed Jun 9, 2023
1 parent f5113ec commit 97483ef
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 71 deletions.
2 changes: 0 additions & 2 deletions frame/contracts/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ pub struct ExecReturnValue {
pub flags: ReturnFlags,
/// Buffer passed along by `seal_return`. Empty when `seal_return` was never called.
pub data: Vec<u8>,
/// Gas consumed during the execution. This is returned from the engine.
pub reftime_consumed: u64,
}

impl ExecReturnValue {
Expand Down
8 changes: 4 additions & 4 deletions frame/contracts/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,18 +680,18 @@ fn expand_functions(def: &EnvDef, expand_blocks: bool, host_state: TokenStream2)
quote! {
let __gas_before__ = {
let engine_consumed = __caller__.fuel_consumed().expect("Fuel metering is enabled; qed");
let reftime_consumed = engine_consumed.saturating_mul(<E::T as Config>::Schedule::get().instruction_weights.base as u64);
__caller__.data_mut().ext.gas_meter_mut().charge_fuel(reftime_consumed).map_err(#into_trap).map_err(#into_host)?.ref_time()
let reftime_consumed = engine_consumed.saturating_mul(__caller__.data_mut().ext().schedule().instruction_weights.base as u64);
__caller__.data_mut().ext().gas_meter_mut().charge_fuel(reftime_consumed).map_err(#into_trap).map_err(#into_host)?.ref_time()
};
}
} else {
quote! { }
};
let sync_gas_after = if expand_blocks {
quote! {
let gas_after = __caller__.data().ext.gas_meter().gas_left().ref_time();
let gas_after = __caller__.data_mut().ext().gas_meter().gas_left().ref_time();
let host_consumed = __gas_before__.saturating_sub(gas_after);
let fuel_consumed = host_consumed.checked_div(<E::T as Config>::Schedule::get().instruction_weights.base as u64).ok_or(Error::<E::T>::InvalidSchedule).map_err(#into_trap).map_err(#into_host)?;
let fuel_consumed = host_consumed.checked_div(__caller__.data_mut().ext().schedule().instruction_weights.base as u64).ok_or(Error::<E::T>::InvalidSchedule).map_err(#into_trap).map_err(#into_host)?;
__caller__.consume_fuel(fuel_consumed).map_err(|_| TrapReason::from(Error::<E::T>::OutOfGas)).map_err(#into_host)?;
}
} else {
Expand Down
8 changes: 2 additions & 6 deletions frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,12 @@ where

/// Returns `true` iff all storage entries related to code storage exist.
fn code_exists(hash: &CodeHash<T>) -> bool {
<PristineCode<T>>::contains_key(hash) &&
<CodeStorage<T>>::contains_key(&hash) &&
<OwnerInfoOf<T>>::contains_key(&hash)
<PristineCode<T>>::contains_key(hash) && <OwnerInfoOf<T>>::contains_key(&hash)
}

/// Returns `true` iff no storage entry related to code storage exist.
fn code_removed(hash: &CodeHash<T>) -> bool {
!<PristineCode<T>>::contains_key(hash) &&
!<CodeStorage<T>>::contains_key(&hash) &&
!<OwnerInfoOf<T>>::contains_key(&hash)
!<PristineCode<T>>::contains_key(hash) && !<OwnerInfoOf<T>>::contains_key(&hash)
}
}

Expand Down
17 changes: 3 additions & 14 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,6 @@ pub trait Executable<T: Config>: Sized {
ext: &mut E,
function: &ExportedFunction,
input_data: Vec<u8>,
schedule: &Schedule<T>,
reftime_limit: u64,
) -> ExecResult;

/// The code hash of the executable.
Expand Down Expand Up @@ -871,21 +869,12 @@ where
// Every non delegate call or instantiate also optionally transfers the balance.
self.initial_transfer()?;

// Take the ref_time part of the gas_left from the current frame.
let frame = self.top_frame();
let reftime_left = frame.nested_gas.gas_left().ref_time();
let schedule = &self.schedule().clone();

// Call into the Wasm blob.
// We pass `reftime_left` into the execution engine to initialize its gas metering.
let output = executable
.execute(self, &entry_point, input_data, schedule, reftime_left)
.execute(self, &entry_point, input_data)
.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;

// Sync this frame's gas meter with the engine's one.
let frame = self.top_frame_mut();
frame.nested_gas.charge_fuel(output.reftime_consumed)?;

// Avoid useless work that would be reverted anyways.
if output.did_revert() {
return Ok(output)
Expand Down Expand Up @@ -1644,8 +1633,8 @@ mod tests {
}
}

fn code_hash(&self) -> &CodeHash<Test> {
&self.code_hash
fn code_hash(&self) -> CodeHash<Test> {
self.code_hash
}

fn code_len(&self) -> u32 {
Expand Down
20 changes: 10 additions & 10 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ use crate::{
tests::test_utils::{get_contract, get_contract_checked},
wasm::{Determinism, ReturnCode as RuntimeReturnCode},
weights::WeightInfo,
BalanceOf, Code, CodeStorage, CollectEvents, Config, ContractInfo, ContractInfoOf, DebugInfo,
BalanceOf, Code, CollectEvents, Config, ContractInfo, ContractInfoOf, DebugInfo,
DefaultAddressGenerator, DeletionQueueCounter, Error, MigrationInProgress, NoopMigration,
Origin, Pallet, Schedule,
Origin, Pallet, PristineCode, Schedule,
};
use assert_matches::assert_matches;
use codec::Encode;
Expand Down Expand Up @@ -357,8 +357,7 @@ impl pallet_proxy::Config for Test {

parameter_types! {
pub MySchedule: Schedule<Test> = {
let mut schedule = <Schedule<Test>>::default();
schedule.instruction_weights.fallback = 1;
let schedule = <Schedule<Test>>::default();
schedule
};
pub static DepositPerByte: BalanceOf<Test> = 1;
Expand Down Expand Up @@ -2564,7 +2563,7 @@ fn refcounter() {
assert_refcount!(code_hash, 1);

// Pristine code should still be there
crate::PristineCode::<Test>::get(code_hash).unwrap();
PristineCode::<Test>::get(code_hash).unwrap();

// remove the last contract
assert_ok!(Contracts::call(
Expand All @@ -2579,7 +2578,6 @@ fn refcounter() {

// refcount is `0` but code should still exists because it needs to be removed manually
assert!(crate::PristineCode::<Test>::contains_key(&code_hash));
assert!(crate::CodeStorage::<Test>::contains_key(&code_hash));
});
}

Expand Down Expand Up @@ -3294,14 +3292,15 @@ fn upload_code_works() {
// Drop previous events
initialize_block(2);

assert!(!<CodeStorage<Test>>::contains_key(code_hash));
assert!(!PristineCode::<Test>::contains_key(&code_hash));

assert_ok!(Contracts::upload_code(
RuntimeOrigin::signed(ALICE),
wasm,
Some(codec::Compact(1_000)),
Determinism::Enforced,
));
assert!(<CodeStorage<Test>>::contains_key(code_hash));
assert!(PristineCode::<Test>::contains_key(&code_hash));

assert_eq!(
System::events(),
Expand Down Expand Up @@ -3389,9 +3388,10 @@ fn remove_code_works() {
Determinism::Enforced,
));

assert!(<CodeStorage<Test>>::contains_key(code_hash));
assert!(PristineCode::<Test>::contains_key(&code_hash));

assert_ok!(Contracts::remove_code(RuntimeOrigin::signed(ALICE), code_hash));
assert!(!<CodeStorage<Test>>::contains_key(code_hash));
assert!(!PristineCode::<Test>::contains_key(&code_hash));

assert_eq!(
System::events(),
Expand Down
36 changes: 23 additions & 13 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,24 +422,22 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
ext: &mut E,
function: &ExportedFunction,
input_data: Vec<u8>,
schedule: &Schedule<T>,
reftime_limit: u64,
) -> ExecResult {
let runtime = Runtime::new(ext, input_data);
let code = self.code.as_slice();
// Extract memory limits from the module.
let contract_module = prepare::ContractModule::new(code)?;
let memory_limits =
prepare::get_memory_limits(contract_module.scan_imports::<T>(&[])?, schedule)?;
prepare::get_memory_limits(contract_module.scan_imports::<T>(&[])?, ext.schedule())?;
// Instantiate the Wasm module to the engine.
let runtime = Runtime::new(ext, input_data);
let (mut store, memory, instance) = Self::instantiate::<crate::wasm::runtime::Env, _>(
code,
runtime,
memory_limits,
StackLimits::default(),
match function {
ExportedFunction::Constructor => AllowDeprecatedInterface::No,
ExportedFunction::Call => AllowDeprecatedInterface::Yes,
ExportedFunction::Constructor => AllowDeprecatedInterface::No,
},
)
.map_err(|msg| {
Expand All @@ -448,9 +446,14 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
})?;
store.data_mut().set_memory(memory);

// Set fuel limit for the Wasm module execution.
// Set fuel limit for the wasmi execution.
// We normalize it by the base instruction weight, as its cost in wasmi engine is 1.
let fuel_limit = reftime_limit
let fuel_limit = store
.data_mut()
.ext()
.gas_meter_mut()
.gas_left()
.ref_time()
.checked_div(T::Schedule::get().instruction_weights.base as u64)
.ok_or(Error::<T>::InvalidSchedule)?;
store
Expand All @@ -472,11 +475,12 @@ impl<T: Config> Executable<T> for WasmBlob<T> {

let result = exported_func.call(&mut store, &[], &mut []);
let engine_consumed = store.fuel_consumed().expect("Fuel metering is enabled; qed");
// Scale the value back to `ref_time` weight.
// Scale the value back to ref_time` weight.
let reftime_consumed =
engine_consumed.saturating_mul(T::Schedule::get().instruction_weights.base as u64);

store.into_data().to_execution_result(result, reftime_consumed)
// Sync this frame's gas meter with the engine's one.
store.data_mut().ext().gas_meter_mut().charge_fuel(reftime_consumed)?;
store.into_data().to_execution_result(result)
}

fn code_hash(&self) -> CodeHash<T> {
Expand Down Expand Up @@ -738,6 +742,9 @@ mod tests {
fn schedule(&self) -> &Schedule<Self::T> {
&self.schedule
}
fn gas_meter(&self) -> &GasMeter<Self::T> {
&self.gas_meter
}
fn gas_meter_mut(&mut self) -> &mut GasMeter<Self::T> {
&mut self.gas_meter
}
Expand Down Expand Up @@ -795,13 +802,16 @@ mod tests {
type RuntimeConfig = <MockExt as Ext>::T;
RuntimeConfig::set_unstable_interface(unstable_interface);
let wasm = wat::parse_str(wat).unwrap();
let schedule = crate::Schedule::default();
let executable = if skip_checks {
WasmBlob::<RuntimeConfig>::from_code_unchecked(wasm, &schedule, ALICE)?
WasmBlob::<RuntimeConfig>::from_code_unchecked(
wasm,
ext.borrow_mut().schedule(),
ALICE,
)?
} else {
WasmBlob::<RuntimeConfig>::from_code(
wasm,
&schedule,
ext.borrow_mut().schedule(),
ALICE,
Determinism::Enforced,
TryInstantiate::Instantiate,
Expand Down
12 changes: 6 additions & 6 deletions frame/contracts/src/wasm/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,10 @@ pub mod benchmarking {
code: Vec<u8>,
schedule: &Schedule<T>,
owner: AccountIdOf<T>,
) -> Result<(WasmBlob<T>, OwnerInfo<T>), &'static str> {
let contract_module = ContractModule::new(&code, schedule)?;
/// We do this here just to check that module's memory import satisfies the schedule
let _memory_limits = get_memory_limits(contract_module.scan_imports(&[])?, schedule)?;
) -> Result<WasmBlob<T>, &'static str> {
let contract_module = ContractModule::new(&code)?;
// We do this here just to check that module's memory import satisfies the schedule
let _memory_limits = get_memory_limits(contract_module.scan_imports::<T>(&[])?, schedule)?;

let code = code.try_into().map_err(|_| "Code too large!")?;
let owner_info = OwnerInfo {
Expand All @@ -473,7 +473,7 @@ pub mod benchmarking {
determinism: Determinism::Enforced,
};

Ok(Self { code, owner_info })
Ok(WasmBlob { code, owner_info })
}
}

Expand All @@ -488,7 +488,7 @@ mod tests {
use pallet_contracts_proc_macro::define_env;
use std::fmt;

impl fmt::Debug for CodeVec<Test> {
impl fmt::Debug for WasmBlob<Test> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "ContractCode {{ .. }}")
}
Expand Down
21 changes: 5 additions & 16 deletions frame/contracts/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,19 +480,11 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> {
}

/// Converts the sandbox result and the runtime state into the execution outcome.
pub fn to_execution_result(
self,
sandbox_result: Result<(), wasmi::Error>,
reftime_consumed: u64,
) -> ExecResult {
pub fn to_execution_result(self, sandbox_result: Result<(), wasmi::Error>) -> ExecResult {
use TrapReason::*;
match sandbox_result {
// Contract returned from main function -> no data was returned.
Ok(_) => Ok(ExecReturnValue {
flags: ReturnFlags::empty(),
data: Vec::new(),
reftime_consumed,
}),
Ok(_) => Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }),
// Contract either trapped or some host function aborted the execution.
Err(wasmi::Error::Trap(trap)) => {
// If we encoded a reason then it is some abort generated by a host function.
Expand All @@ -503,13 +495,10 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> {
Return(ReturnData { flags, data }) => {
let flags =
ReturnFlags::from_bits(flags).ok_or(Error::<E::T>::InvalidCallFlags)?;
Ok(ExecReturnValue { flags, data, reftime_consumed })
Ok(ExecReturnValue { flags, data })
},
Termination => Ok(ExecReturnValue {
flags: ReturnFlags::empty(),
data: Vec::new(),
reftime_consumed,
}),
Termination =>
Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }),
SupervisorError(error) => return Err(error.into()),
}
},
Expand Down

0 comments on commit 97483ef

Please sign in to comment.