Ensure system accounts always exist for programs; ditch the concept o…

…f insufficient value in transfers

core-processor/src/common.rs

@@ -263,6 +263,8 @@ pub enum JournalNote {
     },
     /// Store programs requested by user to be initialized later
     StoreNewPrograms {
+        /// Current program id.
+        program_id: ProgramId,
         /// Code hash used to create new programs with ids in `candidates` field
         code_id: CodeId,
         /// Collection of program candidate ids and their init message ids.
@@ -385,7 +387,12 @@ pub trait JournalHandler {
     /// Store new programs in storage.
     ///
     /// Program ids are ids of _potential_ (planned to be initialized) programs.
-    fn store_new_programs(&mut self, code_id: CodeId, candidates: Vec<(MessageId, ProgramId)>);
+    fn store_new_programs(
+        &mut self,
+        program_id: ProgramId,
+        code_id: CodeId,
+        candidates: Vec<(MessageId, ProgramId)>,
+    );
     /// Stop processing queue.
     ///
     /// Pushes StoredDispatch back to the top of the queue and decreases gas allowance.

core-processor/src/ext.rs

@@ -474,16 +474,6 @@ impl BackendExternalities for Ext {
 }
 
 impl Ext {
-    fn check_message_value(&mut self, message_value: u128) -> Result<(), FallibleExtError> {
-        let existential_deposit = self.context.existential_deposit;
-        // Sending value should apply the range {0} ∪ [existential_deposit; +inf)
-        if message_value != 0 && message_value < existential_deposit {
-            Err(MessageError::InsufficientValue.into())
-        } else {
-            Ok(())
-        }
-    }
-
     fn check_gas_limit(
         &mut self,
         gas_limit: Option<GasLimit>,
@@ -610,7 +600,6 @@ impl Ext {
         packet: &T,
         check_gas_limit: bool,
     ) -> Result<(), FallibleExtError> {
-        self.check_message_value(packet.value())?;
         // Charge for using expiring resources. Charge for calling syscall was done earlier.
         let gas_limit = if check_gas_limit {
             self.check_gas_limit(packet.gas_limit())?
@@ -893,7 +882,6 @@ impl Externalities for Ext {
         delay: u32,
     ) -> Result<MessageId, Self::FallibleError> {
         self.check_forbidden_destination(msg.destination())?;
-        self.check_message_value(msg.value())?;
         // TODO: unify logic around different source of gas (may be origin msg,
         // or reservation) in order to implement #1828.
         self.check_reservation_gas_limit_for_delayed_sending(&id, delay)?;
@@ -932,7 +920,6 @@ impl Externalities for Ext {
         msg: ReplyPacket,
     ) -> Result<MessageId, Self::FallibleError> {
         self.check_forbidden_destination(self.context.message_context.reply_destination())?;
-        self.check_message_value(msg.value())?;
         // TODO: gasful sending (#1828)
         self.charge_message_value(msg.value())?;
         self.charge_sending_fee(0)?;
@@ -1212,6 +1199,9 @@ impl Externalities for Ext {
         self.charge_sending_fee(delay)?;
         self.charge_for_dispatch_stash_hold(delay)?;
 
+        // Charge ED to value_counter
+        self.charge_message_value(self.context.existential_deposit)?;
+
         let code_hash = packet.code_id();
 
         // Send a message for program creation

core-processor/src/handler.rs

@@ -74,9 +74,10 @@ pub fn handle_journal(
             }
             JournalNote::SendValue { from, to, value } => handler.send_value(from, to, value),
             JournalNote::StoreNewPrograms {
+                program_id,
                 code_id,
                 candidates,
-            } => handler.store_new_programs(code_id, candidates),
+            } => handler.store_new_programs(program_id, code_id, candidates),
             JournalNote::StopProcessing {
                 dispatch,
                 gas_burned,

core-processor/src/processing.rs

@@ -446,6 +446,7 @@ pub fn process_success(
     // Must be handled before handling generated dispatches.
     for (code_id, candidates) in program_candidates {
         journal.push(JournalNote::StoreNewPrograms {
+            program_id,
             code_id,
             candidates,
         });

examples/program-factory/src/lib.rs

@@ -55,7 +55,7 @@ mod tests {
     extern crate std;
 
     use super::*;
-    use gtest::{calculate_program_id, Program, System};
+    use gtest::{calculate_program_id, constants::UNITS, Program, System};
     use std::io::Write;
 
     // Creates a new factory and initializes it.
@@ -67,8 +67,11 @@ mod tests {
         // Instantiate factory
         let factory = Program::current_with_id(sys, 100);
 
+        let user_id = 10001;
+        sys.mint_to(user_id, 100 * UNITS);
+
         // Send `init` msg to factory
-        let res = factory.send_bytes(10001, "EMPTY");
+        let res = factory.send_bytes_with_value(user_id, "EMPTY", 10 * UNITS);
         assert!(!res.main_failed());
         assert!(sys.is_active_program(100));
 

examples/syscall-error/src/wasm.rs

@@ -17,19 +17,19 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 use gstd::{
-    errors::{Error, ExtError, MessageError},
+    errors::{Error, ExecutionError, ExtError},
     msg,
     prelude::*,
     ActorId,
 };
 
 #[no_mangle]
 extern "C" fn init() {
-    let res = msg::send(ActorId::default(), "dummy", 250);
+    let res = msg::send(ActorId::default(), "dummy", u128::MAX / 2);
     assert_eq!(
         res,
         Err(Error::Core(
-            ExtError::Message(MessageError::InsufficientValue).into()
+            ExtError::Execution(ExecutionError::NotEnoughValue).into()
         ))
     );
 }

gsdk/src/metadata/generated.rs

@@ -2230,21 +2230,18 @@ pub mod runtime_types {
                     #[doc = "Failed to create a program."]
                     ProgramConstructionFailed,
                     #[codec(index = 10)]
-                    #[doc = "Value doesn't cover ExistentialDeposit."]
-                    ValueLessThanMinimal,
-                    #[codec(index = 11)]
                     #[doc = "Message queue processing is disabled."]
                     MessageQueueProcessingDisabled,
-                    #[codec(index = 12)]
+                    #[codec(index = 11)]
                     #[doc = "Block count doesn't cover MinimalResumePeriod."]
                     ResumePeriodLessThanMinimal,
-                    #[codec(index = 13)]
+                    #[codec(index = 12)]
                     #[doc = "Program with the specified id is not found."]
                     ProgramNotFound,
-                    #[codec(index = 14)]
+                    #[codec(index = 13)]
                     #[doc = "Gear::run() already included in current block."]
                     GearRunAlreadyInBlock,
-                    #[codec(index = 15)]
+                    #[codec(index = 14)]
                     #[doc = "The program rent logic is disabled."]
                     ProgramRentDisabled,
                 }
@@ -2590,6 +2587,10 @@ pub mod runtime_types {
                     #[doc = "Deposit of funds that will not keep bank account alive."]
                     #[doc = "**Must be unreachable in Gear main protocol.**"]
                     InsufficientDeposit,
+                    #[codec(index = 5)]
+                    #[doc = "Overflow during funds transfer."]
+                    #[doc = "**Must be unreachable in Gear main protocol.**"]
+                    Overflow,
                 }
             }
         }

gtest/src/manager.rs

@@ -410,14 +410,6 @@ impl ExtManager {
 
     #[track_caller]
     fn validate_dispatch(&mut self, dispatch: &Dispatch) {
-        if 0 < dispatch.value() && dispatch.value() < crate::EXISTENTIAL_DEPOSIT {
-            panic!(
-                "Value greater than 0, but less than \
-                required existential deposit ({})",
-                crate::EXISTENTIAL_DEPOSIT
-            );
-        }
-
         if self.is_program(&dispatch.source()) {
             panic!("Sending messages allowed only from users id");
         }
@@ -1145,7 +1137,12 @@ impl JournalHandler for ExtManager {
     }
 
     #[track_caller]
-    fn store_new_programs(&mut self, code_id: CodeId, candidates: Vec<(MessageId, ProgramId)>) {
+    fn store_new_programs(
+        &mut self,
+        _program_id: ProgramId,
+        code_id: CodeId,
+        candidates: Vec<(MessageId, ProgramId)>,
+    ) {
         if let Some(code) = self.opt_binaries.get(&code_id).cloned() {
             for (init_message_id, candidate_id) in candidates {
                 if !self.actors.contains_key(&candidate_id) {

pallets/gear-bank/src/lib.rs

@@ -28,7 +28,11 @@ mod tests;
 
 pub use pallet::*;
 
-use frame_support::traits::{Currency, StorageVersion};
+use frame_support::traits::{
+    fungible,
+    tokens::{Fortitude, Preservation, Provenance},
+    Currency, StorageVersion,
+};
 
 #[macro_export]
 macro_rules! impl_config {
@@ -57,8 +61,10 @@ pub mod pallet {
     use frame_support::{
         ensure,
         pallet_prelude::{StorageMap, StorageValue, ValueQuery},
-        sp_runtime::{traits::CheckedSub, Saturating},
-        traits::{ExistenceRequirement, Get, Hooks, ReservableCurrency},
+        sp_runtime::Saturating,
+        traits::{
+            tokens::DepositConsequence, ExistenceRequirement, Get, Hooks, ReservableCurrency,
+        },
         weights::Weight,
         Identity,
     };
@@ -77,7 +83,8 @@ pub mod pallet {
     #[pallet::config]
     pub trait Config: frame_system::Config + pallet_authorship::Config {
         /// Balances management trait for gas/value migrations.
-        type Currency: ReservableCurrency<AccountIdOf<Self>>;
+        type Currency: ReservableCurrency<AccountIdOf<Self>>
+            + fungible::Inspect<AccountIdOf<Self>, Balance = BalanceOf<Self>>;
 
         #[pallet::constant]
         /// Bank account address, that will keep all reserved funds.
@@ -103,6 +110,9 @@ pub mod pallet {
         /// Deposit of funds that will not keep bank account alive.
         /// **Must be unreachable in Gear main protocol.**
         InsufficientDeposit,
+        /// Overflow during funds transfer.
+        /// **Must be unreachable in Gear main protocol.**
+        Overflow,
     }
 
     /// Type containing info of locked in special address funds of each account.
@@ -224,11 +234,15 @@ pub mod pallet {
         ) -> Result<(), Error<T>> {
             let bank_address = T::BankAddress::get();
 
-            ensure!(
-                CurrencyOf::<T>::free_balance(&bank_address).saturating_add(value)
-                    >= CurrencyOf::<T>::minimum_balance(),
-                Error::InsufficientDeposit
-            );
+            match <CurrencyOf<T> as fungible::Inspect<_>>::can_deposit(
+                &bank_address,
+                value,
+                Provenance::Extant,
+            ) {
+                DepositConsequence::BelowMinimum => return Err(Error::<T>::InsufficientDeposit),
+                DepositConsequence::Overflow => return Err(Error::<T>::Overflow),
+                _ => (), // Any other outcome considered successful
+            };
 
             let existence_requirement = if keep_alive {
                 ExistenceRequirement::KeepAlive
@@ -243,13 +257,15 @@ pub mod pallet {
 
         /// Ensures that bank account is able to transfer requested value.
         fn ensure_bank_can_transfer(value: BalanceOf<T>) -> Result<(), Error<T>> {
-            let minimum_balance = CurrencyOf::<T>::minimum_balance()
-                .saturating_add(UnusedValue::<T>::get())
-                .saturating_add(OnFinalizeValue::<T>::get());
+            let available_balance = <CurrencyOf<T> as fungible::Inspect<_>>::reducible_balance(
+                &T::BankAddress::get(),
+                Preservation::Expendable,
+                Fortitude::Polite,
+            )
+            .saturating_sub(UnusedValue::<T>::get())
+            .saturating_sub(OnFinalizeValue::<T>::get());
 
-            CurrencyOf::<T>::free_balance(&T::BankAddress::get())
-                .checked_sub(&value)
-                .map_or(false, |balance| balance >= minimum_balance)
+            (value <= available_balance)
                 .then_some(())
                 .ok_or(Error::<T>::InsufficientBankBalance)
         }
@@ -258,13 +274,17 @@ pub mod pallet {
         fn withdraw(account_id: &AccountIdOf<T>, value: BalanceOf<T>) -> Result<(), Error<T>> {
             Self::ensure_bank_can_transfer(value)?;
 
-            // The check is similar to one that used in transfer implementation.
-            // It allows us define if we cannot transfer funds on early stage
-            // to be able mean any transfer error as insufficient bank
-            // account balance, because other conditions are checked
-            // here and in other caller places.
-            if CurrencyOf::<T>::free_balance(account_id).saturating_add(value)
-                < CurrencyOf::<T>::minimum_balance()
+            // Since funds are not being minted here but transferred, the only error we can
+            // possibly observe is the `TokenError::BelowMinimum` one (no overflow whatsoever).
+            // It means we can check for the outcome being just any error and be sure it is
+            // that the recipient account would die as a result of this transfer.
+            if <CurrencyOf<T> as fungible::Inspect<_>>::can_deposit(
+                account_id,
+                value,
+                Provenance::Extant,
+            )
+            .into_result()
+            .is_err()
             {
                 UnusedValue::<T>::mutate(|unused_value| {
                     *unused_value = unused_value.saturating_add(value);

pallets/gear-bank/src/tests.rs

@@ -737,12 +737,26 @@ fn deposit_value_zero() {
 }
 
 #[test]
-fn deposit_value_insufficient_balance() {
+fn deposit_value_overflow() {
     new_test_ext().execute_with(|| {
         const VALUE: Balance = Balance::MAX;
 
         assert!(VALUE > Balances::free_balance(ALICE));
 
+        assert_noop!(
+            GearBank::deposit_value(&ALICE, VALUE, false),
+            Error::<Test>::Overflow
+        );
+    })
+}
+
+#[test]
+fn deposit_value_insufficient_balance() {
+    new_test_ext().execute_with(|| {
+        const VALUE: Balance = Balance::MAX / 2;
+
+        assert!(VALUE > Balances::free_balance(ALICE));
+
         assert_noop!(
             GearBank::deposit_value(&ALICE, VALUE, false),
             Error::<Test>::InsufficientBalance
@@ -1563,6 +1577,7 @@ mod utils {
                 Self::InsufficientGasBalance => matches!(other, Self::InsufficientGasBalance),
                 Self::InsufficientValueBalance => matches!(other, Self::InsufficientValueBalance),
                 Self::InsufficientDeposit => matches!(other, Self::InsufficientDeposit),
+                Self::Overflow => matches!(other, Self::Overflow),
                 _ => unimplemented!(),
             }
         }