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

[TS SDK] Improve IndexerClient to support tokenv2 sorting and new queries #9395

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

0xmaayan
Copy link
Contributor

@0xmaayan 0xmaayan commented Jul 31, 2023

Description

Per @kent-white's request in aptos-labs/explorer#578

  • Export indexer types export * from "./indexer/generated/types";
  • Support for token v2 activities
  • Aggregate query for token v2 activities
  • Support for sorting indexer queries
  • Support for get owned tokens by token_data_id

Test Plan

ts sdk indexer tests are passing

@0xmaayan 0xmaayan requested a review from kent-white July 31, 2023 20:51
@0xmaayan 0xmaayan changed the title improve indexer client to support tokenv2 sorting and new queries [TS SDK] Improve IndexerClient to support tokenv2 sorting and new queries Jul 31, 2023
Copy link
Contributor

@kent-white kent-white left a comment

Choose a reason for hiding this comment

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

noice

@@ -146,31 +168,52 @@ export class IndexerClient {
}

/**
* Queries a token activities by token id hash
* Queries a token activities by token address (token data id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe to be a little clearer: * Queries a token activities by token address or token data id

Copy link
Contributor Author

@0xmaayan 0xmaayan Aug 1, 2023

Choose a reason for hiding this comment

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

yea... so token data id is token address, indexer uses token_data_id key but token address might be more understandable for users..? open for suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

it's either the token_data_id hash or the token address, the former for v1 and latter for v2, right? that was my understanding

Copy link
Contributor Author

Copy link
Contributor

@xbtmatt xbtmatt Aug 1, 2023

Choose a reason for hiding this comment

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

yeah, but it's just a naming convention to make v1 and v2 queries coalesce, the value for v2 is just the token address, whereas with token v1 the value is the hash of the token name + creator + collection name (or maybe something else, I could be wrong, maybe it's a table handle or something), but I've only seen the indexer using it

Sorry if you already knew this, don't mean to explain it to you if you did, but just saying because some people might not understand that the field for the query works for both v1 and v2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean! yes I was just thinking about sdk vs indexer naming - I think what you are saying makes sense, will update the code! thanks!

Copy link
Contributor

@xbtmatt xbtmatt Aug 1, 2023

Choose a reason for hiding this comment

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

Yeah some of the names are very confusing for the indexer. At first glance you'd think all the _v2 queries are specifically for token v2, but they're not, they're just more robust versions of the "v1" queries haha (although maybe the v1 queries don't handle v2, I don't know anymore 🤣)

That's the main reason I suggest it, because then it's a little clearer to people what the purpose of the "_v2" queries are

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I think the idea was that all _v2 queries can handle v1 and v2 data, and all v1 queries can only handle v1 data

}

/**
* Queries account's current owned tokens by the token address (token data id).
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, (and maybe anywhere else..?) token address or token data id

@0xmaayan 0xmaayan force-pushed the indexer_improv branch 3 times, most recently from bc40449 to 9e0b2e6 Compare August 1, 2023 21:52
@0xmaayan 0xmaayan enabled auto-merge (squash) August 1, 2023 22:16
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

✅ Forge suite compat success on aptos-node-v1.5.1 ==> 6be384298ed93f66e896f3d38df0d2b8893f0328

Compatibility test results for aptos-node-v1.5.1 ==> 6be384298ed93f66e896f3d38df0d2b8893f0328 (PR)
1. Check liveness of validators at old version: aptos-node-v1.5.1
compatibility::simple-validator-upgrade::liveness-check : committed: 4101 txn/s, latency: 6992 ms, (p50: 6600 ms, p90: 10400 ms, p99: 19300 ms), latency samples: 176360
2. Upgrading first Validator to new version: 6be384298ed93f66e896f3d38df0d2b8893f0328
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1743 txn/s, latency: 16620 ms, (p50: 19200 ms, p90: 22200 ms, p99: 22500 ms), latency samples: 90640
3. Upgrading rest of first batch to new version: 6be384298ed93f66e896f3d38df0d2b8893f0328
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1884 txn/s, latency: 15529 ms, (p50: 19200 ms, p90: 21800 ms, p99: 22300 ms), latency samples: 92320
4. upgrading second batch to new version: 6be384298ed93f66e896f3d38df0d2b8893f0328
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2936 txn/s, latency: 9896 ms, (p50: 10200 ms, p90: 13800 ms, p99: 15000 ms), latency samples: 126280
5. check swarm health
Compatibility test for aptos-node-v1.5.1 ==> 6be384298ed93f66e896f3d38df0d2b8893f0328 passed
Test Ok

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

✅ Forge suite realistic_env_max_load success on 6be384298ed93f66e896f3d38df0d2b8893f0328

two traffics test: inner traffic : committed: 6258 txn/s, latency: 6258 ms, (p50: 6000 ms, p90: 7800 ms, p99: 11100 ms), latency samples: 2710020
two traffics test : committed: 100 txn/s, latency: 2872 ms, (p50: 2700 ms, p90: 3600 ms, p99: 6900 ms), latency samples: 1720
Max round gap was 1 [limit 4] at version 1273813. Max no progress secs was 3.808821 [limit 10] at version 1273813.
Test Ok

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

✅ Forge suite framework_upgrade success on aptos-node-v1.5.1 ==> 6be384298ed93f66e896f3d38df0d2b8893f0328

Compatibility test results for aptos-node-v1.5.1 ==> 6be384298ed93f66e896f3d38df0d2b8893f0328 (PR)
Upgrade the nodes to version: 6be384298ed93f66e896f3d38df0d2b8893f0328
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 4294 txn/s, latency: 7393 ms, (p50: 7800 ms, p90: 10500 ms, p99: 11100 ms), latency samples: 167480
5. check swarm health
Compatibility test for aptos-node-v1.5.1 ==> 6be384298ed93f66e896f3d38df0d2b8893f0328 passed
Test Ok

@0xmaayan 0xmaayan merged commit f0de1b3 into main Aug 2, 2023
@0xmaayan 0xmaayan deleted the indexer_improv branch August 2, 2023 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants