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

Display handles are whitespace trimed and collapsed #2157

Merged
merged 5 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions designdocs/user-handles.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ The high level requirements for user handles are:
* **Suffix**: A suffix is a numeric value that is appended to a base handle to make it unique. The suffix is randomly constrained and selected within a range configured on chain.
* **Delimiter**: A delimiter is a character that is used to separate the base handle and suffix. The delimiter is a period (.) by default.
* **Display Handle**: A display handle is formed by concatenating the base handle and suffix with the delimiter. The display handle is the handle that is displayed to the user.

### Handle Guidelines

* User handles must be unique and each MSA can only be mapped to one handle.
Expand All @@ -48,6 +48,7 @@ The high level requirements for user handles are:
* Suffix will be randomly constrained.
* Homoglyph versions of handles should still resolve to the same ```msa_id``` (e.g. ```user.1234``` and ```u$er.1234``` should resolve to the same ```msa_id```).
* `Display Handle` is the handle that is displayed to the user and must be unique.
* `Base Handle` should have UTF-8 whitespace trimmed and concatenated (e.g. ``` u s \t\t e\tr ``` will be trimmed to ```u s e r```).

## Proposal

Expand All @@ -66,7 +67,7 @@ Handles```registry``` on Frequency chain.

### Handle replay attack

To prevent replay attacks, the chain will require a mortality period for handles. This can be included in signed payload from user as a `expiration` field. The chain will check if the `expiration` is greater than the current block number. If not, the transaction will fail.
To prevent replay attacks, the chain will require a mortality period for handles. This can be included in the user's signed payload as an `expiration` field. The chain will check if the `expiration` is greater than the current block number. If not, the transaction will fail.

### Handling Race Conditions

Expand Down Expand Up @@ -135,7 +136,7 @@ ClaimHandlePayload {

### Claim handle

As a network, Frequency should allow users to choose their own handle, while the chain will generate a random numeric suffix within the range of suffixes allowed. The display handle will be the base handle with the suffix.
As a network, Frequency should allow users to choose their own handle, while the chain will generate a random numeric suffix within the range of suffixes allowed. The display handle will be the base handle with whitespace trimmed and consolidated, concatenated with the suffix.

Input

Expand Down
3 changes: 1 addition & 2 deletions pallets/handles/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ Provides MSAs with an optional, but unique handle.
A handle consists of:
- **Base Handle:** The user's chosen handle. It is *not* guaranteed to be unique without the suffix. It is linked to a normalized version for Handle to MSA Id resolution. See [UTF-8 Support](#utf-8-support) and [Homoglyph Attack Resistence](#homoglyph-attack-resistence) below.
- **Suffix:** A suffix is a unique numeric value appended to a user's base handle to make it unique.
- **Display Handle:** The user's original (un-normalized) base handle string and the suffix together (`base`.`suffix`) constitute a unique identifier for a user.

- **Display Handle:** The user's original (un-normalized, but with whitespace trimmed and consolidated) base handle string and the suffix together (`base`.`suffix`) constitute a unique identifier for a user.

### UTF-8 Support

Expand Down
20 changes: 19 additions & 1 deletion pallets/handles/src/handles-utils/src/converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub fn split_display_name(display_name_str: &str) -> Option<(String, HandleSuffi
let parts: Vec<&str> = display_name_str.split(HANDLE_DELIMITER).collect();
let base_handle_str = parts[0].to_string();
if parts.len() != 2 {
return None
return None;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My formatter really wants to have ; at the end of the line when it is return

}

let suffix = parts[1];
Expand All @@ -108,3 +108,21 @@ pub fn strip_unicode_whitespace(input_str: &str) -> String {
.filter(|character| !character.is_whitespace())
.collect::<alloc::string::String>()
}

/// Trims whitespace from the head and tail and collapses all other whitespace to just a single space
///
/// # Arguments
///
/// * `input_str` - A string slice that holds the input string from which the whitespace characters need to be trimmed and collapsed
///
/// # Returns
///
/// A new string without any Unicode whitespace characters.
pub fn trim_and_collapse_whitespace(input_str: &str) -> String {
// Benchmarked as slightly faster than https://crates.io/crates/collapse
input_str
.split_whitespace()
.filter(|s| !s.is_empty())
.collect::<Vec<_>>()
.join(" ")
}
11 changes: 11 additions & 0 deletions pallets/handles/src/handles-utils/src/tests/converter_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::{
convert_to_canonical,
converter::{
replace_confusables, split_display_name, strip_diacriticals, strip_unicode_whitespace,
trim_and_collapse_whitespace,
},
};

Expand Down Expand Up @@ -144,3 +145,13 @@ fn test_split_display_name_failure() {
assert_eq!(split_display_name("hello.65536"), None);
assert_eq!(split_display_name("hello.999999999"), None);
}

#[test]
fn test_trim_and_collapse_whitespace() {
assert_eq!(trim_and_collapse_whitespace(" h e llo "), "h e llo");
assert_eq!(trim_and_collapse_whitespace(" h e l lo "), "h e l lo");
assert_eq!(
trim_and_collapse_whitespace("\u{3000}h\u{2000}e\u{000D}l\u{2002}l\u{000C}o\u{0009}"),
"h e l l o"
);
}
5 changes: 4 additions & 1 deletion pallets/handles/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ impl<T: Config> HandleProvider for Pallet<T> {
#[frame_support::pallet]
pub mod pallet {

use handles_utils::trim_and_collapse_whitespace;

use super::*;
#[pallet::config]
pub trait Config: frame_system::Config {
Expand Down Expand Up @@ -711,8 +713,9 @@ pub mod pallet {
base_handle: &str,
suffix: HandleSuffix,
) -> Result<DisplayHandle, DispatchError> {
let base_handle_trimmed = trim_and_collapse_whitespace(base_handle);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Believe it or not, this appears to be the place that builds the display handle that is saved.

let mut full_handle_vec: Vec<u8> = vec![];
full_handle_vec.extend(base_handle.as_bytes());
full_handle_vec.extend(base_handle_trimmed.as_bytes());
full_handle_vec.push(HANDLE_DELIMITER as u8); // The delimiter
let mut buff = [0u8; SUFFIX_MAX_DIGITS];
full_handle_vec.extend(suffix.numtoa(10, &mut buff));
Expand Down
29 changes: 29 additions & 0 deletions pallets/handles/src/tests/handle_creation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,35 @@ fn claim_handle_with_max_bytes_should_get_correct_display_handle() {
});
}

#[test]
fn claim_handle_with_various_spaces_should_get_correct_display_handle() {
new_test_ext().execute_with(|| {
let alice = sr25519::Pair::from_seed(&[0; 32]);
let expiration = 100;
let handle = "\u{2000}\u{3000}\u{2000}w\u{2000}h\u{000D}itesp\u{2002}a\u{000C}ce\u{0009}\u{2002}\u{0009}";
let (payload, proof) =
get_signed_claims_payload(&alice, handle.as_bytes().to_vec(), expiration);
assert_ok!(Handles::claim_handle(
RuntimeOrigin::signed(alice.public().into()),
alice.public().into(),
proof,
payload.clone()
));
let msa_id = MessageSourceId::decode(&mut &alice.public().encode()[..]).unwrap();
let handle = Handles::get_handle_for_msa(msa_id);
assert!(handle.is_some());
let handle_result = handle.unwrap();
assert_eq!(handle_result.base_handle, "w h itesp a ce".as_bytes().to_vec());
assert!(handle_result.suffix > 0);
let display_handle = "whitespace.".to_owned() + &handle_result.suffix.to_string();
let display_handle_vec = display_handle.as_bytes().to_vec();
let msa_id_from_state =
Handles::get_msa_id_for_handle(display_handle_vec.try_into().unwrap());
assert!(msa_id_from_state.is_some());
assert_eq!(msa_id_from_state.unwrap(), msa_id);
});
}

#[test]
fn test_verify_handle_length() {
new_test_ext().execute_with(|| {
Expand Down
8 changes: 4 additions & 4 deletions runtime/frequency/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("frequency"),
impl_name: create_runtime_str!("frequency"),
authoring_version: 1,
spec_version: 110,
spec_version: 111,
impl_version: 0,
apis: apis::RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand All @@ -389,7 +389,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("frequency-testnet"),
impl_name: create_runtime_str!("frequency"),
authoring_version: 1,
spec_version: 110,
spec_version: 111,
impl_version: 0,
apis: apis::RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down Expand Up @@ -1483,7 +1483,7 @@ impl StaleHashCheckExtension {

return ValidTransaction::with_tag_prefix(TAG_PREFIX)
.and_provides((msa_id, schema_id))
.build()
.build();
}
Ok(Default::default())
}
Expand All @@ -1510,7 +1510,7 @@ impl StaleHashCheckExtension {

return ValidTransaction::with_tag_prefix(TAG_PREFIX)
.and_provides((msa_id, schema_id, page_id))
.build()
.build();
}

Ok(Default::default())
Expand Down
Loading