diff --git a/libs/coin-modules/coin-xrp/package.json b/libs/coin-modules/coin-xrp/package.json index 96213623506b..c64993a689b4 100644 --- a/libs/coin-modules/coin-xrp/package.json +++ b/libs/coin-modules/coin-xrp/package.json @@ -134,6 +134,7 @@ "lint": "eslint ./src --no-error-on-unmatched-pattern --ext .ts,.tsx --cache", "lint:fix": "pnpm lint --fix", "test": "jest", + "test-watch": "jest --watch", "test-integ": "DOTENV_CONFIG_PATH=.env.integ.test jest --config=jest.integ.config.js", "typecheck": "tsc --noEmit", "unimported": "unimported" diff --git a/libs/coin-modules/coin-xrp/src/api/index.ts b/libs/coin-modules/coin-xrp/src/api/index.ts index 552e58a4336e..b6aa8ade1067 100644 --- a/libs/coin-modules/coin-xrp/src/api/index.ts +++ b/libs/coin-modules/coin-xrp/src/api/index.ts @@ -45,7 +45,12 @@ async function operations( address: string, { limit, start }: Pagination, ): Promise<[Operation[], number]> { - const [ops, index] = await listOperations(address, { limit, startAt: start ?? 0 }); + const options: { + limit?: number; + minHeight?: number; + } = { limit: limit }; + if (start) options.minHeight = start; + const [ops, index] = await listOperations(address, options); return [ ops.map(op => { const { simpleType, blockHash, blockTime, blockHeight, ...rest } = op; diff --git a/libs/coin-modules/coin-xrp/src/bridge/synchronization.ts b/libs/coin-modules/coin-xrp/src/bridge/synchronization.ts index 12388d793500..dd939ca5b956 100644 --- a/libs/coin-modules/coin-xrp/src/bridge/synchronization.ts +++ b/libs/coin-modules/coin-xrp/src/bridge/synchronization.ts @@ -63,7 +63,7 @@ async function filterOperations( address: string, blockHeight: number, ): Promise { - const [operations, _] = await listOperations(address, { startAt: blockHeight }); + const [operations, _] = await listOperations(address, { minHeight: blockHeight }); return operations.map( op => diff --git a/libs/coin-modules/coin-xrp/src/logic/listOperations.test.ts b/libs/coin-modules/coin-xrp/src/logic/listOperations.test.ts index b7696581435c..f83de40ab2d5 100644 --- a/libs/coin-modules/coin-xrp/src/logic/listOperations.test.ts +++ b/libs/coin-modules/coin-xrp/src/logic/listOperations.test.ts @@ -1,9 +1,12 @@ +import { assert } from "console"; import { listOperations } from "./listOperations"; import { RIPPLE_EPOCH } from "./utils"; +const maxHeight = 2; +const minHeight = 1; const mockGetServerInfos = jest.fn().mockResolvedValue({ info: { - complete_ledgers: "1-2", + complete_ledgers: `${minHeight}-${maxHeight}`, }, }); const mockGetTransactions = jest.fn(); @@ -18,6 +21,94 @@ describe("listOperations", () => { mockGetTransactions.mockClear(); }); + it("when there are no transactions then the result is empty", async () => { + // Given + mockGetTransactions.mockResolvedValue([]); + // When + const [results, token] = await listOperations("any address", { minHeight: 0 }); + // Then + expect(mockGetServerInfos).toHaveBeenCalledTimes(1); + expect(mockGetTransactions).toHaveBeenCalledTimes(1); + expect(results).toEqual([]); + expect(token).toEqual(minHeight); + }); + + it("when there are no transactions and a limit then the result is empty", async () => { + // Given + mockGetTransactions.mockResolvedValue([]); + // When + const [results, token] = await listOperations("any address", { minHeight: 0, limit: 1 }); + // Then + expect(mockGetServerInfos).toHaveBeenCalledTimes(1); + expect(mockGetTransactions).toHaveBeenCalledTimes(1); + expect(results).toEqual([]); + expect(token).toEqual(minHeight); + }); + + const paymentTx = { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: "100" }, + tx_json: { + TransactionType: "Payment", + Fee: "1", + ledger_index: 2, + date: 1000, + Account: "sender", + Destination: "dest", + Sequence: 1, + }, + }; + const otherTx = { ...paymentTx, tx_json: { ...paymentTx.tx_json, TransactionType: "Other" } }; + + it("should only list operations of type payment", async () => { + // Given + const lastTransaction = paymentTx; + mockGetTransactions.mockResolvedValueOnce([paymentTx, otherTx, lastTransaction]); + mockGetTransactions.mockResolvedValue([]); // subsequent calls + + // When + const [results, token] = await listOperations("any address", { minHeight: 0, limit: 3 }); + + // Then + expect(mockGetServerInfos).toHaveBeenCalledTimes(1); + // it's called twice because first call yields only 2 transactions, and 3 are asked + expect(mockGetTransactions).toHaveBeenCalledTimes(2); + expect(results.length).toEqual(2); + expect(token).toEqual(lastTransaction.tx_json.ledger_index - 1); + }); + + it("should make enough calls so that the limit requested is satified", async () => { + const lastTransaction = otherTx; + const txs = [paymentTx, paymentTx, otherTx, otherTx, otherTx, otherTx, otherTx, otherTx]; + assert(txs.length === 8); + mockGetTransactions.mockResolvedValue(txs); + + const [results, token] = await listOperations("any address", { minHeight: 0, limit: 8 }); + + expect(mockGetServerInfos).toHaveBeenCalledTimes(1); + // it's called 4 times because each call yields only 2 transactions, and 8 are asked + expect(mockGetTransactions).toHaveBeenCalledTimes(4); + expect(results.length).toEqual(8); + expect(token).toEqual(lastTransaction.tx_json.ledger_index - 1); + }); + + it("should make enough calls, even if there is not enough txs to satisfy the limit", async () => { + mockGetTransactions.mockResolvedValueOnce([otherTx, otherTx, otherTx, otherTx]); + mockGetTransactions.mockResolvedValueOnce([paymentTx, paymentTx]); + mockGetTransactions.mockResolvedValue([]); // subsequent calls + const lastTransaction = paymentTx; + + const [results, token] = await listOperations("any address", { minHeight: 0, limit: 4 }); + + expect(mockGetServerInfos).toHaveBeenCalledTimes(1); + // it's called 2 times because the second call is a shortage of txs + expect(mockGetTransactions).toHaveBeenCalledTimes(2); + expect(results.length).toEqual(2); + expect(token).toEqual(lastTransaction.tx_json.ledger_index - 1); + }); + it.each([ { address: "WHATEVER_ADDRESS", @@ -34,7 +125,7 @@ describe("listOperations", () => { ])( "should return the list of operations associated to an account", async ({ address, opSender, opDestination, expectedType }) => { - // Givem + // Given const deliveredAmount = 100; const fee = 10; mockGetTransactions.mockResolvedValue([ @@ -95,7 +186,7 @@ describe("listOperations", () => { ]); // When - const [results, _] = await listOperations(address, { startAt: 0 }); + const [results, _] = await listOperations(address, { minHeight: 0 }); // Then expect(mockGetServerInfos).toHaveBeenCalledTimes(1); diff --git a/libs/coin-modules/coin-xrp/src/logic/listOperations.ts b/libs/coin-modules/coin-xrp/src/logic/listOperations.ts index b96fbed6b4a3..755f75c3ded0 100644 --- a/libs/coin-modules/coin-xrp/src/logic/listOperations.ts +++ b/libs/coin-modules/coin-xrp/src/logic/listOperations.ts @@ -13,12 +13,12 @@ export async function listOperations( address: string, { limit, - mostRecentIndex, - startAt, + maxHeight, + minHeight, }: { limit?: number; - mostRecentIndex?: number | undefined; - startAt?: number; + maxHeight?: number | undefined; // used for pagination + minHeight?: number; // used to retrieve operations from a specific block height until top most }, ): Promise<[XrpOperation[], number]> { const serverInfo = await getServerInfos(); @@ -26,13 +26,15 @@ export async function listOperations( const minLedgerVersion = Number(ledgers[0]); const maxLedgerVersion = Number(ledgers[1]); - let options: { + type Options = { ledger_index_min?: number; ledger_index_max?: number; limit?: number; tx_type?: string; - } = { - ledger_index_max: mostRecentIndex ?? maxLedgerVersion, + }; + + let options: Options = { + ledger_index_max: maxHeight ?? maxLedgerVersion, tx_type: "Payment", }; @@ -42,38 +44,55 @@ export async function listOperations( limit, }; } - if (startAt) { + if (minHeight) { options = { ...options, // if there is no ops, it might be after a clear and we prefer to pull from the oldest possible history - ledger_index_min: Math.max(startAt, minLedgerVersion), + ledger_index_min: Math.max(minHeight, minLedgerVersion), }; } - // We need to filter out the transactions that are not "Payment" type because the filter on "tx_type" of the node RPC - // is not working as expected. It returns all the transactions. We need to call the node RPC multiple times to get the - // desired number of transactions by the limiter. - let transactions: XrplOperation[] = []; - let needToStop = true; - do { - const newTransactions = await getTransactions(address, options); - const newPaymentsTxs = newTransactions.filter(tx => tx.tx_json.TransactionType === "Payment"); - if (options.limit) { - needToStop = newTransactions.length < options.limit; - options.ledger_index_max = newTransactions.slice(-1)[0].tx_json.ledger_index - 1; + async function getPaymentTransactions( + address: string, + options: Options, + ): Promise<[boolean, Options, XrplOperation[]]> { + const txs = await getTransactions(address, options); + // Filter out the transactions that are not "Payment" type because the filter on "tx_type" of the node RPC is not working as expected. + const paymentTxs = txs.filter(tx => tx.tx_json.TransactionType === "Payment"); + const shortage = (options.limit && txs.length < options.limit) || false; + const lastTransaction = txs.slice(-1)[0]; + const nextOptions = { ...options }; + if (lastTransaction) { + nextOptions.ledger_index_max = lastTransaction.tx_json.ledger_index - 1; + if (nextOptions.limit) nextOptions.limit -= paymentTxs.length; } - transactions = transactions.concat(newPaymentsTxs); - } while ( - options.limit && - !needToStop && - transactions.length < options.limit && - (options.limit -= transactions.length) - ); - - return [ - transactions.map(convertToCoreOperation(address)), - transactions.slice(-1)[0].tx_json.ledger_index - 1, // Returns the next index to start from for pagination - ]; + return [shortage, nextOptions, paymentTxs]; + } + + // TODO BUG: given the number of txs belonging to the SAME block > limit + // when user loop over pages using the provided token + // then user misses some txs that doesn't fit the page size limit + // because the "next token" is a block height (solution is to use an opaque token instead) + let [txShortage, nextOptions, transactions] = await getPaymentTransactions(address, options); + const isEnough = () => txShortage || (limit && transactions.length >= limit); + // We need to call the node RPC multiple times to get the desired number of transactions by the limiter. + while (nextOptions.limit && !isEnough()) { + const [newTxShortage, newNextOptions, newTransactions] = await getPaymentTransactions( + address, + nextOptions, + ); + txShortage = newTxShortage; + nextOptions = newNextOptions; + transactions = transactions.concat(newTransactions); + } + + const lastTransaction = transactions.slice(-1)[0]; + // the next index to start the pagination from + const nextIndex = lastTransaction + ? Math.max(lastTransaction.tx_json.ledger_index - 1, minLedgerVersion) + : minLedgerVersion; + + return [transactions.map(convertToCoreOperation(address)), nextIndex]; } const convertToCoreOperation = diff --git a/libs/coin-modules/coin-xrp/src/network/index.ts b/libs/coin-modules/coin-xrp/src/network/index.ts index d35233abb33d..6d403fc49c67 100644 --- a/libs/coin-modules/coin-xrp/src/network/index.ts +++ b/libs/coin-modules/coin-xrp/src/network/index.ts @@ -72,10 +72,12 @@ export const getTransactions = async ( ): Promise => { const result = await rpcCall("account_tx", { account: address, + // newest first + // note that order within the results is not guaranteed (see documentation of account_tx) + forward: false, ...options, api_version: 2, }); - return result.transactions; }; @@ -91,7 +93,7 @@ export async function getLedgerIndex(): Promise { async function rpcCall( method: string, - params: Record, + params: Record = {}, ): Promise { const { data: { result },