From a582e955fbf67f5934d37ccbf7c85f1f18c53c4e Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 24 Jul 2024 18:40:20 -0700 Subject: [PATCH 01/10] fix price diff calculation --- .../Price/AggregatedPercentage/AggregatedPercentage.tsx | 2 +- app/core/Engine.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx b/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx index 3e0907f937b..8ca7ee1b35f 100644 --- a/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx +++ b/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx @@ -35,7 +35,7 @@ const AggregatedPercentage = ({ const amountChange = totalBalance - totalBalance1dAgo; const percentageChange = - ((totalBalance - totalBalance1dAgo) / totalBalance1dAgo) * 100 || 0; + ((totalBalance - totalBalance1dAgo) / totalBalance) * 100 || 0; let percentageTextColor = TextColor.Default; diff --git a/app/core/Engine.ts b/app/core/Engine.ts index eaece76f899..c1417fc3311 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1635,7 +1635,7 @@ class Engine { } ethFiat1dAgo = - ethFiat + + ethFiat - (ethFiat * tokenExchangeRates?.[toHexadecimal(chainId)]?.[ zeroAddress() as `0x${string}` @@ -1671,7 +1671,7 @@ class Engine { ); const tokenBalance1dAgo = - tokenBalanceFiat + + tokenBalanceFiat - (tokenBalanceFiat * tokenExchangeRates?.[item.address as `0x${string}`] ?.pricePercentChange1d) / From 55001f97743cac619c8f59edd5462ae75d14f986 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 24 Jul 2024 20:16:03 -0700 Subject: [PATCH 02/10] fix 2 day ago calculation --- .../AggregatedPercentage.tsx | 2 +- app/core/Engine.ts | 26 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx b/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx index 8ca7ee1b35f..3e0907f937b 100644 --- a/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx +++ b/app/component-library/components-temp/Price/AggregatedPercentage/AggregatedPercentage.tsx @@ -35,7 +35,7 @@ const AggregatedPercentage = ({ const amountChange = totalBalance - totalBalance1dAgo; const percentageChange = - ((totalBalance - totalBalance1dAgo) / totalBalance) * 100 || 0; + ((totalBalance - totalBalance1dAgo) / totalBalance1dAgo) * 100 || 0; let percentageTextColor = TextColor.Default; diff --git a/app/core/Engine.ts b/app/core/Engine.ts index c1417fc3311..6725a2a748d 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1634,13 +1634,15 @@ class Engine { ); } + const ethPricePercentChange1d = + tokenExchangeRates?.[toHexadecimal(chainId)]?.[ + zeroAddress() as `0x${string}` + ]?.pricePercentChange1d; + ethFiat1dAgo = - ethFiat - - (ethFiat * - tokenExchangeRates?.[toHexadecimal(chainId)]?.[ - zeroAddress() as `0x${string}` - ]?.pricePercentChange1d) / - 100 || ethFiat; + ethPricePercentChange1d !== undefined + ? ethFiat / (1 + ethPricePercentChange1d / 100) + : ethFiat; if (tokens.length > 0) { const { contractBalances: tokenBalances } = TokenBalancesController.state; @@ -1670,12 +1672,14 @@ class Engine { decimalsToShow, ); + const tokePricePercentChange1d = + tokenExchangeRates?.[item.address as `0x${string}`] + ?.pricePercentChange1d; + const tokenBalance1dAgo = - tokenBalanceFiat - - (tokenBalanceFiat * - tokenExchangeRates?.[item.address as `0x${string}`] - ?.pricePercentChange1d) / - 100 || tokenBalanceFiat; + tokePricePercentChange1d !== undefined + ? tokenBalanceFiat / (1 + tokePricePercentChange1d / 100) + : tokenBalanceFiat; tokenFiat += tokenBalanceFiat; tokenFiat1dAgo += tokenBalance1dAgo; From f28e11d38d7d2931e04ded47b7871c3ba22f1835 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 24 Jul 2024 20:20:26 -0700 Subject: [PATCH 03/10] variable typo --- app/core/Engine.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/core/Engine.ts b/app/core/Engine.ts index 6725a2a748d..deab15f2f6b 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1672,13 +1672,13 @@ class Engine { decimalsToShow, ); - const tokePricePercentChange1d = + const tokenPricePercentChange1d = tokenExchangeRates?.[item.address as `0x${string}`] ?.pricePercentChange1d; const tokenBalance1dAgo = - tokePricePercentChange1d !== undefined - ? tokenBalanceFiat / (1 + tokePricePercentChange1d / 100) + tokenPricePercentChange1d !== undefined + ? tokenBalanceFiat / (1 + tokenPricePercentChange1d / 100) : tokenBalanceFiat; tokenFiat += tokenBalanceFiat; From a4fe2906a3de656070a9b0e8631e324b50cabb93 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 24 Jul 2024 20:32:18 -0700 Subject: [PATCH 04/10] reuse variable --- app/core/Engine.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/core/Engine.ts b/app/core/Engine.ts index deab15f2f6b..3dbb9e7b6f5 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1619,7 +1619,8 @@ class Engine { const { accountsByChainId } = AccountTrackerController.state; const { tokens } = TokensController.state; - const { marketData: tokenExchangeRates } = TokenRatesController.state; + const { marketData } = TokenRatesController.state; + const tokenExchangeRates = marketData?.[toHexadecimal(chainId)]; let ethFiat = 0; let ethFiat1dAgo = 0; @@ -1635,9 +1636,7 @@ class Engine { } const ethPricePercentChange1d = - tokenExchangeRates?.[toHexadecimal(chainId)]?.[ - zeroAddress() as `0x${string}` - ]?.pricePercentChange1d; + tokenExchangeRates?.[zeroAddress() as Hex]?.pricePercentChange1d; ethFiat1dAgo = ethPricePercentChange1d !== undefined @@ -1673,8 +1672,7 @@ class Engine { ); const tokenPricePercentChange1d = - tokenExchangeRates?.[item.address as `0x${string}`] - ?.pricePercentChange1d; + tokenExchangeRates?.[item.address as Hex]?.pricePercentChange1d; const tokenBalance1dAgo = tokenPricePercentChange1d !== undefined From 8601ed61e5075d09adadd87f204b8d9be81f82c6 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 24 Jul 2024 20:36:22 -0700 Subject: [PATCH 05/10] remove duplicated variables --- app/core/Engine.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/core/Engine.ts b/app/core/Engine.ts index 3dbb9e7b6f5..1b7d8312708 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1645,14 +1645,9 @@ class Engine { if (tokens.length > 0) { const { contractBalances: tokenBalances } = TokenBalancesController.state; - const { marketData } = TokenRatesController.state; - const tokenExchangeRates = marketData[chainId]; tokens.forEach( (item: { address: string; balance?: string; decimals: number }) => { - const exchangeRate = - tokenExchangeRates && item.address in tokenExchangeRates - ? tokenExchangeRates[item.address as Hex]?.price - : undefined; + const exchangeRate = tokenExchangeRates?.[item.address as Hex]?.price; const tokenBalance = item.balance || From f3d3e74fb3d783bf8059979feffd8bda991bc954 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Thu, 25 Jul 2024 18:45:01 -0700 Subject: [PATCH 06/10] unit tests --- app/core/Engine.test.js | 146 ++++++++++++++++++++++++++++++++++++++++ app/core/Engine.ts | 4 +- 2 files changed, 147 insertions(+), 3 deletions(-) diff --git a/app/core/Engine.test.js b/app/core/Engine.test.js index 8ee16837372..ba5d974edd9 100644 --- a/app/core/Engine.test.js +++ b/app/core/Engine.test.js @@ -1,7 +1,9 @@ import Engine from './Engine'; import { backgroundState } from '../util/test/initial-root-state'; +import { zeroAddress } from 'ethereumjs-util'; jest.unmock('./Engine'); +jest.mock('../store', () => ({ store: { getState: jest.fn(() => ({})) } })); describe('Engine', () => { it('should expose an API', () => { @@ -91,4 +93,148 @@ describe('Engine', () => { `No account found for address: ${invalidAddress}`, ); }); + + describe('getTotalFiatAccountBalance', () => { + const selectedAddress = '0x123'; + const chainId = '0x1'; + const ticker = 'ETH'; + const ethConversionRate = 4000; // $4,000 / ETH + + const state = { + PreferencesController: { selectedAddress }, + NetworkController: { + state: { providerConfig: { chainId, ticker } }, + }, + CurrencyRateController: { + currencyRates: { + [ticker]: { conversionRate: ethConversionRate }, + }, + }, + }; + + it('calculates when theres no balances', () => { + const engine = Engine.init(state); + const totalFiatBalance = engine.getTotalFiatAccountBalance(); + expect(totalFiatBalance).toStrictEqual({ + ethFiat: 0, + ethFiat1dAgo: 0, + tokenFiat: 0, + tokenFiat1dAgo: 0, + }); + }); + + // TODO: Why do these tests work when running individually but not when running all tests? + it('calculates when theres only ETH', () => { + const ethBalance = 1; // 1 ETH + const ethPricePercentChange1d = 5; // up 5% + + const engine = Engine.init({ + ...state, + AccountTrackerController: { + accountsByChainId: { + [chainId]: { + [selectedAddress]: { balance: ethBalance * 1e18 }, + }, + }, + }, + TokenRatesController: { + marketData: { + [chainId]: { + [zeroAddress()]: { + pricePercentChange1d: ethPricePercentChange1d, + }, + }, + }, + }, + }); + + const totalFiatBalance = engine.getTotalFiatAccountBalance(); + + const ethFiat = ethBalance * ethConversionRate; + expect(totalFiatBalance).toStrictEqual({ + ethFiat, + ethFiat1dAgo: ethFiat / (1 + ethPricePercentChange1d / 100), + tokenFiat: 0, + tokenFiat1dAgo: 0, + }); + }); + + // TODO: Why do these tests work when running individually but not when running all tests? + it('calculates when there are ETH and tokens', () => { + const ethBalance = 1; + const ethPricePercentChange1d = 5; + + const tokens = [ + { + address: '0x001', + balance: 1, + price: '1', + pricePercentChange1d: -1, + }, + { + address: '0x002', + balance: 2, + price: '2', + pricePercentChange1d: 2, + }, + ]; + + const engine = Engine.init({ + ...state, + AccountTrackerController: { + accountsByChainId: { + [chainId]: { + [selectedAddress]: { balance: ethBalance * 1e18 }, + }, + }, + }, + TokensController: { + tokens: tokens.map((token) => ({ + address: token.address, + balance: token.balance, + })), + }, + TokenRatesController: { + marketData: { + [chainId]: { + [zeroAddress()]: { + pricePercentChange1d: ethPricePercentChange1d, + }, + ...tokens.reduce( + (acc, token) => ({ + ...acc, + [token.address]: { + price: token.price, + pricePercentChange1d: token.pricePercentChange1d, + }, + }), + {}, + ), + }, + }, + }, + }); + + const totalFiatBalance = engine.getTotalFiatAccountBalance(); + + const ethFiat = ethBalance * ethConversionRate; + const [tokenFiat, tokenFiat1dAgo] = tokens.reduce( + ([fiat, fiat1d], token) => { + const value = token.balance * token.price * ethConversionRate; + return [ + fiat + value, + fiat1d + value / (1 + token.pricePercentChange1d / 100), + ]; + }, + [0, 0], + ); + + expect(totalFiatBalance).toStrictEqual({ + ethFiat, + ethFiat1dAgo: ethFiat / (1 + ethPricePercentChange1d / 100), + tokenFiat, + tokenFiat1dAgo, + }); + }); + }); }); diff --git a/app/core/Engine.ts b/app/core/Engine.ts index 1b7d8312708..c48d9c555e4 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1605,9 +1605,7 @@ class Engine { const { selectedAddress } = PreferencesController.state; const { currentCurrency } = CurrencyRateController.state; const { chainId, ticker } = NetworkController.state.providerConfig; - const { - settings: { showFiatOnTestnets }, - } = store.getState(); + const { settings: { showFiatOnTestnets } = {} } = store.getState(); if (isTestNet(chainId) && !showFiatOnTestnets) { return { ethFiat: 0, tokenFiat: 0, ethFiat1dAgo: 0, tokenFiat1dAgo: 0 }; From 34e35b8cf762c7a06684f1649447a820ef7dd1ae Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Thu, 25 Jul 2024 19:01:08 -0700 Subject: [PATCH 07/10] fix running tests in parallel --- app/core/Engine.test.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/core/Engine.test.js b/app/core/Engine.test.js index ba5d974edd9..31bc9bdaab8 100644 --- a/app/core/Engine.test.js +++ b/app/core/Engine.test.js @@ -95,6 +95,9 @@ describe('Engine', () => { }); describe('getTotalFiatAccountBalance', () => { + let engine; + afterEach(() => engine?.destroyEngineInstance()); + const selectedAddress = '0x123'; const chainId = '0x1'; const ticker = 'ETH'; @@ -113,7 +116,7 @@ describe('Engine', () => { }; it('calculates when theres no balances', () => { - const engine = Engine.init(state); + engine = Engine.init(state); const totalFiatBalance = engine.getTotalFiatAccountBalance(); expect(totalFiatBalance).toStrictEqual({ ethFiat: 0, @@ -128,7 +131,7 @@ describe('Engine', () => { const ethBalance = 1; // 1 ETH const ethPricePercentChange1d = 5; // up 5% - const engine = Engine.init({ + engine = Engine.init({ ...state, AccountTrackerController: { accountsByChainId: { @@ -179,7 +182,7 @@ describe('Engine', () => { }, ]; - const engine = Engine.init({ + engine = Engine.init({ ...state, AccountTrackerController: { accountsByChainId: { From 3c85e940a02bc84ac224814a08496b113e0b3fbd Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Fri, 26 Jul 2024 08:31:18 -0700 Subject: [PATCH 08/10] remove comment --- app/core/Engine.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/core/Engine.test.js b/app/core/Engine.test.js index 31bc9bdaab8..fad3f5d71fc 100644 --- a/app/core/Engine.test.js +++ b/app/core/Engine.test.js @@ -126,7 +126,6 @@ describe('Engine', () => { }); }); - // TODO: Why do these tests work when running individually but not when running all tests? it('calculates when theres only ETH', () => { const ethBalance = 1; // 1 ETH const ethPricePercentChange1d = 5; // up 5% @@ -162,7 +161,6 @@ describe('Engine', () => { }); }); - // TODO: Why do these tests work when running individually but not when running all tests? it('calculates when there are ETH and tokens', () => { const ethBalance = 1; const ethPricePercentChange1d = 5; From 88c15f0322aef10e7f110d1b3b9c2588e7888e94 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 31 Jul 2024 11:39:48 -0700 Subject: [PATCH 09/10] change how test mocks current address --- app/core/Engine.test.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/core/Engine.test.js b/app/core/Engine.test.js index fad3f5d71fc..31123c6dcf7 100644 --- a/app/core/Engine.test.js +++ b/app/core/Engine.test.js @@ -98,13 +98,19 @@ describe('Engine', () => { let engine; afterEach(() => engine?.destroyEngineInstance()); - const selectedAddress = '0x123'; + const selectedAddress = '0x9DeE4BF1dE9E3b930E511Db5cEBEbC8d6F855Db0'; const chainId = '0x1'; const ticker = 'ETH'; const ethConversionRate = 4000; // $4,000 / ETH const state = { PreferencesController: { selectedAddress }, + AccountsController: { + internalAccounts: { + selectedAccount: selectedAddress, + accounts: { [selectedAddress]: { address: selectedAddress } }, + }, + }, NetworkController: { state: { providerConfig: { chainId, ticker } }, }, From 7cd5c7848dcab03134bf7cc142359b9c5851892e Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 31 Jul 2024 11:41:23 -0700 Subject: [PATCH 10/10] remove uncesssary mocked state --- app/core/Engine.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/core/Engine.test.js b/app/core/Engine.test.js index 31123c6dcf7..dc49543ba6b 100644 --- a/app/core/Engine.test.js +++ b/app/core/Engine.test.js @@ -104,7 +104,6 @@ describe('Engine', () => { const ethConversionRate = 4000; // $4,000 / ETH const state = { - PreferencesController: { selectedAddress }, AccountsController: { internalAccounts: { selectedAccount: selectedAddress,