Merge branch 'master' into cl/issue-3661

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace.package]
-version = "1.3.0"
+version = "1.3.1"
 authors = ["Gear Technologies"]
 edition = "2021"
 license = "GPL-3.0"

gsdk/src/metadata/generated.rs

@@ -8740,13 +8740,17 @@ pub mod storage {
     pub enum GearBankStorage {
         Bank,
         UnusedValue,
+        OnFinalizeTransfers,
+        OnFinalizeValue,
     }
     impl StorageInfo for GearBankStorage {
         const PALLET: &'static str = "GearBank";
         fn storage_name(&self) -> &'static str {
             match self {
                 Self::Bank => "Bank",
                 Self::UnusedValue => "UnusedValue",
+                Self::OnFinalizeTransfers => "OnFinalizeTransfers",
+                Self::OnFinalizeValue => "OnFinalizeValue",
             }
         }
     }

pallets/gear-bank/src/lib.rs

@@ -58,9 +58,11 @@ pub mod pallet {
         ensure,
         pallet_prelude::{StorageMap, StorageValue, ValueQuery},
         sp_runtime::{traits::CheckedSub, Saturating},
-        traits::{ExistenceRequirement, Get, ReservableCurrency, WithdrawReasons},
+        traits::{ExistenceRequirement, Get, Hooks, ReservableCurrency},
+        weights::Weight,
         Identity,
     };
+    use frame_system::pallet_prelude::BlockNumberFor;
     use pallet_authorship::Pallet as Authorship;
     use parity_scale_codec::{Decode, Encode, EncodeLike, MaxEncodedLen};
     use scale_info::TypeInfo;
@@ -158,12 +160,61 @@ pub mod pallet {
 
     // Private storage that keeps account bank details.
     #[pallet::storage]
-    pub type Bank<T> = StorageMap<_, Identity, AccountIdOf<T>, BankAccount<BalanceOf<T>>>;
+    type Bank<T> = StorageMap<_, Identity, AccountIdOf<T>, BankAccount<BalanceOf<T>>>;
 
     // Private storage that keeps amount of value that wasn't sent because owner is inexistent account.
     #[pallet::storage]
     pub type UnusedValue<T> = StorageValue<_, BalanceOf<T>, ValueQuery>;
 
+    // Private storage that keeps registry of transfers to be performed at the end of the block.
+    #[pallet::storage]
+    type OnFinalizeTransfers<T> = StorageMap<_, Identity, AccountIdOf<T>, BalanceOf<T>>;
+
+    // Private storage that represents sum of values in OnFinalizeTransfers.
+    #[pallet::storage]
+    pub(crate) type OnFinalizeValue<T> = StorageValue<_, BalanceOf<T>, ValueQuery>;
+
+    #[pallet::hooks]
+    impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+        /// Start of the block.
+        fn on_initialize(bn: BlockNumberFor<T>) -> Weight {
+            if OnFinalizeTransfers::<T>::iter().next().is_some() {
+                log::error!("Block #{bn:?} started with non-empty on-finalize transfers");
+            }
+
+            if !OnFinalizeValue::<T>::get().is_zero() {
+                log::error!("Block #{bn:?} started with non-zero on-finalize value");
+            }
+
+            T::DbWeight::get().reads(2)
+        }
+
+        /// End of the block.
+        fn on_finalize(bn: BlockNumberFor<T>) {
+            // Take of on-finalize value should always be performed before
+            // `withdraw`s, since `withdraw`s ensure bank balance,
+            // that relies on that value "locked".
+            let expected = OnFinalizeValue::<T>::take();
+
+            let mut total = BalanceOf::<T>::zero();
+
+            while let Some((account_id, value)) = OnFinalizeTransfers::<T>::drain().next() {
+                total = total.saturating_add(value);
+
+                if let Err(e) = Self::withdraw(&account_id, value) {
+                    log::error!(
+                        "Block #{bn:?} ended with unreachable error while performing on-finalize transfer to {account_id:?}: {e:?}"
+                    );
+                }
+            }
+
+            if total != expected {
+                log::error!("Block #{bn:?} ended with unreachable error while performing cleaning of on-finalize value: \
+                total tried to transfer is {total:?}, expected amount is {expected:?}")
+            }
+        }
+    }
+
     impl<T: Config> Pallet<T> {
         /// Transfers value from `account_id` to bank address.
         fn deposit(
@@ -192,19 +243,13 @@ 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 bank_address = T::BankAddress::get();
+            let minimum_balance = CurrencyOf::<T>::minimum_balance()
+                .saturating_add(UnusedValue::<T>::get())
+                .saturating_add(OnFinalizeValue::<T>::get());
 
-            CurrencyOf::<T>::free_balance(&bank_address)
+            CurrencyOf::<T>::free_balance(&T::BankAddress::get())
                 .checked_sub(&value)
-                .map_or(false, |new_balance| {
-                    CurrencyOf::<T>::ensure_can_withdraw(
-                        &bank_address,
-                        value,
-                        WithdrawReasons::TRANSFER,
-                        new_balance,
-                    )
-                    .is_ok()
-                })
+                .map_or(false, |balance| balance >= minimum_balance)
                 .then_some(())
                 .ok_or(Error::<T>::InsufficientBankBalance)
         }
@@ -239,6 +284,26 @@ pub mod pallet {
             .map_err(|_| Error::<T>::InsufficientBankBalance)
         }
 
+        /// Transfers value from bank address to `account_id` on block finalize.
+        fn withdraw_on_finalize(
+            account_id: &AccountIdOf<T>,
+            value: BalanceOf<T>,
+        ) -> Result<(), Error<T>> {
+            if value.is_zero() {
+                return Ok(());
+            };
+
+            Self::ensure_bank_can_transfer(value)?;
+
+            OnFinalizeValue::<T>::mutate(|v| *v = v.saturating_add(value));
+            OnFinalizeTransfers::<T>::mutate(account_id, |v| {
+                let inner = v.get_or_insert(Zero::zero());
+                *inner = inner.saturating_add(value);
+            });
+
+            Ok(())
+        }
+
         pub fn deposit_gas(
             account_id: &AccountIdOf<T>,
             amount: u64,
@@ -337,11 +402,8 @@ pub mod pallet {
 
             let value = Self::withdraw_gas_no_transfer(account_id, amount, multiplier)?;
 
-            // All the checks and internal values withdrawals performed in
-            // `*_no_transfer` function above.
-            //
-            // This call does only currency trait final transfer.
-            Self::withdraw(to, value).unwrap_or_else(|e| unreachable!("qed above: {e:?}"));
+            Self::withdraw_on_finalize(to, value)
+                .unwrap_or_else(|e| unreachable!("qed above: {e:?}"));
 
             Ok(())
         }

pallets/gear-bank/src/tests.rs

@@ -16,8 +16,8 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-use crate::{mock::*, GasMultiplier, UnusedValue, *};
-use frame_support::{assert_noop, assert_ok};
+use crate::{mock::*, GasMultiplier, OnFinalizeValue, UnusedValue, *};
+use frame_support::{assert_noop, assert_ok, traits::Hooks};
 use sp_runtime::{traits::Zero, StateVersion};
 use utils::*;
 
@@ -403,6 +403,7 @@ fn spend_gas_different_users() {
 
         const ALICE_BURN: u64 = ALICE_GAS - 123_456;
         assert_ok!(GearBank::spend_gas(&ALICE, ALICE_BURN, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(ALICE_GAS - ALICE_BURN + BOB_GAS, 0);
 
@@ -416,6 +417,7 @@ fn spend_gas_different_users() {
 
         const BOB_BURN: u64 = BOB_GAS - 1_234;
         assert_ok!(GearBank::spend_gas(&BOB, BOB_BURN, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(ALICE_GAS - ALICE_BURN + BOB_GAS - BOB_BURN, 0);
 
@@ -437,6 +439,7 @@ fn spend_gas_single_user() {
 
         const BURN_1: u64 = GAS_AMOUNT - 23_456;
         assert_ok!(GearBank::spend_gas(&ALICE, BURN_1, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(GAS_AMOUNT - BURN_1, 0);
 
@@ -447,6 +450,7 @@ fn spend_gas_single_user() {
 
         const BURN_2: u64 = GAS_AMOUNT - BURN_1 - 10_000;
         assert_ok!(GearBank::spend_gas(&ALICE, BURN_2, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(GAS_AMOUNT - BURN_1 - BURN_2, 0);
 
@@ -464,6 +468,7 @@ fn spend_gas_all_balance() {
         assert_ok!(GearBank::deposit_gas(&ALICE, GAS_AMOUNT, false));
 
         assert_ok!(GearBank::spend_gas(&ALICE, GAS_AMOUNT, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(0, 0);
 
@@ -490,6 +495,7 @@ fn spend_gas_all_balance_validator_account_deleted() {
         ));
 
         assert_ok!(GearBank::spend_gas(&ALICE, GAS_AMOUNT, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(0, 0);
 
@@ -510,6 +516,7 @@ fn spend_gas_small_amount() {
         assert_ok!(GearBank::deposit_gas(&ALICE, GAS_AMOUNT, false));
 
         assert_ok!(GearBank::spend_gas(&ALICE, GAS_AMOUNT, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(0, 0);
 
@@ -538,6 +545,7 @@ fn spend_gas_small_amount_validator_account_deleted() {
         ));
 
         assert_ok!(GearBank::spend_gas(&ALICE, GAS_AMOUNT, mult()));
+        GearBank::on_finalize(1);
 
         assert_eq!(UnusedValue::<Test>::get(), GAS_VALUE_AMOUNT);
         assert_balance(&BANK_ADDRESS, EXISTENTIAL_DEPOSIT + GAS_VALUE_AMOUNT);
@@ -1348,6 +1356,7 @@ fn empty_accounts_deleted() {
 
         assert_ok!(GearBank::spend_gas(&ALICE, GAS_AMOUNT, mult()));
         assert!(GearBank::account(ALICE).is_none());
+        GearBank::on_finalize(1);
 
         const VALUE: Balance = 123_456_000;
 
@@ -1384,6 +1393,7 @@ fn empty_zero_accounts_deleted() {
 
         assert_ok!(GearBank::spend_gas(&Zero::zero(), 0, mult()));
         assert!(GearBank::account(<AccountId as Zero>::zero()).is_none());
+        GearBank::on_finalize(1);
 
         assert_ok!(GearBank::deposit_value(&Zero::zero(), 0, false));
         assert!(GearBank::account(<AccountId as Zero>::zero()).is_none());
@@ -1423,6 +1433,7 @@ fn empty_composite_accounts_deleted() {
         const GAS_BURN: u64 = GAS_AMOUNT / 2;
 
         assert_ok!(GearBank::spend_gas(&ALICE, GAS_BURN, mult()));
+        GearBank::on_finalize(1);
 
         assert_bank_balance(GAS_AMOUNT - GAS_BURN, VALUE);
 
@@ -1452,6 +1463,94 @@ fn empty_composite_accounts_deleted() {
     })
 }
 
+#[test]
+fn spend_gas_on_finalize_different_users() {
+    new_test_ext().execute_with(|| {
+        const ALICE_GAS: u64 = 1_234_567;
+        assert_ok!(GearBank::deposit_gas(&ALICE, ALICE_GAS, false));
+
+        const BOB_GAS: u64 = 56_789;
+        assert_ok!(GearBank::deposit_gas(&BOB, BOB_GAS, false));
+
+        assert_eq!(OnFinalizeValue::<Test>::get(), 0);
+
+        const ALICE_BURN: u64 = ALICE_GAS - 123_456;
+        assert_ok!(GearBank::spend_gas(&ALICE, ALICE_BURN, mult()));
+
+        assert_bank_balance(ALICE_GAS - ALICE_BURN + BOB_GAS, 0);
+
+        assert_block_author_inc(0);
+        assert_eq!(OnFinalizeValue::<Test>::get(), gas_price(ALICE_BURN));
+
+        assert_alice_dec(gas_price(ALICE_GAS));
+        assert_gas_value(&ALICE, ALICE_GAS - ALICE_BURN, 0);
+
+        assert_bob_dec(gas_price(BOB_GAS));
+        assert_gas_value(&BOB, BOB_GAS, 0);
+
+        const BOB_BURN: u64 = BOB_GAS - 1_234;
+        assert_ok!(GearBank::spend_gas(&BOB, BOB_BURN, mult()));
+
+        assert_bank_balance(ALICE_GAS - ALICE_BURN + BOB_GAS - BOB_BURN, 0);
+
+        assert_block_author_inc(0);
+        assert_eq!(
+            OnFinalizeValue::<Test>::get(),
+            gas_price(ALICE_BURN + BOB_BURN)
+        );
+
+        assert_alice_dec(gas_price(ALICE_GAS));
+        assert_gas_value(&ALICE, ALICE_GAS - ALICE_BURN, 0);
+
+        assert_bob_dec(gas_price(BOB_GAS));
+        assert_gas_value(&BOB, BOB_GAS - BOB_BURN, 0);
+
+        /* what happens at the end of block */
+        GearBank::on_finalize(1);
+        assert_eq!(OnFinalizeValue::<Test>::get(), 0);
+        assert_block_author_inc(gas_price(ALICE_BURN + BOB_BURN));
+
+        GearBank::on_initialize(2);
+    })
+}
+
+#[test]
+fn spend_gas_on_finalize_single_user() {
+    new_test_ext().execute_with(|| {
+        const GAS_AMOUNT: u64 = 123_456;
+        assert_ok!(GearBank::deposit_gas(&ALICE, GAS_AMOUNT, false));
+
+        const BURN_1: u64 = GAS_AMOUNT - 23_456;
+        assert_ok!(GearBank::spend_gas(&ALICE, BURN_1, mult()));
+
+        assert_bank_balance(GAS_AMOUNT - BURN_1, 0);
+
+        assert_eq!(OnFinalizeValue::<Test>::get(), gas_price(BURN_1));
+        assert_block_author_inc(0);
+
+        assert_alice_dec(gas_price(GAS_AMOUNT));
+        assert_gas_value(&ALICE, GAS_AMOUNT - BURN_1, 0);
+
+        const BURN_2: u64 = GAS_AMOUNT - BURN_1 - 10_000;
+        assert_ok!(GearBank::spend_gas(&ALICE, BURN_2, mult()));
+
+        assert_bank_balance(GAS_AMOUNT - BURN_1 - BURN_2, 0);
+
+        assert_eq!(OnFinalizeValue::<Test>::get(), gas_price(BURN_1 + BURN_2));
+        assert_block_author_inc(0);
+
+        assert_alice_dec(gas_price(GAS_AMOUNT));
+        assert_gas_value(&ALICE, GAS_AMOUNT - BURN_1 - BURN_2, 0);
+
+        /* what happens at the end of block */
+        GearBank::on_finalize(1);
+        assert_eq!(OnFinalizeValue::<Test>::get(), 0);
+        assert_block_author_inc(gas_price(BURN_1 + BURN_2));
+
+        GearBank::on_initialize(2);
+    })
+}
+
 mod utils {
     use super::*;
 
@@ -1491,7 +1590,11 @@ mod utils {
         let gas_value = gas_price(gas);
         assert_balance(
             &BANK_ADDRESS,
-            CurrencyOf::<Test>::minimum_balance() + UnusedValue::<Test>::get() + gas_value + value,
+            CurrencyOf::<Test>::minimum_balance()
+                + UnusedValue::<Test>::get()
+                + OnFinalizeValue::<Test>::get()
+                + gas_value
+                + value,
         );
     }
 

pallets/gear-builtin/src/mock.rs

@@ -302,12 +302,14 @@ pub(crate) fn on_initialize(new_block_number: BlockNumberFor<Test>) {
     GearGas::on_initialize(new_block_number);
     GearMessenger::on_initialize(new_block_number);
     Gear::on_initialize(new_block_number);
+    GearBank::on_initialize(new_block_number);
 }
 
 // Run on_finalize hooks (in pallets reverse order, as they appear in AllPalletsWithSystem)
 pub(crate) fn on_finalize(current_blk: BlockNumberFor<Test>) {
     Authorship::on_finalize(current_blk);
     Gear::on_finalize(current_blk);
+    GearBank::on_finalize(current_blk);
     assert!(!System::events().iter().any(|e| {
         matches!(
             e.event,