codex-storage / nim-codex

Decentralized Durability Engine
https://codex.storage
Apache License 2.0
69 stars 25 forks source link

Cleanup API and extend its expressiveness #367

Open AuHau opened 1 year ago

AuHau commented 1 year ago

Going through the API as part of documenting it for OpenAPI and have several suggestions on how to make it cleaner.

  1. Use json for most, if not all, responses to have them unified
    • /connect returns a simple string for all the responses
  2. Expanding expressiveness of our API using a more comprehensive range of HTTP status codes etc
    • /connect could return 404 when a peer is not found
  3. Validate data input and provide proper error messages
    • POST /sales, for example, accepts a normal number instead of the required hex-decimal string for values and creates a Request with all values zeroed. Instead, it should properly error out with status 400 and a description of what is wrong.
    • I would suggest creating a structure for error responses in the form of JSON with properties message and code, where a message would be a user-friendly verbose message and code would be a short string for API consumers to do assertions on similarly like NodeJS defines it: https://nodejs.org/api/errors.html#errorcode
  4. Correct path semantic
    • POST /storage/request/{cid} passes CID parameter of the request as a path section. I would argue that it should be better to use a query parameter or move it directly into the JSON body. Moreover, there is GET /storage/purchases/{id}, which has a similar format, but the ID is a Request ID instead of a CID, which is quite confusing.
  5. Proper and easy-to-use input types
    • I would suggest moving from the hexadecimal string for BigInts to using simply decimal string (for numeric values). It is very cumbersome to work with the API, where I have to convert all the inputs into hex numbers.
    • For some properties, the hex strings are not even needed, like for Storage Ask's proofProbability which should not be big int anyhow if I understand its usage correctly.
markspanbroek commented 1 year ago

Good idea, the REST API could use some love. I'm ok with only supporting json for the POST bodies and the responses. Would require some changes when scripting the API with e.g. curl, but nothing that jq can't handle.

dryajov commented 1 year ago

Great ideas and I agree that it needs a lot of love.

I would follow a similar pattern that the nimbus Eth2 api follows when it comes to the rest framework. An example can be found here - https://github.com/status-im/nimbus-eth2/blob/stable/beacon_chain/rpc/rest_beacon_api.nim.