Skip to content

Commit

Permalink
Account for a program balance being potentially locked throughout its…
Browse files Browse the repository at this point in the history
… lifecycle
  • Loading branch information
ekovalev committed Apr 20, 2024
1 parent e599ae9 commit 1c00532
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 56 deletions.
67 changes: 36 additions & 31 deletions pallets/gear-bank/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ mod tests;

pub use pallet::*;

use frame_support::traits::{Currency, StorageVersion};
use frame_support::traits::{fungible, tokens::Provenance, Currency, StorageVersion};

#[macro_export]
macro_rules! impl_config {
Expand Down Expand Up @@ -57,14 +57,14 @@ pub mod pallet {
use frame_support::{
ensure,
pallet_prelude::{StorageMap, StorageValue, ValueQuery},
sp_runtime::{traits::CheckedSub, Saturating},
traits::{ExistenceRequirement, Get, ReservableCurrency, WithdrawReasons},
sp_runtime::Saturating,
traits::{ExistenceRequirement, Get, ReservableCurrency},
Identity,
};
use pallet_authorship::Pallet as Authorship;
use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
use sp_runtime::traits::Zero;
use sp_runtime::{traits::Zero, ArithmeticError, DispatchError, TokenError};

// Funds pallet struct itself.
#[pallet::pallet]
Expand All @@ -75,7 +75,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.
Expand All @@ -101,6 +102,8 @@ 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.
Overflow,
}

/// Type containing info of locked in special address funds of each account.
Expand Down Expand Up @@ -175,11 +178,17 @@ 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
);
<CurrencyOf<T> as fungible::Inspect<_>>::can_deposit(
&bank_address,
value,
Provenance::Extant,
)
.into_result()
.map_err(|e| match e {
DispatchError::Token(TokenError::BelowMinimum) => Error::<T>::InsufficientDeposit,
DispatchError::Arithmetic(ArithmeticError::Overflow) => Error::<T>::Overflow,
_ => unreachable!("Unexpected error: {e:?}"),
})?;

let existence_requirement = if keep_alive {
ExistenceRequirement::KeepAlive
Expand All @@ -192,36 +201,32 @@ pub mod pallet {
.map_err(|_| Error::<T>::InsufficientBalance)
}

/// Ensures that bank account is able to transfer requested value.
/// Ensures that bank account is able to transfer requested value
/// while keeping its account alive as a result of this transfer.
fn ensure_bank_can_transfer(value: BalanceOf<T>) -> Result<(), Error<T>> {
let bank_address = T::BankAddress::get();

CurrencyOf::<T>::free_balance(&bank_address)
.checked_sub(&value)
.map_or(false, |new_balance| {
CurrencyOf::<T>::ensure_can_withdraw(
&bank_address,
value,
WithdrawReasons::TRANSFER,
new_balance,
)
.is_ok()
})
.then_some(())
.ok_or(Error::<T>::InsufficientBankBalance)
<CurrencyOf<T> as fungible::Inspect<_>>::can_withdraw(&bank_address, value)
.into_result(true)
.map_err(|_| Error::<T>::InsufficientBankBalance)
.map(|_| ())
}

/// Transfers value from bank address to `account_id`.
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 the 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);
Expand Down
17 changes: 16 additions & 1 deletion pallets/gear-bank/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,12 +729,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
Expand Down Expand Up @@ -1464,6 +1478,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!(),
}
}
Expand Down
10 changes: 7 additions & 3 deletions pallets/gear/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ use frame_support::{
dispatch::{DispatchResultWithPostInfo, PostDispatchInfo},
ensure,
pallet_prelude::*,
traits::{ConstBool, Currency, ExistenceRequirement, Get, Randomness, StorageVersion},
traits::{
fungible,
tokens::{Fortitude, Preservation},
ConstBool, Currency, ExistenceRequirement, Get, Randomness, StorageVersion,
},
weights::Weight,
};
use frame_system::{
Expand Down Expand Up @@ -1656,8 +1660,6 @@ pub mod pallet {
let who = origin;
let origin = who.clone().into_origin();

Self::check_gas_limit_and_value(gas_limit, value)?;

let message = HandleMessage::from_packet(
Self::next_message_id(origin),
HandlePacket::new_with_gas(
Expand All @@ -1675,6 +1677,8 @@ pub mod pallet {
Error::<T>::InactiveProgram
);

Self::check_gas_limit_and_value(gas_limit, value)?;

// Message is not guaranteed to be executed, that's why value is not immediately transferred.
// That's because destination can fail to be initialized, while this dispatch message is next
// in the queue.
Expand Down
22 changes: 18 additions & 4 deletions pallets/gear/src/manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ use gear_core_errors::{ReplyCode, SignalCode};
pub use task::*;

use crate::{
BuiltinDispatcherFactory, Config, CurrencyOf, Event, GasHandlerOf, Pallet, ProgramStorageOf,
QueueOf, TaskPoolOf, WaitlistOf,
fungible, BuiltinDispatcherFactory, Config, CurrencyOf, Event, Fortitude, GasHandlerOf, Pallet,
Preservation, ProgramStorageOf, QueueOf, TaskPoolOf, WaitlistOf,
};
use common::{
event::*,
Expand Down Expand Up @@ -228,7 +228,13 @@ where
let active: ActiveProgram<_> = ProgramStorageOf::<T>::get_program(id)?.try_into().ok()?;
let code_id = active.code_hash.cast();

let balance = CurrencyOf::<T>::free_balance(&id.cast()).unique_saturated_into();
// Actor can only use so much of its balance that can be spent keeping the account alive.
let balance = <CurrencyOf<T> as fungible::Inspect<_>>::reducible_balance(
&id.cast(),
Preservation::Preserve,
Fortitude::Polite,
)
.unique_saturated_into();

Some(Actor {
balance,
Expand Down Expand Up @@ -377,7 +383,15 @@ where
ProgramStorageOf::<T>::remove_program_pages(program_id, memory_infix);

let program_account = program_id.cast();
let balance = CurrencyOf::<T>::free_balance(&program_account);

// Only `reducible_balance` is allowed to be transferred out, even if a part of the
// `free` balance of a deactivated program is still `frozen` for some reason.
// Note: the preservation requirement here actually allows the account to be removed.
let balance = <CurrencyOf<T> as fungible::Inspect<_>>::reducible_balance(
&program_account,
Preservation::Expendable,
Fortitude::Polite,
);
if !balance.is_zero() {
let destination = Pallet::<T>::inheritor_for(value_destination).cast();

Expand Down
6 changes: 5 additions & 1 deletion pallets/gear/src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,11 @@ where
continue;
}

let balance = CurrencyOf::<T>::free_balance(&program_id.cast());
let balance = <CurrencyOf<T> as fungible::Inspect<T::AccountId>>::reducible_balance(
&program_id.cast(),
Preservation::Expendable,
Fortitude::Polite,
);

let journal = Self::run_queue_step(QueueStep {
block_config: &block_config,
Expand Down
18 changes: 15 additions & 3 deletions pallets/gear/src/runtime_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,13 @@ where
gas: u64,
value: BalanceOf<T>,
) -> RawOrigin<AccountIdOf<T>> {
// Querying balance of the account.
let origin_balance = CurrencyOf::<T>::free_balance(&origin);
// Querying transferrable balance of the origin account taking into account a possibility
// of a part of the `origin`'s `free` balance being `frozen`.
let origin_balance = <CurrencyOf<T> as fungible::Inspect<_>>::reducible_balance(
&origin,
Preservation::Preserve,
Fortitude::Polite,
);

// Calculating amount of value to be paid for gas.
let value_for_gas = <T as pallet_gear_bank::Config>::GasMultiplier::get().gas_to_value(gas);
Expand Down Expand Up @@ -625,13 +630,20 @@ where
block_config.forbidden_funcs = forbidden_funcs;
}

// Program's balance that it can spend during the execution keeping its account alive.
let disposable_balance = <CurrencyOf<T> as fungible::Inspect<_>>::reducible_balance(
&destination.cast(),
Preservation::Preserve,
Fortitude::Polite,
);

// Processing of the message, if destination is common program.
let journal = Self::run_queue_step(QueueStep {
block_config: &block_config,
ext_manager,
gas_limit,
dispatch,
balance: CurrencyOf::<T>::free_balance(&destination.cast()).unique_saturated_into(),
balance: disposable_balance.unique_saturated_into(),
});

Ok(Some((processed, journal, false)))
Expand Down
Loading

0 comments on commit 1c00532

Please sign in to comment.