From df21554b7a28eadb9a68739e622789273062bf1a Mon Sep 17 00:00:00 2001 From: Keith Date: Tue, 6 May 2025 15:10:49 +0800 Subject: [PATCH 01/28] Ensure we only reset BondsMovingAverage to 975000 only if it exceeds --- .../src/migrations/migrate_reset_bonds_moving_average.rs | 2 +- pallets/subtensor/src/migrations/migrate_reset_max_burn.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs b/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs index 5bb442af18..57018a03c5 100644 --- a/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs +++ b/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs @@ -31,7 +31,7 @@ pub fn migrate_reset_bonds_moving_average() -> Weight { for netuid in BondsMovingAverage::::iter_keys() { BondsMovingAverage::::mutate(netuid, |average| { - *average = (*average).min(975000); + *average = average.min(975000); }); reset_entries_count = reset_entries_count.saturating_add(1); } diff --git a/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs b/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs index a2662f7de3..d5b85ad189 100644 --- a/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs +++ b/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs @@ -39,7 +39,10 @@ pub fn migrate_reset_max_burn() -> Weight { weight = weight .saturating_add(T::DbWeight::get().reads_writes(reset_entries_count, reset_entries_count)); - log::info!("Reset {} subnets from MaxBurn.", reset_entries_count); + log::info!( + "Reset {} subnets from MaxBurn.", + reset_entries_count + ); // ------------------------------ // Step 2: Mark Migration as Completed From 8bec1629660c270c34dfcc95c23a2455bf946047 Mon Sep 17 00:00:00 2001 From: Keith Date: Tue, 6 May 2025 17:14:51 +0800 Subject: [PATCH 02/28] Fixes --- .../src/migrations/migrate_reset_bonds_moving_average.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs b/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs index 57018a03c5..5bb442af18 100644 --- a/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs +++ b/pallets/subtensor/src/migrations/migrate_reset_bonds_moving_average.rs @@ -31,7 +31,7 @@ pub fn migrate_reset_bonds_moving_average() -> Weight { for netuid in BondsMovingAverage::::iter_keys() { BondsMovingAverage::::mutate(netuid, |average| { - *average = average.min(975000); + *average = (*average).min(975000); }); reset_entries_count = reset_entries_count.saturating_add(1); } From 4610c8bfd849973235bc6ec69e077c1bf63cbd2f Mon Sep 17 00:00:00 2001 From: Keith Date: Tue, 6 May 2025 17:38:36 +0800 Subject: [PATCH 03/28] cargo fmt --- pallets/subtensor/src/migrations/migrate_reset_max_burn.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs b/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs index d5b85ad189..a2662f7de3 100644 --- a/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs +++ b/pallets/subtensor/src/migrations/migrate_reset_max_burn.rs @@ -39,10 +39,7 @@ pub fn migrate_reset_max_burn() -> Weight { weight = weight .saturating_add(T::DbWeight::get().reads_writes(reset_entries_count, reset_entries_count)); - log::info!( - "Reset {} subnets from MaxBurn.", - reset_entries_count - ); + log::info!("Reset {} subnets from MaxBurn.", reset_entries_count); // ------------------------------ // Step 2: Mark Migration as Completed From 8493bbfa151c370304d441a5266dd86be14cfd45 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 2 May 2025 16:38:56 +0400 Subject: [PATCH 04/28] Introduce SameSubnetId error for move and swap stake extrinsics. --- pallets/subtensor/src/macros/errors.rs | 4 ++-- pallets/subtensor/src/staking/stake_utils.rs | 5 ++++- pallets/subtensor/src/tests/move_stake.rs | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 69a8fe1646..a5bf028831 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -210,9 +210,9 @@ mod errors { InvalidRecoveredPublicKey, /// SubToken disabled now SubtokenDisabled, - /// Estimating the maximum stake for limited staking operations returned zero. + /// Zero max stake amount ZeroMaxStakeAmount, /// Invalid netuid duplication - SameNetuid, + SameSubnetId, } } diff --git a/pallets/subtensor/src/staking/stake_utils.rs b/pallets/subtensor/src/staking/stake_utils.rs index 65280c8a43..a793a56edc 100644 --- a/pallets/subtensor/src/staking/stake_utils.rs +++ b/pallets/subtensor/src/staking/stake_utils.rs @@ -1012,7 +1012,10 @@ impl Pallet { ) -> Result<(), Error> { // Ensure stake transition is actually happening if origin_coldkey == destination_coldkey && origin_hotkey == destination_hotkey { - ensure!(origin_netuid != destination_netuid, Error::::SameNetuid); + ensure!( + origin_netuid != destination_netuid, + Error::::SameSubnetId + ); } // Ensure that both subnets exist. diff --git a/pallets/subtensor/src/tests/move_stake.rs b/pallets/subtensor/src/tests/move_stake.rs index dd85ab9075..0590fb9a1c 100644 --- a/pallets/subtensor/src/tests/move_stake.rs +++ b/pallets/subtensor/src/tests/move_stake.rs @@ -596,7 +596,7 @@ fn test_do_move_same_hotkey_fails() { netuid, alpha, ), - Err(Error::::SameNetuid.into()) + Err(Error::::SameSubnetId.into()) ); // Check that stake remains unchanged @@ -1309,7 +1309,7 @@ fn test_do_swap_same_subnet() { netuid, alpha_before ), - Err(Error::::SameNetuid.into()) + Err(Error::::SameSubnetId.into()) ); let alpha_after = From f974a94f379766bc14f83671456c59a315620787 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Tue, 6 May 2025 19:25:22 +0400 Subject: [PATCH 05/28] Rename error SameSubnetId -> SameNetuid --- pallets/subtensor/src/macros/errors.rs | 2 +- pallets/subtensor/src/staking/stake_utils.rs | 5 +---- pallets/subtensor/src/tests/move_stake.rs | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index a5bf028831..2a8e5bc346 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -213,6 +213,6 @@ mod errors { /// Zero max stake amount ZeroMaxStakeAmount, /// Invalid netuid duplication - SameSubnetId, + SameNetuid, } } diff --git a/pallets/subtensor/src/staking/stake_utils.rs b/pallets/subtensor/src/staking/stake_utils.rs index a793a56edc..65280c8a43 100644 --- a/pallets/subtensor/src/staking/stake_utils.rs +++ b/pallets/subtensor/src/staking/stake_utils.rs @@ -1012,10 +1012,7 @@ impl Pallet { ) -> Result<(), Error> { // Ensure stake transition is actually happening if origin_coldkey == destination_coldkey && origin_hotkey == destination_hotkey { - ensure!( - origin_netuid != destination_netuid, - Error::::SameSubnetId - ); + ensure!(origin_netuid != destination_netuid, Error::::SameNetuid); } // Ensure that both subnets exist. diff --git a/pallets/subtensor/src/tests/move_stake.rs b/pallets/subtensor/src/tests/move_stake.rs index 0590fb9a1c..dd85ab9075 100644 --- a/pallets/subtensor/src/tests/move_stake.rs +++ b/pallets/subtensor/src/tests/move_stake.rs @@ -596,7 +596,7 @@ fn test_do_move_same_hotkey_fails() { netuid, alpha, ), - Err(Error::::SameSubnetId.into()) + Err(Error::::SameNetuid.into()) ); // Check that stake remains unchanged @@ -1309,7 +1309,7 @@ fn test_do_swap_same_subnet() { netuid, alpha_before ), - Err(Error::::SameSubnetId.into()) + Err(Error::::SameNetuid.into()) ); let alpha_after = From 5862a43b2f718f9012943b6349511811d6255083 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 5 May 2025 17:57:22 +0200 Subject: [PATCH 06/28] keep track of contributors count and allow a maximum of contributors, added migration --- pallets/crowdloan/src/lib.rs | 34 +++- .../migrate_add_contributors_count.rs | 160 +----------------- 2 files changed, 34 insertions(+), 160 deletions(-) diff --git a/pallets/crowdloan/src/lib.rs b/pallets/crowdloan/src/lib.rs index 1d4ed4e263..2615d78bf3 100644 --- a/pallets/crowdloan/src/lib.rs +++ b/pallets/crowdloan/src/lib.rs @@ -168,6 +168,11 @@ pub mod pallet { OptionQuery, >; + /// A map of crowdloan ids to their contributors count. + #[pallet::storage] + pub type ContributorsCount = + StorageMap<_, Twox64Concat, CrowdloanId, u32, OptionQuery>; + /// The current crowdloan id that will be set during the finalize call, making it /// temporarily accessible to the dispatched call. #[pallet::storage] @@ -386,6 +391,7 @@ pub mod pallet { )?; Contributions::::insert(crowdloan_id, &creator, deposit); + ContributorsCount::::insert(crowdloan_id, 1); Self::deposit_event(Event::::Created { crowdloan_id, @@ -431,8 +437,10 @@ pub mod pallet { ); // Ensure the crowdloan has not reached the maximum number of contributors + let contributors_count = + ContributorsCount::::get(crowdloan_id).ok_or(Error::::InvalidCrowdloanId)?; ensure!( - crowdloan.contributors_count < T::MaxContributors::get(), + contributors_count < T::MaxContributors::get(), Error::::MaxContributorsReached ); @@ -462,10 +470,7 @@ pub mod pallet { .ok_or(Error::::Overflow)? } else { // We have a new contribution - crowdloan.contributors_count = crowdloan - .contributors_count - .checked_add(1) - .ok_or(Error::::Overflow)?; + Self::increment_contributor_count(crowdloan_id); amount }; @@ -524,10 +529,7 @@ pub mod pallet { Contributions::::insert(crowdloan_id, &who, crowdloan.deposit); } else { Contributions::::remove(crowdloan_id, &who); - crowdloan.contributors_count = crowdloan - .contributors_count - .checked_sub(1) - .ok_or(Error::::Underflow)?; + Self::decrement_contributor_count(crowdloan_id); } CurrencyOf::::transfer( @@ -686,6 +688,7 @@ pub mod pallet { // Clear refunded contributors for contributor in refunded_contributors { Contributions::::remove(crowdloan_id, &contributor); + Self::decrement_contributor_count(crowdloan_id); } if all_refunded { @@ -748,6 +751,7 @@ pub mod pallet { // Remove the crowdloan let _ = frame_system::Pallet::::dec_providers(&crowdloan.funds_account).defensive(); Crowdloans::::remove(crowdloan_id); + ContributorsCount::::remove(crowdloan_id); Self::deposit_event(Event::::Dissolved { crowdloan_id }); Ok(()) @@ -888,4 +892,16 @@ impl Pallet { ); Ok(()) } + + fn increment_contributor_count(crowdloan_id: CrowdloanId) { + ContributorsCount::::mutate(crowdloan_id, |count| { + *count = count.map(|v| v.saturating_add(1)) + }); + } + + fn decrement_contributor_count(crowdloan_id: CrowdloanId) { + ContributorsCount::::mutate(crowdloan_id, |count| { + *count = count.map(|v| v.saturating_sub(1)) + }); + } } diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index 3b094843ce..684c4b1cff 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -1,30 +1,11 @@ use alloc::string::String; -use frame_support::{BoundedVec, migration::storage_key_iter, traits::Get, weights::Weight}; -use subtensor_macros::freeze_struct; +use frame_support::{BoundedVec, traits::Get, weights::Weight}; use crate::*; -mod old_storage { - use super::*; - - #[freeze_struct("84bcbf9b8d3f0ddf")] - #[derive(Encode, Decode, Debug)] - pub struct OldCrowdloanInfo { - pub creator: AccountId, - pub deposit: Balance, - pub min_contribution: Balance, - pub end: BlockNumber, - pub cap: Balance, - pub funds_account: AccountId, - pub raised: Balance, - pub target_address: Option, - pub call: Option, - pub finalized: bool, - } -} - pub fn migrate_add_contributors_count() -> Weight { - let migration_name = BoundedVec::truncate_from(b"migrate_add_contributors_count".to_vec()); + let migration_name = + BoundedVec::truncate_from(b"migrate_crowdloan_contributors_count".to_vec()); let mut weight = T::DbWeight::get().reads(1); if HasMigrationRun::::get(&migration_name) { @@ -40,44 +21,17 @@ pub fn migrate_add_contributors_count() -> Weight { String::from_utf8_lossy(&migration_name) ); - let pallet_name = b"Crowdloan"; - let item_name = b"Crowdloans"; - let crowdloans = storage_key_iter::< - CrowdloanId, - old_storage::OldCrowdloanInfo< - T::AccountId, - BalanceOf, - BlockNumberFor, - BoundedCallOf, - >, - Twox64Concat, - >(pallet_name, item_name) - .collect::>(); - weight = weight.saturating_add(T::DbWeight::get().reads(crowdloans.len() as u64)); + // Get all crowdloans, there is not so many at the moment so we are safe. + let crowdloan_ids = Crowdloans::::iter_keys().collect::>(); + weight = weight.saturating_add(T::DbWeight::get().reads(crowdloan_ids.len() as u64)); - for (id, crowdloan) in crowdloans { - let contributions = Contributions::::iter_key_prefix(id) + for crowdloan_id in crowdloan_ids { + let contributions = Contributions::::iter_key_prefix(crowdloan_id) .collect::>() .len(); weight = weight.saturating_add(T::DbWeight::get().reads(contributions as u64)); - Crowdloans::::insert( - id, - CrowdloanInfo { - creator: crowdloan.creator, - deposit: crowdloan.deposit, - min_contribution: crowdloan.min_contribution, - end: crowdloan.end, - cap: crowdloan.cap, - funds_account: crowdloan.funds_account, - raised: crowdloan.raised, - target_address: crowdloan.target_address, - call: crowdloan.call, - finalized: crowdloan.finalized, - contributors_count: contributions as u32, - }, - ); - weight = weight.saturating_add(T::DbWeight::get().writes(1)); + ContributorsCount::::insert(crowdloan_id, contributions as u32); } HasMigrationRun::::insert(&migration_name, true); @@ -90,99 +44,3 @@ pub fn migrate_add_contributors_count() -> Weight { weight } - -#[cfg(test)] -mod tests { - use frame_support::{Hashable, storage::unhashed::put_raw}; - use sp_core::U256; - use sp_io::hashing::twox_128; - - use super::*; - use crate::mock::{Test, TestState}; - - #[test] - fn test_migrate_add_contributors_count_works() { - TestState::default().build_and_execute(|| { - let pallet_name = twox_128(b"Crowdloan"); - let storage_name = twox_128(b"Crowdloans"); - let prefix = [pallet_name, storage_name].concat(); - - let items = vec![ - ( - old_storage::OldCrowdloanInfo { - creator: U256::from(1), - deposit: 100u64, - min_contribution: 10u64, - end: 100u64, - cap: 1000u64, - funds_account: U256::from(2), - raised: 0u64, - target_address: None, - call: None::>, - finalized: false, - }, - vec![(U256::from(1), 100)], - ), - ( - old_storage::OldCrowdloanInfo { - creator: U256::from(1), - deposit: 100u64, - min_contribution: 10u64, - end: 100u64, - cap: 1000u64, - funds_account: U256::from(2), - raised: 0u64, - target_address: None, - call: None::>, - finalized: false, - }, - vec![ - (U256::from(1), 100), - (U256::from(2), 100), - (U256::from(3), 100), - ], - ), - ( - old_storage::OldCrowdloanInfo { - creator: U256::from(1), - deposit: 100u64, - min_contribution: 10u64, - end: 100u64, - cap: 1000u64, - funds_account: U256::from(2), - raised: 0u64, - target_address: None, - call: None::>, - finalized: false, - }, - vec![ - (U256::from(1), 100), - (U256::from(2), 100), - (U256::from(3), 100), - (U256::from(4), 100), - (U256::from(5), 100), - ], - ), - ]; - - for (id, (crowdloan, contributions)) in items.into_iter().enumerate() { - let key = [prefix.clone(), (id as u32).twox_64_concat()].concat(); - put_raw(&key, &crowdloan.encode()); - - for (contributor, amount) in contributions { - Contributions::::insert(id as u32, contributor, amount); - } - } - - migrate_add_contributors_count::(); - - assert!(Crowdloans::::get(0).is_some_and(|c| c.contributors_count == 1)); - assert!(Crowdloans::::get(1).is_some_and(|c| c.contributors_count == 3)); - assert!(Crowdloans::::get(2).is_some_and(|c| c.contributors_count == 5)); - - assert!(HasMigrationRun::::get(BoundedVec::truncate_from( - b"migrate_add_contributors_count".to_vec() - ))); - }); - } -} From 774a5fcdd0a0d82df0b2e7f11c214e597d42d635 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 5 May 2025 17:57:32 +0200 Subject: [PATCH 07/28] fix tests --- pallets/crowdloan/src/tests.rs | 84 +++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/pallets/crowdloan/src/tests.rs b/pallets/crowdloan/src/tests.rs index 1e03854b1f..2fdc84c0a7 100644 --- a/pallets/crowdloan/src/tests.rs +++ b/pallets/crowdloan/src/tests.rs @@ -59,6 +59,11 @@ fn test_create_succeeds() { .collect::>(), vec![(creator, deposit)] ); + // ensure the contributor count is updated + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) + ); // ensure the raised amount is updated correctly assert!( pallet_crowdloan::Crowdloans::::get(crowdloan_id) @@ -334,9 +339,9 @@ fn test_contribute_succeeds() { let crowdloan_id: CrowdloanId = 0; // only the creator has contributed so far - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 1) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) ); // first contribution to the crowdloan from creator @@ -363,6 +368,10 @@ fn test_contribute_succeeds() { pallet_crowdloan::Crowdloans::::get(crowdloan_id) .is_some_and(|c| c.contributors_count == 1) ); + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) + ); assert_eq!( Balances::free_balance(creator), 200 - amount - initial_deposit @@ -389,9 +398,9 @@ fn test_contribute_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor1), Some(100) ); - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 2) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(2) ); assert_eq!(Balances::free_balance(contributor1), 500 - amount); @@ -416,9 +425,9 @@ fn test_contribute_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor2), Some(50) ); - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 3) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(3) ); assert_eq!(Balances::free_balance(contributor2), 200 - amount); @@ -820,9 +829,9 @@ fn test_withdraw_from_contributor_succeeds() { run_to_block(60); // ensure the contributor count is correct - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 3) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(3) ); // withdraw from contributor1 @@ -835,9 +844,9 @@ fn test_withdraw_from_contributor_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor1), None, ); - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 2) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(2) ); // ensure the contributor1 has the correct amount assert_eq!( @@ -855,9 +864,9 @@ fn test_withdraw_from_contributor_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor2), None, ); - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 1) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) ); // ensure the contributor2 has the correct amount assert_eq!( @@ -909,9 +918,9 @@ fn test_withdraw_from_creator_with_contribution_over_deposit_succeeds() { )); // ensure the contributor count is correct - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 1) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) ); // withdraw @@ -932,9 +941,9 @@ fn test_withdraw_from_creator_with_contribution_over_deposit_succeeds() { Some(initial_deposit), ); // ensure the contributor count hasn't changed because deposit is kept - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 1) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) ); // ensure the crowdloan account has the correct amount @@ -1580,9 +1589,9 @@ fn test_refund_succeeds() { } // ensure the contributor count is correct - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 7) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(7) ); // run some more blocks past the end of the contribution period @@ -1595,9 +1604,9 @@ fn test_refund_succeeds() { )); // ensure the contributor count is correct, we processed 5 refunds - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 2) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(2) ); // ensure the crowdloan account has the correct amount @@ -1625,9 +1634,9 @@ fn test_refund_succeeds() { // ensure the contributor count is correct, we processed 1 more refund // keeping deposit - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 1) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) ); // ensure the crowdloan account has the correct amount @@ -1761,9 +1770,9 @@ fn test_dissolve_succeeds() { let crowdloan_id: CrowdloanId = 0; // ensure the contributor count is correct - assert!( - pallet_crowdloan::Crowdloans::::get(crowdloan_id) - .is_some_and(|c| c.contributors_count == 1) + assert_eq!( + pallet_crowdloan::ContributorsCount::::get(crowdloan_id), + Some(1) ); // dissolve the crowdloan @@ -1780,6 +1789,9 @@ fn test_dissolve_succeeds() { crowdloan_id )); + // ensure the contributor count is removed + assert!(pallet_crowdloan::ContributorsCount::::get(crowdloan_id).is_none()); + // ensure the event is emitted assert_eq!( last_event(), From d3477071aec6ca1c39c8a3a4910239646cebf45d Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 5 May 2025 18:01:36 +0200 Subject: [PATCH 08/28] fix migration name --- .../crowdloan/src/migrations/migrate_add_contributors_count.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index 684c4b1cff..fc5de151cb 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -4,8 +4,7 @@ use frame_support::{BoundedVec, traits::Get, weights::Weight}; use crate::*; pub fn migrate_add_contributors_count() -> Weight { - let migration_name = - BoundedVec::truncate_from(b"migrate_crowdloan_contributors_count".to_vec()); + let migration_name = BoundedVec::truncate_from(b"migrate_add_contributors_count".to_vec()); let mut weight = T::DbWeight::get().reads(1); if HasMigrationRun::::get(&migration_name) { From 017f3ff09e35ff085eef0c3bd99c69cb191106ea Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 5 May 2025 18:18:41 +0200 Subject: [PATCH 09/28] added migration test --- .../migrate_add_contributors_count.rs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index fc5de151cb..cb399634eb 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -43,3 +43,54 @@ pub fn migrate_add_contributors_count() -> Weight { weight } + +#[cfg(test)] +mod tests { + use crate::mock::{Test, TestState}; + + use super::*; + use sp_core::U256; + + #[test] + fn test_migrate_add_contributors_count_works() { + TestState::default().build_and_execute(|| { + Crowdloans::::insert( + 0, + CrowdloanInfo { + creator: U256::from(1), + deposit: 100, + min_contribution: 10, + cap: 1000, + end: 100, + call: None, + finalized: false, + raised: 0, + funds_account: U256::from(2), + target_address: None, + }, + ); + + Contributions::::insert(0, U256::from(1), 100); + Contributions::::insert(0, U256::from(2), 100); + Contributions::::insert(0, U256::from(3), 100); + + assert_eq!(ContributorsCount::::get(0), None); + assert_eq!( + HasMigrationRun::::get(BoundedVec::truncate_from( + b"migrate_add_contributors_count".to_vec() + )), + false + ); + + migrate_add_contributors_count::(); + + assert_eq!(ContributorsCount::::get(0), Some(3)); + assert_eq!( + HasMigrationRun::::get(BoundedVec::truncate_from( + b"migrate_add_contributors_count".to_vec() + )), + true + ); + }); + } +} From 291878918eb5607f81c959830679786436eba296 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 5 May 2025 18:21:39 +0200 Subject: [PATCH 10/28] cargo clippy --- .../src/migrations/migrate_add_contributors_count.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index cb399634eb..9d610fd94e 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -75,21 +75,19 @@ mod tests { Contributions::::insert(0, U256::from(3), 100); assert_eq!(ContributorsCount::::get(0), None); - assert_eq!( - HasMigrationRun::::get(BoundedVec::truncate_from( + assert!( + !HasMigrationRun::::get(BoundedVec::truncate_from( b"migrate_add_contributors_count".to_vec() - )), - false + )) ); migrate_add_contributors_count::(); assert_eq!(ContributorsCount::::get(0), Some(3)); - assert_eq!( + assert!( HasMigrationRun::::get(BoundedVec::truncate_from( b"migrate_add_contributors_count".to_vec() - )), - true + )) ); }); } From ae65d498948c7c3a1b7f3f7cd1ba00a744234e06 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 5 May 2025 18:22:17 +0200 Subject: [PATCH 11/28] cargo fmt --- .../migrations/migrate_add_contributors_count.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index 9d610fd94e..378e8bcc3d 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -75,20 +75,16 @@ mod tests { Contributions::::insert(0, U256::from(3), 100); assert_eq!(ContributorsCount::::get(0), None); - assert!( - !HasMigrationRun::::get(BoundedVec::truncate_from( - b"migrate_add_contributors_count".to_vec() - )) - ); + assert!(!HasMigrationRun::::get(BoundedVec::truncate_from( + b"migrate_add_contributors_count".to_vec() + ))); migrate_add_contributors_count::(); assert_eq!(ContributorsCount::::get(0), Some(3)); - assert!( - HasMigrationRun::::get(BoundedVec::truncate_from( - b"migrate_add_contributors_count".to_vec() - )) - ); + assert!(HasMigrationRun::::get(BoundedVec::truncate_from( + b"migrate_add_contributors_count".to_vec() + ))); }); } } From d3c05ee6589d0ed1503c636d50e10a46839bda92 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Tue, 6 May 2025 18:07:12 +0200 Subject: [PATCH 12/28] move contributors count to Crowdloan struct and update migration --- pallets/crowdloan/Cargo.toml | 4 +- pallets/crowdloan/src/lib.rs | 33 +--- .../migrate_add_contributors_count.rs | 166 ++++++++++++++---- pallets/crowdloan/src/tests.rs | 84 ++++----- 4 files changed, 173 insertions(+), 114 deletions(-) diff --git a/pallets/crowdloan/Cargo.toml b/pallets/crowdloan/Cargo.toml index e8d582fa44..b662d7269f 100644 --- a/pallets/crowdloan/Cargo.toml +++ b/pallets/crowdloan/Cargo.toml @@ -22,12 +22,12 @@ frame-system.workspace = true sp-runtime.workspace = true sp-std.workspace = true log = { workspace = true } +sp-core = { default-features = true, workspace = true } +sp-io = { default-features = true, workspace = true } [dev-dependencies] pallet-balances = { default-features = true, workspace = true } pallet-preimage = { default-features = true, workspace = true } -sp-core = { default-features = true, workspace = true } -sp-io = { default-features = true, workspace = true } [features] default = ["std"] diff --git a/pallets/crowdloan/src/lib.rs b/pallets/crowdloan/src/lib.rs index 2615d78bf3..b25807151a 100644 --- a/pallets/crowdloan/src/lib.rs +++ b/pallets/crowdloan/src/lib.rs @@ -168,11 +168,6 @@ pub mod pallet { OptionQuery, >; - /// A map of crowdloan ids to their contributors count. - #[pallet::storage] - pub type ContributorsCount = - StorageMap<_, Twox64Concat, CrowdloanId, u32, OptionQuery>; - /// The current crowdloan id that will be set during the finalize call, making it /// temporarily accessible to the dispatched call. #[pallet::storage] @@ -391,7 +386,6 @@ pub mod pallet { )?; Contributions::::insert(crowdloan_id, &creator, deposit); - ContributorsCount::::insert(crowdloan_id, 1); Self::deposit_event(Event::::Created { crowdloan_id, @@ -437,10 +431,8 @@ pub mod pallet { ); // Ensure the crowdloan has not reached the maximum number of contributors - let contributors_count = - ContributorsCount::::get(crowdloan_id).ok_or(Error::::InvalidCrowdloanId)?; ensure!( - contributors_count < T::MaxContributors::get(), + crowdloan.contributors_count < T::MaxContributors::get(), Error::::MaxContributorsReached ); @@ -470,7 +462,7 @@ pub mod pallet { .ok_or(Error::::Overflow)? } else { // We have a new contribution - Self::increment_contributor_count(crowdloan_id); + crowdloan.contributors_count += 1; amount }; @@ -529,7 +521,7 @@ pub mod pallet { Contributions::::insert(crowdloan_id, &who, crowdloan.deposit); } else { Contributions::::remove(crowdloan_id, &who); - Self::decrement_contributor_count(crowdloan_id); + crowdloan.contributors_count -= 1; } CurrencyOf::::transfer( @@ -679,16 +671,12 @@ pub mod pallet { refund_count = refund_count.checked_add(1).ok_or(Error::::Overflow)?; } - crowdloan.contributors_count = crowdloan - .contributors_count - .checked_sub(refund_count) - .ok_or(Error::::Underflow)?; + crowdloan.contributors_count -= refund_count; Crowdloans::::insert(crowdloan_id, &crowdloan); // Clear refunded contributors for contributor in refunded_contributors { Contributions::::remove(crowdloan_id, &contributor); - Self::decrement_contributor_count(crowdloan_id); } if all_refunded { @@ -751,7 +739,6 @@ pub mod pallet { // Remove the crowdloan let _ = frame_system::Pallet::::dec_providers(&crowdloan.funds_account).defensive(); Crowdloans::::remove(crowdloan_id); - ContributorsCount::::remove(crowdloan_id); Self::deposit_event(Event::::Dissolved { crowdloan_id }); Ok(()) @@ -892,16 +879,4 @@ impl Pallet { ); Ok(()) } - - fn increment_contributor_count(crowdloan_id: CrowdloanId) { - ContributorsCount::::mutate(crowdloan_id, |count| { - *count = count.map(|v| v.saturating_add(1)) - }); - } - - fn decrement_contributor_count(crowdloan_id: CrowdloanId) { - ContributorsCount::::mutate(crowdloan_id, |count| { - *count = count.map(|v| v.saturating_sub(1)) - }); - } } diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index 378e8bcc3d..d5fbc4ec97 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -1,8 +1,26 @@ use alloc::string::String; -use frame_support::{BoundedVec, traits::Get, weights::Weight}; +use frame_support::{BoundedVec, migration::storage_key_iter, traits::Get, weights::Weight}; use crate::*; +mod old_storage { + use super::*; + + #[derive(Encode, Decode, Debug)] + pub struct OldCrowdloanInfo { + pub creator: AccountId, + pub deposit: Balance, + pub min_contribution: Balance, + pub end: BlockNumber, + pub cap: Balance, + pub funds_account: AccountId, + pub raised: Balance, + pub target_address: Option, + pub call: Option, + pub finalized: bool, + } +} + pub fn migrate_add_contributors_count() -> Weight { let migration_name = BoundedVec::truncate_from(b"migrate_add_contributors_count".to_vec()); let mut weight = T::DbWeight::get().reads(1); @@ -20,17 +38,44 @@ pub fn migrate_add_contributors_count() -> Weight { String::from_utf8_lossy(&migration_name) ); - // Get all crowdloans, there is not so many at the moment so we are safe. - let crowdloan_ids = Crowdloans::::iter_keys().collect::>(); - weight = weight.saturating_add(T::DbWeight::get().reads(crowdloan_ids.len() as u64)); - - for crowdloan_id in crowdloan_ids { - let contributions = Contributions::::iter_key_prefix(crowdloan_id) + let pallet_name = b"Crowdloan"; + let item_name = b"Crowdloans"; + let crowdloans = storage_key_iter::< + CrowdloanId, + old_storage::OldCrowdloanInfo< + T::AccountId, + BalanceOf, + BlockNumberFor, + BoundedCallOf, + >, + Twox64Concat, + >(pallet_name, item_name) + .collect::>(); + weight = weight.saturating_add(T::DbWeight::get().reads(crowdloans.len() as u64)); + + for (id, crowdloan) in crowdloans { + let contributions = Contributions::::iter_key_prefix(id) .collect::>() .len(); weight = weight.saturating_add(T::DbWeight::get().reads(contributions as u64)); - ContributorsCount::::insert(crowdloan_id, contributions as u32); + Crowdloans::::insert( + id, + CrowdloanInfo { + creator: crowdloan.creator, + deposit: crowdloan.deposit, + min_contribution: crowdloan.min_contribution, + end: crowdloan.end, + cap: crowdloan.cap, + funds_account: crowdloan.funds_account, + raised: crowdloan.raised, + target_address: crowdloan.target_address, + call: crowdloan.call, + finalized: crowdloan.finalized, + contributors_count: contributions as u32, + }, + ); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); } HasMigrationRun::::insert(&migration_name, true); @@ -46,42 +91,93 @@ pub fn migrate_add_contributors_count() -> Weight { #[cfg(test)] mod tests { - use crate::mock::{Test, TestState}; + use frame_support::{Hashable, storage::unhashed::put_raw}; + use sp_core::U256; + use sp_io::hashing::twox_128; use super::*; - use sp_core::U256; + use crate::mock::{Test, TestState}; #[test] fn test_migrate_add_contributors_count_works() { TestState::default().build_and_execute(|| { - Crowdloans::::insert( - 0, - CrowdloanInfo { - creator: U256::from(1), - deposit: 100, - min_contribution: 10, - cap: 1000, - end: 100, - call: None, - finalized: false, - raised: 0, - funds_account: U256::from(2), - target_address: None, - }, - ); - - Contributions::::insert(0, U256::from(1), 100); - Contributions::::insert(0, U256::from(2), 100); - Contributions::::insert(0, U256::from(3), 100); - - assert_eq!(ContributorsCount::::get(0), None); - assert!(!HasMigrationRun::::get(BoundedVec::truncate_from( - b"migrate_add_contributors_count".to_vec() - ))); + let pallet_name = twox_128(b"Crowdloan"); + let storage_name = twox_128(b"Crowdloans"); + let prefix = [pallet_name, storage_name].concat(); + + let items = vec![ + ( + old_storage::OldCrowdloanInfo { + creator: U256::from(1), + deposit: 100u64, + min_contribution: 10u64, + end: 100u64, + cap: 1000u64, + funds_account: U256::from(2), + raised: 0u64, + target_address: None, + call: None::>, + finalized: false, + }, + vec![(U256::from(1), 100)], + ), + ( + old_storage::OldCrowdloanInfo { + creator: U256::from(1), + deposit: 100u64, + min_contribution: 10u64, + end: 100u64, + cap: 1000u64, + funds_account: U256::from(2), + raised: 0u64, + target_address: None, + call: None::>, + finalized: false, + }, + vec![ + (U256::from(1), 100), + (U256::from(2), 100), + (U256::from(3), 100), + ], + ), + ( + old_storage::OldCrowdloanInfo { + creator: U256::from(1), + deposit: 100u64, + min_contribution: 10u64, + end: 100u64, + cap: 1000u64, + funds_account: U256::from(2), + raised: 0u64, + target_address: None, + call: None::>, + finalized: false, + }, + vec![ + (U256::from(1), 100), + (U256::from(2), 100), + (U256::from(3), 100), + (U256::from(4), 100), + (U256::from(5), 100), + ], + ), + ]; + + for (id, (crowdloan, contributions)) in items.into_iter().enumerate() { + let key = [prefix.clone(), (id as u32).twox_64_concat()].concat(); + put_raw(&key, &crowdloan.encode()); + + for (contributor, amount) in contributions { + Contributions::::insert(id as u32, contributor, amount); + } + } migrate_add_contributors_count::(); - assert_eq!(ContributorsCount::::get(0), Some(3)); + assert!(Crowdloans::::get(0).is_some_and(|c| c.contributors_count == 1)); + assert!(Crowdloans::::get(1).is_some_and(|c| c.contributors_count == 3)); + assert!(Crowdloans::::get(2).is_some_and(|c| c.contributors_count == 5)); + assert!(HasMigrationRun::::get(BoundedVec::truncate_from( b"migrate_add_contributors_count".to_vec() ))); diff --git a/pallets/crowdloan/src/tests.rs b/pallets/crowdloan/src/tests.rs index 2fdc84c0a7..1e03854b1f 100644 --- a/pallets/crowdloan/src/tests.rs +++ b/pallets/crowdloan/src/tests.rs @@ -59,11 +59,6 @@ fn test_create_succeeds() { .collect::>(), vec![(creator, deposit)] ); - // ensure the contributor count is updated - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) - ); // ensure the raised amount is updated correctly assert!( pallet_crowdloan::Crowdloans::::get(crowdloan_id) @@ -339,9 +334,9 @@ fn test_contribute_succeeds() { let crowdloan_id: CrowdloanId = 0; // only the creator has contributed so far - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 1) ); // first contribution to the crowdloan from creator @@ -368,10 +363,6 @@ fn test_contribute_succeeds() { pallet_crowdloan::Crowdloans::::get(crowdloan_id) .is_some_and(|c| c.contributors_count == 1) ); - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) - ); assert_eq!( Balances::free_balance(creator), 200 - amount - initial_deposit @@ -398,9 +389,9 @@ fn test_contribute_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor1), Some(100) ); - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(2) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 2) ); assert_eq!(Balances::free_balance(contributor1), 500 - amount); @@ -425,9 +416,9 @@ fn test_contribute_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor2), Some(50) ); - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(3) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 3) ); assert_eq!(Balances::free_balance(contributor2), 200 - amount); @@ -829,9 +820,9 @@ fn test_withdraw_from_contributor_succeeds() { run_to_block(60); // ensure the contributor count is correct - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(3) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 3) ); // withdraw from contributor1 @@ -844,9 +835,9 @@ fn test_withdraw_from_contributor_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor1), None, ); - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(2) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 2) ); // ensure the contributor1 has the correct amount assert_eq!( @@ -864,9 +855,9 @@ fn test_withdraw_from_contributor_succeeds() { pallet_crowdloan::Contributions::::get(crowdloan_id, contributor2), None, ); - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 1) ); // ensure the contributor2 has the correct amount assert_eq!( @@ -918,9 +909,9 @@ fn test_withdraw_from_creator_with_contribution_over_deposit_succeeds() { )); // ensure the contributor count is correct - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 1) ); // withdraw @@ -941,9 +932,9 @@ fn test_withdraw_from_creator_with_contribution_over_deposit_succeeds() { Some(initial_deposit), ); // ensure the contributor count hasn't changed because deposit is kept - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 1) ); // ensure the crowdloan account has the correct amount @@ -1589,9 +1580,9 @@ fn test_refund_succeeds() { } // ensure the contributor count is correct - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(7) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 7) ); // run some more blocks past the end of the contribution period @@ -1604,9 +1595,9 @@ fn test_refund_succeeds() { )); // ensure the contributor count is correct, we processed 5 refunds - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(2) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 2) ); // ensure the crowdloan account has the correct amount @@ -1634,9 +1625,9 @@ fn test_refund_succeeds() { // ensure the contributor count is correct, we processed 1 more refund // keeping deposit - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 1) ); // ensure the crowdloan account has the correct amount @@ -1770,9 +1761,9 @@ fn test_dissolve_succeeds() { let crowdloan_id: CrowdloanId = 0; // ensure the contributor count is correct - assert_eq!( - pallet_crowdloan::ContributorsCount::::get(crowdloan_id), - Some(1) + assert!( + pallet_crowdloan::Crowdloans::::get(crowdloan_id) + .is_some_and(|c| c.contributors_count == 1) ); // dissolve the crowdloan @@ -1789,9 +1780,6 @@ fn test_dissolve_succeeds() { crowdloan_id )); - // ensure the contributor count is removed - assert!(pallet_crowdloan::ContributorsCount::::get(crowdloan_id).is_none()); - // ensure the event is emitted assert_eq!( last_event(), From 677749addd8a733dea1b60092dd683dedec46638 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Tue, 6 May 2025 18:26:48 +0200 Subject: [PATCH 13/28] fix benchmark --- pallets/crowdloan/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/crowdloan/Cargo.toml b/pallets/crowdloan/Cargo.toml index b662d7269f..e8d582fa44 100644 --- a/pallets/crowdloan/Cargo.toml +++ b/pallets/crowdloan/Cargo.toml @@ -22,12 +22,12 @@ frame-system.workspace = true sp-runtime.workspace = true sp-std.workspace = true log = { workspace = true } -sp-core = { default-features = true, workspace = true } -sp-io = { default-features = true, workspace = true } [dev-dependencies] pallet-balances = { default-features = true, workspace = true } pallet-preimage = { default-features = true, workspace = true } +sp-core = { default-features = true, workspace = true } +sp-io = { default-features = true, workspace = true } [features] default = ["std"] From 8d238964b7ed41733b046c942dac0edde1fcb949 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Tue, 6 May 2025 18:28:28 +0200 Subject: [PATCH 14/28] fix arithmetic --- pallets/crowdloan/src/lib.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pallets/crowdloan/src/lib.rs b/pallets/crowdloan/src/lib.rs index b25807151a..1d4ed4e263 100644 --- a/pallets/crowdloan/src/lib.rs +++ b/pallets/crowdloan/src/lib.rs @@ -462,7 +462,10 @@ pub mod pallet { .ok_or(Error::::Overflow)? } else { // We have a new contribution - crowdloan.contributors_count += 1; + crowdloan.contributors_count = crowdloan + .contributors_count + .checked_add(1) + .ok_or(Error::::Overflow)?; amount }; @@ -521,7 +524,10 @@ pub mod pallet { Contributions::::insert(crowdloan_id, &who, crowdloan.deposit); } else { Contributions::::remove(crowdloan_id, &who); - crowdloan.contributors_count -= 1; + crowdloan.contributors_count = crowdloan + .contributors_count + .checked_sub(1) + .ok_or(Error::::Underflow)?; } CurrencyOf::::transfer( @@ -671,7 +677,10 @@ pub mod pallet { refund_count = refund_count.checked_add(1).ok_or(Error::::Overflow)?; } - crowdloan.contributors_count -= refund_count; + crowdloan.contributors_count = crowdloan + .contributors_count + .checked_sub(refund_count) + .ok_or(Error::::Underflow)?; Crowdloans::::insert(crowdloan_id, &crowdloan); // Clear refunded contributors From f7363820324ed73b37b65f5ae2324e0aef28498c Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 7 May 2025 09:22:46 +0200 Subject: [PATCH 15/28] added freeze_struct to OldCrowdloanInfo --- .../crowdloan/src/migrations/migrate_add_contributors_count.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs index d5fbc4ec97..3b094843ce 100644 --- a/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs +++ b/pallets/crowdloan/src/migrations/migrate_add_contributors_count.rs @@ -1,11 +1,13 @@ use alloc::string::String; use frame_support::{BoundedVec, migration::storage_key_iter, traits::Get, weights::Weight}; +use subtensor_macros::freeze_struct; use crate::*; mod old_storage { use super::*; + #[freeze_struct("84bcbf9b8d3f0ddf")] #[derive(Encode, Decode, Debug)] pub struct OldCrowdloanInfo { pub creator: AccountId, From b43e332f97ca0696490695872fa51526682ac108 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 7 May 2025 13:40:39 -0400 Subject: [PATCH 16/28] have Dockerfile use a local user instead of root * also use rust:latest so we don't have to keep bumping --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 447ed98b5e..3d0e1600aa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # ------------------------------------------------------------------------------ -# Subtensor Dockerfile (hardened) +# Subtensor Dockerfile (hardened, full-functionality) # – Builds production and local binaries # – Final runtime images run as non-root `subtensor` user (UID/GID 10001) # ------------------------------------------------------------------------------ From 7ca5bcbeba67c37014a4eb040beb08dedd3971ad Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 7 May 2025 13:47:27 -0400 Subject: [PATCH 17/28] clean up --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 3d0e1600aa..447ed98b5e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # ------------------------------------------------------------------------------ -# Subtensor Dockerfile (hardened, full-functionality) +# Subtensor Dockerfile (hardened) # – Builds production and local binaries # – Final runtime images run as non-root `subtensor` user (UID/GID 10001) # ------------------------------------------------------------------------------ From 04a86f825d16a9d4b1f2dbf7c763bf6d463b30cd Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Tue, 4 Feb 2025 08:06:49 +0000 Subject: [PATCH 18/28] add yuma4 scenario test --- pallets/subtensor/src/tests/epoch.rs | 155 +++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index e4b2f02574..840c77dba7 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -3197,3 +3197,158 @@ fn assert_approx_eq_vec_of_vec( } } } + +// test Yuma 4 scenarios over a sequence of epochs. +fn setup_yuma_4_scenario(netuid: u16, n: u16, max_stake: u64, stakes: Vec) { + let block_number = System::block_number(); + let tempo: u16 = u16::MAX - 1; // high tempo to skip automatic epochs in on_initialize, use manual epochs instead + add_network(netuid, tempo, 0); + + SubtensorModule::set_max_allowed_uids(netuid, n); + assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); + SubtensorModule::set_max_registrations_per_block(netuid, n); + SubtensorModule::set_target_registrations_per_interval(netuid, n); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + SubtensorModule::set_min_allowed_weights(netuid, 1); + SubtensorModule::set_max_weight_limit(netuid, u16::MAX); + SubtensorModule::set_bonds_penalty(netuid, 0); + + // === Register + for key in 0..n as u64 { + SubtensorModule::add_balance_to_coldkey_account(&U256::from(key), max_stake); + let (nonce, work): (u64, Vec) = SubtensorModule::create_work_for_block_number( + netuid, + block_number, + key * 1_000_000, + &U256::from(key), + ); + assert_ok!(SubtensorModule::register( + <::RuntimeOrigin>::signed(U256::from(key)), + netuid, + block_number, + nonce, + work, + U256::from(key), + U256::from(key) + )); + SubtensorModule::increase_stake_for_hotkey_and_coldkey_on_subnet( + &U256::from(key), + &U256::from(key), + netuid, + stakes[key as usize], + ); + } + assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); + assert_eq!(SubtensorModule::get_subnetwork_n(netuid), n); + + // Enable Liquid Alpha + SubtensorModule::set_kappa(netuid, 0); + SubtensorModule::set_liquid_alpha_enabled(netuid, true); + + SubtensorModule::set_alpha_values_32(netuid, I32F32::from_num(0.9), I32F32::from_num(0.99)); + + // === Issue validator permits + SubtensorModule::set_max_allowed_validators(netuid, 3); + assert_eq!(SubtensorModule::get_max_allowed_validators(netuid), 3); + SubtensorModule::epoch(netuid, 1_000_000_000); // run first epoch to set allowed validators + next_block(); // run to next block to ensure weights are set on nodes after their registration block +} + +fn run_epoch_check_bonds(netuid: u16, sparse: bool, target_bonds: Vec>) { + next_block(); + if sparse { + SubtensorModule::epoch(netuid, 1_000_000_000); + } else { + SubtensorModule::epoch_dense(netuid, 1_000_000_000); + } + let bonds = SubtensorModule::get_bonds(netuid); + + // server 1 + assert_eq!(bonds[0][3], target_bonds[0][0]); + assert_eq!(bonds[1][3], target_bonds[1][0]); + assert_eq!(bonds[2][3], target_bonds[2][0]); + + // server 2 + assert_eq!(bonds[0][4], target_bonds[0][1]); + assert_eq!(bonds[1][4], target_bonds[1][1]); + assert_eq!(bonds[2][4], target_bonds[2][1]); +} + +#[test] +fn test_yuma_4_kappa_moves_last() { + new_test_ext(1).execute_with(|| { + let sparse: bool = false; + let n: u16 = 5; // 3 validators, 2 servers + let netuid: u16 = 1; + let max_stake: u64 = 8; + + // Validator A: kappa / Big validator (0.8) - moves last + // Validator B: Small eager validator (0.1) - moves first + // Validator C: Small lazy validator (0.1) - moves second + let stakes: Vec = vec![8, 1, 1, 0, 0]; + + setup_yuma_4_scenario(netuid, n, max_stake, stakes); + + // Initially, consensus is achieved by all Validators + for uid in [0, 1, 2] { + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(uid)), + netuid, + vec![3, 4], + vec![u16::MAX, 0], + 0 + )); + } + let target = vec![vec![65535, 0], vec![65535, 0], vec![65535, 0]]; + run_epoch_check_bonds(netuid, sparse, target); + + // Validator A -> Server 1 + // Validator B -> Server 2 + // Validator C -> Server 1 + for (uid, weights) in [vec![u16::MAX, 0], vec![0, u16::MAX], vec![u16::MAX, 0]] + .iter() + .enumerate() + { + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(uid)), + netuid, + vec![3, 4], + weights.to_vec(), + 0 + )); + } + let target = vec![vec![65535, 0], vec![220, 65535], vec![65535, 0]]; + run_epoch_check_bonds(netuid, sparse, target); + + // Validator A -> Server 1 + // Validator B -> Server 2 + // Validator C -> Server 2 + for (uid, weights) in [vec![u16::MAX, 0], vec![0, u16::MAX], vec![0, u16::MAX]] + .iter() + .enumerate() + { + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(uid)), + netuid, + vec![3, 4], + weights.to_vec(), + 0 + )); + } + let target = vec![vec![65535, 0], vec![1, 65535], vec![329, 64878]]; + run_epoch_check_bonds(netuid, sparse, target); + + // Subsequent epochs All validators -> Server 2 + for uid in [0, 1, 2] { + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(uid)), + netuid, + vec![3, 4], + vec![0, u16::MAX], + 0 + )); + } + let target = vec![vec![65535, 11866], vec![0, 65535], vec![328, 64996]]; + run_epoch_check_bonds(netuid, sparse, target); + }) +} From 4a16441c56ac178b96022c143db9f896d10280ac Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Wed, 5 Feb 2025 14:09:34 +0000 Subject: [PATCH 19/28] fix alpha values --- pallets/subtensor/src/tests/epoch.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index 840c77dba7..97c74c5335 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -3242,7 +3242,7 @@ fn setup_yuma_4_scenario(netuid: u16, n: u16, max_stake: u64, stakes: Vec) assert_eq!(SubtensorModule::get_subnetwork_n(netuid), n); // Enable Liquid Alpha - SubtensorModule::set_kappa(netuid, 0); + SubtensorModule::set_kappa(netuid, u16::MAX / 2); SubtensorModule::set_liquid_alpha_enabled(netuid, true); SubtensorModule::set_alpha_values_32(netuid, I32F32::from_num(0.9), I32F32::from_num(0.99)); @@ -3299,7 +3299,7 @@ fn test_yuma_4_kappa_moves_last() { 0 )); } - let target = vec![vec![65535, 0], vec![65535, 0], vec![65535, 0]]; + let target = vec![vec![656, 0], vec![656, 0], vec![656, 0]]; run_epoch_check_bonds(netuid, sparse, target); // Validator A -> Server 1 @@ -3317,7 +3317,7 @@ fn test_yuma_4_kappa_moves_last() { 0 )); } - let target = vec![vec![65535, 0], vec![220, 65535], vec![65535, 0]]; + let target = vec![vec![1305, 0], vec![649, 6553], vec![1305, 0]]; run_epoch_check_bonds(netuid, sparse, target); // Validator A -> Server 1 @@ -3335,7 +3335,7 @@ fn test_yuma_4_kappa_moves_last() { 0 )); } - let target = vec![vec![65535, 0], vec![1, 65535], vec![329, 64878]]; + let target = vec![vec![1947, 0], vec![642, 12451], vec![1291, 6553]]; run_epoch_check_bonds(netuid, sparse, target); // Subsequent epochs All validators -> Server 2 @@ -3348,7 +3348,7 @@ fn test_yuma_4_kappa_moves_last() { 0 )); } - let target = vec![vec![65535, 11866], vec![0, 65535], vec![328, 64996]]; + let target = vec![vec![1752, 656], vec![577, 12982], vec![1161, 7143]]; run_epoch_check_bonds(netuid, sparse, target); }) } From 5cd75279d13c5b07058079842b668c8d5c3ecf3f Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Tue, 11 Feb 2025 12:03:40 +0000 Subject: [PATCH 20/28] tests cleanup wip --- pallets/subtensor/src/tests/epoch.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index 97c74c5335..9366869272 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -3262,16 +3262,39 @@ fn run_epoch_check_bonds(netuid: u16, sparse: bool, target_bonds: Vec>) SubtensorModule::epoch_dense(netuid, 1_000_000_000); } let bonds = SubtensorModule::get_bonds(netuid); + let dividends = SubtensorModule::get_dividends(netuid); + // Check the bonds // server 1 assert_eq!(bonds[0][3], target_bonds[0][0]); assert_eq!(bonds[1][3], target_bonds[1][0]); assert_eq!(bonds[2][3], target_bonds[2][0]); - // server 2 assert_eq!(bonds[0][4], target_bonds[0][1]); assert_eq!(bonds[1][4], target_bonds[1][1]); assert_eq!(bonds[2][4], target_bonds[2][1]); + + // Check the dividends + let epsilon = I32F32::from_num(1e-3); + for (dividend, target_dividend) in dividends.iter().zip(target_dividends.iter()) { + assert_approx_eq( + u16_proportion_to_fixed(*dividend), + fixed(*target_dividend), + epsilon, + ); + } +} + +fn set_yuma_4_weights(netuid: u16, weights: Vec>) { + for (uid, weight) in weights.iter().enumerate() { + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(uid as u64)), + netuid, + vec![3, 4], + weight.to_vec(), + 0 + )); + } } #[test] From 1b541e61e4e0b2bffb3cb12290aac6803b5f1111 Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Mon, 17 Feb 2025 12:13:27 +0000 Subject: [PATCH 21/28] add BondsResetOn param --- hyperparameters.md | 2 ++ pallets/admin-utils/src/tests/mock.rs | 2 ++ pallets/subtensor/src/lib.rs | 15 ++++++++++++++- pallets/subtensor/src/macros/config.rs | 3 +++ pallets/subtensor/src/macros/events.rs | 2 ++ pallets/subtensor/src/tests/mock.rs | 2 ++ pallets/subtensor/src/utils/misc.rs | 8 ++++++++ runtime/src/lib.rs | 2 ++ 8 files changed, 35 insertions(+), 1 deletion(-) diff --git a/hyperparameters.md b/hyperparameters.md index c8d2ce1106..66ab41c203 100644 --- a/hyperparameters.md +++ b/hyperparameters.md @@ -33,6 +33,7 @@ MaxRegistrationsPerBlock: u16 = 1; PruningScore : u16 = u16::MAX; BondsMovingAverage: u64 = 900_000; BondsPenalty: u16 = 0; +BondsResetOn: bool = false; WeightsVersionKey: u64 = 1020; MinDifficulty: u64 = 10_000_000; MaxDifficulty: u64 = u64::MAX / 4; @@ -72,6 +73,7 @@ MaxRegistrationsPerBlock: u16 = 1; PruningScore : u16 = u16::MAX; BondsMovingAverage: u64 = 900_000; BondsPenalty: u16 = 0; +BondsResetOn: bool = false; WeightsVersionKey: u64 = 400; MinDifficulty: u64 = 10_000_000; MaxDifficulty: u64 = u64::MAX / 4; diff --git a/pallets/admin-utils/src/tests/mock.rs b/pallets/admin-utils/src/tests/mock.rs index 3b87445dc8..ff9c21f22a 100644 --- a/pallets/admin-utils/src/tests/mock.rs +++ b/pallets/admin-utils/src/tests/mock.rs @@ -87,6 +87,7 @@ parameter_types! { pub const InitialMaxAllowedUids: u16 = 2; pub const InitialBondsMovingAverage: u64 = 900_000; pub const InitialBondsPenalty: u16 = u16::MAX; + pub const InitialBondsResetOn: bool = 0; pub const InitialStakePruningMin: u16 = 0; pub const InitialFoundationDistribution: u64 = 0; pub const InitialDefaultDelegateTake: u16 = 11_796; // 18% honest number. @@ -168,6 +169,7 @@ impl pallet_subtensor::Config for Test { type InitialPruningScore = InitialPruningScore; type InitialBondsMovingAverage = InitialBondsMovingAverage; type InitialBondsPenalty = InitialBondsPenalty; + type InitialBondsResetOn = InitialBondsResetOn; type InitialMaxAllowedValidators = InitialMaxAllowedValidators; type InitialDefaultDelegateTake = InitialDefaultDelegateTake; type InitialMinDelegateTake = InitialMinDelegateTake; diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 2194c49764..789c0bd10a 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -695,8 +695,13 @@ pub mod pallet { pub fn DefaultBondsPenalty() -> u16 { T::InitialBondsPenalty::get() } + /// Default value for bonds reset - will not reset bonds #[pallet::type_value] + pub fn DefaultBondsResetOn() -> bool { + T::InitialBondsResetOn::get() + } /// Default validator prune length. + #[pallet::type_value] pub fn DefaultValidatorPruneLen() -> u64 { T::InitialValidatorPruneLen::get() } @@ -804,7 +809,11 @@ pub mod pallet { pub fn DefaultAlphaValues() -> (u16, u16) { (45875, 58982) } - + #[pallet::type_value] + /// Default value for network max stake. + pub fn DefaultNetworkMaxStake() -> u64 { + T::InitialNetworkMaxStake::get() + } #[pallet::type_value] /// Default value for coldkey swap schedule duration pub fn DefaultColdkeySwapScheduleDuration() -> BlockNumberFor { @@ -1370,6 +1379,10 @@ pub mod pallet { /// --- MAP ( netuid ) --> bonds_penalty pub type BondsPenalty = StorageMap<_, Identity, u16, u16, ValueQuery, DefaultBondsPenalty>; + #[pallet::storage] + /// --- MAP ( netuid ) --> bonds_reset + pub type BondsResetOn = + StorageMap<_, Identity, u16, bool, ValueQuery, DefaultBondsResetOn>; /// --- MAP ( netuid ) --> weights_set_rate_limit #[pallet::storage] pub type WeightsSetRateLimit = diff --git a/pallets/subtensor/src/macros/config.rs b/pallets/subtensor/src/macros/config.rs index 4291b67c6f..188372987e 100644 --- a/pallets/subtensor/src/macros/config.rs +++ b/pallets/subtensor/src/macros/config.rs @@ -96,6 +96,9 @@ mod config { /// Initial bonds penalty. #[pallet::constant] type InitialBondsPenalty: Get; + /// Initial bonds reset. + #[pallet::constant] + type InitialBondsResetOn: Get; /// Initial target registrations per interval. #[pallet::constant] type InitialTargetRegistrationsPerInterval: Get; diff --git a/pallets/subtensor/src/macros/events.rs b/pallets/subtensor/src/macros/events.rs index ccbfed9eff..557a4b97c6 100644 --- a/pallets/subtensor/src/macros/events.rs +++ b/pallets/subtensor/src/macros/events.rs @@ -83,6 +83,8 @@ mod events { BondsMovingAverageSet(u16, u64), /// bonds penalty is set for a subnet. BondsPenaltySet(u16, u16), + /// bonds reset is set for a subnet. + BondsResetOnSet(u16, bool), /// setting the max number of allowed validators on a subnet. MaxAllowedValidatorsSet(u16, u16), /// the axon server information is added to the network. diff --git a/pallets/subtensor/src/tests/mock.rs b/pallets/subtensor/src/tests/mock.rs index ba1395843c..e10b5c4db2 100644 --- a/pallets/subtensor/src/tests/mock.rs +++ b/pallets/subtensor/src/tests/mock.rs @@ -139,6 +139,7 @@ parameter_types! { pub const InitialMaxAllowedUids: u16 = 2; pub const InitialBondsMovingAverage: u64 = 900_000; pub const InitialBondsPenalty:u16 = u16::MAX; + pub const InitialBondsResetOn: bool = false; pub const InitialStakePruningMin: u16 = 0; pub const InitialFoundationDistribution: u64 = 0; pub const InitialDefaultDelegateTake: u16 = 11_796; // 18%, same as in production @@ -377,6 +378,7 @@ impl crate::Config for Test { type InitialPruningScore = InitialPruningScore; type InitialBondsMovingAverage = InitialBondsMovingAverage; type InitialBondsPenalty = InitialBondsPenalty; + type InitialBondsResetOn = InitialBondsResetOn; type InitialMaxAllowedValidators = InitialMaxAllowedValidators; type InitialDefaultDelegateTake = InitialDefaultDelegateTake; type InitialMinDelegateTake = InitialMinDelegateTake; diff --git a/pallets/subtensor/src/utils/misc.rs b/pallets/subtensor/src/utils/misc.rs index b375cc66e4..357a537f21 100644 --- a/pallets/subtensor/src/utils/misc.rs +++ b/pallets/subtensor/src/utils/misc.rs @@ -582,6 +582,14 @@ impl Pallet { Self::deposit_event(Event::BondsPenaltySet(netuid, bonds_penalty)); } + pub fn get_bonds_reset(netuid: u16) -> bool { + BondsResetOn::::get(netuid) + } + pub fn set_bonds_reset(netuid: u16, bonds_reset: bool) { + BondsResetOn::::insert(netuid, bonds_reset); + Self::deposit_event(Event::BondsResetOnSet(netuid, bonds_reset)); + } + pub fn get_max_registrations_per_block(netuid: u16) -> u16 { MaxRegistrationsPerBlock::::get(netuid) } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 9fc76f69dc..11792dd93f 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1041,6 +1041,7 @@ parameter_types! { pub const SubtensorInitialPruningScore : u16 = u16::MAX; pub const SubtensorInitialBondsMovingAverage: u64 = 900_000; pub const SubtensorInitialBondsPenalty: u16 = u16::MAX; + pub const SubtensorInitialBondsResetOn: bool = false; pub const SubtensorInitialDefaultTake: u16 = 11_796; // 18% honest number. pub const SubtensorInitialMinDelegateTake: u16 = 0; // Allow 0% delegate take pub const SubtensorInitialDefaultChildKeyTake: u16 = 0; // Allow 0% childkey take @@ -1096,6 +1097,7 @@ impl pallet_subtensor::Config for Runtime { type InitialMaxAllowedUids = SubtensorInitialMaxAllowedUids; type InitialBondsMovingAverage = SubtensorInitialBondsMovingAverage; type InitialBondsPenalty = SubtensorInitialBondsPenalty; + type InitialBondsResetOn = SubtensorInitialBondsResetOn; type InitialIssuance = SubtensorInitialIssuance; type InitialMinAllowedWeights = SubtensorInitialMinAllowedWeights; type InitialEmissionValue = SubtensorInitialEmissionValue; From 5d14a1be4c235835c420ce0e588deb2f6548e424 Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Mon, 17 Feb 2025 12:23:25 +0000 Subject: [PATCH 22/28] add OnMetadataCommitment hook --- pallets/admin-utils/src/tests/mock.rs | 2 +- pallets/commitments/src/lib.rs | 13 ++++++++++++- pallets/commitments/src/weights.rs | 6 +++--- pallets/subtensor/src/epoch/run_epoch.rs | 9 +++++++++ runtime/src/lib.rs | 14 +++++++++++++- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/pallets/admin-utils/src/tests/mock.rs b/pallets/admin-utils/src/tests/mock.rs index ff9c21f22a..7c6a000b09 100644 --- a/pallets/admin-utils/src/tests/mock.rs +++ b/pallets/admin-utils/src/tests/mock.rs @@ -87,7 +87,7 @@ parameter_types! { pub const InitialMaxAllowedUids: u16 = 2; pub const InitialBondsMovingAverage: u64 = 900_000; pub const InitialBondsPenalty: u16 = u16::MAX; - pub const InitialBondsResetOn: bool = 0; + pub const InitialBondsResetOn: bool = false; pub const InitialStakePruningMin: u16 = 0; pub const InitialFoundationDistribution: u64 = 0; pub const InitialDefaultDelegateTake: u16 = 11_796; // 18% honest number. diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 34bf27566a..20c7b43752 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -56,6 +56,9 @@ pub mod pallet { /// Interface to access-limit metadata commitments type CanCommit: CanCommit; + /// Interface to trigger other pallets when metadata is committed + type OnMetadataCommitment: OnMetadataCommitment; + /// The maximum number of additional fields that can be added to a commitment #[pallet::constant] type MaxFields: Get + TypeInfo + 'static; @@ -193,7 +196,7 @@ pub mod pallet { netuid: u16, info: Box>, ) -> DispatchResult { - let who = ensure_signed(origin)?; + let who = ensure_signed(origin.clone())?; ensure!( T::CanCommit::can_commit(netuid, &who), Error::::AccountNotAllowedCommit @@ -349,6 +352,14 @@ impl CanCommit for () { } } +pub trait OnMetadataCommitment { + fn on_metadata_commitment(netuid: u16, account: &AccountId); +} + +impl OnMetadataCommitment for () { + fn on_metadata_commitment(_: u16, _: &A) {} +} + /************************************************************ CallType definition ************************************************************/ diff --git a/pallets/commitments/src/weights.rs b/pallets/commitments/src/weights.rs index b91017e050..e1bd05fcc7 100644 --- a/pallets/commitments/src/weights.rs +++ b/pallets/commitments/src/weights.rs @@ -53,7 +53,7 @@ impl WeightInfo for SubstrateWeight { fn set_rate_limit() -> Weight { Weight::from_parts(10_000_000, 2000) .saturating_add(RocksDbWeight::get().reads(1_u64)) - } + } } // For backwards compatibility and tests. @@ -76,5 +76,5 @@ impl WeightInfo for () { fn set_rate_limit() -> Weight { Weight::from_parts(10_000_000, 2000) .saturating_add(RocksDbWeight::get().reads(1_u64)) - } -} \ No newline at end of file + } +} diff --git a/pallets/subtensor/src/epoch/run_epoch.rs b/pallets/subtensor/src/epoch/run_epoch.rs index 62027f9636..931cf0d776 100644 --- a/pallets/subtensor/src/epoch/run_epoch.rs +++ b/pallets/subtensor/src/epoch/run_epoch.rs @@ -1294,4 +1294,13 @@ impl Pallet { ); Ok(()) } + + pub fn do_reset_bonds(netuid: u16, account_id: T::AccountId) -> Result<(), DispatchError> { + // check bonds reset enabled for this subnet + let bonds_reset_enabled: bool = Self::get_bonds_reset(netuid); + if !bonds_reset_enabled { + return Ok(()); + } + Ok(()) + } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 11792dd93f..979ecc3998 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -26,7 +26,7 @@ use frame_support::{ }, }; use frame_system::{EnsureNever, EnsureRoot, EnsureRootWithSuccess, RawOrigin}; -use pallet_commitments::CanCommit; +use pallet_commitments::{CanCommit, OnMetadataCommitment}; use pallet_grandpa::{ AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList, fg_primitives, }; @@ -980,12 +980,24 @@ impl CanCommit for AllowCommitments { } } +pub struct ResetBondsOnCommit; +impl OnMetadataCommitment for ResetBondsOnCommit { + #[cfg(not(feature = "runtime-benchmarks"))] + fn on_metadata_commitment(netuid: u16, address: &AccountId) { + SubtensorModule::do_reset_bonds(netuid, address); + } + + #[cfg(feature = "runtime-benchmarks")] + fn on_metadata_commitment(_: u16, _: &AccountId) {} +} + impl pallet_commitments::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Currency = Balances; type WeightInfo = pallet_commitments::weights::SubstrateWeight; type CanCommit = AllowCommitments; + type OnMetadataCommitment = ResetBondsOnCommit; type MaxFields = MaxCommitFields; type InitialDeposit = CommitmentInitialDeposit; From c0544298aaf86cdd1939d7bbd1edef916ae7b17d Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Thu, 20 Feb 2025 06:21:20 +0000 Subject: [PATCH 23/28] add bonds reset --- pallets/subtensor/src/epoch/run_epoch.rs | 12 +++ pallets/subtensor/src/lib.rs | 5 - pallets/subtensor/src/tests/epoch.rs | 121 +++++++++++++++++++++++ 3 files changed, 133 insertions(+), 5 deletions(-) diff --git a/pallets/subtensor/src/epoch/run_epoch.rs b/pallets/subtensor/src/epoch/run_epoch.rs index 931cf0d776..a1b792e9fb 100644 --- a/pallets/subtensor/src/epoch/run_epoch.rs +++ b/pallets/subtensor/src/epoch/run_epoch.rs @@ -1301,6 +1301,18 @@ impl Pallet { if !bonds_reset_enabled { return Ok(()); } + + // Reset all bonds for this subnet + log::info!( + "Reseting bonds for netuid: {:?} triggered by {:?}", + netuid, + account_id + ); + let n = Self::get_subnetwork_n(netuid); + let new_bonds_row: Vec<(u16, u16)> = (0..n).zip(vec![0; n as usize]).collect(); + for uid in 0..n { + Bonds::::insert(netuid, uid, new_bonds_row.clone()); + } Ok(()) } } diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 789c0bd10a..585cab0ed0 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -810,11 +810,6 @@ pub mod pallet { (45875, 58982) } #[pallet::type_value] - /// Default value for network max stake. - pub fn DefaultNetworkMaxStake() -> u64 { - T::InitialNetworkMaxStake::get() - } - #[pallet::type_value] /// Default value for coldkey swap schedule duration pub fn DefaultColdkeySwapScheduleDuration() -> BlockNumberFor { T::InitialColdkeySwapScheduleDuration::get() diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index 9366869272..0a9c2d9a63 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -2765,6 +2765,38 @@ fn test_compute_ema_bonds_with_liquid_alpha_sparse_empty() { ); } +#[test] +fn test_bonds_reset() { + new_test_ext(1).execute_with(|| { + let netuid = 1; + let n: u16 = 10; + SubnetworkN::::insert(netuid, n); + SubtensorModule::set_max_allowed_uids(netuid, n); + let initial_bonds: Vec<(u16, u16)> = (1..n).map(|i| (i, i)).collect(); + for uid in 0..n { + Bonds::::insert(netuid, uid, initial_bonds.clone()); + } + + SubtensorModule::set_bonds_reset(netuid, false); + let _ = SubtensorModule::do_reset_bonds(netuid, U256::from(0)); + // should noop + let new_bonds = SubtensorModule::get_bonds(netuid); + let target_bond: Vec<_> = (0..n).map(u16_to_fixed).collect(); + for new_bond in new_bonds.iter() { + assert_eq!(new_bond, &target_bond); + } + + SubtensorModule::set_bonds_reset(netuid, true); + let _ = SubtensorModule::do_reset_bonds(netuid, U256::from(0)); + // should reset all netuid bonds to 0 + let new_bonds = SubtensorModule::get_bonds(netuid); + let target_bond: Vec = vec![I32F32::from(0); n as usize]; + for new_bond in new_bonds.iter() { + assert_eq!(new_bond, &target_bond); + } + }) +} + #[test] fn test_get_set_alpha() { new_test_ext(1).execute_with(|| { @@ -3375,3 +3407,92 @@ fn test_yuma_4_kappa_moves_last() { run_epoch_check_bonds(netuid, sparse, target); }) } + +#[test] +fn test_yuma_4_bonds_reset() { + new_test_ext(1).execute_with(|| { + let sparse: bool = true; + let n: u16 = 5; // 3 validators, 2 servers + let netuid: u16 = 1; + let max_stake: u64 = 8; + + // "Case 8 - big vali moves late, then late" + // Big dishonest lazy vali. (0.8) + // Small eager-eager vali. (0.1) + // Small eager-eager vali 2. (0.1) + let stakes: Vec = vec![8, 1, 1, 0, 0]; + + setup_yuma_4_scenario(netuid, n, sparse, max_stake, stakes); + + // target bonds and dividends for specific epoch + let targets_dividends: std::collections::HashMap<_, _> = [ + (0, vec![0.8000, 0.1000, 0.1000, 0.0000, 0.0000]), + (1, vec![0.8894, 0.0553, 0.0553, 0.0000, 0.0000]), + (2, vec![0.2686, 0.3656, 0.3656, 0.0000, 0.0000]), + (19, vec![0.7267, 0.1366, 0.1366, 0.0000, 0.0000]), + (20, vec![0.8000, 0.1000, 0.1000, 0.0000, 0.0000]), + (21, vec![0.8893, 0.0553, 0.0553, 0.0000, 0.0000]), + (40, vec![0.7306, 0.1346, 0.1346, 0.0000, 0.0000]), + ] + .into_iter() + .collect(); + let targets_bonds: std::collections::HashMap<_, _> = [ + (0, vec![vec![656, 0], vec![656, 0], vec![656, 0]]), + (1, vec![vec![1305, 0], vec![649, 6553], vec![649, 6553]]), + (2, vec![vec![1174, 656], vec![584, 7143], vec![584, 7143]]), + (19, vec![vec![191, 10847], vec![93, 16313], vec![93, 16313]]), + (20, vec![vec![0, 656], vec![0, 656], vec![0, 656]]), + (21, vec![vec![0, 1305], vec![6553, 649], vec![6553, 649]]), + (40, vec![vec![11394, 171], vec![16805, 83], vec![16805, 83]]), + ] + .into_iter() + .collect(); + + for epoch in 0..=40 { + log::warn!("Epoch: {}", epoch); + match epoch { + 0 => { + // All validators -> Server 1 + set_yuma_4_weights(netuid, vec![vec![u16::MAX, 0]; 3]); + } + 1 => { + // Validator A -> Server 1 + // Validator B -> Server 2 + // Validator C -> Server 2 + set_yuma_4_weights( + netuid, + vec![vec![u16::MAX, 0], vec![0, u16::MAX], vec![0, u16::MAX]], + ); + } + (2..=20) => { + // All validators -> Server 2 + set_yuma_4_weights(netuid, vec![vec![0, u16::MAX]; 3]); + if epoch == 20 { + let _ = SubtensorModule::do_reset_bonds(netuid, U256::from(0)); + } + } + 21 => { + // Validator A -> Server 2 + // Validator B -> Server 1 + // Validator C -> Server 1 + set_yuma_4_weights( + netuid, + vec![vec![0, u16::MAX], vec![u16::MAX, 0], vec![u16::MAX, 0]], + ); + } + _ => { + // All validators -> Server 1 + set_yuma_4_weights(netuid, vec![vec![u16::MAX, 0]; 3]); + } + }; + + if let Some((target_dividend, target_bond)) = + targets_dividends.get(&epoch).zip(targets_bonds.get(&epoch)) + { + run_epoch_and_check_bonds_dividends(netuid, sparse, target_bond, target_dividend); + } else { + run_epoch(netuid, sparse); + } + } + }) +} From 249901a1abc4a24aa14137fad42c8dd3a16e9090 Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Wed, 26 Feb 2025 13:51:16 +0000 Subject: [PATCH 24/28] add ResetBondsFlag check for set_commitment --- pallets/commitments/src/lib.rs | 10 +++++++++- pallets/commitments/src/mock.rs | 1 + pallets/commitments/src/tests.rs | 3 +++ pallets/commitments/src/types.rs | 12 ++++++++++-- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 20c7b43752..0221b3f670 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -75,7 +75,7 @@ pub mod pallet { type TempoInterface: GetTempoInterface; } - /// Used to retreive the given subnet's tempo + /// Used to retreive the given subnet's tempo pub trait GetTempoInterface { /// Used to retreive the epoch index for the given subnet. fn get_epoch_index(netuid: u16, cur_block: u64) -> u64; @@ -227,6 +227,14 @@ pub mod pallet { usage.used_space = 0; } + // check if ResetBondsFlag is set in the fields + for field in info.fields.iter() { + if let Data::ResetBondsFlag = field { + T::OnMetadataCommitment::on_metadata_commitment(netuid, &who); + break; + } + } + let max_allowed = MaxSpace::::get() as u64; ensure!( usage.used_space.saturating_add(required_space) <= max_allowed, diff --git a/pallets/commitments/src/mock.rs b/pallets/commitments/src/mock.rs index 8f9dbb4e5f..4e6aa123bd 100644 --- a/pallets/commitments/src/mock.rs +++ b/pallets/commitments/src/mock.rs @@ -101,6 +101,7 @@ impl pallet_commitments::Config for Test { type FieldDeposit = ConstU64<0>; type InitialDeposit = ConstU64<0>; type TempoInterface = MockTempoInterface; + type OnMetadataCommitment = (); } pub struct MockTempoInterface; diff --git a/pallets/commitments/src/tests.rs b/pallets/commitments/src/tests.rs index 5d28e0733b..55e406eb53 100644 --- a/pallets/commitments/src/tests.rs +++ b/pallets/commitments/src/tests.rs @@ -34,6 +34,7 @@ fn manual_data_type_info() { Data::ShaThree256(_) => "ShaThree256".to_string(), Data::Raw(bytes) => format!("Raw{}", bytes.len()), Data::TimelockEncrypted { .. } => "TimelockEncrypted".to_string(), + Data::ResetBondsFlag => "ResetBondsFlag".to_string(), }; if let scale_info::TypeDef::Variant(variant) = &type_info.type_def { let variant = variant @@ -63,6 +64,7 @@ fn manual_data_type_info() { let reveal_round_len = reveal_round.encode().len() as u32; // Typically 8 bytes encrypted_len + reveal_round_len } + Data::ResetBondsFlag => 0, }; assert_eq!( encoded.len() as u32 - 1, // Subtract variant byte @@ -89,6 +91,7 @@ fn manual_data_type_info() { Data::Sha256(Default::default()), Data::Keccak256(Default::default()), Data::ShaThree256(Default::default()), + Data::ResetBondsFlag, ]; // Add Raw instances for all possible sizes diff --git a/pallets/commitments/src/types.rs b/pallets/commitments/src/types.rs index 0f1d2302a5..a537514f61 100644 --- a/pallets/commitments/src/types.rs +++ b/pallets/commitments/src/types.rs @@ -58,6 +58,8 @@ pub enum Data { encrypted: BoundedVec>, reveal_round: u64, }, + /// Flag to trigger bonds reset for subnet + ResetBondsFlag, } impl Data { @@ -79,6 +81,7 @@ impl Data { | Data::Keccak256(arr) | Data::ShaThree256(arr) => arr.len() as u64, Data::TimelockEncrypted { encrypted, .. } => encrypted.len() as u64, + Data::ResetBondsFlag => 0, } } } @@ -108,6 +111,7 @@ impl Decode for Data { reveal_round, } } + 135 => Data::ResetBondsFlag, _ => return Err(codec::Error::from("invalid leading byte")), }) } @@ -136,6 +140,7 @@ impl Encode for Data { r.extend_from_slice(&reveal_round.encode()); r } + Data::ResetBondsFlag => vec![135], } } } @@ -158,7 +163,9 @@ impl TypeInfo for Data { type Identity = Self; fn type_info() -> Type { - let variants = Variants::new().variant("None", |v| v.index(0)); + let variants = Variants::new() + .variant("None", |v| v.index(0)) + .variant("ResetBondsFlag", |v| v.index(135)); // create a variant for all sizes of Raw data from 0-32 let variants = data_raw_variants!( @@ -321,7 +328,8 @@ impl TypeInfo for Data { }) .field(|f| f.name("reveal_round").ty::()), ) - }); + }) + .variant("ResetBondsFlag", |v| v.index(135)); Type::builder() .path(Path::new("Data", module_path!())) From 6222e219b976b7274eff1f1925b555d49eafcc8d Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Fri, 21 Mar 2025 13:35:21 +0000 Subject: [PATCH 25/28] fix bonds reset --- pallets/subtensor/src/epoch/run_epoch.rs | 34 +++++-- pallets/subtensor/src/tests/epoch.rs | 121 +++++++++++++++++++---- 2 files changed, 124 insertions(+), 31 deletions(-) diff --git a/pallets/subtensor/src/epoch/run_epoch.rs b/pallets/subtensor/src/epoch/run_epoch.rs index a1b792e9fb..9658a7a9f1 100644 --- a/pallets/subtensor/src/epoch/run_epoch.rs +++ b/pallets/subtensor/src/epoch/run_epoch.rs @@ -1302,17 +1302,31 @@ impl Pallet { return Ok(()); } - // Reset all bonds for this subnet - log::info!( - "Reseting bonds for netuid: {:?} triggered by {:?}", - netuid, - account_id - ); - let n = Self::get_subnetwork_n(netuid); - let new_bonds_row: Vec<(u16, u16)> = (0..n).zip(vec![0; n as usize]).collect(); - for uid in 0..n { - Bonds::::insert(netuid, uid, new_bonds_row.clone()); + if let Ok(uid) = Self::get_uid_for_net_and_hotkey(netuid, &account_id) { + for (i, bonds_vec) in + as IterableStorageDoubleMap>>::iter_prefix( + netuid, + ) + { + Bonds::::insert( + netuid, + i, + bonds_vec + .clone() + .iter() + .filter(|(j, _)| *j != uid) + .collect::>(), + ); + } + log::debug!("Reset bonds for {:?}, netuid {:?}", account_id, netuid); + } else { + log::warn!( + "Uid not found for {:?}, netuid {:?} - skipping bonds reset", + account_id, + netuid + ); } + Ok(()) } } diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index 0a9c2d9a63..f8ddc33235 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -5,7 +5,7 @@ )] use super::mock::*; -use crate::epoch::math::safe_exp; +use crate::epoch::math::{fixed, safe_exp, u16_proportion_to_fixed, u16_to_fixed}; use crate::*; use approx::assert_abs_diff_eq; @@ -2770,15 +2770,41 @@ fn test_bonds_reset() { new_test_ext(1).execute_with(|| { let netuid = 1; let n: u16 = 10; + add_network(netuid, u16::MAX - 1, 0); SubnetworkN::::insert(netuid, n); SubtensorModule::set_max_allowed_uids(netuid, n); + SubtensorModule::set_max_registrations_per_block(netuid, n); + SubtensorModule::set_target_registrations_per_interval(netuid, n); + for key in 0..n as u64 { + SubtensorModule::add_balance_to_coldkey_account(&U256::from(key), 10); + let (nonce, work): (u64, Vec) = SubtensorModule::create_work_for_block_number( + netuid, + 0, + key * 1_000_000, + &U256::from(key), + ); + assert_ok!(SubtensorModule::register( + <::RuntimeOrigin>::signed(U256::from(key)), + netuid, + 0, + nonce, + work, + U256::from(key), + U256::from(key) + )); + } + let initial_bonds: Vec<(u16, u16)> = (1..n).map(|i| (i, i)).collect(); for uid in 0..n { Bonds::::insert(netuid, uid, initial_bonds.clone()); } + let uid_to_reset = 7; + let hotkey = SubtensorModule::get_hotkey_for_net_and_uid(netuid, uid_to_reset) + .expect("Hotkey not found"); + SubtensorModule::set_bonds_reset(netuid, false); - let _ = SubtensorModule::do_reset_bonds(netuid, U256::from(0)); + let _ = SubtensorModule::do_reset_bonds(netuid, hotkey); // should noop let new_bonds = SubtensorModule::get_bonds(netuid); let target_bond: Vec<_> = (0..n).map(u16_to_fixed).collect(); @@ -2787,12 +2813,10 @@ fn test_bonds_reset() { } SubtensorModule::set_bonds_reset(netuid, true); - let _ = SubtensorModule::do_reset_bonds(netuid, U256::from(0)); - // should reset all netuid bonds to 0 + let _ = SubtensorModule::do_reset_bonds(netuid, hotkey); let new_bonds = SubtensorModule::get_bonds(netuid); - let target_bond: Vec = vec![I32F32::from(0); n as usize]; for new_bond in new_bonds.iter() { - assert_eq!(new_bond, &target_bond); + assert_eq!(new_bond[uid_to_reset as usize], 0); } }) } @@ -3423,39 +3447,89 @@ fn test_yuma_4_bonds_reset() { let stakes: Vec = vec![8, 1, 1, 0, 0]; setup_yuma_4_scenario(netuid, n, sparse, max_stake, stakes); + SubtensorModule::set_bonds_reset(netuid, true); // target bonds and dividends for specific epoch let targets_dividends: std::collections::HashMap<_, _> = [ (0, vec![0.8000, 0.1000, 0.1000, 0.0000, 0.0000]), - (1, vec![0.8894, 0.0553, 0.0553, 0.0000, 0.0000]), - (2, vec![0.2686, 0.3656, 0.3656, 0.0000, 0.0000]), - (19, vec![0.7267, 0.1366, 0.1366, 0.0000, 0.0000]), - (20, vec![0.8000, 0.1000, 0.1000, 0.0000, 0.0000]), - (21, vec![0.8893, 0.0553, 0.0553, 0.0000, 0.0000]), - (40, vec![0.7306, 0.1346, 0.1346, 0.0000, 0.0000]), + (1, vec![0.8944, 0.0528, 0.0528, 0.0000, 0.0000]), + (2, vec![0.5230, 0.2385, 0.2385, 0.0000, 0.0000]), + (19, vec![0.7919, 0.1040, 0.1040, 0.0000, 0.0000]), + (20, vec![0.7928, 0.1036, 0.1036, 0.0000, 0.0000]), + (21, vec![0.8467, 0.0766, 0.0766, 0.0000, 0.0000]), + (40, vec![0.7928, 0.1036, 0.1036, 0.0000, 0.0000]), ] .into_iter() .collect(); let targets_bonds: std::collections::HashMap<_, _> = [ - (0, vec![vec![656, 0], vec![656, 0], vec![656, 0]]), - (1, vec![vec![1305, 0], vec![649, 6553], vec![649, 6553]]), - (2, vec![vec![1174, 656], vec![584, 7143], vec![584, 7143]]), - (19, vec![vec![191, 10847], vec![93, 16313], vec![93, 16313]]), - (20, vec![vec![0, 656], vec![0, 656], vec![0, 656]]), - (21, vec![vec![0, 1305], vec![6553, 649], vec![6553, 649]]), - (40, vec![vec![11394, 171], vec![16805, 83], vec![16805, 83]]), + ( + 0, + vec![ + vec![0.1013, 0.0000], + vec![0.1013, 0.0000], + vec![0.1013, 0.0000], + ], + ), + ( + 1, + vec![ + vec![0.1924, 0.0000], + vec![0.0908, 0.2987], + vec![0.0908, 0.2987], + ], + ), + ( + 2, + vec![ + vec![0.1715, 0.1013], + vec![0.0815, 0.3697], + vec![0.0815, 0.3697], + ], + ), + ( + 19, + vec![ + vec![0.0269, 0.8539], + vec![0.0131, 0.8975], + vec![0.0131, 0.8975], + ], + ), + ( + 20, + vec![ + vec![0.0000, 0.8687], + vec![0.0000, 0.9079], + vec![0.0000, 0.9079], + ], + ), + ( + 21, + vec![ + vec![0.0000, 0.8820], + vec![0.2987, 0.6386], + vec![0.2987, 0.6386], + ], + ), + ( + 40, + vec![ + vec![0.8687, 0.0578], + vec![0.9079, 0.0523], + vec![0.9079, 0.0523], + ], + ), ] .into_iter() .collect(); for epoch in 0..=40 { - log::warn!("Epoch: {}", epoch); match epoch { 0 => { // All validators -> Server 1 set_yuma_4_weights(netuid, vec![vec![u16::MAX, 0]; 3]); } 1 => { + // validators B, C switch // Validator A -> Server 1 // Validator B -> Server 2 // Validator C -> Server 2 @@ -3465,13 +3539,17 @@ fn test_yuma_4_bonds_reset() { ); } (2..=20) => { + // validator A copies weights // All validators -> Server 2 set_yuma_4_weights(netuid, vec![vec![0, u16::MAX]; 3]); if epoch == 20 { - let _ = SubtensorModule::do_reset_bonds(netuid, U256::from(0)); + let hotkey = SubtensorModule::get_hotkey_for_net_and_uid(netuid, 3) + .expect("Hotkey not found"); + let _ = SubtensorModule::do_reset_bonds(netuid, hotkey); } } 21 => { + // validators B, C switch back // Validator A -> Server 2 // Validator B -> Server 1 // Validator C -> Server 1 @@ -3481,6 +3559,7 @@ fn test_yuma_4_bonds_reset() { ); } _ => { + // validator A copies weights // All validators -> Server 1 set_yuma_4_weights(netuid, vec![vec![u16::MAX, 0]; 3]); } From 59bcbf1ef07e6e73fe90111f3f1769abcf36d8bd Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Tue, 25 Mar 2025 12:56:11 +0000 Subject: [PATCH 26/28] fix rebase --- pallets/subtensor/src/epoch/run_epoch.rs | 4 +- pallets/subtensor/src/tests/epoch.rs | 200 ++--------------------- runtime/src/lib.rs | 2 +- 3 files changed, 15 insertions(+), 191 deletions(-) diff --git a/pallets/subtensor/src/epoch/run_epoch.rs b/pallets/subtensor/src/epoch/run_epoch.rs index 9658a7a9f1..af48420779 100644 --- a/pallets/subtensor/src/epoch/run_epoch.rs +++ b/pallets/subtensor/src/epoch/run_epoch.rs @@ -1295,14 +1295,14 @@ impl Pallet { Ok(()) } - pub fn do_reset_bonds(netuid: u16, account_id: T::AccountId) -> Result<(), DispatchError> { + pub fn do_reset_bonds(netuid: u16, account_id: &T::AccountId) -> Result<(), DispatchError> { // check bonds reset enabled for this subnet let bonds_reset_enabled: bool = Self::get_bonds_reset(netuid); if !bonds_reset_enabled { return Ok(()); } - if let Ok(uid) = Self::get_uid_for_net_and_hotkey(netuid, &account_id) { + if let Ok(uid) = Self::get_uid_for_net_and_hotkey(netuid, account_id) { for (i, bonds_vec) in as IterableStorageDoubleMap>>::iter_prefix( netuid, diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index f8ddc33235..8878a94850 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -2804,7 +2804,7 @@ fn test_bonds_reset() { .expect("Hotkey not found"); SubtensorModule::set_bonds_reset(netuid, false); - let _ = SubtensorModule::do_reset_bonds(netuid, hotkey); + let _ = SubtensorModule::do_reset_bonds(netuid, &hotkey); // should noop let new_bonds = SubtensorModule::get_bonds(netuid); let target_bond: Vec<_> = (0..n).map(u16_to_fixed).collect(); @@ -2813,7 +2813,7 @@ fn test_bonds_reset() { } SubtensorModule::set_bonds_reset(netuid, true); - let _ = SubtensorModule::do_reset_bonds(netuid, hotkey); + let _ = SubtensorModule::do_reset_bonds(netuid, &hotkey); let new_bonds = SubtensorModule::get_bonds(netuid); for new_bond in new_bonds.iter() { assert_eq!(new_bond[uid_to_reset as usize], 0); @@ -3254,186 +3254,8 @@ fn assert_approx_eq_vec_of_vec( } } -// test Yuma 4 scenarios over a sequence of epochs. -fn setup_yuma_4_scenario(netuid: u16, n: u16, max_stake: u64, stakes: Vec) { - let block_number = System::block_number(); - let tempo: u16 = u16::MAX - 1; // high tempo to skip automatic epochs in on_initialize, use manual epochs instead - add_network(netuid, tempo, 0); - - SubtensorModule::set_max_allowed_uids(netuid, n); - assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); - SubtensorModule::set_max_registrations_per_block(netuid, n); - SubtensorModule::set_target_registrations_per_interval(netuid, n); - SubtensorModule::set_weights_set_rate_limit(netuid, 0); - SubtensorModule::set_min_allowed_weights(netuid, 1); - SubtensorModule::set_max_weight_limit(netuid, u16::MAX); - SubtensorModule::set_bonds_penalty(netuid, 0); - - // === Register - for key in 0..n as u64 { - SubtensorModule::add_balance_to_coldkey_account(&U256::from(key), max_stake); - let (nonce, work): (u64, Vec) = SubtensorModule::create_work_for_block_number( - netuid, - block_number, - key * 1_000_000, - &U256::from(key), - ); - assert_ok!(SubtensorModule::register( - <::RuntimeOrigin>::signed(U256::from(key)), - netuid, - block_number, - nonce, - work, - U256::from(key), - U256::from(key) - )); - SubtensorModule::increase_stake_for_hotkey_and_coldkey_on_subnet( - &U256::from(key), - &U256::from(key), - netuid, - stakes[key as usize], - ); - } - assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); - assert_eq!(SubtensorModule::get_subnetwork_n(netuid), n); - - // Enable Liquid Alpha - SubtensorModule::set_kappa(netuid, u16::MAX / 2); - SubtensorModule::set_liquid_alpha_enabled(netuid, true); - - SubtensorModule::set_alpha_values_32(netuid, I32F32::from_num(0.9), I32F32::from_num(0.99)); - - // === Issue validator permits - SubtensorModule::set_max_allowed_validators(netuid, 3); - assert_eq!(SubtensorModule::get_max_allowed_validators(netuid), 3); - SubtensorModule::epoch(netuid, 1_000_000_000); // run first epoch to set allowed validators - next_block(); // run to next block to ensure weights are set on nodes after their registration block -} - -fn run_epoch_check_bonds(netuid: u16, sparse: bool, target_bonds: Vec>) { - next_block(); - if sparse { - SubtensorModule::epoch(netuid, 1_000_000_000); - } else { - SubtensorModule::epoch_dense(netuid, 1_000_000_000); - } - let bonds = SubtensorModule::get_bonds(netuid); - let dividends = SubtensorModule::get_dividends(netuid); - - // Check the bonds - // server 1 - assert_eq!(bonds[0][3], target_bonds[0][0]); - assert_eq!(bonds[1][3], target_bonds[1][0]); - assert_eq!(bonds[2][3], target_bonds[2][0]); - // server 2 - assert_eq!(bonds[0][4], target_bonds[0][1]); - assert_eq!(bonds[1][4], target_bonds[1][1]); - assert_eq!(bonds[2][4], target_bonds[2][1]); - - // Check the dividends - let epsilon = I32F32::from_num(1e-3); - for (dividend, target_dividend) in dividends.iter().zip(target_dividends.iter()) { - assert_approx_eq( - u16_proportion_to_fixed(*dividend), - fixed(*target_dividend), - epsilon, - ); - } -} - -fn set_yuma_4_weights(netuid: u16, weights: Vec>) { - for (uid, weight) in weights.iter().enumerate() { - assert_ok!(SubtensorModule::set_weights( - RuntimeOrigin::signed(U256::from(uid as u64)), - netuid, - vec![3, 4], - weight.to_vec(), - 0 - )); - } -} - -#[test] -fn test_yuma_4_kappa_moves_last() { - new_test_ext(1).execute_with(|| { - let sparse: bool = false; - let n: u16 = 5; // 3 validators, 2 servers - let netuid: u16 = 1; - let max_stake: u64 = 8; - - // Validator A: kappa / Big validator (0.8) - moves last - // Validator B: Small eager validator (0.1) - moves first - // Validator C: Small lazy validator (0.1) - moves second - let stakes: Vec = vec![8, 1, 1, 0, 0]; - - setup_yuma_4_scenario(netuid, n, max_stake, stakes); - - // Initially, consensus is achieved by all Validators - for uid in [0, 1, 2] { - assert_ok!(SubtensorModule::set_weights( - RuntimeOrigin::signed(U256::from(uid)), - netuid, - vec![3, 4], - vec![u16::MAX, 0], - 0 - )); - } - let target = vec![vec![656, 0], vec![656, 0], vec![656, 0]]; - run_epoch_check_bonds(netuid, sparse, target); - - // Validator A -> Server 1 - // Validator B -> Server 2 - // Validator C -> Server 1 - for (uid, weights) in [vec![u16::MAX, 0], vec![0, u16::MAX], vec![u16::MAX, 0]] - .iter() - .enumerate() - { - assert_ok!(SubtensorModule::set_weights( - RuntimeOrigin::signed(U256::from(uid)), - netuid, - vec![3, 4], - weights.to_vec(), - 0 - )); - } - let target = vec![vec![1305, 0], vec![649, 6553], vec![1305, 0]]; - run_epoch_check_bonds(netuid, sparse, target); - - // Validator A -> Server 1 - // Validator B -> Server 2 - // Validator C -> Server 2 - for (uid, weights) in [vec![u16::MAX, 0], vec![0, u16::MAX], vec![0, u16::MAX]] - .iter() - .enumerate() - { - assert_ok!(SubtensorModule::set_weights( - RuntimeOrigin::signed(U256::from(uid)), - netuid, - vec![3, 4], - weights.to_vec(), - 0 - )); - } - let target = vec![vec![1947, 0], vec![642, 12451], vec![1291, 6553]]; - run_epoch_check_bonds(netuid, sparse, target); - - // Subsequent epochs All validators -> Server 2 - for uid in [0, 1, 2] { - assert_ok!(SubtensorModule::set_weights( - RuntimeOrigin::signed(U256::from(uid)), - netuid, - vec![3, 4], - vec![0, u16::MAX], - 0 - )); - } - let target = vec![vec![1752, 656], vec![577, 12982], vec![1161, 7143]]; - run_epoch_check_bonds(netuid, sparse, target); - }) -} - #[test] -fn test_yuma_4_bonds_reset() { +fn test_yuma_3_bonds_reset() { new_test_ext(1).execute_with(|| { let sparse: bool = true; let n: u16 = 5; // 3 validators, 2 servers @@ -3446,7 +3268,7 @@ fn test_yuma_4_bonds_reset() { // Small eager-eager vali 2. (0.1) let stakes: Vec = vec![8, 1, 1, 0, 0]; - setup_yuma_4_scenario(netuid, n, sparse, max_stake, stakes); + setup_yuma_3_scenario(netuid, n, sparse, max_stake, stakes); SubtensorModule::set_bonds_reset(netuid, true); // target bonds and dividends for specific epoch @@ -3526,26 +3348,27 @@ fn test_yuma_4_bonds_reset() { match epoch { 0 => { // All validators -> Server 1 - set_yuma_4_weights(netuid, vec![vec![u16::MAX, 0]; 3]); + set_yuma_3_weights(netuid, vec![vec![u16::MAX, 0]; 3], vec![3, 4]); } 1 => { // validators B, C switch // Validator A -> Server 1 // Validator B -> Server 2 // Validator C -> Server 2 - set_yuma_4_weights( + set_yuma_3_weights( netuid, vec![vec![u16::MAX, 0], vec![0, u16::MAX], vec![0, u16::MAX]], + vec![3, 4], ); } (2..=20) => { // validator A copies weights // All validators -> Server 2 - set_yuma_4_weights(netuid, vec![vec![0, u16::MAX]; 3]); + set_yuma_3_weights(netuid, vec![vec![0, u16::MAX]; 3], vec![3, 4]); if epoch == 20 { let hotkey = SubtensorModule::get_hotkey_for_net_and_uid(netuid, 3) .expect("Hotkey not found"); - let _ = SubtensorModule::do_reset_bonds(netuid, hotkey); + let _ = SubtensorModule::do_reset_bonds(netuid, &hotkey); } } 21 => { @@ -3553,15 +3376,16 @@ fn test_yuma_4_bonds_reset() { // Validator A -> Server 2 // Validator B -> Server 1 // Validator C -> Server 1 - set_yuma_4_weights( + set_yuma_3_weights( netuid, vec![vec![0, u16::MAX], vec![u16::MAX, 0], vec![u16::MAX, 0]], + vec![3, 4], ); } _ => { // validator A copies weights // All validators -> Server 1 - set_yuma_4_weights(netuid, vec![vec![u16::MAX, 0]; 3]); + set_yuma_3_weights(netuid, vec![vec![u16::MAX, 0]; 3], vec![3, 4]); } }; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 979ecc3998..b4bcf097b9 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -984,7 +984,7 @@ pub struct ResetBondsOnCommit; impl OnMetadataCommitment for ResetBondsOnCommit { #[cfg(not(feature = "runtime-benchmarks"))] fn on_metadata_commitment(netuid: u16, address: &AccountId) { - SubtensorModule::do_reset_bonds(netuid, address); + let _ = SubtensorModule::do_reset_bonds(netuid, address); } #[cfg(feature = "runtime-benchmarks")] From 6ae811b84ece0ddb19a02d4863bc5d521721f8db Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Fri, 4 Apr 2025 19:09:48 +0100 Subject: [PATCH 27/28] increased commitment fields to allow for reset_bonds flag --- runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index b4bcf097b9..96b3495773 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -952,7 +952,7 @@ impl pallet_registry::Config for Runtime { } parameter_types! { - pub const MaxCommitFieldsInner: u32 = 1; + pub const MaxCommitFieldsInner: u32 = 2; pub const CommitmentInitialDeposit: Balance = 0; // Free pub const CommitmentFieldDeposit: Balance = 0; // Free } From f1229200946ca511af8277fd58bfff09dafa9a52 Mon Sep 17 00:00:00 2001 From: Andreea Popescu Date: Tue, 6 May 2025 08:41:53 +0100 Subject: [PATCH 28/28] add LastBondsReset tracking --- pallets/commitments/src/lib.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 0221b3f670..d0d8d14c9b 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -151,6 +151,19 @@ pub mod pallet { BlockNumberFor, OptionQuery, >; + + #[pallet::storage] + #[pallet::getter(fn last_bonds_reset)] + pub(super) type LastBondsReset = StorageDoubleMap< + _, + Identity, + u16, + Twox64Concat, + T::AccountId, + BlockNumberFor, + OptionQuery, + >; + #[pallet::storage] #[pallet::getter(fn revealed_commitments)] pub(super) type RevealedCommitments = StorageDoubleMap< @@ -230,6 +243,8 @@ pub mod pallet { // check if ResetBondsFlag is set in the fields for field in info.fields.iter() { if let Data::ResetBondsFlag = field { + // track when bonds reset was last triggered + >::insert(netuid, &who, cur_block); T::OnMetadataCommitment::on_metadata_commitment(netuid, &who); break; }