Skip to content
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

Propose: Full Bisq node as a "Black Box" with Restful interface #5

Closed
citkane opened this issue Mar 1, 2018 · 7 comments
Closed

Propose: Full Bisq node as a "Black Box" with Restful interface #5

citkane opened this issue Mar 1, 2018 · 7 comments
Assignees

Comments

@citkane
Copy link

citkane commented Mar 1, 2018

I am proposing a module to provide a full Bisq node as a headless "Black Box" with Rest interface. It is optional from command line to launch a http API interface on localhost and/or the javaFX GUI client.

Code and instructions can be found here: bisq-engine

Packaging logic

enginepom

Application logic

engineapp

Business logic

Presently, the Bisq business logic resides in the dataModels of the gui module. Engine uses interfaces on the io.bisq.gui classpath for the REST service to interact with the logic. Minor refactoring on some dataModel 's is needed to prune out pre-existing GUI logic that would throw errors in headless.

This is done by:

  • copying the whole dataModel class over to bisq-engine on it's existing classpath, thus overriding gui.
  • Implementing the required refactor locally

It is envisaged that this will allow development of bisq-engine to be unhindered, while providing a clear space for diffs and collaboration on building a common, gui agnostic, business logic architecture for the whole stack.

@ManfredKarrer
Copy link
Member

Feel free to add PRs for refactoring of existing dataModels which contain UI elements. Should not be the case anyway, so good to clean those up. I think most can be refactored easily without adding much risk/effort.

@citkane
Copy link
Author

citkane commented Mar 1, 2018

Yes, needs a bit of discussion on the best common approach to messaging, ie. the dataModel clearly has something to say or complain about. ATM this is going to GUI as a pop-up or error. Easy enough to block this from happening in headless, but the API also needs to be informed about these events - so we need a common messaging vehicle.

I am refactoring with something that works (message-wise) for me in the context of the bisq-engine module, but will break the stack if pulled into bisq-exchange. Good enough for now, but needs a considered solution.

@ManfredKarrer
Copy link
Member

I just refactored a bit the Create- and TakeOfferDataModels:

Use common superclass for Create- and TakeOfferDataModel to avoid code duplication:
bisq-network/bisq@a1befee

Move Notification out of DataModel to View:
bisq-network/bisq@a30f9f0

Still more to do but I think it's not hard. Duplicated code can be moved to the superclass and references to view domain should be represented as a property and the view registers a listener to it. The data for display is usually already available for the view so no need to pass over data.

@citkane
Copy link
Author

citkane commented Mar 2, 2018

Much cleaner Manfred. So satisfying to get rid of repetition :)

I am going to set up slack channels for bisq-engine and bisq-front, just getting my head around a few things ATM.

A typical example of what needs co-ordination for compatibility is here: https://github.com/citkane/bisq-engine/blob/978d324e9468f191516fbad277689fedff0d0819/engine/src/main/java/io/bisq/gui/main/offer/takeoffer/TakeOfferDataModel.java#L308

The Args.gui needs to become a global boolean from argument parser in main method so that the app is aware if it is headless or GUI mode (probably BisqEnvironment).

The message' is a very simple class that could live in common` module. It is used not only to pass information back to the API, but also by the API to format return information and pass to a client in JSON.

I have declared it here for now: https://github.com/citkane/bisq-engine/blob/978d324e9468f191516fbad277689fedff0d0819/engine/src/main/java/io/bisq/engine/app/api/ApiData.java#L94

@ManfredKarrer
Copy link
Member

Regarding TakeOfferDataModel.java#L308:
I would refactor it in the way that the data model holds the message and the popup is created in the view class and the view listens to a stringProperty change event to show the popup. The stringProperty is set only in error case so the boolean is expressed there inherently. There are already such case at other places....
The creating of the Popup is in the wrong place it the dataModel so better to fix that instead of using a condition with a flag (if(Args.gui) ...).

@citkane
Copy link
Author

citkane commented Mar 2, 2018

That should do it. Replied to your message on slack. I am at my desk for the next 30mins or so.

@cbeams
Copy link
Member

cbeams commented Apr 6, 2018

Closing as inactive and for the same reasons described in #8. See also #11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants