Skip to content

feat: Multichain NFT import #14748

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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
c0da631
fix: Add Network picker to nft import flow
gambinish Apr 18, 2025
b7f194e
fix: Add Network Selector to Import NFT form, add networkClientId sig…
gambinish Apr 19, 2025
0eefa7e
Merge branch 'main' into feat/multichain-nft-import
gambinish Apr 19, 2025
2e23fdc
fix: Lint
gambinish Apr 19, 2025
fc9f5d4
Merge branch 'feat/multichain-nft-import' of github.com:MetaMask/meta…
gambinish Apr 19, 2025
14a546d
fix: Lint tsc
gambinish Apr 19, 2025
dbdf1d1
chore: Update snapshots
gambinish Apr 21, 2025
7ab19b1
chore: Merge main, address conflicts
gambinish Apr 21, 2025
678666d
fix: e2e
gambinish Apr 21, 2025
c7bb118
fix: Incorrect testId
gambinish Apr 22, 2025
4b76a45
fix: e2e nft detail network selection
gambinish Apr 22, 2025
1f58f3e
chore: Update snapshots
gambinish Apr 22, 2025
ad40a0f
fix: Existing unit tests
gambinish Apr 22, 2025
885e5b1
fix: lint
gambinish Apr 22, 2025
f0cbc5a
chore: disable import button when no network is selected
gambinish Apr 22, 2025
caf70e1
Merge branch 'main' into feat/multichain-nft-import
gambinish Apr 22, 2025
02cd49c
chore: Update snapshots
gambinish Apr 22, 2025
69996ae
fix: Temporarily comment out audit:ci step
gambinish Apr 22, 2025
6821e82
fix: Remove warning text
gambinish Apr 22, 2025
377c2ca
chore: Update snapshot
gambinish Apr 22, 2025
29221dd
fix: Replace ci audit
gambinish Apr 22, 2025
066d2db
Merge branch 'main' into feat/multichain-nft-import
gambinish Apr 22, 2025
5baf36a
fix: Break out into utility files, unit tests
gambinish Apr 22, 2025
2e74bf6
fix: Lint tsc
gambinish Apr 22, 2025
aa042d5
Merge branch 'main' into feat/multichain-nft-import
gambinish Apr 22, 2025
5a0f70c
Break out removeNft function, write utility, and unit test
gambinish Apr 22, 2025
ca3833e
Merge branch 'feat/multichain-nft-import' of github.com:MetaMask/meta…
gambinish Apr 22, 2025
16049db
Merge branch 'main' into feat/multichain-nft-import
gambinish Apr 22, 2025
673957d
fix: Lint tsc
gambinish Apr 22, 2025
72bc3c9
chore: Bump unit test coverage
gambinish Apr 23, 2025
cf3933f
fix: Lint
gambinish Apr 23, 2025
1034417
fix: Bump test coverage
gambinish Apr 23, 2025
a07a754
chore: Merge main, address conflicts
gambinish May 6, 2025
7373b39
chore: Pass networkClientId instead of chainId
gambinish May 7, 2025
df8f18d
Merge branch 'main' into feat/multichain-nft-import
gambinish May 7, 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
Expand Up @@ -27,6 +27,10 @@ exports[`AddCustomCollectible should render correctly 1`] = `
}
}
>
<AddCustomCollectible />
<AddCustomCollectible
chainId="0x1"
selectedNetwork="mainnet"
setOpenNetworkSelector={[MockFunction]}
/>
</ContextProvider>
`;
132 changes: 131 additions & 1 deletion app/components/UI/AddCustomCollectible/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
import configureMockStore from 'redux-mock-store';
import { Provider } from 'react-redux';
import initialRootState from '../../../util/test/initial-root-state';
import {
validateCustomCollectibleAddress,
validateCollectibleOwnership,
validateCustomCollectibleTokenId,
} from './util';
import { isValidAddress } from 'ethereumjs-util';
import { isSmartContractAddress } from '../../../util/transactions';
import Engine from '../../../core/Engine';

const mockStore = configureMockStore();

Expand All @@ -14,13 +22,135 @@
useSelector: jest.fn().mockImplementation(() => ''),
}));

jest.mock('ethereumjs-util', () => ({
isValidAddress: jest.fn(),
}));

jest.mock('../../../util/transactions', () => ({
isSmartContractAddress: jest.fn(),
}));

jest.mock('../../../../locales/i18n', () => ({
strings: jest.fn((key) => key),
}));

jest.mock('../../../core/Engine', () => ({
context: {
NftController: {
isNftOwner: jest.fn(),
},
},
}));

describe('AddCustomCollectible', () => {
it('should render correctly', () => {
const wrapper = shallow(
<Provider store={store}>
<AddCustomCollectible />
<AddCustomCollectible
setOpenNetworkSelector={jest.fn()}
selectedNetwork={'mainnet'}

Check failure on line 51 in app/components/UI/AddCustomCollectible/index.test.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint:tsc)

Type 'string' is not assignable to type 'NetworkConfiguration'.
chainId={'0x1'}
/>
</Provider>,
);
expect(wrapper).toMatchSnapshot();
});
});

describe('validateCustomCollectibleAddress', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('should return invalid when address is empty', async () => {
const result = await validateCustomCollectibleAddress('', '0x1');
expect(result).toEqual({
isValid: false,
warningMessage: 'collectible.address_cant_be_empty',
});
});

it('should return invalid when address is not a valid Ethereum address', async () => {
(isValidAddress as jest.Mock).mockReturnValue(false);
const result = await validateCustomCollectibleAddress(
'invalid-address',
'0x1',
);
expect(result).toEqual({
isValid: false,
warningMessage: 'collectible.address_must_be_valid',
});
});

it('should return invalid when address is not a smart contract', async () => {
(isValidAddress as jest.Mock).mockReturnValue(true);
(isSmartContractAddress as jest.Mock).mockResolvedValue(false);
const result = await validateCustomCollectibleAddress('0x123', '0x1');
expect(result).toEqual({
isValid: false,
warningMessage: 'collectible.address_must_be_smart_contract',
});
});

it('should return valid when address is a valid smart contract', async () => {
(isValidAddress as jest.Mock).mockReturnValue(true);
(isSmartContractAddress as jest.Mock).mockResolvedValue(true);
const result = await validateCustomCollectibleAddress('0x123', '0x1');
expect(result).toEqual({
isValid: true,
warningMessage: '',
});
});

it('should not check smart contract when chainId is not provided', async () => {
(isValidAddress as jest.Mock).mockReturnValue(true);
const result = await validateCustomCollectibleAddress('0x123');
expect(result).toEqual({
isValid: true,
warningMessage: '',
});
expect(isSmartContractAddress).not.toHaveBeenCalled();
});
});

describe('validateCollectibleOwnership', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('should return false with error when verification fails', async () => {
(Engine.context.NftController.isNftOwner as jest.Mock).mockRejectedValue(
new Error('Verification failed'),
);
const result = await validateCollectibleOwnership('0x123', '0x456', '789');
expect(result).toEqual({
isOwner: false,
error: {
title: 'collectible.ownership_verification_error_title',
message: 'collectible.ownership_verification_error',
},
});
});
});

describe('validateCustomCollectibleTokenId', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('should return invalid when tokenId is empty', () => {
const result = validateCustomCollectibleTokenId('');
expect(result).toEqual({
isValid: false,
warningMessage: 'collectible.token_id_cant_be_empty',
});
});

it('should return valid when tokenId is not empty', () => {
const result = validateCustomCollectibleTokenId('123');
expect(result).toEqual({
isValid: true,
warningMessage: '',
});
});
});
104 changes: 45 additions & 59 deletions app/components/UI/AddCustomCollectible/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,24 @@ import { Alert, Text, TextInput, View, StyleSheet } from 'react-native';
import { fontStyles } from '../../../styles/common';
import Engine from '../../../core/Engine';
import { strings } from '../../../../locales/i18n';
import { isValidAddress } from 'ethereumjs-util';
import ActionView from '../ActionView';
import { isSmartContractAddress } from '../../../util/transactions';
import Device from '../../../util/device';
import { MetaMetricsEvents } from '../../../core/Analytics';

import {
validateCustomCollectibleAddress as validateAddress,
validateCollectibleOwnership as validateOwnership,
validateCustomCollectibleTokenId as validateTokenId,
} from './util';
import { useTheme } from '../../../util/theme';
import { NFTImportScreenSelectorsIDs } from '../../../../e2e/selectors/wallet/ImportNFTView.selectors';
import { selectChainId } from '../../../selectors/networkController';
import { selectSelectedInternalAccountFormattedAddress } from '../../../selectors/accountsController';
import { getDecimalChainId } from '../../../util/networks';
import { useMetrics } from '../../../components/hooks/useMetrics';
import Logger from '../../../util/Logger';
import { NetworkSelectorDropdown } from '../AddCustomToken/NetworkSelectorDropdown';
import { Hex } from '@metamask/utils';
import { TraceName, endTrace, trace } from '../../../util/trace';
import { NetworkConfiguration } from '@metamask/network-controller';

// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -63,11 +67,17 @@ interface AddCustomCollectibleProps {
collectibleContract?: {
address: string;
};
setOpenNetworkSelector: (val: boolean) => void;
selectedNetwork: NetworkConfiguration | null;
chainId: string | null;
}

const AddCustomCollectible = ({
navigation,
collectibleContract,
setOpenNetworkSelector,
selectedNetwork,
chainId,
}: AddCustomCollectibleProps) => {
const [mounted, setMounted] = useState<boolean>(true);
const [address, setAddress] = useState<string>('');
Expand All @@ -88,7 +98,8 @@ const AddCustomCollectible = ({
const selectedAddress = useSelector(
selectSelectedInternalAccountFormattedAddress,
);
const chainId = useSelector(selectChainId);

const defaultRpcIndex = selectedNetwork?.defaultRpcEndpointIndex || 0;

useEffect(() => {
setMounted(true);
Expand Down Expand Up @@ -116,32 +127,15 @@ const AddCustomCollectible = ({
}, [chainId]);

const validateCustomCollectibleAddress = async (): Promise<boolean> => {
let validated = true;
const isValidEthAddress = isValidAddress(address);
if (address.length === 0) {
setWarningAddress(strings('collectible.address_cant_be_empty'));
validated = false;
} else if (!isValidEthAddress) {
setWarningAddress(strings('collectible.address_must_be_valid'));
validated = false;
} else if (!(await isSmartContractAddress(address, chainId))) {
setWarningAddress(strings('collectible.address_must_be_smart_contract'));
validated = false;
} else {
setWarningAddress(``);
}
return validated;
const result = await validateAddress(address, chainId);
setWarningAddress(result.warningMessage);
return result.isValid;
};

const validateCustomCollectibleTokenId = (): boolean => {
let validated = false;
if (tokenId.length === 0) {
setWarningTokenId(strings('collectible.token_id_cant_be_empty'));
} else {
setWarningTokenId(``);
validated = true;
}
return validated;
const result = validateTokenId(tokenId);
setWarningTokenId(result.warningMessage);
return result.isValid;
};

const validateCustomCollectible = async (): Promise<boolean> => {
Expand All @@ -150,37 +144,18 @@ const AddCustomCollectible = ({
return validatedAddress && validatedTokenId;
};

/**
* Method to validate collectible ownership.
*
* @returns Promise that resolves ownershio as a boolean.
*/
const validateCollectibleOwnership = async (): Promise<boolean> => {
try {
// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { NftController } = Engine.context as any;
const isOwner = await NftController.isNftOwner(
selectedAddress,
address,
tokenId,
);

if (!isOwner)
Alert.alert(
strings('collectible.not_owner_error_title'),
strings('collectible.not_owner_error'),
);

return isOwner;
} catch {
Alert.alert(
strings('collectible.ownership_verification_error_title'),
strings('collectible.ownership_verification_error'),
);

return false;
if (!address) return false;
const result = await validateOwnership(
selectedAddress ?? '',
address,
tokenId,
selectedNetwork?.rpcEndpoints[defaultRpcIndex].networkClientId,
);
if (!result.isOwner && result.error) {
Alert.alert(result.error.title, result.error.message);
}
return result.isOwner;
};

const addNft = async (): Promise<void> => {
Expand All @@ -200,7 +175,11 @@ const AddCustomCollectible = ({

trace({ name: TraceName.ImportNfts });

await NftController.addNft(address, tokenId);
NftController.addNft(address, tokenId, {
networkClientId:
selectedNetwork?.rpcEndpoints[defaultRpcIndex].networkClientId,
userAddress: selectedAddress,
});

endTrace({ name: TraceName.ImportNfts });

Expand Down Expand Up @@ -240,10 +219,17 @@ const AddCustomCollectible = ({
confirmText={strings('add_asset.collectibles.add_collectible')}
onCancelPress={cancelAddCollectible}
onConfirmPress={addNft}
confirmDisabled={!address || !tokenId}
confirmDisabled={!address || !tokenId || !selectedNetwork}
loading={loading}
>
<View>
<View style={styles.rowWrapper}>
<NetworkSelectorDropdown
setOpenNetworkSelector={setOpenNetworkSelector}
selectedNetwork={selectedNetwork?.name ?? ''}
chainId={chainId as Hex}
/>
</View>
<View style={styles.rowWrapper}>
<Text style={styles.rowTitleText}>
{strings('collectible.collectible_address')}
Expand Down
Loading
Loading