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 <adam.dossa@gmail.com>

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<T: Config> Module<T> {
         }
     }
 
-    /// 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::<T>::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::<T>::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::<T>::TickerNotAlphanumeric);
+                return Err(Error::<T>::InvalidTickerCharacter.into());
+            }
+        }
         Ok(())
     }
 
@@ -1210,7 +1221,7 @@ impl<T: Config> Module<T> {
         no_re_register: bool,
         config: impl FnOnce() -> TickerRegistrationConfig<T::Moment>,
     ) -> Result<Option<T::Moment>, DispatchError> {
-        Self::ensure_ticker_ascii(&ticker)?;
+        Self::verify_ticker_characters(&ticker)?;
         Self::ensure_asset_fresh(&ticker)?;
 
         let config = config();
@@ -1709,7 +1720,7 @@ impl<T: Config> Module<T> {
 
         // 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::<T>::get_token_did(&ticker)?;

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<u8> = (48..58).collect();
+    let mut valid_ascii_letters: Vec<u8> = (65..91).collect();
+    let mut valid_special_characters: Vec<u8> = 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<u8> = (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<u8> = (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<u8> = (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()
+    );
+}

pallets/settlement/src/benchmarking.rs

@@ -108,7 +108,7 @@ where
     // Creates offchain legs and new portfolios for each leg
     let offchain_legs: Vec<Leg> = (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<Leg> = (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<Leg> = (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<T: Config>(
     instruction_id: InstructionId,
     leg_id: u32,
 ) -> ReceiptDetails<T::AccountId, T::OffChainSignature> {
-    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,