Skip to content

fix: add percent change for 24h on asset list #14839

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

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
766c563
feat: init
gambinish Apr 14, 2025
a52f4c7
chore: Add Solana MarketDetails
gambinish Apr 18, 2025
4597d04
chore: Add price chart and 24 hour diff
gambinish Apr 18, 2025
16ab115
chore: Cleanup
gambinish Apr 18, 2025
b4ac7d5
chore: Merge main
gambinish Apr 18, 2025
8e819ce
Merge branch 'main' into feat/solana-token-details-2
gambinish Apr 18, 2025
8996f86
chore: Merge main, address conflicts
gambinish Apr 23, 2025
2844d0f
chore: Integrate non-evm historical prices into price chart
gambinish Apr 23, 2025
2ed94ce
chore: Uncomment code
gambinish Apr 23, 2025
35cb919
fix: Update market details
gambinish Apr 23, 2025
e6fc961
feat: Add non evm market data and token details
gambinish Apr 23, 2025
bc27757
chore: Update asset price calculation to account for non evm asset types
gambinish Apr 23, 2025
9575438
fix: Cleanup
gambinish Apr 23, 2025
d079304
chore: Merge main
gambinish Apr 23, 2025
73e8397
fix: Cleanup and memoize callbacks
gambinish Apr 23, 2025
bc51d06
fix: Cleanup, lint, tweak unit tests
gambinish Apr 23, 2025
1725579
fix: lint:tsc
gambinish Apr 23, 2025
a9a34a7
fix: Lint
gambinish Apr 23, 2025
5102a40
fix: Update incorrect unit tests
gambinish Apr 24, 2025
3df6ffe
fix: Update AssetOverview test
gambinish Apr 24, 2025
cdb9327
fix: Price tests
gambinish Apr 24, 2025
9969265
fix: Remove unused useTokenHistoricalPricesV3 hook
gambinish Apr 24, 2025
283eb7b
chore: codefencing
gambinish Apr 24, 2025
5427bd4
fix: add percent change for 24h on asset list
sahar-fehri Apr 24, 2025
fffb8e7
fix: fix type
sahar-fehri Apr 24, 2025
37466be
fix: add non evm aggregated balance percent change
sahar-fehri Apr 24, 2025
b0b6244
fix: fix test
sahar-fehri Apr 24, 2025
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
import React from 'react';
import { render } from '@testing-library/react-native';
import { mockTheme } from '../../../../util/theme';
import { useSelector } from 'react-redux';
import { selectCurrentCurrency } from '../../../../selectors/currencyRateController';
import {
FORMATTED_VALUE_PRICE_TEST_ID,
FORMATTED_PERCENTAGE_TEST_ID,
} from './AggregatedPercentage.constants';
import NonEvmAggregatedPercentage from './NonEvmAggregatedPercentage';
// eslint-disable-next-line import/no-namespace
import * as multichain from '../../../../selectors/multichain/multichain';
import { selectMultichainAssetsRates } from '../../../../selectors/multichain/multichain';

const mockGetMultichainNetworkAggregatedBalance = {
balances: {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
amount: '1',
unit: 'SOL',
},
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv':
{
amount: '5',
unit: 'PENGU',
},
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v':
{
amount: '10',
unit: 'USDC',
},
},
fiatBalances: {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': '1',
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv':
'5.4',
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v':
'10.',
},
totalBalanceFiat: 16.4,
totalNativeTokenBalance: {
amount: '1',
unit: 'SOL',
},
};

const mockMultichainAssetsRatesPositive = {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
marketData: {
pricePercentChange: {
P1D: +1.1,
},
},
rate: '147.98',
},
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv':
{
marketData: {
pricePercentChange: {
P1D: +2.7,
},
},
rate: '0.00624788',
},
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v':
{
marketData: {
pricePercentChange: {
P1D: +0.5,
},
},
rate: '0.999998',
},
};

const mockMultichainAssetsRatesNegative = {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
marketData: {
pricePercentChange: {
P1D: -1.1,
},
},
rate: '147.98',
},
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:2zMMhcVQEXDtdE6vsFS7S7D5oUodfJHE8vd1gnBouauv':
{
marketData: {
pricePercentChange: {
P1D: -2.7,
},
},
rate: '0.00624788',
},
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v':
{
marketData: {
pricePercentChange: {
P1D: -0.5,
},
},
rate: '0.999998',
},
};

jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useSelector: jest.fn(),
}));
describe('NonEvmAggregatedPercentage', () => {
beforeEach(() => {
(useSelector as jest.Mock).mockImplementation((selector) => {
if (selector === selectCurrentCurrency) return 'USD';
});
});
afterEach(() => {
(useSelector as jest.Mock).mockClear();
});

it('renders positive percentage change correctly', () => {
jest
.spyOn(multichain, 'getMultichainNetworkAggregatedBalance')
.mockReturnValue(mockGetMultichainNetworkAggregatedBalance);
(useSelector as jest.Mock).mockImplementation((selector) => {
if (selector === selectCurrentCurrency) return 'USD';
if (selector === selectMultichainAssetsRates)
return mockMultichainAssetsRatesPositive;
});
const { getByText } = render(<NonEvmAggregatedPercentage />);

expect(getByText('(+1.25%)')).toBeTruthy();
expect(getByText('+0.2 USD')).toBeTruthy();

expect(getByText('(+1.25%)').props.style).toMatchObject({
color: mockTheme.colors.success.default,
});
});

it('renders negative percentage change correctly', () => {
jest
.spyOn(multichain, 'getMultichainNetworkAggregatedBalance')
.mockReturnValue(mockGetMultichainNetworkAggregatedBalance);
(useSelector as jest.Mock).mockImplementation((selector) => {
if (selector === selectCurrentCurrency) return 'USD';
if (selector === selectMultichainAssetsRates)
return mockMultichainAssetsRatesNegative;
});
const { getByText } = render(<NonEvmAggregatedPercentage />);

expect(getByText('(-1.27%)')).toBeTruthy();
expect(getByText('-0.21 USD')).toBeTruthy();

expect(getByText('(-1.27%)').props.style).toMatchObject({
color: mockTheme.colors.error.default,
});
});

it('renders correctly with privacy mode on', () => {
jest
.spyOn(multichain, 'getMultichainNetworkAggregatedBalance')
.mockReturnValue(mockGetMultichainNetworkAggregatedBalance);
(useSelector as jest.Mock).mockImplementation((selector) => {
if (selector === selectCurrentCurrency) return 'USD';
if (selector === selectMultichainAssetsRates)
return mockMultichainAssetsRatesNegative;
});
const { getByTestId } = render(<NonEvmAggregatedPercentage privacyMode />);

const formattedPercentage = getByTestId(FORMATTED_PERCENTAGE_TEST_ID);
const formattedValuePrice = getByTestId(FORMATTED_VALUE_PRICE_TEST_ID);

expect(formattedPercentage.props.children).toBe('••••••••••');
expect(formattedValuePrice.props.children).toBe('••••••••••');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import React from 'react';
import {
TextColor,
TextVariant,
} from '../../../../component-library/components/Texts/Text';
import SensitiveText from '../../../../component-library/components/Texts/SensitiveText';
import { View } from 'react-native';
import { renderFiat } from '../../../../util/number';
import { useSelector } from 'react-redux';
import { selectCurrentCurrency } from '../../../../selectors/currencyRateController';
import styleSheet from './AggregatedPercentage.styles';
import { useStyles } from '../../../hooks';
import {
FORMATTED_VALUE_PRICE_TEST_ID,
FORMATTED_PERCENTAGE_TEST_ID,
} from './AggregatedPercentage.constants';
import { DECIMALS_TO_SHOW } from '../../../../components/UI/Tokens/constants';
import {
getMultichainNetworkAggregatedBalance,
selectMultichainAssets,
selectMultichainAssetsRates,
selectMultichainBalances,
} from '../../../../selectors/multichain/multichain';
import { selectSelectedInternalAccount } from '../../../../selectors/accountsController';
import { selectSelectedNonEvmNetworkChainId } from '../../../../selectors/multichainNetworkController';
import { InternalAccount } from '@metamask/keyring-internal-api';
import { CaipAssetType } from '@metamask/keyring-api';
import { getCalculatedTokenAmount1dAgo } from './AggregatedPercentageCrossChains';

const isValidAmount = (amount: number | null | undefined): boolean =>
amount !== null && amount !== undefined && !Number.isNaN(amount);

const NonEvmAggregatedPercentage = ({
privacyMode = false,
}: {
privacyMode?: boolean;
}) => {
const { styles } = useStyles(styleSheet, {});

const currentCurrency = useSelector(selectCurrentCurrency);

const account = useSelector(selectSelectedInternalAccount);
const multichainBalances = useSelector(selectMultichainBalances);
const multichainAssets = useSelector(selectMultichainAssets);
const multichainAssetsRates = useSelector(selectMultichainAssetsRates);
const nonEvmChainId = useSelector(selectSelectedNonEvmNetworkChainId);

const nonEvmAccountBalance = getMultichainNetworkAggregatedBalance(
account as unknown as InternalAccount,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the casting is unnecessary here. If not we should explicit mark the return type of this selector to be InternalAccount.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make the return type explicit in the selector;
Although i think i might need to still cast because the return type is InternalAccount | undefined

multichainBalances,
multichainAssets,
multichainAssetsRates,
nonEvmChainId,
);

const nonEvmFiatBalancesArray = Object.entries(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both this and nonEvmAccountBalance should be memoized. The Object.entries will eagerly create a new object on every render which can trigger more re renders. Instead we can warp these two in a useMemo.

nonEvmAccountBalance?.fiatBalances ?? {},
).map(([id, amount]) => ({
id: id as CaipAssetType,
fiatAmount: Number(amount),
}));

const getNonEvmTotalFiat1dAgo = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also memoize this?

const totalNonEvm1dAgo = nonEvmFiatBalancesArray.reduce(
(total1dAgo: number, item: { id: CaipAssetType; fiatAmount: number }) => {
const pricePercentChange1d =
multichainAssetsRates?.[item.id]?.marketData?.pricePercentChange
.P1D ?? 0;
const splTokenFiat1dAgo = getCalculatedTokenAmount1dAgo(
Number(item.fiatAmount),
pricePercentChange1d,
);
return total1dAgo + Number(splTokenFiat1dAgo);
},
0,
);

return totalNonEvm1dAgo;
};

const totalNonEvmBalance = nonEvmAccountBalance?.totalBalanceFiat;

if (!totalNonEvmBalance) {
return null;
}

const totalNonEvmBalance1dAgo = getNonEvmTotalFiat1dAgo();
const amountChange = totalNonEvmBalance - totalNonEvmBalance1dAgo;

const percentageChange =
((totalNonEvmBalance - totalNonEvmBalance1dAgo) / totalNonEvmBalance1dAgo) *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make sure that the totalNonEvmBalance1dAgo is greater than 0 in order to avoid a divide by 0 issue? I believe if the value is 0 we will see infinity. Is this a valid case or am I being paranoid?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also can we just reuse the amountChange var from above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point; i can add a check but i think that to be in that case everything would have to be zero
if
const totalNonEvmBalance1dAgo = getNonEvmTotalFiat1dAgo();
is zero then totalNonEvmBalance is also zero in which case we return null

  if (!totalNonEvmBalance) {
    return null;
  }

100 || 0;

let percentageTextColor = TextColor.Default;

if (!privacyMode) {
if (percentageChange === 0) {
percentageTextColor = TextColor.Default;
} else if (percentageChange > 0) {
percentageTextColor = TextColor.Success;
} else {
percentageTextColor = TextColor.Error;
}
} else {
percentageTextColor = TextColor.Alternative;
}

const formattedPercentage = isValidAmount(percentageChange)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should move these into utils.

? `(${(percentageChange as number) >= 0 ? '+' : ''}${(
percentageChange as number
).toFixed(2)}%)`
: '';

const formattedValuePrice = isValidAmount(amountChange)
? `${(amountChange as number) >= 0 ? '+' : ''}${renderFiat(
amountChange,
currentCurrency,
DECIMALS_TO_SHOW,
)} `
: '';

return (
<View style={styles.wrapper}>
<SensitiveText
isHidden={privacyMode}
length="10"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the length always going to be 10?

color={percentageTextColor}
variant={TextVariant.BodyMDMedium}
testID={FORMATTED_VALUE_PRICE_TEST_ID}
>
{formattedValuePrice}
</SensitiveText>
<SensitiveText
isHidden={privacyMode}
length="10"
color={percentageTextColor}
variant={TextVariant.BodyMDMedium}
testID={FORMATTED_PERCENTAGE_TEST_ID}
>
{formattedPercentage}
</SensitiveText>
</View>
);
};

export default NonEvmAggregatedPercentage;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets wrap this whole thing in a React.memo

29 changes: 21 additions & 8 deletions app/components/UI/AssetOverview/AssetOverview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ const mockInitialState = {
logo: 'https://upload.wikimedia.org/wikipedia/commons/0/05/Ethereum_logo_2014.svg',
name: 'Ethereum',
symbol: 'ETH',
price: {}
}
price: {},
},
],
},
},
Expand Down Expand Up @@ -129,12 +129,14 @@ jest.mock('../../../core/Engine', () => ({
},
}));

const mockAddPopularNetwork = jest.fn().mockImplementation(() => Promise.resolve());
const mockAddPopularNetwork = jest
.fn()
.mockImplementation(() => Promise.resolve());
jest.mock('../../../components/hooks/useAddNetwork', () => ({
useAddNetwork: jest.fn().mockImplementation(() => ({
addPopularNetwork: mockAddPopularNetwork,
networkModal: null,
}))
addPopularNetwork: mockAddPopularNetwork,
networkModal: null,
})),
}));

const asset = {
Expand Down Expand Up @@ -437,6 +439,7 @@ describe('AssetOverview', () => {
<AssetOverview
asset={{
...asset,
address: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501',
chainId: SolScope.Mainnet,
isNative: true,
}}
Expand All @@ -455,6 +458,13 @@ describe('AssetOverview', () => {
MultichainNetworkController: {
selectedMultichainNetworkChainId: SolScope.Mainnet,
},
MultichainAssetsRatesController: {
conversionRates: {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
rate: '151.23',
},
},
},
},
},
},
Expand Down Expand Up @@ -488,12 +498,15 @@ describe('AssetOverview', () => {
destinationToken: assetFromSearch.address,
sourcePage: 'MainView',
chainId: assetFromSearch.chainId,
}
},
});
});

it('should prompt to add the network if coming from search and on a different chain', async () => {
(Engine.context.NetworkController.getNetworkConfigurationByChainId as jest.Mock).mockReturnValueOnce(null);
(
Engine.context.NetworkController
.getNetworkConfigurationByChainId as jest.Mock
).mockReturnValueOnce(null);
const differentChainAssetFromSearch = {
...assetFromSearch,
chainId: '0xa',
Expand Down
Loading
Loading