Refactor: chain features and versioned features (#3120)

katspaugh · web-flow · commit a2cd631b8f76 · 2021-12-07T11:05:09.000+01:00
* Refactor: chain features and versioned features

* Update FEATURES type

* Fix tests
diff --git a/package.json b/package.json
@@ -169,7 +169,7 @@
     "@gnosis.pm/safe-core-sdk": "^0.3.1",
     "@gnosis.pm/safe-deployments": "^1.2.0",
     "@gnosis.pm/safe-react-components": "^0.9.0",
-    "@gnosis.pm/safe-react-gateway-sdk": "^2.5.7",
+    "@gnosis.pm/safe-react-gateway-sdk": "^2.5.8",
     "@ledgerhq/hw-transport-node-hid-singleton": "6.3.0",
     "@material-ui/core": "^4.12.3",
     "@material-ui/icons": "^4.11.0",
diff --git a/src/components/AppLayout/Sidebar/useSidebarItems.tsx b/src/components/AppLayout/Sidebar/useSidebarItems.tsx
@@ -3,7 +3,7 @@ import { useSelector } from 'react-redux'
 import { useRouteMatch } from 'react-router-dom'
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 
-import { getShortName, isFeatureEnabled } from 'src/config'
+import { getShortName } from 'src/config'
 import { ListItemType } from 'src/components/List'
 import ListIcon from 'src/components/List/ListIcon'
 import { currentSafeFeaturesEnabled, currentSafeWithNames } from 'src/logic/safe/store/selectors'
@@ -15,12 +15,13 @@ import {
   generatePrefixedAddressRoutes,
 } from 'src/routes/routes'
 import { IS_PRODUCTION } from 'src/utils/constants'
+import { hasFeature } from 'src/logic/safe/utils/safeVersion'
 
 const useSidebarItems = (): ListItemType[] => {
   const featuresEnabled = useSelector(currentSafeFeaturesEnabled)
-  const safeAppsEnabled = isFeatureEnabled(FEATURES.SAFE_APPS)
-  const isCollectiblesEnabled = isFeatureEnabled(FEATURES.ERC721)
-  const isSpendingLimitEnabled = isFeatureEnabled(FEATURES.SPENDING_LIMIT)
+  const safeAppsEnabled = hasFeature(FEATURES.SAFE_APPS)
+  const isCollectiblesEnabled = hasFeature(FEATURES.ERC721)
+  const isSpendingLimitEnabled = hasFeature(FEATURES.SPENDING_LIMIT)
   const { needsUpdate } = useSelector(currentSafeWithNames)
   const safeAddress = extractSafeAddress()
   const granted = useSelector(grantedSelector)
diff --git a/src/components/forms/validator.ts b/src/components/forms/validator.ts
@@ -2,11 +2,12 @@ import memoize from 'lodash/memoize'
 
 import { sameAddress } from 'src/logic/wallets/ethAddresses'
 import { getWeb3 } from 'src/logic/wallets/getWeb3'
-import { getShortName, isFeatureEnabled } from 'src/config'
+import { getShortName } from 'src/config'
 import { isValidAddress } from 'src/utils/isValidAddress'
 import { ADDRESS_BOOK_INVALID_NAMES, isValidAddressBookName } from 'src/logic/addressBook/utils'
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 import { isValidPrefix, parsePrefixedAddress } from 'src/utils/prefixedAddress'
+import { hasFeature } from 'src/logic/safe/utils/safeVersion'
 
 type ValidatorReturnType = string | undefined
 export type GenericValidatorType = (...args: unknown[]) => ValidatorReturnType
@@ -101,7 +102,7 @@ export const mustBeEthereumAddress = (fullAddress: string): ValidatorReturnType
   if (prefixError) return prefixError
 
   const result = mustBeAddressHash(address)
-  if (result !== undefined && isFeatureEnabled(FEATURES.DOMAIN_LOOKUP)) {
+  if (result !== undefined && hasFeature(FEATURES.DOMAIN_LOOKUP)) {
     return errorMessage
   }
   return result
diff --git a/src/config/cache/chains.ts b/src/config/cache/chains.ts
@@ -23,8 +23,8 @@ export const emptyChainInfo: ChainInfo = {
   l2: false,
   description: '',
   rpcUri: { authentication: '' as RPC_AUTHENTICATION, value: '' },
-  safeAppsRpcUri: { authentication: '' as RPC_AUTHENTICATION, value: '' },
   publicRpcUri: { authentication: '' as RPC_AUTHENTICATION, value: '' },
+  safeAppsRpcUri: { authentication: '' as RPC_AUTHENTICATION, value: '' },
   blockExplorerUriTemplate: {
     address: '',
     txHash: '',
diff --git a/src/config/chain-workarounds.ts b/src/config/chain-workarounds.ts
diff --git a/src/config/chain.d.ts b/src/config/chain.d.ts
@@ -44,8 +44,3 @@ export enum WALLETS {
   LATTICE = 'lattice',
   KEYSTONE = 'keystone',
 }
-
-// This is unrelated to the chains and will be removed when retrieved from core SDK
-export enum SAFE_FEATURES {
-  SAFE_TX_GAS_OPTIONAL = 'SAFE_TX_GAS_OPTIONAL',
-}
diff --git a/src/config/index.ts b/src/config/index.ts
@@ -2,7 +2,6 @@ import {
   ChainInfo,
   GasPriceOracle,
   GAS_PRICE_TYPE,
-  FEATURES,
   RPC_AUTHENTICATION,
   RpcUri,
 } from '@gnosis.pm/safe-react-gateway-sdk'
@@ -16,7 +15,7 @@ import {
   SAFE_APPS_RPC_TOKEN,
   TX_SERVICE_VERSION,
 } from 'src/utils/constants'
-import { ChainId, ChainName, SAFE_FEATURES, ShortName } from './chain.d'
+import { ChainId, ChainName, ShortName } from './chain.d'
 import { emptyChainInfo, getChains } from './cache/chains'
 import { evalTemplate } from './utils'
 
@@ -123,11 +122,8 @@ export const getExplorerUrl = (): string => {
 
 export const getHashedExplorerUrl = (hash: string): string => {
   const isTx = hash.length > 42
-
   const param = isTx ? 'txHash' : 'address'
   const uri = getExplorerUriTemplate()[param]
-  const params = { [param]: hash }
-
   return evalTemplate(uri, { [param]: hash })
 }
 
@@ -140,13 +136,6 @@ export const getExplorerInfo = (hash: string): (() => { url: string; alt: string
   return () => ({ url, alt })
 }
 
-export const isFeatureEnabled = (feature: FEATURES | SAFE_FEATURES) => {
-  return (
-    getChainInfo().features.includes(feature as FEATURES) ||
-    Object.values(SAFE_FEATURES).includes(feature as SAFE_FEATURES)
-  )
-}
-
 const getExplorerApiKey = (apiUrl: string): string | undefined => {
   return apiUrl.includes('etherscan') && ETHERSCAN_API_KEY ? ETHERSCAN_API_KEY : undefined
 }
diff --git a/src/logic/safe/store/actions/__tests__/fetchSafe.test.ts b/src/logic/safe/store/actions/__tests__/fetchSafe.test.ts
@@ -127,7 +127,16 @@ describe('fetchSafe', () => {
           nonce: 492,
           currentVersion: '1.3.0',
           needsUpdate: false,
-          featuresEnabled: ['ERC721', 'ERC1155', 'SAFE_APPS', 'CONTRACT_INTERACTION', 'SAFE_TX_GAS_OPTIONAL'],
+          featuresEnabled: [
+            'CONTRACT_INTERACTION',
+            'DOMAIN_LOOKUP',
+            'EIP1559',
+            'ERC1155',
+            'ERC721',
+            'SAFE_APPS',
+            'SAFE_TX_GAS_OPTIONAL',
+            'SPENDING_LIMIT',
+          ],
         },
       },
     ]
diff --git a/src/logic/safe/store/actions/__tests__/utils.test.ts b/src/logic/safe/store/actions/__tests__/utils.test.ts
@@ -1,7 +1,7 @@
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 import { TransactionStatus } from '@gnosis.pm/safe-react-gateway-sdk'
 
-import { ChainId, SAFE_FEATURES } from 'src/config/chain.d'
+import { ChainId } from 'src/config/chain.d'
 import {
   buildSafeOwners,
   extractRemoteSafeInfo,
@@ -149,12 +149,15 @@ describe('extractRemoteSafeInfo', () => {
       needsUpdate: false,
       guard: undefined,
       featuresEnabled: [
-        FEATURES.ERC721,
-        FEATURES.ERC1155,
-        FEATURES.SAFE_APPS,
-        FEATURES.CONTRACT_INTERACTION,
-        SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL,
-      ],
+        'CONTRACT_INTERACTION',
+        'DOMAIN_LOOKUP',
+        'EIP1559',
+        'ERC1155',
+        'ERC721',
+        'SAFE_APPS',
+        'SAFE_TX_GAS_OPTIONAL',
+        'SPENDING_LIMIT',
+      ] as FEATURES[],
     }
 
     const remoteSafeInfo = await extractRemoteSafeInfo(remoteSafeInfoWithoutModules as any)
@@ -179,12 +182,15 @@ describe('extractRemoteSafeInfo', () => {
       needsUpdate: false,
       guard: '0x4f8a82d73729A33E0165aDeF3450A7F85f007528',
       featuresEnabled: [
-        FEATURES.ERC721,
-        FEATURES.ERC1155,
-        FEATURES.SAFE_APPS,
-        FEATURES.CONTRACT_INTERACTION,
-        SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL,
-      ],
+        'CONTRACT_INTERACTION',
+        'DOMAIN_LOOKUP',
+        'EIP1559',
+        'ERC1155',
+        'ERC721',
+        'SAFE_APPS',
+        'SAFE_TX_GAS_OPTIONAL',
+        'SPENDING_LIMIT',
+      ] as FEATURES[],
     }
 
     const remoteSafeInfo = await extractRemoteSafeInfo(remoteSafeInfoWithModules as any)
diff --git a/src/logic/safe/store/models/safe.ts b/src/logic/safe/store/models/safe.ts
@@ -1,6 +1,6 @@
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 import { Record, RecordOf } from 'immutable'
-import { ChainId, SAFE_FEATURES } from 'src/config/chain.d'
+import { ChainId } from 'src/config/chain.d'
 
 import { BalanceRecord } from 'src/logic/tokens/store/actions/fetchSafeTokens'
 
@@ -37,7 +37,7 @@ export type SafeRecordProps = {
   recurringUser?: boolean
   currentVersion: string
   needsUpdate: boolean
-  featuresEnabled: Array<FEATURES | SAFE_FEATURES>
+  featuresEnabled: FEATURES[]
   loadedViaUrl: boolean
   guard: string
   collectiblesTag: string
diff --git a/src/logic/safe/transactions/gas.ts b/src/logic/safe/transactions/gas.ts
@@ -7,10 +7,8 @@ import { generateSignaturesFromTxConfirmations } from 'src/logic/safe/safeTxSign
 import { fetchSafeTxGasEstimation } from 'src/logic/safe/api/fetchSafeTxGasEstimation'
 import { Confirmation } from 'src/logic/safe/store/models/types/confirmation'
 import { checksumAddress } from 'src/utils/checksumAddress'
-import { EIP1559Chains } from 'src/config/chain-workarounds'
-import { _getChainId } from 'src/config'
 import { hasFeature } from '../utils/safeVersion'
-import { SAFE_FEATURES } from 'src/config/chain.d'
+import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 
 type SafeTxGasEstimationProps = {
   safeAddress: string
@@ -24,7 +22,7 @@ export const estimateSafeTxGas = async (
   { safeAddress, txData, txRecipient, txAmount, operation }: SafeTxGasEstimationProps,
   safeVersion: string,
 ): Promise<string> => {
-  if (hasFeature(safeVersion, SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL)) {
+  if (hasFeature(FEATURES.SAFE_TX_GAS_OPTIONAL, safeVersion)) {
     return '0'
   }
 
@@ -231,5 +229,5 @@ export const estimateGasForTransactionApproval = async ({
 }
 
 export const getGasParam = (): string => {
-  return EIP1559Chains.includes(_getChainId()) ? 'maxFeePerGas' : 'gasPrice'
+  return hasFeature(FEATURES.EIP1559) ? 'maxFeePerGas' : 'gasPrice'
 }
diff --git a/src/logic/safe/utils/__tests__/safeVersion.test.ts b/src/logic/safe/utils/__tests__/safeVersion.test.ts
@@ -1,5 +1,4 @@
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
-import { SAFE_FEATURES } from 'src/config/chain.d'
 import { checkIfSafeNeedsUpdate, hasFeature } from 'src/logic/safe/utils/safeVersion'
 
 describe('Check safe version', () => {
@@ -24,16 +23,17 @@ describe('Check safe version', () => {
 
   describe('hasFeature', () => {
     it('returns false for old Safes and SAFE_TX_GAS_OPTIONAL', () => {
-      expect(hasFeature('1.1.1', SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL)).toBe(false)
+      expect(hasFeature(FEATURES.SAFE_TX_GAS_OPTIONAL, '1.1.1')).toBe(false)
     })
 
     it('returns true for new Safes and SAFE_TX_GAS_OPTIONAL', () => {
-      expect(hasFeature('1.3.0', SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL)).toBe(true)
+      expect(hasFeature(FEATURES.SAFE_TX_GAS_OPTIONAL, '1.3.0')).toBe(true)
     })
 
     it('returns true for any Safes and SAFE_APPS', () => {
-      expect(hasFeature('1.3.0', FEATURES.SAFE_APPS)).toBe(true)
-      expect(hasFeature('1.1.1', FEATURES.SAFE_APPS)).toBe(true)
+      expect(hasFeature(FEATURES.SAFE_APPS)).toBe(true)
+      expect(hasFeature(FEATURES.SAFE_APPS, '1.3.0')).toBe(true)
+      expect(hasFeature(FEATURES.SAFE_APPS, '1.1.1')).toBe(true)
     })
   })
 })
diff --git a/src/logic/safe/utils/safeVersion.ts b/src/logic/safe/utils/safeVersion.ts
@@ -6,25 +6,14 @@ import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 import { GnosisSafe } from 'src/types/contracts/gnosis_safe.d'
 import { getSafeMasterContract } from 'src/logic/contracts/safeContracts'
 import { LATEST_SAFE_VERSION } from 'src/utils/constants'
-import { isFeatureEnabled } from 'src/config'
 import { Errors, logError } from 'src/logic/exceptions/CodedException'
-import { SAFE_FEATURES } from 'src/config/chain.d'
+import { getChainInfo } from 'src/config'
 
-type FeatureConfigByVersion = {
-  name: FEATURES | SAFE_FEATURES
-  validVersion?: string
+const FEATURES_BY_VERSION: Record<string, string> = {
+  [FEATURES.ERC1155]: '>=1.1.1',
+  [FEATURES.SAFE_TX_GAS_OPTIONAL]: '>=1.3.0',
 }
 
-const FEATURES_BY_VERSION: FeatureConfigByVersion[] = [
-  { name: FEATURES.ERC721 },
-  { name: FEATURES.ERC1155, validVersion: '>=1.1.1' },
-  { name: FEATURES.SAFE_APPS },
-  { name: FEATURES.CONTRACT_INTERACTION },
-  { name: SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL, validVersion: '>=1.3.0' },
-]
-
-type Feature = typeof FEATURES_BY_VERSION[number]
-
 export const safeNeedsUpdate = (currentVersion?: string, latestVersion?: string): boolean => {
   if (!currentVersion || !latestVersion) {
     return false
@@ -39,23 +28,21 @@ export const safeNeedsUpdate = (currentVersion?: string, latestVersion?: string)
 export const getCurrentSafeVersion = (gnosisSafeInstance: GnosisSafe): Promise<string> =>
   gnosisSafeInstance.methods.VERSION().call()
 
-const checkFeatureEnabledByVersion = (featureConfig: FeatureConfigByVersion, version?: string) => {
-  if (!version) {
-    return false
-  }
-  return featureConfig.validVersion ? semverSatisfies(version, featureConfig.validVersion) : true
+const isEnabledByVersion = (feature: FEATURES, version: string): boolean => {
+  if (!(feature in FEATURES_BY_VERSION)) return true
+  return semverSatisfies(version, FEATURES_BY_VERSION[feature])
 }
 
-export const enabledFeatures = (version?: string): (FEATURES | SAFE_FEATURES)[] => {
-  return FEATURES_BY_VERSION.reduce((acc, feature: Feature) => {
-    if (isFeatureEnabled(feature.name) && checkFeatureEnabledByVersion(feature, version)) {
-      acc.push(feature.name)
-    }
-    return acc
-  }, [] as (FEATURES | SAFE_FEATURES)[])
+/**
+ * Get a combined list of features enabled per chain and per version
+ */
+export const enabledFeatures = (version?: string): FEATURES[] => {
+  const chainFeatures = getChainInfo().features
+  if (!version) return chainFeatures
+  return chainFeatures.filter((feat) => isEnabledByVersion(feat, version))
 }
 
-export const hasFeature = (version: string, name: FEATURES | SAFE_FEATURES): boolean => {
+export const hasFeature = (name: FEATURES, version?: string): boolean => {
   return enabledFeatures(version).includes(name)
 }
 
@@ -80,7 +67,7 @@ export const checkIfSafeNeedsUpdate = async (
 }
 
 export const getCurrentMasterContractLastVersion = async (): Promise<string> => {
-  let safeMasterVersion
+  let safeMasterVersion: string
   try {
     const safeMaster = await getSafeMasterContract()
     safeMasterVersion = await safeMaster.methods.VERSION().call()
diff --git a/src/routes/safe/components/Balances/SendModal/screens/AddressBookInput/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/AddressBookInput/index.tsx
@@ -4,7 +4,6 @@ import { Dispatch, ReactElement, SetStateAction, useEffect, useState } from 'rea
 import { useSelector } from 'react-redux'
 
 import { mustBeEthereumAddress, mustBeEthereumContractAddress } from 'src/components/forms/validator'
-import { isFeatureEnabled } from 'src/config'
 import { AddressBookEntry } from 'src/logic/addressBook/model/addressBook'
 import { currentNetworkAddressBook } from 'src/logic/addressBook/store/selectors'
 import { filterContractAddressBookEntries, filterAddressEntries } from 'src/logic/addressBook/utils'
@@ -21,6 +20,7 @@ import { checksumAddress } from 'src/utils/checksumAddress'
 import { currentChainId } from 'src/logic/config/store/selectors'
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 import { parsePrefixedAddress } from 'src/utils/prefixedAddress'
+import { hasFeature } from 'src/logic/safe/utils/safeVersion'
 
 export interface AddressBookProps {
   fieldMutator: (address: string) => void
@@ -96,7 +96,7 @@ const BaseAddressBookInput = ({
 
         // ENS-enabled resolve/validation
         if (
-          isFeatureEnabled(FEATURES.DOMAIN_LOOKUP) &&
+          hasFeature(FEATURES.DOMAIN_LOOKUP) &&
           (isValidEnsName(normalizedValue) || isValidCryptoDomainName(normalizedValue))
         ) {
           let address = ''
diff --git a/src/routes/safe/components/Transactions/helpers/useSafeTxGas.ts b/src/routes/safe/components/Transactions/helpers/useSafeTxGas.ts
@@ -1,12 +1,11 @@
+import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 import { useSelector } from 'react-redux'
-
-import { SAFE_FEATURES } from 'src/config/chain.d'
 import { currentSafeCurrentVersion } from 'src/logic/safe/store/selectors'
 import { hasFeature } from 'src/logic/safe/utils/safeVersion'
 
 const useSafeTxGas = (): boolean => {
   const safeVersion = useSelector(currentSafeCurrentVersion)
-  const showSafeTxGas = !hasFeature(safeVersion, SAFE_FEATURES.SAFE_TX_GAS_OPTIONAL)
+  const showSafeTxGas = !hasFeature(FEATURES.SAFE_TX_GAS_OPTIONAL, safeVersion)
   return showSafeTxGas
 }
 
diff --git a/yarn.lock b/yarn.lock
@@ -2169,7 +2169,7 @@
   dependencies:
     isomorphic-unfetch "^3.1.0"
 
-"@gnosis.pm/safe-react-gateway-sdk@^2.5.7":
+"@gnosis.pm/safe-react-gateway-sdk@^2.5.8":
   version "2.5.8"
   resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-react-gateway-sdk/-/safe-react-gateway-sdk-2.5.8.tgz#e43b1346829a127f5e87c3e45bfc0afb3d28773c"
   integrity sha512-ZMVblmKe4YEGoznNzZxKr2coXS3ZIRpLX80ErlcAauv1YbJgdIGpRyloiGkOtwshwI3y5pRm0FI1QU8mrcUlwg==

Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,3 @@ export enum WALLETS {`
`44`	`44`	`LATTICE = 'lattice',`
`45`	`45`	`KEYSTONE = 'keystone',`
`46`	`46`	`}`
`47`		`-`
`48`		`-// This is unrelated to the chains and will be removed when retrieved from core SDK`
`49`		`-export enum SAFE_FEATURES {`
`50`		`- SAFE_TX_GAS_OPTIONAL = 'SAFE_TX_GAS_OPTIONAL',`
`51`		`-}`