diff --git a/prdoc/pr_10233.prdoc b/prdoc/pr_10233.prdoc new file mode 100644 index 0000000000000..7d149536d226b --- /dev/null +++ b/prdoc/pr_10233.prdoc @@ -0,0 +1,7 @@ +title: not charging ED in ctor if account already has enough balance +doc: +- audience: Runtime Dev + description: fixes https://github.com/paritytech/contract-issues/issues/179 +crates: +- name: pallet-revive + bump: patch diff --git a/substrate/frame/revive/dev-node/node/src/service.rs b/substrate/frame/revive/dev-node/node/src/service.rs index f412fc6cd975a..3e06fff574bb4 100644 --- a/substrate/frame/revive/dev-node/node/src/service.rs +++ b/substrate/frame/revive/dev-node/node/src/service.rs @@ -16,13 +16,11 @@ // limitations under the License. use crate::cli::Consensus; -use futures::FutureExt; use polkadot_sdk::{ - sc_client_api::{backend::Backend, StorageProvider}, + sc_client_api::StorageProvider, sc_executor::WasmExecutor, sc_service::{error::Error as ServiceError, Configuration, TaskManager}, sc_telemetry::{Telemetry, TelemetryWorker}, - sc_transaction_pool_api::OffchainTransactionPoolFactory, sp_runtime::traits::Block as BlockT, *, }; @@ -142,27 +140,6 @@ pub fn new_full::Ha metrics, })?; - if config.offchain_worker.enabled { - let offchain_workers = - sc_offchain::OffchainWorkers::new(sc_offchain::OffchainWorkerOptions { - runtime_api_provider: client.clone(), - is_validator: config.role.is_authority(), - keystore: Some(keystore_container.keystore()), - offchain_db: backend.offchain_storage(), - transaction_pool: Some(OffchainTransactionPoolFactory::new( - transaction_pool.clone(), - )), - network_provider: Arc::new(network.clone()), - enable_http_requests: true, - custom_extensions: |_| vec![], - })?; - task_manager.spawn_handle().spawn( - "offchain-workers-runner", - "offchain-worker", - offchain_workers.run(client.clone(), task_manager.spawn_handle()).boxed(), - ); - } - let rpc_extensions_builder = { let client = client.clone(); let pool = transaction_pool.clone(); diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 4115cd5c6895b..5962055c69a68 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1222,11 +1222,12 @@ where // if we reached this point the origin has an associated account. let origin = &self.origin.account_id()?; - let ed = >::min_balance(); - frame.nested_storage.record_charge(&StorageDeposit::Charge(ed))?; - >::charge_deposit(None, origin, account_id, ed, self.exec_config) - .map_err(|_| >::StorageDepositNotEnoughFunds)?; - + if !frame_system::Pallet::::account_exists(&account_id) { + let ed = >::min_balance(); + frame.nested_storage.record_charge(&StorageDeposit::Charge(ed))?; + >::charge_deposit(None, origin, account_id, ed, self.exec_config) + .map_err(|_| >::StorageDepositNotEnoughFunds)?; + } // A consumer is added at account creation and removed it on termination, otherwise // the runtime could remove the account. As long as a contract exists its // account must exist. With the consumer, a correct runtime cannot remove the diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index b9cde375ac3f2..c2a765981489d 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -5133,3 +5133,35 @@ fn consume_all_gas_works() { ); }); } + +#[test] +fn existential_deposit_shall_not_charged_twice() { + let (code, _) = compile_module("dummy").unwrap(); + + let salt = [0u8; 32]; + + ExtBuilder::default().build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 100_000_000_000_000); + let callee_addr = create2( + &ALICE_ADDR, + &code, + &[0u8; 0], // empty input + &salt, + ); + let callee_account = ::AddressMapper::to_account_id(&callee_addr); + + // first send funds to callee_addr + let _ = ::Currency::set_balance(&callee_account, Contracts::min_balance()); + assert_eq!(get_balance(&callee_account), Contracts::min_balance()); + + // then deploy contract to callee_addr using create2 + let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(code.clone())) + .salt(Some(salt)) + .build_and_unwrap_contract(); + + assert_eq!(callee_addr, addr); + + // check we charged ed only 1 time + assert_eq!(get_balance(&callee_account), Contracts::min_balance()); + }); +}