Rococo Identity Migration Part 2 + Bug Fix (#2946)

joepetrowski · web-flow · commit f574868822d5 · 2024-01-18T06:46:08.000Z
Order: - [x] Start People Chain - [RPC node](https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frococo-people-rpc.polkadot.io#/explorer) - [x] Upgrade Rococo Relay (`EnsureRoot` -> `EnsureSigned`) (v1,006,002) - Done [here](https://rococo.subscan.io/extrinsic/0xef07e0f9dbb2b9e829305f132e6ce45d291239286e409177e20895e6687daa6c) - [x] Migrate all identities - Done, see extrinsics from [this account](https://rococo.subscan.io/account/5FyNYrBwndvBttTkGUqGGCRAXtBH4Mh8xELDaxaFywTsjDKb) - [x] Upgrade Rococo People (remove call filter) (v1,006,002) - Authorized [here](https://rococo.subscan.io/extrinsic/0xedf6a80229bd411b7ed8d3a489a767b0f773bed5c49239987a294c293a35b98b) With added: - [x] Upgrade Rococo People to fix `poke_deposit` bug (v1,006,001) - Authorized [here](https://rococo.subscan.io/extrinsic/0xd1dc3cd6e8274bd0196f8d9f13ed09f6e9c76e6a40f9786a1629f4cb22cf948d) Note: It's also possible to remove the Identity Migrator pallet from both the Relay Chain and the parachain at this time. I will leave them in for now to preserve the test cases until we run them on Kusama/Polkadot. We will also want a follow up to remove all Identity-related state from the Relay Chain.
diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/reap_identity.rs b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/src/tests/reap_identity.rs
@@ -291,7 +291,7 @@ fn assert_reap_id_relay(total_deposit: Balance, id: &Identity) {
 		assert_eq!(reserved_balance, total_deposit);
 
 		assert_ok!(RococoIdentityMigrator::reap_identity(
-			RococoOrigin::root(),
+			RococoOrigin::signed(RococoRelaySender::get()),
 			RococoRelaySender::get()
 		));
 
diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs
@@ -30,8 +30,7 @@ use frame_support::{
 	genesis_builder_helper::{build_config, create_default_config},
 	parameter_types,
 	traits::{
-		ConstBool, ConstU32, ConstU64, ConstU8, Contains, EitherOfDiverse, EverythingBut,
-		TransformOrigin,
+		ConstBool, ConstU32, ConstU64, ConstU8, EitherOfDiverse, Everything, TransformOrigin,
 	},
 	weights::{ConstantMultiplier, Weight},
 	PalletId,
@@ -124,7 +123,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("people-rococo"),
 	impl_name: create_runtime_str!("people-rococo"),
 	authoring_version: 1,
-	spec_version: 1_006_000,
+	spec_version: 1_006_002,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 0,
@@ -162,16 +161,9 @@ parameter_types! {
 	pub const SS58Prefix: u8 = 42;
 }
 
-pub struct IdentityCalls;
-impl Contains<RuntimeCall> for IdentityCalls {
-	fn contains(c: &RuntimeCall) -> bool {
-		matches!(c, RuntimeCall::Identity(_))
-	}
-}
-
 #[derive_impl(frame_system::config_preludes::ParaChainDefaultConfig as frame_system::DefaultConfig)]
 impl frame_system::Config for Runtime {
-	type BaseCallFilter = EverythingBut<IdentityCalls>;
+	type BaseCallFilter = Everything;
 	type BlockWeights = RuntimeBlockWeights;
 	type BlockLength = RuntimeBlockLength;
 	type AccountId = AccountId;
diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/people/people-rococo/src/xcm_config.rs
@@ -176,13 +176,17 @@ impl Contains<RuntimeCall> for SafeCallFilter {
 
 		matches!(
 			call,
-			RuntimeCall::PolkadotXcm(pallet_xcm::Call::force_xcm_version { .. }) |
-				RuntimeCall::System(
-					frame_system::Call::set_heap_pages { .. } |
-						frame_system::Call::set_code { .. } |
-						frame_system::Call::set_code_without_checks { .. } |
-						frame_system::Call::kill_prefix { .. },
-				) | RuntimeCall::ParachainSystem(..) |
+			RuntimeCall::PolkadotXcm(
+				pallet_xcm::Call::force_xcm_version { .. } |
+					pallet_xcm::Call::force_default_xcm_version { .. }
+			) | RuntimeCall::System(
+				frame_system::Call::set_heap_pages { .. } |
+					frame_system::Call::set_code { .. } |
+					frame_system::Call::set_code_without_checks { .. } |
+					frame_system::Call::authorize_upgrade { .. } |
+					frame_system::Call::authorize_upgrade_without_checks { .. } |
+					frame_system::Call::kill_prefix { .. },
+			) | RuntimeCall::ParachainSystem(..) |
 				RuntimeCall::Timestamp(..) |
 				RuntimeCall::Balances(..) |
 				RuntimeCall::CollatorSelection(
diff --git a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs
@@ -183,13 +183,17 @@ impl Contains<RuntimeCall> for SafeCallFilter {
 
 		matches!(
 			call,
-			RuntimeCall::PolkadotXcm(pallet_xcm::Call::force_xcm_version { .. }) |
-				RuntimeCall::System(
-					frame_system::Call::set_heap_pages { .. } |
-						frame_system::Call::set_code { .. } |
-						frame_system::Call::set_code_without_checks { .. } |
-						frame_system::Call::kill_prefix { .. },
-				) | RuntimeCall::ParachainSystem(..) |
+			RuntimeCall::PolkadotXcm(
+				pallet_xcm::Call::force_xcm_version { .. } |
+					pallet_xcm::Call::force_default_xcm_version { .. }
+			) | RuntimeCall::System(
+				frame_system::Call::set_heap_pages { .. } |
+					frame_system::Call::set_code { .. } |
+					frame_system::Call::set_code_without_checks { .. } |
+					frame_system::Call::authorize_upgrade { .. } |
+					frame_system::Call::authorize_upgrade_without_checks { .. } |
+					frame_system::Call::kill_prefix { .. },
+			) | RuntimeCall::ParachainSystem(..) |
 				RuntimeCall::Timestamp(..) |
 				RuntimeCall::Balances(..) |
 				RuntimeCall::CollatorSelection(
diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs
@@ -79,7 +79,7 @@ use frame_support::{
 	weights::{ConstantMultiplier, WeightMeter},
 	PalletId,
 };
-use frame_system::EnsureRoot;
+use frame_system::{EnsureRoot, EnsureSigned};
 use pallet_grandpa::{fg_primitives, AuthorityId as GrandpaId};
 use pallet_identity::legacy::IdentityInfo;
 use pallet_session::historical as session_historical;
@@ -150,7 +150,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("rococo"),
 	impl_name: create_runtime_str!("parity-rococo-v2.0"),
 	authoring_version: 0,
-	spec_version: 1_006_001,
+	spec_version: 1_006_002,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 24,
@@ -1142,8 +1142,7 @@ impl auctions::Config for Runtime {
 
 impl identity_migrator::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
-	// To be changed to `EnsureSigned` once there is a People Chain to migrate to.
-	type Reaper = EnsureRoot<AccountId>;
+	type Reaper = EnsureSigned<AccountId>;
 	type ReapIdentityHandler = ToParachainIdentityReaper<Runtime, Self::AccountId>;
 	type WeightInfo = weights::runtime_common_identity_migrator::WeightInfo<Runtime>;
 }
diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs
@@ -1395,16 +1395,21 @@ impl<T: Config> Pallet<T> {
 			},
 		)?;
 
-		// Subs Deposit
-		let new_subs_deposit = SubsOf::<T>::try_mutate(
-			&target,
-			|(current_subs_deposit, subs_of)| -> Result<BalanceOf<T>, DispatchError> {
-				let new_subs_deposit = Self::subs_deposit(subs_of.len() as u32);
-				Self::rejig_deposit(&target, *current_subs_deposit, new_subs_deposit)?;
-				*current_subs_deposit = new_subs_deposit;
-				Ok(new_subs_deposit)
-			},
-		)?;
+		let new_subs_deposit = if SubsOf::<T>::contains_key(&target) {
+			SubsOf::<T>::try_mutate(
+				&target,
+				|(current_subs_deposit, subs_of)| -> Result<BalanceOf<T>, DispatchError> {
+					let new_subs_deposit = Self::subs_deposit(subs_of.len() as u32);
+					Self::rejig_deposit(&target, *current_subs_deposit, new_subs_deposit)?;
+					*current_subs_deposit = new_subs_deposit;
+					Ok(new_subs_deposit)
+				},
+			)?
+		} else {
+			// If the item doesn't exist, there is no "old" deposit, and the new one is zero, so no
+			// need to call rejig, it'd just be zero -> zero.
+			Zero::zero()
+		};
 		Ok((new_id_deposit, new_subs_deposit))
 	}
 
diff --git a/substrate/frame/identity/src/tests.rs b/substrate/frame/identity/src/tests.rs
@@ -952,6 +952,57 @@ fn poke_deposit_works() {
 	});
 }
 
+#[test]
+fn poke_deposit_does_not_insert_new_subs_storage() {
+	new_test_ext().execute_with(|| {
+		let [_, _, _, _, ten, _, _, _] = accounts();
+		let ten_info = infoof_ten();
+		// Set a custom registration with 0 deposit
+		IdentityOf::<Test>::insert::<
+			_,
+			(
+				Registration<u64, MaxRegistrars, IdentityInfo<MaxAdditionalFields>>,
+				Option<Username<Test>>,
+			),
+		>(
+			&ten,
+			(
+				Registration {
+					judgements: Default::default(),
+					deposit: Zero::zero(),
+					info: ten_info.clone(),
+				},
+				None::<Username<Test>>,
+			),
+		);
+		assert!(Identity::identity(ten.clone()).is_some());
+
+		// Balance is free
+		assert_eq!(Balances::free_balance(ten.clone()), 1000);
+
+		// poke
+		assert_ok!(Identity::poke_deposit(&ten));
+
+		// free balance reduced correctly
+		let id_deposit = id_deposit(&ten_info);
+		assert_eq!(Balances::free_balance(ten.clone()), 1000 - id_deposit);
+		// new registration deposit is 10
+		assert_eq!(
+			Identity::identity(&ten),
+			Some((
+				Registration {
+					judgements: Default::default(),
+					deposit: id_deposit,
+					info: infoof_ten()
+				},
+				None
+			))
+		);
+		// No new subs storage item.
+		assert!(!SubsOf::<Test>::contains_key(&ten));
+	});
+}
+
 #[test]
 fn adding_and_removing_authorities_should_work() {
 	new_test_ext().execute_with(|| {
