diff --git a/background/accounts.ts b/background/accounts.ts index 631465661a..5c821da8f7 100644 --- a/background/accounts.ts +++ b/background/accounts.ts @@ -1,5 +1,5 @@ -import { AnyAssetAmount } from "./assets" -import { EVMNetwork } from "./networks" +import { AnyAssetAmount, SmartContractFungibleAsset } from "./assets" +import { EVMNetwork, NetworkBaseAsset } from "./networks" import { HexString } from "./types" /** @@ -15,7 +15,7 @@ export type AccountBalance = { /** * The measured balance and the asset in which it's denominated. */ - assetAmount: AnyAssetAmount + assetAmount: AnyAssetAmount /** * The network on which the account balance was measured. */ diff --git a/background/main.ts b/background/main.ts index 52dbfd9b69..cdc1bfc8df 100644 --- a/background/main.ts +++ b/background/main.ts @@ -159,7 +159,6 @@ import { assetAmountToDesiredDecimals, convertAssetAmountViaPricePoint, isSmartContractFungibleAsset, - SmartContractAsset, SmartContractFungibleAsset, } from "./assets" import { FeatureFlags, isEnabled } from "./features" @@ -188,7 +187,10 @@ import { isOneTimeAnalyticsEvent, OneTimeAnalyticsEvent, } from "./lib/posthog" -import { isBuiltInNetworkBaseAsset } from "./redux-slices/utils/asset-utils" +import { + isBuiltInNetworkBaseAsset, + isSameAsset, +} from "./redux-slices/utils/asset-utils" import { getPricePoint, getTokenPrices } from "./lib/prices" import { DismissableItem } from "./services/preferences" @@ -1001,62 +1003,30 @@ export default class Main extends BaseService { const filteredBalancesToDispatch: AccountBalance[] = [] - const sortedBalances: AccountBalance[] = [] - balances .filter((balance) => { - const isSmartContract = - "contractAddress" in balance.assetAmount.asset - - if (!isSmartContract) { - sortedBalances.push(balance) - } - // Network base assets with smart contract addresses from some networks // e.g. Optimism, Polygon might have been retrieved through alchemy as // token balances but they should not be handled here as they would // not be correctly treated as base assets + return !isBuiltInNetworkBaseAsset( + balance.assetAmount.asset, + balance.network + ) + }) + .forEach((balance) => { + // TODO support multi-network assets + const balanceHasAnAlreadyTrackedAsset = assetsToTrack.some( + (tracked) => isSameAsset(tracked, balance.assetAmount.asset) + ) + if ( - isBuiltInNetworkBaseAsset( - balance.assetAmount.asset, - balance.network - ) + balance.assetAmount.amount > 0 || + balanceHasAnAlreadyTrackedAsset ) { - return false + filteredBalancesToDispatch.push(balance) } - - return isSmartContract - }) - // Sort verified last to prevent shadowing assets from token lists - // FIXME: Balances should not be indexed by symbol in redux - .sort((balance, otherBalance) => { - const asset = balance.assetAmount.asset as SmartContractAsset - const other = otherBalance.assetAmount.asset as SmartContractAsset - - return ( - (other.metadata?.tokenLists?.length ?? 0) - - (asset.metadata?.tokenLists?.length ?? 0) - ) }) - .forEach((balance) => sortedBalances.unshift(balance)) - - sortedBalances.forEach((balance) => { - // TODO support multi-network assets - const balanceHasAnAlreadyTrackedAsset = assetsToTrack.some( - (tracked) => - tracked.symbol === balance.assetAmount.asset.symbol && - isSmartContractFungibleAsset(balance.assetAmount.asset) && - normalizeEVMAddress(tracked.contractAddress) === - normalizeEVMAddress(balance.assetAmount.asset.contractAddress) - ) - - if ( - balance.assetAmount.amount > 0 || - balanceHasAnAlreadyTrackedAsset - ) { - filteredBalancesToDispatch.push(balance) - } - }) this.store.dispatch( updateAccountBalance({ diff --git a/background/redux-slices/accounts.ts b/background/redux-slices/accounts.ts index 0c7526d276..126532658c 100644 --- a/background/redux-slices/accounts.ts +++ b/background/redux-slices/accounts.ts @@ -13,7 +13,11 @@ import { AssetMainCurrencyAmount, AssetDecimalAmount, isBuiltInNetworkBaseAsset, + AssetID, + getAssetID, + isNetworkBaseAsset, isSameAsset, + isUntrustedAsset, } from "./utils/asset-utils" import { DomainName, HexString, URI } from "../types" import { normalizeEVMAddress, sameEVMAddress } from "../lib/utils" @@ -49,7 +53,7 @@ export type AccountData = { address: HexString network: Network balances: { - [assetSymbol: string]: AccountBalance + [assetID: AssetID]: AccountBalance } ens: { name?: DomainName @@ -180,13 +184,24 @@ function updateCombinedData(immerState: AccountState) { immerState.combinedData.assets = Object.values( combinedAccountBalances.reduce<{ - [symbol: string]: AnyAssetAmount + [assetID: string]: AnyAssetAmount }>((acc, combinedAssetAmount) => { - const assetSymbol = combinedAssetAmount.asset.symbol + const { asset } = combinedAssetAmount + /** + * Asset amounts can be aggregated if the asset is a base network asset + * or comes from a token list, e.g. ETH on Optimism, Mainnet + */ + const canBeAggregated = + isNetworkBaseAsset(asset) || !isUntrustedAsset(asset) + + const assetID = canBeAggregated + ? asset.symbol + : `${asset.homeNetwork.chainID}/${getAssetID(asset)}` + let { amount } = combinedAssetAmount - if (acc[assetSymbol]?.asset) { - const accAsset = acc[assetSymbol].asset + if (acc[assetID]?.asset) { + const accAsset = acc[assetID].asset const existingDecimals = isFungibleAsset(accAsset) ? accAsset.decimals : 0 @@ -199,13 +214,14 @@ function updateCombinedData(immerState: AccountState) { } } - if (acc[assetSymbol]) { - acc[assetSymbol].amount += amount + if (acc[assetID]) { + acc[assetID].amount += amount } else { - acc[assetSymbol] = { + acc[assetID] = { ...combinedAssetAmount, } } + return acc }, {}) ) @@ -297,7 +313,7 @@ const accountSlice = createSlice({ network, assetAmount: { asset }, } = updatedAccountBalance - const { symbol: updatedAssetSymbol } = asset + const assetID = getAssetID(asset) const normalizedAddress = normalizeEVMAddress(address) const existingAccountData = @@ -311,20 +327,19 @@ const accountSlice = createSlice({ if (existingAccountData !== "loading") { if ( updatedAccountBalance.assetAmount.amount === 0n && - existingAccountData.balances[updatedAssetSymbol] === undefined && + existingAccountData.balances[assetID] === undefined && !isBuiltInNetworkBaseAsset(asset, network) // add base asset even if balance is 0 ) { return } - existingAccountData.balances[updatedAssetSymbol] = - updatedAccountBalance + existingAccountData.balances[assetID] = updatedAccountBalance } else { immerState.accountsData.evm[network.chainID][normalizedAddress] = { // TODO Figure out the best way to handle default name assignment // TODO across networks. ...newAccountData(address, network, immerState), balances: { - [updatedAssetSymbol]: updatedAccountBalance, + [assetID]: updatedAccountBalance, }, } } @@ -433,7 +448,7 @@ const accountSlice = createSlice({ if (account !== "loading") { Object.values(account.balances).forEach(({ assetAmount }) => { if (isSameAsset(assetAmount.asset, asset)) { - delete account.balances[assetAmount.asset.symbol] + delete account.balances[getAssetID(assetAmount.asset)] } }) } diff --git a/background/redux-slices/migrations/index.ts b/background/redux-slices/migrations/index.ts index eb069ff3f6..e68823df87 100644 --- a/background/redux-slices/migrations/index.ts +++ b/background/redux-slices/migrations/index.ts @@ -27,12 +27,13 @@ import to27 from "./to-27" import to28 from "./to-28" import to29 from "./to-29" import to30 from "./to-30" +import to31 from "./to-31" /** * The version of persisted Redux state the extension is expecting. Any previous * state without this version, or with a lower version, ought to be migrated. */ -export const REDUX_STATE_VERSION = 30 +export const REDUX_STATE_VERSION = 31 /** * Common type for all migration functions. @@ -72,6 +73,7 @@ const allMigrations: { [targetVersion: string]: Migration } = { 28: to28, 29: to29, 30: to30, + 31: to31, } /** diff --git a/background/redux-slices/migrations/to-31.ts b/background/redux-slices/migrations/to-31.ts new file mode 100644 index 0000000000..22651e20a5 --- /dev/null +++ b/background/redux-slices/migrations/to-31.ts @@ -0,0 +1,52 @@ +type PrevState = { + account: { + accountsData: { + evm: { + [chainID: string]: { + [address: string]: { + balances: { + [symbol: string]: unknown + } + [other: string]: unknown + } + } + } + } + [sliceKey: string]: unknown + } +} + +type NewState = { + account: { + accountsData: { + evm: { + [chainID: string]: { + [address: string]: { + balances: { + [assetID: string]: unknown + } + [other: string]: unknown + } + } + } + } + [sliceKey: string]: unknown + } +} + +export default (prevState: Record): NewState => { + const typedPrevState = prevState as PrevState + + const { + account: { accountsData }, + } = typedPrevState + + Object.keys(accountsData.evm).forEach((chainID) => + Object.keys(accountsData.evm[chainID]).forEach((address) => { + // Clear all accounts cached balances + accountsData.evm[chainID][address].balances = {} + }) + ) + + return { ...typedPrevState } +} diff --git a/background/redux-slices/tests/accounts.unit.test.ts b/background/redux-slices/tests/accounts.unit.test.ts index ec528ebe87..848802c0f8 100644 --- a/background/redux-slices/tests/accounts.unit.test.ts +++ b/background/redux-slices/tests/accounts.unit.test.ts @@ -15,6 +15,7 @@ import reducer, { updateAccountBalance, updateAssetReferences, } from "../accounts" +import { getAssetID } from "../utils/asset-utils" import { determineAssetDisplayAndVerify } from "../selectors" const ADDRESS_MOCK = "0x208e94d5661a73360d9387d3ca169e5c130090cd" @@ -73,7 +74,7 @@ describe("Accounts redux slice", () => { expect(updatedAccountData).not.toEqual("loading") const updatedBalance = (updatedAccountData as AccountData)?.balances - expect(updatedBalance?.[ETH.symbol].assetAmount.amount).toBe(1n) + expect(updatedBalance?.[getAssetID(ETH)].assetAmount.amount).toBe(1n) expect(updated.combinedData.totalMainCurrencyValue).toBe("") }) @@ -93,7 +94,7 @@ describe("Accounts redux slice", () => { updated.accountsData.evm[ETHEREUM.chainID][ADDRESS_MOCK] const updatedBalance = (updatedAccountData as AccountData)?.balances - expect(updatedBalance?.[ETH.symbol].assetAmount.amount).toBe(1n) + expect(updatedBalance?.[getAssetID(ETH)].assetAmount.amount).toBe(1n) expect(updated.combinedData.totalMainCurrencyValue).toBe("") }) @@ -124,7 +125,8 @@ describe("Accounts redux slice", () => { expect(updatedAccountData).not.toEqual("loading") const updatedBalance = (updatedAccountData as AccountData)?.balances - expect(updatedBalance?.[ETH.symbol].assetAmount.amount).toBe(0n) + + expect(updatedBalance?.[getAssetID(ETH)].assetAmount.amount).toBe(0n) }) it("should update zero balance for account that is loaded", () => { @@ -151,7 +153,7 @@ describe("Accounts redux slice", () => { updated.accountsData.evm[ETHEREUM.chainID][ADDRESS_MOCK] const updatedBalance = (updatedAccountData as AccountData)?.balances - expect(updatedBalance?.[ETH.symbol].assetAmount.amount).toBe(0n) + expect(updatedBalance?.[getAssetID(ETH)].assetAmount.amount).toBe(0n) }) it("should update positive balance multiple times", () => { @@ -186,8 +188,63 @@ describe("Accounts redux slice", () => { updated.accountsData.evm[ETHEREUM.chainID][ADDRESS_MOCK] const updatedBalance = (updatedAccountData as AccountData)?.balances - expect(updatedBalance?.[ETH.symbol].assetAmount.amount).toBe(1n) - expect(updatedBalance?.[ASSET_MOCK.symbol].assetAmount.amount).toBe(10n) + expect(updatedBalance?.[getAssetID(ETH)].assetAmount.amount).toBe(1n) + expect(updatedBalance?.[getAssetID(ASSET_MOCK)].assetAmount.amount).toBe( + 10n + ) + }) + + it("should support storing balances for assets with the same symbol", () => { + state.accountsData.evm = { + [ETHEREUM.chainID]: { [ADDRESS_MOCK]: ACCOUNT_MOCK }, + } + + const someToken = createSmartContractAsset({ symbol: "USDC" }) + const someOtherToken = createSmartContractAsset({ symbol: "USDC" }) + + const initial = reducer( + state, + updateAccountBalance({ + balances: [BALANCE_MOCK], + addressOnNetwork: { address: ADDRESS_MOCK, network: ETHEREUM }, + }) + ) + + const updated = reducer( + initial, + updateAccountBalance({ + balances: [ + BALANCE_MOCK, + { + ...BALANCE_MOCK, + assetAmount: { asset: someToken, amount: 1n }, + }, + { + ...BALANCE_MOCK, + assetAmount: { asset: someOtherToken, amount: 2n }, + }, + ], + addressOnNetwork: { address: ADDRESS_MOCK, network: ETHEREUM }, + }) + ) + + const updatedAccountData = + updated.accountsData.evm[ETHEREUM.chainID][ADDRESS_MOCK] + const balances = (updatedAccountData as AccountData)?.balances + + expect(balances?.[getAssetID(ETH)].assetAmount.asset).toEqual(ETH) + + expect(balances?.[getAssetID(someToken)].assetAmount.asset).toEqual( + someToken + ) + expect(balances?.[getAssetID(someToken)].assetAmount.amount).toEqual(1n) + + expect(balances?.[getAssetID(someOtherToken)].assetAmount.asset).toEqual( + someOtherToken + ) + expect(balances?.[getAssetID(someOtherToken)].assetAmount.amount).toEqual( + 2n + ) }) it("updates cached asset data for all accounts", () => { @@ -241,11 +298,11 @@ describe("Accounts redux slice", () => { ][otherAccount.address] as AccountData expect( - firstAccountData.balances[asset.symbol].assetAmount.asset.metadata + firstAccountData.balances[getAssetID(asset)].assetAmount.asset.metadata ?.verified ).not.toBeDefined() expect( - secondAccountData.balances[asset.symbol].assetAmount.asset.metadata + secondAccountData.balances[getAssetID(asset)].assetAmount.asset.metadata ?.verified ).not.toBeDefined() @@ -267,11 +324,11 @@ describe("Accounts redux slice", () => { ][otherAccount.address] as AccountData expect( - updatedFirstAccountData.balances[asset.symbol].assetAmount.asset + updatedFirstAccountData.balances[getAssetID(asset)].assetAmount.asset .metadata?.verified ).toBe(true) expect( - updatedSecondAccountData.balances[asset.symbol].assetAmount.asset + updatedSecondAccountData.balances[getAssetID(asset)].assetAmount.asset .metadata?.verified ).toBe(true) }) diff --git a/background/redux-slices/utils/asset-utils.ts b/background/redux-slices/utils/asset-utils.ts index 24e7b4560b..266d02f858 100644 --- a/background/redux-slices/utils/asset-utils.ts +++ b/background/redux-slices/utils/asset-utils.ts @@ -10,6 +10,7 @@ import { AnyAsset, CoinGeckoAsset, isSmartContractFungibleAsset, + SmartContractFungibleAsset, } from "../../assets" import { BUILT_IN_NETWORK_BASE_ASSETS, @@ -376,6 +377,20 @@ export function isUnverifiedAssetByUser(asset: AnyAsset | undefined): boolean { return false } +type AssetType = "base" | "erc20" + +export type AssetID = `${AssetType}/${string}` + +export const getAssetID = ( + asset: NetworkBaseAsset | SmartContractFungibleAsset +): AssetID => { + if (isNetworkBaseAsset(asset)) { + return `base/${asset.symbol}` + } + + return `erc20/${asset.contractAddress}` +} + /** * Assets that are untrusted and have not been verified by the user * should not be swapped or sent. diff --git a/background/services/wallet-connect/utils.ts b/background/services/wallet-connect/utils.ts index c5a394c8b7..e8c0c983b8 100644 --- a/background/services/wallet-connect/utils.ts +++ b/background/services/wallet-connect/utils.ts @@ -4,6 +4,7 @@ import browser from "webextension-polyfill" export const getMetaPort = ( name: string, senderUrl: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any postMessage: (message: any) => void ): Required => { const port: browser.Runtime.Port = browser.runtime.connect({