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

Allow traverse iterators #715

Merged
merged 13 commits into from
Jun 24, 2022
Merged

Allow traverse iterators #715

merged 13 commits into from
Jun 24, 2022

Conversation

erikzhang
Copy link
Member

@erikzhang erikzhang commented May 22, 2022

src/RpcServer/Signers.cs Outdated Show resolved Hide resolved
src/RpcServer/Session.cs Outdated Show resolved Hide resolved
@devhawk
Copy link
Contributor

devhawk commented May 22, 2022

This is certainly a better approach than I sketched out with #708.

One thing I got to thinking about while I was sketching #708: Should we consider exposing some RPC methods via WebSockets and/or SignalR? WebSockets would be a better model for exposing iterators that anything that runs over HTTP. Plus we could implement callback style operations like "notify me for every new block" and "submit this transaction and wait for it to be included in a block" with reasonable efficiency.

Obviously, such an approach should be a separate PR if we decide to go this route. I opened #716 to discuss

@roman-khimov
Copy link
Contributor

Should we consider exposing some RPC methods via WebSockets

NeoGo provides that along with event subscription mechanism.

@erikzhang
Copy link
Member Author

Do we need to update the RpcClient?

@devhawk
Copy link
Contributor

devhawk commented Jun 2, 2022

Do we need to update the RpcClient?

Yes. It would be nice to expose Iterators in RpcClient via IAsyncEnumerable. But I personally don't have an issue doing that as a separate PR

@Ashuaidehao Ashuaidehao mentioned this pull request Jun 3, 2022
[RpcMethod]
protected virtual JObject InvokeFunction(JArray _params)
{
UInt160 script_hash = UInt160.Parse(_params[0].AsString());
string operation = _params[1].AsString();
ContractParameter[] args = _params.Count >= 3 ? ((JArray)_params[2]).Select(p => ContractParameter.FromJson(p)).ToArray() : System.Array.Empty<ContractParameter>();
Signers signers = _params.Count >= 4 ? SignersFromJson((JArray)_params[3], system.Settings) : null;
Signer[] signers = _params.Count >= 4 ? SignersFromJson((JArray)_params[3], system.Settings) : null;
Witness[] witnesses = _params.Count >= 4 ? WitnessesFromJson((JArray)_params[3]) : null;
Copy link
Member

Choose a reason for hiding this comment

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

Saved in the same _params as signers? (_params[3])

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

Comment on lines 158 to 160
Guid id = Guid.NewGuid();
session.Iterators.Add(id, iterator);
json["id"] = id.ToString();
Copy link
Member

Choose a reason for hiding this comment

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

The resulting JSON contains id for each iterator even if sessions are disabled. Is it the desired behaviour?

src/RpcServer/RpcServer.SmartContract.cs Show resolved Hide resolved
@ProDog
Copy link
Contributor

ProDog commented Jun 24, 2022

Tested OK with #721.

@erikzhang erikzhang merged commit aa730fb into master Jun 24, 2022
@erikzhang erikzhang deleted the traverse-iterator branch June 24, 2022 02:52
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.

Operations that return Iterators have limited functionality when called via JSON-RPC
8 participants