From e2fe0c0a7e3ce747cde947d787cc48e26427c99c Mon Sep 17 00:00:00 2001 From: librelois <c@elo.tf> Date: Wed, 24 Aug 2022 21:18:00 +0200 Subject: [PATCH] fix(runtime): first_eligible_ud not initialised for post-genesis identities (!98) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(identity): add call force_set_first_eligible_ud * fix(runtime): first_eligible_ud not init for post-genesis identities * tests(gdev):Â add test test_validate_new_idty_after_few_uds --- pallets/universal-dividend/src/lib.rs | 16 ++++++ pallets/universal-dividend/src/weights.rs | 8 +++ runtime/common/src/handlers.rs | 30 +++++------ .../src/weights/pallet_universal_dividend.rs | 7 +++ runtime/gdev/tests/integration_tests.rs | 54 +++++++++++++++++++ 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/pallets/universal-dividend/src/lib.rs b/pallets/universal-dividend/src/lib.rs index faf5749d2..78be0ed1d 100644 --- a/pallets/universal-dividend/src/lib.rs +++ b/pallets/universal-dividend/src/lib.rs @@ -398,6 +398,22 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { Self::do_transfer_ud(origin, dest, value, ExistenceRequirement::KeepAlive) } + + #[pallet::weight(T::WeightInfo::force_set_first_eligible_ud())] + pub fn force_set_first_eligible_ud( + origin: OriginFor<T>, + who: T::AccountId, + first_eligible_ud: FirstEligibleUd, + ) -> DispatchResultWithPostInfo { + ensure_root(origin)?; + + T::MembersStorage::try_mutate_exists(&who, |maybe_first_eligible_ud| { + if let Some(ref mut first_eligible_ud_) = maybe_first_eligible_ud { + *first_eligible_ud_ = first_eligible_ud; + } + Ok(().into()) + }) + } } // PUBLIC FUNCTIONS diff --git a/pallets/universal-dividend/src/weights.rs b/pallets/universal-dividend/src/weights.rs index e3b860880..9fa338ccf 100644 --- a/pallets/universal-dividend/src/weights.rs +++ b/pallets/universal-dividend/src/weights.rs @@ -26,6 +26,7 @@ pub trait WeightInfo { fn claim_uds(n: u32) -> Weight; fn transfer_ud() -> Weight; fn transfer_ud_keep_alive() -> Weight; + fn force_set_first_eligible_ud() -> Weight; } // Insecure weights implementation, use it for tests only! @@ -82,4 +83,11 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } + // Storage: Identity IdentityIndexOf (r:1 w:0) + // Storage: Identity Identities (r:1 w:1) + fn force_set_first_eligible_ud() -> Weight { + (0 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } } diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs index ca9ccd363..0ada22e65 100644 --- a/runtime/common/src/handlers.rs +++ b/runtime/common/src/handlers.rs @@ -41,10 +41,21 @@ impl<T> pallet_identity::traits::OnIdtyChange<T> for OnIdtyChangeHandler<T> where T: frame_system::Config<AccountId = AccountId>, T: pallet_authority_members::Config<MemberId = IdtyIndex>, - T: pallet_identity::Config<IdtyIndex = IdtyIndex>, + T: pallet_identity::Config<IdtyIndex = IdtyIndex, IdtyData = IdtyData>, + T: pallet_universal_dividend::Config, { fn on_idty_change(idty_index: IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight { match idty_event { + IdtyEvent::Validated => { + pallet_identity::Identities::<T>::mutate_exists(idty_index, |idty_val_opt| { + if let Some(ref mut idty_val) = idty_val_opt { + idty_val.data = IdtyData { + first_eligible_ud: + pallet_universal_dividend::Pallet::<T>::init_first_eligible_ud(), + } + } + }); + } IdtyEvent::ChangedOwnerKey { new_owner_key } => { if let Err(e) = pallet_authority_members::Pallet::<T>::change_owner_key( idty_index, @@ -56,10 +67,7 @@ where ); } } - IdtyEvent::Created { .. } - | IdtyEvent::Confirmed - | IdtyEvent::Validated - | IdtyEvent::Removed { .. } => {} + IdtyEvent::Created { .. } | IdtyEvent::Confirmed | IdtyEvent::Removed { .. } => {} } 0 } @@ -80,18 +88,6 @@ impl< { fn on_event(membership_event: &sp_membership::Event<IdtyIndex, MembershipMetaData>) -> Weight { (match membership_event { - sp_membership::Event::MembershipAcquired(idty_index, _owner_key) => { - pallet_identity::Identities::<Runtime>::mutate_exists(idty_index, |idty_val_opt| { - if let Some(ref mut idty_val) = idty_val_opt { - idty_val.data = IdtyData { - first_eligible_ud: - pallet_universal_dividend::Pallet::<Runtime>::init_first_eligible_ud( - ), - } - } - }); - Runtime::DbWeight::get().reads_writes(1, 1) - } sp_membership::Event::MembershipRevoked(idty_index) => { if let Some(idty_value) = pallet_identity::Identities::<Runtime>::get(idty_index) { if let Some(first_ud_index) = idty_value.data.first_eligible_ud.into() { diff --git a/runtime/common/src/weights/pallet_universal_dividend.rs b/runtime/common/src/weights/pallet_universal_dividend.rs index 9c9efb8f9..d5b7bfcab 100644 --- a/runtime/common/src/weights/pallet_universal_dividend.rs +++ b/runtime/common/src/weights/pallet_universal_dividend.rs @@ -96,4 +96,11 @@ impl<T: frame_system::Config> pallet_universal_dividend::WeightInfo for WeightIn .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } + // Storage: Identity IdentityIndexOf (r:1 w:0) + // Storage: Identity Identities (r:1 w:1) + fn force_set_first_eligible_ud() -> Weight { + (57_000_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } } diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs index c3c43dc8b..262f8977a 100644 --- a/runtime/gdev/tests/integration_tests.rs +++ b/runtime/gdev/tests/integration_tests.rs @@ -431,6 +431,60 @@ fn test_create_new_idty_without_founds() { }); } +#[test] +fn test_validate_new_idty_after_few_uds() { + ExtBuilder::new(1, 3, 4) + .with_initial_balances(vec![ + (AccountKeyring::Alice.to_account_id(), 1_000), + (AccountKeyring::Bob.to_account_id(), 1_000), + (AccountKeyring::Charlie.to_account_id(), 1_000), + (AccountKeyring::Eve.to_account_id(), 1_000), + ]) + .build() + .execute_with(|| { + run_to_block(21); + + // Should be able to create an identity + assert_ok!(Balances::transfer( + frame_system::RawOrigin::Signed(AccountKeyring::Alice.to_account_id()).into(), + MultiAddress::Id(AccountKeyring::Eve.to_account_id()), + 200 + )); + assert_ok!(Identity::create_identity( + frame_system::RawOrigin::Signed(AccountKeyring::Alice.to_account_id()).into(), + AccountKeyring::Eve.to_account_id(), + )); + + // At next block, the created identity should be confirmed by its owner + run_to_block(22); + assert_ok!(Identity::confirm_identity( + frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(), + pallet_identity::IdtyName::from("Eve"), + )); + + // At next block, Bob should be able to certify and validate the new identity + run_to_block(23); + assert_ok!(Cert::add_cert( + frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(), + 2, + 5, + )); + assert_ok!(Identity::validate_identity( + frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(), + 5, + )); + + // The new member should have first_eligible_ud equal to one + assert!(Identity::identity(5).is_some()); + assert_eq!( + Identity::identity(5).unwrap().data, + IdtyData { + first_eligible_ud: pallet_universal_dividend::FirstEligibleUd::from(3), + } + ); + }); +} + #[test] fn test_oneshot_accounts() { ExtBuilder::new(1, 3, 4) -- GitLab