Refactor API Design (API v1)

[redphix]

Status: Draft

This issue aims to document and draft a new refactored API design for Robosats (API v1) along with laying down TODOs for accomplishing the refactor

Routes Overview

• /robot
  • POST /robot - Get me a new robot (conventional /login route)
  • GET /robot - Get your robot info (conventional /profile route)
  • PATCH /robot/preferences - Update your preferences (things like stealth invoices, tg enabled/disable)
  • POST /robot/withdraw-reward - Claim/Withdraw rewards
  • POST /robot/presence - Send online status
• /book
  • GET /book
• /exchange
  • GET /exchange/info
  • GET /exchange/history
  • GET /exchange/limits
• /market
  • GET /market/prices
  • GET /market/ticks
• /order
  • POST /order - Create a new order
  • GET /order/:id
  • POST /order/:id/take
  • POST /order/:id/pause
  • POST /order/:id/invoice
  • POST /order/:id/address
  • POST /order/:id/cancel
  • POST /order/:id/confirm
  • POST /order/:id/dispute

Notes - Routes

• Having an hierarchy as described above, we can also get a nice hierarchy in our codebase. Each of /market, /order, /exchange, /book and /robot can be a django ViewSet and under each viewset we can have separate views in separate .py files to organize the code better.
• If you create an order you are implicitly a maker, so don't really required to mention it as /make or /order/make, POST /order conveys the intention of "making a new order"
• skip submit_statement action and add as part of /dispute route itself

Things to improve from old design

1. Define each route to return only one type of response type (i.e the response model should be deterministic). In other words each route's json response body's keys must always be deterministic, no matter the input. Try using nested model/serializers if it's not possible to fit all types of response in a single model. At most, as a last resort, you may define another response model that may be returned.

2. Use arrays where ever sequential data is involved as the order matters.
   Currently, the /historical route returns a response like this:

   {
       "2022-02-22 00:00:00+00:00": {
           "volume": 0.001,
           "num_contracts": 3
       },
       "2022-02-23 00:00:00+00:00": {
           "volume": 0.004,
           "num_contracts": 2
       },
       "2022-02-24 00:00:00+00:00": {
           "volume": 0.003,
           "num_contracts": 3
       },
   }
   

   A better alternative would be to change it to be contained in an array, since json keys are not ordered, and should not be relied as an order method.

   [
     {
       "timestamp": "2022-02-22 00:00:00+00:00",
       "volume": 0.001,
       "num_contracts": 3
     },
     {
       "timestamp": "2022-02-23 00:00:00+00:00",
       "volume": 0.004,
       "num_contracts": 2
     },
     {
       "timestamp": "2022-02-24 00:00:00+00:00",
       "volume": 0.003,
       "num_contracts": 3
     },
   ]
   

3. Do not use dynamic json keys. Do not use keys that are either dynamic (like the /historic route having timestamp as a key) or are produced at runtime.Again, the reason being keys/fields in the response need to be deterministic
   The /limits route currently returns:

   {
     "1": {
       "code": "USD",
       "price": 19534.495,
       "min_amount": 3.906899,
       "max_amount": 781.3797999999999,
       "max_bondless_amount": 9.7672475
     },
     "2": {
       "code": "EUR",
       "price": 19922.42,
       "min_amount": 3.9844839999999997,
       "max_amount": 796.8968,
       "max_bondless_amount": 9.96121
     },
   }
   

   Better to return an array like so:

   [
     {
       "currency_code": "1",
       "currency_symbol": "USD",
       "price": 19534.495,
       "min_amount": 3.906899,
       "max_amount": 781.3797999999999,
       "max_bondless_amount": 9.7672475
     },
     {
       "currency_code": "2",
       "currency_symbol": "EUR",
       "price": 19922.42,
       "min_amount": 3.9844839999999997,
       "max_amount": 796.8968,
       "max_bondless_amount": 9.96121
     },
   ]
   

4. Currently, nick, nickname and username refer to the same thing - The robot name. Need to keep the naming of this consistent.

5. Keep scalar/percentage representation consistent:
   Currently, fields representing percentages are either a scalar like 0.14 (14%) or percentages like 50 (50%). Need to follow one convention. Either a scalar (0-1) or a percentage (0-100)

6. Provide a route to get all the available currencies (avilable here) (and, not sure if possible, or even required, also add a route to list predefined payment method strings?)

General Notes

• I know the term RESTful is thrown around a lot but at the end of the day, it's just a convention and design pattern. In my experience there's not been a need to comply with REST 100% all the time. However, I try to make sure I take care of these things:
  • Try to represent things as "Resources" and try to have an hierarchy e.g /order/234/take, or /market/prices etc. Not necessary to follow 100%. Only when you have too many resources/models in your system, do you start seeing the benefits
  • Use GET only for stateless things; i.e hitting the route doesn't change any state. Just for listing/viewing things. If it changes the state (in db of a model/resource) then make it a POST route.
  • It's generally okay to mix a bit of REST with a bit of RPC these days. These are just conventions and design patterns. Sometimes going full REST doesn't bode well and neither does going full RPC. I like to mix and match both. Following Rest's Resource and hierarchy we get cleaner, more organized docs and a codebase as well, where each route is responsible for once type of functionality. In an RPC this might get confusing very fast, especially when one single route does many things.

[Reckless-Satoshi]

Hey @redphix

Thanks a lot for the time to review the current state of the backend and hit back with such a concise plan for enhancement. The backend/API has certainly gotten very little love since the first two weeks of prototype development.

It is in fact a good moment to start sticking to other best practices (write unit tests!) and solve some of the base core issues we have been dragging since the prototype.

• Use GET only for stateless things; i.e hitting the route doesn't change any state.

Indeed, GET /order induces some database change at the moment. For example, if the user checks an order that is waiting for a bond, the GET request will fire an RPC call to LND to check whether the HTLC is ACCEPTED. If it is accepted, it will change the status of the Order, the expiration time, etc. In order to solve this, we first need to write a new module (or extend some of the current ones) that opens threads to watch for state on every single active order (probably a Celery task is good for this?). There is a precarious backend thread that also takes cares of some of these, but we need to be more consistent and methodical.

Things to improve

The 5 points on things to improve are fantastic. I would add some more to the list that probably will require a much more profound rework but will greatly enhance reliability. These are not strictly related to how the API behaves for the user, but how it operates internally:

• Logic is written into the models and none of the logic takes place in the Views. For example, robot avatar creation is fully written in its view at the moment. By moving the logic into the models we ease writing unit tests, increase overall reliability and the codebase will be easier to understand for future django devs.
• To ease PR reviews and keep the project manageable, we probably want to refactor the API source code into modules e.g. api/models.py into a module /api/models/__init__.py where there is order.py ln_payment.py robot.py (currently Profile model), etc. We also might want to do the same with the views api/views.py into api/views/<module>.
• Unit tests everywhere! This one is a must. We have a GH workflow that runs the unit tests before releases, yet not a single unit test written.

Risks and mitigation

This API enhancement carries the risk of introducing new bugs and breaking things that do work Okaish currently. The current API, while unconventional and quirky, is very battle tested. I would favor splitting the work into several tasks and test, merge and put into production as soon as each tasks gets finished. It makes sense that each task takes care of a route. It also makes sense to start small and easy with the routes that are not mission critical /market /exchange /book, then go for robot and finally order.

The opposite approach would be to fully refactor the API first, then test and launch into production. However, many things could be broken at once (no matter how much you test, you cannot do 10K orders by yourself, bugs will come up on production). So I favor merging as we go and slowly evolve the v0 API until it becomes the full flesh v1. For every backwards incompatible API upgrade we merge, we increase the minor version by one, this will automatically fire the dialog for clients to upgrade.

Getting it done

How do you see this refactor split into tasks?

We can create a draft of the tasks that need to get done. As I said, a task per route makes sense, probably /order can be split into a few. Some task might have to focus on creating/improving the backend threads that watch order status. Once we have a rough draft of all the tasks, we can create dedicated issues, assign some development reward to each and put together a project panel in Github (RoboAPI v1) to keep track of it all.

[redphix]

Agree with the additional three points you mentioned. I really like the idea of baking the logics into the model. I see all these points being a good amount of work in themselves and being split into individual tasks, fully agree. The more I look into the work required, the more I feel it's going to to take multiple organized tasks to accomplish :sweat_smile:

RIsks and mitigation

Biggest point I was concerned about as well! I initially thought we could deploying the api side by side (i.e one would be v1 and the other v0) and then swap the frontend client at one point, but this would require thorough testing as you mention, and that's not very easy, I agree. Unit tests can't catch all the problems that will arise. A stacked approach of changes sounds smarter in this case.

For every backwards incompatible API upgrade we merge, we increase the minor version by one, this will automatically fire the dialog for clients to upgrade.

But the minor versions are for backwards compatible changes only. Although, I feel like it's okay here to do so and introduce a backwards incompatible and break the convention since it's beta anyways? The popup to update is cool, I am assuming it's for Umbrel users?

Once we have a rough draft of all the tasks, we can create dedicated issues, assign some development reward to each and put together a project panel in Github (RoboAPI v1) to keep track of it all.

Sounds like a solid plan! Will get to drafting those issues and reading a bit more of the code to see if I am missing something.

[Reckless-Satoshi]

But the minor versions are for backwards compatible changes only. Although, I feel like it's okay here to do so and introduce a backwards incompatible and break the convention since it's beta anyways? The popup to update is cool, I am assuming it's for Umbrel users?

I am not very versed into Semantic Versioning. But I had the understanding that while major version is 0 (i.e, beta or under development), minor versions can introduce backward incompatible changes. In any case, that won't make any difference ^^

The update Dialog fires in any frontend and let's the user know how to update (install the new node app, download the new Android APK from releases, or force refresh Tor Browser)

Indeed, getting to API v1 is a lot of work, but it sounds fun to be honest. There is not time pressure and we can allocate at least a total of ~10M Sats in rewards for it. I would appreciate if someone with experience (maybe you!?) wants to lead the overall project of building API v1 and create tasks and rewards we can distribute (I will be doing tasks of course!). ... leading the project itself, of course, deserves rewards too :smile:

[redphix]

but it sounds fun to be honest.

Definitely :yum:

I would appreciate if someone with experience (maybe you!?) wants to lead the overall project of building API v1

Would love to!

Will organize and create tasks soon!

Was curious, can you give permissions to other users to be able to create new projects/boards (not too familiar with the project features of GH) or is that something possible only for organizations?

[Reckless-Satoshi]

That's really great to hear! :rocket: :

... or is that something possible only for organizations?

Projects get created on your account, then linked to the repository. So I think it probably needs to be an organization who create the project. Time to create RoboSats organization and move this repo? :smile:

[redphix]

Time to create RoboSats organization and move this repo? :smile:

Yup! :) Was gonna give you the suggestion earlier as well, but slipped my mind.