diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 07deb750f93df..cc53c2c616a82 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1192,6 +1192,7 @@ impl pallet_revive::Config for Runtime { type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12. type EthGasEncoder = (); type FindAuthor = ::FindAuthor; + type DepositSource = (); } parameter_types! { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs index 52bb4a74cdb6d..ebc1846246923 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs @@ -57,7 +57,7 @@ use frame_support::{ use hex_literal::hex; use pallet_revive::{ test_utils::builder::{BareInstantiateBuilder, Contract}, - Code, DepositLimit, + Code, }; use pallet_revive_fixtures::compile_module; use pallet_uniques::{asset_ops::Item, asset_strategies::Attribute}; @@ -1689,7 +1689,7 @@ fn withdraw_and_deposit_erc20s() { let constructor_data = sol_data::Uint::<256>::abi_encode(&initial_amount_u256); let Contract { addr: erc20_address, .. } = bare_instantiate(&sender, code) .gas_limit(Weight::from_parts(2_000_000_000, 200_000)) - .storage_deposit_limit(DepositLimit::Balance(Balance::MAX)) + .storage_deposit_limit(Balance::MAX) .data(constructor_data) .build_and_unwrap_contract(); @@ -1802,7 +1802,7 @@ fn smart_contract_not_erc20_will_error() { let Contract { addr: non_erc20_address, .. } = bare_instantiate(&sender, code) .gas_limit(Weight::from_parts(2_000_000_000, 200_000)) - .storage_deposit_limit(DepositLimit::Balance(Balance::MAX)) + .storage_deposit_limit(Balance::MAX) .build_and_unwrap_contract(); let wnd_amount_for_fees = 1_000_000_000_000u128; @@ -1860,7 +1860,7 @@ fn smart_contract_does_not_return_bool_fails() { let Contract { addr: non_erc20_address, .. } = bare_instantiate(&sender, code) .gas_limit(Weight::from_parts(2_000_000_000, 200_000)) - .storage_deposit_limit(DepositLimit::Balance(Balance::MAX)) + .storage_deposit_limit(Balance::MAX) .data(constructor_data) .build_and_unwrap_contract(); @@ -1916,7 +1916,7 @@ fn expensive_erc20_runs_out_of_gas() { let constructor_data = sol_data::Uint::<256>::abi_encode(&initial_amount_u256); let Contract { addr: non_erc20_address, .. } = bare_instantiate(&sender, code) .gas_limit(Weight::from_parts(2_000_000_000, 200_000)) - .storage_deposit_limit(DepositLimit::Balance(Balance::MAX)) + .storage_deposit_limit(Balance::MAX) .data(constructor_data) .build_and_unwrap_contract(); diff --git a/cumulus/parachains/runtimes/assets/common/src/erc20_transactor.rs b/cumulus/parachains/runtimes/assets/common/src/erc20_transactor.rs index 29e40d1ab1cf8..4729bf9570259 100644 --- a/cumulus/parachains/runtimes/assets/common/src/erc20_transactor.rs +++ b/cumulus/parachains/runtimes/assets/common/src/erc20_transactor.rs @@ -24,7 +24,7 @@ use pallet_revive::{ primitives::{Address, U256 as EU256}, sol_types::SolCall, }, - AddressMapper, ContractResult, DepositLimit, MomentOf, + AddressMapper, ContractResult, ExecConfig, MomentOf, }; use sp_core::{Get, H160, H256, U256}; use sp_runtime::Weight; @@ -126,8 +126,9 @@ where asset_id, U256::zero(), gas_limit, - DepositLimit::Balance(StorageDepositLimit::get()), + StorageDepositLimit::get(), data, + ExecConfig::new_substrate_tx(), ); // We need to return this surplus for the executor to allow refunding it. let surplus = gas_limit.saturating_sub(gas_consumed); @@ -184,8 +185,9 @@ where asset_id, U256::zero(), gas_limit, - DepositLimit::Balance(StorageDepositLimit::get()), + StorageDepositLimit::get(), data, + ExecConfig::new_substrate_tx(), ); // We need to return this surplus for the executor to allow refunding it. let surplus = gas_limit.saturating_sub(gas_consumed); diff --git a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs index a56b59b4a47c7..6d3898696d567 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs @@ -832,6 +832,7 @@ impl pallet_revive::Config for Runtime { type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12. type EthGasEncoder = (); type FindAuthor = ::FindAuthor; + type DepositSource = (); } impl pallet_sudo::Config for Runtime { diff --git a/polkadot/xcm/pallet-xcm/src/precompiles.rs b/polkadot/xcm/pallet-xcm/src/precompiles.rs index b673a1183055e..79a06e33259c5 100644 --- a/polkadot/xcm/pallet-xcm/src/precompiles.rs +++ b/polkadot/xcm/pallet-xcm/src/precompiles.rs @@ -184,7 +184,7 @@ mod test { }, H160, }, - DepositLimit, U256, + ExecConfig, U256, }; use polkadot_parachain_primitives::primitives::Id as ParaId; use sp_runtime::traits::AccountIdConversion; @@ -231,8 +231,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); assert!(result.result.is_ok()); let sent_message = Xcm(Some(DescendOrigin(sender.clone().try_into().unwrap())) @@ -279,8 +280,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); assert!(result.result.is_ok()); @@ -327,8 +329,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { Ok(value) => value, @@ -376,8 +379,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { Ok(value) => value, @@ -402,8 +406,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { Ok(value) => value, @@ -451,8 +456,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { Ok(value) => value, @@ -478,8 +484,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { Ok(value) => value, @@ -520,8 +527,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_weight_call, + ExecConfig::new_substrate_tx(), ); let weight_result = match xcm_weight_results.result { @@ -542,8 +550,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); assert!(result.result.is_ok()); @@ -580,8 +589,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_weight_call, + ExecConfig::new_substrate_tx(), ); let weight_result = match xcm_weight_results.result { @@ -602,8 +612,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { @@ -648,8 +659,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_weight_call, + ExecConfig::new_substrate_tx(), ); let weight_result = match xcm_weight_results.result { @@ -670,8 +682,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { Ok(value) => value, @@ -715,8 +728,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_weight_call, + ExecConfig::new_substrate_tx(), ); let weight_result = match xcm_weight_results.result { @@ -744,8 +758,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { @@ -770,8 +785,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_call, + ExecConfig::new_substrate_tx(), ); let return_value = match result.result { @@ -818,8 +834,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_weight_call, + ExecConfig::new_substrate_tx(), ); let result = match xcm_weight_results.result { @@ -842,8 +859,9 @@ mod test { xcm_precompile_addr, U256::zero(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u128::MAX, encoded_weight_call, + ExecConfig::new_substrate_tx(), ); let result = match xcm_weight_results.result { diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 8522d10d2522b..62a632a592971 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1492,6 +1492,7 @@ impl pallet_revive::Config for Runtime { type EthGasEncoder = (); type FindAuthor = ::FindAuthor; type AllowEVMBytecode = ConstBool; + type DepositSource = (); } impl pallet_sudo::Config for Runtime { diff --git a/substrate/frame/assets/src/precompiles.rs b/substrate/frame/assets/src/precompiles.rs index 107145a524d81..d81624e2df263 100644 --- a/substrate/frame/assets/src/precompiles.rs +++ b/substrate/frame/assets/src/precompiles.rs @@ -316,7 +316,7 @@ mod test { }; use alloy::primitives::U256; use frame_support::{assert_ok, traits::Currency}; - use pallet_revive::DepositLimit; + use pallet_revive::ExecConfig; use sp_core::H160; use sp_runtime::Weight; @@ -371,8 +371,9 @@ mod test { H160::from(asset_addr), 0u32.into(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u64::MAX, data, + ExecConfig::new_substrate_tx(), ); assert_contract_event( @@ -407,8 +408,9 @@ mod test { H160::from(asset_addr), 0u32.into(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u64::MAX, data, + ExecConfig::new_substrate_tx(), ) .result .unwrap() @@ -438,8 +440,9 @@ mod test { H160::from(asset_addr), 0u32.into(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u64::MAX, data, + ExecConfig::new_substrate_tx(), ) .result .unwrap() @@ -484,8 +487,9 @@ mod test { H160::from(asset_addr), 0u32.into(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u64::MAX, data, + ExecConfig::new_substrate_tx(), ); assert_contract_event( @@ -508,8 +512,9 @@ mod test { H160::from(asset_addr), 0u32.into(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u64::MAX, data, + ExecConfig::new_substrate_tx(), ) .result .unwrap() @@ -530,8 +535,9 @@ mod test { H160::from(asset_addr), 0u32.into(), Weight::MAX, - DepositLimit::UnsafeOnlyForDryRun, + u64::MAX, data, + ExecConfig::new_substrate_tx(), ); assert_eq!(Assets::balance(asset_id, owner), 90); assert_eq!(Assets::allowance(asset_id, &owner, &spender), 15); diff --git a/substrate/frame/revive/src/benchmarking.rs b/substrate/frame/revive/src/benchmarking.rs index 2505aecd8abcb..69293bdf96e77 100644 --- a/substrate/frame/revive/src/benchmarking.rs +++ b/substrate/frame/revive/src/benchmarking.rs @@ -42,7 +42,7 @@ use frame_support::{ self, assert_ok, migrations::SteppedMigration, storage::child, - traits::fungible::InspectHold, + traits::fungible::{InspectHold, UnbalancedHold}, weights::{Weight, WeightMeter}, }; use frame_system::RawOrigin; @@ -286,7 +286,6 @@ mod benchmarks { let dust = 42u32 * d; let evm_value = Pallet::::convert_native_to_evm(BalanceWithDust::new_unchecked::(value, dust)); - let caller = whitelisted_caller(); T::Currency::set_balance(&caller, caller_funding::()); let VmBinaryModule { code, .. } = VmBinaryModule::sized(c); @@ -300,6 +299,11 @@ mod benchmarks { assert!(AccountInfoOf::::get(&deployer).is_none()); + let hold_reason = T::DepositSource::get(); + if let Some(hold_reason) = hold_reason.as_ref() { + T::Currency::set_balance_on_hold(hold_reason, &caller, caller_funding::()).unwrap(); + } + #[extrinsic_call] _(origin, evm_value, Weight::MAX, storage_deposit, code, input); @@ -313,16 +317,29 @@ mod benchmarks { let mapping_deposit = T::Currency::balance_on_hold(&HoldReason::AddressMapping.into(), &caller); - assert_eq!( - Pallet::::evm_balance(&deployer), - Pallet::::convert_native_to_evm( - caller_funding::() - - Pallet::::min_balance() - - Pallet::::min_balance() - - value - deposit - code_deposit - - mapping_deposit, - ) - dust, - ); + if let Some(hold_reason) = hold_reason.as_ref() { + assert_eq!( + T::Currency::balance_on_hold(&hold_reason, &caller), + caller_funding::() - deposit - code_deposit - Pallet::::min_balance(), + ); + assert_eq!( + Pallet::::evm_balance(&deployer), + Pallet::::convert_native_to_evm( + caller_funding::() - Pallet::::min_balance() - value - mapping_deposit, + ) - dust, + ); + } else { + assert_eq!( + Pallet::::evm_balance(&deployer), + Pallet::::convert_native_to_evm( + caller_funding::() - + Pallet::::min_balance() - + Pallet::::min_balance() - + value - mapping_deposit - + deposit - code_deposit, + ) - dust, + ) + } // contract has the full value assert_eq!(Pallet::::evm_balance(&addr), evm_value); diff --git a/substrate/frame/revive/src/call_builder.rs b/substrate/frame/revive/src/call_builder.rs index 6d6ca9348fdfc..5a09dde45e2cf 100644 --- a/substrate/frame/revive/src/call_builder.rs +++ b/substrate/frame/revive/src/call_builder.rs @@ -32,9 +32,8 @@ use crate::{ storage::meter::Meter, transient_storage::MeterEntry, vm::pvm::{PreparedCall, Runtime}, - AccountInfo, BalanceOf, BalanceWithDust, BumpNonce, Code, CodeInfoOf, Config, ContractBlob, - ContractInfo, DepositLimit, Error, GasMeter, MomentOf, Origin, Pallet as Contracts, - PristineCode, Weight, + AccountInfo, BalanceOf, BalanceWithDust, Code, CodeInfoOf, Config, ContractBlob, ContractInfo, + Error, ExecConfig, GasMeter, MomentOf, Origin, Pallet as Contracts, PristineCode, Weight, }; use alloc::{vec, vec::Vec}; use frame_support::{storage::child, traits::fungible::Mutate}; @@ -56,6 +55,7 @@ pub struct CallSetup { value: BalanceOf, data: Vec, transient_storage_size: u32, + exec_config: ExecConfig, } impl Default for CallSetup @@ -111,6 +111,7 @@ where value: 0u32.into(), data: vec![], transient_storage_size: 0, + exec_config: ExecConfig::new_substrate_tx(), } } @@ -157,6 +158,7 @@ where &mut self.gas_meter, &mut self.storage_meter, self.value, + &self.exec_config, ); if self.transient_storage_size > 0 { Self::with_transient_storage(&mut ext.0, self.transient_storage_size).unwrap(); @@ -266,11 +268,11 @@ where origin, U256::zero(), Weight::MAX, - DepositLimit::Balance(default_deposit_limit::()), + default_deposit_limit::(), Code::Upload(module.code), data, salt, - BumpNonce::Yes, + ExecConfig::new_substrate_tx(), ); let address = outcome.result?.addr; diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index e0f190b60051d..9444e55cfb0f0 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -20,7 +20,7 @@ use crate::{ gas::GasMeter, limits, precompiles::{All as AllPrecompiles, Instance as PrecompileInstance, Precompiles}, - primitives::{BumpNonce, ExecReturnValue, StorageDeposit}, + primitives::{ExecConfig, ExecReturnValue, StorageDeposit}, runtime_decl_for_revive_api::{Decode, Encode, RuntimeDebugNoBound, TypeInfo}, storage::{self, meter::Diff, AccountIdOrAddress, WriteOutcome}, tracing::if_tracing, @@ -528,9 +528,8 @@ pub struct Stack<'a, T: Config, E> { first_frame: Frame, /// Transient storage used to store data, which is kept for the duration of a transaction. transient_storage: TransientStorage, - /// Whether or not actual transfer of funds should be performed. - /// This is set to `true` exclusively when we simulate a call through eth_transact. - skip_transfer: bool, + /// Global behavior determined by the creater of this stack. + exec_config: &'a ExecConfig, /// No executable is held by the struct but influences its behaviour. _phantom: PhantomData, } @@ -783,7 +782,7 @@ where storage_meter: &mut storage::meter::Meter, value: U256, input_data: Vec, - skip_transfer: bool, + exec_config: &ExecConfig, ) -> ExecResult { let dest = T::AddressMapper::to_account_id(&dest); if let Some((mut stack, executable)) = Stack::<'_, T, E>::new( @@ -792,11 +791,9 @@ where gas_meter, storage_meter, value, - skip_transfer, + exec_config, )? { - stack - .run(executable, input_data, BumpNonce::Yes) - .map(|_| stack.first_frame.last_frame_output) + stack.run(executable, input_data).map(|_| stack.first_frame.last_frame_output) } else { if_tracing(|t| { t.enter_child_span( @@ -834,8 +831,7 @@ where value: U256, input_data: Vec, salt: Option<&[u8; 32]>, - skip_transfer: bool, - bump_nonce: BumpNonce, + exec_config: &ExecConfig, ) -> Result<(H160, ExecReturnValue), ExecError> { let deployer = T::AddressMapper::to_address(&origin); let (mut stack, executable) = Stack::<'_, T, E>::new( @@ -849,12 +845,12 @@ where gas_meter, storage_meter, value, - skip_transfer, + exec_config, )? .expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE); let address = T::AddressMapper::to_address(&stack.top_frame().account_id); let result = stack - .run(executable, input_data, bump_nonce) + .run(executable, input_data) .map(|_| (address, stack.first_frame.last_frame_output)); if let Ok((contract, ref output)) = result { if !output.did_revert() { @@ -871,6 +867,7 @@ where gas_meter: &'a mut GasMeter, storage_meter: &'a mut storage::meter::Meter, value: BalanceOf, + exec_config: &'a ExecConfig, ) -> (Self, E) { let call = Self::new( FrameArgs::Call { @@ -882,7 +879,7 @@ where gas_meter, storage_meter, value.into(), - false, + exec_config, ) .unwrap() .unwrap(); @@ -899,7 +896,7 @@ where gas_meter: &'a mut GasMeter, storage_meter: &'a mut storage::meter::Meter, value: U256, - skip_transfer: bool, + exec_config: &'a ExecConfig, ) -> Result)>, ExecError> { origin.ensure_mapped()?; let Some((first_frame, executable)) = Self::new_frame( @@ -925,7 +922,7 @@ where first_frame, frames: Default::default(), transient_storage: TransientStorage::new(limits::TRANSIENT_STORAGE_BYTES), - skip_transfer, + exec_config, _phantom: Default::default(), }; @@ -1112,7 +1109,6 @@ where &mut self, executable: ExecutableOrPrecompile, input_data: Vec, - bump_nonce: BumpNonce, ) -> Result<(), ExecError> { let frame = self.top_frame(); let entry_point = frame.entry_point; @@ -1146,7 +1142,9 @@ where let do_transaction = || -> ExecResult { let caller = self.caller(); - let skip_transfer = self.skip_transfer; + let skip_transfer = self.exec_config.unsafe_skip_transfers; + let is_first_frame = self.frames.len() == 0; + let bump_nonce = self.exec_config.bump_nonce; let frame = top_frame_mut!(self); let account_id = &frame.account_id.clone(); @@ -1166,10 +1164,10 @@ where let ed = >::min_balance(); frame.nested_storage.record_charge(&StorageDeposit::Charge(ed))?; - if self.skip_transfer { + if skip_transfer { T::Currency::set_balance(account_id, ed); } else { - T::Currency::transfer(origin, account_id, ed, Preservation::Preserve) + >::charge_deposit(None, origin, account_id, ed, self.exec_config) .map_err(|_| >::StorageDepositNotEnoughFunds)?; } @@ -1182,7 +1180,7 @@ where // Contracts nonce starts at 1 >::inc_account_nonce(account_id); - if matches!(bump_nonce, BumpNonce::Yes) { + if bump_nonce || !is_first_frame { // Needs to be incremented before calling into the code so that it is visible // in case of recursion. >::inc_account_nonce(caller.account_id()?); @@ -1257,7 +1255,7 @@ where return Ok(output); } - let frame = self.top_frame_mut(); + let frame = top_frame_mut!(self); // The deposit we charge for a contract depends on the size of the immutable data. // Hence we need to delay charging the base deposit after execution. @@ -1277,7 +1275,7 @@ where data, caller.account_id()?.clone(), )?; - module.store_code(skip_transfer)?; + module.store_code(&self.exec_config)?; code_deposit = module.code_info().deposit(); contract_info.code_hash = *module.code_hash(); @@ -1675,7 +1673,7 @@ where deposit_limit.saturated_into::>(), self.is_read_only(), )? { - self.run(executable, input_data, BumpNonce::Yes) + self.run(executable, input_data) } else { // Delegate-calls to non-contract accounts are considered success. Ok(()) @@ -1849,7 +1847,7 @@ where let executable = executable.expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE); let address = T::AddressMapper::to_address(&self.top_frame().account_id); if_tracing(|t| t.instantiate_code(&code, salt)); - self.run(executable, input_data, BumpNonce::Yes).map(|_| address) + self.run(executable, input_data).map(|_| address) } } @@ -1915,7 +1913,7 @@ where deposit_limit.saturated_into::>(), is_read_only, )? { - self.run(executable, input_data, BumpNonce::Yes) + self.run(executable, input_data) } else { if_tracing(|t| { t.enter_child_span( diff --git a/substrate/frame/revive/src/exec/tests.rs b/substrate/frame/revive/src/exec/tests.rs index 163aa1cbf3258..73373d82a07ec 100644 --- a/substrate/frame/revive/src/exec/tests.rs +++ b/substrate/frame/revive/src/exec/tests.rs @@ -225,7 +225,7 @@ fn it_works() { &mut storage_meter, value.into(), vec![], - false, + &ExecConfig::new_substrate_tx(), ), Ok(_) ); @@ -260,7 +260,9 @@ fn transfer_works() { assert_eq!(get_balance(&ALICE), 100 - value - min_balance); assert_eq!(get_balance(&BOB), min_balance + value); assert_eq!( - storage_meter.try_into_deposit(&Origin::from_account_id(ALICE), false).unwrap(), + storage_meter + .try_into_deposit(&Origin::from_account_id(ALICE), &ExecConfig::new_substrate_tx()) + .unwrap(), StorageDeposit::Charge(min_balance) ); }); @@ -348,7 +350,7 @@ fn correct_transfer_on_call() { &mut storage_meter, evm_value.as_u64().into(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap(); @@ -388,7 +390,7 @@ fn correct_transfer_on_delegate_call() { &mut storage_meter, evm_value.as_u64().into(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); assert_eq!(get_balance(&ALICE), 100 - value); @@ -422,7 +424,7 @@ fn delegate_call_missing_contract() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); // add missing contract code @@ -434,7 +436,7 @@ fn delegate_call_missing_contract() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -462,7 +464,7 @@ fn changes_are_reverted_on_failing_call() { &mut storage_meter, 55u64.into(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap(); @@ -520,7 +522,7 @@ fn output_is_returned_on_success() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); let output = result.unwrap(); @@ -549,7 +551,7 @@ fn output_is_returned_on_failure() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); let output = result.unwrap(); @@ -578,7 +580,7 @@ fn input_data_to_call() { &mut storage_meter, U256::zero(), vec![1, 2, 3, 4], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -610,8 +612,7 @@ fn input_data_to_instantiate() { min_balance.into(), vec![1, 2, 3, 4], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -665,7 +666,7 @@ fn max_depth() { &mut storage_meter, value.into(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); @@ -727,7 +728,7 @@ fn caller_returns_proper_values() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); @@ -790,7 +791,7 @@ fn origin_returns_proper_values() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); @@ -831,7 +832,7 @@ fn code_hash_returns_proper_values() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -857,7 +858,7 @@ fn own_code_hash_returns_proper_values() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -893,7 +894,7 @@ fn caller_is_origin_returns_proper_values() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -919,7 +920,7 @@ fn root_caller_succeeds() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -945,7 +946,7 @@ fn root_caller_does_not_succeed_when_value_not_zero() { &mut storage_meter, 1u64.into(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Err(_)); }); @@ -981,7 +982,7 @@ fn root_caller_succeeds_with_consecutive_calls() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -1026,7 +1027,7 @@ fn address_returns_proper_values() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); @@ -1051,8 +1052,7 @@ fn refuse_instantiate_with_value_below_existential_deposit() { U256::zero(), // <- zero value vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ), Err(_) ); @@ -1085,8 +1085,7 @@ fn instantiation_work_with_success_output() { Pallet::::convert_native_to_evm(min_balance), vec![], Some(&[0 ;32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ), Ok((address, ref output)) if output.data == vec![80, 65, 83, 83] => address ); @@ -1137,8 +1136,7 @@ fn instantiation_fails_with_failing_output() { Pallet::::convert_native_to_evm(min_balance), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ), Ok((address, ref output)) if output.data == vec![70, 65, 73, 76] => address ); @@ -1200,7 +1198,7 @@ fn instantiation_from_contract() { &mut storage_meter, Pallet::::convert_native_to_evm(min_balance * 10), vec![], - false, + &ExecConfig::new_substrate_tx(), ), Ok(_) ); @@ -1269,7 +1267,7 @@ fn instantiation_traps() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ), Ok(_) ); @@ -1302,8 +1300,7 @@ fn termination_from_instantiate_fails() { Pallet::::convert_native_to_evm(100u64), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ), Err(ExecError { error: Error::::TerminatedInConstructor.into(), @@ -1370,7 +1367,7 @@ fn in_memory_changes_not_discarded() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -1429,8 +1426,7 @@ fn recursive_call_during_constructor_is_balance_transfer() { 10u64.into(), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -1476,7 +1472,7 @@ fn cannot_send_more_balance_than_available_to_self() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap(); }); @@ -1508,7 +1504,7 @@ fn call_reentry_direct_recursion() { &mut storage_meter, U256::zero(), CHARLIE_ADDR.as_bytes().to_vec(), - false, + &ExecConfig::new_substrate_tx(), )); // Calling into oneself fails @@ -1520,7 +1516,7 @@ fn call_reentry_direct_recursion() { &mut storage_meter, U256::zero(), BOB_ADDR.as_bytes().to_vec(), - false, + &ExecConfig::new_substrate_tx(), ) .map_err(|e| e.error), >::ReentranceDenied, @@ -1570,7 +1566,7 @@ fn call_deny_reentry() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ) .map_err(|e| e.error), >::ReentranceDenied, @@ -1656,8 +1652,7 @@ fn nonce() { (min_balance * 100).into(), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ) .ok(); assert_eq!(System::account_nonce(&ALICE), 0); @@ -1670,8 +1665,7 @@ fn nonce() { (min_balance * 100).into(), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), )); assert_eq!(System::account_nonce(&ALICE), 1); @@ -1683,8 +1677,7 @@ fn nonce() { (min_balance * 200).into(), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), )); assert_eq!(System::account_nonce(&ALICE), 2); @@ -1696,8 +1689,7 @@ fn nonce() { (min_balance * 200).into(), vec![], Some(&[0; 32]), - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), )); assert_eq!(System::account_nonce(&ALICE), 3); }); @@ -1764,7 +1756,7 @@ fn set_storage_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -1862,7 +1854,7 @@ fn set_storage_varsized_key_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -1900,7 +1892,7 @@ fn get_storage_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -1938,7 +1930,7 @@ fn get_storage_size_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -1987,7 +1979,7 @@ fn get_storage_varsized_key_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -2036,7 +2028,7 @@ fn get_storage_size_varsized_key_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -2110,7 +2102,7 @@ fn set_transient_storage_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -2180,7 +2172,7 @@ fn get_transient_storage_works() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -2218,7 +2210,7 @@ fn get_transient_storage_size_works() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), )); }); } @@ -2280,7 +2272,7 @@ fn rollback_transient_storage_works() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -2311,7 +2303,7 @@ fn ecdsa_to_eth_address_returns_proper_value() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -2409,7 +2401,7 @@ fn last_frame_output_works_on_instantiate() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap() }); @@ -2477,7 +2469,7 @@ fn last_frame_output_works_on_nested_call() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -2545,7 +2537,7 @@ fn last_frame_output_is_always_reset() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ); assert_matches!(result, Ok(_)); }); @@ -2594,7 +2586,7 @@ fn immutable_data_access_checks_work() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap() }); @@ -2663,7 +2655,7 @@ fn correct_immutable_data_in_delegate_call() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap() }); @@ -2702,8 +2694,7 @@ fn immutable_data_set_overrides() { U256::zero(), vec![], None, - false, - BumpNonce::Yes, + &ExecConfig::new_substrate_tx(), ) .unwrap() .0; @@ -2715,7 +2706,7 @@ fn immutable_data_set_overrides() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap() }); @@ -2761,7 +2752,7 @@ fn immutable_data_set_errors_with_empty_data() { &mut storage_meter, U256::zero(), vec![], - false, + &ExecConfig::new_substrate_tx(), ) .unwrap() }); @@ -2816,7 +2807,7 @@ fn block_hash_returns_proper_values() { &mut storage_meter, U256::zero(), vec![0], - false, + &ExecConfig::new_substrate_tx(), ), Ok(_) ); diff --git a/substrate/frame/revive/src/impl_fungibles.rs b/substrate/frame/revive/src/impl_fungibles.rs index 404690c6765b4..70f0814f0e6e2 100644 --- a/substrate/frame/revive/src/impl_fungibles.rs +++ b/substrate/frame/revive/src/impl_fungibles.rs @@ -42,7 +42,7 @@ use sp_core::{H160, H256, U256}; use sp_runtime::{traits::AccountIdConversion, DispatchError}; use super::{ - address::AddressMapper, pallet, BalanceOf, Bounded, Config, ContractResult, DepositLimit, + address::AddressMapper, pallet, BalanceOf, Bounded, Config, ContractResult, ExecConfig, MomentOf, Pallet, Weight, }; use ethereum_standards::IERC20; @@ -77,10 +77,9 @@ where asset_id, U256::zero(), GAS_LIMIT, - DepositLimit::Balance( - <::Currency as fungible::Inspect<_>>::total_issuance(), - ), + <::Currency as fungible::Inspect<_>>::total_issuance(), data, + ExecConfig::new_substrate_tx(), ); if let Ok(return_value) = result { if let Ok(eu256) = EU256::abi_decode_validate(&return_value.data) { @@ -113,10 +112,9 @@ where asset_id, U256::zero(), GAS_LIMIT, - DepositLimit::Balance( - <::Currency as fungible::Inspect<_>>::total_issuance(), - ), + <::Currency as fungible::Inspect<_>>::total_issuance(), data, + ExecConfig::new_substrate_tx(), ); if let Ok(return_value) = result { if let Ok(eu256) = EU256::abi_decode_validate(&return_value.data) { @@ -188,10 +186,9 @@ where asset_id, U256::zero(), GAS_LIMIT, - DepositLimit::Balance( - <::Currency as fungible::Inspect<_>>::total_issuance(), - ), + <::Currency as fungible::Inspect<_>>::total_issuance(), data, + ExecConfig::new_substrate_tx(), ); log::trace!(target: "whatiwant", "{gas_consumed}"); if let Ok(return_value) = result { @@ -225,10 +222,9 @@ where asset_id, U256::zero(), GAS_LIMIT, - DepositLimit::Balance( - <::Currency as fungible::Inspect<_>>::total_issuance(), - ), + <::Currency as fungible::Inspect<_>>::total_issuance(), data, + ExecConfig::new_substrate_tx(), ); if let Ok(return_value) = result { if return_value.did_revert() { @@ -416,7 +412,7 @@ mod tests { RuntimeOrigin::signed(checking_account.clone()), Code::Upload(code), ) - .storage_deposit_limit((1_000_000_000_000).into()) + .storage_deposit_limit(1_000_000_000_000) .data(constructor_data) .build_and_unwrap_contract(); assert_eq!( diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 9ed4e02dee25d..569630a5691fc 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -59,8 +59,8 @@ use codec::{Codec, Decode, Encode}; use environmental::*; use frame_support::{ dispatch::{ - DispatchErrorWithPostInfo, DispatchResultWithPostInfo, GetDispatchInfo, Pays, - PostDispatchInfo, RawOrigin, + DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo, + Pays, PostDispatchInfo, RawOrigin, }, ensure, pallet_prelude::DispatchClass, @@ -278,6 +278,15 @@ pub mod pallet { /// Only valid value is `()`. See [`GasEncoder`]. #[pallet::no_default_bounds] type EthGasEncoder: GasEncoder>; + + /// Set to `Some` in order to collect all storage deposits from the specified hold. + /// + /// If `None` the deposits are collected from free balance. In any case, they are + /// collected from the transaction signers native balance. + /// + /// It only applies to eth_* dispatchables. The non eth flavor functions will continue + /// to take from the free balance. + type DepositSource: Get>; } /// Container for different types that implement [`DefaultConfig`]` of this pallet. @@ -333,6 +342,7 @@ pub mod pallet { #[inject_runtime_type] type RuntimeCall = (); + type Precompiles = (); type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent; type DepositPerByte = DepositPerByte; @@ -350,6 +360,7 @@ pub mod pallet { type NativeToEthRatio = ConstU32<1_000_000>; type EthGasEncoder = (); type FindAuthor = (); + type DepositSource = (); } } @@ -835,8 +846,9 @@ pub mod pallet { dest, Pallet::::convert_native_to_evm(value), gas_limit, - DepositLimit::Balance(storage_deposit_limit), + storage_deposit_limit, data, + ExecConfig::new_substrate_tx(), ); if let Ok(return_value) = &output.result { @@ -870,11 +882,11 @@ pub mod pallet { origin, Pallet::::convert_native_to_evm(value), gas_limit, - DepositLimit::Balance(storage_deposit_limit), + storage_deposit_limit, Code::Existing(code_hash), data, salt, - BumpNonce::Yes, + ExecConfig::new_substrate_tx(), ); if let Ok(retval) = &output.result { if retval.result.did_revert() { @@ -935,11 +947,11 @@ pub mod pallet { origin, Pallet::::convert_native_to_evm(value), gas_limit, - DepositLimit::Balance(storage_deposit_limit), + storage_deposit_limit, Code::Upload(code), data, salt, - BumpNonce::Yes, + ExecConfig::new_substrate_tx(), ); if let Ok(retval) = &output.result { if retval.result.did_revert() { @@ -979,11 +991,11 @@ pub mod pallet { origin, value, gas_limit, - DepositLimit::Balance(storage_deposit_limit), + storage_deposit_limit, Code::Upload(code), data, None, - BumpNonce::No, + ExecConfig::new_eth_tx(), ); if let Ok(retval) = &output.result { @@ -1019,8 +1031,9 @@ pub mod pallet { dest, value, gas_limit, - DepositLimit::Balance(storage_deposit_limit), + storage_deposit_limit, data, + ExecConfig::new_eth_tx(), ); if let Ok(return_value) = &output.result { @@ -1190,15 +1203,16 @@ where dest: H160, evm_value: U256, gas_limit: Weight, - storage_deposit_limit: DepositLimit>, + storage_deposit_limit: BalanceOf, data: Vec, + exec_config: ExecConfig, ) -> ContractResult> { let mut gas_meter = GasMeter::new(gas_limit); let mut storage_deposit = Default::default(); let try_call = || { let origin = Origin::from_runtime_origin(origin)?; - let mut storage_meter = StorageMeter::new(storage_deposit_limit.limit()); + let mut storage_meter = StorageMeter::new(storage_deposit_limit); let result = ExecStack::>::run_call( origin.clone(), dest, @@ -1206,11 +1220,10 @@ where &mut storage_meter, evm_value, data, - storage_deposit_limit.is_unchecked(), + &exec_config, )?; - storage_deposit = storage_meter - .try_into_deposit(&origin, storage_deposit_limit.is_unchecked()) - .inspect_err(|err| { + storage_deposit = + storage_meter.try_into_deposit(&origin, &exec_config).inspect_err(|err| { log::debug!(target: LOG_TARGET, "Failed to transfer deposit: {err:?}"); })?; Ok(result) @@ -1244,16 +1257,14 @@ where origin: OriginFor, evm_value: U256, gas_limit: Weight, - storage_deposit_limit: DepositLimit>, + mut storage_deposit_limit: BalanceOf, code: Code, data: Vec, salt: Option<[u8; 32]>, - bump_nonce: BumpNonce, + exec_config: ExecConfig, ) -> ContractResult> { let mut gas_meter = GasMeter::new(gas_limit); let mut storage_deposit = Default::default(); - let unchecked_deposit_limit = storage_deposit_limit.is_unchecked(); - let mut storage_deposit_limit = storage_deposit_limit.limit(); let try_instantiate = || { let instantiate_account = T::InstantiateOrigin::ensure_origin(origin.clone())?; @@ -1265,7 +1276,7 @@ where upload_account, code, storage_deposit_limit, - unchecked_deposit_limit, + &exec_config, )?; storage_deposit_limit.saturating_reduce(upload_deposit); (executable, upload_deposit) @@ -1291,11 +1302,10 @@ where evm_value, data, salt.as_ref(), - unchecked_deposit_limit, - bump_nonce, + &exec_config, ); storage_deposit = storage_meter - .try_into_deposit(&instantiate_origin, unchecked_deposit_limit)? + .try_into_deposit(&instantiate_origin, &exec_config)? .saturating_add(&StorageDeposit::Charge(upload_deposit)); result }; @@ -1339,11 +1349,9 @@ where let origin = T::AddressMapper::to_account_id(&from); Self::prepare_dry_run(&origin); - let storage_deposit_limit = if tx.gas.is_some() { - DepositLimit::Balance(BalanceOf::::max_value()) - } else { - DepositLimit::UnsafeOnlyForDryRun - }; + let storage_deposit_limit = BalanceOf::::max_value(); + let mut exec_config = ExecConfig::new_eth_tx(); + exec_config.unsafe_skip_transfers = tx.gas.is_none(); if tx.nonce.is_none() { tx.nonce = Some(>::account_nonce(&origin).into()); @@ -1423,6 +1431,7 @@ where gas_limit, storage_deposit_limit, input.clone(), + exec_config, ); let data = match result.result { @@ -1478,7 +1487,7 @@ where Code::Upload(code.clone()), data.clone(), None, - BumpNonce::No, + exec_config, ); let returned_data = match result.result { @@ -1648,8 +1657,12 @@ where storage_deposit_limit: BalanceOf, ) -> CodeUploadResult> { let origin = T::UploadOrigin::ensure_origin(origin)?; - let (module, deposit) = - Self::try_upload_pvm_code(origin, code, storage_deposit_limit, false)?; + let (module, deposit) = Self::try_upload_pvm_code( + origin, + code, + storage_deposit_limit, + &ExecConfig::new_substrate_tx(), + )?; Ok(CodeUploadReturnValue { code_hash: *module.code_hash(), deposit }) } @@ -1680,10 +1693,10 @@ where origin: T::AccountId, code: Vec, storage_deposit_limit: BalanceOf, - skip_transfer: bool, + exec_config: &ExecConfig, ) -> Result<(ContractBlob, BalanceOf), DispatchError> { let mut module = ContractBlob::from_pvm_code(code, origin)?; - let deposit = module.store_code(skip_transfer)?; + let deposit = module.store_code(exec_config)?; ensure!(storage_deposit_limit >= deposit, >::StorageDepositLimitExhausted); Ok((module, deposit)) } @@ -1769,6 +1782,55 @@ impl Pallet { .map(|code| code.into()) .unwrap_or_default() } + + /// Transfer a deposit from some account to another. + /// + /// `from` is usually the transaction origin and `to` a contract or + /// the pallets own account. + fn charge_deposit( + hold_reason: Option, + from: &T::AccountId, + to: &T::AccountId, + amount: BalanceOf, + exec_config: &ExecConfig, + ) -> DispatchResult { + use frame_support::traits::tokens::{Fortitude, Precision, Preservation, Restriction}; + let deposit_source = exec_config + .collect_deposit_from_hold + .then_some(T::DepositSource::get()) + .flatten(); + match (deposit_source, hold_reason) { + (Some(deposit_source), hold_reason) => { + T::Currency::transfer_on_hold( + &deposit_source, + from, + to, + amount, + Precision::Exact, + Restriction::Free, + Fortitude::Polite, + )?; + if let Some(hold_reason) = hold_reason { + T::Currency::hold(&hold_reason.into(), to, amount)?; + } + }, + (None, Some(hold_reason)) => { + T::Currency::transfer_and_hold( + &hold_reason.into(), + from, + to, + amount, + Precision::Exact, + Preservation::Preserve, + Fortitude::Polite, + )?; + }, + (None, None) => { + T::Currency::transfer(from, to, amount, Preservation::Preserve)?; + }, + } + Ok(()) + } } /// The address used to call the runtime's pallets dispatchables @@ -1998,8 +2060,9 @@ macro_rules! impl_runtime_apis_plus_revive { dest, $crate::Pallet::::convert_native_to_evm(value), gas_limit.unwrap_or(blockweights.max_block), - $crate::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)), + storage_deposit_limit.unwrap_or(u128::MAX), input_data, + $crate::ExecConfig::new_substrate_tx(), ) } @@ -2021,11 +2084,11 @@ macro_rules! impl_runtime_apis_plus_revive { ::RuntimeOrigin::signed(origin), $crate::Pallet::::convert_native_to_evm(value), gas_limit.unwrap_or(blockweights.max_block), - $crate::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)), + storage_deposit_limit.unwrap_or(u128::MAX), code, data, salt, - $crate::BumpNonce::Yes, + $crate::ExecConfig::new_substrate_tx(), ) } diff --git a/substrate/frame/revive/src/primitives.rs b/substrate/frame/revive/src/primitives.rs index 4690e79d4144d..e28071f563e5e 100644 --- a/substrate/frame/revive/src/primitives.rs +++ b/substrate/frame/revive/src/primitives.rs @@ -23,46 +23,12 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::weights::Weight; use pallet_revive_uapi::ReturnFlags; use scale_info::TypeInfo; -use sp_arithmetic::traits::Bounded; use sp_core::Get; use sp_runtime::{ traits::{One, Saturating, Zero}, DispatchError, RuntimeDebug, }; -#[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] -pub enum DepositLimit { - /// Allows bypassing all balance transfer checks. - UnsafeOnlyForDryRun, - - /// Specifies a maximum allowable balance for a deposit. - Balance(Balance), -} - -impl DepositLimit { - pub fn is_unchecked(&self) -> bool { - match self { - Self::UnsafeOnlyForDryRun => true, - _ => false, - } - } -} - -impl From for DepositLimit { - fn from(value: T) -> Self { - Self::Balance(value) - } -} - -impl DepositLimit { - pub fn limit(&self) -> T { - match self { - Self::UnsafeOnlyForDryRun => T::max_value(), - Self::Balance(limit) => *limit, - } - } -} - /// Result type of a `bare_call` or `bare_instantiate` call as well as `ContractsApi::call` and /// `ContractsApi::instantiate`. /// @@ -340,22 +306,50 @@ where } } -/// Indicates whether the account nonce should be incremented after instantiating a new contract. -/// -/// In Substrate, where transactions can be batched, the account's nonce should be incremented after -/// each instantiation, ensuring that each instantiation uses a unique nonce. -/// -/// For transactions sent from Ethereum wallets, which cannot be batched, the nonce should only be -/// incremented once. In these cases, Use `BumpNonce::No` to suppress an extra nonce increment. -/// -/// Note: -/// The origin's nonce is already incremented pre-dispatch by the `CheckNonce` transaction -/// extension. -pub enum BumpNonce { - /// Do not increment the nonce after contract instantiation - No, - /// Increment the nonce after contract instantiation - Yes, +/// `Stack` wide configuration options. +#[derive(Debug, Clone)] +pub struct ExecConfig { + /// Indicates whether the account nonce should be incremented after instantiating a new + /// contract. + /// + /// In Substrate, where transactions can be batched, the account's nonce should be incremented + /// after each instantiation, ensuring that each instantiation uses a unique nonce. + /// + /// For transactions sent from Ethereum wallets, which cannot be batched, the nonce should only + /// be incremented once. In these cases, set this to `false` to suppress an extra nonce + /// increment. + /// + /// Note: + /// The origin's nonce is already incremented pre-dispatch by the `CheckNonce` transaction + /// extension. + /// + /// This does not apply to contract initiated instantatiations. Those will always bump the + /// instantiating contract's nonce. + pub bump_nonce: bool, + /// If set to `true` deposits will be collected from [`Config::DepositSource`] instead of + /// free_balance. + /// + /// Only applies if [`Config::DepositSource`] is `Some`. Otherwise it will fall back to collect + /// from fee balance. + pub collect_deposit_from_hold: bool, + /// Skip all transfers (deposits, contract instantiation, value transfer). + /// + /// Must only be used in dry runs where no wallet is available to charge those funds from, + /// Using it for on-chain code is unsafe as it will allow execution without taking any money + /// from the origin, + pub unsafe_skip_transfers: bool, +} + +impl ExecConfig { + /// Create a default config appropriate when the call originated from a subtrate tx. + pub fn new_substrate_tx() -> Self { + Self { bump_nonce: true, collect_deposit_from_hold: false, unsafe_skip_transfers: false } + } + + /// Create a default config appropriate when the call originated from a ethereum tx. + pub fn new_eth_tx() -> Self { + Self { bump_nonce: false, collect_deposit_from_hold: true, unsafe_skip_transfers: false } + } } /// Indicates whether the code was removed after the last refcount was decremented. diff --git a/substrate/frame/revive/src/storage/meter.rs b/substrate/frame/revive/src/storage/meter.rs index f002b1f8b108d..d4059a59bfd4b 100644 --- a/substrate/frame/revive/src/storage/meter.rs +++ b/substrate/frame/revive/src/storage/meter.rs @@ -18,8 +18,8 @@ //! This module contains functions to meter the storage deposit. use crate::{ - storage::ContractInfo, AccountIdOf, BalanceOf, Config, Error, HoldReason, Inspect, Origin, - StorageDeposit as Deposit, System, LOG_TARGET, + storage::ContractInfo, AccountIdOf, BalanceOf, Config, Error, ExecConfig, HoldReason, Inspect, + Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET, }; use alloc::vec::Vec; use core::{fmt::Debug, marker::PhantomData}; @@ -65,6 +65,7 @@ pub trait Ext { contract: &T::AccountId, amount: &DepositOf, state: &ContractState, + exec_config: &ExecConfig, ) -> Result<(), DispatchError>; } @@ -364,9 +365,9 @@ where pub fn try_into_deposit( self, origin: &Origin, - skip_transfer: bool, + exec_config: &ExecConfig, ) -> Result, DispatchError> { - if !skip_transfer { + if !exec_config.unsafe_skip_transfers { // Only refund or charge deposit if the origin is not root. let origin = match origin { Origin::Root => return Ok(Deposit::Charge(Zero::zero())), @@ -375,11 +376,23 @@ where let try_charge = || { for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Refund(_))) { - E::charge(origin, &charge.contract, &charge.amount, &charge.state)?; + E::charge( + origin, + &charge.contract, + &charge.amount, + &charge.state, + exec_config, + )?; } for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Charge(_))) { - E::charge(origin, &charge.contract, &charge.amount, &charge.state)?; + E::charge( + origin, + &charge.contract, + &charge.amount, + &charge.state, + exec_config, + )?; } Ok(()) }; @@ -456,18 +469,17 @@ impl Ext for ReservingExt { contract: &T::AccountId, amount: &DepositOf, state: &ContractState, + exec_config: &ExecConfig, ) -> Result<(), DispatchError> { match amount { Deposit::Charge(amount) | Deposit::Refund(amount) if amount.is_zero() => return Ok(()), Deposit::Charge(amount) => { - T::Currency::transfer_and_hold( - &HoldReason::StorageDepositReserve.into(), + >::charge_deposit( + Some(HoldReason::StorageDepositReserve), origin, contract, *amount, - Precision::Exact, - Preservation::Preserve, - Fortitude::Polite, + exec_config, )?; }, Deposit::Refund(amount) => { @@ -551,6 +563,7 @@ mod tests { contract: &AccountIdOf, amount: &DepositOf, state: &ContractState, + _exec_config: &ExecConfig, ) -> Result<(), DispatchError> { TestExtTestValue::mutate(|ext| { ext.charges.push(Charge { @@ -747,7 +760,9 @@ mod tests { meter.absorb(nested0, &BOB, Some(&mut nested0_info)); assert_eq!( - meter.try_into_deposit(&test_case.origin, false).unwrap(), + meter + .try_into_deposit(&test_case.origin, &ExecConfig::new_substrate_tx()) + .unwrap(), test_case.deposit ); @@ -820,7 +835,9 @@ mod tests { meter.absorb(nested0, &BOB, None); assert_eq!( - meter.try_into_deposit(&test_case.origin, false).unwrap(), + meter + .try_into_deposit(&test_case.origin, &ExecConfig::new_substrate_tx()) + .unwrap(), test_case.deposit ); assert_eq!(TestExtTestValue::get(), test_case.expected) diff --git a/substrate/frame/revive/src/test_utils/builder.rs b/substrate/frame/revive/src/test_utils/builder.rs index 2769484c68558..9c4825025231c 100644 --- a/substrate/frame/revive/src/test_utils/builder.rs +++ b/substrate/frame/revive/src/test_utils/builder.rs @@ -17,8 +17,8 @@ use super::{deposit_limit, GAS_LIMIT}; use crate::{ - address::AddressMapper, AccountIdOf, BalanceOf, BumpNonce, Code, Config, ContractResult, - DepositLimit, ExecReturnValue, InstantiateReturnValue, OriginFor, Pallet, Weight, U256, + address::AddressMapper, AccountIdOf, BalanceOf, Code, Config, ContractResult, ExecConfig, + ExecReturnValue, InstantiateReturnValue, OriginFor, Pallet, Weight, U256, }; use alloc::{vec, vec::Vec}; use frame_support::pallet_prelude::DispatchResultWithPostInfo; @@ -134,11 +134,11 @@ builder!( origin: OriginFor, evm_value: U256, gas_limit: Weight, - storage_deposit_limit: DepositLimit>, + storage_deposit_limit: BalanceOf, code: Code, data: Vec, salt: Option<[u8; 32]>, - bump_nonce: BumpNonce, + exec_config: ExecConfig, ) -> ContractResult>; /// Set the call's evm_value using a native_value amount. @@ -168,11 +168,11 @@ builder!( origin, evm_value: Default::default(), gas_limit: GAS_LIMIT, - storage_deposit_limit: DepositLimit::Balance(deposit_limit::()), + storage_deposit_limit: deposit_limit::(), code, data: vec![], salt: Some([0; 32]), - bump_nonce: BumpNonce::Yes, + exec_config: ExecConfig::new_substrate_tx(), } } ); @@ -206,8 +206,9 @@ builder!( dest: H160, evm_value: U256, gas_limit: Weight, - storage_deposit_limit: DepositLimit>, + storage_deposit_limit: BalanceOf, data: Vec, + exec_config: ExecConfig, ) -> ContractResult>; /// Set the call's evm_value using a native_value amount. @@ -228,8 +229,9 @@ builder!( dest, evm_value: Default::default(), gas_limit: GAS_LIMIT, - storage_deposit_limit: DepositLimit::Balance(deposit_limit::()), + storage_deposit_limit: deposit_limit::(), data: vec![], + exec_config: ExecConfig::new_substrate_tx(), } } ); diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 9dda6f55cadb9..277c629a204f8 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -332,6 +332,7 @@ parameter_types! { pub static UnstableInterface: bool = true; pub static AllowEvmBytecode: bool = true; pub CheckingAccount: AccountId32 = BOB.clone(); + pub DepositSource: Option = Some(pallet_transaction_payment::HoldReason::Payment.into()); } impl FindAuthor<::AccountId> for Test { @@ -358,6 +359,7 @@ impl Config for Test { type ChainId = ChainId; type FindAuthor = Test; type Precompiles = (precompiles::WithInfo, precompiles::NoInfo); + type DepositSource = DepositSource; } impl TryFrom for crate::Call { diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index d25060322699a..907713a9a2f48 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -36,9 +36,8 @@ use crate::{ }, tracing::trace, weights::WeightInfo, - AccountInfo, AccountInfoOf, BalanceWithDust, BumpNonce, Code, Config, ContractInfo, - DeletionQueueCounter, DepositLimit, Error, EthTransactError, HoldReason, Pallet, PristineCode, - StorageDeposit, H160, + AccountInfo, AccountInfoOf, BalanceWithDust, Code, Config, ContractInfo, DeletionQueueCounter, + Error, EthTransactError, ExecConfig, HoldReason, Pallet, PristineCode, StorageDeposit, H160, }; use assert_matches::assert_matches; use codec::Encode; @@ -46,7 +45,7 @@ use frame_support::{ assert_err, assert_err_ignore_postinfo, assert_noop, assert_ok, storage::child, traits::{ - fungible::{BalancedHold, Inspect, Mutate}, + fungible::{BalancedHold, Inspect, Mutate, MutateHold}, tokens::Preservation, OnIdle, OnInitialize, }, @@ -239,17 +238,14 @@ fn deposit_limit_enforced_on_plain_transfer() { // sending balance to a new account should fail when the limit is lower than the ed let result = builder::bare_call(CHARLIE_ADDR) .native_value(1) - .storage_deposit_limit(190.into()) + .storage_deposit_limit(190) .build(); assert_err!(result.result, >::StorageDepositLimitExhausted); assert_eq!(result.storage_deposit, StorageDeposit::Charge(0)); assert_eq!(get_balance(&CHARLIE), 0); // works when the account is prefunded - let result = builder::bare_call(BOB_ADDR) - .native_value(1) - .storage_deposit_limit(0.into()) - .build(); + let result = builder::bare_call(BOB_ADDR).native_value(1).storage_deposit_limit(0).build(); assert_ok!(result.result); assert_eq!(result.storage_deposit, StorageDeposit::Charge(0)); assert_eq!(get_balance(&BOB), 1_000_001); @@ -257,7 +253,7 @@ fn deposit_limit_enforced_on_plain_transfer() { // also works allowing enough deposit let result = builder::bare_call(CHARLIE_ADDR) .native_value(1) - .storage_deposit_limit(200.into()) + .storage_deposit_limit(200) .build(); assert_ok!(result.result); assert_eq!(result.storage_deposit, StorageDeposit::Charge(200)); @@ -2649,7 +2645,7 @@ fn deposit_limit_in_nested_calls() { // that the nested call should have a deposit limit of at least 2 Balance. The // sub-call should be rolled back, which is covered by the next test case. let ret = builder::bare_call(addr_caller) - .storage_deposit_limit(DepositLimit::Balance(u64::MAX)) + .storage_deposit_limit(u64::MAX) .data((102u32, &addr_callee, U256::from(1u64)).encode()) .build_and_unwrap_result(); assert_return_code!(ret, RuntimeReturnCode::OutOfResources); @@ -2727,7 +2723,7 @@ fn deposit_limit_in_nested_instantiate() { // Sub calls return first to they are checked first. let ret = builder::bare_call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) - .storage_deposit_limit(DepositLimit::Balance(0)) + .storage_deposit_limit(0) .data((&code_hash_callee, 100u32, &U256::MAX.to_little_endian()).encode()) .build_and_unwrap_result(); assert_return_code!(ret, RuntimeReturnCode::OutOfResources); @@ -2740,7 +2736,7 @@ fn deposit_limit_in_nested_instantiate() { // succeeds but the parent call runs out of storage. let ret = builder::bare_call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) - .storage_deposit_limit(DepositLimit::Balance(callee_min_deposit)) + .storage_deposit_limit(callee_min_deposit) .data((&code_hash_callee, 0u32, &U256::MAX.to_little_endian()).encode()) .build(); assert_err!(ret.result, >::StorageDepositLimitExhausted); @@ -2752,7 +2748,7 @@ fn deposit_limit_in_nested_instantiate() { // Same as above but stores one byte in both caller and callee. let ret = builder::bare_call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) - .storage_deposit_limit(DepositLimit::Balance(caller_min_deposit + 1)) + .storage_deposit_limit(caller_min_deposit + 1) .data((&code_hash_callee, 1u32, U256::from(callee_min_deposit)).encode()) .build_and_unwrap_result(); assert_return_code!(ret, RuntimeReturnCode::OutOfResources); @@ -2764,7 +2760,7 @@ fn deposit_limit_in_nested_instantiate() { // Same as above but stores one byte in both caller and callee. let ret = builder::bare_call(addr_caller) .origin(RuntimeOrigin::signed(BOB)) - .storage_deposit_limit(DepositLimit::Balance(callee_min_deposit + 1)) + .storage_deposit_limit(callee_min_deposit + 1) .data((&code_hash_callee, 1u32, U256::from(callee_min_deposit + 1)).encode()) .build(); assert_err!(ret.result, >::StorageDepositLimitExhausted); @@ -4711,12 +4707,14 @@ fn bump_nonce_once_works() { let _ = ::Currency::set_balance(&ALICE, 1_000_000); frame_system::Account::::mutate(&ALICE, |account| account.nonce = 1); + let mut do_not_bump = ExecConfig::new_substrate_tx(); + do_not_bump.bump_nonce = false; + let _ = ::Currency::set_balance(&BOB, 1_000_000); frame_system::Account::::mutate(&BOB, |account| account.nonce = 1); builder::bare_instantiate(Code::Upload(code.clone())) .origin(RuntimeOrigin::signed(ALICE)) - .bump_nonce(BumpNonce::Yes) .salt(None) .build_and_unwrap_result(); assert_eq!(System::account_nonce(&ALICE), 2); @@ -4724,7 +4722,6 @@ fn bump_nonce_once_works() { // instantiate again is ok let result = builder::bare_instantiate(Code::Existing(hash)) .origin(RuntimeOrigin::signed(ALICE)) - .bump_nonce(BumpNonce::Yes) .salt(None) .build() .result; @@ -4732,7 +4729,7 @@ fn bump_nonce_once_works() { builder::bare_instantiate(Code::Upload(code.clone())) .origin(RuntimeOrigin::signed(BOB)) - .bump_nonce(BumpNonce::No) + .exec_config(do_not_bump.clone()) .salt(None) .build_and_unwrap_result(); assert_eq!(System::account_nonce(&BOB), 1); @@ -4740,7 +4737,7 @@ fn bump_nonce_once_works() { // instantiate again should fail let err = builder::bare_instantiate(Code::Upload(code)) .origin(RuntimeOrigin::signed(BOB)) - .bump_nonce(BumpNonce::No) + .exec_config(do_not_bump) .salt(None) .build() .result @@ -4914,3 +4911,41 @@ fn return_data_limit_is_enforced() { } }); } + +#[test] +fn storage_deposit_from_hold_works() { + let ed = 200; + let (binary, code_hash) = compile_module("dummy").unwrap(); + ExtBuilder::default().existential_deposit(ed).build().execute_with(|| { + let hold_initial = 500_000; + let reason = DepositSource::get().unwrap(); + ::Currency::set_balance(&ALICE, 1_000_000); + ::Currency::set_on_hold(&reason, &ALICE, hold_initial).unwrap(); + let mut exec_config = ExecConfig::new_substrate_tx(); + exec_config.collect_deposit_from_hold = true; + + // Instantiate the BOB contract. + let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(binary)) + .exec_config(exec_config) + .native_value(1_000) + .build_and_unwrap_contract(); + + // Check that the BOB contract has been instantiated. + get_contract(&addr); + + let account = ::AddressMapper::to_account_id(&addr); + let base_deposit = contract_base_deposit(&addr); + let code_deposit = get_code_deposit(&code_hash); + assert!(base_deposit > 0); + assert!(code_deposit > 0); + + assert_eq!( + get_balance_on_hold(&HoldReason::StorageDepositReserve.into(), &account), + base_deposit, + ); + assert_eq!( + get_balance_on_hold(&reason, &ALICE), + hold_initial - base_deposit - code_deposit - ed, + ); + }); +} diff --git a/substrate/frame/revive/src/vm/mod.rs b/substrate/frame/revive/src/vm/mod.rs index dc01b7879ec3e..3e2e62fb6f057 100644 --- a/substrate/frame/revive/src/vm/mod.rs +++ b/substrate/frame/revive/src/vm/mod.rs @@ -30,8 +30,8 @@ use crate::{ gas::{GasMeter, Token}, storage::meter::Diff, weights::WeightInfo, - AccountIdOf, BalanceOf, CodeInfoOf, CodeRemoved, Config, Error, ExecError, HoldReason, - PristineCode, Weight, LOG_TARGET, + AccountIdOf, BalanceOf, CodeInfoOf, CodeRemoved, Config, Error, ExecConfig, ExecError, + HoldReason, Pallet, PristineCode, Weight, LOG_TARGET, }; use alloc::vec::Vec; use codec::{Decode, Encode, MaxEncodedLen}; @@ -39,7 +39,7 @@ use frame_support::{ dispatch::DispatchResult, traits::{ fungible::MutateHold, - tokens::{Fortitude, Precision, Preservation}, + tokens::{Fortitude, Precision}, }, }; use pallet_revive_uapi::ReturnErrorCode; @@ -176,7 +176,7 @@ where ensure!(&code_info.owner == origin, BadOrigin); T::Currency::transfer_on_hold( &HoldReason::CodeUploadDepositReserve.into(), - &crate::Pallet::::account_id(), + &Pallet::::account_id(), &code_info.owner, code_info.deposit, Precision::Exact, @@ -194,7 +194,7 @@ where } /// Puts the module blob into storage, and returns the deposit collected for the storage. - pub fn store_code(&mut self, skip_transfer: bool) -> Result, Error> { + pub fn store_code(&mut self, exec_config: &ExecConfig) -> Result, Error> { let code_hash = *self.code_hash(); ensure!(code_hash != H256::zero(), >::CodeNotFound); @@ -209,15 +209,13 @@ where None => { let deposit = self.code_info.deposit; - if !skip_transfer { - T::Currency::transfer_and_hold( - &HoldReason::CodeUploadDepositReserve.into(), + if !exec_config.unsafe_skip_transfers { + >::charge_deposit( + Some(HoldReason::CodeUploadDepositReserve), &self.code_info.owner, - &crate::Pallet::::account_id(), + &Pallet::::account_id(), deposit, - Precision::Exact, - Preservation::Preserve, - Fortitude::Polite, + &exec_config, ) .map_err(|err| { log::debug!(target: LOG_TARGET, "failed to hold store code deposit {deposit:?} for owner: {:?}: {err:?}", self.code_info.owner);