AcalaNetwork / chopsticks

Create parallel reality of your Substrate network.
Apache License 2.0
131 stars 79 forks source link

Chopsticks client is not compatible with Papi #801

Closed Ad96el closed 19 hours ago

Ad96el commented 1 month ago

Description

When attempting to create a client instance in PAPI, Chopsticks throws the following error:

ERROR (ws): Invalid request: {"jsonrpc":"2.0","id":"1-1","method":"chainHead_v1_follow","params":[true]}
app: "chopsticks"

After some investigation, it appears that the desired RPC call is not available after forking.

Steps to Reproduce

  1. Connect to any network using Polkadot.js and verify if the chainHead_v1_follow method is available under Developer > RPC calls > rpc > methods.
  2. Fork the network.
  3. Check if the method is still listed. It should no longer be available.
ermalkaleci commented 1 month ago

Chopsticks is a client itself and it and has its own methods, this one is not implemented

xlc commented 1 month ago

yeah we need to implement those new methods

Ad96el commented 1 month ago

@xlc, thanks for the response. Could you estimate which version will include the fix?

xlc commented 1 month ago

Unfortunately, I can't provide an estimate as we don't have extra resources atm However happy to mentor and review if anyone is interested working on this

rflechtner commented 1 month ago

There are ways for papi to work with clients that do not yet implement these rpc methods (https://papi.how/requirements#polkadot-sdk-110--x--1110) - as long as the client at least acts as a client on polkadot api > v1.1.0 would. We're still running into issues with this method though, which strangely results in the following error message from the chopsticks client:

Invalid request: {"jsonrpc":"2.0","id":"__INTERNAL_ID","method":"rpc_methods","params":[]}

Any idea what the issue may be here? May it be that the id coming from papi is throwing it off?

ermalkaleci commented 1 month ago

id needs to be number

voliva commented 1 month ago

Mhh I see we have to document this better, but Polkadot-API is built on top of the new Polkadot JSON-RPC spec, and requires chainHead, rpc, chainSpec and transaction group of functions to be implemented in the node.

The compatibility layer from polkadot-api/polkadot-sdk-compat fixes some implementation bugs of this spec that were present in older versions of Polkadot-SDK, and that we can easily fix on the client-side. But the RPC methods defined in the new specification must be there.

Another issue might be this:

id needs to be number

However, the JSON-RPC spec (the global one) specifies that the id parameter of the request object must be an integer, a string or null: https://www.jsonrpc.org/specification#request_object - And it is properly handled by polkadot-sdk nodes too.

All of the requests coming from PAPI use string id, in the shape of {interger}-{integer} (because for some use cases it needs to multiplex across different chains, so the first integer represents the chainId, and the second the requestId)

josepot commented 1 month ago

However happy to mentor and review if anyone is interested working on this

We would be super happy to collaborate on this. We can assign some resources for the upcoming OpenGov proposal to tackle this. Meaning that I would be super happy to be mentored and contribute 🙂

id needs to be number

As @voliva pointed out this is bad, as it makes the API not JSON-RPC compliant. In other words: this API seems to be tightly coupled to an implementation detail of PJS... which kinda defeats the point of having a JSON-RPC interface.

xlc commented 1 month ago

Here are some pointers for people wanting to contribute:

To update JSON RPC id type schema: https://github.com/AcalaNetwork/chopsticks/blob/7c4a7a92a4ea774ae10f1a2b1349063f760990e0/packages/chopsticks/src/server.ts#L12

To implement new RPC methods:

Add a new file if needed (e.g. chainHead.ts): https://github.com/AcalaNetwork/chopsticks/tree/master/packages/core/src/rpc/substrate

and have it added to the handlers: https://github.com/AcalaNetwork/chopsticks/blob/7c4a7a92a4ea774ae10f1a2b1349063f760990e0/packages/core/src/rpc/substrate/index.ts

Implement the handler methods. Use existing methods as an example. e.g. https://github.com/AcalaNetwork/chopsticks/blob/7c4a7a92a4ea774ae10f1a2b1349063f760990e0/packages/core/src/rpc/substrate/chain.ts#L83

and that's pretty much it.

The JSON RPC spec should contain all the info about what's needed: https://paritytech.github.io/json-rpc-interface-spec/

However, I will suggest just use papi to connect to Chopsticks, and implement whatever missing RPC methods it complaints until it runs to get an MVP version out.

The API docs for Chopsticks core types could be helpful: https://acalanetwork.github.io/chopsticks/docs/core/README.html

In theory, the existing core types should be enough to implement all the RPC handlers but let me know if that's not the case.

xlc commented 18 hours ago

Making a new release with papi support https://github.com/AcalaNetwork/chopsticks/releases/tag/0.15.0