Redesign abstraction around /ob/exchangerate to support Fiat and Crypto conversions

[placer14]

Proposal by @rmisio:

In my mind the exchange rate api is _mostly_ good as is. By _mostly_, I mean if it’s using the OB ticker, it’s good. Because you just have to make one call and from that you could extrapolate everything. So, the desktop client just calls `/ob/exchangerate/btc` and that includes all the exchange rates between BTC and fiat and also, because it’s infused with CMI, it has the rates between BTC and the other wallet curs.

With that, you could go between any currency conversions we need, whether it’s fiat <=> fiat, btc <=> fiat, non-btc-wallet-cur <=> fiat or non-btc-wallet-cur <=> non-btc-wallet-cur.

So, what’s in place now in my mind is good *when the OB ticker is used* (which should be 95%+ of the time, I would think - remember crypto listing curs also depends on the CMI data being infused).

Where it falls apart is if the OB ticker isn’t used and it goes to some fallback api, because then `/ob/exchangerate/btc` would only include the rates for btc <=> fiat and not btc <=> other wallet curs. So, in that scenario, any purchases for non-btc would be disabled on the client.

In my mind, this whole situation could be fixed if the server would just ensure the exchange rate api is returning, in addition to fiat, the rates between the requested wallet cur and other wallet curs, which could be as simple as checking if they are there (default ticker must be being used - so no additional action is necessary), otherwise call other APIs to ensure the data is there. Essentially, the server aggregate the data, if necessary. (edited)

Tasks:

• [ ] validate the desired logic is feasible
• [ ] define success/failure responses
• [ ] identify changes required in the tickerproxy

Current price conversion logic:

[drwasho]

I agree with @rmisio that the API is good enough in that it serves its purpose in allowing us to calculate prices based on one API call (/ob/exchangerate/btc). That said, it imposes a lot of work on client developers to reconstruct the logic as per the diagram @placer14 included in this issue. This diagram took me about half a day to create and a lot of yelling at my computer, let alone the frustration it must cost to adhere to the logic in client code.

This goes back to the architectural philosophy of the server, which we have never formally stated or wrote down anywhere (but maybe we should), that should endeavour to facilitate the construction of 'thin clients'. What I mean by that is that generally we should make things as easy as possible for client developers, and abstract away complexity for the server. More specifically for this example, client developers shouldn't need to reimplement complex price conversions; the server should handle it.

I believe this could be resolved for API calls such as GET /ob/sales and GET /ob/purchases, as well as order-related notifications). For this issue, I want to focus on the current response of GET /ob/sales and GET /ob/purchases:

{
    "queryCount": 1,
    "sales": [
        {
            "buyerHandle": "",
            "buyerId": "QmPNU7ZD2tU6F6aJF5h4sT13QdJncKyUFHrXidvBzpwdbm",
            "coinType": "",
            "moderated": false,
            "orderId": "QmNc9GyyqvS4qjLhgqci5TH32vdmipDhT8uB6yHznHpuT5",
            "paymentCoin": "ZEC",
            "read": false,
            "shippingAddress": "1060 W Addison ",
            "shippingName": "Elwood Blues",
            "slug": "vintage-dress-physical-w-options",
            "state": "AWAITING_PAYMENT",
            "thumbnail": "QmbjyAxYee4y3443kAMLcmRVwggZsRDKiyXnXus1qdJJWz",
            "timestamp": "2018-10-10T07:15:26Z",
            "title": "Vintage dress (physical; w/ options)",
            "total": 804474,
            "unreadChatMessages": 0
        }
    ]
}

To make things easier, I suggest:

1. We refactor the response to make it consistent to how we're presenting the price in notifications by creating a price object. This would represent the price we paid in crypto to purchase the item.
2. We introduce a second object called settingsPrice (or localCurrencyPrice, or something more elegant) that displays the price of the item at the current exchange rate against the currency selected in Settings (localCurrency).

The price object would be unchanging unlike the settingsPrice that would change based on what the exchange rate or currency settings was at the time the API was hit.

An example of how it might look:

{
    "queryCount": 1,
    "sales": [{
        "buyerHandle": "",
        "buyerId": "QmPNU7ZD2tU6F6aJF5h4sT13QdJncKyUFHrXidvBzpwdbm",
        "coinType": "",
        "moderated": false,
        "orderId": "QmNc9GyyqvS4qjLhgqci5TH32vdmipDhT8uB6yHznHpuT5",
        "read": false,
        "shippingAddress": "1060 W Addison ",
        "shippingName": "Elwood Blues",
        "slug": "vintage-dress-physical-w-options",
        "state": "AWAITING_PAYMENT",
        "thumbnail": "QmbjyAxYee4y3443kAMLcmRVwggZsRDKiyXnXus1qdJJWz",
        "timestamp": "2018-10-10T07:15:26Z",
        "title": "Vintage dress (physical; w/ options)",
        "unreadChatMessages": 0,
        "price": {
            "amount": 804474,
            "currencyCode": "ZEC",
            "priceModifier": 0.0,
            "coinDivisibility": 100000000
        },
        "settingsPrice": {
            "amount": 89.68,
            "currencyCode": "USD",
            "priceModifier": 0.0,
            "coinDivisibility": 100
        }
    }]
}

This makes it super easy for client developers to display the correct value of the order without needing to do any price conversion calculations, which are abstracted to the server.

[cpacia]

I think Rob's suggestion that the server aggregate the exchange rates into one API is fine. We'll still need to leave the exchange rate part of each wallet implementation though. The server just needs aggregating logic.

[rmisio]

We introduce a second object called settingsPrice (or localCurrencyPrice, or something more elegant) that displays the price of the item at the current exchange rate against the currency selected in Settings (localCurrency).

I guess that's all a matter of how you architect your client. For the desktop client, we wouldn't use something like that because we have the settings in memory and we also have the exchange rate in memory (which we are updating regularly), so we wouldn't even use something like this.

With that said, I guess I wouldn't have some huge objection to those suggestions as long as they were additive, so the desktop client can continue to use it's current approach.

I haven't read that logic diagram super intensively and it's very possible our flow is also like that, but it really doesn't seem that bad because we really decouple our data fetches and make them as efficient as possible. For example, settings are only fetched on start-up, not each time they're needed. The exchange rate is fetched on start-up and then every 5 minutes to keep it fresh. Fiat / crypto determinations as well as listing base units (e.g. dividing by 100000000) are based off of a local configuration file, so they don't require any calls to the server. Also, at this point, we've abstracted most of our currency related calculations into a currency module that does most of the heavy lifting for us.