From 36d4a67c7a1bd14b0e1249bb54d123e54a2f253b Mon Sep 17 00:00:00 2001 From: playX18 <158266309+playX18@users.noreply.github.com> Date: Tue, 10 Sep 2024 17:07:45 +0700 Subject: [PATCH] chore(gear): replace dust handling entirely with gear-bank pallet (#4112) --- pallets/gear/src/internal.rs | 61 +++++++++++++----------------------- pallets/gear/src/mock.rs | 17 ++-------- pallets/gear/src/tests.rs | 11 +++---- 3 files changed, 28 insertions(+), 61 deletions(-) diff --git a/pallets/gear/src/internal.rs b/pallets/gear/src/internal.rs index b1bec73c1d4..d10ace6a605 100644 --- a/pallets/gear/src/internal.rs +++ b/pallets/gear/src/internal.rs @@ -19,9 +19,9 @@ //! Internal details of Gear Pallet implementation. use crate::{ - AccountIdOf, Config, CostsPerBlockOf, CurrencyOf, DispatchStashOf, Event, ExtManager, - GasBalanceOf, GasHandlerOf, GasNodeIdOf, GearBank, MailboxOf, Pallet, QueueOf, - SchedulingCostOf, TaskPoolOf, WaitlistOf, + AccountIdOf, Config, CostsPerBlockOf, DispatchStashOf, Event, ExtManager, GasBalanceOf, + GasHandlerOf, GasNodeIdOf, GearBank, MailboxOf, Pallet, QueueOf, SchedulingCostOf, TaskPoolOf, + WaitlistOf, }; use alloc::{collections::BTreeSet, format}; use common::{ @@ -38,7 +38,6 @@ use core::{ cmp::{Ord, Ordering}, num::NonZeroUsize, }; -use frame_support::traits::{fungible, Currency, ExistenceRequirement}; use frame_system::pallet_prelude::BlockNumberFor; use gear_core::{ ids::{prelude::*, MessageId, ProgramId, ReservationId}, @@ -47,10 +46,7 @@ use gear_core::{ UserStoredMessage, }, }; -use sp_runtime::{ - traits::{Get, One, SaturatedConversion, Saturating, UniqueSaturatedInto, Zero}, - DispatchError, TokenError, -}; +use sp_runtime::traits::{Get, One, SaturatedConversion, Saturating, UniqueSaturatedInto, Zero}; type MailboxError = <<::Messenger as Messenger>::Mailbox as Mailbox>::OutputError; type WaitlistError = @@ -976,6 +972,16 @@ where let to = message.destination().cast::(); let value = message.value().unique_saturated_into(); + // Reserving value from source for future transfer or unreserve. + GearBank::::deposit_value(&from, value, false).unwrap_or_else(|e| { + let err_msg = format!( + "send_user_message: failed depositting value on gear bank. \ + From - {from:?}, value - {value:?}. Got error - {e:?}", + ); + + log::error!("{err_msg}"); + unreachable!("{err_msg}"); + }); // If gas limit can cover threshold, message will be added to mailbox, // task created and funds reserved. let expiration = if message.details().is_none() && gas_limit >= threshold { @@ -1007,17 +1013,6 @@ where unreachable!("{err_msg}"); }); - // Reserving value from source for future transfer or unreserve. - GearBank::::deposit_value(&from, value, false).unwrap_or_else(|e| { - let err_msg = format!( - "send_user_message: failed depositting value on gear bank. \ - From - {from:?}, value - {value:?}. Got error - {e:?}", - ); - - log::error!("{err_msg}"); - unreachable!("{err_msg}"); - }); - // Lock the entire `gas_limit` since the only purpose of it is payment for storage. GasHandlerOf::::lock(message.id(), LockId::Mailbox, gas_limit).unwrap_or_else(|e| { let err_msg = format!( @@ -1077,28 +1072,14 @@ where // Permanently transferring funds. // Note that we have no guarantees of the user account to exist. Since no minimum // transfer value is enforced, the transfer can fail. Handle it gracefully. - // TODO #4018 Introduce a safer way to handle this. - CurrencyOf::::transfer(&from, &to, value, ExistenceRequirement::AllowDeath) - .unwrap_or_else(|e| match e { - DispatchError::Token(TokenError::BelowMinimum) => { - log::debug!( - "Failed to transfer value: {:?}. User account balance is too low.", - e - ); - as fungible::Unbalanced<_>>::handle_dust(fungible::Dust( - value, - )); - } - e => { - // Other errors are ruled out by the protocol guarantees. - let err_msg = format!( - "send_user_message: failed to transfer value. Got error: {e:?}" - ); + GearBank::::transfer_value(&from, &to, value).unwrap_or_else(|e| { + // errors are ruled out by the protocol guarantees. + let err_msg = + format!("send_user_message: failed to transfer value. Got error: {e:?}"); - log::error!("{err_msg}"); - unreachable!("{err_msg}"); - } - }); + log::error!("{err_msg}"); + unreachable!("{err_msg}"); + }); if message.details().is_none() { // Creating auto reply message. diff --git a/pallets/gear/src/mock.rs b/pallets/gear/src/mock.rs index a66c04f9e07..c229f625596 100644 --- a/pallets/gear/src/mock.rs +++ b/pallets/gear/src/mock.rs @@ -23,7 +23,7 @@ use frame_support::{ construct_runtime, pallet_prelude::*, parameter_types, - traits::{ConstU64, FindAuthor, Get, OnUnbalanced}, + traits::{ConstU64, FindAuthor, Get}, PalletId, }; use frame_support_test::TestRandomness; @@ -44,7 +44,6 @@ pub type BlockNumber = BlockNumberFor; type Balance = u128; type BlockWeightsOf = ::BlockWeights; -type CreditOf = fungible::Credit<::AccountId, Balances>; pub(crate) const USER_1: AccountId = 1; pub(crate) const USER_2: AccountId = 2; @@ -52,7 +51,6 @@ pub(crate) const USER_3: AccountId = 3; pub(crate) const LOW_BALANCE_USER: AccountId = 4; pub(crate) const BLOCK_AUTHOR: AccountId = 255; pub(crate) const RENT_POOL: AccountId = 256; -pub(crate) const DUST_TRAP_TARGET: AccountId = 999; macro_rules! dry_run { ( @@ -97,17 +95,7 @@ pallet_gear::impl_config!(Test, Schedule = DynamicSchedule, RentPoolId = ConstU6 pallet_gear_gas::impl_config!(Test); common::impl_pallet_authorship!(Test); common::impl_pallet_timestamp!(Test); - -pub struct DustTrap; - -impl OnUnbalanced> for DustTrap { - fn on_nonzero_unbalanced(amount: CreditOf) { - let result = >::resolve(&DUST_TRAP_TARGET, amount); - debug_assert!(result.is_ok()); - } -} - -common::impl_pallet_balances!(Test, DustRemoval = DustTrap); +common::impl_pallet_balances!(Test); parameter_types! { pub const BlockHashCount: BlockNumber = 250; @@ -210,7 +198,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities { (BLOCK_AUTHOR, 500_000_u128), (RENT_POOL, ExistentialDeposit::get()), (BankAddress::get(), ExistentialDeposit::get()), - (DUST_TRAP_TARGET, ExistentialDeposit::get()), ], } .assimilate_storage(&mut t) diff --git a/pallets/gear/src/tests.rs b/pallets/gear/src/tests.rs index 22f0c7b3418..410bee73d21 100644 --- a/pallets/gear/src/tests.rs +++ b/pallets/gear/src/tests.rs @@ -24,7 +24,7 @@ use crate::{ self, new_test_ext, run_for_blocks, run_to_block, run_to_block_maybe_with_queue, run_to_next_block, Balances, BlockNumber, DynamicSchedule, Gear, GearVoucher, RuntimeEvent as MockRuntimeEvent, RuntimeOrigin, System, Test, Timestamp, BLOCK_AUTHOR, - DUST_TRAP_TARGET, LOW_BALANCE_USER, RENT_POOL, USER_1, USER_2, USER_3, + LOW_BALANCE_USER, RENT_POOL, USER_1, USER_2, USER_3, }, pallet, runtime_api::{ALLOWANCE_LIMIT_ERR, RUNTIME_API_BLOCK_LIMITS_COUNT}, @@ -15666,8 +15666,6 @@ fn dust_in_message_to_user_handled_ok() { init_logger(); new_test_ext().execute_with(|| { - let ed = CurrencyOf::::minimum_balance(); - let pid = Gear::upload_program( RuntimeOrigin::signed(USER_1), WASM_BINARY.to_vec(), @@ -15698,9 +15696,10 @@ fn dust_in_message_to_user_handled_ok() { run_to_block(3, None); - // USER_1 account doesn't receive the funds; instead, the dust handler kicks in. + // USER_1 account doesn't receive the funds; instead, the value is below ED so + // account dies and value goes to UnusedValue assert_eq!(CurrencyOf::::free_balance(USER_1), 0); - assert_eq!(CurrencyOf::::free_balance(DUST_TRAP_TARGET), ed + 300); + assert_eq!(pallet_gear_bank::UnusedValue::::get(), 300); // Test case 2: Make the program send a message to USER_1 with the value below the ED // and gas sufficient for a message to be placed into the mailbox (for 30 blocks). @@ -15718,7 +15717,7 @@ fn dust_in_message_to_user_handled_ok() { // USER_1 account doesn't receive the funds again; instead, the value is stored as the // `UnusedValue` in Gear Bank. assert_eq!(CurrencyOf::::free_balance(USER_1), 0); - assert_eq!(pallet_gear_bank::UnusedValue::::get(), 300); + assert_eq!(pallet_gear_bank::UnusedValue::::get(), 600); }); }