Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Token Discovery - Remap redux asset balances #3195

Merged
merged 15 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions background/accounts.ts
Original file line number Diff line number Diff line change
@@ -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"

/**
Expand All @@ -15,7 +15,7 @@ export type AccountBalance = {
/**
* The measured balance and the asset in which it's denominated.
*/
assetAmount: AnyAssetAmount
assetAmount: AnyAssetAmount<NetworkBaseAsset | SmartContractFungibleAsset>
/**
* The network on which the account balance was measured.
*/
Expand Down
30 changes: 2 additions & 28 deletions background/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ import { getActivityDetails } from "./redux-slices/utils/activities-utils"
import { getRelevantTransactionAddresses } from "./services/enrichment/utils"
import { AccountSignerWithId } from "./signing"
import { AnalyticsPreferences } from "./services/preferences/types"
import { isSmartContractFungibleAsset, SmartContractAsset } from "./assets"
import { isSmartContractFungibleAsset } from "./assets"
import { FeatureFlags, isEnabled } from "./features"
import { NFTCollection } from "./nfts"
import {
Expand Down Expand Up @@ -975,33 +975,7 @@ export default class Main extends BaseService<never> {

const filteredBalancesToDispatch: AccountBalance[] = []

const sortedBalances: AccountBalance[] = []

balances
.filter((balance) => {
const isSmartContract =
"contractAddress" in balance.assetAmount.asset

if (!isSmartContract) {
sortedBalances.push(balance)
}

return isSmartContract
})
// Sort trusted 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) => {
balances.forEach((balance) => {
// TODO support multi-network assets
const balanceHasAnAlreadyTrackedAsset = assetsToTrack.some(
(tracked) =>
Expand Down
41 changes: 28 additions & 13 deletions background/redux-slices/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
AssetMainCurrencyAmount,
AssetDecimalAmount,
isBuiltInNetworkBaseAsset,
AssetID,
getAssetID,
isNetworkBaseAsset,
} from "./utils/asset-utils"
import { DomainName, HexString, URI } from "../types"
import { normalizeEVMAddress } from "../lib/utils"
Expand Down Expand Up @@ -47,7 +50,7 @@ export type AccountData = {
address: HexString
network: Network
balances: {
[assetSymbol: string]: AccountBalance
[assetID: AssetID]: AccountBalance
}
ens: {
name?: DomainName
Expand Down Expand Up @@ -178,13 +181,25 @@ 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) ||
(asset.metadata?.tokenLists?.length ?? 0) > 0
hyphenized marked this conversation as resolved.
Show resolved Hide resolved

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
Expand All @@ -197,13 +212,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
}, {})
)
Expand Down Expand Up @@ -295,7 +311,7 @@ const accountSlice = createSlice({
network,
assetAmount: { asset },
} = updatedAccountBalance
const { symbol: updatedAssetSymbol } = asset
const assetID = getAssetID(asset)

const normalizedAddress = normalizeEVMAddress(address)
const existingAccountData =
Expand All @@ -309,20 +325,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,
},
}
}
Expand Down
4 changes: 3 additions & 1 deletion background/redux-slices/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ import to23 from "./to-23"
import to24 from "./to-24"
import to25 from "./to-25"
import to26 from "./to-26"
import to27 from "./to-27"

/**
* 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 = 26
export const REDUX_STATE_VERSION = 27

/**
* Common type for all migration functions.
Expand Down Expand Up @@ -64,6 +65,7 @@ const allMigrations: { [targetVersion: string]: Migration } = {
24: to24,
25: to25,
26: to26,
27: to27,
}

/**
Expand Down
52 changes: 52 additions & 0 deletions background/redux-slices/migrations/to-27.ts
Original file line number Diff line number Diff line change
@@ -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 = {
jagodarybacka marked this conversation as resolved.
Show resolved Hide resolved
account: {
accountsData: {
evm: {
[chainID: string]: {
[address: string]: {
balances: {
[assetID: string]: unknown
}
[other: string]: unknown
}
}
}
}
[sliceKey: string]: unknown
}
}

export default (prevState: Record<string, unknown>): 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
jagodarybacka marked this conversation as resolved.
Show resolved Hide resolved
accountsData.evm[chainID][address].balances = {}
})
)

return { ...typedPrevState }
}
69 changes: 63 additions & 6 deletions background/redux-slices/tests/accounts.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import reducer, {
AccountState,
updateAccountBalance,
} from "../accounts"
import { getAssetID } from "../utils/asset-utils"

const ADDRESS_MOCK = "0x208e94d5661a73360d9387d3ca169e5c130090cd"
const ACCOUNT_MOCK = {
Expand Down Expand Up @@ -64,7 +65,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("")
})

Expand All @@ -84,7 +85,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("")
})

Expand Down Expand Up @@ -115,7 +116,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", () => {
Expand All @@ -142,7 +144,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", () => {
Expand Down Expand Up @@ -177,8 +179,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
)
})
})
})
Loading