[bisq-network/proposals] Integrate HTTP API to bisq-network (#22)

Fri Jun 1 12:28:27 UTC 2018

@cbeams: 
Thanks for your feedback!

Regarding proposal:
No I was not aware of the conversations you mentioned and kept myself intentionally rather out of the API topic. 

I started on that work and proposal as I felt that @blabno was stuck in getting the current code base connected with the recent code base and how to integrate the startup process in a way that the headless API version, the Desktop+ API and the Desktop-only version can work. 

I agree to all your points and my intention was not to write that kind of proposal but only to deliver a refactoring of the startup process (MainViewModel mostly) so that they are not blocked anymore to work on top of the current Bisq master.

When starting coding I got a bit further and added support for a simple dummy gRPC server/client and a BisqDaemon (headless full features Bisq - not implemented much there though). My plan was actually to make a small simple user case with gRPC like getting the real Bisq balance via gRPC calls and if that model works well the same should be possible for the HTTP-API stuff. 
I was not planning to integrate the current HTTP-API as that comes with quite a bit of complexity and probably needs some changes. I just wanted to understand the requirements they have so I can deliver a structure which works for them (thats why I copied over their code in my http-api project to have the code at hand and see better the requirements, but that will get removed later and I leave it to @blabno to integrate the current API code to that project).

Regarding how to start the HTTP-API:
Yes my plan was to pass a option key which enables the HTTP-API. The HTTP-API will come with its own set of new options (e.g. for security) but that I leave to @blabno to integrate.
>From a build/deployment point of view I was considering to have the combined (Desktop+API) modes without any change to Desktop from a build perspective (beside adding the dependencies), so the Bisq app will be API-enabled if the option is set. For the headless version I would add a new application (is setup in my projects in gradle) inside the new API projects. 

Regarding headless versions:
The effort is not terrible high. It is mainly the MainViewModel (where I did a test refectoring a while agoe in 2 hours to get it headless, though that was very quick and dirty). I estimate 1-2 days to do it properly. And it is work need to be done anyway some day as the MainViewModel is terribly overloaded/polluted.

The other areas where they have atm duplicated code from the UI ViewModels to avoid the dependency to JavaFX I would not touch but it would be for me one "exit criteria of incubation" to get rid of that duplicated code and have properly refactored all those areas to core classes. That should be mostly rather trivial technical refactoring, expect the create and take offer parts which might be a bit more complex. But I think all in all it is no terrible much effort and I estimate that can be done in about 1 week.

Regarding the option key:
Yes all passed options to the app should work as options set in bisq.properties as well. I assume that is the case how they are implemented atm, but I have not tested it. Though the current code expects the bisq.properties file in the root directory of the data directory not in the .config subfolder.
And yes I agree it should be same like in Bitcoin Core. But as we already support the bisq.properties and maybe some users are using that, we have to be careful how to change that. The base currency (BTC, LTC,...) for instance was written in that property file. Supporting both is confusing as well... Maybe we keep that discussion to an own proposal/issue?

Regarding integration:
I would have seen it as a separate project as well as a gRPC project (which might get added later once someone works on its implementation, but the basics I wanted to provide as well). It's helpful for me to prove that the design works well with different APIs and it is not intended to get in Bisq master before it's implemented (and we have a dev who takes it as its project).

Regarding design:
My approach was to have BisqDaemon which lives in core. BisqDaemon does all the bootstrapping and delivers a fully initialized Bisq app.  
You could also wrap that in a main class and start that as headless application, but it does not have any outside interface. One of the APIs (HTTP, gRPC) would be next to the Desktop an interface and we pass the BisqDaemon to those so they get a fully functional Bisq app.

The HTTP-API could either use BisqDaemon directly or startup gRPC and use that to access Bisq. I assume using BisqDaemon directly is a preferred approach but both are possible. But the setup for the gRPC is not really much and would be a valid alternative. But the main work will be to define all protobuffer data and as long that is not implemented in gRPC it would be not feasible. 

The Desktop creates the BisqDaemon and in case the HTTP-API is enabled via options it passes it to the HTTP-API as well as to the JavaFX Application so both operate on the same process and instance of BisqDaemon.

If BisqDaemon also provides a high level API for all the typically required access methods (I would orient on the current HTTP-API feature set like in BisqProxy) is an open question. I think I prefer to use the existing APIs in the domains (e.g. OpenOfferManager,...) as otherwise there is a huge class which flattens all domains into one place. It would be a Facade to make access to the more complex and distributed APIs in the specific domains easier but I am not sure if that is needed and if it would just introduce more maintenance costs, etc.... But I have not a 100% clear/strong opinion regarding that so far.

Regarding:
https://github.com/bisq-network/bisq-desktop/pull/1535#pullrequestreview-118781492
- I use "bisq-http-api" as project name
- I use gradle in my repo (I started to convert the dependencies of the API project to gradle, tough that will require more work missing the excluded, some issue with some test framework libs,... - but basically I could compile all after removing problematic classes and not including the tests from the API project)
- Regarding API docs: I agree

Conclusion:
I am fine with any plan to go on, either to wait until a proper high level proposal is there or to continue in a WIP style to enable faster and easier integration.

My plan was not to integrate that in Bisq as long the code base is not fully reviewed, accepted and meet our acceptance requirements. 

My suggested solution would be that I continue to deliver a base structure where they can start to integrate it with the current Bisq master in my repository. They can fork from my repo and do the required work to make the HTPTP-API work in that design. That way we can bring the API work in sync with master and get rid of that block regarding the startup process and the hacks how to start Bisq with the API in combined and headless mode.
I fear the longer it takes that they continue with the current out of sync project the harder it will become later to merge, specially once the DAO gets into master. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/proposals/issues/22#issuecomment-393865416
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20180601/aca0fb71/attachment-0002.html>