Fix GetCandidates #2686
Fix GetCandidates #2686
Conversation
| @@ -344,7 +344,26 @@ private async ContractTask<bool> Vote(ApplicationEngine engine, UInt160 account, | |||
| ( | |||
| p.Key.Key.AsSerializable<ECPoint>(1), | |||
| p.Value.GetInteroperable<CandidateState>() | |||
| )).Where(p => p.Item2.Registered).Select(p => (p.Item1, p.Item2.Votes)).ToArray(); | |||
| )).Where(p => p.Item2.Registered).Select(p => (p.Item1, p.Item2.Votes)).Take(100).ToArray(); | |||
There was a problem hiding this comment.
How does the caller retrieve the remaining candidates if there are more than 100?
There was a problem hiding this comment.
Do you think that it's needed?
There was a problem hiding this comment.
This method was used by Grant Shares so or we add a new method for paginate it, or I think that we can't add a new arguments here (without versioning)
There was a problem hiding this comment.
Do you think that it's needed?
Yes, I think it's a bad idea to arbitrarily limit return values without providing a mechanism to paginate. We have similar issues w/ accessing iterators over JSON-RPC (neo-project/neo-modules#629 and neo-project/neo-devpack-dotnet#647)
This method was used by Grant Shares so or we add a new method for paginate it, or I think that we can't add a new arguments here (without versioning)
We can add an overload that takes skip + take parameters and returns a structure that has the (ECPoint PublicKey, BigInteger Votes) tuple array + a boolean to indicate if there are more results (similar to the FindState State Service RPC method). That way we don't break existing callers but we can still provide the full functionality to contracts that switch to the new overload.
There was a problem hiding this comment.
Please, take a look again
There was a problem hiding this comment.
This works, though I like the FindState pattern much better.
I decided to build a single GitHub issue to track the Iterator JSON RPC return issue #2699. I actually think it would be pretty straightforward to add Iterator skip support. Assuming we can add that, then we could change GetCandidates to return an Iterator
|
First draft of paged iterator support : neo-project/neo-modules#708 |
| /// <param name="engine">The <see cref="ApplicationEngine"/> that is executing the contract.</param> | ||
| /// <returns>All the registered candidates.</returns> | ||
| [ContractMethod(CpuFee = 1 << 22, RequiredCallFlags = CallFlags.ReadStates)] | ||
| private IIterator GetAllCandidates(ApplicationEngine engine) |
There was a problem hiding this comment.
I'm glad you've added this, but we still have no mechanism to retrieve all the values of an iterator over JSON-RPC
Close #2683