From abc5e855a3cc119b1ba64d28cf2a3d1676661325 Mon Sep 17 00:00:00 2001 From: thibautbremand Date: Wed, 18 Oct 2023 10:29:31 +0200 Subject: [PATCH] Refactor mintNFT messaging --- .../api/src/helpers/extensionMessaging.ts | 8 --- packages/extension/cypress/e2e/NFT.cy.ts | 57 +++++++++---------- .../extension/cypress/utils/navigation.ts | 12 ++-- .../src/chromeServices/background/index.ts | 19 +++---- .../src/components/pages/MintNFT/MintNFT.tsx | 48 ++++++++++------ .../extension/src/utils/baseParams.test.ts | 48 ++++++++++++++++ packages/extension/src/utils/baseParams.ts | 35 ++++++++++++ .../src/utils/parseFromString.test.ts | 20 ------- .../extension/src/utils/parseFromString.ts | 33 ----------- 9 files changed, 157 insertions(+), 123 deletions(-) diff --git a/packages/api/src/helpers/extensionMessaging.ts b/packages/api/src/helpers/extensionMessaging.ts index 222dae732..57d2e9f7d 100644 --- a/packages/api/src/helpers/extensionMessaging.ts +++ b/packages/api/src/helpers/extensionMessaging.ts @@ -80,10 +80,6 @@ export const sendMessageToContentScript = (msg: APIMessages): Promise => { */ const serializeMessage = (msg: APIMessages): any => { const modifiedMsg: any = { ...msg }; - if (modifiedMsg.payload?.memos) { - modifiedMsg.payload.memos = JSON.stringify(modifiedMsg.payload.memos); - } - if (modifiedMsg.payload?.signers) { modifiedMsg.payload.signers = JSON.stringify(modifiedMsg.payload.signers); } @@ -108,10 +104,6 @@ const serializeMessage = (msg: APIMessages): any => { modifiedMsg.payload.NFTokenBrokerFee = JSON.stringify(modifiedMsg.payload.NFTokenBrokerFee); } - if (typeof modifiedMsg.payload?.flags === 'object') { - modifiedMsg.payload.flags = JSON.stringify(modifiedMsg.payload.flags); - } - if (modifiedMsg.payload?.NFTokenOffers) { modifiedMsg.payload.NFTokenOffers = JSON.stringify(modifiedMsg.payload.NFTokenOffers); } diff --git a/packages/extension/cypress/e2e/NFT.cy.ts b/packages/extension/cypress/e2e/NFT.cy.ts index 4b8da35e1..be1b66c1d 100644 --- a/packages/extension/cypress/e2e/NFT.cy.ts +++ b/packages/extension/cypress/e2e/NFT.cy.ts @@ -2,10 +2,33 @@ import { Network, NETWORK } from '@gemwallet/constants'; +import { navigate } from '../utils/navigation'; + +// deepcode ignore NoHardcodedPasswords: password used for testing purposes +const PASSWORD = 'SECRET_PASSWORD'; + describe('Mint', () => { - // deepcode ignore NoHardcodedPasswords: password used for testing purposes - const PASSWORD = 'SECRET_PASSWORD'; - const MINT_NFT_URL = `http://localhost:3000?URI=4d696e746564207468726f7567682047656d57616c6c657421&flags=%7B%22tfOnlyXRP%22%3Afalse%2C%22tfTransferable%22%3Atrue%7D&fee=199&transferFee=3000&NFTokenTaxon=0&memos=%5B%7B%22memo%22%3A%7B%22memoType%22%3A%224465736372697074696f6e%22%2C%22memoData%22%3A%2254657374206d656d6f%22%7D%7D%5D&id=210324818&requestMessage=undefined&transaction=mintNFT`; + const MINT_NFT_URL = + 'http://localhost:3000?mint-nft&storageKey=1693425372955.3833&id=210401976&requestMessage=undefined&transaction=mintNFT'; + + const params = { + fee: '199', + transferFee: 3000, + NFTokenTaxon: 0, + memos: [ + { + memo: { + memoType: '4465736372697074696f6e', + memoData: '54657374206d656d6f' + } + } + ], + flags: { + tfOnlyXRP: false, + tfTransferable: true + }, + URI: '4d696e746564207468726f7567682047656d57616c6c657421' + }; beforeEach(() => { // Mock the localStorage with a wallet already loaded @@ -24,7 +47,7 @@ describe('Mint', () => { }); it('Mint NFT', () => { - navigate(MINT_NFT_URL, PASSWORD); + navigate(MINT_NFT_URL, PASSWORD, '1693425372955.3833', params); cy.get('h1[data-testid="page-title"]').should('have.text', 'Mint NFT'); @@ -334,30 +357,4 @@ describe('Mint', () => { }); cy.get('p[data-testid="transaction-subtitle"]').should('have.text', 'Transaction Successful'); }); - - const navigate = (url: string, password: string) => { - cy.visit(url, { - onBeforeLoad(win) { - (win as any).chrome = (win as any).chrome || {}; - (win as any).chrome.runtime = { - sendMessage(message, cb) {} - }; - - (win as any).chrome.storage = { - local: { - get(key, cb) {}, - set(obj, cb) { - if (cb) cb(); - } - } - }; - - cy.stub((win as any).chrome.runtime, 'sendMessage').resolves({}); - } - }); - - // Login - cy.get('input[name="password"]').type(password); - cy.contains('button', 'Unlock').click(); - }; }); diff --git a/packages/extension/cypress/utils/navigation.ts b/packages/extension/cypress/utils/navigation.ts index f271013f9..d294190a6 100644 --- a/packages/extension/cypress/utils/navigation.ts +++ b/packages/extension/cypress/utils/navigation.ts @@ -1,6 +1,6 @@ const mockSessionStorage = {}; -export const navigate = (url: string, password: string, storageKey: string, data) => { +export const navigate = (url: string, password: string, storageKey?: string, data?) => { cy.visit(url, { onBeforeLoad(win) { (win as any).chrome = (win as any).chrome || {}; @@ -30,11 +30,15 @@ export const navigate = (url: string, password: string, storageKey: string, data } }; - (win as any).chrome.storage.session.set({ - [storageKey]: { [storageKey]: JSON.stringify(data) } - }); + if (storageKey && data) { + (win as any).chrome.storage.session.set({ + [storageKey]: { [storageKey]: JSON.stringify(data) } + }); + } (win as any).chrome.runtime.lastError = null; + + cy.stub((win as any).chrome.runtime, 'sendMessage').resolves({}); } }); diff --git a/packages/extension/src/chromeServices/background/index.ts b/packages/extension/src/chromeServices/background/index.ts index a1b0c522b..6ae8cca66 100644 --- a/packages/extension/src/chromeServices/background/index.ts +++ b/packages/extension/src/chromeServices/background/index.ts @@ -292,16 +292,15 @@ chrome.runtime.onMessage.addListener( } }); } else if (type === 'REQUEST_MINT_NFT/V3') { - handleTransactionRequest({ - payload: message.payload, - sender, - parameter: PARAMETER_TRANSACTION_MINT_NFT, - receivingMessage: 'RECEIVE_MINT_NFT/V3', - errorPayload: { - type: ResponseType.Reject, - result: undefined - } - }); + const { payload } = message; + try { + sendInMemoryMessage({ + payload, + parameter: PARAMETER_TRANSACTION_MINT_NFT, + receivingMessage: 'RECEIVE_MINT_NFT/V3', + sender + }); + } catch (e) {} } else if (type === 'REQUEST_CREATE_NFT_OFFER/V3') { handleTransactionRequest({ payload: message.payload, diff --git a/packages/extension/src/components/pages/MintNFT/MintNFT.tsx b/packages/extension/src/components/pages/MintNFT/MintNFT.tsx index 56b87c8c5..67547d871 100644 --- a/packages/extension/src/components/pages/MintNFT/MintNFT.tsx +++ b/packages/extension/src/components/pages/MintNFT/MintNFT.tsx @@ -5,10 +5,12 @@ import { NFTokenMint } from 'xrpl/dist/npm/models/transactions/NFTokenMint'; import { API_ERROR_BAD_REQUEST, GEM_WALLET, + MintNFTRequest, ReceiveMintNFTBackgroundMessage, ResponseType } from '@gemwallet/constants'; +import { STORAGE_MESSAGING_KEY } from '../../../constants'; import { buildNFTokenMint, TransactionProgressStatus, @@ -17,10 +19,9 @@ import { useTransactionProgress, useWallet } from '../../../contexts'; -import { useFees, useTransactionStatus } from '../../../hooks'; +import { useFees, useFetchFromSessionStorage, useTransactionStatus } from '../../../hooks'; import { TransactionStatus } from '../../../types'; -import { parseMintNFTFlags } from '../../../utils'; -import { parseBaseParamsFromURLParamsNew } from '../../../utils/baseParams'; +import { parseBaseParamsFromStoredData } from '../../../utils/baseParams'; import { serializeError } from '../../../utils/errors'; import { TransactionDetails } from '../../organisms'; import { TransactionPage } from '../../templates'; @@ -55,6 +56,14 @@ export const MintNFT: FC = () => { params.transaction?.Fee ); + const queryString = window.location.search; + const urlParams = new URLSearchParams(queryString); + const { fetchedData } = useFetchFromSessionStorage( + urlParams.get(STORAGE_MESSAGING_KEY) ?? undefined + ) as { + fetchedData: MintNFTRequest | undefined; + }; + const sendMessageToBackground = useCallback( (message: ReceiveMintNFTBackgroundMessage) => { chrome.runtime.sendMessage(message); @@ -114,30 +123,33 @@ export const MintNFT: FC = () => { const queryString = window.location.search; const urlParams = new URLSearchParams(queryString); const id = Number(urlParams.get('id')) || 0; - - // MintNFT fields - const URI = urlParams.get('URI'); - const flags = parseMintNFTFlags(urlParams.get('flags')); - const transferFee = urlParams.get('transferFee') ? Number(urlParams.get('transferFee')) : null; - const NFTokenTaxon = Number(urlParams.get('NFTokenTaxon')) ?? 0; - const issuer = urlParams.get('issuer'); const wallet = getCurrentWallet(); - if (!URI && !flags && !transferFee && !issuer) { - // At least one parameter should be present to mint an NFT - // It would still work, but we assume it's an error from the caller + if (!wallet) { setIsParamsMissing(true); + return; } - if (!wallet) { - setIsParamsMissing(true); + if (!fetchedData) { return; } + const URI = 'URI' in fetchedData ? fetchedData.URI : undefined; + const flags = 'flags' in fetchedData ? fetchedData.flags : undefined; + const transferFee = 'transferFee' in fetchedData ? fetchedData.transferFee : undefined; + const NFTokenTaxon = 'NFTokenTaxon' in fetchedData ? fetchedData.NFTokenTaxon : undefined; + const issuer = 'issuer' in fetchedData ? fetchedData.issuer : undefined; + + if (!URI && !flags && !transferFee && !issuer) { + // At least one parameter should be present to mint an NFT + // It would still work, but we assume it's an error from the caller + setIsParamsMissing(true); + } + const transaction = buildNFTokenMint( { - ...parseBaseParamsFromURLParamsNew(urlParams), - NFTokenTaxon: NFTokenTaxon, + ...parseBaseParamsFromStoredData(fetchedData), + NFTokenTaxon: NFTokenTaxon ?? 0, ...(issuer && { issuer }), ...(transferFee && { transferFee }), ...(URI && { URI }), @@ -150,7 +162,7 @@ export const MintNFT: FC = () => { id, transaction }); - }, [getCurrentWallet]); + }, [fetchedData, getCurrentWallet]); const handleReject = useCallback(() => { setTransaction(TransactionStatus.Rejected); diff --git a/packages/extension/src/utils/baseParams.test.ts b/packages/extension/src/utils/baseParams.test.ts index 45cbcf62f..98f1f5106 100644 --- a/packages/extension/src/utils/baseParams.test.ts +++ b/packages/extension/src/utils/baseParams.test.ts @@ -1,6 +1,7 @@ import { getBaseFromParams, initialBaseTransactionParams, + parseBaseParamsFromStoredData, parseBaseParamsFromURLParams, parseBaseParamsFromURLParamsNew } from './baseParams'; @@ -116,3 +117,50 @@ describe('getBaseFromParams', () => { }); }); }); + +describe('parseBaseParamsFromStoredData', () => { + it('should parse all values correctly', () => { + const storedObject = { + fee: '12', + sequence: '2', + accountTxnID: 'abc123', + lastLedgerSequence: '3456', + memos: ['memo1'], + signers: ['signer1'], + sourceTag: '1', + signingPubKey: 'pubKey1', + ticketSequence: '1', + txnSignature: 'txnSig1' + }; + + const result = parseBaseParamsFromStoredData(storedObject); + + expect(result.fee).toEqual('12'); + expect(result.sequence).toEqual(2); + expect(result.accountTxnID).toEqual('abc123'); + expect(result.lastLedgerSequence).toEqual(3456); + expect(result.memos).toEqual(['memo1']); + expect(result.signers).toEqual(['signer1']); + expect(result.sourceTag).toEqual(1); + expect(result.signingPubKey).toEqual('pubKey1'); + expect(result.ticketSequence).toEqual(1); + expect(result.txnSignature).toEqual('txnSig1'); + }); + + it('should return undefined for missing values', () => { + const storedObject = {}; + + const result = parseBaseParamsFromStoredData(storedObject); + + expect(result.fee).toBeUndefined(); + expect(result.sequence).toBeUndefined(); + expect(result.accountTxnID).toBeUndefined(); + expect(result.lastLedgerSequence).toBeUndefined(); + expect(result.memos).toBeUndefined(); + expect(result.signers).toBeUndefined(); + expect(result.sourceTag).toBeUndefined(); + expect(result.signingPubKey).toBeUndefined(); + expect(result.ticketSequence).toBeUndefined(); + expect(result.txnSignature).toBeUndefined(); + }); +}); diff --git a/packages/extension/src/utils/baseParams.ts b/packages/extension/src/utils/baseParams.ts index a7271e46a..7a76f4c5b 100644 --- a/packages/extension/src/utils/baseParams.ts +++ b/packages/extension/src/utils/baseParams.ts @@ -53,6 +53,41 @@ export const parseBaseParamsFromURLParamsNew = ( return result; }; +export const parseBaseParamsFromStoredData = (storedObject: any): BaseTransactionParamsNew => { + const result: Partial = {}; + + const addParam = ( + key: T, + value: BaseTransactionParamsNew[T] | null + ) => { + if (value !== null && value !== undefined) { + result[key] = value; + } + }; + + addParam('fee', 'fee' in storedObject ? checkFee(storedObject.fee) : undefined); + addParam('sequence', 'sequence' in storedObject ? Number(storedObject.sequence) : undefined); + addParam('accountTxnID', 'accountTxnID' in storedObject ? storedObject.accountTxnID : undefined); + addParam( + 'lastLedgerSequence', + 'lastLedgerSequence' in storedObject ? Number(storedObject.lastLedgerSequence) : undefined + ); + addParam('memos', 'memos' in storedObject ? storedObject.memos : undefined); + addParam('signers', 'signers' in storedObject ? storedObject.signers : undefined); + addParam('sourceTag', 'sourceTag' in storedObject ? Number(storedObject.sourceTag) : undefined); + addParam( + 'signingPubKey', + 'signingPubKey' in storedObject ? storedObject.signingPubKey : undefined + ); + addParam( + 'ticketSequence', + 'ticketSequence' in storedObject ? Number(storedObject.ticketSequence) : undefined + ); + addParam('txnSignature', 'txnSignature' in storedObject ? storedObject.txnSignature : undefined); + + return result; +}; + /* * Legacy part: Will be removed after all the views have been migrated. */ diff --git a/packages/extension/src/utils/parseFromString.test.ts b/packages/extension/src/utils/parseFromString.test.ts index 0317ea63b..6b4e0f42e 100644 --- a/packages/extension/src/utils/parseFromString.test.ts +++ b/packages/extension/src/utils/parseFromString.test.ts @@ -10,7 +10,6 @@ import { parseCreateOfferFlags, parseLimitAmount, parseMemos, - parseMintNFTFlags, parsePaymentFlags, parseSetAccountFlags, parseSigners, @@ -182,25 +181,6 @@ describe('parseTrustSetFlags', () => { }); }); -describe('parseMintNFTFlags', () => { - test('parse flags', () => { - expect(parseMintNFTFlags('123')).toEqual(123); - }); - - test('parse flags object', () => { - expect( - parseMintNFTFlags( - '{"tfBurnable":false,"tfOnlyXRP":false,"tfTrustLine":true,"tfTransferable":true}' - ) - ).toEqual({ - tfBurnable: false, - tfOnlyXRP: false, - tfTrustLine: true, - tfTransferable: true - }); - }); -}); - describe('parseCreateOfferFlags', () => { test('should return null when flagsString is null', () => { const result = parseCreateOfferFlags(null); diff --git a/packages/extension/src/utils/parseFromString.ts b/packages/extension/src/utils/parseFromString.ts index e026c00bb..d5273122f 100644 --- a/packages/extension/src/utils/parseFromString.ts +++ b/packages/extension/src/utils/parseFromString.ts @@ -11,7 +11,6 @@ import { CreateNFTOfferFlags, CreateOfferFlags, Memo, - MintNFTFlags, PaymentFlags, SetAccountFlags, Signer, @@ -247,38 +246,6 @@ export const parseSetAccountFlags = (flagsString: string | null): SetAccountFlag return null; }; -export const parseMintNFTFlags = (flagsString: string | null): MintNFTFlags | null => { - if (!flagsString) { - return null; - } - - if (Number(flagsString)) { - return Number(flagsString); - } - - try { - const parsedFlags = JSON.parse(flagsString); - - if ( - typeof parsedFlags === 'object' && - parsedFlags !== null && - ('tfBurnable' in parsedFlags || - 'tfOnlyXRP' in parsedFlags || - 'tfTrustLine' in parsedFlags || - 'tfTransferable' in parsedFlags) - ) { - return parsedFlags as { - tfBurnable?: boolean; - tfOnlyXRP?: boolean; - tfTrustLine?: boolean; - tfTransferable?: boolean; - }; - } - } catch (error) {} - - return null; -}; - export const parseCreateNFTOfferFlags = ( flagsString: string | null ): CreateNFTOfferFlags | null => {