From dc0686a15c64cb041b1db59221271f2a9c968d36 Mon Sep 17 00:00:00 2001 From: Henrique Nogara Date: Tue, 6 Feb 2024 11:27:17 -0300 Subject: [PATCH] Mesh 2046/change ticker rules (#1590) * Allow subset of non alphanumeric chars; Block lower case letters; Add unit test * Add allowed ascii character in a set --------- Co-authored-by: Adam Dossa --- pallets/asset/src/lib.rs | 43 +++++++++++++-------- pallets/runtime/tests/src/asset_test.rs | 50 ++++++++++++++++++++++++- pallets/settlement/src/benchmarking.rs | 8 ++-- 3 files changed, 80 insertions(+), 21 deletions(-) diff --git a/pallets/asset/src/lib.rs b/pallets/asset/src/lib.rs index 6318e937bd..756c81ddeb 100644 --- a/pallets/asset/src/lib.rs +++ b/pallets/asset/src/lib.rs @@ -84,9 +84,6 @@ pub mod checkpoint; #[cfg(feature = "std")] use sp_runtime::{Deserialize, Serialize}; -#[cfg(feature = "runtime-benchmarks")] -use sp_std::collections::btree_set::BTreeSet; - use arrayvec::ArrayVec; use codec::{Decode, Encode}; use core::mem; @@ -99,6 +96,7 @@ use frame_support::{decl_error, decl_module, decl_storage, ensure, fail}; use frame_system::ensure_root; use scale_info::TypeInfo; use sp_runtime::traits::Zero; +use sp_std::collections::btree_set::BTreeSet; use sp_std::{convert::TryFrom, prelude::*}; use pallet_base::{ @@ -1018,6 +1016,8 @@ decl_error! { AssetMetadataValueIsEmpty, /// Number of asset mediators would exceed the maximum allowed. NumberOfAssetMediatorsExceeded, + /// Invalid ticker character - valid set: A`..`Z` `0`..`9` `_` `-` `.` `/`. + InvalidTickerCharacter } } @@ -1188,18 +1188,29 @@ impl Module { } } - /// Ensure `ticker` is fully printable ASCII (SPACE to '~'). - fn ensure_ticker_ascii(ticker: &Ticker) -> DispatchResult { - let bytes = ticker.as_slice(); + /// Returns `Ok` if the ticker contains only the following characters: `A`..`Z` `0`..`9` `_` `-` `.` `/`. + pub fn verify_ticker_characters(ticker: &Ticker) -> DispatchResult { + let ticker_bytes = ticker.as_ref(); + + // The first byte of the ticker cannot be NULL + if *ticker_bytes.first().unwrap_or(&0) == 0 { + return Err(Error::::TickerFirstByteNotValid.into()); + } + + // Allows the following characters: `A`..`Z` `0`..`9` `_` `-` `.` `/` + let valid_characters = BTreeSet::from([b'_', b'-', b'.', b'/']); + for (byte_index, ticker_byte) in ticker_bytes.iter().enumerate() { + if !ticker_byte.is_ascii_uppercase() + && !ticker_byte.is_ascii_digit() + && !valid_characters.contains(ticker_byte) + { + if ticker_bytes[byte_index..].iter().all(|byte| *byte == 0) { + return Ok(()); + } - ensure!(bytes[0] != 0, Error::::TickerFirstByteNotValid); - // Find first byte not alphanumeric. - let good = bytes - .iter() - .position(|b| !(*b).is_ascii_alphanumeric()) - // Everything after must be a NULL byte. - .map_or(true, |nm_pos| bytes[nm_pos..].iter().all(|b| *b == 0)); - ensure!(good, Error::::TickerNotAlphanumeric); + return Err(Error::::InvalidTickerCharacter.into()); + } + } Ok(()) } @@ -1210,7 +1221,7 @@ impl Module { no_re_register: bool, config: impl FnOnce() -> TickerRegistrationConfig, ) -> Result, DispatchError> { - Self::ensure_ticker_ascii(&ticker)?; + Self::verify_ticker_characters(&ticker)?; Self::ensure_asset_fresh(&ticker)?; let config = config(); @@ -1709,7 +1720,7 @@ impl Module { // If `ticker` isn't registered, it will be, so ensure it is fully ascii. if available { - Self::ensure_ticker_ascii(&ticker)?; + Self::verify_ticker_characters(&ticker)?; } let token_did = Identity::::get_token_did(&ticker)?; diff --git a/pallets/runtime/tests/src/asset_test.rs b/pallets/runtime/tests/src/asset_test.rs index 19b2fb61e6..809ddd0031 100644 --- a/pallets/runtime/tests/src/asset_test.rs +++ b/pallets/runtime/tests/src/asset_test.rs @@ -36,6 +36,7 @@ use polymesh_primitives::asset_metadata::{ }; use polymesh_primitives::calendar::{CalendarPeriod, CalendarUnit, FixedOrVariableCalendarUnit}; use polymesh_primitives::statistics::StatType; +use polymesh_primitives::ticker::TICKER_LEN; use polymesh_primitives::{ AccountId, AssetIdentifier, AssetPermissions, AuthorizationData, AuthorizationError, Document, DocumentId, Fund, FundDescription, IdentityId, Memo, Moment, NFTCollectionKeys, Permissions, @@ -522,7 +523,7 @@ fn register_ticker() { ] { assert_noop!( register(Ticker::from_slice_truncated(&bs[..])), - AssetError::TickerNotAlphanumeric + AssetError::InvalidTickerCharacter ); } }) @@ -2693,3 +2694,50 @@ fn successfully_remove_mediators() { } }); } + +#[test] +fn verify_ticker_characters() { + let mut all_valid_characters = Vec::new(); + let mut valid_ascii_digits: Vec = (48..58).collect(); + let mut valid_ascii_letters: Vec = (65..91).collect(); + let mut valid_special_characters: Vec = Vec::from([b'-', b'.', b'/', b'_']); + all_valid_characters.append(&mut valid_ascii_digits); + all_valid_characters.append(&mut valid_ascii_letters); + all_valid_characters.append(&mut valid_special_characters); + + let mut rng = rand::thread_rng(); + + // Generates 10 random valid tickers + for _ in 0..10 { + let valid_ticker: Vec = (0..TICKER_LEN + 1) + .map(|_| all_valid_characters[rng.gen_range(0, all_valid_characters.len())]) + .collect(); + assert_ok!(Asset::verify_ticker_characters( + &Ticker::from_slice_truncated(&valid_ticker) + )); + } + + let valid_set: BTreeSet<&u8> = all_valid_characters.iter().collect(); + let mut all_invalid_characters: Vec = (0..=255).collect(); + all_invalid_characters.retain(|ascii_code| !valid_set.contains(ascii_code)); + + // Generates 10 random invalid tickers + for _ in 0..10 { + let mut invalid_ticker: Vec = (0..TICKER_LEN - 1) + .map(|_| all_valid_characters[rng.gen_range(0, all_valid_characters.len())]) + .collect(); + invalid_ticker.push(all_invalid_characters[rng.gen_range(0, all_invalid_characters.len())]); + + assert_eq!( + Asset::verify_ticker_characters(&Ticker::from_slice_truncated(&invalid_ticker)) + .unwrap_err(), + AssetError::InvalidTickerCharacter.into() + ); + } + + assert_eq!( + Asset::verify_ticker_characters(&Ticker::from_slice_truncated(&[0; TICKER_LEN])) + .unwrap_err(), + AssetError::TickerFirstByteNotValid.into() + ); +} diff --git a/pallets/settlement/src/benchmarking.rs b/pallets/settlement/src/benchmarking.rs index 803eb8acf9..2529010fee 100644 --- a/pallets/settlement/src/benchmarking.rs +++ b/pallets/settlement/src/benchmarking.rs @@ -108,7 +108,7 @@ where // Creates offchain legs and new portfolios for each leg let offchain_legs: Vec = (0..o) .map(|i| { - let ticker = Ticker::from_slice_truncated(format!("OFFTicker{}", i).as_bytes()); + let ticker = Ticker::from_slice_truncated(format!("OFFTICKER{}", i).as_bytes()); Leg::OffChain { sender_identity: sender.did(), receiver_identity: receiver.did(), @@ -121,7 +121,7 @@ where // Creates f assets, creates two portfolios, adds maximum compliance requirements, adds maximum transfer conditions and pauses them let fungible_legs: Vec = (0..f) .map(|i| { - let ticker = Ticker::from_slice_truncated(format!("Ticker{}", i).as_bytes()); + let ticker = Ticker::from_slice_truncated(format!("TICKER{}", i).as_bytes()); let sdr_portfolio_name = format!("SdrPortfolioTicker{}", i); let rcv_portfolio_name = format!("RcvPortfolioTicker{}", i); let (sdr_portfolio, rvc_portfolio, mut mediators) = setup_asset_transfer( @@ -149,7 +149,7 @@ where // Creates n collections, mints one NFT, creates two portfolios, adds maximum compliance requirements and pauses it let nft_legs: Vec = (0..n) .map(|i| { - let ticker = Ticker::from_slice_truncated(format!("NFTTicker{}", i).as_bytes()); + let ticker = Ticker::from_slice_truncated(format!("NFTTICKER{}", i).as_bytes()); let sdr_portfolio_name = format!("SdrPortfolioNFTTicker{}", i); let rcv_portfolio_name = format!("RcvPortfolioNFTTicker{}", i); let (sdr_portfolio, rcv_portfolio, mut mediators) = setup_nft_transfer( @@ -282,7 +282,7 @@ fn setup_receipt_details( instruction_id: InstructionId, leg_id: u32, ) -> ReceiptDetails { - let ticker = Ticker::from_slice_truncated(format!("OFFTicker{}", leg_id).as_bytes()); + let ticker = Ticker::from_slice_truncated(format!("OFFTICKER{}", leg_id).as_bytes()); let receipt = Receipt::new( leg_id as u64, instruction_id,