From 5969aaabafeea81eed12d78d0c99d1fc8cab36f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Prudent?= Date: Fri, 31 Jan 2025 10:13:20 +0100 Subject: [PATCH] feat(coin:xrp): apply api change on operations (#8987) * fix(coin:xrp): api iterates over all pages --- libs/coin-framework/src/api/types.ts | 18 +- .../common-logic/history/listOperations.ts | 9 +- .../src/network/indexer.ts | 2 +- .../coin-polkadot/src/api/index.integ.test.ts | 8 +- .../coin-polkadot/src/api/index.ts | 10 +- .../coin-stellar/src/api/index.integ.test.ts | 8 +- .../coin-stellar/src/api/index.ts | 9 +- .../coin-stellar/src/logic/listOperations.ts | 8 +- .../coin-tezos/src/api/index.integ.test.ts | 8 +- libs/coin-modules/coin-tezos/src/api/index.ts | 6 +- .../coin-tezos/src/logic/listOperations.ts | 14 +- libs/coin-modules/coin-xrp/package.json | 1 + .../coin-xrp/src/api/index.integ.test.ts | 18 +- .../coin-xrp/src/api/index.test.ts | 194 +++++++++++------- libs/coin-modules/coin-xrp/src/api/index.ts | 72 ++++++- .../src/bridge/synchronization.test.ts | 46 +++-- .../coin-xrp/src/logic/listOperations.test.ts | 189 +++++++++-------- .../coin-xrp/src/logic/listOperations.ts | 81 +++++--- .../coin-xrp/src/network/index.ts | 29 ++- .../coin-xrp/src/network/types.ts | 6 + pnpm-lock.yaml | 3 + 21 files changed, 460 insertions(+), 279 deletions(-) diff --git a/libs/coin-framework/src/api/types.ts b/libs/coin-framework/src/api/types.ts index 90d677ab6bab..b9b7b896c0c1 100644 --- a/libs/coin-framework/src/api/types.ts +++ b/libs/coin-framework/src/api/types.ts @@ -27,7 +27,13 @@ export type Transaction = { supplement?: unknown; }; -export type Pagination = { limit: number; start?: number }; +// TODO rename start to minHeight +// and add a `token: string` field to the pagination if we really need to support pagination +// (which is not the case for now) +// for now start is used as a minHeight from which we want to fetch ALL operations +// limit is unused for now +// see design document at https://ledgerhq.atlassian.net/wiki/spaces/BE/pages/5446205788/coin-modules+lama-adapter+APIs+refinements +export type Pagination = { minHeight: number }; export type Api = { broadcast: (tx: string) => Promise; combine: (tx: string, signature: string, pubkey?: string) => string; @@ -35,13 +41,5 @@ export type Api = { estimateFees: (addr: string, amount: bigint) => Promise; getBalance: (address: string) => Promise; lastBlock: () => Promise; - /** - * - * @param address - * @param pagination The max number of operation to receive and the "id" or "index" to start from (see returns value). - * @returns Operations found and the next "id" or "index" to use for pagination (i.e. `start` property).\ - * If `0` is returns, no pagination needed. - * This "id" or "index" value, thus it has functional meaning, is different for each blockchain. - */ - listOperations: (address: string, pagination: Pagination) => Promise<[Operation[], number]>; + listOperations: (address: string, pagination: Pagination) => Promise<[Operation[], string]>; }; diff --git a/libs/coin-modules/coin-module-boilerplate/src/common-logic/history/listOperations.ts b/libs/coin-modules/coin-module-boilerplate/src/common-logic/history/listOperations.ts index 5cfd7b3ef983..fcf8f7f022c3 100644 --- a/libs/coin-modules/coin-module-boilerplate/src/common-logic/history/listOperations.ts +++ b/libs/coin-modules/coin-module-boilerplate/src/common-logic/history/listOperations.ts @@ -11,11 +11,10 @@ import { getTransactions } from "../../network/indexer"; */ export async function listOperations( address: string, - { limit, start }: Pagination, -): Promise<[Operation[], number]> { - const transactions = await getTransactions(address, { from: start || 0, size: limit }); - - return [transactions.map(convertToCoreOperation(address)), transactions.length]; + page: Pagination, +): Promise<[Operation[], string]> { + const transactions = await getTransactions(address, { from: page.minHeight }); + return [transactions.map(convertToCoreOperation(address)), ""]; } const convertToCoreOperation = (address: string) => (operation: any) => { diff --git a/libs/coin-modules/coin-module-boilerplate/src/network/indexer.ts b/libs/coin-modules/coin-module-boilerplate/src/network/indexer.ts index 07413e7ccfee..f14f80acf60c 100644 --- a/libs/coin-modules/coin-module-boilerplate/src/network/indexer.ts +++ b/libs/coin-modules/coin-module-boilerplate/src/network/indexer.ts @@ -4,7 +4,7 @@ import { getEnv } from "@ledgerhq/live-env"; export const getTransactions = async ( address: string, - params: { from: number; size: number }, + params: { from: number; size?: number }, ): Promise => { const { data } = await network({ // NOTE: add INDEXER_BOILERPLATE to libs/env/src/env.ts diff --git a/libs/coin-modules/coin-polkadot/src/api/index.integ.test.ts b/libs/coin-modules/coin-polkadot/src/api/index.integ.test.ts index f94acc7f8900..6959d8b7fe0c 100644 --- a/libs/coin-modules/coin-polkadot/src/api/index.integ.test.ts +++ b/libs/coin-modules/coin-polkadot/src/api/index.integ.test.ts @@ -42,7 +42,7 @@ describe("Polkadot Api", () => { describe("listOperations", () => { it("returns a list regarding address parameter", async () => { // When - const [tx, _] = await module.listOperations(address, { limit: 100 }); + const [tx, _] = await module.listOperations(address, { minHeight: 0 }); // Then expect(tx.length).toBeGreaterThanOrEqual(1); @@ -54,11 +54,9 @@ describe("Polkadot Api", () => { }); }, 20000); - it("returns paginated operations", async () => { + it("returns all operations", async () => { // When - const [tx, idx] = await module.listOperations(address, { limit: 100 }); - const [tx2, _] = await module.listOperations(address, { limit: 100, start: idx }); - tx.push(...tx2); + const [tx, _] = await module.listOperations(address, { minHeight: 0 }); // Then const checkSet = new Set(tx.map(elt => elt.hash)); diff --git a/libs/coin-modules/coin-polkadot/src/api/index.ts b/libs/coin-modules/coin-polkadot/src/api/index.ts index 3a38e8d4afae..bdd2f4e54e5d 100644 --- a/libs/coin-modules/coin-polkadot/src/api/index.ts +++ b/libs/coin-modules/coin-polkadot/src/api/index.ts @@ -1,6 +1,7 @@ import type { Api, Transaction as ApiTransaction, + Operation, Pagination, } from "@ledgerhq/coin-framework/api/index"; import coinConfig, { type PolkadotConfig } from "../config"; @@ -47,5 +48,10 @@ async function estimate(addr: string, amount: bigint): Promise { return estimateFees(tx); } -const operations = async (addr: string, { limit, start }: Pagination) => - listOperations(addr, { limit, startAt: start }); +async function operations( + address: string, + { minHeight }: Pagination, +): Promise<[Operation[], string]> { + const [ops, nextHeight] = await listOperations(address, { limit: 0, startAt: minHeight }); + return [ops, JSON.stringify(nextHeight)]; +} diff --git a/libs/coin-modules/coin-stellar/src/api/index.integ.test.ts b/libs/coin-modules/coin-stellar/src/api/index.integ.test.ts index 50328419d120..5d1c160b4489 100644 --- a/libs/coin-modules/coin-stellar/src/api/index.integ.test.ts +++ b/libs/coin-modules/coin-stellar/src/api/index.integ.test.ts @@ -32,7 +32,7 @@ describe("Stellar Api", () => { describe.only("listOperations", () => { it("returns a list regarding address parameter", async () => { // When - const [tx, _] = await module.listOperations(address, { limit: 100 }); + const [tx, _] = await module.listOperations(address, { minHeight: 0 }); // Then expect(tx.length).toBeGreaterThanOrEqual(100); @@ -44,11 +44,9 @@ describe("Stellar Api", () => { }); }); - it("returns paginated operations", async () => { + it("returns all operations", async () => { // When - const [tx, idx] = await module.listOperations(address, { limit: 200 }); - const [tx2, _] = await module.listOperations(address, { limit: 200, start: idx }); - tx.push(...tx2); + const [tx, _] = await module.listOperations(address, { minHeight: 0 }); // Then const checkSet = new Set(tx.map(elt => elt.hash)); diff --git a/libs/coin-modules/coin-stellar/src/api/index.ts b/libs/coin-modules/coin-stellar/src/api/index.ts index 2a9ca6f0f21e..b3f101f87a52 100644 --- a/libs/coin-modules/coin-stellar/src/api/index.ts +++ b/libs/coin-modules/coin-stellar/src/api/index.ts @@ -67,7 +67,10 @@ function compose(tx: string, signature: string, pubkey?: string): string { return combine(tx, signature, pubkey); } -const operations = async ( +async function operations( address: string, - { limit, start }: Pagination, -): Promise<[Operation[], number]> => listOperations(address, { limit, cursor: start }); + _pagination: Pagination, +): Promise<[Operation[], string]> { + // TODO will be fixed with https://github.com/LedgerHQ/ledger-live/pull/8898/files + return listOperations(address, { limit: 200 }); +} diff --git a/libs/coin-modules/coin-stellar/src/logic/listOperations.ts b/libs/coin-modules/coin-stellar/src/logic/listOperations.ts index d54a2812aba7..43b0c9cf2707 100644 --- a/libs/coin-modules/coin-stellar/src/logic/listOperations.ts +++ b/libs/coin-modules/coin-stellar/src/logic/listOperations.ts @@ -20,8 +20,8 @@ export type Operation = { export async function listOperations( address: string, - { limit, cursor }: { limit: number; cursor?: number | undefined }, -): Promise<[Operation[], number]> { + { limit, cursor }: { limit?: number; cursor?: string }, +): Promise<[Operation[], string]> { // Fake accountId const accountId = ""; const operations = await fetchOperations({ @@ -29,12 +29,12 @@ export async function listOperations( addr: address, order: "asc", limit, - cursor: cursor?.toString(), + cursor: cursor, }); return [ operations.map(convertToCoreOperation(address)), - parseInt(operations.slice(-1)[0].extra.pagingToken ?? "0"), + operations.slice(-1)[0].extra.pagingToken ?? "", ]; } diff --git a/libs/coin-modules/coin-tezos/src/api/index.integ.test.ts b/libs/coin-modules/coin-tezos/src/api/index.integ.test.ts index 14416b4cfb97..c30cafca75b8 100644 --- a/libs/coin-modules/coin-tezos/src/api/index.integ.test.ts +++ b/libs/coin-modules/coin-tezos/src/api/index.integ.test.ts @@ -46,7 +46,7 @@ describe("Tezos Api", () => { describe.only("listOperations", () => { it("returns a list regarding address parameter", async () => { // When - const [tx, _] = await module.listOperations(address, { limit: 100 }); + const [tx, _] = await module.listOperations(address, { minHeight: 0 }); // Then expect(tx.length).toBeGreaterThanOrEqual(1); @@ -59,11 +59,9 @@ describe("Tezos Api", () => { }); }); - it("returns paginated operations", async () => { + it("returns all operations", async () => { // When - const [tx, idx] = await module.listOperations(address, { limit: 100 }); - const [tx2, _] = await module.listOperations(address, { limit: 100, start: idx }); - tx.push(...tx2); + const [tx, _] = await module.listOperations(address, { minHeight: 0 }); // Then // Find a way to create a unique id. In Tezos, the same hash may represent different operations in case of delegation. diff --git a/libs/coin-modules/coin-tezos/src/api/index.ts b/libs/coin-modules/coin-tezos/src/api/index.ts index dcd8234655f8..62a0d67d6f3d 100644 --- a/libs/coin-modules/coin-tezos/src/api/index.ts +++ b/libs/coin-modules/coin-tezos/src/api/index.ts @@ -65,5 +65,7 @@ async function estimate(addr: string, amount: bigint): Promise { return estimatedFees.estimatedFees; } -const operations = (address: string, { limit, start }: Pagination) => - listOperations(address, { limit, lastId: start }); +function operations(address: string, _pagination: Pagination) { + //TODO implement properly with https://github.com/LedgerHQ/ledger-live/pull/8875 + return listOperations(address, {}); +} diff --git a/libs/coin-modules/coin-tezos/src/logic/listOperations.ts b/libs/coin-modules/coin-tezos/src/logic/listOperations.ts index 833303b21662..9220d872c31d 100644 --- a/libs/coin-modules/coin-tezos/src/logic/listOperations.ts +++ b/libs/coin-modules/coin-tezos/src/logic/listOperations.ts @@ -25,14 +25,20 @@ export type Operation = { export async function listOperations( address: string, - { lastId, limit }: { lastId?: number; limit?: number }, -): Promise<[Operation[], number]> { - const operations = await tzkt.getAccountOperations(address, { lastId, limit }); + { token, limit }: { limit?: number; token?: string }, +): Promise<[Operation[], string]> { + let options: { lastId?: number; limit?: number } = { limit: limit }; + if (token) { + options = { ...options, lastId: JSON.parse(token) }; + } + const operations = await tzkt.getAccountOperations(address, options); + const lastOperation = operations.slice(-1)[0]; + const nextId = lastOperation ? JSON.stringify(lastOperation?.id) : ""; return [ operations .filter(op => isAPITransactionType(op) || isAPIDelegationType(op)) .reduce((acc, op) => acc.concat(convertOperation(address, op)), [] as Operation[]), - operations.slice(-1)[0].id, + nextId, ]; } diff --git a/libs/coin-modules/coin-xrp/package.json b/libs/coin-modules/coin-xrp/package.json index e4c697e91e88..13aa9e4365cd 100644 --- a/libs/coin-modules/coin-xrp/package.json +++ b/libs/coin-modules/coin-xrp/package.json @@ -105,6 +105,7 @@ "@ledgerhq/errors": "workspace:^", "@ledgerhq/live-network": "workspace:^", "@ledgerhq/types-live": "workspace:^", + "@ledgerhq/logs": "workspace:^", "bignumber.js": "^9.1.2", "invariant": "^2.2.4", "ripple-address-codec": "^5.0.0", diff --git a/libs/coin-modules/coin-xrp/src/api/index.integ.test.ts b/libs/coin-modules/coin-xrp/src/api/index.integ.test.ts index b81629bf0ace..b617530bcc32 100644 --- a/libs/coin-modules/coin-xrp/src/api/index.integ.test.ts +++ b/libs/coin-modules/coin-xrp/src/api/index.integ.test.ts @@ -6,6 +6,7 @@ import { sign } from "ripple-keypairs"; describe("Xrp Api", () => { let module: Api; const address = "rh1HPuRVsYYvThxG2Bs1MfjmrVC73S16Fb"; + const bigAddress = "rUxSkt6hQpWxXQwTNRUCYYRQ7BC2yRA3F8"; // An account with more that 4000 txs const emptyAddress = "rKtXXTVno77jhu6tto1MAXjepyuaKaLcqB"; // Account with no transaction (at the time of this writing) const xrpPubKey = process.env["PUB_KEY"]!; const xrpSecretKey = process.env["SECRET_KEY"]!; @@ -30,7 +31,7 @@ describe("Xrp Api", () => { describe("listOperations", () => { it("returns a list regarding address parameter", async () => { // When - const [tx, _] = await module.listOperations(address, { limit: 200 }); + const [tx, _] = await module.listOperations(address, { minHeight: 200 }); // Then expect(tx.length).toBe(200); @@ -42,15 +43,20 @@ describe("Xrp Api", () => { }); }); - it("returns paginated operations", async () => { + it("returns all operations", async () => { // When - const [tx, idx] = await module.listOperations(address, { limit: 200 }); - const [tx2, _] = await module.listOperations(address, { limit: 200, start: idx }); - tx.push(...tx2); - + const [tx, _] = await module.listOperations(bigAddress, { minHeight: 0 }); // Then const checkSet = new Set(tx.map(elt => elt.hash)); expect(checkSet.size).toEqual(tx.length); + // the first transaction is returned + expect(tx[0].block.height).toEqual(73126713); + expect(tx[0].hash.toUpperCase).toEqual( + "0FC3792449E5B1E431D45E3606017D10EC1FECC8EDF988A98E36B8FE0C33ACAE", + ); + // 200 is the default XRP explorer hard limit, + // so here we are checking that this limit is bypassed + expect(tx.length).toBeGreaterThan(200); }); }); diff --git a/libs/coin-modules/coin-xrp/src/api/index.test.ts b/libs/coin-modules/coin-xrp/src/api/index.test.ts index 6c5c2cf9f611..d8046f7f3435 100644 --- a/libs/coin-modules/coin-xrp/src/api/index.test.ts +++ b/libs/coin-modules/coin-xrp/src/api/index.test.ts @@ -20,6 +20,98 @@ describe("listOperations", () => { mockGetTransactions.mockClear(); }); + const defaultMarker = { ledger: 1, seq: 1 }; + function mockNetworkTxs(txs: unknown, marker: undefined | unknown): unknown { + return { + account: "account", + ledger_index_max: 1, + ledger_index_min: 1, + limit: 1, + validated: false, + transactions: txs, + marker: marker, + error: "", + }; + } + + function givenTxs( + fee: bigint, + deliveredAmount: bigint, + opSender: string, + opDestination: string, + ): unknown[] { + return [ + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: deliveredAmount.toString() }, + tx_json: { + TransactionType: "Payment", + Fee: fee.toString(), + ledger_index: 1, + date: 1000, + Account: opSender, + Destination: opDestination, + Sequence: 1, + }, + }, + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: deliveredAmount.toString() }, + tx_json: { + TransactionType: "Payment", + Fee: fee.toString(), + ledger_index: 1, + date: 1000, + Account: opSender, + Destination: opDestination, + DestinationTag: 509555, + Sequence: 1, + }, + }, + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: deliveredAmount.toString() }, + tx_json: { + TransactionType: "Payment", + Fee: fee.toString(), + ledger_index: 1, + date: 1000, + Account: opSender, + Destination: opDestination, + Memos: [ + { + Memo: { + MemoType: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", + MemoData: "72656e74", + }, + }, + ], + Sequence: 1, + }, + }, + ]; + } + + it("should kill the loop after 10 iterations", async () => { + const txs = givenTxs(BigInt(10), BigInt(10), "src", "dest"); + // each time it's called it returns a marker, so in theory it would loop forever + mockGetTransactions.mockResolvedValue(mockNetworkTxs(txs, defaultMarker)); + const [results, _] = await api.listOperations("src", { minHeight: 0 }); + + // called 10 times because there is a hard limit of 10 iterations in case something goes wrong + // with interpretation of the token (bug / explorer api changed ...) + expect(mockGetServerInfos).toHaveBeenCalledTimes(10); + expect(mockGetTransactions).toHaveBeenCalledTimes(10); + + expect(results.length).toBe(txs.length * 10); + }); + it.each([ { address: "WHATEVER_ADDRESS", @@ -34,84 +126,36 @@ describe("listOperations", () => { expectedType: "OUT", }, ])( - "should return the list of operations associated to an account", + `should return the list of operations associated to an account when expected type is %s`, async ({ address, opSender, opDestination, expectedType }) => { // Givem - const deliveredAmount = 100; - const fee = 10; - mockGetTransactions.mockResolvedValue([ - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - close_time_iso: "2000-01-01T00:00:01Z", - meta: { delivered_amount: deliveredAmount.toString() }, - tx_json: { - TransactionType: "Payment", - Fee: fee.toString(), - ledger_index: 1, - date: 1000, - Account: opSender, - Destination: opDestination, - Sequence: 1, - }, - }, - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - close_time_iso: "2000-01-01T00:00:01Z", - meta: { delivered_amount: deliveredAmount.toString() }, - tx_json: { - TransactionType: "Payment", - Fee: fee.toString(), - ledger_index: 1, - date: 1000, - Account: opSender, - Destination: opDestination, - DestinationTag: 509555, - Sequence: 1, - }, - }, - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - close_time_iso: "2000-01-01T00:00:01Z", - meta: { delivered_amount: deliveredAmount.toString() }, - tx_json: { - TransactionType: "Payment", - Fee: fee.toString(), - ledger_index: 1, - date: 1000, - Account: opSender, - Destination: opDestination, - Memos: [ - { - Memo: { - MemoType: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", - MemoData: "72656e74", - }, - }, - ], - Sequence: 1, - }, - }, - ]); + const deliveredAmount = BigInt(100); + const fee = BigInt(10); + mockGetTransactions.mockResolvedValueOnce( + mockNetworkTxs(givenTxs(fee, deliveredAmount, opSender, opDestination), defaultMarker), + ); + + // second call to kill the loop + mockGetTransactions.mockResolvedValue(mockNetworkTxs([], undefined)); // When - const [results, _] = await api.listOperations(address, { limit: 100 }); + const [results, _] = await api.listOperations(address, { minHeight: 0 }); // Then - expect(mockGetServerInfos).toHaveBeenCalledTimes(1); - expect(mockGetTransactions).toHaveBeenCalledTimes(1); + // called twice because the marker is set the first time + expect(mockGetServerInfos).toHaveBeenCalledTimes(2); + expect(mockGetTransactions).toHaveBeenCalledTimes(2); + // if expectedType is "OUT", compute value with fees (i.e. delivered_amount + Fee) - const expectedValue = - expectedType === "IN" ? BigInt(deliveredAmount) : BigInt(deliveredAmount + fee); + const expectedValue = expectedType === "IN" ? deliveredAmount : deliveredAmount + fee; + // the order is reversed so that the result is always sorted by newest tx first element of the list expect(results).toEqual([ { hash: "HASH_VALUE", address, type: "Payment", value: expectedValue, - fee: BigInt(10), + fee: fee, block: { hash: "HASH_VALUE_BLOCK", height: 1, @@ -121,13 +165,21 @@ describe("listOperations", () => { recipients: [opDestination], date: new Date(1000000 + RIPPLE_EPOCH * 1000), transactionSequenceNumber: 1, + details: { + memos: [ + { + type: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", + data: "72656e74", + }, + ], + }, }, { hash: "HASH_VALUE", address, type: "Payment", value: expectedValue, - fee: BigInt(10), + fee: fee, block: { hash: "HASH_VALUE_BLOCK", height: 1, @@ -146,7 +198,7 @@ describe("listOperations", () => { address, type: "Payment", value: expectedValue, - fee: BigInt(10), + fee: fee, block: { hash: "HASH_VALUE_BLOCK", height: 1, @@ -156,14 +208,6 @@ describe("listOperations", () => { recipients: [opDestination], date: new Date(1000000 + RIPPLE_EPOCH * 1000), transactionSequenceNumber: 1, - details: { - memos: [ - { - type: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", - data: "72656e74", - }, - ], - }, }, ]); }, diff --git a/libs/coin-modules/coin-xrp/src/api/index.ts b/libs/coin-modules/coin-xrp/src/api/index.ts index b6aa8ade1067..321c6ae4bcb8 100644 --- a/libs/coin-modules/coin-xrp/src/api/index.ts +++ b/libs/coin-modules/coin-xrp/src/api/index.ts @@ -4,6 +4,7 @@ import type { Transaction as ApiTransaction, Pagination, } from "@ledgerhq/coin-framework/api/index"; +import { log } from "@ledgerhq/logs"; import coinConfig, { type XrpConfig } from "../config"; import { broadcast, @@ -15,6 +16,7 @@ import { lastBlock, listOperations, } from "../logic"; +import { XrpOperation } from "../types"; export function createApi(config: XrpConfig): Api { coinConfig.setCoinConfig(() => ({ ...config, status: { type: "active" } })); @@ -41,16 +43,66 @@ async function estimate(_addr: string, _amount: bigint): Promise { return fees.fee; } +type PaginationState = { + pageSize: number; // must be large enough to avoid unnecessary calls to the underlying explorer + maxIterations: number; // a security to avoid infinite loop + currentIteration: number; + minHeight: number; + continueIterations: boolean; + apiNextCursor?: string; + accumulator: XrpOperation[]; +}; + +async function operationsFromHeight( + address: string, + minHeight: number, +): Promise<[XrpOperation[], string]> { + async function fetchNextPage(state: PaginationState): Promise { + const [operations, apiNextCursor] = await listOperations(address, { + limit: state.pageSize, + minHeight: state.minHeight, + order: "asc", + }); + const newCurrentIteration = state.currentIteration + 1; + let continueIteration = true; + if (apiNextCursor === "") { + continueIteration = false; + } else if (newCurrentIteration >= state.maxIterations) { + log("coin:xrp", "(api/operations): max iterations reached", state.maxIterations); + continueIteration = false; + } + const accumulated = state.accumulator.concat(operations); + return { + ...state, + currentIteration: newCurrentIteration, + continueIterations: continueIteration, + apiNextCursor: apiNextCursor, + accumulator: accumulated, + }; + } + + const firstState: PaginationState = { + pageSize: 200, + maxIterations: 10, + currentIteration: 0, + minHeight: minHeight, + continueIterations: true, + accumulator: [], + }; + + let state = await fetchNextPage(firstState); + while (state.continueIterations) { + state = await fetchNextPage(state); + } + return [state.accumulator, state.apiNextCursor ?? ""]; +} + async function operations( address: string, - { limit, start }: Pagination, -): Promise<[Operation[], number]> { - const options: { - limit?: number; - minHeight?: number; - } = { limit: limit }; - if (start) options.minHeight = start; - const [ops, index] = await listOperations(address, options); + { minHeight }: Pagination, +): Promise<[Operation[], string]> { + const [ops, token] = await operationsFromHeight(address, minHeight); + // TODO token must be implemented properly (waiting ack from the design document) return [ ops.map(op => { const { simpleType, blockHash, blockTime, blockHeight, ...rest } = op; @@ -61,8 +113,8 @@ async function operations( hash: blockHash, time: blockTime, }, - } satisfies Operation; + }; }), - index, + token, ]; } diff --git a/libs/coin-modules/coin-xrp/src/bridge/synchronization.test.ts b/libs/coin-modules/coin-xrp/src/bridge/synchronization.test.ts index 1b06b81d385d..e0efa65cb80a 100644 --- a/libs/coin-modules/coin-xrp/src/bridge/synchronization.test.ts +++ b/libs/coin-modules/coin-xrp/src/bridge/synchronization.test.ts @@ -68,6 +68,20 @@ describe("getAccountShape", () => { }); }); + const someMarker = { ledger: 1, seq: 1 }; + function mockNetworkTxs(txs: unknown): unknown { + return { + account: "account", + ledger_index_max: 1, + ledger_index_min: 1, + limit: 1, + validated: false, + transactions: txs, + marker: someMarker, + error: "", + }; + } + it("convert correctly operations", async () => { // Given mockGetAccountInfo.mockResolvedValue({ @@ -76,22 +90,24 @@ describe("getAccountShape", () => { ownerCount: 0, sequence: 0, }); - mockGetTransactions.mockResolvedValue([ - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - meta: { delivered_amount: "100" }, - tx_json: { - TransactionType: "Payment", - Fee: "10", - ledger_index: 1, - date: 1000, - Account: "account_addr", - Destination: "destination_addr", - Sequence: 1, + mockGetTransactions.mockResolvedValue( + mockNetworkTxs([ + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + meta: { delivered_amount: "100" }, + tx_json: { + TransactionType: "Payment", + Fee: "10", + ledger_index: 1, + date: 1000, + Account: "account_addr", + Destination: "destination_addr", + Sequence: 1, + }, }, - }, - ]); + ]), + ); // When const account = await getAccountShape( 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 f83de40ab2d5..a4d9d74cd6eb 100644 --- a/libs/coin-modules/coin-xrp/src/logic/listOperations.test.ts +++ b/libs/coin-modules/coin-xrp/src/logic/listOperations.test.ts @@ -1,6 +1,7 @@ import { assert } from "console"; import { listOperations } from "./listOperations"; import { RIPPLE_EPOCH } from "./utils"; +import { Marker } from "../network/types"; const maxHeight = 2; const minHeight = 1; @@ -9,45 +10,60 @@ const mockGetServerInfos = jest.fn().mockResolvedValue({ complete_ledgers: `${minHeight}-${maxHeight}`, }, }); -const mockGetTransactions = jest.fn(); +const mockNetworkGetTransactions = jest.fn(); jest.mock("../network", () => ({ getServerInfos: () => mockGetServerInfos(), - getTransactions: () => mockGetTransactions(), + getTransactions: () => mockNetworkGetTransactions(), })); describe("listOperations", () => { afterEach(() => { mockGetServerInfos.mockClear(); - mockGetTransactions.mockClear(); + mockNetworkGetTransactions.mockClear(); }); + const someMarker: Marker = { ledger: 1, seq: 1 }; + function mockNetworkTxs(txs: unknown): unknown { + return { + account: "account", + ledger_index_max: 1, + ledger_index_min: 1, + limit: 1, + validated: false, + transactions: txs, + marker: someMarker, + error: "", + }; + } + it("when there are no transactions then the result is empty", async () => { // Given - mockGetTransactions.mockResolvedValue([]); + mockNetworkGetTransactions.mockResolvedValue(mockNetworkTxs([])); // When const [results, token] = await listOperations("any address", { minHeight: 0 }); // Then expect(mockGetServerInfos).toHaveBeenCalledTimes(1); - expect(mockGetTransactions).toHaveBeenCalledTimes(1); + expect(mockNetworkGetTransactions).toHaveBeenCalledTimes(1); expect(results).toEqual([]); - expect(token).toEqual(minHeight); + expect(JSON.parse(token)).toEqual(someMarker); }); it("when there are no transactions and a limit then the result is empty", async () => { // Given - mockGetTransactions.mockResolvedValue([]); + mockNetworkGetTransactions.mockResolvedValue(mockNetworkTxs([])); // When const [results, token] = await listOperations("any address", { minHeight: 0, limit: 1 }); // Then expect(mockGetServerInfos).toHaveBeenCalledTimes(1); - expect(mockGetTransactions).toHaveBeenCalledTimes(1); + expect(mockNetworkGetTransactions).toHaveBeenCalledTimes(1); expect(results).toEqual([]); - expect(token).toEqual(minHeight); + expect(JSON.parse(token)).toEqual(someMarker); }); const paymentTx = { ledger_hash: "HASH_VALUE_BLOCK", hash: "HASH_VALUE", + validated: true, close_time_iso: "2000-01-01T00:00:01Z", meta: { delivered_amount: "100" }, tx_json: { @@ -65,8 +81,9 @@ describe("listOperations", () => { it("should only list operations of type payment", async () => { // Given const lastTransaction = paymentTx; - mockGetTransactions.mockResolvedValueOnce([paymentTx, otherTx, lastTransaction]); - mockGetTransactions.mockResolvedValue([]); // subsequent calls + const txs = [paymentTx, otherTx, lastTransaction]; + mockNetworkGetTransactions.mockResolvedValueOnce(mockNetworkTxs(txs)); + mockNetworkGetTransactions.mockResolvedValue(mockNetworkTxs([])); // subsequent calls // When const [results, token] = await listOperations("any address", { minHeight: 0, limit: 3 }); @@ -74,39 +91,39 @@ describe("listOperations", () => { // 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(mockNetworkGetTransactions).toHaveBeenCalledTimes(2); expect(results.length).toEqual(2); - expect(token).toEqual(lastTransaction.tx_json.ledger_index - 1); + expect(JSON.parse(token)).toEqual(someMarker); }); 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); + mockNetworkGetTransactions.mockResolvedValue(mockNetworkTxs(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(mockNetworkGetTransactions).toHaveBeenCalledTimes(4); expect(results.length).toEqual(8); - expect(token).toEqual(lastTransaction.tx_json.ledger_index - 1); + expect(JSON.parse(token)).toEqual(someMarker); }); 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; + mockNetworkGetTransactions.mockResolvedValueOnce( + mockNetworkTxs([otherTx, otherTx, otherTx, otherTx]), + ); + mockNetworkGetTransactions.mockResolvedValueOnce(mockNetworkTxs([paymentTx, paymentTx])); + mockNetworkGetTransactions.mockResolvedValue([]); // subsequent calls 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(mockNetworkGetTransactions).toHaveBeenCalledTimes(2); expect(results.length).toEqual(2); - expect(token).toEqual(lastTransaction.tx_json.ledger_index - 1); + expect(JSON.parse(token)).toEqual(someMarker); }); it.each([ @@ -128,69 +145,71 @@ describe("listOperations", () => { // Given const deliveredAmount = 100; const fee = 10; - mockGetTransactions.mockResolvedValue([ - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - close_time_iso: "2000-01-01T00:00:01Z", - meta: { delivered_amount: deliveredAmount.toString() }, - tx_json: { - TransactionType: "Payment", - Fee: fee.toString(), - ledger_index: 1, - date: 1000, - Account: opSender, - Destination: opDestination, - Sequence: 1, + mockNetworkGetTransactions.mockResolvedValue( + mockNetworkTxs([ + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: deliveredAmount.toString() }, + tx_json: { + TransactionType: "Payment", + Fee: fee.toString(), + ledger_index: 1, + date: 1000, + Account: opSender, + Destination: opDestination, + Sequence: 1, + }, }, - }, - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - close_time_iso: "2000-01-01T00:00:01Z", - meta: { delivered_amount: deliveredAmount.toString() }, - tx_json: { - TransactionType: "Payment", - Fee: fee.toString(), - ledger_index: 1, - date: 1000, - Account: opSender, - Destination: opDestination, - DestinationTag: 509555, - Sequence: 1, + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: deliveredAmount.toString() }, + tx_json: { + TransactionType: "Payment", + Fee: fee.toString(), + ledger_index: 1, + date: 1000, + Account: opSender, + Destination: opDestination, + DestinationTag: 509555, + Sequence: 1, + }, }, - }, - { - ledger_hash: "HASH_VALUE_BLOCK", - hash: "HASH_VALUE", - close_time_iso: "2000-01-01T00:00:01Z", - meta: { delivered_amount: deliveredAmount.toString() }, - tx_json: { - TransactionType: "Payment", - Fee: fee.toString(), - ledger_index: 1, - date: 1000, - Account: opSender, - Destination: opDestination, - Memos: [ - { - Memo: { - MemoType: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", - MemoData: "72656e74", + { + ledger_hash: "HASH_VALUE_BLOCK", + hash: "HASH_VALUE", + close_time_iso: "2000-01-01T00:00:01Z", + meta: { delivered_amount: deliveredAmount.toString() }, + tx_json: { + TransactionType: "Payment", + Fee: fee.toString(), + ledger_index: 1, + date: 1000, + Account: opSender, + Destination: opDestination, + Memos: [ + { + Memo: { + MemoType: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", + MemoData: "72656e74", + }, }, - }, - ], - Sequence: 1, + ], + Sequence: 1, + }, }, - }, - ]); + ]), + ); // When - const [results, _] = await listOperations(address, { minHeight: 0 }); + const [results, _] = await listOperations(address, { minHeight: 0, order: "asc" }); // Then expect(mockGetServerInfos).toHaveBeenCalledTimes(1); - expect(mockGetTransactions).toHaveBeenCalledTimes(1); + expect(mockNetworkGetTransactions).toHaveBeenCalledTimes(1); // if expectedType is "OUT", compute value with fees (i.e. delivered_amount + Fee) const expectedValue = expectedType === "IN" ? BigInt(deliveredAmount) : BigInt(deliveredAmount + fee); @@ -209,6 +228,14 @@ describe("listOperations", () => { recipients: [opDestination], date: new Date(1000000 + RIPPLE_EPOCH * 1000), transactionSequenceNumber: 1, + details: { + memos: [ + { + type: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", + data: "72656e74", + }, + ], + }, }, { hash: "HASH_VALUE", @@ -242,14 +269,6 @@ describe("listOperations", () => { recipients: [opDestination], date: new Date(1000000 + RIPPLE_EPOCH * 1000), transactionSequenceNumber: 1, - details: { - memos: [ - { - type: "687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963", - data: "72656e74", - }, - ], - }, }, ]); }, diff --git a/libs/coin-modules/coin-xrp/src/logic/listOperations.ts b/libs/coin-modules/coin-xrp/src/logic/listOperations.ts index 755f75c3ded0..f3312e80c1fb 100644 --- a/libs/coin-modules/coin-xrp/src/logic/listOperations.ts +++ b/libs/coin-modules/coin-xrp/src/logic/listOperations.ts @@ -1,41 +1,50 @@ -import { getServerInfos, getTransactions } from "../network"; +import { getServerInfos, getTransactions, GetTransactionsOptions } from "../network"; import type { XrplOperation } from "../network/types"; import { XrpMemo, XrpOperation } from "../types"; import { RIPPLE_EPOCH } from "./utils"; +type Order = "asc" | "desc"; /** * Returns list of "Payment" Operations associated to an account. * @param address Account address - * @param blockHeight Height to start searching for operations - * @returns + * @param minHeight retrieve operations from a specific block height until top most (inclusive) + * if not provided, it will start from the oldest possible history. + * The result is not guaranteed to contain all operations until top height (it depends of the underlying explorer), + * so you might need to call this function multiple times to get all operations. + * @param order whether to return operations from the top block or from the oldest block + * it defaults to "desc" (newest first) + * it doesn't control the order of the operations in the result list. + * this parameter is added as a workaround for the issue LIVE-16705 + * @returns a list of operations is descending order and a token to be used for pagination */ export async function listOperations( address: string, { limit, - maxHeight, minHeight, + token, + order, }: { + // pagination: limit?: number; - maxHeight?: number | undefined; // used for pagination - minHeight?: number; // used to retrieve operations from a specific block height until top most + token?: string; + order?: Order; + // filters: + minHeight?: number; }, -): Promise<[XrpOperation[], number]> { +): Promise<[XrpOperation[], string]> { const serverInfo = await getServerInfos(); const ledgers = serverInfo.info.complete_ledgers.split("-"); const minLedgerVersion = Number(ledgers[0]); - const maxLedgerVersion = Number(ledgers[1]); - type Options = { - ledger_index_min?: number; - ledger_index_max?: number; - limit?: number; - tx_type?: string; - }; + // by default the explorer queries the transactions in descending order (newest first) + let forward = false; + if (order && order === "asc") { + forward = true; + } - let options: Options = { - ledger_index_max: maxHeight ?? maxLedgerVersion, - tx_type: "Payment", + let options: GetTransactionsOptions = { + forward: forward, }; if (limit) { @@ -44,7 +53,15 @@ export async function listOperations( limit, }; } - if (minHeight) { + + if (token) { + options = { + ...options, + marker: JSON.parse(token), + }; + } + + if (minHeight !== undefined) { options = { ...options, // if there is no ops, it might be after a clear and we prefer to pull from the oldest possible history @@ -54,25 +71,22 @@ export async function listOperations( async function getPaymentTransactions( address: string, - options: Options, - ): Promise<[boolean, Options, XrplOperation[]]> { - const txs = await getTransactions(address, options); + options: GetTransactionsOptions, + ): Promise<[boolean, GetTransactionsOptions, XrplOperation[]]> { + const response = await getTransactions(address, options); + const txs = response.transactions; + const marker = response.marker; // 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 (marker) { + nextOptions.marker = marker; if (nextOptions.limit) nextOptions.limit -= paymentTxs.length; } 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. @@ -86,13 +100,12 @@ export async function listOperations( 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; + // the order is reversed so that the results are always sorted by newest tx first element of the list + if (order === "asc") transactions.reverse(); - return [transactions.map(convertToCoreOperation(address)), nextIndex]; + // the next index to start the pagination from + const next = nextOptions.marker ? JSON.stringify(nextOptions.marker) : ""; + return [transactions.map(convertToCoreOperation(address)), next]; } 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 6d403fc49c67..824b522e0961 100644 --- a/libs/coin-modules/coin-xrp/src/network/index.ts +++ b/libs/coin-modules/coin-xrp/src/network/index.ts @@ -4,13 +4,13 @@ import type { AccountInfo } from "../types/model"; import { isErrorResponse, isResponseStatus, + Marker, type AccountInfoResponse, type AccountTxResponse, type ErrorResponse, type LedgerResponse, type ServerInfoResponse, type SubmitReponse, - type XrplOperation, } from "./types"; const getNodeUrl = () => coinConfig.getCoinConfig().node; @@ -66,19 +66,32 @@ export const getServerInfos = async (): Promise => { return rpcCall("server_info", { ledger_index: "validated" }); }; +// https://xrpl.org/docs/references/http-websocket-apis/public-api-methods/account-methods/account_tx +export type GetTransactionsOptions = { + ledger_index_min?: number; + ledger_index_max?: number; + limit?: number; + marker?: Marker; + // this property controls the order of the transactions + // true: oldest first + // false: newest first + forward: boolean; +}; + export const getTransactions = async ( address: string, - options: { ledger_index_min?: number; ledger_index_max?: number; limit?: number } | undefined, -): Promise => { + options: GetTransactionsOptions | undefined, +): 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, + // this property controls the order of the transactions + // looks like there is a bug in LL (https://ledgerhq.atlassian.net/browse/LIVE-16705) + // so we need to set it to false (newest first) to get the transactions in the right order + // for lama-adapter we need to set it to true (oldest first) ...options, api_version: 2, }); - return result.transactions; + return result; }; export async function getLedger(): Promise { @@ -93,7 +106,7 @@ export async function getLedgerIndex(): Promise { async function rpcCall( method: string, - params: Record = {}, + params: Record = {}, ): Promise { const { data: { result }, diff --git a/libs/coin-modules/coin-xrp/src/network/types.ts b/libs/coin-modules/coin-xrp/src/network/types.ts index 047e5c3897cc..de4669d92208 100644 --- a/libs/coin-modules/coin-xrp/src/network/types.ts +++ b/libs/coin-modules/coin-xrp/src/network/types.ts @@ -168,6 +168,11 @@ export type ServerInfoResponse = { }; } & ResponseStatus; +export type Marker = { + ledger: number; + seq: number; +}; + export type AccountTxResponse = { account: string; ledger_index_max: number; @@ -175,6 +180,7 @@ export type AccountTxResponse = { limit: number; transactions: XrplOperation[]; validated: boolean; + marker?: Marker; } & ResponseStatus; export type LedgerResponse = { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2299c896cc13..9569cb1046dd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3545,6 +3545,9 @@ importers: '@ledgerhq/live-network': specifier: workspace:^ version: link:../../live-network + '@ledgerhq/logs': + specifier: workspace:^ + version: link:../../ledgerjs/packages/logs '@ledgerhq/types-live': specifier: workspace:^ version: link:../../ledgerjs/packages/types-live