Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[account] Fix pagination on AccountModules and AccountResources #581

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
All notable changes to the Aptos TypeScript SDK will be captured in this file. This changelog is written by hand for now. It adheres to the format set out by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

# Unreleased
- [`Fix`] Fixes pagination for GetAccountModules and GetAccountResources. Also, adds more appropriate documentation on offset.

# 1.33.0 (2024-11-13)
- Allow optional provision of public keys in transaction simulation
Expand Down
4 changes: 2 additions & 2 deletions src/api/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class Account {
* This function may call the API multiple times to auto paginate through results.
*
* @param args.accountAddress - The Aptos account address to query modules for.
* @param args.options.offset - The number of modules to start returning results from.
* @param args.options.offset - The cursor to start returning results from. Note, this is obfuscated and is not an index.
* @param args.options.limit - The maximum number of results to return.
* @param args.options.ledgerVersion - The ledger version to query; if not provided, it retrieves the latest version.
*
Expand Down Expand Up @@ -223,7 +223,7 @@ export class Account {
* This function may call the API multiple times to auto paginate through results.
*
* @param args.accountAddress - The Aptos account address to query resources for.
* @param args.options.offset - The number of resources to start returning results from.
* @param args.options.offset - The cursor to start returning results from. Note, this is obfuscated and is not an index.
* @param args.options.limit - The maximum number of results to return.
* @param args.options.ledgerVersion - The ledger version to query; if not provided, it will get the latest version.
* @returns Account resources.
Expand Down
42 changes: 42 additions & 0 deletions src/client/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,45 @@ export async function paginateWithCursor<Req extends Record<string, any>, Res ex
} while (cursor !== null && cursor !== undefined);
return out as Res;
}

/// This function is a helper for paginating using a function wrapping an API using offset instead of start
export async function paginateWithObfuscatedCursor<Req extends Record<string, any>, Res extends Array<{}>>(
Copy link
Collaborator

@0xmaayan 0xmaayan Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiuc, the only 2 differences is the start vs offset param and the re-evaluate length calculation - any reason not to simply use the existing paginateWithCursor but just provide a flag for the re-evaluate length calculation part? requestParams are already being passed as an argument to the paginateWithCursor function.

Copy link
Collaborator Author

@gregnazario gregnazario Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to be more explicit because the start vs offset is actually more of the problem that the one with the obfuscated cursor is not useful / a user shouldn't provide it directly. You would have to get it from a previous function call.

Also, I think the cursor header isn't always set, so that whole logic probably needs to be checked.

Basically there are 2 code paths on the serverside for pagination, and I'm trying to separate them in the event we have other errors with it.

I kinda wish the API just used the same system for both.

options: GetAptosRequestOptions,
): Promise<Res> {
const out: any[] = [];
let cursor: string | undefined;
const requestParams = options.params as { offset?: string; limit?: number };
const totalLimit = requestParams.limit;
do {
// eslint-disable-next-line no-await-in-loop
const response = await get<Req, Res>({
type: AptosApiType.FULLNODE,
aptosConfig: options.aptosConfig,
originMethod: options.originMethod,
path: options.path,
params: requestParams,
overrides: options.overrides,
});
/**
* the cursor is a "state key" from the API perspective. Client
* should not need to "care" what it represents but just use it
* to query the next chunk of data.
*/
cursor = response.headers["x-aptos-cursor"];
// Now that we have the cursor (if any), we remove the headers before
// adding these to the output of this function.
delete response.headers;
out.push(...response.data);
requestParams.offset = cursor;

// Re-evaluate length
if (totalLimit !== undefined) {
const newLimit = totalLimit - out.length;
if (newLimit <= 0) {
break;
}
requestParams.limit = newLimit;
}
} while (cursor !== null && cursor !== undefined);
return out as Res;
}
14 changes: 7 additions & 7 deletions src/internal/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* account namespace and without having a dependency cycle error.
*/
import { AptosConfig } from "../api/aptosConfig";
import { getAptosFullNode, paginateWithCursor } from "../client";
import { getAptosFullNode, paginateWithCursor, paginateWithObfuscatedCursor } from "../client";
import {
AccountData,
GetAccountCoinsDataResponse,
Expand Down Expand Up @@ -85,7 +85,7 @@ export async function getInfo(args: {
* @param args.accountAddress - The address of the account whose modules are to be retrieved.
* @param args.options - Optional parameters for pagination and ledger version.
* @param args.options.limit - The maximum number of modules to retrieve (default is 1000).
* @param args.options.offset - The starting point for pagination.
* @param args.options.offset - The starting point for pagination. Note, this is obfuscated and is not an index.
* @param args.options.ledgerVersion - The specific ledger version to query.
*/
export async function getModules(args: {
Expand All @@ -94,13 +94,13 @@ export async function getModules(args: {
options?: PaginationArgs & LedgerVersionArg;
}): Promise<MoveModuleBytecode[]> {
const { aptosConfig, accountAddress, options } = args;
return paginateWithCursor<{}, MoveModuleBytecode[]>({
return paginateWithObfuscatedCursor<{}, MoveModuleBytecode[]>({
aptosConfig,
originMethod: "getModules",
path: `accounts/${AccountAddress.from(accountAddress).toString()}/modules`,
params: {
ledger_version: options?.ledgerVersion,
start: options?.offset,
offset: options?.offset,
limit: options?.limit ?? 1000,
},
});
Expand Down Expand Up @@ -196,7 +196,7 @@ export async function getTransactions(args: {
* @param args.aptosConfig - The configuration settings for Aptos.
* @param args.accountAddress - The address of the account to fetch resources for.
* @param args.options - Optional pagination and ledger version parameters.
* @param args.options.offset - The starting point for pagination.
* @param args.options.offset - The starting point for pagination. Note, this is obfuscated and is not an index.
* @param args.options.limit - The maximum number of resources to retrieve (default is 999).
* @param args.options.ledgerVersion - The specific ledger version to query.
*/
Expand All @@ -206,13 +206,13 @@ export async function getResources(args: {
options?: PaginationArgs & LedgerVersionArg;
}): Promise<MoveResource[]> {
const { aptosConfig, accountAddress, options } = args;
return paginateWithCursor<{}, MoveResource[]>({
return paginateWithObfuscatedCursor<{}, MoveResource[]>({
aptosConfig,
originMethod: "getResources",
path: `accounts/${AccountAddress.from(accountAddress).toString()}/resources`,
params: {
ledger_version: options?.ledgerVersion,
start: options?.offset,
offset: options?.offset,
limit: options?.limit ?? 999,
},
});
Expand Down
27 changes: 26 additions & 1 deletion tests/e2e/api/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,24 @@ describe("account api", () => {
});

test("it fetches account modules", async () => {
const { aptos } = getAptosClient();
const data = await aptos.getAccountModules({
accountAddress: "0x1",
});
expect(data.length).toBeGreaterThan(0);
});

test("it fetches account modules with pagination", async () => {
const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
const data = await aptos.getAccountModules({
accountAddress: "0x1",
options: {
offset: 1,
limit: 1,
},
});
expect(data.length).toBeGreaterThan(0);
expect(data.length).toEqual(1);
});

test("it fetches an account module", async () => {
Expand All @@ -57,6 +69,19 @@ describe("account api", () => {
expect(data.length).toBeGreaterThan(0);
});

test("it fetches account resources with pagination", async () => {
const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
const data = await aptos.getAccountResources({
accountAddress: "0x1",
options: {
offset: 1,
limit: 1,
},
});
expect(data.length).toEqual(1);
});

test("it fetches an account resource without a type", async () => {
const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
Expand Down
Loading