Add websocket support #92

HenrikJannsen · 2024-12-06T11:54:05Z

Currently implemented are those topics:

MARKET_PRICE (pushes updates for market price data)
NUM_OFFERS (pushes updates for number of offers per market)
OFFERS (pushes updates of offers)

We use the same patter as in the 'Easybind' observer library we use in JavaFx where you get the existing data delivered when subscribing the observer. That is very handy as otherwise one need to do the extra call (as needed in default JavaFx observers). So when we subscribe to a topic we get returned the available data as response to our subscribe request.

For MARKET_PRICE and NUM_OFFERS we get the full map of quotes or numOffers per market code. It would be wasteful to do a new subscribe and unsubscribe every time the user changes the currency. So we keep the data in a map and take out the actual number depending on the selected market.

For OFFERS we dont want to get the full data set at each change as that could be a significant data size.
Instead we use a ModificationType (REPLACE, ADDED, REMOVED) to signal if data was added, removed or if we want to replace the whole data set. The default for the MARKET_PRICE and NUM_OFFERS is REPLACE. For OFFERS we support ADDED/REMOVED to insert items or remove them.
We can send a parameter for subscribing to a topic like currency code, in which case we only would get updates for that currency. If the parameter is not set we get all data. We also handle here the full map of offers per currency, as it would be wasteful to subscribe/unsubscribe at each change of markets.

The payload we get is deserialized to the expected data type at the consumer who knows what data to expect. The generic code does not deserialize that data due lack of type info. Maybe there are some magic hacks to get that resolved as we store the type info in the topic, but I did not get that working so far.

So adding new topics is just to add the topic with its expected type and the consumer defines what type to expect.
We will see with the next user cases if that covers all we need.

The events are emitted on a single thread executor on the server per subscriber, thus guaranteeing the order, but additionally we use a sequence number to check if the later received events are new as the one we got already.

Rest API calls over Websockets:
To avoid that we need to create new Tor circuits at each rest api call, we use the existing websocket connection to route those http messages to the server and there it will get unpacked to call the local rest api endpoints.
So far most rest api calls have been anyway replaced by the websocket subscribe calls. Only the getMarkets is a GET request to the rest api endpoint (though that could be omitted as the markets are a rather static list and only change if we add a market to the price node).
With POST calls there is still a problem (likely on the server side), so we use the normal http client and not the websocket client for that.

For testing one need to run the HttpApiApp (former we used the RestApiApp, this was renamed to HttpApiApp and covers both RestApi and WebsocketApi).

rodvar

@HenrikJannsen great PR overall Henrik 💪 .

I've added these comments on a first pass purely doing code review, I'm gonna do some testing ASAP and put my feedback as well.

This is a foundational PR so we might be a bit more strict on this one I hope you don't hate me for it haha :)

shared/domain/src/commonMain/kotlin/network/bisq/mobile/client/di/ClientModule.kt

shared/domain/src/commonMain/kotlin/network/bisq/mobile/client/market/PriceFormatter.kt

.../commonMain/kotlin/network/bisq/mobile/client/user_profile/ClientUserProfileServiceFacade.kt

.../domain/src/commonMain/kotlin/network/bisq/mobile/client/websocket/RequestResponseHandler.kt

.../src/commonMain/kotlin/network/bisq/mobile/client/websocket/subscription/ModificationType.kt

shared/domain/src/iosMain/kotlin/network/bisq/mobile/domain/di/ClientModule.ios.kt

shared/presentation/src/commonMain/kotlin/network/bisq/mobile/client/ClientMainPresenter.kt

...src/commonMain/kotlin/network/bisq/mobile/presentation/ui/uicases/GettingStartedPresenter.kt

rodvar · 2024-12-09T01:04:35Z

@HenrikJannsen gonna need some help testing this one, I'll reach out on M

HenrikJannsen · 2024-12-09T06:35:34Z

@HenrikJannsen gonna need some help testing this one, I'll reach out on M

Added a comment at the initial post about the renamed HttpApiApp in Bisq2.

.../kotlin/network/bisq/mobile/android/node/domain/user_profile/NodeUserProfileServiceFacade.kt

rodvar · 2024-12-10T05:17:59Z

rebased to main updates

rodvar · 2024-12-10T05:27:59Z

android node is working as expected, testing WS clients now

rodvar · 2024-12-10T22:13:45Z

@HenrikJannsen ok awesome, It works on the 3 apps. I'll add some docs on how to setup the env properly so that its easier for everyone to contribute.

There are some things we need to discuss, it looks like each running API instance only supports one client only? This causes issues when using the same API instance with a different device, the app assumes its the other user but obviously the cathash has not been generated in that device and then it won't load properly. We need to think how to solve this and allow support for multiple clients per trusted node ? If this is too complex we can support one for the MVP but soon after work on extending it.

Gonna do the last code review and add docs and merge this one in 💪

…king values into gradle.properties

rodvar

Merging as is, please have a look at the comments to discuss/consider for next PR! :)

The main thing to deal with is support for multiple clients per trusted node.
If its fesiable to do for MVP then great, otherwise we just need to refuse connections from any device that is not the one that generated the profile (and therefore the cathash) for security purposes right?

I've added some commits with docs, fix to the crash when it can't connect, some logs and the posibility to configure default networking from gradle.properties.

I tested it though in my LAN but it won't work, let's chat about it when you are avail

Awesome work! 💪

rodvar · 2024-12-10T22:15:41Z

...de/src/androidMain/kotlin/network/bisq/mobile/android/node/presentation/NodeMainPresenter.kt

+            }
+        }.onFailure { e ->
+            // TODO give user feedback (we could have a general error screen covering usual
+            //  issues like connection issues and potential solutions)


rodvar · 2024-12-10T23:16:10Z

...mmonMain/kotlin/network/bisq/mobile/client/offerbook/offer/ClientOfferbookListItemService.kt

@@ -24,48 +28,89 @@ class ClientOfferbookListItemService(private val apiGateway: OfferbookApiGateway

    // Misc
    private var job: Job? = null
-    private var polling = Polling(10000) { updateOffers() }
    private var selectedMarket: MarketListItem? = null


This "selectedMarket" approach on the markets service limit the market information accessibility for all the software components when any of them selects a market. This is probably not something we want as different components might have different needs to cater for different features.

This is why the architecture suggests to use repositories for holding / saving the data as a separate entity from the services.

Its certainly ok for MVP though just leaving this comment as food-for-thought

We just use the selectedMarket as trigger to select the market price. If we dont want to be triggered by offer market selection but like in bisq2 with a dropdown at the market price, we just change that.

rodvar · 2024-12-10T23:18:59Z

.../src/commonMain/kotlin/network/bisq/mobile/client/replicated_model/common/currency/Market.kt

@@ -19,7 +19,7 @@ package network.bisq.mobile.client.replicated_model.common.currency
 import kotlinx.serialization.Serializable

 @Serializable
-class Market(
+data class Market(


should probably be a BaseModel as most of the models introduced in this PR. Leaving as is for now till the need arises.

shared/presentation/src/commonMain/kotlin/network/bisq/mobile/client/ClientMainPresenter.kt

rodvar · 2024-12-10T23:35:37Z

.../domain/src/commonMain/kotlin/network/bisq/mobile/client/websocket/RequestResponseHandler.kt

+            throw e
+        }
+
+        return withTimeout(timeoutMillis) {


I'll add a log for now and we can improve in further PRs

rodvar · 2024-12-10T23:36:02Z

.../domain/src/commonMain/kotlin/network/bisq/mobile/client/websocket/RequestResponseHandler.kt

+    suspend fun dispose() {
+        log.i { "Disposing request handler for ID: $requestId" }
+        mutex.withLock {
+            deferredWebSocketResponse?.cancel()


leaving this for further PRs

rodvar · 2024-12-10T23:36:58Z

shared/domain/src/commonMain/kotlin/network/bisq/mobile/client/websocket/WebSocketClient.kt

+import network.bisq.mobile.utils.Logging
+import network.bisq.mobile.utils.createUuid
+
+class WebSocketClient(


leaving this for further PR dev

rodvar · 2024-12-10T23:37:35Z

shared/domain/src/commonMain/kotlin/network/bisq/mobile/client/websocket/WebSocketClient.kt

+    }
+
+    suspend fun unSubscribe(topic: Topic, requestId: String) {
+        //todo


leaving this for next PR

...monMain/kotlin/network/bisq/mobile/client/websocket/rest_api_proxy/WebSocketRestApiClient.kt

…pect that class naming

HenrikJannsen force-pushed the add-websocket-support branch 2 times, most recently from b2d340a to 456aeda Compare December 7, 2024 00:32

rodvar requested changes Dec 7, 2024

View reviewed changes

rodvar reviewed Dec 9, 2024

View reviewed changes

.../kotlin/network/bisq/mobile/android/node/domain/user_profile/NodeUserProfileServiceFacade.kt Show resolved Hide resolved

HenrikJannsen added 4 commits December 10, 2024 16:16

Add websocket support

588caf9

Use backgroundScope instead of CoroutineScope(BackgroundDispatcher)

ee39ed6

Remove comment

b1280f0

Make class thread safe.

696735d

rodvar force-pushed the add-websocket-support branch from e4e3d53 to ecade78 Compare December 10, 2024 05:17

Use return triple instead of callback

d25e4e3

rodvar force-pushed the add-websocket-support branch from ecade78 to d25e4e3 Compare December 10, 2024 05:23

- fix crash on service init in iOS (coming from main)

e21cf38

rodvar added 2 commits December 11, 2024 09:55

- move BuildConfig generation to domain module + move default networ…

0e688da

…king values into gradle.properties

- added networking setup to README docs for local dev

0d65a0f

rodvar force-pushed the add-websocket-support branch from 1c1ea72 to 0d65a0f Compare December 10, 2024 23:04

- fix crash when connect fails

e1a24d3

rodvar force-pushed the add-websocket-support branch from e627f7d to e1a24d3 Compare December 10, 2024 23:29

rodvar added 2 commits December 11, 2024 10:35

- log timeouts on ws connection request handler

14a736c

- refactor: renaming to remove REST prefixes

d7e3a7e

rodvar approved these changes Dec 11, 2024

View reviewed changes

- rollback websocket api request/response naming cause the server ex…

6c6154a

…pect that class naming

rodvar force-pushed the add-websocket-support branch from 4fe88d2 to 6c6154a Compare December 11, 2024 02:46

rodvar merged commit c643e26 into bisq-network:main Dec 11, 2024
1 check passed

rodvar mentioned this pull request Dec 11, 2024

EnvironmentController implementation #106

Merged

HenrikJannsen deleted the add-websocket-support branch December 12, 2024 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add websocket support #92

Add websocket support #92

HenrikJannsen commented Dec 6, 2024 •

edited

Loading

rodvar left a comment

rodvar commented Dec 9, 2024

HenrikJannsen commented Dec 9, 2024

rodvar commented Dec 10, 2024

rodvar commented Dec 10, 2024

rodvar commented Dec 10, 2024

rodvar left a comment

rodvar Dec 10, 2024

rodvar Dec 10, 2024

HenrikJannsen Dec 11, 2024 •

edited

Loading

rodvar Dec 10, 2024

rodvar Dec 10, 2024

rodvar Dec 10, 2024

rodvar Dec 10, 2024

rodvar Dec 10, 2024

Add websocket support #92

Add websocket support #92

Conversation

HenrikJannsen commented Dec 6, 2024 • edited Loading

rodvar left a comment

Choose a reason for hiding this comment

rodvar commented Dec 9, 2024

HenrikJannsen commented Dec 9, 2024

rodvar commented Dec 10, 2024

rodvar commented Dec 10, 2024

rodvar commented Dec 10, 2024

rodvar left a comment

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

HenrikJannsen Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

rodvar Dec 10, 2024

Choose a reason for hiding this comment

HenrikJannsen commented Dec 6, 2024 •

edited

Loading

HenrikJannsen Dec 11, 2024 •

edited

Loading