feat: improve nft support + dont send funds to assetId (#1648)

- Don't allow sending funds to assetId - Improve logic of getting nft data - Fix flaky test on transaction approval due to button entering/leaving loading state
FuelLabs · Nov 7, 2024 · e63d6eb · e63d6eb
1 parent 7cf54ae
commit e63d6eb
Show file tree

Hide file tree

Showing 13 changed files with 241 additions and 160 deletions.
diff --git a/.changeset/fresh-poets-count.md b/.changeset/fresh-poets-count.md
@@ -0,0 +1,6 @@
+---
+"@fuel-wallet/types": patch
+"fuels-wallet": patch
+---
+
+feat: improve nft support + dont allow sending funds to assetId address
diff --git a/packages/app/package.json b/packages/app/package.json
@@ -26,6 +26,7 @@
     "@fuel-ui/test-utils": "0.17.0",
     "@fuel-wallet/connections": "workspace:*",
     "@fuels/local-storage": "0.20.0",
+    "@fuels/playwright-utils": "workspace:*",
     "@fuels/react-xstore": "0.20.0",
     "@hookform/resolvers": "3.9.0",
     "@react-aria/utils": "3.21.0",

diff --git a/packages/app/playwright.config.ts b/packages/app/playwright.config.ts
@@ -8,7 +8,7 @@ const IS_CI = process.env.CI;
 
 export const playwrightConfig: PlaywrightTestConfig = {
   workers: 1,
-  retries: 1,
+  retries: IS_CI ? 1 : 0,
   testMatch: join(__dirname, './playwright/**/*.test.ts'),
   testDir: join(__dirname, './playwright/'),
   outputDir: join(__dirname, './playwright-results/'),
@@ -30,6 +30,7 @@ export const playwrightConfig: PlaywrightTestConfig = {
     trace: 'on-first-retry',
     actionTimeout: 5000,
     screenshot: 'only-on-failure',
+    headless: false,
   },
   // ignore lock test because it takes too long and it will be tested in a separate config
   testIgnore: [join(__dirname, './playwright/crx/lock.test.ts')],

diff --git a/packages/app/playwright/e2e/SendTransaction.test.ts b/packages/app/playwright/e2e/SendTransaction.test.ts
@@ -1,14 +1,8 @@
 import type { Account } from '@fuel-wallet/types';
+import { expectButtonToBeEnabled } from '@fuels/playwright-utils';
 import type { Browser, Page } from '@playwright/test';
 import test, { chromium, expect } from '@playwright/test';
-import {
-  type Bech32Address,
-  Provider,
-  Wallet,
-  bn,
-  fromBech32,
-  toB256,
-} from 'fuels';
+import { Provider, Wallet, bn } from 'fuels';
 
 import {
   getButtonByText,
@@ -62,7 +56,13 @@ test.describe('SendTransaction', () => {
     await getInputByName(page, 'amount').fill('0.001');
 
     // Submit transaction
-    await getButtonByText(page, 'Review').click();
+
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     await getButtonByText(page, 'Approve').click();
     await hasText(page, '0.001 ETH');
@@ -92,7 +92,13 @@ test.describe('SendTransaction', () => {
     await getInputByName(page, 'amount').fill('0.001');
 
     // Submit transaction
-    await getButtonByText(page, 'Review').click();
+    // make sure the button is enabled
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     // Approve transaction
     await hasText(page, '0.001 ETH');
@@ -131,7 +137,13 @@ test.describe('SendTransaction', () => {
     await hasText(page, 'Balance: 1,000,000.000');
 
     // Submit transaction
-    await getButtonByText(page, 'Review').click();
+
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     // Approve transaction
     await hasText(page, `0.01 ${ALT_ASSET.symbol}`);
@@ -163,7 +175,12 @@ test.describe('SendTransaction', () => {
     await page.waitForSelector('button:has-text("Review")');
     await page.waitForTimeout(1000);
 
-    await getButtonByText(page, 'Review').click();
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     await hasText(page, '0.001 ETH');
 
@@ -200,7 +217,12 @@ test.describe('SendTransaction', () => {
     await page.waitForSelector('button:has-text("Review")');
     await page.waitForTimeout(1000);
 
-    await getButtonByText(page, 'Review').click();
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     await hasText(page, '0.001 ETH');
 
@@ -241,7 +263,12 @@ test.describe('SendTransaction', () => {
     await page.waitForSelector('button:has-text("Review")');
     await page.waitForTimeout(1000);
 
-    await getButtonByText(page, 'Review').click();
+    const btnLocatorBeforeApprv = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocatorBeforeApprv);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocatorBeforeApprv);
+    await btnLocatorBeforeApprv.click();
 
     // Waiting button change to Approve in order to get updated fee amount
     await page.waitForSelector('button:has-text("Approve")');
@@ -258,7 +285,12 @@ test.describe('SendTransaction', () => {
     await page.waitForSelector('button:has-text("Review")');
     await page.waitForTimeout(1000);
 
-    await getButtonByText(page, 'Review').click();
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     // Waiting button change to Approve in order to get updated fee amount
     await page.waitForSelector('button:has-text("Approve")');
@@ -308,7 +340,13 @@ test.describe('SendTransaction', () => {
     const maxAmountAfterFee = await getInputByName(page, 'amount').inputValue();
 
     // Submit transaction
-    await getButtonByText(page, 'Review').click();
+
+    const btnLocator = getButtonByText(page, 'Review');
+
+    await expectButtonToBeEnabled(btnLocator);
+    await page.waitForTimeout(5000);
+    await expectButtonToBeEnabled(btnLocator);
+    await btnLocator.click();
 
     // Approve transaction
     await hasText(page, `${maxAmountAfterFee} ETH`);

diff --git a/packages/app/src/systems/Account/components/BalanceAssets/BalanceAssets.tsx b/packages/app/src/systems/Account/components/BalanceAssets/BalanceAssets.tsx
@@ -1,6 +1,7 @@
 import { Button, CardList } from '@fuel-ui/react';
 import type { CoinAsset } from '@fuel-wallet/types';
 import { useMemo, useState } from 'react';
+import { isUnknownAsset } from '~/systems/Asset';
 import { AssetItem, AssetList } from '~/systems/Asset/components';
 import type { AssetListEmptyProps } from '~/systems/Asset/components/AssetList/AssetListEmpty';
 
@@ -21,31 +22,42 @@ export const BalanceAssets = ({
 }: BalanceAssetListProp) => {
   const [showUnknown, setShowUnknown] = useState(false);
   const unknownLength = useMemo(
-    () => balances?.filter((balance) => !balance.asset?.name).length,
+    () =>
+      balances?.filter(
+        (balance) => balance.asset && isUnknownAsset(balance.asset)
+      ).length,
     [balances]
   );
 
   if (isLoading) return <AssetList.Loading items={4} />;
   const isEmpty = !balances || !balances.length;
   if (isEmpty) return <AssetList.Empty {...emptyProps} />;
   const balancesToShow = balances.filter(
-    (balance) => showUnknown || balance.asset?.name
+    (balance) =>
+      showUnknown || (balance.asset && !isUnknownAsset(balance.asset))
   );
 
   function toggle() {
     setShowUnknown((s) => !s);
   }
   return (
     <CardList>
-      {balancesToShow.map((balance) => (
-        <AssetItem
-          key={balance.asset?.name}
-          fuelAsset={balance.asset}
-          amount={balance.amount}
-          onRemove={onRemove}
-          onEdit={onEdit}
-        />
-      ))}
+      {balancesToShow.map((balance) => {
+        if (!balance.asset) return null;
+
+        const shouldShowAddAssetBtn = isUnknownAsset(balance.asset);
+
+        return (
+          <AssetItem
+            key={balance.asset?.name}
+            fuelAsset={balance.asset}
+            amount={balance.amount}
+            onRemove={onRemove}
+            onEdit={onEdit}
+            shouldShowAddAssetBtn={shouldShowAddAssetBtn}
+          />
+        );
+      })}
       {!!(!isLoading && unknownLength) && (
         <Button size="xs" variant="link" onPress={toggle}>
           {showUnknown ? 'Hide' : 'Show'} unknown assets ({unknownLength})

diff --git a/packages/app/src/systems/Asset/cache/AssetsCache.ts b/packages/app/src/systems/Asset/cache/AssetsCache.ts
@@ -1,7 +1,7 @@
 import type { AssetData } from '@fuel-wallet/types';
 import type { Asset, Provider } from 'fuels';
 import { db } from '~/systems/Core/utils/database';
-import { isNft } from '../utils/isNft';
+import { fetchNftData } from '../utils/nft';
 
 type Endpoint = {
   chainId: number;
@@ -45,7 +45,8 @@ export class AssetsCache {
         timeout,
       ]);
       if (response instanceof Response) {
-        return response.json();
+        const jsonResponse = await response.json();
+        return jsonResponse;
       }
     } catch (_e: unknown) {}
   }
@@ -61,39 +62,49 @@ export class AssetsCache {
     const endpoint = this.getIndexerEndpoint(chainId);
     if (!endpoint) return;
     // try to get from memory cache first
-    this.cache[endpoint.chainId] = this.cache[endpoint.chainId] || {};
-    if (this.cache[endpoint.chainId][assetId]) {
-      return this.cache[endpoint.chainId][assetId];
+    this.cache[chainId] = this.cache[chainId] || {};
+    const assetFromCache = this.cache[chainId][assetId];
+    if (assetFromCache?.name) {
+      return assetFromCache;
     }
+
     // get from indexed db if not in memory
-    const savedAsset = await this.storage.getItem(
-      `${endpoint.chainId}/${assetId}`
-    );
-    if (savedAsset) {
-      this.cache[endpoint.chainId][assetId] = savedAsset;
-      return savedAsset;
+    const assetFromDb = await this.storage.getItem(`${chainId}/${assetId}`);
+    if (assetFromDb?.name) {
+      this.cache[chainId][assetId] = assetFromDb;
+      return assetFromDb;
     }
+
     const assetFromIndexer = await this.fetchAssetFromIndexer(
       endpoint.url,
       assetId
     );
+    console.log('asd assetFromIndexer', assetFromIndexer);
     if (!assetFromIndexer) return;
 
-    const isNftAsset = await isNft({
-      assetId,
-      contractId: assetFromIndexer.contractId,
-      provider,
-    });
-
     const asset = {
       ...assetFromIndexer,
-      isNft: isNftAsset,
+      isNft: false,
     };
 
-    this.cache[endpoint.chainId][assetId] = asset;
-    this.storage.setItem(`${endpoint.chainId}/${assetId}`, asset);
+    if (assetFromIndexer.contractId) {
+      const nftData = await fetchNftData({
+        assetId,
+        contractId: assetFromIndexer.contractId,
+        provider,
+      });
+      Object.assign(asset, nftData);
+    }
+
+    this.cache[chainId][assetId] = asset;
+    this.storage.setItem(`${chainId}/${assetId}`, asset);
     return asset;
   }
+  asset = {
+    name: '',
+    symbol: '',
+    metadata: {},
+  };
 
   static getInstance() {
     if (!AssetsCache.instance) {

diff --git a/packages/app/src/systems/Asset/components/AssetItem/AssetItem.tsx b/packages/app/src/systems/Asset/components/AssetItem/AssetItem.tsx
@@ -36,6 +36,7 @@ export type AssetItemProps = {
   showActions?: boolean;
   onRemove?: (assetId: string) => void;
   onEdit?: (assetId: string) => void;
+  shouldShowAddAssetBtn?: boolean;
 };
 
 type AssetItemComponent = FC<AssetItemProps> & {
@@ -49,6 +50,7 @@ export const AssetItem: AssetItemComponent = ({
   showActions,
   onRemove,
   onEdit,
+  shouldShowAddAssetBtn,
 }) => {
   const navigate = useNavigate();
   const { visibility } = useBalanceVisibility();
@@ -194,7 +196,7 @@ export const AssetItem: AssetItemComponent = ({
                 NFT
               </Badge>
             )}
-            {(!name || asset.indexed) && !asset.isNft && (
+            {shouldShowAddAssetBtn && (
               <Button
                 size="xs"
                 intent="primary"

diff --git a/packages/app/src/systems/Asset/utils/assetId.ts b/packages/app/src/systems/Asset/utils/assetId.ts
@@ -1,3 +1,4 @@
+import type { AssetFuelData } from '@fuel-wallet/types';
 import {
   type Asset,
   type AssetFuel,
@@ -68,3 +69,7 @@ export const getFuelAssetByAssetId = async (input: {
     assetId: input.assetId,
   };
 };
+
+export const isUnknownAsset = (asset: AssetFuelData) => {
+  return !asset.name && !asset.verified && !asset.isCustom && !asset.isNft;
+};
diff --git a/...ages/app/src/systems/Asset/utils/isNft.ts → packages/app/src/systems/Asset/utils/nft.ts b/...ages/app/src/systems/Asset/utils/isNft.ts → packages/app/src/systems/Asset/utils/nft.ts
@@ -1,6 +1,6 @@
 import { Contract, type Provider } from 'fuels';
 
-export const isNft = async ({
+export const fetchNftData = async ({
   assetId,
   contractId,
   provider,
@@ -11,16 +11,22 @@ export const isNft = async ({
     .multiCall([
       contract.functions.total_supply({ bits: assetId }),
       contract.functions.decimals({ bits: assetId }),
+      contract.functions.name({ bits: assetId }),
+      contract.functions.symbol({ bits: assetId }),
     ])
     .dryRun();
 
-  const [total_supply, decimals] = result.value;
+  const [total_supply, decimals, name, symbol] = result.value;
 
-  /*
-    according to sway standards this is how you recognize an NFT:
-    https://docs.fuel.network/docs/sway-standards/src-20-native-asset/#non-fungible-asset-restrictions
-  */
-  return total_supply.toNumber() === 1 && !decimals;
+  return {
+    /*
+      according to sway standards this is how you recognize an NFT:
+      https://docs.fuel.network/docs/sway-standards/src-20-native-asset/#non-fungible-asset-restrictions
+    */
+    isNft: total_supply.toNumber() === 1 && !decimals,
+    name: name as string,
+    symbol: symbol as string,
+  };
 };
 
 const SRC_20_ABI = {