From d270e9dd6887203d4878d236f5298345ef0e0a43 Mon Sep 17 00:00:00 2001 From: Raphael Date: Wed, 5 Mar 2025 15:50:26 +0100 Subject: [PATCH 1/3] refactor: batch updating of asset team --- pallets/pallet-bonded-coins/src/lib.rs | 26 ++++----- .../src/tests/transactions/reset_team.rs | 58 ++++++++++++++++--- 2 files changed, 64 insertions(+), 20 deletions(-) diff --git a/pallets/pallet-bonded-coins/src/lib.rs b/pallets/pallet-bonded-coins/src/lib.rs index 469d02089..fc01ae71e 100644 --- a/pallets/pallet-bonded-coins/src/lib.rs +++ b/pallets/pallet-bonded-coins/src/lib.rs @@ -478,31 +478,31 @@ pub mod pallet { origin: OriginFor, pool_id: T::PoolId, team: PoolManagingTeam>, - currency_idx: u32, + currency_count: u32, ) -> DispatchResult { let who = T::DefaultOrigin::ensure_origin(origin)?; let pool_details = Pools::::get(&pool_id).ok_or(Error::::PoolUnknown)?; + let number_of_currencies = Self::get_currencies_number(&pool_details); + ensure!(number_of_currencies <= currency_count, Error::::CurrencyCount); + ensure!(pool_details.is_manager(&who), Error::::NoPermission); ensure!(pool_details.state.is_live(), Error::::PoolNotLive); - let asset_id = pool_details - .bonded_currencies - .get(currency_idx.saturated_into::()) - .ok_or(Error::::IndexOutOfBounds)?; - let pool_id_account = pool_id.into(); let PoolManagingTeam { freezer, admin } = team; - T::Fungibles::reset_team( - asset_id.to_owned(), - pool_id_account.clone(), - admin, - pool_id_account, - freezer, - ) + pool_details.bonded_currencies.into_iter().try_for_each(|asset_id| { + T::Fungibles::reset_team( + asset_id, + pool_id_account.clone(), + admin.clone(), + pool_id_account.clone(), + freezer.clone(), + ) + }) } /// Resets the manager of a pool. The new manager will be set to the diff --git a/pallets/pallet-bonded-coins/src/tests/transactions/reset_team.rs b/pallets/pallet-bonded-coins/src/tests/transactions/reset_team.rs index 6de6eda4f..b8974496e 100644 --- a/pallets/pallet-bonded-coins/src/tests/transactions/reset_team.rs +++ b/pallets/pallet-bonded-coins/src/tests/transactions/reset_team.rs @@ -51,7 +51,7 @@ fn resets_team() { admin: ACCOUNT_00, freezer: ACCOUNT_01, }, - 0 + 1 )); assert_eq!(Assets::admin(DEFAULT_BONDED_CURRENCY_ID), Some(ACCOUNT_00)); @@ -61,6 +61,50 @@ fn resets_team() { }) } +#[test] +fn resets_team_for_all() { + let currencies = vec![DEFAULT_BONDED_CURRENCY_ID, DEFAULT_BONDED_CURRENCY_ID + 1]; + + let pool_details = generate_pool_details( + currencies.clone(), + get_linear_bonding_curve(), + false, + Some(PoolStatus::Active), + Some(ACCOUNT_00), + None, + None, + None, + ); + let pool_id: AccountIdOf = calculate_pool_id(¤cies); + + ExtBuilder::default() + .with_pools(vec![(pool_id.clone(), pool_details)]) + .with_collaterals(vec![DEFAULT_COLLATERAL_CURRENCY_ID]) + .build_and_execute_with_sanity_tests(|| { + let manager_origin = RawOrigin::Signed(ACCOUNT_00).into(); + + assert_ok!(BondingPallet::reset_team( + manager_origin, + pool_id.clone(), + PoolManagingTeam { + admin: ACCOUNT_00, + freezer: ACCOUNT_01, + }, + 2 + )); + + assert_eq!(Assets::admin(currencies[0]), Some(ACCOUNT_00)); + assert_eq!(Assets::freezer(currencies[0]), Some(ACCOUNT_01)); + assert_eq!(Assets::owner(currencies[0]), Some(pool_id.clone())); + assert_eq!(Assets::issuer(currencies[0]), Some(pool_id.clone())); + + assert_eq!(Assets::admin(currencies[1]), Some(ACCOUNT_00)); + assert_eq!(Assets::freezer(currencies[1]), Some(ACCOUNT_01)); + assert_eq!(Assets::owner(currencies[1]), Some(pool_id.clone())); + assert_eq!(Assets::issuer(currencies[1]), Some(pool_id)); + }) +} + #[test] fn does_not_change_team_when_not_live() { let pool_details = generate_pool_details( @@ -89,7 +133,7 @@ fn does_not_change_team_when_not_live() { admin: ACCOUNT_00, freezer: ACCOUNT_00, }, - 0 + 1 ), BondingPalletErrors::::PoolNotLive ); @@ -130,7 +174,7 @@ fn only_manager_can_change_team() { admin: ACCOUNT_00, freezer: ACCOUNT_00, }, - 0 + 1 ), BondingPalletErrors::::NoPermission ); @@ -143,7 +187,7 @@ fn only_manager_can_change_team() { admin: ACCOUNT_00, freezer: ACCOUNT_00, }, - 0 + 1 ), BondingPalletErrors::::NoPermission ); @@ -153,7 +197,7 @@ fn only_manager_can_change_team() { } #[test] -fn handles_currency_idx_out_of_bounds() { +fn handles_currency_number_incorrect() { let pool_details = generate_pool_details( vec![DEFAULT_BONDED_CURRENCY_ID], get_linear_bonding_curve(), @@ -180,9 +224,9 @@ fn handles_currency_idx_out_of_bounds() { admin: ACCOUNT_00, freezer: ACCOUNT_00, }, - 2 + 0 ), - BondingPalletErrors::::IndexOutOfBounds + BondingPalletErrors::::CurrencyCount ); }) } From a96da0ee541b5a65576ca341e323d44a92626c6e Mon Sep 17 00:00:00 2001 From: Raphael Date: Wed, 5 Mar 2025 16:12:26 +0100 Subject: [PATCH 2/3] docs: update extrinsic description --- pallets/pallet-bonded-coins/src/lib.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pallets/pallet-bonded-coins/src/lib.rs b/pallets/pallet-bonded-coins/src/lib.rs index fc01ae71e..c560a792f 100644 --- a/pallets/pallet-bonded-coins/src/lib.rs +++ b/pallets/pallet-bonded-coins/src/lib.rs @@ -449,18 +449,17 @@ pub mod pallet { Ok(()) } - /// Changes the managing team of a bonded currency which is issued by - /// this pool. The new team will be set to the provided team. The - /// currency index is used to select the currency that the team will - /// manage. The origin account must be a manager of the pool. + /// Changes the managing team of all bonded currencies issued by this + /// pool, setting it to the provided `team`. The origin account must be + /// a manager of the pool. /// /// # Parameters /// - `origin`: The origin of the call, requiring the caller to be a /// manager of the pool. /// - `pool_id`: The identifier of the pool. /// - `team`: The new managing team. - /// - `currency_idx`: The index of the currency in the bonded currencies - /// vector. + /// - `currency_count`: The number of bonded currencies vector linked to + /// the pool. Required for weight estimations. /// /// # Returns /// - `DispatchResult`: The result of the dispatch. @@ -469,8 +468,8 @@ pub mod pallet { /// - `Error::::PoolUnknown`: If the pool does not exist. /// - `Error::::NoPermission`: If the caller is not a manager of the /// pool. - /// - `Error::::IndexOutOfBounds`: If the currency index is out of - /// bounds. + /// - `Error::::CurrencyCount`: If the actual number of currencies in + /// the pool is larger than `currency_count`. /// - Other errors depending on the types in the config. #[pallet::call_index(1)] #[pallet::weight(T::WeightInfo::reset_team())] From 701410cb54ee4d17d1921caf914cb8ac91787960 Mon Sep 17 00:00:00 2001 From: Raphael Date: Thu, 6 Mar 2025 11:05:39 +0100 Subject: [PATCH 3/3] test: update benchmarks --- .../pallet-bonded-coins/src/benchmarking.rs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/pallets/pallet-bonded-coins/src/benchmarking.rs b/pallets/pallet-bonded-coins/src/benchmarking.rs index 9bb68b93b..f1f962600 100644 --- a/pallets/pallet-bonded-coins/src/benchmarking.rs +++ b/pallets/pallet-bonded-coins/src/benchmarking.rs @@ -355,7 +355,7 @@ mod benchmarks { } #[benchmark] - fn reset_team() { + fn reset_team(c: Linear<1, { T::MaxCurrenciesPerPool::get() }>) { let origin = T::DefaultOrigin::try_successful_origin().expect("creating origin should not fail"); let account_origin = origin .clone() @@ -363,17 +363,10 @@ mod benchmarks { .expect("generating account_id from origin should not fail"); make_free_for_deposit::(&account_origin); - let bonded_coin_id = T::BenchmarkHelper::calculate_bonded_asset_id(0); - create_bonded_asset::(bonded_coin_id.clone()); + let bonded_currencies = create_bonded_currencies_in_range::(c, false); let curve = get_linear_bonding_curve::>(); - let pool_id = create_pool::( - curve, - [bonded_coin_id.clone()].to_vec(), - Some(account_origin), - None, - None, - ); + let pool_id = create_pool::(curve, bonded_currencies.clone(), Some(account_origin), None, None); let admin: AccountIdOf = account("admin", 0, 0); let freezer: AccountIdOf = account("freezer", 0, 0); @@ -382,12 +375,15 @@ mod benchmarks { freezer: freezer.clone(), }; + let max_currencies = T::MaxCurrenciesPerPool::get(); #[extrinsic_call] - _(origin as T::RuntimeOrigin, pool_id, fungibles_team, 0); + _(origin as T::RuntimeOrigin, pool_id, fungibles_team, max_currencies); // Verify - assert_eq!(T::Fungibles::admin(bonded_coin_id.clone()), Some(admin)); - assert_eq!(T::Fungibles::freezer(bonded_coin_id), Some(freezer)); + bonded_currencies.iter().for_each(|asset_id| { + assert_eq!(T::Fungibles::admin(asset_id.clone()), Some(admin.clone())); + assert_eq!(T::Fungibles::freezer(asset_id.clone()), Some(freezer.clone())); + }); } #[benchmark]