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

DRAFT pagination for nns governance list_neurons #833

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yhabib
Copy link
Contributor

@yhabib yhabib commented Jan 30, 2025

Motivation

As present here and changed here, the NNS governance canister will implement pagination for the list_neurons endpoint.

Pagination will only activate if the response contains more than 500 neurons. Currently, most of our cases involve requesting non-empty neurons, and we do not have users with such a high number of neurons, so for those users nothing has changed. However, the nns-dapp has a use case, Reporting, where we also request empty neurons. In this case, we have a few hundred users with more than 500 neurons.

Changes

  • Ran ./scripts/import-candid ../../ic
  • Ran ./scripts/compile-idl-js
  • Reverted all canisters except the governance canisters.
  • Recursive logic

Tests

  • todo

Todos

  • Add entry to changelog (if necessary).

Copy link
Contributor

size-limit report 📦

Path Size
@dfinity/ckbtc 8.12 KB (0%)
@dfinity/cketh 3.68 KB (0%)
@dfinity/cmc 1.41 KB (0%)
@dfinity/ledger-icrc 4.29 KB (0%)
@dfinity/ledger-icp 15.45 KB (0%)
@dfinity/nns 37.43 KB (+0.65% 🔺)
@dfinity/nns-proto 140.98 KB (0%)
@dfinity/sns 17.19 KB (0%)
@dfinity/utils 4.71 KB (0%)
@dfinity/zod-schemas 626 B (0%)
@dfinity/ic-management 3.22 KB (0%)

@yhabib
Copy link
Contributor Author

yhabib commented Jan 30, 2025

Hi @dskloetd @mstrasinskis @peterpeterparker @lmuntaner

I'm pinging you all as you've contributed to the code related to the area of this change, and I would like your opinion on how to proceed.

As mentioned in the description, the governance canister will introduce pagination to the list_neurons endpoint. I see several ways to move forward:

  1. Update the existing endpoint to make recursive calls, abstracting the pagination from the clients.
  2. Update the existing endpoint and expose the pagination logic to the client, allowing them to manage it. This would be a breaking change, as it would significantly alter the response signature.
  3. Create a new function that abstracts the recursion from the client.
  4. Create a new function that exposes the recursion to the client.

I prefer option 1, but I would like to know your thoughts.

Note: the PR is a basic draft. I wanted to get a feeling of how it would look such a feature in ic-js.

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.

1 participant