Skip to content

Commit 4f90d1a

Browse files
authored
fix: ShareAddressQR displays correct address for non-EVM assets (#21449)
## **Description** ### Problem When viewing a non-EVM asset (e.g., Solana) and clicking "Receive", the `ShareAddressQR` screen incorrectly displayed the user's EVM address instead of the chain-specific address (e.g., Solana address). ### Root Cause `AssetOverview.tsx:215` always passed `selectedInternalAccountAddress` (EVM address) to the `ShareAddressQR` screen, regardless of the asset's chain type. ### Solution - Use `selectSelectedInternalAccountByScope` selector to get chain-specific accounts - For non-EVM assets: retrieve the account using `getAccountByScope(asset.chainId as CaipChainId)` - For EVM assets: continue using `selectedInternalAccount` - Enhanced error logging with `isNonEvmAsset` and `assetChainId` fields ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Fixed issue where QR code share screen shows the incorrect wallet address ## **Related issues** Fixes: #21447 ## **Manual testing steps** ```gherkin Feature: Receive QR Share Screen Background: Given the user has a wallet with both EVM and non-EVM accounts And the user is on the Asset Overview screen Scenario: User shares QR code for EVM asset Given the user is viewing an Ethereum (EVM) asset When the user taps the "Receive" button Then the Share Address QR screen is displayed And the QR code contains the user's EVM address Scenario: User shares QR code for Solana asset Given the user is viewing a Solana asset When the user taps the "Receive" button Then the Share Address QR screen is displayed And the QR code contains the user's Solana address ``` ## **Screenshots/Recordings** `~` ### **Before** https://github.com/user-attachments/assets/58872d26-829e-4bd1-aef7-08aa8abf2ff5 ### **After** https://github.com/user-attachments/assets/82ea57f2-c50a-4f91-9484-233f8363299f ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > ShareAddressQR now shows the chain-specific address for non-EVM assets; adds selector usage, logging, and tests for EVM and Solana. > > - **Asset Overview (Receive flow)**: > - Use `selectSelectedInternalAccountByScope` to fetch chain-scoped account for non-EVM assets (`CaipChainId`), falling back to `selectedInternalAccount` for EVM. > - Pass chain-appropriate `address` to `Routes.SHEET.MULTICHAIN_ACCOUNT_DETAILS.SHARE_ADDRESS_QR`. > - Improve error logging with `isNonEvmAsset` and `assetChainId` context. > - **Tests**: > - Mock `selectSelectedInternalAccountByScope` and add cases for EVM asset (uses EVM address) and Solana asset (uses Solana address, asserts scope call). > - Adjust existing receive test setup and cleanup to incorporate new selector behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 55c41ac. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 97ba5d6 commit 4f90d1a

File tree

2 files changed

+115
-7
lines changed

2 files changed

+115
-7
lines changed

app/components/UI/AssetOverview/AssetOverview.test.tsx

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ jest.mock(
5151
}),
5252
);
5353

54+
jest.mock('../../../selectors/multichainAccounts/accounts', () => ({
55+
...jest.requireActual('../../../selectors/multichainAccounts/accounts'),
56+
selectSelectedInternalAccountByScope: jest.fn(),
57+
}));
58+
5459
const MOCK_CHAIN_ID = '0x1';
5560

5661
const mockInitialState = {
@@ -266,6 +271,15 @@ describe('AssetOverview', () => {
266271
address: MOCK_ADDRESS_2,
267272
type: 'eip155:eoa',
268273
});
274+
275+
// Default mock for selectSelectedInternalAccountByScope
276+
const { selectSelectedInternalAccountByScope } = jest.requireMock(
277+
'../../../selectors/multichainAccounts/accounts',
278+
);
279+
const mockGetAccountByScope = jest.fn().mockReturnValue({
280+
address: MOCK_ADDRESS_2,
281+
});
282+
selectSelectedInternalAccountByScope.mockReturnValue(mockGetAccountByScope);
269283
});
270284

271285
afterEach(() => {
@@ -545,17 +559,29 @@ describe('AssetOverview', () => {
545559
});
546560
});
547561

548-
it('should handle receive button press', async () => {
562+
it('should handle receive button press for EVM asset with EVM address', async () => {
549563
// Arrange - Mock the selectors directly to ensure conditions are met
550564
const { selectSelectedInternalAccount } = jest.requireMock(
551565
'../../../selectors/accountsController',
552566
);
553567
const { selectSelectedAccountGroup } = jest.requireMock(
554568
'../../../selectors/multichainAccounts/accountTreeController',
555569
);
556-
selectSelectedInternalAccount.mockReturnValue({ address: MOCK_ADDRESS_2 });
570+
const { selectSelectedInternalAccountByScope } = jest.requireMock(
571+
'../../../selectors/multichainAccounts/accounts',
572+
);
573+
574+
selectSelectedInternalAccount.mockReturnValue({
575+
address: MOCK_ADDRESS_2,
576+
type: 'eip155:eoa',
577+
});
557578
selectSelectedAccountGroup.mockReturnValue({ id: 'group-id-123' });
558579

580+
const mockGetAccountByScope = jest.fn().mockReturnValue({
581+
address: MOCK_ADDRESS_2,
582+
});
583+
selectSelectedInternalAccountByScope.mockReturnValue(mockGetAccountByScope);
584+
559585
const { getByTestId } = renderWithProvider(
560586
<AssetOverview
561587
asset={asset}
@@ -570,15 +596,15 @@ describe('AssetOverview', () => {
570596
const receiveButton = getByTestId('token-receive-button');
571597
fireEvent.press(receiveButton);
572598

573-
// Assert - Should navigate to ShareAddressQR
599+
// Assert - Should navigate to ShareAddressQR with EVM address
574600
expect(navigate).toHaveBeenCalledTimes(1);
575601
expect(navigate).toHaveBeenNthCalledWith(
576602
1,
577603
Routes.MODAL.MULTICHAIN_ACCOUNT_DETAIL_ACTIONS,
578604
{
579605
screen: Routes.SHEET.MULTICHAIN_ACCOUNT_DETAILS.SHARE_ADDRESS_QR,
580606
params: expect.objectContaining({
581-
address: MOCK_ADDRESS_2,
607+
address: MOCK_ADDRESS_2, // Should use EVM address for EVM assets
582608
networkName: 'Ethereum Mainnet',
583609
chainId: MOCK_CHAIN_ID,
584610
groupId: 'group-id-123',
@@ -589,6 +615,7 @@ describe('AssetOverview', () => {
589615
// Cleanup mocks for isolation
590616
selectSelectedInternalAccount.mockReset();
591617
selectSelectedAccountGroup.mockReset();
618+
selectSelectedInternalAccountByScope.mockReset();
592619
});
593620

594621
it('should track receive button click analytics with correct properties', async () => {
@@ -640,6 +667,75 @@ describe('AssetOverview', () => {
640667
selectSelectedAccountGroup.mockReset();
641668
});
642669

670+
it('should handle receive button press for Solana asset with Solana address', async () => {
671+
const SOLANA_ADDRESS = 'HN7cABqLq46Es1jh92dQQisAq662SmxELLLsHHe4YWrH';
672+
const SOLANA_CHAIN_ID = SolScope.Mainnet;
673+
674+
const { selectSelectedInternalAccount } = jest.requireMock(
675+
'../../../selectors/accountsController',
676+
);
677+
const { selectSelectedAccountGroup } = jest.requireMock(
678+
'../../../selectors/multichainAccounts/accountTreeController',
679+
);
680+
const { selectSelectedInternalAccountByScope } = jest.requireMock(
681+
'../../../selectors/multichainAccounts/accounts',
682+
);
683+
684+
selectSelectedInternalAccount.mockReturnValue({
685+
address: MOCK_ADDRESS_2,
686+
type: 'eip155:eoa',
687+
});
688+
selectSelectedAccountGroup.mockReturnValue({ id: 'group-id-123' });
689+
690+
const mockGetAccountByScope = jest.fn().mockReturnValue({
691+
address: SOLANA_ADDRESS,
692+
type: SolAccountType.DataAccount,
693+
});
694+
selectSelectedInternalAccountByScope.mockReturnValue(mockGetAccountByScope);
695+
696+
const solanaAsset = {
697+
...asset,
698+
address: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501',
699+
chainId: SOLANA_CHAIN_ID,
700+
isNative: true,
701+
symbol: 'SOL',
702+
};
703+
704+
const { getByTestId } = renderWithProvider(
705+
<AssetOverview
706+
asset={solanaAsset}
707+
displayBuyButton
708+
displaySwapsButton
709+
networkName="Solana Mainnet"
710+
/>,
711+
{ state: mockInitialState },
712+
);
713+
714+
const receiveButton = getByTestId('token-receive-button');
715+
fireEvent.press(receiveButton);
716+
717+
expect(navigate).toHaveBeenCalledTimes(1);
718+
expect(navigate).toHaveBeenNthCalledWith(
719+
1,
720+
Routes.MODAL.MULTICHAIN_ACCOUNT_DETAIL_ACTIONS,
721+
{
722+
screen: Routes.SHEET.MULTICHAIN_ACCOUNT_DETAILS.SHARE_ADDRESS_QR,
723+
params: expect.objectContaining({
724+
address: SOLANA_ADDRESS, // Should use Solana address, not EVM address
725+
networkName: 'Solana Mainnet',
726+
chainId: SOLANA_CHAIN_ID,
727+
groupId: 'group-id-123',
728+
}),
729+
},
730+
);
731+
732+
expect(mockGetAccountByScope).toHaveBeenCalledWith(SOLANA_CHAIN_ID);
733+
734+
selectSelectedInternalAccount.mockReset();
735+
selectSelectedAccountGroup.mockReset();
736+
selectSelectedInternalAccountByScope.mockReset();
737+
});
738+
643739
it('should not render swap button if displaySwapsButton is false', async () => {
644740
const { queryByTestId } = renderWithProvider(
645741
<AssetOverview

app/components/UI/AssetOverview/AssetOverview.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
Hex,
77
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
88
CaipAssetType,
9+
CaipChainId,
910
isCaipAssetType,
1011
///: END:ONLY_INCLUDE_IF
1112
} from '@metamask/utils';
@@ -60,6 +61,7 @@ import {
6061
ActionPosition,
6162
} from '../../../util/analytics/actionButtonTracking';
6263
import { selectSelectedAccountGroup } from '../../../selectors/multichainAccounts/accountTreeController';
64+
import { selectSelectedInternalAccountByScope } from '../../../selectors/multichainAccounts/accounts';
6365
import { createBuyNavigationDetails } from '../Ramp/Aggregator/routes/utils';
6466
import { TokenI } from '../Tokens/types';
6567
import AssetDetailsActions from '../../../components/Views/AssetDetails/AssetDetailsActions';
@@ -110,6 +112,7 @@ const AssetOverview: React.FC<AssetOverviewProps> = ({
110112
const selectedInternalAccount = useSelector(selectSelectedInternalAccount);
111113
const selectedInternalAccountAddress = selectedInternalAccount?.address;
112114
const selectedAccountGroup = useSelector(selectSelectedAccountGroup);
115+
const getAccountByScope = useSelector(selectSelectedInternalAccountByScope);
113116
const conversionRateByTicker = useSelector(selectCurrencyRates);
114117
const currentCurrency = useSelector(selectCurrentCurrency);
115118
const accountsByChainId = useSelector(selectAccountsByChainId);
@@ -204,12 +207,19 @@ const AssetOverview: React.FC<AssetOverviewProps> = ({
204207
location: ActionLocation.ASSET_DETAILS,
205208
});
206209

210+
const accountForChain =
211+
isNonEvmAsset && asset.chainId
212+
? getAccountByScope(asset.chainId as CaipChainId)
213+
: selectedInternalAccount;
214+
215+
const addressForChain = accountForChain?.address;
216+
207217
// Show QR code for receiving this specific asset
208-
if (selectedInternalAccountAddress && selectedAccountGroup && chainId) {
218+
if (addressForChain && selectedAccountGroup && chainId) {
209219
navigation.navigate(Routes.MODAL.MULTICHAIN_ACCOUNT_DETAIL_ACTIONS, {
210220
screen: Routes.SHEET.MULTICHAIN_ACCOUNT_DETAILS.SHARE_ADDRESS_QR,
211221
params: {
212-
address: selectedInternalAccountAddress,
222+
address: addressForChain,
213223
networkName: networkName || 'Unknown Network',
214224
chainId,
215225
groupId: selectedAccountGroup.id,
@@ -221,9 +231,11 @@ const AssetOverview: React.FC<AssetOverviewProps> = ({
221231
'AssetOverview::onReceive - Missing required data for navigation',
222232
),
223233
{
224-
hasAddress: !!selectedInternalAccountAddress,
234+
hasAddress: !!addressForChain,
225235
hasAccountGroup: !!selectedAccountGroup,
226236
hasChainId: !!chainId,
237+
isNonEvmAsset,
238+
assetChainId: asset.chainId,
227239
},
228240
);
229241
}

0 commit comments

Comments
 (0)