Untrusted assets should not block the addition of custom tokens (#3491)

Closes #3467 ## What Currently, the user isn't able to add an asset that is in the untrusted list. We display a message about the existence of the token and block the button. We should allow users to add an asset that is in the untrusted list. ### Changes - Ability to manually add unverified assets. - Simplification of utils function for untrusted/trusted tokens. - When adding an unverified asset, don't display a dust warning. - Don't update the metadata of assets in the token list and network base assets. ## UI https://github.com/tahowallet/extension/assets/23117945/f5dd9dea-e8c5-4bf1-b535-73edbd93a86c ## Testing - [x] Select an unverified token from the list and then try adding it manually. The asset should be added correctly. - [x] Try adding an unverified token which is dust. There should be a dust warning. - [x] Verify the asset from the list and then try to add it manually. A message should appear saying that the asset is already added. - [x] Try adding a trusted asset that already exists in the wallet. A message should appear saying that the asset is already added. - [x] Check that adding tokens works for assets outside the unverified list. - Test address: `testertesting.eth` - Network: Fantom Opera - Token: `0x841fad6eae12c286d1fd18d1d525dffa75c7effe` - [x] Ensure that you continue to be unable to send and swap unverified assets until verified. After verification, the token should appear in the list of assets to be sent and swapped. - [x] Verification by manually importing the token - [x] Verification by the add to list button (asset warning slide-up menu) Latest build: [extension-builds-3491](https://github.com/tahowallet/extension/suites/14149590642/artifacts/792592880) (as of Sat, 08 Jul 2023 04:51:18 GMT).
tahowallet · Jul 8, 2023 · 25367ee · 25367ee
2 parents 713acec + c01bef5
commit 25367ee
Show file tree

Hide file tree

Showing 11 changed files with 146 additions and 128 deletions.
diff --git a/background/redux-slices/selectors/0xSwapSelectors.ts b/background/redux-slices/selectors/0xSwapSelectors.ts
@@ -3,11 +3,12 @@ import { selectCurrentNetwork } from "./uiSelectors"
 import { SwappableAsset, isSmartContractFungibleAsset } from "../../assets"
 import { sameNetwork } from "../../networks"
 import {
- canBeUsedForTransaction,
  isBuiltInNetworkBaseAsset,
+ isVerifiedOrTrustedAsset,
 } from "../utils/asset-utils"
 import { RootState } from ".."
 import { SingleAssetState } from "../assets"
+import { FeatureFlags, isEnabled } from "../../features"
 
 export const selectLatestQuoteRequest = createSelector(
  (state: RootState) => state.swap.latestQuoteRequest,
@@ -29,21 +30,15 @@ export const selectSwapBuyAssets = createSelector(
  ): asset is SwappableAsset & {
  recentPrices: SingleAssetState["recentPrices"]
  } => {
- if (!canBeUsedForTransaction(asset)) {
- return false
- }
- if (isSmartContractFungibleAsset(asset)) {
- if (sameNetwork(asset.homeNetwork, currentNetwork)) {
- return true
- }
- }
- if (
- // Explicitly add a network's base asset.
- isBuiltInNetworkBaseAsset(asset, currentNetwork)
- ) {
- return true
- }
- return false
+ return (
+ // When the flag is disabled all assets can be sent and swapped
+ (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) ||
+ isVerifiedOrTrustedAsset(asset)) &&
+ // Only list assets for the current network.
+ (isBuiltInNetworkBaseAsset(asset, currentNetwork) ||
+ (isSmartContractFungibleAsset(asset) &&
+ sameNetwork(asset.homeNetwork, currentNetwork)))
+ )
  }
  )
  }

diff --git a/background/redux-slices/selectors/accountsSelectors.ts b/background/redux-slices/selectors/accountsSelectors.ts
@@ -13,7 +13,7 @@ import {
  formatCurrencyAmount,
  heuristicDesiredDecimalsForUnitPrice,
  isNetworkBaseAsset,
- isUnverifiedAssetByUser,
+ isVerifiedOrTrustedAsset,
 } from "../utils/asset-utils"
 import {
  AnyAsset,
@@ -76,11 +76,11 @@ export function determineAssetDisplayAndVerify(
  hideDust,
  showUnverifiedAssets,
  }: { hideDust: boolean; showUnverifiedAssets: boolean }
-): { displayAsset: boolean; verifiedAsset: boolean } {
- const isVerified = !isUnverifiedAssetByUser(assetAmount.asset)
+): { displayAsset: boolean; verifiedOrTrustedAsset: boolean } {
+ const isVerifiedOrTrusted = isVerifiedOrTrustedAsset(assetAmount.asset)
 
  if (shouldForciblyDisplayAsset(assetAmount)) {
- return { displayAsset: true, verifiedAsset: isVerified }
+ return { displayAsset: true, verifiedOrTrustedAsset: isVerifiedOrTrusted }
  }
 
  const isNotDust =
@@ -90,13 +90,14 @@ export function determineAssetDisplayAndVerify(
  const isPresent = assetAmount.decimalAmount > 0
  const showDust = !hideDust
 
- const verificationStatusAllowsVisibility = showUnverifiedAssets || isVerified
+ const verificationStatusAllowsVisibility =
+ showUnverifiedAssets || isVerifiedOrTrusted
  const enoughBalanceToBeVisible = isPresent && (isNotDust || showDust)
 
  return {
  displayAsset:
  verificationStatusAllowsVisibility && enoughBalanceToBeVisible,
- verifiedAsset: isVerified,
+ verifiedOrTrustedAsset: isVerifiedOrTrusted,
  }
 }
 
@@ -186,13 +187,14 @@ const computeCombinedAssetAmountsData = (
  unverifiedAssetAmounts: CompleteAssetAmount[]
  }>(
  (acc, assetAmount) => {
- const { displayAsset, verifiedAsset } = determineAssetDisplayAndVerify(
- assetAmount,
- { hideDust, showUnverifiedAssets }
- )
+ const { displayAsset, verifiedOrTrustedAsset } =
+ determineAssetDisplayAndVerify(assetAmount, {
+ hideDust,
+ showUnverifiedAssets,
+ })
 
  if (displayAsset) {
- if (verifiedAsset) {
+ if (verifiedOrTrustedAsset) {
  acc.combinedAssetAmounts.push(assetAmount)
  } else {
  acc.unverifiedAssetAmounts.push(assetAmount)

diff --git a/background/redux-slices/utils/asset-utils.ts b/background/redux-slices/utils/asset-utils.ts
@@ -17,7 +17,6 @@ import {
  OPTIMISM,
  POLYGON,
 } from "../../constants"
-import { FeatureFlags, isEnabled } from "../../features"
 import { fromFixedPointNumber } from "../../lib/fixed-point"
 import { sameEVMAddress } from "../../lib/utils"
 import { AnyNetwork, NetworkBaseAsset, sameNetwork } from "../../networks"
@@ -342,41 +341,50 @@ export function heuristicDesiredDecimalsForUnitPrice(
  )
 }
 
+export function isTokenListAsset(asset: AnyAsset): boolean {
+ return !!asset.metadata?.tokenLists?.length
+}
+
 /**
- * Check if the asset has a list of tokens.
- * Assets that do not have it are considered untrusted.
+ * Check if the asset is in a token list or is a network base asset.
+ * If not it means it is an untrusted asset.
  *
  */
-export function isUntrustedAsset(asset: AnyAsset | undefined): boolean {
- if (asset) {
- return !asset?.metadata?.tokenLists?.length
- }
- return false
+export function isUntrustedAsset(asset: AnyAsset): boolean {
+ return !isTokenListAsset(asset) && !isNetworkBaseAsset(asset)
 }
 
 /**
- * NB: non-base assets that don't have any token lists are considered
- * untrusted. Reifying base assets clearly will improve this check down the
- * road. Eventually, assets can be flagged as trusted by adding them to an
- * "internal" token list that users can export and share.
+ * Checks the user has explicitly verified the asset.
+ * The verified property was manually set to true.
  *
  */
-export function isUnverifiedAssetByUser(asset: AnyAsset | undefined): boolean {
- if (asset) {
- if (asset.metadata?.verified !== undefined) {
- // If we have verified metadata return it
- return !asset.metadata.verified
- }
-
- const baseAsset = isNetworkBaseAsset(asset)
- const isUntrusted = isUntrustedAsset(asset)
-
- return !baseAsset && isUntrusted
+export function isVerifiedAssetByUser(asset: AnyAsset): boolean {
+ if (asset.metadata?.verified !== undefined) {
+ // If we have verified metadata return it
+ return asset.metadata.verified
  }
-
  return false
 }
 
+/**
+ * Check if an asset is verified or trusted.
+ * The asset can be trusted when is in a token list or the asset is a network base asset.
+ * Untrusted asset can be manually verified by the user.
+ *
+ * Only such assets can take part in wallet actions.
+ * By actions is meant:
+ * - doing an swap with this asset
+ * - sending this asset to another address
+ */
+export function isVerifiedOrTrustedAsset(asset: AnyAsset): boolean {
+ return (
+ isVerifiedAssetByUser(asset) ||
+ isNetworkBaseAsset(asset) ||
+ isTokenListAsset(asset)
+ )
+}
+
 type AssetType = "base" | "erc20"
 
 export type AssetID = `${AssetType}/${string}`
@@ -391,17 +399,6 @@ export const getAssetID = (
  return `erc20/${asset.contractAddress}`
 }
 
-/**
- * Assets that are untrusted and have not been verified by the user
- * should not be swapped or sent.
- */
-export function canBeUsedForTransaction(asset: AnyAsset): boolean {
- if (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET)) {
- return true
- }
- return isUntrustedAsset(asset) ? !isUnverifiedAssetByUser(asset) : true
-}
-
 // FIXME Unify once asset similarity code is unified.
 export function isSameAsset(asset1?: AnyAsset, asset2?: AnyAsset): boolean {
  if (typeof asset1 === "undefined" || typeof asset2 === "undefined") {

diff --git a/background/redux-slices/utils/tests/asset-utils.unit.test.ts b/background/redux-slices/utils/tests/asset-utils.unit.test.ts
@@ -1,11 +1,11 @@
 import { createSmartContractAsset } from "../../../tests/factories"
 import { ETH, OPTIMISTIC_ETH } from "../../../constants"
-import { isSameAsset, isUnverifiedAssetByUser } from "../asset-utils"
+import { isSameAsset, isVerifiedAssetByUser } from "../asset-utils"
 import { NetworkBaseAsset } from "../../../networks"
 
 describe("Asset utils", () => {
- describe("isUnverifiedAssetByUser", () => {
- test("should return true if is an unverified asset", () => {
+ describe("isVerifiedAssetByUser", () => {
+ test("should return false if is an unverified asset", () => {
  const asset = {
  name: "Test",
  symbol: "TST",
@@ -16,10 +16,10 @@ describe("Asset utils", () => {
  websiteURL: "",
  },
  }
- expect(isUnverifiedAssetByUser(asset)).toBeTruthy()
+ expect(isVerifiedAssetByUser(asset)).toBeFalsy()
  })
 
- test("should return false if is a verified asset", () => {
+ test("should return true if is a verified asset", () => {
  const asset = {
  name: "Test",
  symbol: "TST",
@@ -36,15 +36,11 @@ describe("Asset utils", () => {
  verified: true,
  },
  }
- expect(isUnverifiedAssetByUser(asset)).toBeFalsy()
+ expect(isVerifiedAssetByUser(asset)).toBeTruthy()
  })
 
  test("should return false if is a base asset", () => {
- expect(isUnverifiedAssetByUser(ETH)).toBeFalsy()
- })
-
- test("should return false if an asset is undefined", () => {
- expect(isUnverifiedAssetByUser(undefined)).toBeFalsy()
+ expect(isVerifiedAssetByUser(ETH)).toBeFalsy()
  })
  })
 

diff --git a/background/services/indexing/index.ts b/background/services/indexing/index.ts
@@ -48,6 +48,10 @@ import {
  normalizeEVMAddress,
  sameEVMAddress,
 } from "../../lib/utils"
+import {
+ isUntrustedAsset,
+ isVerifiedAssetByUser,
+} from "../../redux-slices/utils/asset-utils"
 
 // Transactions seen within this many blocks of the chain tip will schedule a
 // token refresh sooner than the standard rate.
@@ -98,6 +102,18 @@ const getActiveAssetsByAddressForNetwork = (
  return getAssetsByAddress(networkActiveAssets)
 }
 
+function allowVerifyAssetByManualImport(
+ asset: SmartContractFungibleAsset,
+ verified?: boolean
+): boolean {
+ // Only untrusted and unverified assets can be verified.
+ if (isUntrustedAsset(asset) && !isVerifiedAssetByUser(asset)) {
+ return !!verified
+ }
+
+ return false
+}
+
 /**
  * IndexingService is responsible for pulling and maintaining all application-
  * level "indexing" data — things like fungible token balances and NFTs, as well
@@ -700,20 +716,14 @@ export default class IndexingService extends BaseService<Events> {
  normalizedAddress
  )
 
- if (knownAsset) {
- const newDiscoveryTxHash = metadata?.discoveryTxHash
- const addressForDiscoveryTxHash = newDiscoveryTxHash
- ? Object.keys(newDiscoveryTxHash)[0]
- : undefined
- const existingDiscoveryTxHash = addressForDiscoveryTxHash
- ? knownAsset.metadata?.discoveryTxHash?.[addressForDiscoveryTxHash]
- : undefined
- // If the discovery tx hash is not specified
- // or if it already exists in the asset, do not update the asset
- if (!newDiscoveryTxHash || existingDiscoveryTxHash) {
- await this.addAssetToTrack(knownAsset)
- return knownAsset
- }
+ if (
+ knownAsset &&
+ // Refresh a known unverified asset if it has been manually imported.
+ // This check allows the user to add an asset from the unverified list.
+ !allowVerifyAssetByManualImport(knownAsset, metadata?.verified)
+ ) {
+ await this.addAssetToTrack(knownAsset)
+ return knownAsset
  }
 
  let customAsset = await this.db.getCustomAssetByAddressAndNetwork(

diff --git a/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx b/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx
@@ -3,7 +3,11 @@ import { Link } from "react-router-dom"
 import { CompleteAssetAmount } from "@tallyho/tally-background/redux-slices/accounts"
 
 import { useTranslation } from "react-i18next"
-import { isUnverifiedAssetByUser } from "@tallyho/tally-background/redux-slices/utils/asset-utils"
+import {
+ isUntrustedAsset,
+ isVerifiedAssetByUser,
+ isVerifiedOrTrustedAsset,
+} from "@tallyho/tally-background/redux-slices/utils/asset-utils"
 import { selectCurrentNetwork } from "@tallyho/tally-background/redux-slices/selectors"
 import { NETWORKS_SUPPORTING_SWAPS } from "@tallyho/tally-background/constants"
 import {
@@ -52,7 +56,8 @@ export default function CommonAssetListItem(
  ? assetAmount.asset.contractAddress
  : undefined
 
- const isUnverified = isUnverifiedAssetByUser(assetAmount.asset)
+ const isUntrusted = isUntrustedAsset(assetAmount.asset)
+ const isVerified = isVerifiedAssetByUser(assetAmount.asset)
 
  const handleVerifyAsset = (event: React.MouseEvent<HTMLButtonElement>) => {
  event.preventDefault()
@@ -88,9 +93,9 @@ export default function CommonAssetListItem(
  </div>
 
  {
- // @TODO don't fetch prices for unverified assets in the first place
- // Only show prices for verified assets
- isUnverified ||
+ // @TODO don't fetch prices for untrusted assets in the first place
+ // Only show prices for trusted or verified assets
+ !isVerifiedOrTrustedAsset(assetAmount.asset) ||
  (initializationLoadingTimeExpired &&
  isMissingLocalizedUserValue) ? (
  <></>
@@ -109,7 +114,8 @@ export default function CommonAssetListItem(
  <div className="asset_right">
  <>
  {isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) &&
- isUnverified ? (
+ isUntrusted &&
+ !isVerified ? (
  <AssetVerifyToggler
  text={t("unverifiedAssets.verifyAsset")}
  icon="notif-attention"
@@ -120,7 +126,7 @@ export default function CommonAssetListItem(
  ) : (
  <>
  {!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) &&
- isUnverified && (
+ isUntrusted && (
  <SharedIcon
  icon="/icons/m/notif-attention.svg"
  width={24}

diff --git a/ui/components/Wallet/UnverifiedAsset/AssetWarning.tsx b/ui/components/Wallet/UnverifiedAsset/AssetWarning.tsx
@@ -12,7 +12,7 @@ import {
  selectCurrentNetwork,
 } from "@tallyho/tally-background/redux-slices/selectors"
 import classNames from "classnames"
-import { isUnverifiedAssetByUser } from "@tallyho/tally-background/redux-slices/utils/asset-utils"
+import { isVerifiedAssetByUser } from "@tallyho/tally-background/redux-slices/utils/asset-utils"
 import { setSnackbarMessage } from "@tallyho/tally-background/redux-slices/ui"
 import { FeatureFlags, isEnabled } from "@tallyho/tally-background/features"
 import { useHistory, useLocation } from "react-router-dom"
@@ -51,7 +51,7 @@ export default function AssetWarning(props: AssetWarningProps): ReactElement {
 
  const network = useBackgroundSelector(selectCurrentNetwork)
 
- const isUnverified = isUnverifiedAssetByUser(asset)
+ const isUnverified = !isVerifiedAssetByUser(asset)
 
  const contractAddress =
  asset && "contractAddress" in asset && asset.contractAddress