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

Operations that return Iterators have limited functionality when called via JSON-RPC #2699

Closed
devhawk opened this issue Apr 22, 2022 · 2 comments · Fixed by neo-project/neo-modules#715
Labels
Discussion Initial issue state - proposed but not yet accepted

Comments

@devhawk
Copy link
Contributor

devhawk commented Apr 22, 2022

Background:

Operations that return iterators only return the first MaxIteratorResultItems items. Developers have NO mechanism to retrieve any values past the first MaxIteratorResultItems count.

This may be purely fixable in neo-modules repo, but I'm filing it in neo repo both for visibility and because at least one neo repo issue (#2683) and PR (#2686) are dealing with it. We can move to a modules repo if that's a better place for it.

I'd like to see Iterator operations work similarly to the StateService.FindStates RPC endpoint. FindStates takes an optional from parameter that indicates which records to skip. It also includes a truncated property that the caller can use to determine if there are more records to retrieve.

RpcServer already returns a truncated property for Iterator types, similar to the JSON returned by FindStates.

We can add logic to InvokeFunction and InvokeScript to retrieve an optional integer parameter. Those two methods already have duplicate logic for parsing signers and useDiagnostic. It would be fairly straightforward to DRY out that logic + add additional code to retrieve an optional skip parameter from the JSON array. This skip parameter value could then be passed to GetInvokeResult which would be used for converting an Iterator to JSON.

For reference, here's the RpcClient method declaration of FindStatesAsync:

Task<RpcFoundStates> FindStatesAsync(
    UInt256 rootHash, UInt160 scriptHash, ReadOnlyMemory<byte> prefix, 
    ReadOnlyMemory<byte> from = default,  int? count = null)

And the declaration of RpcFoundStates

public class RpcFoundStates
{
    public bool Truncated;
    public (byte[] key, byte[] value)[] Results;
    public byte[] FirstProof;
    public byte[] LastProof;
}

Here's an example of how FindStates can be used in a loop to retrieve all the storage records:

ReadOnlyMemory<byte> from = default;
var states = Enumerable.Empty<(byte[] key, byte[] value)>();

while (true)
{
    var response = await stateApi.FindStatesAsync(rootHash, contractHash, prefix, from);
    if (response.Results.Length == 0) break;
    // validate proof code omitted
    states = states.Concat(response.Results);
    if (!response.Truncated) break;
    start = response.Results[^1].key;
}
// states now has all the found states. 
@devhawk devhawk added the Discussion Initial issue state - proposed but not yet accepted label Apr 22, 2022
@roman-khimov
Copy link
Contributor

  1. A stack returned from these RPCs is a set of items, so we can have multiple iterators there, what are we going to do with them?
  2. Paging will require performing the same invocation again and again with this scheme (findstates is much easier wrt this).

That's not to say that we don't have a problem, but I'm not sure what's the best solution.

@devhawk
Copy link
Contributor Author

devhawk commented Apr 23, 2022

  1. A stack returned from these RPCs is a set of items, so we can have multiple iterators there, what are we going to do with them?

I'm not that worried about multiple return values. Returning multiple items at all is atypical and I would expect returning more than one iterator is even less common.

  1. Paging will require performing the same invocation again and again with this scheme (findstates is much easier wrt this).

I don't see any way to avoid multiple invocations if we're limited to using a stateless transport like HTTP. We could look at using a stateful protocol like WebSockets for pulling additional pages of data out of the iterator.

That's not to say that we don't have a problem, but I'm not sure what's the best solution.

I am very much open to suggestions. The design in #708 is an experiment to see what could be done w/ the least code churn. I'll think about the WebSocket approach and sketch that out as well in the next week or so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@devhawk @roman-khimov and others