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

RPC-JSON over WebSockets #847

Closed
wants to merge 50 commits into from

Conversation

cschuchardt88
Copy link
Member

@cschuchardt88 cschuchardt88 commented Nov 23, 2023

Description will be updated soon....

Some things to know

  • URL(s) - path and query can be anything. (Note: use OnRequest event to get custom paths/headers and query params)
    • ws://127.0.0.1:10340/
    • ws://127.0.0.1:10340/?MyCustomParam=HelloWorld
    • ws://127.0.0.1:10340/MyCustomPath/FakePage.html?say=Hello%20Bob
  • Uses Neo.Json
  • Has expandability of API in other plugins.
  • Emits events that plugins can hook into for more information on clients.
  • 99% uptime (for clients) (unless neo-core fails)
  • It's up to clients to filter events
  • Emits block/transaction/contract events to clients
  • RPC like functionally over WebSocket

Events and their IDs

public enum WebSocketResponseMessageEvent : byte
{
    Block = 0x02,               // Blockchain.Committed
    MemoryPool = 0x03,          // MemoryPool
    Transaction = 0x05,         // Transaction
    ContractNotify = 0x14,      // ApplicationEngine.Notify
    AppLog = 0x15,              // ApplicationEngine.Log
    DebugLog = 0x16,            // Utility.Log
    Error = 0x19,               // Error(s)
    Method = 0x20,              // Result(s) from a method call
}

Request information

{
    "version": "1.0",       // Protocol version
    "requestid": 999,       // For tracking which response goto which request (-1 for system error)
    "method": "getblock",   // the name of the method to invoke
    "params": [             // parameters for the method
        "0"
    ]
}

config.json

{
  "PluginConfiguration": {
    "Network": 860833102,
    "BindAddress": "127.0.0.1",
    "Port": 10340,
    "SslCertFile": "",
    "SslCertPassword": "",
    "TrustedAuthorities": [],
    "EnableBasicAuthentication": false,
    "User": "",
    "Pass": "",
    "AllowOrigins": [],
    "ConcurrentProxyTimeout": 120,
    "MessageSize": 4096,
    "MaxGasInvoke": 2000000000,
    "DebugMode": false
  }
}

Developers API (all static)

Events

WebSocketServerPlugin.OnRequest(HttpContext httpContext)               // Emits after valid WebSocket connection request is made
WebSocketServerPlugin.OnServerStarted()                                // Emits after WebSocket host is up
WebSocketConnection.OnMessageSent(Guid clientId, JToken json)          // Emits after message is successfully sent to client
WebSocketConnection.OnMessageReceived(Guid clientId, JToken json)      // Emits after a message is received and serialized
WebSocketConnection.OnConnect(Guid clientId)                           // Emits after a successful connection is made
WebSocketConnection.OnDisconnect(Guid clientId)                        // Emits after a successful disconnect

Methods

WebSocketServerPlugin.RegisterMethods(object handler)                                      // Registers methods for a given class with attribute WebSocketMethod
WebSocketServerPlugin.SendAllJson(JToken json, int pluginId)                               // Sends to all clients
WebSocketServerPlugin.SendJson(Guid clientId, int requestId byte eventId, JToken json)     // Send to a specified WebSocket by client id, (note: use "OnConnect" event to get client id)

Properties

WebSocketServerPlugin.HasClients        // Whether the server has valid connections to clients

Creating a method in Code

[WebSocketMethod]
private JToken Echo(JArray @params) // turns into all lowercase for method name
{
    return @params;
}
// -----OR-----
[WebSocketMethod(Name = "echo")] // name is case sensitive
private async Task<JToken> EchoAsync(JArray @params) // method would be "echoasync`, if not specified by "Name" in attribute
{
    return @params;
}

Video of Syncing (Blockchain)

Recording.2023-11-25.233845.mp4

Screenshots

Block Event (on commit)
Screenshot 2023-11-22 184425

Invoke Response (Echo Method)
Screenshot 2023-11-22 184405

Contract Response (on syncing/new block)
image

Transaction Response (on syncing/new block)
image

Block Response (on syncing/new block)
image

Change Log

  • Added WebSocketServer

closes #716

@roman-khimov
Copy link
Contributor

I think community will love two absolutely different websocket interfaces.

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Nov 23, 2023

This would make 3? Because neo-cli node has one built-in too (but thats for syncing blockchain). But this is meant to replace #795. Now are you talking about neo-go's? If that is the case maybe we can build the same way in this PR. What my goal is to allow users to have a fully functioning API that does everything that neo-cli and neo-express does. The reason behind this; is to be able to support other platforms and programming languages along with special use cases. The way that neo-core sits right now... it can't be done easily; if not at all. But with the help of this and other people expanding this API. We can tackle web, dotnet and other ecosystems. This also allows others to make plugins to expand the api for custom in house applications that developers or organizations may need for there only business needs. I think the add-on of this new sidechain; it would be time for ALL developers and users to be able to use neo if they choose too. WebSockets would be the easiest way to start. With web3 most people want their own dapp to be able to get blockchain information and full neo-core functionally. Currently you would have to build your own.

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Nov 25, 2023

What this feature offers, is MUST to have. My other reasoning is that with neo3-visual-tracker (vscode-extension) uses 3rd party javascript library (neon-js) for accessing the RPCServer of neo-express. Now this is very buggy as we know. Could be the because of old version. With this websocket API we can easily get new blocks and other information that is needed for this extension. I am currently rebuilding this extension, and need this plugin, or else I'll make it just for neo-express, But i thought the features it offers would be a good choice for community and ecosystem.

@cschuchardt88
Copy link
Member Author

@roman-khimov @superboyiii We would like to use this feature in neo-express for the updated neo3-visual-tracker to use (vscode extension). Any ETA on or how or if this feature will be added to core?

@cschuchardt88 cschuchardt88 marked this pull request as ready for review December 30, 2023 19:35
{
internal class WebSocketClient : IDisposable, IEquatable<WebSocketClient>
{
public WebSocket Socket { get; init; }
Copy link
Member

Choose a reason for hiding this comment

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

required and remove null checks?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not null there. Hints the reason why no ? and init on property. We don't want null.

Copy link
Member

Choose a reason for hiding this comment

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

It is not null there. Hints the reason why no ? and init on property. We don't want null.

Yep, but it's used with ? inside the class

Copy link
Member Author

@cschuchardt88 cschuchardt88 Jan 3, 2024

Choose a reason for hiding this comment

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

Right and that reason, is for if client disconnect and the class becomes disposed. But you should never pass in a null WebSocket.

Copy link
Member

Choose a reason for hiding this comment

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

I marked the project as nullable and I detected it, I was going to fix it, I think that new projects should be nullable

@cschuchardt88
Copy link
Member Author

@superboyiii If this PR can you on your list ASAP for testing. I need this PR to update the UI for neo-express (vscode extension). The extension is pretty bad at the moment. It unworkable. All the project and templates it generates are useless for any new developer; even those that don't know C#.

Copy link
Member Author

@cschuchardt88 cschuchardt88 left a comment

Choose a reason for hiding this comment

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

Please fix @shargon

src/WebSocketServer/WebSocketClient.cs Outdated Show resolved Hide resolved
@@ -13,10 +13,9 @@

namespace Neo.Plugins
{
internal class WebSocketConnection<TClient> : IDictionary<Guid, TClient>, IDisposable
where TClient : WebSocketClient, new()
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to change this back. WebSocketClient is a base class. You can extend the class by inheriting it. For example if you want to add more properties/fields or methods to it. Like a tracking number or Client's login information.

Copy link
Member Author

@cschuchardt88 cschuchardt88 Jan 3, 2024

Choose a reason for hiding this comment

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

I program dynamically and make things reuse able.

Copy link
Member

Choose a reason for hiding this comment

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

Need to change this back. WebSocketClient is a base class. You can extend the class by inheriting it. For example if you want to add more properties/fields or methods to it. Like a tracking number or Client's login information.

We can extend WebSocketClient, we don't need more types of WebSocketClients, sometimes simpler is better

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 in theory. But this also limits the API or makes more work in the future. If the code is already there or built for reusability, than the code should stay.

Copy link
Member

Choose a reason for hiding this comment

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

If the code is already there or built for reusability, than the code should stay.

I'm not agree, we don't need two WebSocketClients

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Please set the project as nullable, I tried to change it, but it was reverted one of my commits

{
internal class WebSocketClient : IDisposable, IEquatable<WebSocketClient>
{
public WebSocket? Socket { get; init; }
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this with ?, only is needed because you want a T type that we don't need

Copy link
Member Author

@cschuchardt88 cschuchardt88 Jan 3, 2024

Choose a reason for hiding this comment

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

I don't understand. Referring to T type; we do need it! This a BASE CLASS. NOT DOING SO WILL LIMIT AND FLAW EXPANDABLE OF AND POINT OF THIS LIBRARY. If we want to keep limiting the api we can do that. Just look at RpcServer is outdated and cannot be expanded or updated easily. Which is why no one touches it and in the state it's in now.

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

remainder to check #859, remove websocket

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

@shargon, @Jim8y, @vncoelho since the work on this PR is continuing, I'd like to pay your attention one more time to the problem mentioned by @roman-khimov in #847 (comment) and to the comment of @cschuchardt88 in #847 (comment).

The case is that NeoGo node already has WS server that handles all standard RPC methods exactly the same way as regular RPC server does (using exactly the same response structures and without handlers code duplication, see the comment below). Also, NeoGo WS server has subscriptions interface and notifications subsystem implemented for more than 3 years (check out the https://github.com/nspcc-dev/neo-go/blob/master/docs/notifications.md and code connected with https://github.com/nspcc-dev/neo-go/blob/d90169761595933febb8a25d1abde2e46154b96d/pkg/services/rpcsrv/server.go#L265). I know that there are community users of the existing NeoGo WS system (and, in particular, WS notification subsystem), and another WS interface implementation would be a problem for our users. We need a single unified and aligned websocket interface. What do you think?

src/WebSocketServer/v1/BlockchainMethods.cs Show resolved Hide resolved
@cschuchardt88 cschuchardt88 changed the title Web Socket Server RPC-JSON over WebSockets Jan 14, 2024
@Jim8y Jim8y added the Need Active Pr will be closed after one week if no new activity. label Feb 12, 2024
@shargon
Copy link
Member

shargon commented May 21, 2024

Repository moved to neo, please re-open there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need Active Pr will be closed after one week if no new activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a session protocol for some JSON-RPC methods
6 participants