fix: get client.requestAll to handle filters better (#2649)

* fix requestAll issue

* update changelog

* fix typo

packages/xrpl/HISTORY.md

@@ -4,6 +4,9 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
 
 ## Unreleased
 
+### Fixed
+* Fix `client.requestAll` to handle filters better
+
 ## 3.0.0 (2024-02-01)
 
 ### BREAKING CHANGES
@@ -52,80 +55,6 @@ Bundler configurations are much more simplified. See [../UNIQUE_STEPS](Unique St
 * Deprecated:
   * `convertHexToString` in favor of `@xrplf/isomorphic/utils`'s `hexToString`
   * `convertStringToHex` in favor of `@xrplf/isomorphic/utils`'s `stringToHex`
-  
-## 3.0.0 Beta 1 (2023-11-30)
-
-### Breaking Changes
-* `Transaction` type has been redefined to include all transactions and `SubmittableTransaction` was created to define the old value. The following functions which only handle transactions to be submitted now use `SubmittableTransaction`:
-  * `Client.autofill`
-  * `Client.submit`
-  * `Client.submitAndWait`
-  * `Client.prepareTransaction`
-  * `getSignedTx`
-  * `isAccountDelete`
-* `dropsToXRP` and `Client.getXrpBalance` now return a `number` instead of a `string`
-* `Buffer` has been replaced with `UInt8Array` for both params and return values.  `Buffer` may continue to work with params since they extend `UInt8Arrays`.
-
-### Bundling Changes
-* `Buffer` and `process` polyfills are no longer required.
-
-### Changes
-* Deprecated:
-  * `convertHexToString` in favor of `@xrplf/isomorphic/utils`'s `hexToString`
-  * `convertStringToHex` in favor of `@xrplf/isomorphic/utils`'s `stringToHex`
-* Remove `lodash` as a dependency
-* Remove many polyfills that were only used for testing in the browser
-* Remove `util` from bundle by switching `inspect` to `JSON.stringify`
-* Add type for metadata for specific transactions(`Payment`, `NFTokenMint`, `NFTokenCreateOffer`, `NFTokenAcceptOffer`, `NFTokenCancelOffer`)
-
-### Fixed
-* Fixed Wallet.generate() ignoring the `algorithm` parameter (Only a problem once binary-codec fix for `derive_keypair` is added)
-* Fixed Wallet.fromSeed() ignoring the `algorithm` parameter
-* Added pseudo-transaction support to hash functions and response types
-
-## 3.0.0 Beta 0 (2023-10-19)
-
-### Breaking Changes
-* Bump typescript to 5.x
-* Remove Node 14 support
-* Remove `crypto` polyfills, `create-hash`, `elliptic`, `hash.js`, and their many dependencies in favor of `@noble/hashes` and `@nobel/curves`
-* Remove `bip32` and `bip39` in favor of `@scure/bip32` and `@scure/bip39`
-* Remove `assert` dependency. If you were catching `AssertionError` you need to change to `Error`
-* Configuring a proxy:
-    * Instead of passing various parameters on the `ConnectionsOptions` you know specify the `agent` parameter. This object can use be created by libraries such as `https-proxy-agent` or any that implements the `http.Agent`.
-    * This was changed to both support the latest `https-proxy-agent` and to remove the need to include the package in bundlers.   Tests will still be done using `https-proxy-agent` and only tested in a node environment which was the only way it was previously supported anyway
-* Remove `BroadcastClient` which was deprecated
-* Uses `@xrplf/secret-numbers` instead of `xrpl-secret-numbers`
-* Improve key algorithm detection. It will now throw Errors with helpful messages
-* Move `authorizeChannel` from `wallet/signer` to `wallet/authorizeChannel` to solve a circular dependency issue.
-* When using a bundler you must remove the mapping of `ws` to `WSWrapper`. ex. `ws: 'xrpl/dist/npm/client/WSWrapper'`. See [../UNIQUE_STEPS](Unique Steps) for the new, much smaller, configs.
-
-### Bundling Changes
-Bundler configurations are much more simplified. See [../UNIQUE_STEPS](Unique Steps) for the new, much smaller, configs.
-* removed the following polyfills:
-    * `assert`
-    * `crypto-browserify`
-    * `https-browserify`
-    * `os-browserify`
-    * `stream-browserify`
-    * `stream-http`
-    * `url`
-    * `util` - previously added automatically by `webpack`
-    * `events` - previously added automatically by `webpack` but manual for `vite`
-* Removed mappings for:
-    * `ws` to `WsWrapper`
-    * Excluding `https-proxy-agent`
-
-### Changed
-* Remove `lodash` as a dependency
-* Remove many polyfills that were only used for testing in the browser
-* Remove `util` from bundle by switching `inspect` to `JSON.stringify`
-* Add type for metadata for specific transactions(`Payment`, `NFTokenMint`, `NFTokenCreateOffer`, `NFTokenAcceptOffer`, `NFTokenCancelOffer`)
-
-### Fixed
-* Fixed Wallet.generate() ignoring the `algorithm` parameter (Only a problem once binary-codec fix for `derive_keypair` is added)
-* Fixed Wallet.fromSeed() ignoring the `algorithm` parameter
-* Added pseudo-transaction support to hash functions and response types
 
 ## 2.14.1 (2024-02-01)
 

packages/xrpl/src/client/index.ts

@@ -441,7 +441,6 @@ class Client extends EventEmitter<EventTypes> {
     const countTo: number = request.limit == null ? Infinity : request.limit
     let count = 0
     let marker: unknown = request.marker
-    let lastBatchLength: number
     const results: U[] = []
     do {
       const countRemaining = clamp(countTo - count, MIN_LIMIT, MAX_LIMIT)
@@ -465,11 +464,8 @@ class Client extends EventEmitter<EventTypes> {
       // Make sure we handle when no data (not even an empty array) is returned.
       if (Array.isArray(collectedData)) {
         count += collectedData.length
-        lastBatchLength = collectedData.length
-      } else {
-        lastBatchLength = 0
       }
-    } while (Boolean(marker) && count < countTo && lastBatchLength !== 0)
+    } while (Boolean(marker) && count < countTo)
     return results
   }
 

packages/xrpl/test/client/requestAll.test.ts

@@ -10,9 +10,18 @@ import {
 
 const rippledResponse = function (request: Request): Record<string, unknown> {
   if ('marker' in request) {
-    return rippled.ledger_data.last_page
+    return rippled.ledger_data.lastPage
   }
-  return rippled.ledger_data.first_page
+  return rippled.ledger_data.firstPage
+}
+
+const rippledResponseFirstEmpty = function (
+  request: Request,
+): Record<string, unknown> {
+  if ('marker' in request) {
+    return rippled.ledger_data.lastPage
+  }
+  return rippled.ledger_data.firstPageEmpty
 }
 
 describe('client.requestAll', function () {
@@ -34,14 +43,25 @@ describe('client.requestAll', function () {
     )
   })
 
-  it('rejects when there are no more pages', async function () {
+  it('stops when there are no more pages', async function () {
     testContext.mockRippled!.addResponse(
       'ledger_data',
-      rippled.ledger_data.last_page,
+      rippled.ledger_data.lastPage,
     )
     const allResponses = await testContext.client.requestAll({
       command: 'ledger_data',
     })
     assert.equal(allResponses.length, 1)
   })
+
+  it('handles when the first page has no results', async function () {
+    testContext.mockRippled!.addResponse(
+      'ledger_data',
+      rippledResponseFirstEmpty,
+    )
+    const allResponses = await testContext.client.requestAll({
+      command: 'ledger_data',
+    })
+    assert.equal(allResponses.length, 2)
+  })
 })

packages/xrpl/test/client/requestNextPage.test.ts

@@ -11,9 +11,9 @@ import { assertRejects } from '../testUtils'
 
 const rippledResponse = function (request: Request): Record<string, unknown> {
   if ('marker' in request) {
-    return rippled.ledger_data.last_page
+    return rippled.ledger_data.lastPage
   }
-  return rippled.ledger_data.first_page
+  return rippled.ledger_data.firstPage
 }
 
 describe('client.requestNextPage', function () {

packages/xrpl/test/fixtures/rippled/index.ts

@@ -6,8 +6,9 @@ import fabric from './bookOffers'
 import usd_xrp from './bookOffersUsdXrp.json'
 import xrp_usd from './bookOffersXrpUsd.json'
 import normalLedger from './ledger.json'
-import first_page from './ledgerDataFirstPage.json'
-import last_page from './ledgerDataLastPage.json'
+import firstPage from './ledgerDataFirstPage.json'
+import firstPageEmpty from './ledgerDataFirstPageEmpty.json'
+import lastPage from './ledgerDataLastPage.json'
 import iouPartialPayment from './partialPaymentIOU.json'
 import xrpPartialPayment from './partialPaymentXRP.json'
 import normalServerInfo from './serverInfo.json'
@@ -82,8 +83,9 @@ const book_offers = {
 }
 
 const ledger_data = {
-  first_page,
-  last_page,
+  firstPage,
+  firstPageEmpty,
+  lastPage,
 }
 
 const server_info = {

packages/xrpl/test/fixtures/rippled/ledgerDataFirstPageEmpty.json

@@ -0,0 +1,14 @@
+{
+  "id": 0,
+  "status": "success",
+  "type": "response",
+  "result": {
+    "ledger_hash":
+      "102A6E70FFB18C18E97BB56E3047B0E45EA1BCC90BFCCB8CBB0D07BF0E2AB449",
+    "ledger_index": 38202000,
+    "marker":
+      "000B714B790C3C79FEE00D17C4DEB436B375466F29679447BA64F265FD63D730",
+    "state": [],
+    "validated": true
+  }
+}

packages/xrpl/test/utils/hasNextPage.test.ts

@@ -5,12 +5,12 @@ import fixtures from '../fixtures/rippled'
 
 describe('hasNextPage', function () {
   it('returns true when response has marker', function () {
-    const firstPage = fixtures.ledger_data.first_page
+    const firstPage = fixtures.ledger_data.firstPage
     assert.isTrue(hasNextPage(firstPage))
   })
 
   it('returns false when response does not have marker', function () {
-    const lastPage = fixtures.ledger_data.last_page
+    const lastPage = fixtures.ledger_data.lastPage
     assert.isFalse(hasNextPage(lastPage))
   })
 })