Skip to content

[FilRPCAudit] Lotus OpenRPC #16

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

Merged
merged 5 commits into from
May 10, 2021
Merged

[FilRPCAudit] Lotus OpenRPC #16

merged 5 commits into from
May 10, 2021

Conversation

olizilla
Copy link
Contributor

@olizilla olizilla commented Apr 7, 2021

Adds exploration report on the state of Lotus OpenRPC support.

View the report in all it's rendered glory

The very short version is, it's a good idea, but we need to make our api definitions more useful, and fix bugs in the docs and client generators.

License: MIT
Signed-off-by: Oli Evans [email protected]

Adds exploration report on the state of Lotus OpenRPC support.

The very short version is, it's a good idea, but we need to make our api definitions more useful, and fix bugs in the docs and client generators.

License: MIT
Signed-off-by: Oli Evans <[email protected]>
Copy link
Contributor

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

Fascinating deep-dive! Thanks for doing this. Some of the suggested recommendations would probably make high-value projects - although they're small enough to fit into "maintenance" but we need to figure out how to get priority for those (or make space to prioritise them ourselves).

/cc @eshon - this might be interesting feedback for the grant work on OpenRPC and worth connecting to any follow-up grant work.

@eshon
Copy link

eshon commented Apr 8, 2021

Wow great analysis @olizilla!

Most of the recommendations seem to be on the Filecoin side. I can ask the OpenRPC devs who worked on the dev grant integration about a follow-up but I'm not sure this scope is in their wheelhouse. If it's not, is this something that can be done internally at PL @rvagg?

We'd also like to get this hosted by Glif Nodes e.g. https://inspector.open-rpc.org/ soon with Lotus API endpoint suggestions to provide some interactive public API Docs for the community. Was going to ask about any existing dockers + devops for that.

@rvagg
Copy link
Contributor

rvagg commented Apr 9, 2021

@eshon yeah, on a whim I decided yesterday to try and tackle the ID problem myself since it seemed like such a high-value improvement: filecoin-project/go-jsonrpc#48
Probably needs some polish to get it land-worthy but it's something.

Improving the state of the docs seem like the next highest-value thing. We're probably best writing that up as a mini project and figure out who can take that on (maybe it's us, just not immediately).

Beyond that, getting integration with js-lotus-client would be fantastic, to minimise duplication and really put this to work. That's yet another project that's not going to be trivial (I suspect).

Like a lot of our audit work at the moment, I think this is going to mainly result in project proposals that will need to be prioritised properly, there's so much to do and we need to be prudent about what gets done but there also might be a bunch of mini-grant work coming out of here too! Will keep you in the loop as we come up for air.

@eshon
Copy link

eshon commented Apr 9, 2021

@olizilla - Can you please briefly eli5 for me what getting this into a Docker + CI should look like so that I can ask Glif Nodes to get this hosted along with one of their existing Lotus endpoints?

I will chat with one of the OpenRPC devs who worked on the dev grant next week about devops.

Will also ask if it can be used somehow in a test harness for the Lotus API - because external collabs depend on the API and need to be aware of changes to the queries they make.

@BelfordZ
Copy link

BelfordZ commented Apr 16, 2021

Firstly let me say thank you for the thorough review. There's no better gift to opensource than the gift of ones time ;). Also, thank you @eshon for brining it to my attention!

I'd love to comment on some of the feedback / points, I hope this is an acceptable venue.

You can't directly use the Typescript API client generated by the recommended open-rpc/generator tool, as Lotus uses a custom json-rpc implementation that does not yet support sending the request id field as a string.

While I see there is progress on fixing in the go tooling, I've also made an issue to make the ID configurable from the client: open-rpc/client-js#279. This would at least give a work around so that others don't run into this as a major roadblock as well.

There does not appear to be a way to pass auth credentails or headers to the generated client.

noted, and made an issue to track this. open-rpc/client-js#280 & open-rpc/generator#624

The open-rpc/generator is under-maintained.

Can't argue with you there. With it's usership growing I can commit more time to it. I've updated the deps on the generator itself and I'm working on speccing out some work to help with making the generator components easier to maintain.

The Typescript api client it generates for Lotus produces 500+ non-fatal errors when compiled with [email protected]

open-rpc/generator#625 to track fixing

The generated api docs fails to build with a gatsby webpack compilation error under npm@6 see: open-rpc/generator#621

open-rpc/generator#626 to track fixing

under npm@7 the deps fail to install.

npm 7 is still pretty fresh, we tend to stick to LTS versions of node, and npm 7 is default starting with nodejs 15. LTS is 14.16.1 - which I have just updated everything to. That being said, thanks for bringing this to my attention. I will get a branch ready for when npm 7 is landed in an LTS release (for all the projects!).

It's difficult to explore the Lotus api definition in the OpenRPC Playground, which should be part of the magic

I'll try to think of some ways to make it more smooth. These are tricky problems though.

Once again, thanks for the review! Let me know if theres any other questions! I'll be working on getting a grant proposal together to move these things along, and I will post back here when the proposal is finished.

@rvagg rvagg merged commit c6fdcfd into master May 10, 2021
@rvagg rvagg deleted the lotus-openrpc branch May 10, 2021 04:04
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.

4 participants