Update GUI labels to reflect multiple rate providers #4406
Conversation
Update the displayed text, as well as the tooltip, of the price box in the top right bar. It now indicates that the price data is provided by Bisq pricenodes (for for fiat, as well as for alts).
The lastRequest timestamp is changed to show the last request to a pricenode. The previous approach of using the "last provider request timestamp" does not make sense in the new setup. Each currency rate is based on rates from several providers, each with their own "request timestamps". In addition, the pricenode returns the timestamp each rate was calculated. On top of that comes the timestamp when the Bisq node queries the pricenode. Since what is most relevant for the Bisq node is the "freshness" of a specific rate, the timestamp most indicative of that is the moment when the pricenode is queried.
Removed getters for the BA and CMC timestamps, both of which are not used anymore.
Rename method to remove reference to the BA provider.
Rename timestamp field which implied it represents an epoch value in seconds, but the way it was used to build a Date object showed that it actually expected a millis value.
|
@wiz this is the label and tooltip change, in a separate PR. |
| // Each currency rate has a different timestamp, depending on when | ||
| // the pricenode aggregate rate was calculated | ||
| // However, the request timestamp is when the pricenode was queried | ||
| epochInMillisAtLastRequest = System.currentTimeMillis(); |
There was a problem hiding this comment.
Hold on a second, this isn't just changing labels, you're actually changing code here. It looks like you're replacing the timestamp of the actual price data, with the timestamp Bisq updated it from the Pricenode, which is incorrect. This behavior needs to be preserved, for example if a pricenode has an issue and doesn't get data updates, the local Bisq node needs to be able to detect that it's old data and switch to a different Pricenode with recently updated data.
There was a problem hiding this comment.
Since there is no more BA provider, the btcAverageTs field is basically useless.
It is however used here to populate the epochInMillisAtLastRequest timestamp -- which is used in the UI to show when the fiat rates were retrieved.
Before, when using BA: this made sense, cause BA was the "single source of truth" for all fiat rates, and Poloniex was the source of alt rates.
Now, when using multiple providers: basically the pricenode itself is the "provider", because it aggregates rates from multiple sources for both fiat and alts.
So, the closest "timestamp" that shows the last time the "provider" (pricenode) was polled, is the one in the code snippet above.
Alternative would be to extract and index every timestamp per exchange rate, then selectively show the right one depending on what currency pair the user selected in the UI. That seems like overkill for what this change tries to achieve. Especially since those "per exchange rate" timestamps are very likely the same, cause they all show the point in time when the avg rate was calculated, and they're all calculated when the pricenode is polled (the avg is freshly calculated based on newest data).
So I would say, this is the simplest and most pragmatic approach (epochInMillisAtLastRequest = timestamp_when_pricenode_last_successfully_queried).
There was a problem hiding this comment.
The timestamp is not for when the data was updated from the pricenode, that's irrelevant. The timestamp is for the price itself, i.e. when the price was obtained from the provider, or in the case of a weighted average aka index, the time when the index was last updated. Can you please use the timestampSec field for each asset's index?
There was a problem hiding this comment.
Done. This field now just stores the "pricenode query timestamp", but the timestampSec of each asset is shown in the tooltip (see PR screenshots).
Update top-right status box and tooltip labels to indicate the price is based on the Bisq Price Index.
Update timestamp shown in top-right tooltip, to indicate the point in time when that specific exchange rate was retrieved (from an Exchange, if only one exchange supported for that currency) or when it was calculated (by the pricenode, based on inputs from multiple exchanges).
Update field describing the source of the shown exchange rates, indicating that the Bisq Price Index is used.
cd2357
force-pushed
the
xchange-integration-gui-label-adjustments
branch
from
5fa0fe6
to
ef5b804
Compare
Co-authored-by: wiz <j@wiz.biz>
Yes, I'd also suggest to wait with this PR. At least until the pricenode PR runs for a while and you're confident it doesn't need a rollback. |
|
|
Update the Bisq GUI labels to better indicate the source of the exchange rate data.
The areas changed are:
1) Top right box / The shown text for fiat and alts:
1) Top right box / The tooltip for fiat and alts:
2) About view / Source of market prices under Data Providers
Addresses bisq-network/projects#35
Related to #4315