address PR comments from Cybele

Author: ckeshava

packages/ripple-binary-codec/src/types/path-set.ts

@@ -109,7 +109,7 @@ class Hop extends SerializedType {
 
     if (type & TYPE_CURRENCY && type & TYPE_MPT) {
       throw new Error(
-        'Invalid binary input: Currency and mpt_issuance_id are mutually exclusive in a path hop',
+        'Currency and mpt_issuance_id are mutually exclusive in a path hop. The BinaryParser has a bitmask containing both Currency and mpt_issuance_id elements',
       )
     }
 

packages/ripple-binary-codec/test/path-set.test.ts

@@ -216,6 +216,41 @@ describe('Path-Set binary-codec unit tests', () => {
     )
   })
 
+  it(`Path with MPT → Currency → MPT hops`, () => {
+    const mptIssuanceId1 = '00000001B5F762798A53D543A014CAF8B297CFF8F2F937E8'
+    const currencyCode = 'ABC'
+    const mptIssuanceId2 = '000004C463C52827307480341125DA0577DEFC38405B0E3E'
+    const path = [
+      [
+        { mpt_issuance_id: mptIssuanceId1 } as HopObject,
+        { currency: currencyCode } as HopObject,
+        { mpt_issuance_id: mptIssuanceId2 } as HopObject,
+      ],
+    ]
+    const serializedHexRepr = coreTypes.PathSet.from(path).toHex().toUpperCase()
+
+    expect(serializedHexRepr).toEqual(
+      '40' /* MPT type code */ +
+        mptIssuanceId1.toUpperCase() /* first 24-byte MPTID */ +
+        '10' /* Currency type code */ +
+        Currency.from(currencyCode)
+          .toHex()
+          .toUpperCase() /* 20-byte currency */ +
+        '40' /* MPT type code */ +
+        mptIssuanceId2.toUpperCase() /* second 24-byte MPTID */ +
+        '00' /* PathSet terminator */,
+    )
+
+    // test the round-trip equivalence
+    expect(coreTypes.PathSet.from(path).toJSON()).toEqual(path)
+
+    // validate the de-serialization via the BinaryParser
+    const parser = new BinaryParser(serializedHexRepr)
+    expect(coreTypes.PathSet.fromParser(parser)).toEqual(
+      coreTypes.PathSet.from(path),
+    )
+  })
+
   it(`PathSet: Currency and MPT are mutually exclusive in a hop`, () => {
     const currencyCode = 'ABC'
     const mptIssuanceId = '00000001B5F762798A53D543A014CAF8B297CFF8F2F937E8'
@@ -244,7 +279,24 @@ describe('Path-Set binary-codec unit tests', () => {
 
     const parser = new BinaryParser(invalidHex)
     expect(() => coreTypes.PathSet.fromParser(parser)).toThrow(
-      'Currency and mpt_issuance_id are mutually exclusive',
+      'Currency and mpt_issuance_id are mutually exclusive in a path hop. The BinaryParser has a bitmask containing both Currency and mpt_issuance_id elements',
+    )
+  })
+
+  it(`PathSet: Deserialization rejects type byte 0x51 (Account + Currency + MPT)`, () => {
+    // Type byte 0x51 = TYPE_ACCOUNT (0x01) | TYPE_CURRENCY (0x10) | TYPE_MPT (0x40)
+    const invalidHex =
+      '51' +
+      AccountID.from('rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh')
+        .toHex()
+        .toUpperCase() +
+      Currency.from('ABC').toHex().toUpperCase() +
+      '00000001B5F762798A53D543A014CAF8B297CFF8F2F937E8' +
+      '00'
+
+    const parser = new BinaryParser(invalidHex)
+    expect(() => coreTypes.PathSet.fromParser(parser)).toThrow(
+      'Currency and mpt_issuance_id are mutually exclusive in a path hop. The BinaryParser has a bitmask containing both Currency and mpt_issuance_id elements',
     )
   })
 })

packages/xrpl/src/models/transactions/AMMClawback.ts

@@ -67,8 +67,8 @@ export interface AMMClawback extends BaseTransaction {
   Asset: IssuedCurrency | MPTCurrency
 
   /**
-   * Specifies the other asset in the AMM's pool. In JSON, this is an object with currency and
-   * issuer fields (omit issuer for XRP), or mpt_issuance_id for MPT.
+   * Specifies the other asset in the AMM's pool. In JSON, this can be XRP (no issuer),
+   * an issued currency (with issuer), or an MPT (with mpt_issuance_id).
    */
   Asset2: Currency
 

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

@@ -1,6 +1,6 @@
 import { assert } from 'chai'
 
-import { CheckCreate, CheckCash } from '../../../src'
+import { CheckCreate, CheckCash, LedgerEntry } from '../../../src'
 import { createMPTIssuanceAndAuthorize } from '../mptUtils'
 import serverUrl from '../serverUrl'
 import {
@@ -111,7 +111,21 @@ describe('CheckCash', function () {
         1,
         'Should be exactly one check on the ledger',
       )
-      const checkId = response1.result.account_objects[0].index
+
+      const checkObject = response1.result
+        .account_objects[0] as LedgerEntry.Check
+      const checkId = checkObject.index
+
+      // Verify the check ledger object contents
+      assert.equal(checkObject.Account, testContext.wallet.classicAddress)
+      assert.equal(
+        checkObject.Destination,
+        checkDestinationWallet.classicAddress,
+      )
+      assert.deepEqual(checkObject.SendMax, {
+        mpt_issuance_id: mptIssuanceId,
+        value: '50',
+      })
 
       // Cash the check with MPT
       const tx: CheckCash = {

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

@@ -162,4 +162,53 @@ describe('OfferCreate', function () {
     },
     TIMEOUT,
   )
+
+  it(
+    'OfferCreate with MPT on TakerPays side',
+    async () => {
+      const issuerWallet = await generateFundedWallet(testContext.client)
+      const sourceWallet = await generateFundedWallet(testContext.client)
+
+      const mptIssuanceId = await createMPTIssuanceAndAuthorize(
+        testContext.client,
+        issuerWallet,
+        sourceWallet,
+        // eslint-disable-next-line no-bitwise -- combining flags requires bitwise OR
+        MPTokenIssuanceCreateFlags.tfMPTCanTrade |
+          MPTokenIssuanceCreateFlags.tfMPTCanTransfer,
+      )
+
+      // Create an offer: sell 100000 XRP drops for 10 MPT
+      const tx: OfferCreate = {
+        TransactionType: 'OfferCreate',
+        Account: sourceWallet.classicAddress,
+        TakerGets: '100000',
+        TakerPays: {
+          mpt_issuance_id: mptIssuanceId,
+          value: '10',
+        },
+      }
+
+      await testTransaction(testContext.client, tx, sourceWallet)
+
+      // Confirm the offer exists on the ledger
+      const accountOffersResponse = await testContext.client.request({
+        command: 'account_offers',
+        account: sourceWallet.classicAddress,
+      })
+      assert.lengthOf(
+        accountOffersResponse.result.offers!,
+        1,
+        'Should be exactly one offer on the ledger',
+      )
+
+      const offer = accountOffersResponse.result.offers![0]
+      assert.equal(offer.taker_gets, '100000')
+      assert.deepEqual(offer.taker_pays, {
+        mpt_issuance_id: mptIssuanceId,
+        value: '10',
+      })
+    },
+    TIMEOUT,
+  )
 })

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

@@ -6,8 +6,11 @@ import {
   MPTokenIssuanceCreate,
   MPTokenIssuanceCreateFlags,
   MPTokenAuthorize,
+  AccountSet,
+  AccountSetAsfFlags,
   AMMCreate,
   TransactionMetadata,
+  TrustSet,
 } from '../../../src'
 import { createMPTIssuanceAndAuthorize } from '../mptUtils'
 import serverUrl from '../serverUrl'
@@ -275,4 +278,90 @@ describe('Payment', function () {
     },
     TIMEOUT,
   )
+
+  it(
+    'Payment with currency PathStep (no account)',
+    async () => {
+      const issuer = await generateFundedWallet(testContext.client)
+      const lpWallet = await generateFundedWallet(testContext.client)
+      const destination = await generateFundedWallet(testContext.client)
+
+      const currencyCode = 'USD'
+
+      // Enable default rippling on the issuer so payments can ripple through
+      const enableRipplingTx: AccountSet = {
+        TransactionType: 'AccountSet',
+        Account: issuer.classicAddress,
+        SetFlag: AccountSetAsfFlags.asfDefaultRipple,
+      }
+      await testTransaction(testContext.client, enableRipplingTx, issuer)
+
+      // LP sets up a trust line for USD and receives funds from the issuer
+      const trustTx: TrustSet = {
+        TransactionType: 'TrustSet',
+        Account: lpWallet.classicAddress,
+        LimitAmount: {
+          currency: currencyCode,
+          issuer: issuer.classicAddress,
+          value: '100000',
+        },
+      }
+      await testTransaction(testContext.client, trustTx, lpWallet)
+
+      const fundLpTx: Payment = {
+        TransactionType: 'Payment',
+        Account: issuer.classicAddress,
+        Destination: lpWallet.classicAddress,
+        Amount: {
+          currency: currencyCode,
+          issuer: issuer.classicAddress,
+          value: '50000',
+        },
+      }
+      await testTransaction(testContext.client, fundLpTx, issuer)
+
+      // Destination sets up a trust line for USD
+      const destTrustTx: TrustSet = {
+        TransactionType: 'TrustSet',
+        Account: destination.classicAddress,
+        LimitAmount: {
+          currency: currencyCode,
+          issuer: issuer.classicAddress,
+          value: '100000',
+        },
+      }
+      await testTransaction(testContext.client, destTrustTx, destination)
+
+      // Create AMM pool: XRP / USD
+      const ammCreateTx: AMMCreate = {
+        TransactionType: 'AMMCreate',
+        Account: lpWallet.classicAddress,
+        Amount: '1000000',
+        Amount2: {
+          currency: currencyCode,
+          issuer: issuer.classicAddress,
+          value: '1000',
+        },
+        TradingFee: 12,
+      }
+      await testTransaction(testContext.client, ammCreateTx, lpWallet)
+
+      // Cross-currency payment: XRP → USD via AMM with a currency-only path step
+      const payTx: Payment = {
+        TransactionType: 'Payment',
+        Account: testContext.wallet.classicAddress,
+        Destination: destination.classicAddress,
+        Amount: {
+          currency: currencyCode,
+          issuer: issuer.classicAddress,
+          value: '5',
+        },
+        SendMax: '500000',
+        Paths: [[{ currency: currencyCode, issuer: issuer.classicAddress }]],
+      }
+
+      await testTransaction(testContext.client, payTx, testContext.wallet)
+    },
+    TIMEOUT,
+  )
 })