From 735c6aeed3899009dc5b0aee40316d10e31ecc1d Mon Sep 17 00:00:00 2001
From: Velenir <Velenir@users.noreply.github.com>
Date: Tue, 13 Oct 2020 09:59:24 +0300
Subject: [PATCH] Deterministic sorting ++ (#1517)

* Refactor and allow to filter in networks with no TCR

* Refactor map creation

* Return filter flag

Co-authored-by: Velenir <Velenir@users.noreply.github.com>

* Fix issue accessing Map

Co-authored-by: Velenir <Velenir@users.noreply.github.com>

* Filter tokenList

* Improve fetch token details

* Refactor fetch into it's own function

* Simplify logic

* Add label

* Provide a deterministic sorting

* simplify async logic in _fetchTokenDetails

* create a one-pass sorting function

* emphasize that sort mutates

* fix switch fallthrough

Co-authored-by: Anxo Rodriguez <anxolin@gmail.com>
---
 src/services/factories/tokenList.ts | 95 +++++++++++++++--------------
 1 file changed, 50 insertions(+), 45 deletions(-)

diff --git a/src/services/factories/tokenList.ts b/src/services/factories/tokenList.ts
index 7522b61c4..92393b834 100644
--- a/src/services/factories/tokenList.ts
+++ b/src/services/factories/tokenList.ts
@@ -173,57 +173,69 @@ export function getTokensFactory(factoryParams: {
     tokensConfig: TokenDetails[],
   ): Promise<TokenDetails[]> {
     const tokensConfigMap = new Map(tokensConfig.map((t) => [t.address, t]))
-    const tokenDetailsPromises: (Promise<TokenDetails | undefined> | TokenDetails)[] = []
-    addressToIdMap.forEach((id, tokenAddress) => {
+    const tokenDetailsPromises: Promise<TokenDetails | undefined>[] = []
+
+    async function _fillToken(id: number, tokenAddress: string): Promise<TokenDetails | undefined> {
       // Resolve the details using the config, otherwise fetch the token
-      const token: undefined | Promise<TokenDetails | undefined> = tokensConfigMap.has(tokenAddress)
-        ? Promise.resolve(tokensConfigMap.get(tokenAddress))
-        : _fetchToken(networkId, id, tokenAddress)
+      const token = tokensConfigMap.get(tokenAddress) || (await _fetchToken(networkId, id, tokenAddress))
 
       if (token) {
-        // Add a label for convenience
-        token.then((token) => {
-          if (token) {
-            token.label = safeTokenName(token)
-          }
-          return token
-        })
-
-        tokenDetailsPromises.push(token)
+        token.label = safeTokenName(token)
       }
+
+      return token
+    }
+
+    addressToIdMap.forEach((id, tokenAddress) => {
+      tokenDetailsPromises.push(_fillToken(id, tokenAddress))
     })
 
     return (await Promise.all(tokenDetailsPromises)).filter(notEmpty)
   }
 
-  function _moveTokenToHeadOfArray(tokenAddress: string, tokenList: TokenDetails[]): TokenDetails[] {
-    const tokenIndex = tokenList.findIndex((t) => t.address === tokenAddress)
-    if (tokenIndex !== -1) {
-      const token = tokenList[tokenIndex]
-      tokenList.splice(tokenIndex, 1)
-
-      return [token, ...tokenList]
-    }
-
-    return tokenList
-  }
+  type TokenComparator = (a: TokenDetails, b: TokenDetails) => number
 
-  function _sortTokens(networkId: number, tokens: TokenDetails[]): TokenDetails[] {
-    // Sort tokens
-    let tokensSorted = tokens.sort(_tokenComparer)
+  function _createTokenComparator(networkId: number): TokenComparator {
+    let comparator: TokenComparator
+    // allows correct unicode comparison
+    const compareByLabel: TokenComparator = (a, b) => a.label.localeCompare(b.label)
 
-    // Make sure wxDAI and WETH are the first tokens
     switch (networkId) {
       case Network.Mainnet:
-        return _moveTokenToHeadOfArray(WETH_ADDRESS_MAINNET, tokensSorted)
+        comparator = (a, b): number => {
+          // WETH first
+          if (a.address === WETH_ADDRESS_MAINNET) return -1
+          if (b.address === WETH_ADDRESS_MAINNET) return 1
+          return compareByLabel(a, b)
+        }
+        break
       case Network.Rinkeby:
-        return _moveTokenToHeadOfArray(WETH_ADDRESS_RINKEBY, tokensSorted)
+        comparator = (a, b): number => {
+          // WETH first
+          if (a.address === WETH_ADDRESS_RINKEBY) return -1
+          if (b.address === WETH_ADDRESS_RINKEBY) return 1
+          return compareByLabel(a, b)
+        }
+        break
       case Network.xDAI:
-        tokensSorted = _moveTokenToHeadOfArray(WETH_ADDRESS_XDAI, tokensSorted)
-        return _moveTokenToHeadOfArray(WXDAI_ADDRESS_XDAI, tokensSorted)
+        comparator = (a, b): number => {
+          // WXDAI before WETH
+          if (a.address === WXDAI_ADDRESS_XDAI && b.address === WETH_ADDRESS_XDAI) return -1
+          // WETH after WXDAI
+          if (a.address === WETH_ADDRESS_XDAI && b.address === WXDAI_ADDRESS_XDAI) return 1
+          // WXDAI and WETH first
+          if (a.address === WXDAI_ADDRESS_XDAI) return -1
+          if (b.address === WXDAI_ADDRESS_XDAI) return 1
+          if (a.address === WETH_ADDRESS_XDAI) return -1
+          if (b.address === WETH_ADDRESS_XDAI) return 1
+          return compareByLabel(a, b)
+        }
+        break
       default:
-        return tokensSorted
+        comparator = compareByLabel
     }
+
+    return comparator
   }
 
   async function _fetchToken(
@@ -259,19 +271,12 @@ export function getTokensFactory(factoryParams: {
     const tokenDetails = await _fetchTokenDetails(networkId, filteredAddressesAndIds, tokensConfig)
 
     // Sort tokens
-    const tokenList = _sortTokens(networkId, tokenDetails)
+    // note that sort mutates tokenDetails
+    // but it's ok as tokenDetails is a newly created array
+    tokenDetails.sort(_createTokenComparator(networkId))
 
     // Persist it
-    tokenListApi.persistTokens({ networkId, tokenList })
-  }
-
-  function _tokenComparer(a: TokenDetails, b: TokenDetails): number {
-    if (a.label < b.label) {
-      return -1
-    } else if (a.label > b.label) {
-      return 1
-    }
-    return 0
+    tokenListApi.persistTokens({ networkId, tokenList: tokenDetails })
   }
 
   async function updateTokens(networkId: number): Promise<void> {