From 9510ca2caa4a4fda0295d15769b54af28597f0ee Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 16:40:08 +0100 Subject: [PATCH 1/9] not charging ED in ctor if account already has enough balance --- substrate/frame/revive/src/exec.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 4115cd5c6895b..d52ba3fe2c616 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1222,11 +1222,14 @@ where // if we reached this point the origin has an associated account. let origin = &self.origin.account_id()?; + // get balance of the account before charging ED, only charge ED if balance insufficient + let balance = T::Currency::total_balance(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 balance < ed { + 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 @@ -1822,6 +1825,7 @@ where input_data: Vec, salt: Option<&[u8; 32]>, ) -> Result { + log::info!(target: LOG_TARGET, "Instantiating contract with value: {value:?}"); // We reset the return data now, so it is cleared out even if no new frame was executed. // This is for example the case when creating the frame fails. *self.last_frame_output_mut() = Default::default(); From c4bd0198e74e37741cb11c8e8d867f0cab11ec57 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 17:11:41 +0100 Subject: [PATCH 2/9] removed offchain_worker from dev-node --- .../frame/revive/dev-node/node/src/service.rs | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/substrate/frame/revive/dev-node/node/src/service.rs b/substrate/frame/revive/dev-node/node/src/service.rs index f412fc6cd975a..5e96494dc7cac 100644 --- a/substrate/frame/revive/dev-node/node/src/service.rs +++ b/substrate/frame/revive/dev-node/node/src/service.rs @@ -142,27 +142,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(); From 16a21b222c164358d4fa33c62c4a8475689de26c Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 17:39:47 +0100 Subject: [PATCH 3/9] added test for double ed in case of create2 --- substrate/frame/revive/src/tests/pvm.rs | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index b9cde375ac3f2..3aeefc5417931 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -5133,3 +5133,38 @@ fn consume_all_gas_works() { ); }); } + +#[test] +fn existential_deposit_shall_not_charged_twice() { + let (code, code_hash) = 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 result = builder::bare_call(callee_addr) + .native_value(666_000_000_000) + .build(); + assert_ok!(result.result); + assert_eq!(get_balance(&callee_account), 666_000_000_000 + 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), 666_000_000_000 + Contracts::min_balance()); + }); +} From be12d99664dbfb7b2e38cc17390ba2032ca83996 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 17:41:28 +0100 Subject: [PATCH 4/9] remove log --- substrate/frame/revive/src/exec.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index d52ba3fe2c616..42c9f177af692 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1825,7 +1825,6 @@ where input_data: Vec, salt: Option<&[u8; 32]>, ) -> Result { - log::info!(target: LOG_TARGET, "Instantiating contract with value: {value:?}"); // We reset the return data now, so it is cleared out even if no new frame was executed. // This is for example the case when creating the frame fails. *self.last_frame_output_mut() = Default::default(); From ebef4966fb4925a12cadf2bb763cc2fa57de5f1b Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 6 Nov 2025 16:47:08 +0000 Subject: [PATCH 5/9] Update from github-actions[bot] running command 'prdoc --audience runtime_dev --bump patch' --- prdoc/pr_10233.prdoc | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 prdoc/pr_10233.prdoc 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 From db0e9c773c6bf70c9868a4925d34b79ee699667a Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 17:42:33 +0100 Subject: [PATCH 6/9] format --- substrate/frame/revive/src/exec.rs | 3 ++- substrate/frame/revive/src/tests/pvm.rs | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 42c9f177af692..6934d502236c3 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1222,7 +1222,8 @@ where // if we reached this point the origin has an associated account. let origin = &self.origin.account_id()?; - // get balance of the account before charging ED, only charge ED if balance insufficient + // get balance of the account before charging ED, only charge ED if balance + // insufficient let balance = T::Currency::total_balance(account_id); let ed = >::min_balance(); if balance < ed { diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index 3aeefc5417931..2cedc0e32f58a 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -5151,9 +5151,7 @@ fn existential_deposit_shall_not_charged_twice() { let callee_account = ::AddressMapper::to_account_id(&callee_addr); // first send funds to callee_addr - let result = builder::bare_call(callee_addr) - .native_value(666_000_000_000) - .build(); + let result = builder::bare_call(callee_addr).native_value(666_000_000_000).build(); assert_ok!(result.result); assert_eq!(get_balance(&callee_account), 666_000_000_000 + Contracts::min_balance()); @@ -5161,7 +5159,7 @@ fn existential_deposit_shall_not_charged_twice() { 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 From efdcf9a97cb3928fe25c310f071197b2fca5913f Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 17:47:28 +0100 Subject: [PATCH 7/9] cleanup --- substrate/frame/revive/dev-node/node/src/service.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/frame/revive/dev-node/node/src/service.rs b/substrate/frame/revive/dev-node/node/src/service.rs index 5e96494dc7cac..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, *, }; From 898b6c18834c30ae4b89654825fe47e915abe338 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 18:14:03 +0100 Subject: [PATCH 8/9] review comment --- substrate/frame/revive/src/tests/pvm.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index 2cedc0e32f58a..c76bae64422b5 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -5151,9 +5151,8 @@ fn existential_deposit_shall_not_charged_twice() { let callee_account = ::AddressMapper::to_account_id(&callee_addr); // first send funds to callee_addr - let result = builder::bare_call(callee_addr).native_value(666_000_000_000).build(); - assert_ok!(result.result); - assert_eq!(get_balance(&callee_account), 666_000_000_000 + Contracts::min_balance()); + 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())) @@ -5163,6 +5162,6 @@ fn existential_deposit_shall_not_charged_twice() { assert_eq!(callee_addr, addr); // check we charged ed only 1 time - assert_eq!(get_balance(&callee_account), 666_000_000_000 + Contracts::min_balance()); + assert_eq!(get_balance(&callee_account), Contracts::min_balance()); }); } From fd99bec87409e20dbf962cb0a8d4adb6b1cd5599 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Thu, 6 Nov 2025 19:28:32 +0100 Subject: [PATCH 9/9] small fix --- substrate/frame/revive/src/exec.rs | 7 ++----- substrate/frame/revive/src/tests/pvm.rs | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 6934d502236c3..5962055c69a68 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1222,11 +1222,8 @@ where // if we reached this point the origin has an associated account. let origin = &self.origin.account_id()?; - // get balance of the account before charging ED, only charge ED if balance - // insufficient - let balance = T::Currency::total_balance(account_id); - let ed = >::min_balance(); - if balance < ed { + 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)?; diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index c76bae64422b5..c2a765981489d 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -5136,7 +5136,7 @@ fn consume_all_gas_works() { #[test] fn existential_deposit_shall_not_charged_twice() { - let (code, code_hash) = compile_module("dummy").unwrap(); + let (code, _) = compile_module("dummy").unwrap(); let salt = [0u8; 32];