From 356472c51bda8f18d559f2414b701bd438102d47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Wed, 7 Aug 2024 02:11:55 -0500 Subject: [PATCH] [Assets] Call implementation for `transfer_all` (#4527) Closes #4517 Polkadot address: 12gMhxHw8QjEwLQvnqsmMVY1z5gFa54vND74aMUbhhwN6mJR --------- Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> --- .../src/weights/pallet_assets_foreign.rs | 10 ++++ .../src/weights/pallet_assets_local.rs | 10 ++++ .../src/weights/pallet_assets_pool.rs | 10 ++++ .../src/weights/pallet_assets_foreign.rs | 10 ++++ .../src/weights/pallet_assets_local.rs | 10 ++++ .../src/weights/pallet_assets_pool.rs | 10 ++++ prdoc/pr_4527.prdoc | 21 ++++++++ substrate/frame/assets/src/benchmarking.rs | 13 ++++- substrate/frame/assets/src/lib.rs | 49 ++++++++++++++++++- substrate/frame/assets/src/tests.rs | 43 ++++++++++++++++ substrate/frame/assets/src/weights.rs | 21 ++++++++ 11 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 prdoc/pr_4527.prdoc diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_foreign.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_foreign.rs index 5148edb0ee9e2..c76c1137335a1 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_foreign.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_foreign.rs @@ -531,4 +531,14 @@ impl pallet_assets::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_local.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_local.rs index 4ee235830aed2..cf4f60042bc68 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_local.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_local.rs @@ -528,4 +528,14 @@ impl pallet_assets::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_pool.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_pool.rs index df7ad2c633867..2cd85de009895 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_pool.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_pool.rs @@ -528,4 +528,14 @@ impl pallet_assets::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_foreign.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_foreign.rs index 52ba2fd6c40fc..2692de9aeb50d 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_foreign.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_foreign.rs @@ -537,4 +537,14 @@ impl pallet_assets::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_local.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_local.rs index e78366b91cbe8..d2e12549a45c7 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_local.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_local.rs @@ -535,4 +535,14 @@ impl pallet_assets::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_pool.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_pool.rs index 65cae81069c40..8368f6e583ccf 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_pool.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_assets_pool.rs @@ -529,4 +529,14 @@ impl pallet_assets::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } diff --git a/prdoc/pr_4527.prdoc b/prdoc/pr_4527.prdoc new file mode 100644 index 0000000000000..12056f87575b8 --- /dev/null +++ b/prdoc/pr_4527.prdoc @@ -0,0 +1,21 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Call implementation for `transfer_all` + +doc: + - audience: Runtime Dev + description: | + This PR introduces the `transfer_all` call for `pallet-assets`. + The parameters are analog to the same call in `pallet-balances`. + This change is expected to be backwards-compatible. + This change requires running benchmarkings to set accurate weights for + the call. + +crates: + - name: pallet-assets + bump: major + - name: asset-hub-rococo-runtime + bump: minor + - name: asset-hub-westend-runtime + bump: minor diff --git a/substrate/frame/assets/src/benchmarking.rs b/substrate/frame/assets/src/benchmarking.rs index 97cc04174a0c6..8988323c19851 100644 --- a/substrate/frame/assets/src/benchmarking.rs +++ b/substrate/frame/assets/src/benchmarking.rs @@ -31,6 +31,7 @@ use sp_runtime::traits::Bounded; use crate::Pallet as Assets; const SEED: u32 = 0; +const MIN_BALANCE: u32 = 1; fn default_asset_id, I: 'static>() -> T::AssetIdParameter { T::BenchmarkHelper::create_asset_id_parameter(0) @@ -48,7 +49,7 @@ fn create_default_asset, I: 'static>( asset_id.clone(), caller_lookup.clone(), is_sufficient, - 1u32.into(), + MIN_BALANCE.into(), ) .is_ok()); (asset_id, caller, caller_lookup) @@ -553,5 +554,15 @@ benchmarks_instance_pallet! { assert_last_event::(Event::Blocked { asset_id: asset_id.into(), who: caller }.into()); } + transfer_all { + let amount = T::Balance::from(2 * MIN_BALANCE); + let (asset_id, caller, caller_lookup) = create_default_minted_asset::(true, amount); + let target: T::AccountId = account("target", 0, SEED); + let target_lookup = T::Lookup::unlookup(target.clone()); + }: _(SystemOrigin::Signed(caller.clone()), asset_id.clone(), target_lookup, false) + verify { + assert_last_event::(Event::Transferred { asset_id: asset_id.into(), from: caller, to: target, amount }.into()); + } + impl_benchmark_test_suite!(Assets, crate::mock::new_test_ext(), crate::mock::Test) } diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index b9b5b2388dfbe..e9b9a7b1e3cc7 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -183,7 +183,11 @@ use frame_support::{ pallet_prelude::DispatchResultWithPostInfo, storage::KeyPrefixIterator, traits::{ - tokens::{fungibles, DepositConsequence, WithdrawConsequence}, + tokens::{ + fungibles, DepositConsequence, Fortitude, + Preservation::{Expendable, Preserve}, + WithdrawConsequence, + }, BalanceStatus::Reserved, Currency, EnsureOriginWithArg, Incrementable, ReservableCurrency, StoredMap, }, @@ -1753,6 +1757,49 @@ pub mod pallet { Self::deposit_event(Event::::Blocked { asset_id: id, who }); Ok(()) } + + /// Transfer the entire transferable balance from the caller asset account. + /// + /// NOTE: This function only attempts to transfer _transferable_ balances. This means that + /// any held, frozen, or minimum balance (when `keep_alive` is `true`), will not be + /// transferred by this function. To ensure that this function results in a killed account, + /// you might need to prepare the account by removing any reference counters, storage + /// deposits, etc... + /// + /// The dispatch origin of this call must be Signed. + /// + /// - `id`: The identifier of the asset for the account holding a deposit. + /// - `dest`: The recipient of the transfer. + /// - `keep_alive`: A boolean to determine if the `transfer_all` operation should send all + /// of the funds the asset account has, causing the sender asset account to be killed + /// (false), or transfer everything except at least the minimum balance, which will + /// guarantee to keep the sender asset account alive (true). + #[pallet::call_index(32)] + #[pallet::weight(T::WeightInfo::refund_other())] + pub fn transfer_all( + origin: OriginFor, + id: T::AssetIdParameter, + dest: AccountIdLookupOf, + keep_alive: bool, + ) -> DispatchResult { + let transactor = ensure_signed(origin)?; + let keep_alive = if keep_alive { Preserve } else { Expendable }; + let reducible_balance = >::reducible_balance( + id.clone().into(), + &transactor, + keep_alive, + Fortitude::Polite, + ); + let dest = T::Lookup::lookup(dest)?; + >::transfer( + id.into(), + &transactor, + &dest, + reducible_balance, + keep_alive, + )?; + Ok(()) + } } /// Implements [`AccountTouch`] trait. diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs index c751fbdcaf1bb..af605c5a3c640 100644 --- a/substrate/frame/assets/src/tests.rs +++ b/substrate/frame/assets/src/tests.rs @@ -799,6 +799,49 @@ fn transferring_to_blocked_account_should_not_work() { }); } +#[test] +fn transfer_all_works_1() { + new_test_ext().execute_with(|| { + // setup + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 0, true, 100)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(0), 0, 1, 200)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(0), 0, 2, 100)); + // transfer all and allow death + assert_ok!(Assets::transfer_all(Some(1).into(), 0, 2, false)); + assert_eq!(Assets::balance(0, &1), 0); + assert_eq!(Assets::balance(0, &2), 300); + }); +} + +#[test] +fn transfer_all_works_2() { + new_test_ext().execute_with(|| { + // setup + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 0, true, 100)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(0), 0, 1, 200)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(0), 0, 2, 100)); + // transfer all and allow death + assert_ok!(Assets::transfer_all(Some(1).into(), 0, 2, true)); + assert_eq!(Assets::balance(0, &1), 100); + assert_eq!(Assets::balance(0, &2), 200); + }); +} + +#[test] +fn transfer_all_works_3() { + new_test_ext().execute_with(|| { + // setup + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 0, true, 100)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(0), 0, 1, 210)); + set_frozen_balance(0, 1, 10); + assert_ok!(Assets::mint(RuntimeOrigin::signed(0), 0, 2, 100)); + // transfer all and allow death w/ frozen + assert_ok!(Assets::transfer_all(Some(1).into(), 0, 2, false)); + assert_eq!(Assets::balance(0, &1), 110); + assert_eq!(Assets::balance(0, &2), 200); + }); +} + #[test] fn origin_guards_should_work() { new_test_ext().execute_with(|| { diff --git a/substrate/frame/assets/src/weights.rs b/substrate/frame/assets/src/weights.rs index 7886cd364d516..57f7e951b73c4 100644 --- a/substrate/frame/assets/src/weights.rs +++ b/substrate/frame/assets/src/weights.rs @@ -83,6 +83,7 @@ pub trait WeightInfo { fn refund() -> Weight; fn refund_other() -> Weight; fn block() -> Weight; + fn transfer_all() -> Weight; } /// Weights for `pallet_assets` using the Substrate node and recommended hardware. @@ -530,6 +531,16 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } // For backwards compatibility and tests. @@ -976,4 +987,14 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } + + fn transfer_all() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `3593` + // Minimum execution time: 46_573_000 picoseconds. + Weight::from_parts(47_385_000, 3593) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } }