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

contracts: Run start function #1367

Merged
merged 9 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions substrate/frame/contracts/fixtures/run_out_of_gas_start_fn.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(module
(import "env" "memory" (memory 1 1))
(start $start)
(func $start
(loop $inf (br $inf)) ;; just run out of gas
(unreachable)
)
(func (export "call"))
(func (export "deploy"))
)
8 changes: 7 additions & 1 deletion substrate/frame/contracts/src/benchmarking/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@ impl<T: Config> From<&WasmModule<T>> for Sandbox {
.add_fuel(u64::MAX)
.expect("We've set up engine to fuel consuming mode; qed");

let entry_point = instance.get_export(&store, "call").unwrap().into_func().unwrap();
let entry_point = instance
.start(&mut store)
.unwrap()
.get_export(&store, "call")
.unwrap()
.into_func()
.unwrap();
Self { entry_point, store }
}
}
Expand Down
111 changes: 45 additions & 66 deletions substrate/frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
storage::{self, meter::Diff, WriteOutcome},
BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf,
DebugBufferVec, Determinism, Error, Event, Nonce, Origin, Pallet as Contracts, Schedule,
WasmBlob, LOG_TARGET,
LOG_TARGET,
};
use frame_support::{
crypto::ecdsa::ECDSAExt,
Expand Down Expand Up @@ -318,6 +318,22 @@ pub trait Ext: sealing::Sealed {
/// Returns a nonce that is incremented for every instantiated contract.
fn nonce(&mut self) -> u64;

/// Increment the reference count of a of a stored code by one.
///
/// # Errors
///
/// [`Error::CodeNotFound`] is returned if no stored code found having the specified
/// `code_hash`.
fn increment_refcount(code_hash: CodeHash<Self::T>) -> Result<(), DispatchError>;

/// Decrement the reference count of a stored code by one.
///
/// # Note
///
/// A contract whose reference count dropped to zero isn't automatically removed. A
/// `remove_code` transaction must be submitted by the original uploader to do so.
fn decrement_refcount(code_hash: CodeHash<Self::T>);

/// Adds a delegate dependency to [`ContractInfo`]'s `delegate_dependencies` field.
///
/// This ensures that the delegated contract is not removed while it is still in use. It
Expand Down Expand Up @@ -381,22 +397,6 @@ pub trait Executable<T: Config>: Sized {
gas_meter: &mut GasMeter<T>,
) -> Result<Self, DispatchError>;

/// Increment the reference count of a of a stored code by one.
///
/// # Errors
///
/// [`Error::CodeNotFound`] is returned if no stored code found having the specified
/// `code_hash`.
fn increment_refcount(code_hash: CodeHash<T>) -> Result<(), DispatchError>;

/// Decrement the reference count of a stored code by one.
///
/// # Note
///
/// A contract whose reference count dropped to zero isn't automatically removed. A
/// `remove_code` transaction must be submitted by the original uploader to do so.
fn decrement_refcount(code_hash: CodeHash<T>);

/// Execute the specified exported function and return the result.
///
/// When the specified function is `Constructor` the executable is stored and its
Expand Down Expand Up @@ -1285,10 +1285,10 @@ where

info.queue_trie_for_deletion();
ContractInfoOf::<T>::remove(&frame.account_id);
E::decrement_refcount(info.code_hash);
Self::decrement_refcount(info.code_hash);

for (code_hash, deposit) in info.delegate_dependencies() {
E::decrement_refcount(*code_hash);
Self::decrement_refcount(*code_hash);
frame
.nested_storage
.charge_deposit(frame.account_id.clone(), StorageDeposit::Refund(*deposit));
Expand Down Expand Up @@ -1491,8 +1491,8 @@ where

frame.nested_storage.charge_deposit(frame.account_id.clone(), deposit);

E::increment_refcount(hash)?;
E::decrement_refcount(prev_hash);
Self::increment_refcount(hash)?;
Self::decrement_refcount(prev_hash);
Contracts::<Self::T>::deposit_event(
vec![T::Hashing::hash_of(&frame.account_id), hash, prev_hash],
Event::ContractCodeUpdated {
Expand Down Expand Up @@ -1525,6 +1525,25 @@ where
}
}

fn increment_refcount(code_hash: CodeHash<Self::T>) -> Result<(), DispatchError> {
<CodeInfoOf<Self::T>>::mutate(code_hash, |existing| -> Result<(), DispatchError> {
if let Some(info) = existing {
*info.refcount_mut() = info.refcount().saturating_add(1);
Ok(())
} else {
Err(Error::<T>::CodeNotFound.into())
}
})
}

fn decrement_refcount(code_hash: CodeHash<T>) {
<CodeInfoOf<T>>::mutate(code_hash, |existing| {
if let Some(info) = existing {
*info.refcount_mut() = info.refcount().saturating_sub(1);
}
});
}

fn add_delegate_dependency(
&mut self,
code_hash: CodeHash<Self::T>,
Expand All @@ -1537,7 +1556,7 @@ where
let deposit = T::CodeHashLockupDepositPercent::get().mul_ceil(code_info.deposit());

info.add_delegate_dependency(code_hash, deposit)?;
<WasmBlob<T>>::increment_refcount(code_hash)?;
Self::increment_refcount(code_hash)?;
frame
.nested_storage
.charge_deposit(frame.account_id.clone(), StorageDeposit::Charge(deposit));
Expand All @@ -1552,8 +1571,7 @@ where
let info = frame.contract_info.get(&frame.account_id);

let deposit = info.remove_delegate_dependency(code_hash)?;
<WasmBlob<T>>::decrement_refcount(*code_hash);

Self::decrement_refcount(*code_hash);
frame
.nested_storage
.charge_deposit(frame.account_id.clone(), StorageDeposit::Refund(deposit));
Expand Down Expand Up @@ -1600,11 +1618,7 @@ mod tests {
use pallet_contracts_primitives::ReturnFlags;
use pretty_assertions::assert_eq;
use sp_runtime::{traits::Hash, DispatchError};
use std::{
cell::RefCell,
collections::hash_map::{Entry, HashMap},
rc::Rc,
};
use std::{cell::RefCell, collections::hash_map::HashMap, rc::Rc};

type System = frame_system::Pallet<Test>;

Expand Down Expand Up @@ -1635,7 +1649,6 @@ mod tests {
func_type: ExportedFunction,
code_hash: CodeHash<Test>,
code_info: CodeInfo<Test>,
refcount: u64,
}

#[derive(Default, Clone)]
Expand Down Expand Up @@ -1664,37 +1677,11 @@ mod tests {
func_type,
code_hash: hash,
code_info: CodeInfo::<Test>::new(ALICE),
refcount: 1,
},
);
hash
})
}

fn increment_refcount(code_hash: CodeHash<Test>) -> Result<(), DispatchError> {
Loader::mutate(|loader| {
match loader.map.entry(code_hash) {
Entry::Vacant(_) => Err(<Error<Test>>::CodeNotFound)?,
Entry::Occupied(mut entry) => entry.get_mut().refcount += 1,
}
Ok(())
})
}

fn decrement_refcount(code_hash: CodeHash<Test>) {
use std::collections::hash_map::Entry::Occupied;
Loader::mutate(|loader| {
let mut entry = match loader.map.entry(code_hash) {
Occupied(e) => e,
_ => panic!("code_hash does not exist"),
};
let refcount = &mut entry.get_mut().refcount;
*refcount -= 1;
if *refcount == 0 {
entry.remove();
}
});
}
}

impl Executable<Test> for MockExecutable {
Expand All @@ -1707,22 +1694,14 @@ mod tests {
})
}

fn increment_refcount(code_hash: CodeHash<Test>) -> Result<(), DispatchError> {
MockLoader::increment_refcount(code_hash)
}

fn decrement_refcount(code_hash: CodeHash<Test>) {
MockLoader::decrement_refcount(code_hash);
}

fn execute<E: Ext<T = Test>>(
self,
ext: &mut E,
function: &ExportedFunction,
input_data: Vec<u8>,
) -> ExecResult {
if let &Constructor = function {
Self::increment_refcount(self.code_hash).unwrap();
E::increment_refcount(self.code_hash).unwrap();
}
// # Safety
//
Expand All @@ -1733,7 +1712,7 @@ mod tests {
// The transmute is necessary because `execute` has to be generic over all
// `E: Ext`. However, `MockExecutable` can't be generic over `E` as it would
// constitute a cycle.
let ext = unsafe { std::mem::transmute(ext) };
let ext = unsafe { mem::transmute(ext) };
if function == &self.func_type {
(self.func)(MockCtx { ext, input_data }, &self)
} else {
Expand Down
8 changes: 5 additions & 3 deletions substrate/frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ pub mod weights;
#[cfg(test)]
mod tests;
use crate::{
exec::{AccountIdOf, ErrorOrigin, ExecError, Executable, Key, MomentOf, Stack as ExecStack},
exec::{
AccountIdOf, ErrorOrigin, ExecError, Executable, Ext, Key, MomentOf, Stack as ExecStack,
},
gas::GasMeter,
storage::{meter::Meter as StorageMeter, ContractInfo, DeletionQueueManager},
wasm::{CodeInfo, WasmBlob},
Expand Down Expand Up @@ -658,8 +660,8 @@ pub mod pallet {
} else {
return Err(<Error<T>>::ContractNotFound.into())
};
<WasmBlob<T>>::increment_refcount(code_hash)?;
<WasmBlob<T>>::decrement_refcount(contract.code_hash);
<ExecStack<T, WasmBlob<T>>>::increment_refcount(code_hash)?;
<ExecStack<T, WasmBlob<T>>>::decrement_refcount(contract.code_hash);
Self::deposit_event(
vec![T::Hashing::hash_of(&dest), code_hash, contract.code_hash],
Event::ContractCodeUpdated {
Expand Down
21 changes: 21 additions & 0 deletions substrate/frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,27 @@ fn deposit_event_max_value_limit() {
});
}

// Fail out of fuel (ref_time weight) inside the start function.
#[test]
fn run_out_of_fuel_start_fun() {
let (wasm, _code_hash) = compile_module::<Test>("run_out_of_gas_start_fn").unwrap();
ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
let _ = <Test as Config>::Currency::set_balance(&ALICE, 1_000_000);
assert_err_ignore_postinfo!(
Contracts::instantiate_with_code(
RuntimeOrigin::signed(ALICE),
0,
Weight::from_parts(1_000_000_000_000, u64::MAX),
None,
wasm,
vec![],
vec![],
),
Error::<Test>::OutOfGas,
);
});
}

// Fail out of fuel (ref_time weight) in the engine.
#[test]
fn run_out_of_fuel_engine() {
Expand Down
Loading