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

Add Rest Web Server WIP #810

Merged
merged 55 commits into from
Nov 8, 2023
Merged

Add Rest Web Server WIP #810

merged 55 commits into from
Nov 8, 2023

Conversation

cschuchardt88
Copy link
Member

@cschuchardt88 cschuchardt88 commented Aug 16, 2023

Adding Restful services to Neo N3 ecosystem. This uses MVC model without the view aspect. I currently don't know how much easier I can make this plugin. When it comes to flexibility and freedom. This system was design with usability. To allow the freedom of making what you want with plugins and will have a fault tolerant model. Also this model can allow upgrades, if Neo ever decides to make neo-cli have a built in webserver that handles RPC, Rest, WSS; this would be the code to do so. The already built plugins that are using this RestServer Plugin would not need to be changed.

This plugin is test ready. I'm not trying to rush it and understand it takes a lot of time. But I would love some feedback on this plugin. If you decide to not use this plugin. By all means this plugin will not end here.

The goal here is to have its client be dependency free of the Neo.dll and all its dependencies. Of course you can still add them if need be. Like Neo.vm.dll for building scripts to send via Rest service call. I want to have this client and this plugins endpoints be available in other parts of the dotnet ecosystem. Just to name a few Mono.Net, .NET Maui and Unity. This will help the Neo user base have no limitations on whatever framework or technology they desire to use.

As for maintainability, I'm not going anywhere; anytime soon and I have tons of time on my hands to write document, answer questions and update code. You have to start somewhere; so let's come together!

Yes I know it seemed fast; for me to make this plugin, But I really started at the begin of the year working on it and learning the neo-cli. 😸 So this is 5 months worth of learning and reading source code. 3 months of programming the code. My background is architecture design of systems.

See neo-project/neo#1004

Documentation

This RestServer Documentation explains the working of the RestServer plugin. Note still a work in progress.

RestServer Addon Plugins
This Example Plugin Documentation explains how to make plugins for the RestServer.

Downloads

Swagger UI

RestClient Code Example

using RestClient;

using var httpClient = new HttpClient();
var restClient = new RestWebClient("http://127.0.0.1:10339/", httpClient);

try
{
    var block = await restClient.GetBlockAsync(0).ConfigureAwait(false);
    Console.WriteLine("Block Hash: {0}", block.Hash);
}
catch (ApiException<ErrorModel> ex)
{
    Console.WriteLine(ex.Result.Message);
}
Console.WriteLine("Please ENTER to exit.");
Console.ReadLine();

Key Features

  • Override Error Handling (for cleaner errors as json responses, will not crash clients or give unexpected error responses)
  • Compression
  • CORS
  • CORS Origins
  • HTTPS
  • Basic Authentication
  • Disable Controllers (from other plugins)
  • Proxy Forwards (for data center usage)
  • Wallet Sessions
  • Json Converter (for having parameter types like UInt160, UInt256, StackItem, etc)
  • Implements Contracts, Ledger, Node, Token, Wallet and Utils full functionality
  • Plugin Support (easy process with no dependencies to this library)
  • Cleaner Plugin Code (for new plugins)
  • Parameter Type Binding
  • Swagger UI
  • Plugin Support

Change Log

  • Ledger Controller API (Interaction Ability)
  • Contracts Controller API (Interaction Ability)
  • Neo/Gas Controller API (Display Accounts)
  • Node Controller API (Block Height, Peers and Other Information)
  • Token Controller API (Nep17/Nep11 Interaction)
  • Wallet Controller API (Open, Close, Transactions, etc)
  • Utils Controller API (Conversion Tools & Helpers)
  • Expandability (Rest Routes)
  • Enable Https
  • CORS
  • Response Compression (GZip)
  • Basic Authentication
  • The ability to disable controller routes
  • Any HTTP Method
  • Any Route
  • Custom HTTP Headers
  • Rest Client (No Dependencies Required) Get It Here
  • Wallet Sessions
  • Multi-thread
  • Swagger
  • Update Swagger Endpoint Descriptions
  • Check for typos & misleading end user text (in Swagger)
  • Plugin Support

Testing Progress

  • JSON Converters (Schema)
  • Routes (Inputs/Outputs/Parameters/Headers)
  • All Configurations
  • Authentication
  • Disable Controllers
  • CORS
  • Proxy Forwards
  • Compression
  • Integration (With Other Plugins)
  • Loading Process (Load Order)
  • Wallet API (File Access, Sessions & Etc)
  • Load MainNet or TestNet T5 (Check Data Integrity & Loading on Big Data)
  • Dummy Controller (Plugin)
  • Swagger

Neo-Cli Commands

image

You can just add Swagger OpenAPI 2.0 to any project by

Step1
image

Step 2
image

Step 3
image

@cschuchardt88 cschuchardt88 marked this pull request as draft August 16, 2023 22:01
Christopher R. Schuchardt and others added 7 commits August 16, 2023 18:16
Added HTTP Basic Auth.
Changed most classes to internal that didnt need to be public.
Added Rest Server Middleware.
Added the ability to block urls paths (for module rest server plugins).
Added the ability to enable CORS and whitelist of urls.
Added HTTP user and pass configurable.
Added hot load of Controllers from plugin assemblies
Changed Controllers to be "ApiController" class
Added MaxTransactionFee to config
Added Anything for Kestrel to be configurable
cschuchardt88 and others added 10 commits August 19, 2023 01:37
Added Nep11 token controller.
Changed namespaces and file names around.
Added peers route for node controller
Changed RemoteNodeModel and TokenBalanceModel namespaces
Added Neo/Gas controller
Added send raw transactions relay
Fixed formating
Added blacklist/disable controllers
Removed DisableRoute list from config (wasn't being used)
… format

Added InvokeScript to helper class
Added ConvertToScriptHash for address to scripthash
Changed all controller parameters for json converters
Added custom error handling
Fixed wallet sessions
Added wallet session manager
removed development mode (not needed)
Added session timeout to config file
Added max allow sessions to config file
@cschuchardt88 cschuchardt88 marked this pull request as ready for review August 21, 2023 01:10
@cschuchardt88
Copy link
Member Author

Before i make client can someone make sure we are going to use this?

@vncoelho
Copy link
Member

@cschuchardt88, It is a mechanism that many developers requested to us in the past, as far as I can remember.
In my opinion, many people would use it.
I would like to hear as well from other developers from the Python team that already did something similar in the past: @ixje @lllwvlvwlll

One point that worries me a little bit is about the maintenance and security of some calls, how could we make it more generic related to the current RPC Server plugin or RPC Client?
That would gives us less burden for keeping this plugin up to date.

@shargon
Copy link
Member

shargon commented Aug 21, 2023

Swagger can be added to the list

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Aug 21, 2023

The RestServer implements the same things for Kestrel as RpcServer does. Plus newer features. Everything is configurable and can be turned off. For added security you can enable basic authentication and/or https, with the ability to use a forward proxy. Also you have the ability to disable controllers and by default we disable WalletController. So if anyone want to make this plugin be public on a public node. We don't want people trying to open wallets or search around on the node for one. This uses middleware to block the controllers from ever being loaded. "DisableControllers": [ "WalletController" ]. Trying to access disabled controller routes with end in a 404 response from the server.

This is very generic but may appear not to be. Most the code is Model/Json converters for serializing the Models to JSON and JSON to Objects. These include UInt160, UInt256, StackItem, Transactions and Blocks to just name a few. If the application breaks I have Middleware set to not leak sensitive information to the end user. Server will return generic responses like see below. In the event of a crash of the application from any controller will result in a generic exception json response. To ensure applications that are consuming this RestServer will not have inconsistent responses for crashes or errors. Also any exception that is thrown will also result in error response like see below. So instead of returning your own response in a plugin for example with your own error. You would just throw an exception and the result will be 400 response with see below as example

{
    "code": 1001,
    "name": "ParameterFormatException",
    "message": "Parameter hash is in an invalid format."
}

if you have a look at theses files RestServerPlugin.cs, RestServerSettings.cs and RestWebServer.cs they can tell the whole story of what's going on and give you the idea of what is happening and the working of the application.

@shargon yes I can add it.

Edit:
I added swagger to the RestServer per @shargon

@cschuchardt88
Copy link
Member Author

This is at the point where you can try it out and see if it meets your requirements. Everything is up for discussion. If you can't use it either that's fine too.

Some questions i have:

  1. Is it something you can use?
  2. Is it something you can maintain?
  3. Missing any features?
  4. What will it take to get moving forward in a release?

But i do think this is a good start to get it moving out the door for merger.

Jim8y
Jim8y previously approved these changes Oct 9, 2023
@Jim8y
Copy link
Contributor

Jim8y commented Oct 9, 2023

too long to review every single line, but functioning ok, maybe we can have it at first, then optimize it little by little. I personally love and welcome new features added.

@shargon shargon requested a review from superboyiii October 22, 2023 07:18
Jim8y
Jim8y previously approved these changes Nov 8, 2023
@Jim8y
Copy link
Contributor

Jim8y commented Nov 8, 2023

@cschuchardt88 do you mind to provide a readme for this plugin? i know others dont have it, but we will add readme for every plugin in the future. Mainly about what it is, what it does, how to use it, extra. basically what you have in the description of this pr

@Jim8y
Copy link
Contributor

Jim8y commented Nov 8, 2023

Merge since its a plugin, people dont install it if they dont like it.

@Jim8y Jim8y merged commit e9d8683 into neo-project:master Nov 8, 2023
@cschuchardt88 cschuchardt88 deleted the RestServer branch November 8, 2023 07:35
@roman-khimov
Copy link
Contributor

@Liaojinghui, you're rushing too fast with this, IMO. People will install it, people will try building things on it. I'm not sure it's tested enough and stable enough for them to do that. And notice that it could've been a separate plugin for some time exactly for these experiments. Now it's an official plugin from the Neo project, people have higher expectations of those.

@cschuchardt88
Copy link
Member Author

@roman-khimov
I use it all the time. So It pretty stable. Its maybe lacking features. But all the basic and some advanced are all there.

@shargon
Copy link
Member

shargon commented Nov 8, 2023

@Liaojinghui, you're rushing too fast with this, IMO. People will install it, people will try building things on it. I'm not sure it's tested enough and stable enough for them to do that. And notice that it could've been a separate plugin for some time exactly for these experiments. Now it's an official plugin from the Neo project, people have higher expectations of those.

Agree and reverted (8337202). Too many files without a deep review.

shargon added a commit that referenced this pull request Nov 8, 2023
Jim8y added a commit that referenced this pull request Nov 8, 2023
@Jim8y
Copy link
Contributor

Jim8y commented Nov 8, 2023

@Liaojinghui, you're rushing too fast with this, IMO. People will install it, people will try building things on it. I'm not sure it's tested enough and stable enough for them to do that. And notice that it could've been a separate plugin for some time exactly for these experiments. Now it's an official plugin from the Neo project, people have higher expectations of those.

Agree, but only if someone review it. I dont see how this can introduce more bugs than there has already be. I dont understand why you still stop merge and expect some one to review it while a pr has being there for 3 months and no one to review. I have checked the code and run it, no abvious issues found.

@shargon
Copy link
Member

shargon commented Nov 8, 2023

@roman-khimov I use it all the time. So It pretty stable. Its maybe lacking features. But all the basic and some advanced are all there.

Agree, but only if someone review it. I dont see how this can introduce more bugs than there has already be. I dont understand why you still stop merge and expect some one to review it while a pr has being there for 3 months and no one to review.

I understand, but understand that you have to review the code before merging, the more changes, the more complicated and time-consuming it requires, imagine (which I don't think is the case) that you add a line to leak the private key, all the code must be reviewed by at least two core developers, and tested by the great @superboyiii team.

Small pr will be reviewed faster than longer ones.

@Jim8y
Copy link
Contributor

Jim8y commented Nov 8, 2023

Yes, the same reason why so many prs stay there for many years without anyone care about them.

@shargon
Copy link
Member

shargon commented Nov 8, 2023

Yes, the same reason why so many prs stay there for many years without anyone care about them.

The solution is not merge all of them.

@shargon
Copy link
Member

shargon commented Nov 8, 2023

@cschuchardt88 please open a new PR with these changes.

@Jim8y
Copy link
Contributor

Jim8y commented Nov 8, 2023

Not all of them, but something i have reviewed, and no one else review it after months. We can wait, but should not be endless waiting.

@cschuchardt88 cschuchardt88 restored the RestServer branch November 8, 2023 08:14
@roman-khimov
Copy link
Contributor

That's why roadmaps and plans matter. Set some target, like "this PR should be dealt with by this time" and work towards that goal. Not planned? Likely not critical for now then (sure, life is complicated and sometimes it is). Add it to some milestone in the future, but keep real concentration on the nearest goals. We can't solve all problems at once and we can't risk destabilizing the system by accepting everything without proper reviews and tests. Going one problem after another with intermediate stable releases is more viable.

@cschuchardt88 cschuchardt88 mentioned this pull request Dec 25, 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.

5 participants