feat: change user handle (#1647)

pallets/frequency-tx-payment/src/capacity_stable_weights.rs

@@ -72,6 +72,7 @@ pub trait WeightInfo {
 	fn delete_page_with_signature() -> Weight;
 	// Handles
 	fn claim_handle(b: u32) -> Weight;
+	fn change_handle(b: u32) -> Weight;
 }
 
 // Update test as well to ensure static weight values `tests/stable_weights_test.rs`
@@ -215,4 +216,16 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 			.saturating_add(T::DbWeight::get().reads(3 as u64))
 			.saturating_add(T::DbWeight::get().writes(3 as u64))
 	}
+	/// Storage: Msa PublicKeyToMsaId (r:1 w:0)
+	/// Storage: Handles MSAIdToDisplayName (r:1 w:1)
+	/// Storage: Handles CanonicalBaseHandleToSuffixIndex (r:1 w:1)
+	/// Storage: Handles CanonicalBaseHandleAndSuffixToMSAId (r:0 w:2)
+	/// The range of component `b` is `[3, 30]`.
+	fn change_handle(b: u32, ) -> Weight {
+		Weight::from_parts(96_221_224, 12434)
+			// Standard Error: 9_682
+			.saturating_add(Weight::from_parts(193_495, 0).saturating_mul(b.into()))
+			.saturating_add(T::DbWeight::get().reads(3_u64))
+			.saturating_add(T::DbWeight::get().writes(4_u64))
+	}
 }

pallets/handles/src/benchmarking.rs

@@ -55,6 +55,7 @@ fn create_signed_claims_payload<T: Config>(
 }
 
 benchmarks! {
+
 	claim_handle {
 		// claim a handle
 		let b in HANDLE_BASE_BYTES_MIN .. HANDLE_BASE_BYTES_MAX-2;
@@ -70,8 +71,24 @@ benchmarks! {
 		assert!(stored_handle.is_some());
 	}
 
+	change_handle {
+		// claim a handle to be changed
+		let b in HANDLE_BASE_BYTES_MIN .. HANDLE_BASE_BYTES_MAX-2;
+		let caller: T::AccountId = whitelisted_caller();
+		let delegator_account_public = SignerId::generate_pair(None);
+		let (payload, proof, key,delegator_msa_id) = create_signed_claims_payload::<T>(delegator_account_public.clone(), b);
+		assert_ok!(T::MsaBenchmarkHelper::add_key(delegator_msa_id.into(), caller.clone()));
+		assert_ok!(T::MsaBenchmarkHelper::add_key(delegator_msa_id.into(), key.clone()));
+		assert_ok!(Handles::<T>::claim_handle(RawOrigin::Signed(caller.clone()).into(), key.clone(), proof.clone(), payload.clone()));
+
+	}: _(RawOrigin::Signed(caller.clone()), key.clone(), proof, payload)
+	verify {
+		let stored_handle = Handles::<T>::get_handle_for_msa(delegator_msa_id.into());
+		assert!(stored_handle.is_some());
+	}
+
 	retire_handle {
-		// claim a handle
+		// claim a handle to be retired
 		let caller: T::AccountId = whitelisted_caller();
 		let delegator_account_public = SignerId::generate_pair(None);
 		let (payload, proof, key,delegator_msa_id) = create_signed_claims_payload::<T>(delegator_account_public.clone(), 32);

pallets/handles/src/lib.rs

@@ -87,6 +87,7 @@ impl<T: Config> HandleProvider for Pallet<T> {
 
 #[frame_support::pallet]
 pub mod pallet {
+
 	use super::*;
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
@@ -297,6 +298,22 @@ pub mod pallet {
 			Ok(())
 		}
 
+		/// Verifies max user handle size in bytes to address potential panic condition
+		///
+		/// # Arguments
+		///
+		/// * `handle` - The user handle.
+		///
+		/// # Errors
+		/// * [`Error::InvalidHandleByteLength`]
+		pub fn verify_max_handle_byte_length(handle: Vec<u8>) -> DispatchResult {
+			ensure!(
+				handle.len() as u32 <= HANDLE_BASE_BYTES_MAX,
+				Error::<T>::InvalidHandleByteLength
+			);
+			Ok(())
+		}
+
 		/// The furthest in the future a mortality_block value is allowed
 		/// to be for current_block
 		/// This is calculated to be past the risk of a replay attack
@@ -345,7 +362,7 @@ pub mod pallet {
 		/// * `origin` - The origin of the caller.
 		/// * `msa_owner_key` - The public key of the MSA owner.
 		/// * `proof` - The multi-signature proof for the payload.
-		/// * `payload` - The payload containing the information needed to claim the handle.
+		/// * `payload` - The payload containing the information needed to claim a new handle.
 		///
 		/// # Errors
 		///
@@ -359,7 +376,6 @@ pub mod pallet {
 		/// # Events
 		/// * [`Event::HandleClaimed`]
 		///
-
 		#[pallet::call_index(0)]
 		#[pallet::weight(T::WeightInfo::claim_handle(payload.base_handle.len() as u32))]
 		pub fn claim_handle(
@@ -371,10 +387,7 @@ pub mod pallet {
 			let _ = ensure_signed(origin)?;
 
 			// Validation: Check for base_handle size to address potential panic condition
-			ensure!(
-				payload.base_handle.len() as u32 <= HANDLE_BASE_BYTES_MAX,
-				Error::<T>::InvalidHandleByteLength
-			);
+			Self::verify_max_handle_byte_length(payload.base_handle.clone())?;
 
 			// Validation: caller must already have a MSA id
 			let msa_id = T::MsaInfoProvider::ensure_valid_msa_key(&msa_owner_key)
@@ -417,11 +430,74 @@ pub mod pallet {
 			let msa_id = T::MsaInfoProvider::ensure_valid_msa_key(&msa_owner_key)
 				.map_err(|_| Error::<T>::InvalidMessageSourceAccount)?;
 
-			let display_handle = Self::do_retire_handle(msa_id)?;
+			let display_handle: Vec<u8> = Self::do_retire_handle(msa_id)?;
 
 			Self::deposit_event(Event::HandleRetired { msa_id, handle: display_handle });
 			Ok(())
 		}
+
+		/// Changes the handle for a caller's MSA (Message Source Account) based on the provided payload.
+		/// This function performs several validations before claiming the handle, including checking
+		/// the size of the base_handle, ensuring the caller has valid MSA keys,
+		/// verifying the payload signature, and finally calling the internal `do_retire_handle` and `do_claim_handle` functions
+		/// to retire the current handle and claim the new one.
+		///
+		/// # Arguments
+		///
+		/// * `origin` - The origin of the caller.
+		/// * `msa_owner_key` - The public key of the MSA owner.
+		/// * `proof` - The multi-signature proof for the payload.
+		/// * `payload` - The payload containing the information needed to change an existing handle.
+		///
+		/// # Errors
+		///
+		/// This function may return an error as part of `DispatchResult` if any of the following
+		/// validations fail:
+		///
+		/// * [`Error::InvalidHandleByteLength`] - The base_handle size exceeds the maximum allowed size.
+		/// * [`Error::InvalidMessageSourceAccount`] - The caller does not have a valid  `MessageSourceId`.
+		/// * [`Error::InvalidSignature`] - The payload signature verification fails.
+		///
+		/// # Events
+		/// * [`Event::HandleRetired`]
+		/// * [`Event::HandleClaimed`]
+		///
+		#[pallet::call_index(2)]
+		#[pallet::weight(T::WeightInfo::change_handle(payload.base_handle.len() as u32))]
+		pub fn change_handle(
+			origin: OriginFor<T>,
+			msa_owner_key: T::AccountId,
+			proof: MultiSignature,
+			payload: ClaimHandlePayload<T::BlockNumber>,
+		) -> DispatchResult {
+			let _ = ensure_signed(origin)?;
+
+			// Validation: Check for base_handle size to address potential panic condition
+			Self::verify_max_handle_byte_length(payload.base_handle.clone())?;
+
+			// Validation: caller must already have a MSA id
+			let msa_id = T::MsaInfoProvider::ensure_valid_msa_key(&msa_owner_key)
+				.map_err(|_| Error::<T>::InvalidMessageSourceAccount)?;
+
+			// Validation: The signature is within the mortality window
+			Self::verify_signature_mortality(payload.expiration.into())?;
+
+			// Validation: Verify the payload was signed
+			Self::verify_signed_payload(&proof, &msa_owner_key, payload.encode())?;
+
+			// Get existing handle to retire
+			Self::get_display_name_for_msa_id(msa_id).ok_or(Error::<T>::MSAHandleDoesNotExist)?;
+
+			// Retire old handle
+			let old_display_handle: Vec<u8> = Self::do_retire_handle(msa_id)?;
+			Self::deposit_event(Event::HandleRetired { msa_id, handle: old_display_handle });
+
+			let display_handle = Self::do_claim_handle(msa_id, payload.clone())?;
+
+			Self::deposit_event(Event::HandleClaimed { msa_id, handle: display_handle.clone() });
+
+			Ok(())
+		}
 	}
 
 	impl<T: Config> Pallet<T> {

pallets/handles/src/tests/handle_change_tests.rs

@@ -0,0 +1,62 @@
+use crate::{tests::mock::*, Error, Event};
+use codec::Decode;
+use common_primitives::msa::MessageSourceId;
+use frame_support::{assert_err, assert_ok};
+use sp_core::{sr25519, Encode, Pair};
+
+#[test]
+fn change_handle_happy_path() {
+	new_test_ext().execute_with(|| {
+		let handle_str = "MyHandle";
+		let handle = handle_str.as_bytes().to_vec();
+		let alice = sr25519::Pair::from_seed(&[0; 32]);
+		let expiry = 100;
+		let (payload, proof) = get_signed_claims_payload(&alice, handle.clone(), expiry);
+		assert_ok!(Handles::claim_handle(
+			RuntimeOrigin::signed(alice.public().into()),
+			alice.public().into(),
+			proof,
+			payload
+		));
+
+		// Confirm that HandleClaimed event was deposited
+		let msa_id = MessageSourceId::decode(&mut &alice.public().encode()[..]).unwrap();
+		let full_handle = create_full_handle_for_index(handle_str, 0);
+		System::assert_last_event(
+			Event::HandleClaimed { msa_id, handle: full_handle.clone() }.into(),
+		);
+
+		let new_handle = "MyNewHandle";
+		let (payload, proof) =
+			get_signed_claims_payload(&alice, new_handle.as_bytes().to_vec(), expiry);
+		assert_ok!(Handles::change_handle(
+			RuntimeOrigin::signed(alice.public().into()),
+			alice.public().into(),
+			proof,
+			payload
+		));
+		let changed_handle = create_full_handle_for_index(new_handle, 0);
+		System::assert_has_event(Event::HandleRetired { msa_id, handle: full_handle }.into());
+		System::assert_last_event(Event::HandleClaimed { msa_id, handle: changed_handle }.into());
+	});
+}
+
+#[test]
+fn change_handle_no_handle() {
+	new_test_ext().execute_with(|| {
+		let handle = "MyNewHandleWithoutAnOldOne";
+		let alice = sr25519::Pair::from_seed(&[0; 32]);
+		let expiry = 100;
+		let (payload, proof) =
+			get_signed_claims_payload(&alice, handle.as_bytes().to_vec(), expiry);
+		assert_err!(
+			Handles::change_handle(
+				RuntimeOrigin::signed(alice.public().into()),
+				alice.public().into(),
+				proof,
+				payload
+			),
+			Error::<Test>::MSAHandleDoesNotExist
+		);
+	});
+}

pallets/handles/src/tests/handle_creation_tests.rs

@@ -1,41 +1,10 @@
 use crate::{tests::mock::*, Error, Event};
 use codec::Decode;
-use common_primitives::{handles::SequenceIndex, msa::MessageSourceId};
-use frame_support::{assert_noop, assert_ok, dispatch::DispatchResult};
-use handles_utils::converter::convert_to_canonical;
+use common_primitives::{handles::HANDLE_BASE_BYTES_MAX, msa::MessageSourceId};
+use frame_support::{assert_err, assert_noop, assert_ok, dispatch::DispatchResult};
 use sp_core::{sr25519, Encode, Pair};
 use sp_std::collections::btree_set::BTreeSet;
 
-/// Creates a full display handle by combining a base handle string with a suffix generated
-/// from an index into the suffix sequence.
-///
-/// # Arguments
-///
-/// * `base_handle_str` - The base handle string.
-/// * `suffix_sequence_index` - The index into the suffix sequence.
-///
-/// # Returns
-///
-/// * `DisplayHandle` - The full display handle.
-///
-fn create_full_handle_for_index(
-	base_handle_str: &str,
-	suffix_sequence_index: SequenceIndex,
-) -> Vec<u8> {
-	// Convert base handle into a canonical base
-	let canonical_handle_str = convert_to_canonical(&base_handle_str);
-
-	// Generate suffix from index into the suffix sequence
-	let suffix = Handles::generate_suffix_for_canonical_handle(
-		&canonical_handle_str,
-		suffix_sequence_index as usize,
-	)
-	.unwrap_or_default();
-
-	let display_handle = Handles::build_full_display_handle(base_handle_str, suffix).unwrap();
-	display_handle.into_inner()
-}
-
 struct TestCase<T> {
 	handle: &'static str,
 	expected: T,
@@ -368,3 +337,23 @@ fn claim_handle_with_max_bytes_should_get_correct_display_handle() {
 		assert_eq!(msa_id_from_state.unwrap(), msa_id);
 	});
 }
+
+#[test]
+fn test_verify_handle_length() {
+	new_test_ext().execute_with(|| {
+		// Max bytes handle is ok
+		let handle_str: String =
+			std::iter::repeat('*').take((HANDLE_BASE_BYTES_MAX) as usize).collect();
+		let handle = handle_str.as_bytes().to_vec();
+		assert_ok!(Handles::verify_max_handle_byte_length(handle));
+
+		// However, max bytes handle plus 1 is not ok
+		let handle_str: String =
+			std::iter::repeat('*').take((HANDLE_BASE_BYTES_MAX + 1) as usize).collect();
+		let handle = handle_str.as_bytes().to_vec();
+		assert_err!(
+			Handles::verify_max_handle_byte_length(handle),
+			Error::<Test>::InvalidHandleByteLength
+		);
+	});
+}

pallets/handles/src/tests/mock.rs

@@ -18,6 +18,7 @@ use sp_runtime::{
 	MultiSignature,
 };
 
+use handles_utils::converter::convert_to_canonical;
 pub const INVALID_MSA_ID: MessageSourceId = 100;
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
@@ -151,7 +152,7 @@ pub fn run_to_block(n: u32) {
 	}
 }
 
-// Create a signed claims payload
+// Create a signed claim handle payload
 pub fn get_signed_claims_payload(
 	account: &sr25519::Pair,
 	handle: Vec<u8>,
@@ -164,3 +165,33 @@ pub fn get_signed_claims_payload(
 
 	(payload, proof)
 }
+
+/// Creates a full display handle by combining a base handle string with a suffix generated
+/// from an index into the suffix sequence.
+///
+/// # Arguments
+///
+/// * `base_handle_str` - The base handle string.
+/// * `suffix_sequence_index` - The index into the suffix sequence.
+///
+/// # Returns
+///
+/// * `DisplayHandle` - The full display handle.
+///
+pub fn create_full_handle_for_index(
+	base_handle_str: &str,
+	suffix_sequence_index: SequenceIndex,
+) -> Vec<u8> {
+	// Convert base handle into a canonical base
+	let canonical_handle_str = convert_to_canonical(&base_handle_str);
+
+	// Generate suffix from index into the suffix sequence
+	let suffix = Handles::generate_suffix_for_canonical_handle(
+		&canonical_handle_str,
+		suffix_sequence_index as usize,
+	)
+	.unwrap_or_default();
+
+	let display_handle = Handles::build_full_display_handle(base_handle_str, suffix).unwrap();
+	display_handle.into_inner()
+}

pallets/handles/src/tests/mod.rs

@@ -1,5 +1,6 @@
 pub mod mock;
 
+mod handle_change_tests;
 mod handle_creation_tests;
 mod handle_retirements_tests;
 mod handles_replay_attack_test;