Update getNFTokenID to properly handle binary blob (#2247)

* Update NFTokenMint test

* Ensure the binary version works as well

* Remove meta being undefined in txResponse

* Remove error test

* Re-add test, error, and lint

* Add meta to test, and add string to allowed type

* Re-add meta being undefined with proper docs

packages/xrpl/HISTORY.md

@@ -13,6 +13,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
 * Adds support for npm v9
 
 ### Fixed
+* `getNFTokenID` now also accepts metadata from `tx` in binary format
 * Fixed `ServerState.transitions` typing, it is now a string instead of a number. (Only used in return from `server_state` request)
 * Added `destination_amount` to `PathOption` which is returned as part of a `path_find` request
 * Removed the `decode(encode(tx)) == tx` check from the wallet signing process

packages/xrpl/snippets/src/getTransaction.ts

@@ -19,10 +19,11 @@ async function getTransaction(): Promise<void> {
     })
     console.log(tx)
 
-    // The meta field would be a string(hex) when the `binary` parameter is `true` for the `tx` request.
+    // The meta field can be undefined if the transaction has not been validated yet (and so has not changed the ledger).
     if (tx.result.meta == null) {
       throw new Error('meta not included in the response')
     }
+
     /*
      * delivered_amount is the amount actually received by the destination account.
      * Use this field to determine how much was delivered, regardless of whether the transaction is a partial payment.

packages/xrpl/src/models/methods/tx.ts

@@ -46,7 +46,8 @@ export interface TxResponse<T extends BaseTransaction = Transaction>
     hash: string
     /** The ledger index of the ledger that includes this transaction. */
     ledger_index?: number
-    /** Transaction metadata, which describes the results of the transaction. */
+    /** Transaction metadata, which describes the results of the transaction.
+     *  Can be undefined if a transaction has not been validated yet. */
     meta?: TransactionMetadata | string
     /**
      * If true, this data comes from a validated ledger version; if omitted or.

packages/xrpl/src/utils/getNFTokenID.ts

@@ -1,4 +1,5 @@
 import flatMap from 'lodash/flatMap'
+import { decode } from 'ripple-binary-codec'
 
 import {
   CreatedNode,
@@ -15,24 +16,41 @@ interface NFToken {
   }
 }
 
+/**
+ * Ensures that the metadata is in a deserialized format to parse.
+ *
+ * @param meta - the metadata from a `tx` method call. Can be in json format or binary format.
+ * @returns the metadata in a deserialized format.
+ */
+function ensureDecodedMeta(
+  meta: TransactionMetadata | string,
+): TransactionMetadata {
+  if (typeof meta === 'string') {
+    // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Meta is either metadata or serialized metadata.
+    return decode(meta) as unknown as TransactionMetadata
+  }
+  return meta
+}
+
 /**
  * Gets the NFTokenID for an NFT recently minted with NFTokenMint.
  *
- * @param meta - Metadata from the response to submitting an NFTokenMint transaction.
+ * @param meta - Metadata from the response to submitting and waiting for an NFTokenMint transaction or from a `tx` method call.
  * @returns The NFTokenID for the minted NFT.
  * @throws if meta is not TransactionMetadata.
  */
 export default function getNFTokenID(
-  meta: TransactionMetadata,
+  meta: TransactionMetadata | string | undefined,
 ): string | undefined {
-  /* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- Provides a nicer error for js users */
+  if (typeof meta !== 'string' && meta?.AffectedNodes === undefined) {
-  if (meta.AffectedNodes === undefined) {
     throw new TypeError(`Unable to parse the parameter given to getNFTokenID.
-    'meta' must be the metadata from an NFTokenMint transaction. Received ${JSON.stringify(
+      'meta' must be the metadata from an NFTokenMint transaction. Received ${JSON.stringify(
-      meta,
+        meta,
-    )} instead.`)
+      )} instead.`)
   }
 
+  const decodedMeta = ensureDecodedMeta(meta)
+
   /*
    * When a mint results in splitting an existing page,
    * it results in a created page and a modified node. Sometimes,
@@ -46,7 +64,7 @@ export default function getNFTokenID(
    * if the PreviousFields contains NFTokens
    */
 
-  const affectedNodes = meta.AffectedNodes.filter((node) => {
+  const affectedNodes = decodedMeta.AffectedNodes.filter((node) => {
     if (isCreatedNode(node)) {
       return node.CreatedNode.LedgerEntryType === 'NFTokenPage'
     }

packages/xrpl/test/integration/transactions/nftokenMint.test.ts

@@ -1,69 +1,87 @@
 import { assert } from 'chai'
-import _ from 'lodash'
+import { TransactionMetadata, TxRequest } from 'xrpl'
-import { Client } from 'xrpl'
 
+import { convertStringToHex, getNFTokenID, NFTokenMint } from '../../../src'
+import { hashSignedTx } from '../../../src/utils/hashes'
+import serverUrl from '../serverUrl'
 import {
-  convertStringToHex,
+  setupClient,
-  getNFTokenID,
+  teardownClient,
-  NFTokenMint,
+  type XrplIntegrationTestContext,
-  TransactionMetadata,
+} from '../setup'
-} from '../../../src'
+import { testTransaction } from '../utils'
 
 // how long before each test case times out
 const TIMEOUT = 20000
 
 describe('NFTokenMint', function () {
-  // TODO: Once we update our integration tests to handle NFTs, replace this client with XrplIntegrationTestContext
+  let testContext: XrplIntegrationTestContext
+
+  beforeEach(async () => {
+    testContext = await setupClient(serverUrl)
+  })
+  afterEach(async () => teardownClient(testContext))
+
   it(
     'get NFTokenID',
     async function () {
-      const client = new Client('wss://s.altnet.rippletest.net:51233/')
-      await client.connect()
-
-      const { wallet, balance: _balance } = await client.fundWallet(null, {
-        usageContext: 'integration-test',
-      })
-
       const tx: NFTokenMint = {
         TransactionType: 'NFTokenMint',
-        Account: wallet.address,
+        Account: testContext.wallet.address,
         URI: convertStringToHex('https://www.google.com'),
         NFTokenTaxon: 0,
       }
-      try {
+      const response = await testTransaction(
-        const response = await client.submitAndWait(tx, {
+        testContext.client,
-          wallet,
+        tx,
-        })
+        testContext.wallet,
-        assert.equal(response.type, 'response')
+      )
-        assert.equal(
+      assert.equal(response.type, 'response')
-          (response.result.meta as TransactionMetadata).TransactionResult,
-          'tesSUCCESS',
-        )
 
-        const accountNFTs = await client.request({
+      const txRequest: TxRequest = {
-          command: 'account_nfts',
+        command: 'tx',
-          account: wallet.address,
+        transaction: hashSignedTx(response.result.tx_blob),
-        })
-
-        const nftokenID =
-          getNFTokenID(response.result.meta as TransactionMetadata) ??
-          'undefined'
-        const accountHasNFT = accountNFTs.result.account_nfts.some(
-          (value) => value.NFTokenID === nftokenID,
-        )
-
-        assert.isTrue(
-          accountHasNFT,
-          `Expected to find an NFT with NFTokenID ${nftokenID} in account ${
-            wallet.address
-          } but did not find it.
-        \n\nHere's what was returned from 'account_nfts' for ${
-          wallet.address
-        }: ${JSON.stringify(accountNFTs)}`,
-        )
-      } finally {
-        await client.disconnect()
       }
+      const txResponse = await testContext.client.request(txRequest)
+
+      assert.equal(
+        (txResponse.result.meta as TransactionMetadata).TransactionResult,
+        'tesSUCCESS',
+      )
+
+      const accountNFTs = await testContext.client.request({
+        command: 'account_nfts',
+        account: testContext.wallet.address,
+      })
+
+      const nftokenID =
+        getNFTokenID(txResponse.result.meta as TransactionMetadata) ??
+        'undefined'
+
+      const accountHasNFT = accountNFTs.result.account_nfts.some(
+        (value) => value.NFTokenID === nftokenID,
+      )
+
+      assert.isTrue(
+        accountHasNFT,
+        `Expected to find an NFT with NFTokenID ${nftokenID} in account ${
+          testContext.wallet.address
+        } but did not find it.
+      \n\nHere's what was returned from 'account_nfts' for ${
+        testContext.wallet.address
+      }: ${JSON.stringify(accountNFTs)}`,
+      )
+
+      const binaryTxResponse = await testContext.client.request({
+        ...txRequest,
+        binary: true,
+      })
+
+      assert.equal(
+        nftokenID,
+        getNFTokenID(binaryTxResponse.result.meta) ?? 'undefined',
+        `getNFTokenID produced a different outcome when decoding the metadata in binary format.`,
+      )
     },
     TIMEOUT,
   )