Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@smiasojed
smiasojed committed May 15, 2024
1 parent 03a77bb commit 08b4ca7
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 56 deletions.
14 changes: 7 additions & 7 deletions substrate/frame/contracts/proc-macro/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,15 @@ impl HostFn {

// process attributes
let msg =
"only #[version(<u8>)], #[unstable], #[prefixed_alias], #[deprecated] and #[mutable] attributes are allowed.";
"only #[version(<u8>)], #[unstable], #[prefixed_alias], #[deprecated] and #[mutating] attributes are allowed.";
let span = item.span();
let mut attrs = item.attrs.clone();
attrs.retain(|a| !a.path().is_ident("doc"));
let mut maybe_version = None;
let mut is_stable = true;
let mut alias_to = None;
let mut not_deprecated = true;
let mut mutable = false;
let mut mutating = false;
while let Some(attr) = attrs.pop() {
let ident = attr.path().get_ident().ok_or(err(span, msg))?.to_string();
match ident.as_str() {
Expand Down Expand Up @@ -207,17 +207,17 @@ impl HostFn {
}
not_deprecated = false;
},
"mutable" => {
if mutable {
return Err(err(span, "#[mutable] can only be specified once"))
"mutating" => {
if mutating {
return Err(err(span, "#[mutating] can only be specified once"))
}
mutable = true;
mutating = true;
},
_ => return Err(err(span, msg)),
}
}

if mutable {
if mutating {
let stmt = syn::parse_quote! {
if ctx.ext().is_read_only() {
return Err(Error::<E::T>::StateChangeDenied.into());
Expand Down
29 changes: 13 additions & 16 deletions substrate/frame/contracts/src/exec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -993,27 +993,24 @@ where
Event::Instantiated { deployer: caller, contract: account_id.clone() },
);
},
(ExportedFunction::Call, Some(code_hash)) =>
if !self.is_read_only() {
Contracts::<T>::deposit_event(
vec![T::Hashing::hash_of(account_id), T::Hashing::hash_of(&code_hash)],
Event::DelegateCalled { contract: account_id.clone(), code_hash },
);
},
(ExportedFunction::Call, Some(code_hash)) => {
Contracts::<T>::deposit_event(
vec![T::Hashing::hash_of(account_id), T::Hashing::hash_of(&code_hash)],
Event::DelegateCalled { contract: account_id.clone(), code_hash },
);
},
(ExportedFunction::Call, None) => {
// If a special limit was set for the sub-call, we enforce it here.
// The sub-call will be rolled back in case the limit is exhausted.
let frame = self.top_frame_mut();
let contract = frame.contract_info.as_contract();
frame.nested_storage.enforce_subcall_limit(contract)?;

if !self.is_read_only() {
let caller = self.caller();
Contracts::<T>::deposit_event(
vec![T::Hashing::hash_of(&caller), T::Hashing::hash_of(&account_id)],
Event::Called { caller: caller.clone(), contract: account_id.clone() },
);
}
let caller = self.caller();
Contracts::<T>::deposit_event(
vec![T::Hashing::hash_of(&caller), T::Hashing::hash_of(&account_id)],
Event::Called { caller: caller.clone(), contract: account_id.clone() },
);
},
}

Expand Down Expand Up @@ -1241,7 +1238,7 @@ where
}

// If the call value is non-zero and state change is not allowed, issue an error.
if !value.is_zero() && frame_read_only {
if !value.is_zero() && read_only {
return Err(<Error<T>>::StateChangeDenied.into());
}
// We ignore instantiate frames in our search for a cached contract.
Expand All @@ -1259,7 +1256,7 @@ where
value,
gas_limit,
deposit_limit,
frame_read_only,
read_only,
)?;
self.run(executable, input_data)
};
Expand Down
14 changes: 0 additions & 14 deletions substrate/frame/contracts/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4342,20 +4342,6 @@ fn read_only_call_works() {
let addr_callee =
builder::bare_instantiate(Code::Upload(wasm_callee)).build_and_unwrap_account_id();

// Drop previous events.
initialize_block(2);
assert_ok!(builder::call(addr_caller.clone()).data(addr_callee.encode()).build());

assert_eq!(
System::events(),
vec![EventRecord {
phase: Phase::Initialization,
event: RuntimeEvent::Contracts(crate::Event::Called {
caller: Origin::from_account_id(ALICE),
contract: addr_caller.clone(),
}),
topics: vec![hash(&Origin::<Test>::from_account_id(ALICE)), hash(&addr_caller)],
},]
);
});
}
38 changes: 19 additions & 19 deletions substrate/frame/contracts/src/wasm/runtime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ pub mod env {
/// Set the value at the given key in the contract storage.
/// See [`pallet_contracts_uapi::HostFn::set_storage`]
#[prefixed_alias]
#[mutable]
#[mutating]
fn set_storage(
ctx: _,
memory: _,
Expand All @@ -978,7 +978,7 @@ pub mod env {
/// See [`pallet_contracts_uapi::HostFn::set_storage_v1`]
#[version(1)]
#[prefixed_alias]
#[mutable]
#[mutating]
fn set_storage(
ctx: _,
memory: _,
Expand All @@ -993,7 +993,7 @@ pub mod env {
/// See [`pallet_contracts_uapi::HostFn::set_storage_v2`]
#[version(2)]
#[prefixed_alias]
#[mutable]
#[mutating]
fn set_storage(
ctx: _,
memory: _,
Expand All @@ -1008,7 +1008,7 @@ pub mod env {
/// Clear the value at the given key in the contract storage.
/// See [`pallet_contracts_uapi::HostFn::clear_storage`]
#[prefixed_alias]
#[mutable]
#[mutating]
fn clear_storage(ctx: _, memory: _, key_ptr: u32) -> Result<(), TrapReason> {
ctx.clear_storage(memory, KeyType::Fix, key_ptr).map(|_| ())
}
Expand All @@ -1017,7 +1017,7 @@ pub mod env {
/// See [`pallet_contracts_uapi::HostFn::clear_storage_v1`]
#[version(1)]
#[prefixed_alias]
#[mutable]
#[mutating]
fn clear_storage(ctx: _, memory: _, key_ptr: u32, key_len: u32) -> Result<u32, TrapReason> {
ctx.clear_storage(memory, KeyType::Var(key_len), key_ptr)
}
Expand Down Expand Up @@ -1068,7 +1068,7 @@ pub mod env {
/// Retrieve and remove the value under the given key from storage.
/// See [`pallet_contracts_uapi::HostFn::take_storage`]
#[prefixed_alias]
#[mutable]
#[mutating]
fn take_storage(
ctx: _,
memory: _,
Expand Down Expand Up @@ -1100,7 +1100,7 @@ pub mod env {
/// Transfer some value to another account.
/// See [`pallet_contracts_uapi::HostFn::transfer`].
#[prefixed_alias]
#[mutable]
#[mutating]
fn transfer(
ctx: _,
memory: _,
Expand Down Expand Up @@ -1258,7 +1258,7 @@ pub mod env {
/// of those types are fixed through [`codec::MaxEncodedLen`]. The fields exist
/// for backwards compatibility. Consider switching to the newest version of this function.
#[prefixed_alias]
#[mutable]
#[mutating]
fn instantiate(
ctx: _,
memory: _,
Expand Down Expand Up @@ -1297,7 +1297,7 @@ pub mod env {
/// See [`pallet_contracts_uapi::HostFn::instantiate_v1`].
#[version(1)]
#[prefixed_alias]
#[mutable]
#[mutating]
fn instantiate(
ctx: _,
memory: _,
Expand Down Expand Up @@ -1333,7 +1333,7 @@ pub mod env {
/// Instantiate a contract with the specified code hash.
/// See [`pallet_contracts_uapi::HostFn::instantiate_v2`].
#[version(2)]
#[mutable]
#[mutating]
fn instantiate(
ctx: _,
memory: _,
Expand Down Expand Up @@ -1377,7 +1377,7 @@ pub mod env {
/// this type is fixed through `[`MaxEncodedLen`]. The field exist for backwards
/// compatibility. Consider switching to the newest version of this function.
#[prefixed_alias]
#[mutable]
#[mutating]
fn terminate(
ctx: _,
memory: _,
Expand All @@ -1391,7 +1391,7 @@ pub mod env {
/// See [`pallet_contracts_uapi::HostFn::terminate_v1`].
#[version(1)]
#[prefixed_alias]
#[mutable]
#[mutating]
fn terminate(ctx: _, memory: _, beneficiary_ptr: u32) -> Result<(), TrapReason> {
ctx.terminate(memory, beneficiary_ptr)
}
Expand Down Expand Up @@ -1890,7 +1890,7 @@ pub mod env {
/// Deposit a contract event with the data buffer and optional list of topics.
/// See [pallet_contracts_uapi::HostFn::deposit_event]
#[prefixed_alias]
#[mutable]
#[mutating]
fn deposit_event(
ctx: _,
memory: _,
Expand Down Expand Up @@ -2071,7 +2071,7 @@ pub mod env {

/// Call some dispatchable of the runtime.
/// See [`frame_support::traits::call_runtime`].
#[mutable]
#[mutating]
fn call_runtime(
ctx: _,
memory: _,
Expand All @@ -2091,7 +2091,7 @@ pub mod env {

/// Execute an XCM program locally, using the contract's address as the origin.
/// See [`pallet_contracts_uapi::HostFn::execute_xcm`].
#[mutable]
#[mutating]
fn xcm_execute(
ctx: _,
memory: _,
Expand Down Expand Up @@ -2129,7 +2129,7 @@ pub mod env {

/// Send an XCM program from the contract to the specified destination.
/// See [`pallet_contracts_uapi::HostFn::send_xcm`].
#[mutable]
#[mutating]
fn xcm_send(
ctx: _,
memory: _,
Expand Down Expand Up @@ -2226,7 +2226,7 @@ pub mod env {
/// Replace the contract code at the specified address with new code.
/// See [`pallet_contracts_uapi::HostFn::set_code_hash`].
#[prefixed_alias]
#[mutable]
#[mutating]
fn set_code_hash(ctx: _, memory: _, code_hash_ptr: u32) -> Result<ReturnErrorCode, TrapReason> {
ctx.charge_gas(RuntimeCosts::SetCodeHash)?;
let code_hash: CodeHash<<E as Ext>::T> =
Expand Down Expand Up @@ -2291,7 +2291,7 @@ pub mod env {

/// Adds a new delegate dependency to the contract.
/// See [`pallet_contracts_uapi::HostFn::lock_delegate_dependency`].
#[mutable]
#[mutating]
fn lock_delegate_dependency(ctx: _, memory: _, code_hash_ptr: u32) -> Result<(), TrapReason> {
ctx.charge_gas(RuntimeCosts::LockDelegateDependency)?;
let code_hash = ctx.read_sandbox_memory_as(memory, code_hash_ptr)?;
Expand All @@ -2301,7 +2301,7 @@ pub mod env {

/// Removes the delegate dependency from the contract.
/// see [`pallet_contracts_uapi::HostFn::unlock_delegate_dependency`].
#[mutable]
#[mutating]
fn unlock_delegate_dependency(ctx: _, memory: _, code_hash_ptr: u32) -> Result<(), TrapReason> {
ctx.charge_gas(RuntimeCosts::UnlockDelegateDependency)?;
let code_hash = ctx.read_sandbox_memory_as(memory, code_hash_ptr)?;
Expand Down

0 comments on commit 08b4ca7

Please sign in to comment.