feat(governance): implement pagination for list_neurons endpoint

[yhabib]

Motivation

As presented 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.

Some insights from the Boundary Node.

Expect usage patterns:

• Non-empty neurons:
  • All users with fewer than 500 neurons → 1 page
• Non-empty and empty neurons, such as Reporting feature:
  • Most users with fewer than 500 neurons → 1 page
  • Some users with approximately 5,000 neurons → 10 pages
  • Few users with approximately 40,000 neurons → 80 pages

Changes

• Implements pagination by checking the number of available pages from the initial call to list-neurons.

Tests

• Added unit tests to check pagination.
• Manually tested locally in the nns-dapp with over 1000 neurons.

Screen.Recording.2025-02-11.at.16.37.00.mov

Todos

• Add entry to changelog (if necessary).

[github-actions]

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.41 KB (0%)
@dfinity/nns	37.55 KB (+0.66% 🔺)
@dfinity/nns-proto	140.99 KB (0%)
@dfinity/sns	17.28 KB (0%)
@dfinity/utils	4.71 KB (0%)
@dfinity/zod-schemas	626 B (0%)
@dfinity/ic-management	3.22 KB (0%)

[yhabib]

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.

[lmuntaner]

Why would option 3 be a breaking change? Aren't the new fields optional?

I can see a client wanted to take care of the pagination, but probably most won't want to deal with it.

IMHO, I would prefer a function with no pagination and another function with pagination. It makes sense that the current function offers no pagination. The current behavior was always to return all the neurons, so there is no change there, right?

[dskloetd]

I think option 1 is fine for now. As long as we are aware that the issue is not solved for hardware wallets.

[mstrasinskis]

Because of the large page size, I doubt that exposing pagination will provide much benefit to users. Option 1 sounds reasonable to me.

[yhabib]

@lmuntaner answering your questions

Why would option 3 be a breaking change? Aren't the new fields optional?

The response signature will change if we want to include pagination information.

IMHO, I would prefer a function with no pagination and another function with pagination. It makes sense that the current function offers no pagination. The current behavior was always to return all the neurons, so there is no change there, right?

The current function without pagination will work until the first user exceeds 500 neurons. At that point, the result will be incorrect, as it will only return the first 500 neurons, leaving the consumer unaware of the limitation. Therefore, everyone should eventually migrate to the new function.

[lmuntaner]

The response signature will change if we want to include pagination information.

That will be managed by agent-js, right? the client of ic-js doesn't know about this. Would it break something for the ic-js client?

leaving the consumer unaware of the limitation

I agree. I meant that the current clients of list_neurons expect to receive all neurons after such a call. If you make recursive calls because now the backend requires pagination is consistent with the behavior of the method as of today.

[dskloetd]

The current listNeurons function does not return the response from the agent directly. It returns NeuronInfo[]:
https://github.com/dfinity/ic-js/pull/833/files#diff-f4f0835b56118edf98f5b690eb524de8bd8738c8d742c69c3c2cd516d55e9857R157

[lmuntaner]

The current listNeurons function does not return the response from the agent directly. It returns NeuronInfo[]: https://github.com/dfinity/ic-js/pull/833/files#diff-f4f0835b56118edf98f5b690eb524de8bd8738c8d742c69c3c2cd516d55e9857R157

Ah, I see. And we can't add the page of those results now. Got it. Thanks! I forgot.

[dskloetd]

In the PR description:

Ran ./scripts/import-candid ../ic from ic-js root folder
Ran ./scripts/compile-idl-js from ic-js root folder
Reverted all canisters except the governance canisters.

Is this outdated?

[mstrasinskis]

Thanks!