Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: change user handle #1647

Merged
merged 29 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
5fbcfc2
feat: Implemented change handle
rlaferla Jul 25, 2023
e17895d
feat: Added new argument for unit test
rlaferla Jul 26, 2023
1a4c520
fix: Fixed lint issue
rlaferla Jul 26, 2023
4ed30ac
Merge branch 'main' into 1627-user-handles-handle-swapping
rlaferla Jul 26, 2023
6a685aa
refactor: Now changes to handles happens in a separate extrinsic
rlaferla Jul 27, 2023
65a69c3
feat: Added scaffolding for change handle unit tests
rlaferla Jul 27, 2023
4f1c642
Merge branch 'main' into 1627-user-handles-handle-swapping
rlaferla Jul 27, 2023
201452c
fix: revised implementation (wip)
rlaferla Jul 28, 2023
0bd099b
fix: revised and working benchmark
rlaferla Jul 28, 2023
6959420
fix: added check for event in unit test
rlaferla Jul 28, 2023
bdd8daf
feat: Added unhappy path to unit tests
rlaferla Jul 28, 2023
87b5ca6
Merge branch 'main' into 1627-user-handles-handle-swapping
rlaferla Jul 28, 2023
acd7228
chore: add preliminary weights for change_handle()
rlaferla Jul 28, 2023
16d875d
Merge branch '1627-user-handles-handle-swapping' of https://github.co…
rlaferla Jul 28, 2023
5ef604a
fix: fixed formatting for unit tests
rlaferla Jul 28, 2023
644f1c5
fix: fixed temporary weight signature (will be replaced when benchmar…
rlaferla Jul 28, 2023
15dd546
fix: Added change_handle() to Capacity payable extrinsics.
rlaferla Jul 28, 2023
f1abc83
Run benchmarks [run-benchmarks handles]
rlaferla Jul 28, 2023
51a0161
Update weights for PR #1647
Jul 28, 2023
ae69257
refactor: consolidated validation of handle length
rlaferla Jul 31, 2023
1f0e94a
fix: Added check for HandleRetired event.
rlaferla Jul 31, 2023
a10510b
fix: Fixed formatting
rlaferla Jul 31, 2023
00a0cca
fix: Fix for capacity stable weights
rlaferla Jul 31, 2023
a5efaab
fix: Added unit test to validate max handle length
rlaferla Jul 31, 2023
08ebd49
fix: Improved unit test for max byte length user handle
rlaferla Jul 31, 2023
71aeaa4
refactor: Refactored and improved handle byte length unit tests
rlaferla Jul 31, 2023
b8d04f1
Update pallets/handles/src/lib.rs
rlaferla Aug 1, 2023
a0d9159
Merge branch 'main' into 1627-user-handles-handle-swapping
rlaferla Aug 1, 2023
25f2102
fix: Increment spec_version
rlaferla Aug 1, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integration-tests/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions pallets/frequency-tx-payment/src/capacity_stable_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
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`
Expand Down Expand Up @@ -215,4 +216,16 @@
.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))
}

Check warning on line 230 in pallets/frequency-tx-payment/src/capacity_stable_weights.rs

View check run for this annotation

Codecov / codecov/patch

pallets/frequency-tx-payment/src/capacity_stable_weights.rs#L224-L230

Added lines #L224 - L230 were not covered by tests
}
19 changes: 18 additions & 1 deletion pallets/handles/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
90 changes: 83 additions & 7 deletions pallets/handles/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@

#[frame_support::pallet]
pub mod pallet {

Check warning on line 90 in pallets/handles/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/handles/src/lib.rs#L90

Added line #L90 was not covered by tests
use super::*;
#[pallet::config]
pub trait Config: frame_system::Config {
Expand Down Expand Up @@ -297,6 +298,22 @@
Ok(())
}

/// Verifies max user handle size in bytes to address potential panic condition
///
/// # Arguments
///
/// * `handle` - The user handle.
///
/// # Errors
/// * [`Error::InvalidHandleByteLength`]

Check warning on line 308 in pallets/handles/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/handles/src/lib.rs#L301-L308

Added lines #L301 - L308 were not covered by tests
pub fn verify_max_handle_byte_length(handle: Vec<u8>) -> DispatchResult {
ensure!(
handle.len() as u32 <= HANDLE_BASE_BYTES_MAX,
Error::<T>::InvalidHandleByteLength
);

Check warning on line 313 in pallets/handles/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/handles/src/lib.rs#L313

Added line #L313 was not covered by tests
Ok(())
}

Check warning on line 316 in pallets/handles/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/handles/src/lib.rs#L316

Added line #L316 was not covered by tests
/// 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
Expand Down Expand Up @@ -345,7 +362,7 @@
/// * `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
///
Expand All @@ -359,7 +376,6 @@
/// # Events
/// * [`Event::HandleClaimed`]
///

#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::claim_handle(payload.base_handle.len() as u32))]
pub fn claim_handle(
Expand All @@ -371,10 +387,7 @@
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)
Expand Down Expand Up @@ -417,11 +430,74 @@
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 have valid MSA keys,
rlaferla marked this conversation as resolved.
Show resolved Hide resolved
/// 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)?;
rlaferla marked this conversation as resolved.
Show resolved Hide resolved
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> {
Expand Down
62 changes: 62 additions & 0 deletions pallets/handles/src/tests/handle_change_tests.rs
Original file line number Diff line number Diff line change
@@ -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());
rlaferla marked this conversation as resolved.
Show resolved Hide resolved
});
}

#[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
);
});
}
rlaferla marked this conversation as resolved.
Show resolved Hide resolved
55 changes: 22 additions & 33 deletions pallets/handles/src/tests/handle_creation_tests.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
);
});
}
33 changes: 32 additions & 1 deletion pallets/handles/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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>,
Expand All @@ -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()
}
1 change: 1 addition & 0 deletions pallets/handles/src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod mock;

mod handle_change_tests;
mod handle_creation_tests;
mod handle_retirements_tests;
mod handles_replay_attack_test;
Expand Down
Loading
Loading