Merge pull request #271 from PolymathNetwork/fix/MSDK-380-unreadable-…

…tickers

fix: filter out unreadable tickers when fetching reservations and tokens

src/Polymesh.ts

@@ -43,7 +43,7 @@ import {
   tickerToString,
   u32ToBigNumber,
 } from '~/utils/conversion';
-import { getDid } from '~/utils/internal';
+import { getDid, stringIsClean } from '~/utils/internal';
 
 import { Claims } from './Claims';
 // import { Governance } from './Governance';
@@ -353,6 +353,8 @@ export class Polymesh {
    *   have already been launched
    *
    * @param args.owner - identity representation or Identity ID as stored in the blockchain
+   *
+   * * @note reservations with unreadable characters in their tickers will be left out
    */
   public async getTickerReservations(args?: {
     owner: string | Identity;
@@ -370,13 +372,20 @@ export class Polymesh {
       stringToIdentityId(did, context)
     );
 
-    const tickerReservations: TickerReservation[] = entries
-      .filter(([, relation]) => relation.isTickerOwned)
-      .map(([key]) => {
-        const ticker = tickerToString(key.args[1] as Ticker);
+    const tickerReservations: TickerReservation[] = entries.reduce<TickerReservation[]>(
+      (result, [key, relation]) => {
+        if (relation.isTickerOwned) {
+          const ticker = tickerToString(key.args[1] as Ticker);
 
-        return new TickerReservation({ ticker }, context);
-      });
+          if (stringIsClean(ticker)) {
+            return [...result, new TickerReservation({ ticker }, context)];
+          }
+        }
+
+        return result;
+      },
+      []
+    );
 
     return tickerReservations;
   }
@@ -503,6 +512,8 @@ export class Polymesh {
    * Retrieve all the Security Tokens owned by an Identity
    *
    * @param args.owner - identity representation or Identity ID as stored in the blockchain
+   *
+   * @note tokens with unreadable characters in their tickers will be left out
    */
   public async getSecurityTokens(args?: { owner: string | Identity }): Promise<SecurityToken[]> {
     const {
@@ -518,13 +529,20 @@ export class Polymesh {
       stringToIdentityId(did, context)
     );
 
-    const securityTokens: SecurityToken[] = entries
-      .filter(([, relation]) => relation.isAssetOwned)
-      .map(([key]) => {
-        const ticker = tickerToString(key.args[1] as Ticker);
+    const securityTokens: SecurityToken[] = entries.reduce<SecurityToken[]>(
+      (result, [key, relation]) => {
+        if (relation.isAssetOwned) {
+          const ticker = tickerToString(key.args[1] as Ticker);
 
-        return new SecurityToken({ ticker }, context);
-      });
+          if (stringIsClean(ticker)) {
+            return [...result, new SecurityToken({ ticker }, context)];
+          }
+        }
+
+        return result;
+      },
+      []
+    );
 
     return securityTokens;
   }

src/__tests__/Polymesh.ts

@@ -501,6 +501,41 @@ describe('Polymesh Class', () => {
       expect(tickerReservations).toHaveLength(1);
       expect(tickerReservations[0].ticker).toBe(fakeTicker);
     });
+
+    test('should filter out tickers with unreadable characters', async () => {
+      const fakeTicker = 'TEST';
+      const unreadableTicker = String.fromCharCode(65533);
+      const did = 'someDid';
+
+      dsMockUtils.configureMocks({ contextOptions: { withSeed: true } });
+
+      dsMockUtils.createQueryStub('asset', 'assetOwnershipRelations', {
+        entries: [
+          tuple(
+            [dsMockUtils.createMockIdentityId(did), dsMockUtils.createMockTicker(fakeTicker)],
+            dsMockUtils.createMockAssetOwnershipRelation('TickerOwned')
+          ),
+          tuple(
+            [dsMockUtils.createMockIdentityId(did), dsMockUtils.createMockTicker('someTicker')],
+            dsMockUtils.createMockAssetOwnershipRelation('AssetOwned')
+          ),
+          tuple(
+            [dsMockUtils.createMockIdentityId(did), dsMockUtils.createMockTicker(unreadableTicker)],
+            dsMockUtils.createMockAssetOwnershipRelation('TickerOwned')
+          ),
+        ],
+      });
+
+      const polymesh = await Polymesh.connect({
+        nodeUrl: 'wss://some.url',
+        accountUri: '//uri',
+      });
+
+      const tickerReservations = await polymesh.getTickerReservations();
+
+      expect(tickerReservations).toHaveLength(1);
+      expect(tickerReservations[0].ticker).toBe(fakeTicker);
+    });
   });
 
   describe('method: getTickerReservation', () => {
@@ -722,6 +757,41 @@ describe('Polymesh Class', () => {
       expect(securityTokens).toHaveLength(1);
       expect(securityTokens[0].ticker).toBe(fakeTicker);
     });
+
+    test('should filter out tokens whose tickers have unreadable characters', async () => {
+      const fakeTicker = 'TEST';
+      const unreadableTicker = String.fromCharCode(65533);
+      const did = 'someDid';
+
+      dsMockUtils.configureMocks({ contextOptions: { withSeed: true } });
+
+      dsMockUtils.createQueryStub('asset', 'assetOwnershipRelations', {
+        entries: [
+          tuple(
+            [dsMockUtils.createMockIdentityId(did), dsMockUtils.createMockTicker(fakeTicker)],
+            dsMockUtils.createMockAssetOwnershipRelation('AssetOwned')
+          ),
+          tuple(
+            [dsMockUtils.createMockIdentityId(did), dsMockUtils.createMockTicker('someTicker')],
+            dsMockUtils.createMockAssetOwnershipRelation('TickerOwned')
+          ),
+          tuple(
+            [dsMockUtils.createMockIdentityId(did), dsMockUtils.createMockTicker(unreadableTicker)],
+            dsMockUtils.createMockAssetOwnershipRelation('AssetOwned')
+          ),
+        ],
+      });
+
+      const polymesh = await Polymesh.connect({
+        nodeUrl: 'wss://some.url',
+        accountUri: '//uri',
+      });
+
+      const securityTokens = await polymesh.getSecurityTokens();
+
+      expect(securityTokens).toHaveLength(1);
+      expect(securityTokens[0].ticker).toBe(fakeTicker);
+    });
   });
 
   describe('method: transferPolyX', () => {

src/api/entities/SecurityToken/index.ts

@@ -166,13 +166,15 @@ export class SecurityToken extends Entity<UniqueIdentifiers> {
     });
     /* eslint-enable @typescript-eslint/camelcase */
 
+    const rawTicker = stringToTicker(ticker, context);
+
     if (callback) {
-      return asset.tokens(ticker, securityToken => {
+      return asset.tokens(rawTicker, securityToken => {
         callback(assembleResult(securityToken));
       });
     }
 
-    const token = await asset.tokens(ticker);
+    const token = await asset.tokens(rawTicker);
 
     return assembleResult(token);
   }
@@ -196,15 +198,18 @@ export class SecurityToken extends Entity<UniqueIdentifiers> {
         },
       },
       ticker,
+      context,
     } = this;
 
+    const rawTicker = stringToTicker(ticker, context);
+
     if (callback) {
-      return asset.fundingRound(ticker, round => {
+      return asset.fundingRound(rawTicker, round => {
         callback(fundingRoundNameToString(round));
       });
     }
 
-    const fundingRound = await asset.fundingRound(ticker);
+    const fundingRound = await asset.fundingRound(rawTicker);
     return fundingRoundNameToString(fundingRound);
   }
 

src/utils/__tests__/conversion.ts

@@ -402,6 +402,13 @@ describe('stringToTicker and tickerToString', () => {
     );
   });
 
+  test('stringToTicker should throw an error if the string contains unreadable characters', () => {
+    const value = `Illegal ${String.fromCharCode(65533)}`;
+    const context = dsMockUtils.getContextInstance();
+
+    expect(() => stringToTicker(value, context)).toThrow('Ticker contains unreadable characters');
+  });
+
   test('tickerToString should convert a polkadot Ticker object to a string', () => {
     const fakeResult = 'someTicker';
     const ticker = dsMockUtils.createMockTicker(fakeResult);

src/utils/__tests__/internal.ts

@@ -22,6 +22,7 @@ import {
   requestAtBlock,
   requestPaginated,
   serialize,
+  stringIsClean,
   unserialize,
   unwrapValue,
   unwrapValues,
@@ -412,4 +413,14 @@ describe('calculateNextKey', () => {
 
     expect(nextKey).toEqual(30);
   });
+
+  describe('stringIsClean', () => {
+    test('should return false if the string contains charcode 65533', () => {
+      expect(stringIsClean(String.fromCharCode(65533))).toBe(false);
+    });
+
+    test("should return true if the string doesn't contain any forbidden characters", () => {
+      expect(stringIsClean('Clean String')).toBe(true);
+    });
+  });
 });

src/utils/conversion.ts

@@ -122,7 +122,7 @@ import {
   MAX_TOKEN_AMOUNT,
   SS58_FORMAT,
 } from '~/utils/constants';
-import { createClaim, padString } from '~/utils/internal';
+import { createClaim, padString, removePadding, stringIsClean } from '~/utils/internal';
 
 export * from '~/generated/utils';
 
@@ -187,6 +187,14 @@ export function stringToTicker(ticker: string, context: Context): Ticker {
       message: `Ticker length cannot exceed ${MAX_TICKER_LENGTH} characters`,
     });
   }
+
+  if (!stringIsClean(ticker)) {
+    throw new PolymeshError({
+      code: ErrorCode.ValidationError,
+      message: 'Ticker contains unreadable characters',
+    });
+  }
+
   return context.polymeshApi.createType('Ticker', ticker);
 }
 
@@ -195,7 +203,7 @@ export function stringToTicker(ticker: string, context: Context): Ticker {
  */
 export function tickerToString(ticker: Ticker): string {
   // eslint-disable-next-line no-control-regex
-  return u8aToString(ticker).replace(/\u0000/g, '');
+  return removePadding(u8aToString(ticker));
 }
 
 /**
@@ -1022,7 +1030,7 @@ export function middlewareScopeToScope(scope: MiddlewareScope): Scope {
   switch (type) {
     case ClaimScopeTypeEnum.Ticker:
       // eslint-disable-next-line no-control-regex
-      return { type: ScopeType.Ticker, value: value.replace(/\u0000/g, '') };
+      return { type: ScopeType.Ticker, value: removePadding(value) };
     case ClaimScopeTypeEnum.Identity:
     case ClaimScopeTypeEnum.Custom:
       return { type: ScopeType[scope.type], value };

src/utils/internal.ts

@@ -5,7 +5,7 @@ import { BlockHash } from '@polkadot/types/interfaces/chain';
 import { AnyFunction, ISubmittableResult } from '@polkadot/types/types';
 import { stringUpperFirst } from '@polkadot/util';
 import stringify from 'json-stable-stringify';
-import { chunk, groupBy, map, padEnd } from 'lodash';
+import { chunk, groupBy, map, padEnd, range } from 'lodash';
 
 import { Identity } from '~/api/entities';
 import { Context, PolymeshError, PostTransactionValue } from '~/base';
@@ -191,6 +191,17 @@ export function removePadding(value: string): string {
   return value.replace(/\u0000/g, '');
 }
 
+/**
+ * @hidden
+ *
+ * Return whether the string is free of unreadable characters
+ */
+export function stringIsClean(value: string): boolean {
+  const forbiddenCharCodes = [65533]; // this should be extended as we find more offending characters
+
+  return !range(value.length).some(index => forbiddenCharCodes.includes(value.charCodeAt(index)));
+}
+
 /**
  * @hidden
  *