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

WSS: Add Web Socket Server #795

Closed
wants to merge 21 commits into from
Closed

WSS: Add Web Socket Server #795

wants to merge 21 commits into from

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Mar 26, 2023

Trying to add a wss plugin.

neo-project/neo#2853 This is a good suggestion, maybe we can have wss supported

@Jim8y Jim8y marked this pull request as draft April 9, 2023 14:44
public override string Description => "Enables WebSocket notifications for the node";

private Settings _settings;
private static readonly Dictionary<Guid, BlockWebSocketBehavior> Handlers = new();
Copy link
Member

@cschuchardt88 cschuchardt88 Aug 3, 2023

Choose a reason for hiding this comment

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

I think you should be using ConcurrentDictionary, so you dont have to use Lock. Reason being if have, let's say a million clients using "lock" will have a delay and lock the thread for new clients connecting to the server. You can use ConcurrentDictionary to fix this problem. because like i said new clients wont get a response. especially if there is blocks with 5k transactions in it. They would be waiting for lock to be lifted.

@cschuchardt88
Copy link
Member

cschuchardt88 commented Aug 15, 2023

is there a way for other plugins to add functionality to the wss like RPCMethhods works?

@Jim8y Jim8y changed the title add wss for block WSS: Add Web Socket Server Sep 3, 2023
@cschuchardt88
Copy link
Member

This for runtime only correct? or can you go back in time. Like the place you disconnected. I didn't see anything in code and was just wondering if it was thought upon for maybe a future feature add-on. Other than that it looks good. Thanks for changing to ConcurrentDictionary. Locking can be a pain, I have ran into it too many times in the pass.

@Jim8y
Copy link
Contributor Author

Jim8y commented Sep 3, 2023

@cschuchardt88 jsut made it runtime. can add other features little by little, otherwise this pr would be too complex to review.

@Jim8y
Copy link
Contributor Author

Jim8y commented Sep 3, 2023

is there a way for other plugins to add functionality to the wss like RPCMethhods works?

No, not going to make it work that way. this is just for event notification.

@Jim8y Jim8y marked this pull request as ready for review September 3, 2023 20:14
@cschuchardt88
Copy link
Member

#716 mentions SendRawTransaction or InvokeScript/InvokeFunction for WSS. I didn't see anything in code. Thought you might be interested in adding.

@Jim8y
Copy link
Contributor Author

Jim8y commented Sep 5, 2023

#716 mentions SendRawTransaction or InvokeScript/InvokeFunction for WSS. I didn't see anything in code. Thought you might be interested in adding.

Well,,,,maybe i should combine rpc and wss plugins to add those rpc methods

* master:
  remove Travis logo (neo-project#818)
  Fix 3.6 (neo-project#816)
  DBFTPlugin: adapt Conflicts attribute verification (neo-project#802)
@cschuchardt88
Copy link
Member

cschuchardt88 commented Oct 9, 2023

Why don't you just import RpcServer plugin? Than you can just call those methods. It would work the same way as neo-express .

<PackageReference Include="Neo.Network.RPC.RpcClient" Version="3.6.1" />

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 10, 2023

casue my first thought is they should be seperate projects and do not rely on each other. Let's try to have your rest done first, rpc will be updated anyway.

@cschuchardt88
Copy link
Member

True, I was just thinking less code to maintain.

@cschuchardt88
Copy link
Member

cschuchardt88 commented Nov 21, 2023

@Liaojinghui
I have a solution that only uses pure asp.net core methods just like rpc server and rest web server. It works the exactly the same as RPCServer with the RpcMethod attributes. You just add WebSocketMethod attribute to the same methods. However i am use newtonsoft json serializers. I really think we need to switch core to use newtonsoft or add some kind of attribute like newtonsoft has for knowing whether to serialize the property or field.

Check it out. If you like I can convert to neo-cli module and create PR and remove newtonsoft to use neo.json https://github.com/cschuchardt88/neo-express/blob/construct/v4.0.0/src/neoxp/Node/Modules/ExpressWebSocketServerPlugin.cs

see in action

Recording.2023-11-21.204801.mp4

@Jim8y
Copy link
Contributor Author

Jim8y commented Nov 22, 2023

@cschuchardt88 its a good idea, we can discuss it later after the monorepo.

@Jim8y
Copy link
Contributor Author

Jim8y commented Dec 13, 2023

use #847 instead

@Jim8y Jim8y closed this Dec 13, 2023
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.

3 participants