From 5a849eabeec20e5d9ff4c32e9bd82ff342211d80 Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Wed, 21 Jun 2023 13:31:38 +0200 Subject: [PATCH 01/16] Allow users to add an asset that is in the untrusted list --- background/main.ts | 5 ++++- background/services/indexing/index.ts | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/background/main.ts b/background/main.ts index cdc1bfc8df..4cf9d24ec7 100644 --- a/background/main.ts +++ b/background/main.ts @@ -190,6 +190,7 @@ import { import { isBuiltInNetworkBaseAsset, isSameAsset, + isUntrustedAsset, } from "./redux-slices/utils/asset-utils" import { getPricePoint, getTokenPrices } from "./lib/prices" import { DismissableItem } from "./services/preferences" @@ -1894,6 +1895,8 @@ export default class Main extends BaseService { const mainCurrencyAmount = convertedAssetAmount ? assetAmountToDesiredDecimals(convertedAssetAmount, 2) : undefined + // The user should be able to add an asset that is on the untrusted list. + const exists = !isUntrustedAsset(cachedAsset) || false return { ...assetData, @@ -1901,7 +1904,7 @@ export default class Main extends BaseService { utils.formatUnits(assetData.amount, assetData.asset.decimals) ), mainCurrencyAmount, - exists: !!cachedAsset, + exists, } } diff --git a/background/services/indexing/index.ts b/background/services/indexing/index.ts index e5cb30f2cc..8294963fd9 100644 --- a/background/services/indexing/index.ts +++ b/background/services/indexing/index.ts @@ -48,6 +48,7 @@ import { normalizeEVMAddress, sameEVMAddress, } from "../../lib/utils" +import { isUntrustedAsset } 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. @@ -699,8 +700,8 @@ export default class IndexingService extends BaseService { network, normalizedAddress ) - - if (knownAsset) { + // The user should be able to add an asset that is on the untrusted list. + if (knownAsset && !isUntrustedAsset(knownAsset)) { const newDiscoveryTxHash = metadata?.discoveryTxHash const addressForDiscoveryTxHash = newDiscoveryTxHash ? Object.keys(newDiscoveryTxHash)[0] From 5f36c192574a53fd279b2e6ef127cfd77313df4f Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Wed, 21 Jun 2023 14:56:15 +0200 Subject: [PATCH 02/16] Fix incorrect condition for adding a custom token --- background/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/background/main.ts b/background/main.ts index 4cf9d24ec7..f2a6148769 100644 --- a/background/main.ts +++ b/background/main.ts @@ -1896,7 +1896,7 @@ export default class Main extends BaseService { ? assetAmountToDesiredDecimals(convertedAssetAmount, 2) : undefined // The user should be able to add an asset that is on the untrusted list. - const exists = !isUntrustedAsset(cachedAsset) || false + const exists = cachedAsset ? !isUntrustedAsset(cachedAsset) : false return { ...assetData, From dce74a933c98c87c2db12913b20974e981720bf1 Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Thu, 22 Jun 2023 10:46:16 +0200 Subject: [PATCH 03/16] Refactor for adding an asset from an unverified list - Simplification of the update logic when an asset already exists. - Create a function to check whether the asset is verified. - Addition of comments. --- background/main.ts | 7 +-- .../redux-slices/selectors/0xSwapSelectors.ts | 4 +- background/redux-slices/utils/asset-utils.ts | 28 +++++++---- background/services/indexing/index.ts | 50 +++++++++++++------ ui/pages/Send.tsx | 4 +- ui/utils/swap.ts | 4 +- 6 files changed, 61 insertions(+), 36 deletions(-) diff --git a/background/main.ts b/background/main.ts index f2a6148769..2737d228fd 100644 --- a/background/main.ts +++ b/background/main.ts @@ -188,9 +188,9 @@ import { OneTimeAnalyticsEvent, } from "./lib/posthog" import { + isVerifiedAsset, isBuiltInNetworkBaseAsset, isSameAsset, - isUntrustedAsset, } from "./redux-slices/utils/asset-utils" import { getPricePoint, getTokenPrices } from "./lib/prices" import { DismissableItem } from "./services/preferences" @@ -1895,8 +1895,9 @@ export default class Main extends BaseService { const mainCurrencyAmount = convertedAssetAmount ? assetAmountToDesiredDecimals(convertedAssetAmount, 2) : undefined - // The user should be able to add an asset that is on the untrusted list. - const exists = cachedAsset ? !isUntrustedAsset(cachedAsset) : false + // Existing assets are those that are verified by default or by the user. + // This check allows the user to add an asset that is on the unverified list. + const exists = cachedAsset ? isVerifiedAsset(cachedAsset) : false return { ...assetData, diff --git a/background/redux-slices/selectors/0xSwapSelectors.ts b/background/redux-slices/selectors/0xSwapSelectors.ts index 0c69e2a40e..ad57a393a2 100644 --- a/background/redux-slices/selectors/0xSwapSelectors.ts +++ b/background/redux-slices/selectors/0xSwapSelectors.ts @@ -3,7 +3,7 @@ import { selectCurrentNetwork } from "./uiSelectors" import { SwappableAsset, isSmartContractFungibleAsset } from "../../assets" import { sameNetwork } from "../../networks" import { - canBeUsedForTransaction, + isVerifiedAsset, isBuiltInNetworkBaseAsset, } from "../utils/asset-utils" import { RootState } from ".." @@ -29,7 +29,7 @@ export const selectSwapBuyAssets = createSelector( ): asset is SwappableAsset & { recentPrices: SingleAssetState["recentPrices"] } => { - if (!canBeUsedForTransaction(asset)) { + if (!isVerifiedAsset(asset)) { return false } if (isSmartContractFungibleAsset(asset)) { diff --git a/background/redux-slices/utils/asset-utils.ts b/background/redux-slices/utils/asset-utils.ts index 266d02f858..8413459832 100644 --- a/background/redux-slices/utils/asset-utils.ts +++ b/background/redux-slices/utils/asset-utils.ts @@ -377,6 +377,23 @@ export function isUnverifiedAssetByUser(asset: AnyAsset | undefined): boolean { return false } +/** + * Check if an asset is verified. + * The asset can be verified by us when it is trusted by default. + * Untrusted asset can be manually verified by the user. + * + * Only verified 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 isVerifiedAsset(asset: AnyAsset): boolean { + if (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET)) { + return true + } + return isUntrustedAsset(asset) ? !isUnverifiedAssetByUser(asset) : true +} + type AssetType = "base" | "erc20" export type AssetID = `${AssetType}/${string}` @@ -391,17 +408,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/services/indexing/index.ts b/background/services/indexing/index.ts index 8294963fd9..f6b2f2302f 100644 --- a/background/services/indexing/index.ts +++ b/background/services/indexing/index.ts @@ -48,7 +48,7 @@ import { normalizeEVMAddress, sameEVMAddress, } from "../../lib/utils" -import { isUntrustedAsset } from "../../redux-slices/utils/asset-utils" +import { isVerifiedAsset } 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. @@ -99,6 +99,35 @@ const getActiveAssetsByAddressForNetwork = ( return getAssetsByAddress(networkActiveAssets) } +function shouldRefreshKnownAsset( + asset: SmartContractFungibleAsset, + metadata: { + discoveryTxHash?: { + [address: HexString]: HexString + } + verified?: boolean + } +): boolean { + const newDiscoveryTxHash = metadata?.discoveryTxHash + const addressForDiscoveryTxHash = newDiscoveryTxHash + ? Object.keys(newDiscoveryTxHash)[0] + : undefined + const existingDiscoveryTxHash = addressForDiscoveryTxHash + ? asset.metadata?.discoveryTxHash?.[addressForDiscoveryTxHash] + : undefined + + // If a known asset does not yet have a tx detection hash, update it. + const noExistingDiscoveryTxHash = !existingDiscoveryTxHash + + // Refresh a known unverified asset if it has been manually imported. + // This check allows the user to add an asset from the unverified list. + const isManuallyImported = asset.metadata?.verified + const allowUpdateUnverifiedAsset = + !isVerifiedAsset(asset) && isManuallyImported + + return allowUpdateUnverifiedAsset || noExistingDiscoveryTxHash +} + /** * IndexingService is responsible for pulling and maintaining all application- * level "indexing" data — things like fungible token balances and NFTs, as well @@ -700,21 +729,10 @@ export default class IndexingService extends BaseService { network, normalizedAddress ) - // The user should be able to add an asset that is on the untrusted list. - if (knownAsset && !isUntrustedAsset(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 && !shouldRefreshKnownAsset(knownAsset, metadata)) { + await this.addAssetToTrack(knownAsset) + return knownAsset } let customAsset = await this.db.getCustomAssetByAddressAndNetwork( diff --git a/ui/pages/Send.tsx b/ui/pages/Send.tsx index 0af3e4158b..8d6b3e90cb 100644 --- a/ui/pages/Send.tsx +++ b/ui/pages/Send.tsx @@ -28,7 +28,7 @@ import { } from "@tallyho/tally-background/redux-slices/assets" import { CompleteAssetAmount } from "@tallyho/tally-background/redux-slices/accounts" import { - canBeUsedForTransaction, + isVerifiedAsset, enrichAssetAmountWithMainCurrencyValues, } from "@tallyho/tally-background/redux-slices/utils/asset-utils" import { useHistory, useLocation } from "react-router-dom" @@ -112,7 +112,7 @@ export default function Send(): ReactElement { (assetAmount): assetAmount is CompleteAssetAmount => isFungibleAssetAmount(assetAmount) && assetAmount.decimalAmount > 0 && - canBeUsedForTransaction(assetAmount.asset) + isVerifiedAsset(assetAmount.asset) ) const assetPricePoint = useBackgroundSelector((state) => selectAssetPricePoint(state.assets, selectedAsset, mainCurrencySymbol) diff --git a/ui/utils/swap.ts b/ui/utils/swap.ts index 9235274f9f..80572a6410 100644 --- a/ui/utils/swap.ts +++ b/ui/utils/swap.ts @@ -18,7 +18,7 @@ import { debounce, DebouncedFunc } from "lodash" import { useState, useRef, useCallback } from "react" import { CompleteAssetAmount } from "@tallyho/tally-background/redux-slices/accounts" import { - canBeUsedForTransaction, + isVerifiedAsset, isSameAsset, } from "@tallyho/tally-background/redux-slices/utils/asset-utils" import { useBackgroundDispatch, useBackgroundSelector } from "../hooks" @@ -271,7 +271,7 @@ export function getOwnedSellAssetAmounts( (isSmartContractFungibleAsset(assetAmount.asset) || assetAmount.asset.symbol === currentNetwork.baseAsset.symbol) && assetAmount.decimalAmount > 0 && - canBeUsedForTransaction(assetAmount.asset) + isVerifiedAsset(assetAmount.asset) ) ?? [] ) } From ee820f5b215d12020e5266f1caac61fb779980b5 Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Thu, 22 Jun 2023 11:55:34 +0200 Subject: [PATCH 04/16] Fix checking that asset is manually imported --- background/services/indexing/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/background/services/indexing/index.ts b/background/services/indexing/index.ts index f6b2f2302f..48775ba3a2 100644 --- a/background/services/indexing/index.ts +++ b/background/services/indexing/index.ts @@ -121,7 +121,7 @@ function shouldRefreshKnownAsset( // Refresh a known unverified asset if it has been manually imported. // This check allows the user to add an asset from the unverified list. - const isManuallyImported = asset.metadata?.verified + const isManuallyImported = metadata?.verified const allowUpdateUnverifiedAsset = !isVerifiedAsset(asset) && isManuallyImported From 12dce071a8d13c52e555d4ea45a9f69b45be07de Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Fri, 23 Jun 2023 14:39:18 +0200 Subject: [PATCH 05/16] Add basic tests and fix issues for adding an asset --- background/services/indexing/index.ts | 58 +-------- background/services/indexing/utils/index.ts | 70 +++++++++++ .../indexing/utils/tests/index.unit.test.ts | 115 ++++++++++++++++++ 3 files changed, 190 insertions(+), 53 deletions(-) create mode 100644 background/services/indexing/utils/index.ts create mode 100644 background/services/indexing/utils/tests/index.unit.test.ts diff --git a/background/services/indexing/index.ts b/background/services/indexing/index.ts index 48775ba3a2..9f32f10c0b 100644 --- a/background/services/indexing/index.ts +++ b/background/services/indexing/index.ts @@ -48,7 +48,11 @@ import { normalizeEVMAddress, sameEVMAddress, } from "../../lib/utils" -import { isVerifiedAsset } from "../../redux-slices/utils/asset-utils" +import { + getActiveAssetsByAddressForNetwork, + getAssetsByAddress, + shouldRefreshKnownAsset, +} from "./utils" // Transactions seen within this many blocks of the chain tip will schedule a // token refresh sooner than the standard rate. @@ -76,58 +80,6 @@ interface Events extends ServiceLifecycleEvents { removeAssetData: SmartContractFungibleAsset } -const getAssetsByAddress = (assets: SmartContractFungibleAsset[]) => { - const activeAssetsByAddress = assets.reduce((agg, t) => { - const newAgg = { - ...agg, - } - newAgg[t.contractAddress.toLowerCase()] = t - return newAgg - }, {} as { [address: string]: SmartContractFungibleAsset }) - - return activeAssetsByAddress -} - -const getActiveAssetsByAddressForNetwork = ( - network: EVMNetwork, - activeAssetsToTrack: SmartContractFungibleAsset[] -) => { - const networkActiveAssets = activeAssetsToTrack.filter( - (asset) => asset.homeNetwork.chainID === network.chainID - ) - - return getAssetsByAddress(networkActiveAssets) -} - -function shouldRefreshKnownAsset( - asset: SmartContractFungibleAsset, - metadata: { - discoveryTxHash?: { - [address: HexString]: HexString - } - verified?: boolean - } -): boolean { - const newDiscoveryTxHash = metadata?.discoveryTxHash - const addressForDiscoveryTxHash = newDiscoveryTxHash - ? Object.keys(newDiscoveryTxHash)[0] - : undefined - const existingDiscoveryTxHash = addressForDiscoveryTxHash - ? asset.metadata?.discoveryTxHash?.[addressForDiscoveryTxHash] - : undefined - - // If a known asset does not yet have a tx detection hash, update it. - const noExistingDiscoveryTxHash = !existingDiscoveryTxHash - - // Refresh a known unverified asset if it has been manually imported. - // This check allows the user to add an asset from the unverified list. - const isManuallyImported = metadata?.verified - const allowUpdateUnverifiedAsset = - !isVerifiedAsset(asset) && isManuallyImported - - return allowUpdateUnverifiedAsset || noExistingDiscoveryTxHash -} - /** * IndexingService is responsible for pulling and maintaining all application- * level "indexing" data — things like fungible token balances and NFTs, as well diff --git a/background/services/indexing/utils/index.ts b/background/services/indexing/utils/index.ts new file mode 100644 index 0000000000..bc7bfece17 --- /dev/null +++ b/background/services/indexing/utils/index.ts @@ -0,0 +1,70 @@ +import { SmartContractFungibleAsset } from "../../../assets" +import { EVMNetwork } from "../../../networks" +import { + isUntrustedAsset, + isVerifiedAsset, +} from "../../../redux-slices/utils/asset-utils" +import { HexString } from "../../../types" + +export const getAssetsByAddress = ( + assets: SmartContractFungibleAsset[] +): { + [address: string]: SmartContractFungibleAsset +} => { + const activeAssetsByAddress = assets.reduce((agg, t) => { + const newAgg = { + ...agg, + } + newAgg[t.contractAddress.toLowerCase()] = t + return newAgg + }, {} as { [address: string]: SmartContractFungibleAsset }) + + return activeAssetsByAddress +} + +export const getActiveAssetsByAddressForNetwork = ( + network: EVMNetwork, + activeAssetsToTrack: SmartContractFungibleAsset[] +): { + [address: string]: SmartContractFungibleAsset +} => { + const networkActiveAssets = activeAssetsToTrack.filter( + (asset) => asset.homeNetwork.chainID === network.chainID + ) + + return getAssetsByAddress(networkActiveAssets) +} + +export function shouldRefreshKnownAsset( + asset: SmartContractFungibleAsset, + metadata: { + discoveryTxHash?: { + [address: HexString]: HexString + } + verified?: boolean + } +): boolean { + const newDiscoveryTxHash = metadata?.discoveryTxHash + const addressForDiscoveryTxHash = newDiscoveryTxHash + ? Object.keys(newDiscoveryTxHash)[0] + : undefined + const existingDiscoveryTxHash = addressForDiscoveryTxHash + ? asset.metadata?.discoveryTxHash?.[addressForDiscoveryTxHash] + : undefined + const noExistingDiscoveryTxHash = !!existingDiscoveryTxHash + + // If the discovery tx hash is specified + // or if it does not already exists in the asset, do update the asset. + // However, this should only happen for untrusted assets. + const allowAddDiscoveryTxHash = + isUntrustedAsset(asset) && + (!!newDiscoveryTxHash || noExistingDiscoveryTxHash) + + // Refresh a known unverified asset if it has been manually imported. + // This check allows the user to add an asset from the unverified list. + const isManuallyImported = metadata?.verified + const allowVerifyAssetByManualImport = + !isVerifiedAsset(asset) && isManuallyImported + + return allowVerifyAssetByManualImport || allowAddDiscoveryTxHash +} diff --git a/background/services/indexing/utils/tests/index.unit.test.ts b/background/services/indexing/utils/tests/index.unit.test.ts new file mode 100644 index 0000000000..b8ae665735 --- /dev/null +++ b/background/services/indexing/utils/tests/index.unit.test.ts @@ -0,0 +1,115 @@ +import { shouldRefreshKnownAsset } from ".." +import { createSmartContractAsset } from "../../../../tests/factories" +import * as featureFlags from "../../../../features" + +const ADDRESS = "0x0000000000000000000000000000000000000000" +const DISCOVERY_TX_HASH = "0x0000000000000000000000000000000000000000" +const METADATA_TX = { + discoveryTxHash: { + [ADDRESS]: DISCOVERY_TX_HASH, + }, +} +const METADATA_VERIFIED = { + verified: true, +} + +const TRUSTED_ASSET = createSmartContractAsset() +const UNTRUSTED_ASSET = createSmartContractAsset({ + metadata: { tokenLists: [] }, +}) + +describe("IndexingService utils", () => { + describe("shouldRefreshKnownAsset", () => { + beforeAll(() => { + jest.spyOn(featureFlags, "isEnabled").mockImplementation(() => true) + }) + + it("Refresh the untrusted asset if manually imported", () => { + const shouldBeRefreshed = shouldRefreshKnownAsset( + UNTRUSTED_ASSET, + METADATA_VERIFIED + ) + + expect(shouldBeRefreshed).toBe(true) + }) + + it("Not refresh the untrusted asset if not manually imported", () => { + const shouldBeRefreshed = shouldRefreshKnownAsset(UNTRUSTED_ASSET, {}) + + expect(shouldBeRefreshed).toBe(false) + }) + + it("Not refresh the trusted asset if not manually imported", () => { + const shouldBeRefreshed = shouldRefreshKnownAsset(TRUSTED_ASSET, {}) + + expect(shouldBeRefreshed).toBe(false) + }) + + // This state is not quite possible in the app because the user is not able to manually import a trusted asset that exists. + it("Not refresh the trusted asset if manually imported", () => { + const shouldBeRefreshed = shouldRefreshKnownAsset( + TRUSTED_ASSET, + METADATA_VERIFIED + ) + + expect(shouldBeRefreshed).toBe(false) + }) + + it("Refresh the untrusted asset if it does not already have a discovered tx hash", () => { + const shouldBeRefreshed = shouldRefreshKnownAsset( + UNTRUSTED_ASSET, + METADATA_TX + ) + + expect(shouldBeRefreshed).toBe(true) + }) + + it("Not refresh the trusted asset if it does not already have a discovered tx hash", () => { + const shouldBeRefreshed = shouldRefreshKnownAsset( + TRUSTED_ASSET, + METADATA_TX + ) + + expect(shouldBeRefreshed).toBe(false) + }) + + it("Not refresh the untrusted asset if it does already have a discovered tx hash", () => { + const asset = { + ...UNTRUSTED_ASSET, + metadata: { ...UNTRUSTED_ASSET, ...METADATA_TX }, + } + const shouldBeRefreshed = shouldRefreshKnownAsset(asset, METADATA_TX) + + expect(shouldBeRefreshed).toBe(false) + }) + + it("Not refresh the trusted asset if it does already have a discovered tx hash", () => { + const asset = { + ...TRUSTED_ASSET, + metadata: { ...TRUSTED_ASSET, ...METADATA_TX }, + } + const shouldBeRefreshed = shouldRefreshKnownAsset(asset, METADATA_TX) + + expect(shouldBeRefreshed).toBe(false) + }) + + it("Not refresh the trusted asset if discovered tx hash is not specified", () => { + const asset = { + ...TRUSTED_ASSET, + metadata: { ...TRUSTED_ASSET, ...METADATA_TX }, + } + + expect(shouldRefreshKnownAsset(TRUSTED_ASSET, {})).toBe(false) + expect(shouldRefreshKnownAsset(asset, {})).toBe(false) + }) + it("Not refresh the untrusted asset if discovered tx hash is not specified", () => { + const asset = { + ...UNTRUSTED_ASSET, + metadata: { ...UNTRUSTED_ASSET, ...METADATA_TX }, + } + + expect(shouldRefreshKnownAsset(UNTRUSTED_ASSET, {})).toBe(false) + expect(shouldRefreshKnownAsset(asset, {})).toBe(false) + }) + }) +}) From 546f8c579bb0fe87ac9ffa7eb047f3be68acfa97 Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Fri, 23 Jun 2023 14:59:43 +0200 Subject: [PATCH 06/16] Fix for allow adding discovery tx hash for asset --- background/services/indexing/utils/index.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/background/services/indexing/utils/index.ts b/background/services/indexing/utils/index.ts index bc7bfece17..9f4044a7c6 100644 --- a/background/services/indexing/utils/index.ts +++ b/background/services/indexing/utils/index.ts @@ -51,14 +51,12 @@ export function shouldRefreshKnownAsset( const existingDiscoveryTxHash = addressForDiscoveryTxHash ? asset.metadata?.discoveryTxHash?.[addressForDiscoveryTxHash] : undefined - const noExistingDiscoveryTxHash = !!existingDiscoveryTxHash - // If the discovery tx hash is specified - // or if it does not already exists in the asset, do update the asset. - // However, this should only happen for untrusted assets. + // If the discovery tx hash is not specified + // or if it already exists in the asset, do not update the asset + // Additionally, discovery tx Hash should only be added for untrusted assets. const allowAddDiscoveryTxHash = - isUntrustedAsset(asset) && - (!!newDiscoveryTxHash || noExistingDiscoveryTxHash) + isUntrustedAsset(asset) && !(!newDiscoveryTxHash || existingDiscoveryTxHash) // Refresh a known unverified asset if it has been manually imported. // This check allows the user to add an asset from the unverified list. From c83cf0eb8dd5b6c93deaf6b4083524fa890491cf Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Tue, 4 Jul 2023 14:05:53 +0200 Subject: [PATCH 07/16] Minor simplification of logic --- background/main.ts | 8 +++---- .../redux-slices/selectors/0xSwapSelectors.ts | 22 ++++++------------- ui/pages/Settings/SettingsAddCustomAsset.tsx | 4 ++-- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/background/main.ts b/background/main.ts index 2737d228fd..199a0911e4 100644 --- a/background/main.ts +++ b/background/main.ts @@ -1864,7 +1864,7 @@ export default class Main extends BaseService { amount: bigint mainCurrencyAmount?: number balance: number - exists?: boolean + shouldDisplay?: boolean }> { const { network } = addressOnNetwork @@ -1895,9 +1895,9 @@ export default class Main extends BaseService { const mainCurrencyAmount = convertedAssetAmount ? assetAmountToDesiredDecimals(convertedAssetAmount, 2) : undefined - // Existing assets are those that are verified by default or by the user. + // The asset should be displayed in the regular list when that is verified by default or by the user. // This check allows the user to add an asset that is on the unverified list. - const exists = cachedAsset ? isVerifiedAsset(cachedAsset) : false + const shouldDisplay = cachedAsset && isVerifiedAsset(cachedAsset) return { ...assetData, @@ -1905,7 +1905,7 @@ export default class Main extends BaseService { utils.formatUnits(assetData.amount, assetData.asset.decimals) ), mainCurrencyAmount, - exists, + shouldDisplay, } } diff --git a/background/redux-slices/selectors/0xSwapSelectors.ts b/background/redux-slices/selectors/0xSwapSelectors.ts index ad57a393a2..8562289dcc 100644 --- a/background/redux-slices/selectors/0xSwapSelectors.ts +++ b/background/redux-slices/selectors/0xSwapSelectors.ts @@ -29,21 +29,13 @@ export const selectSwapBuyAssets = createSelector( ): asset is SwappableAsset & { recentPrices: SingleAssetState["recentPrices"] } => { - if (!isVerifiedAsset(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 ( + isVerifiedAsset(asset) && + // Only list assets for the current network. + (isBuiltInNetworkBaseAsset(asset, currentNetwork) || + (isSmartContractFungibleAsset(asset) && + sameNetwork(asset.homeNetwork, currentNetwork))) + ) } ) } diff --git a/ui/pages/Settings/SettingsAddCustomAsset.tsx b/ui/pages/Settings/SettingsAddCustomAsset.tsx index 6a0b7fb9f3..ca7ae84118 100644 --- a/ui/pages/Settings/SettingsAddCustomAsset.tsx +++ b/ui/pages/Settings/SettingsAddCustomAsset.tsx @@ -377,7 +377,7 @@ export default function SettingsAddCustomAsset(): ReactElement { !assetData || isLoadingAssetDetails || hasAssetDetailLoadError || - assetData.exists || + assetData.shouldDisplay || isImportingToken } isLoading={isLoadingAssetDetails || isImportingToken} @@ -385,7 +385,7 @@ export default function SettingsAddCustomAsset(): ReactElement { {t("submit")} - {assetData?.exists ? ( + {assetData?.shouldDisplay ? (
Date: Thu, 6 Jul 2023 08:54:09 +0200 Subject: [PATCH 08/16] Simplify the condition for `isVerifiedAsset` --- background/redux-slices/utils/asset-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/background/redux-slices/utils/asset-utils.ts b/background/redux-slices/utils/asset-utils.ts index 8413459832..39d3e66700 100644 --- a/background/redux-slices/utils/asset-utils.ts +++ b/background/redux-slices/utils/asset-utils.ts @@ -391,7 +391,7 @@ export function isVerifiedAsset(asset: AnyAsset): boolean { if (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET)) { return true } - return isUntrustedAsset(asset) ? !isUnverifiedAssetByUser(asset) : true + return isNetworkBaseAsset(asset) || !isUntrustedAsset(asset) } type AssetType = "base" | "erc20" From 58972755f5e27d7a63fc37e52dc7834569e2da1e Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Thu, 6 Jul 2023 11:43:41 +0200 Subject: [PATCH 09/16] Fix for incorrect conditions- unverified assets --- background/main.ts | 6 +-- .../redux-slices/selectors/0xSwapSelectors.ts | 7 ++- .../selectors/accountsSelectors.ts | 24 +++++---- background/redux-slices/utils/asset-utils.ts | 51 +++++++++---------- .../utils/tests/asset-utils.unit.test.ts | 16 +++--- background/services/indexing/utils/index.ts | 14 +++-- .../AssetListItem/CommonAssetListItem.tsx | 15 ++++-- .../Wallet/UnverifiedAsset/AssetWarning.tsx | 4 +- ui/pages/Send.tsx | 6 ++- ui/pages/SingleAsset.tsx | 48 +++++++++-------- ui/utils/swap.ts | 7 ++- 11 files changed, 109 insertions(+), 89 deletions(-) diff --git a/background/main.ts b/background/main.ts index 199a0911e4..e25c98c616 100644 --- a/background/main.ts +++ b/background/main.ts @@ -188,9 +188,9 @@ import { OneTimeAnalyticsEvent, } from "./lib/posthog" import { - isVerifiedAsset, isBuiltInNetworkBaseAsset, isSameAsset, + isVerifiedOrTrustedAsset, } from "./redux-slices/utils/asset-utils" import { getPricePoint, getTokenPrices } from "./lib/prices" import { DismissableItem } from "./services/preferences" @@ -1895,9 +1895,9 @@ export default class Main extends BaseService { const mainCurrencyAmount = convertedAssetAmount ? assetAmountToDesiredDecimals(convertedAssetAmount, 2) : undefined - // The asset should be displayed in the regular list when that is verified by default or by the user. + // The asset should be displayed in the regular list when that is trusted by default or verified by the user. // This check allows the user to add an asset that is on the unverified list. - const shouldDisplay = cachedAsset && isVerifiedAsset(cachedAsset) + const shouldDisplay = cachedAsset && isVerifiedOrTrustedAsset(cachedAsset) return { ...assetData, diff --git a/background/redux-slices/selectors/0xSwapSelectors.ts b/background/redux-slices/selectors/0xSwapSelectors.ts index 8562289dcc..545fc9074f 100644 --- 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 { - isVerifiedAsset, 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, @@ -30,7 +31,9 @@ export const selectSwapBuyAssets = createSelector( recentPrices: SingleAssetState["recentPrices"] } => { return ( - isVerifiedAsset(asset) && + // 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) && diff --git a/background/redux-slices/selectors/accountsSelectors.ts b/background/redux-slices/selectors/accountsSelectors.ts index ba4d0e01e9..18b01c7150 100644 --- 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, @@ -79,11 +79,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 = @@ -93,13 +93,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, } } @@ -189,13 +190,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 index 39d3e66700..f95adc013b 100644 --- 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,56 +341,54 @@ 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 !isTokenListAsset(asset) && !isNetworkBaseAsset(asset) } return false } /** - * 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. - * The asset can be verified by us when it is trusted by default. + * 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 verified assets can take part in wallet actions. + * 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 isVerifiedAsset(asset: AnyAsset): boolean { - if (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET)) { - return true +export function isVerifiedOrTrustedAsset(asset?: AnyAsset): boolean { + if (asset) { + return ( + isVerifiedAssetByUser(asset) || + isNetworkBaseAsset(asset) || + isTokenListAsset(asset) + ) } - return isNetworkBaseAsset(asset) || !isUntrustedAsset(asset) + return false } type AssetType = "base" | "erc20" diff --git a/background/redux-slices/utils/tests/asset-utils.unit.test.ts b/background/redux-slices/utils/tests/asset-utils.unit.test.ts index 712c8a6714..60f3291ac9 100644 --- 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,15 @@ 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() + expect(isVerifiedAssetByUser(ETH)).toBeFalsy() }) test("should return false if an asset is undefined", () => { - expect(isUnverifiedAssetByUser(undefined)).toBeFalsy() + expect(isVerifiedAssetByUser(undefined)).toBeFalsy() }) }) diff --git a/background/services/indexing/utils/index.ts b/background/services/indexing/utils/index.ts index 9f4044a7c6..896882afd0 100644 --- a/background/services/indexing/utils/index.ts +++ b/background/services/indexing/utils/index.ts @@ -2,7 +2,7 @@ import { SmartContractFungibleAsset } from "../../../assets" import { EVMNetwork } from "../../../networks" import { isUntrustedAsset, - isVerifiedAsset, + isVerifiedAssetByUser, } from "../../../redux-slices/utils/asset-utils" import { HexString } from "../../../types" @@ -44,6 +44,10 @@ export function shouldRefreshKnownAsset( verified?: boolean } ): boolean { + // The asset that is in a token list or is a network base asset should not be refreshed. + // They shouldn't have a discovery tx hash or any custom metadata. + if (!isUntrustedAsset(asset)) return false + const newDiscoveryTxHash = metadata?.discoveryTxHash const addressForDiscoveryTxHash = newDiscoveryTxHash ? Object.keys(newDiscoveryTxHash)[0] @@ -54,15 +58,15 @@ export function shouldRefreshKnownAsset( // If the discovery tx hash is not specified // or if it already exists in the asset, do not update the asset - // Additionally, discovery tx Hash should only be added for untrusted assets. - const allowAddDiscoveryTxHash = - isUntrustedAsset(asset) && !(!newDiscoveryTxHash || existingDiscoveryTxHash) + const allowAddDiscoveryTxHash = !( + !newDiscoveryTxHash || existingDiscoveryTxHash + ) // Refresh a known unverified asset if it has been manually imported. // This check allows the user to add an asset from the unverified list. const isManuallyImported = metadata?.verified const allowVerifyAssetByManualImport = - !isVerifiedAsset(asset) && isManuallyImported + !isVerifiedAssetByUser(asset) && isManuallyImported return allowVerifyAssetByManualImport || allowAddDiscoveryTxHash } diff --git a/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx b/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx index a8354f3b15..7bdddc8c9e 100644 --- a/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx +++ b/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx @@ -3,7 +3,10 @@ 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, +} 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 +55,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) => { event.preventDefault() @@ -90,7 +94,7 @@ export default function CommonAssetListItem( { // @TODO don't fetch prices for unverified assets in the first place // Only show prices for verified assets - isUnverified || + isUntrusted || (initializationLoadingTimeExpired && isMissingLocalizedUserValue) ? ( <> @@ -109,7 +113,8 @@ export default function CommonAssetListItem(
<> {isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) && - isUnverified ? ( + isUntrusted && + !isVerified ? ( {!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) && - isUnverified && ( + isUntrusted && ( => isFungibleAssetAmount(assetAmount) && assetAmount.decimalAmount > 0 && - isVerifiedAsset(assetAmount.asset) + // When the flag is disabled all assets can be sent and swapped + (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) || + isVerifiedOrTrustedAsset(assetAmount.asset)) ) const assetPricePoint = useBackgroundSelector((state) => selectAssetPricePoint(state.assets, selectedAsset, mainCurrencySymbol) diff --git a/ui/pages/SingleAsset.tsx b/ui/pages/SingleAsset.tsx index 2c8777a40b..7cd93c3f55 100644 --- a/ui/pages/SingleAsset.tsx +++ b/ui/pages/SingleAsset.tsx @@ -20,7 +20,8 @@ import { } from "@tallyho/tally-background/constants" import { isUntrustedAsset, - isUnverifiedAssetByUser, + isVerifiedAssetByUser, + isVerifiedOrTrustedAsset, } from "@tallyho/tally-background/redux-slices/utils/asset-utils" import { FeatureFlags, isEnabled } from "@tallyho/tally-background/features" import { useBackgroundSelector } from "../hooks" @@ -99,12 +100,13 @@ export default function SingleAsset(): ReactElement { } const isUntrusted = isUntrustedAsset(asset) - const isUnverifiedByUser = isUnverifiedAssetByUser(asset) + const isVerifiedOrTrusted = isVerifiedOrTrustedAsset(asset) + const isVerified = isVerifiedAssetByUser(asset) const [warnedAsset, setWarnedAsset] = useState(null) const showActionButtons = isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) - ? !isUnverifiedByUser + ? isVerifiedOrTrusted : true return ( @@ -120,7 +122,7 @@ export default function SingleAsset(): ReactElement { {isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) && ( <> {isUntrusted && - !isUnverifiedByUser && + isVerified && asset && isSmartContractFungibleAsset(asset) && ( {isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) && ( <> - {isUnverifiedByUser && isSmartContractFungibleAsset(asset) && ( -
- setWarnedAsset(asset)} - /> -
- + setWarnedAsset(asset)} - > - {t("assets.verifyAsset")} - + /> +
+ setWarnedAsset(asset)} + > + {t("assets.verifyAsset")} + +
-
- )} + )} )} diff --git a/ui/utils/swap.ts b/ui/utils/swap.ts index 80572a6410..efc0f6cc87 100644 --- a/ui/utils/swap.ts +++ b/ui/utils/swap.ts @@ -18,9 +18,10 @@ import { debounce, DebouncedFunc } from "lodash" import { useState, useRef, useCallback } from "react" import { CompleteAssetAmount } from "@tallyho/tally-background/redux-slices/accounts" import { - isVerifiedAsset, isSameAsset, + isVerifiedOrTrustedAsset, } from "@tallyho/tally-background/redux-slices/utils/asset-utils" +import { FeatureFlags, isEnabled } from "@tallyho/tally-background/features" import { useBackgroundDispatch, useBackgroundSelector } from "../hooks" import { useValueRef, useIsMounted, useSetState } from "../hooks/react-hooks" @@ -271,7 +272,9 @@ export function getOwnedSellAssetAmounts( (isSmartContractFungibleAsset(assetAmount.asset) || assetAmount.asset.symbol === currentNetwork.baseAsset.symbol) && assetAmount.decimalAmount > 0 && - isVerifiedAsset(assetAmount.asset) + // When the flag is disabled all assets can be sent and swapped + (!isEnabled(FeatureFlags.SUPPORT_UNVERIFIED_ASSET) || + isVerifiedOrTrustedAsset(assetAmount.asset)) ) ?? [] ) } From c77920a031d8aaf7632c4f3ba07ddcf1804d5b9c Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Thu, 6 Jul 2023 11:55:42 +0200 Subject: [PATCH 10/16] Fix the issue for dust warning during adding token When a user adds an unverified asset manually it should not bring up a dust warning. This token should be displayed in the list even is dust. --- background/main.ts | 2 ++ ui/pages/Settings/SettingsAddCustomAsset.tsx | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/background/main.ts b/background/main.ts index e25c98c616..656f8e39e6 100644 --- a/background/main.ts +++ b/background/main.ts @@ -1865,6 +1865,7 @@ export default class Main extends BaseService { mainCurrencyAmount?: number balance: number shouldDisplay?: boolean + exists?: boolean }> { const { network } = addressOnNetwork @@ -1906,6 +1907,7 @@ export default class Main extends BaseService { ), mainCurrencyAmount, shouldDisplay, + exists: !!cachedAsset, } } diff --git a/ui/pages/Settings/SettingsAddCustomAsset.tsx b/ui/pages/Settings/SettingsAddCustomAsset.tsx index ca7ae84118..5cbf37c6f6 100644 --- a/ui/pages/Settings/SettingsAddCustomAsset.tsx +++ b/ui/pages/Settings/SettingsAddCustomAsset.tsx @@ -178,11 +178,14 @@ export default function SettingsAddCustomAsset(): ReactElement { } const hideDustEnabled = useBackgroundSelector(selectHideDust) - const showWarningAboutDust = - hideDustEnabled && + const isDust = assetData?.mainCurrencyAmount !== 0 && assetData?.mainCurrencyAmount !== undefined && assetData?.mainCurrencyAmount < userValueDustThreshold + // The dust warning should be displayed when the asset does not yet exist in the wallet. + // Unverified assets are sometimes hidden by the user. + // After they have been added manually, they will be displayed normally despite being dust. + const showWarningAboutDust = hideDustEnabled && isDust && !assetData?.exists const warningOptions = { amount: userValueDustThreshold, From 0db98407e758cbce5a2d51f648f39d613464a7b8 Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Thu, 6 Jul 2023 15:29:07 +0200 Subject: [PATCH 11/16] Minor refactoring for importing unverified tokens - Minor changes to the comments - Removal of the `discoveryTxHash` refresh logic. It is no longer needed. The change was to update the `discoveryTxHash` after the data migration, where `discoveryTxHash` was removed for all custom assets. This also resulted in the wrong adding of a `discoveryTxHash` for the asset in the token list. The change solves this issue. --- background/services/indexing/index.ts | 9 +- background/services/indexing/utils/index.ts | 37 ++------ .../indexing/utils/tests/index.unit.test.ts | 89 +++---------------- .../AssetListItem/CommonAssetListItem.tsx | 4 +- ui/pages/Settings/SettingsAddCustomAsset.tsx | 2 +- 5 files changed, 29 insertions(+), 112 deletions(-) diff --git a/background/services/indexing/index.ts b/background/services/indexing/index.ts index 9f32f10c0b..aecadfffe0 100644 --- a/background/services/indexing/index.ts +++ b/background/services/indexing/index.ts @@ -49,9 +49,9 @@ import { sameEVMAddress, } from "../../lib/utils" import { + allowVerifyUntrustedAssetByManualImport, getActiveAssetsByAddressForNetwork, getAssetsByAddress, - shouldRefreshKnownAsset, } from "./utils" // Transactions seen within this many blocks of the chain tip will schedule a @@ -682,7 +682,12 @@ export default class IndexingService extends BaseService { normalizedAddress ) - if (knownAsset && !shouldRefreshKnownAsset(knownAsset, metadata)) { + 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. + !allowVerifyUntrustedAssetByManualImport(knownAsset, metadata?.verified) + ) { await this.addAssetToTrack(knownAsset) return knownAsset } diff --git a/background/services/indexing/utils/index.ts b/background/services/indexing/utils/index.ts index 896882afd0..0d938cd832 100644 --- a/background/services/indexing/utils/index.ts +++ b/background/services/indexing/utils/index.ts @@ -35,38 +35,13 @@ export const getActiveAssetsByAddressForNetwork = ( return getAssetsByAddress(networkActiveAssets) } -export function shouldRefreshKnownAsset( +export function allowVerifyUntrustedAssetByManualImport( asset: SmartContractFungibleAsset, - metadata: { - discoveryTxHash?: { - [address: HexString]: HexString - } - verified?: boolean - } + verified?: boolean ): boolean { - // The asset that is in a token list or is a network base asset should not be refreshed. - // They shouldn't have a discovery tx hash or any custom metadata. - if (!isUntrustedAsset(asset)) return false - - const newDiscoveryTxHash = metadata?.discoveryTxHash - const addressForDiscoveryTxHash = newDiscoveryTxHash - ? Object.keys(newDiscoveryTxHash)[0] - : undefined - const existingDiscoveryTxHash = addressForDiscoveryTxHash - ? asset.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 - const allowAddDiscoveryTxHash = !( - !newDiscoveryTxHash || existingDiscoveryTxHash - ) - - // Refresh a known unverified asset if it has been manually imported. - // This check allows the user to add an asset from the unverified list. - const isManuallyImported = metadata?.verified - const allowVerifyAssetByManualImport = - !isVerifiedAssetByUser(asset) && isManuallyImported + if (isUntrustedAsset(asset) && !isVerifiedAssetByUser(asset)) { + return !!verified + } - return allowVerifyAssetByManualImport || allowAddDiscoveryTxHash + return false } diff --git a/background/services/indexing/utils/tests/index.unit.test.ts b/background/services/indexing/utils/tests/index.unit.test.ts index b8ae665735..42361e0e94 100644 --- a/background/services/indexing/utils/tests/index.unit.test.ts +++ b/background/services/indexing/utils/tests/index.unit.test.ts @@ -1,115 +1,52 @@ -import { shouldRefreshKnownAsset } from ".." +import { allowVerifyUntrustedAssetByManualImport } from ".." import { createSmartContractAsset } from "../../../../tests/factories" import * as featureFlags from "../../../../features" -const ADDRESS = "0x0000000000000000000000000000000000000000" -const DISCOVERY_TX_HASH = "0x0000000000000000000000000000000000000000" -const METADATA_TX = { - discoveryTxHash: { - [ADDRESS]: DISCOVERY_TX_HASH, - }, -} -const METADATA_VERIFIED = { - verified: true, -} - const TRUSTED_ASSET = createSmartContractAsset() const UNTRUSTED_ASSET = createSmartContractAsset({ metadata: { tokenLists: [] }, }) describe("IndexingService utils", () => { - describe("shouldRefreshKnownAsset", () => { + describe("allowVerifyUntrustedAssetByManualImport", () => { beforeAll(() => { jest.spyOn(featureFlags, "isEnabled").mockImplementation(() => true) }) it("Refresh the untrusted asset if manually imported", () => { - const shouldBeRefreshed = shouldRefreshKnownAsset( + const shouldBeRefreshed = allowVerifyUntrustedAssetByManualImport( UNTRUSTED_ASSET, - METADATA_VERIFIED + true ) expect(shouldBeRefreshed).toBe(true) }) it("Not refresh the untrusted asset if not manually imported", () => { - const shouldBeRefreshed = shouldRefreshKnownAsset(UNTRUSTED_ASSET, {}) + const shouldBeRefreshed = allowVerifyUntrustedAssetByManualImport( + UNTRUSTED_ASSET, + false + ) expect(shouldBeRefreshed).toBe(false) }) it("Not refresh the trusted asset if not manually imported", () => { - const shouldBeRefreshed = shouldRefreshKnownAsset(TRUSTED_ASSET, {}) - - expect(shouldBeRefreshed).toBe(false) - }) - - // This state is not quite possible in the app because the user is not able to manually import a trusted asset that exists. - it("Not refresh the trusted asset if manually imported", () => { - const shouldBeRefreshed = shouldRefreshKnownAsset( + const shouldBeRefreshed = allowVerifyUntrustedAssetByManualImport( TRUSTED_ASSET, - METADATA_VERIFIED + false ) expect(shouldBeRefreshed).toBe(false) }) - it("Refresh the untrusted asset if it does not already have a discovered tx hash", () => { - const shouldBeRefreshed = shouldRefreshKnownAsset( - UNTRUSTED_ASSET, - METADATA_TX - ) - - expect(shouldBeRefreshed).toBe(true) - }) - - it("Not refresh the trusted asset if it does not already have a discovered tx hash", () => { - const shouldBeRefreshed = shouldRefreshKnownAsset( + it("Not refresh the trusted asset if manually imported", () => { + const shouldBeRefreshed = allowVerifyUntrustedAssetByManualImport( TRUSTED_ASSET, - METADATA_TX + true ) expect(shouldBeRefreshed).toBe(false) }) - - it("Not refresh the untrusted asset if it does already have a discovered tx hash", () => { - const asset = { - ...UNTRUSTED_ASSET, - metadata: { ...UNTRUSTED_ASSET, ...METADATA_TX }, - } - const shouldBeRefreshed = shouldRefreshKnownAsset(asset, METADATA_TX) - - expect(shouldBeRefreshed).toBe(false) - }) - - it("Not refresh the trusted asset if it does already have a discovered tx hash", () => { - const asset = { - ...TRUSTED_ASSET, - metadata: { ...TRUSTED_ASSET, ...METADATA_TX }, - } - const shouldBeRefreshed = shouldRefreshKnownAsset(asset, METADATA_TX) - - expect(shouldBeRefreshed).toBe(false) - }) - - it("Not refresh the trusted asset if discovered tx hash is not specified", () => { - const asset = { - ...TRUSTED_ASSET, - metadata: { ...TRUSTED_ASSET, ...METADATA_TX }, - } - - expect(shouldRefreshKnownAsset(TRUSTED_ASSET, {})).toBe(false) - expect(shouldRefreshKnownAsset(asset, {})).toBe(false) - }) - it("Not refresh the untrusted asset if discovered tx hash is not specified", () => { - const asset = { - ...UNTRUSTED_ASSET, - metadata: { ...UNTRUSTED_ASSET, ...METADATA_TX }, - } - - expect(shouldRefreshKnownAsset(UNTRUSTED_ASSET, {})).toBe(false) - expect(shouldRefreshKnownAsset(asset, {})).toBe(false) - }) }) }) diff --git a/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx b/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx index 7bdddc8c9e..b625b7df3b 100644 --- a/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx +++ b/ui/components/Wallet/AssetListItem/CommonAssetListItem.tsx @@ -92,8 +92,8 @@ export default function CommonAssetListItem(
{ - // @TODO don't fetch prices for unverified assets in the first place - // Only show prices for verified assets + // @TODO don't fetch prices for untrusted assets in the first place + // Only show prices for trusted assets isUntrusted || (initializationLoadingTimeExpired && isMissingLocalizedUserValue) ? ( diff --git a/ui/pages/Settings/SettingsAddCustomAsset.tsx b/ui/pages/Settings/SettingsAddCustomAsset.tsx index 5cbf37c6f6..ba6dffdf79 100644 --- a/ui/pages/Settings/SettingsAddCustomAsset.tsx +++ b/ui/pages/Settings/SettingsAddCustomAsset.tsx @@ -182,9 +182,9 @@ export default function SettingsAddCustomAsset(): ReactElement { assetData?.mainCurrencyAmount !== 0 && assetData?.mainCurrencyAmount !== undefined && assetData?.mainCurrencyAmount < userValueDustThreshold - // The dust warning should be displayed when the asset does not yet exist in the wallet. // Unverified assets are sometimes hidden by the user. // After they have been added manually, they will be displayed normally despite being dust. + // Therefore, the dust warning should be displayed when the asset does not yet exist in the wallet. const showWarningAboutDust = hideDustEnabled && isDust && !assetData?.exists const warningOptions = { From 14414f41aa3604df3ae46b4e9bcdc435ab1b3e25 Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Thu, 6 Jul 2023 15:34:58 +0200 Subject: [PATCH 12/16] Fix issue for lint --- background/services/indexing/utils/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/background/services/indexing/utils/index.ts b/background/services/indexing/utils/index.ts index 0d938cd832..2fb99d79fb 100644 --- a/background/services/indexing/utils/index.ts +++ b/background/services/indexing/utils/index.ts @@ -4,7 +4,6 @@ import { isUntrustedAsset, isVerifiedAssetByUser, } from "../../../redux-slices/utils/asset-utils" -import { HexString } from "../../../types" export const getAssetsByAddress = ( assets: SmartContractFungibleAsset[] From b0969f4a52936c7284201523a0e0e4ee1c07479b Mon Sep 17 00:00:00 2001 From: Karolina Kosiorowska Date: Fri, 7 Jul 2023 09:14:57 +0200 Subject: [PATCH 13/16] Fix the issue with a warning about dust After manually adding a token from the list of unverified assets, it will be displayed normally, even though it is dust. Therefore, the dust warning should be displayed when the asset does not yet exist in the wallet. --- background/main.ts | 6 --- ui/pages/Settings/SettingsAddCustomAsset.tsx | 52 +++++++++++--------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/background/main.ts b/background/main.ts index 656f8e39e6..cdc1bfc8df 100644 --- a/background/main.ts +++ b/background/main.ts @@ -190,7 +190,6 @@ import { import { isBuiltInNetworkBaseAsset, isSameAsset, - isVerifiedOrTrustedAsset, } from "./redux-slices/utils/asset-utils" import { getPricePoint, getTokenPrices } from "./lib/prices" import { DismissableItem } from "./services/preferences" @@ -1864,7 +1863,6 @@ export default class Main extends BaseService { amount: bigint mainCurrencyAmount?: number balance: number - shouldDisplay?: boolean exists?: boolean }> { const { network } = addressOnNetwork @@ -1896,9 +1894,6 @@ export default class Main extends BaseService { const mainCurrencyAmount = convertedAssetAmount ? assetAmountToDesiredDecimals(convertedAssetAmount, 2) : undefined - // The asset should be displayed in the regular list when that is trusted by default or verified by the user. - // This check allows the user to add an asset that is on the unverified list. - const shouldDisplay = cachedAsset && isVerifiedOrTrustedAsset(cachedAsset) return { ...assetData, @@ -1906,7 +1901,6 @@ export default class Main extends BaseService { utils.formatUnits(assetData.amount, assetData.asset.decimals) ), mainCurrencyAmount, - shouldDisplay, exists: !!cachedAsset, } } diff --git a/ui/pages/Settings/SettingsAddCustomAsset.tsx b/ui/pages/Settings/SettingsAddCustomAsset.tsx index ba6dffdf79..3fa554668c 100644 --- a/ui/pages/Settings/SettingsAddCustomAsset.tsx +++ b/ui/pages/Settings/SettingsAddCustomAsset.tsx @@ -24,6 +24,7 @@ import { HexString } from "@tallyho/tally-background/types" import React, { FormEventHandler, ReactElement, useRef, useState } from "react" import { Trans, useTranslation } from "react-i18next" import { useHistory } from "react-router-dom" +import { isVerifiedOrTrustedAsset } from "@tallyho/tally-background/redux-slices/utils/asset-utils" import SharedAssetIcon from "../../components/Shared/SharedAssetIcon" import SharedButton from "../../components/Shared/SharedButton" import SharedIcon from "../../components/Shared/SharedIcon" @@ -178,14 +179,11 @@ export default function SettingsAddCustomAsset(): ReactElement { } const hideDustEnabled = useBackgroundSelector(selectHideDust) - const isDust = + const showWarningAboutDust = + hideDustEnabled && assetData?.mainCurrencyAmount !== 0 && assetData?.mainCurrencyAmount !== undefined && assetData?.mainCurrencyAmount < userValueDustThreshold - // Unverified assets are sometimes hidden by the user. - // After they have been added manually, they will be displayed normally despite being dust. - // Therefore, the dust warning should be displayed when the asset does not yet exist in the wallet. - const showWarningAboutDust = hideDustEnabled && isDust && !assetData?.exists const warningOptions = { amount: userValueDustThreshold, @@ -209,6 +207,11 @@ export default function SettingsAddCustomAsset(): ReactElement { return t("warning.alreadyExists.desc.visibility") } + // The asset should be displayed in the regular list when that is trusted by default or verified by the user. + // This check allows the user to add an asset that is on the unverified list. + const isVerifiedOrTrusted = isVerifiedOrTrustedAsset(assetData?.asset) + const shouldDisplayAsset = assetData?.exists && isVerifiedOrTrusted + return (
{t(`title`)} @@ -380,7 +383,7 @@ export default function SettingsAddCustomAsset(): ReactElement { !assetData || isLoadingAssetDetails || hasAssetDetailLoadError || - assetData.shouldDisplay || + shouldDisplayAsset || isImportingToken } isLoading={isLoadingAssetDetails || isImportingToken} @@ -388,7 +391,7 @@ export default function SettingsAddCustomAsset(): ReactElement { {t("submit")}
- {assetData?.shouldDisplay ? ( + {shouldDisplayAsset && (
{renderWarningText()}
- ) : ( - <> - {showWarningAboutDust && ( -
- -
-
- {t("warning.dust.title", warningOptions)} -
+ )} + { + // After manually adding a token from the list of unverified assets, it will be displayed normally, even though it is dust. + // Therefore, the dust warning should be displayed when the asset does not yet exist in the wallet. + showWarningAboutDust && !assetData?.exists && ( +
+ +
+
+ {t("warning.dust.title", warningOptions)}
- )} - - )} +
+ ) + }