-
Notifications
You must be signed in to change notification settings - Fork 4
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
design: asynchronously load tokens on both wallets #267
Comments
For me this Guide Level is approved, I like the new UX. For the low level, one important note is that this might affect the real time update from the ws. With this change, we must update the internal history only if the new real time tx has one of the loaded tokens. |
The Guide Level is approved for me. I think we should have a low level design to discuss how we will handle all loading tasks internally. I feel we should replace all "integer fields" by "object fields" with a status (stopped, loading, loaded), last update time, and value. We should also discuss the updating policy for these fields. |
From my point of view, the UX is clear and adequate for this solution. |
I like the reference level design in general, just some comments:
When are we going to use this? If the history/balance has been loaded a long time ago, should still be fine because of the real time update, right?
|
One note about the real time
We should dispatch the actions only if the token has already been loaded, right? Actually I'm not sure about the wallet service facade, when do we start getting real time update? We wait until the wallet is ready, or until the wallet loads all tokens nowadays? |
You are right, I guess that this time to live does not make sense as we will always have the updated history because of real-time updates. I will remove this part, but wait until @msbrogli reads this |
I agree that we should dispatch the load balance and history actions only if the token has already been loaded, will update the design Currently on the wallet service, we are only setting up the connection after the wallet is ready (the wallet start lambda returns 'ready') but before the history and balances are loaded |
@andreabadesso I like the idea of using saga. Good job! Here are some thoughts:
|
Thanks for your review
That is correct, I described the new data structure for I added this note to make it clearer what will happen on the old facade:
What do you think?
I agree, updated the
Also added a note for that on the |
Approved |
@andreabadesso Thanks! This design is approved by me. |
Summary
We should download token information asynchronously on the wallets, changing the UX accordingly to inform the user that the balances may not yet have been downloaded
One point note mentioning is that the changes proposed on this document are specifically for the new wallet-service facade, so no changes should be made when the wallets are using the old facade
Motivation
Currently, while using the wallet-service facade, our wallets download the token balances and history synchronously, downloading one token a time, as we can see on this wallet-desktop example:
Only after that complete download, the dashboard screen is displayed with the downloaded balances and history.
Another problem is that we download the balances and history for all the tokens the user has ever interacted with, even if they are not registered
This is a problem for users with multiple tokens as the perceived loading time of the wallet is very big
Guide-level explanation
The idea is to show the main screen of the wallet as soon as the token set as default on the config and the hathor token balance is loaded and start downloading asynchronously the list of tokens the user has registered on his device.
We also should change the behaviour of the wallet to only download the history of transactions when the user effectively clicks the token to see it, also displaying a loading indicator until the page is downloaded
UI Changes
Dashboard screen
We should display a loading indicator right next to the names of the registered tokens, indicating that the balance for this token is still being downloaded.
This download will be triggered by default when the user enters this screen for each token he has registered.
Unknown tokens screen
On the desktop wallet, we have a screen that displays all the tokens that have not been registered yet, including their balances and history
If the user clicks the
Show history
button and the balance was already loaded, we should display only theLoading history...
spinnerIf the user clicks the "Hide history" button before the history download finishes, we will continue the download in background and the next time the user clicks the "Show history" button, we will show the downloaded history.
History screen
Since the history will only be downloaded from the wallet-service lambda when the user clicks the token to see it, we should show a loading indicator until the current page of transactions is downloaded
Once the history is downloaded, we should not trigger a download when the user clicks to see the details for a token
On the desktop wallet, since we have multiple downloads on the same screen, we should have a single loading that will be up until every request is successfully resolved
If any of the requests fail, we should display an error screen with a button to retry
Send token
While on the SendAmountInput screen, after clicking the select button to change token, we should also display the loading indicator instead of the balance so the user knows that it is still downloading
Reference-level explanation
Currently, both facades load all the history and balances for all tokens that the wallet has ever interacted with (at least one transaction involving at least one of the addresses that the xpriv owns)
Redux-saga
I propose adding a new redux middleware library to the project called
redux-saga
. The idea is to move all logic that contains side-effects tosagas
that reacts to actions and deals with handling data and errorsRedux-saga can be understood as a separate thread to the application, listening for specific actions from the main application to perform complex asynchronous tasks and updating the application state once it is completed.
Alternatives
Currently, we are using
redux-thunk
which is another redux middleware library that allows us to execute a function before returning an object on a redux action. It is also a great library, it would also (although with less clarity) solve the problem of asynchronously load tokens, but I believe thatredux-saga
is a better fit for this usecase as it has multiple helpers to deal with asynchronicity (like waiting for two actions before continuing), as we will see on theSagas
section of this design.Sagas
We should have some new
sagas
to deal with requesting tokens history and balances and also listen to new tokens being added by the userThis saga will use the
takeEvery
helper to react to everyTOKEN_FETCH_HISTORY_REQUESTED
action, it shoud:false
, do nothing as the history will already be loadedtokenId
sent as payload on the actionTOKEN_FETCH_HISTORY_SUCCESS
action, with the history andtokenId
as payloadTOKEN_FETCH_HISTORY_FAILURE
action, with thetokenId
as payloadThis saga will also use the
takeEvery
helper to react to everyTOKEN_FETCH_BALANCE_REQUESTED
action, it shoud:false
, do nothing as the balance will already be loadedupdatedAt
of the last stored information is higher thanTOKEN_BALANCE_TTL
false
, do dispatchTOKEN_FETCH_BALANCE_SUCCESS
to setloading
tofalse
and breaktokenId
sent as payload on the actionTOKEN_FETCH_BALANCE_SUCCESS
action, with the history andtokenId
as payloadTOKEN_FETCH_BALANCE_SUCCESS
action, with thetokenId
as payloadCurrently, we are requesting the entire history and balance for all tokens that ever interacted with the wallet, this is being done on an event-listener that listens for
state
changes from the wallet facadeWhen it detects that the wallet is ready, meaning that the wallet is already started and loaded on the wallet-service side, it starts fetching balance and history for all tokens (fetched from the
getTokens
API), this is the method as it is today:After the complete download of all history and balances for all tokens is done, we finally dispatch the
FETCH_HISTORY_SUCCESS
action that causes the loading history screen to hide, displaying the dashboard screen with the list of all tokens and their balances.I propose we refactor the
startWallet
(to a saga) to dispatch theFETCH_HISTORY_SUCCESS
action right after the hathor token (and any other token that is defined as theDEFAULT_TOKEN
) history and balance is loaded and dispatch actions to load asynchronously the balance (and only the balance) of all other tokens that the user already registered (fetched from AsyncStorage).It would look something like this:
sagas.js
Real-time updates
Currently, while on the wallet-service facade, we are downloading the entire token balances and history every time a new transaction is received through the websocket channel:
We should refactor this to dispatch actions to download the balances and history for the registered tokens only and also only for the tokens involved in the transaction
State and reducers
Currently, the
tokensHistory
andtokensBalance
data on the mobile and desktop wallet are stored in memory onredux
The way the data is represented after it is loaded is the following:
This is the new proposed state for
tokensBalance
andtokensHistory
after it is loaded (which will be detailed in the following section of this design):Where:
status
might be one ofready
: The download was successful and the data is readyloading
: Download currently in progresserror
: Some error happened while downloading token dataupdatedAt
is a timestamp for the last time this data has been updateddata
stored informationOn the old facade, since we will already have this information loaded on the facade (after the address scan is done), status for the
tokensHistory
andtokensBalance
will always be populated withready
Here are some code examples of what the reducers for the actions dispatched by the sagas would look like:
Same as above
Old facade
While using the old facade, the wallets will use the same sagas as the new facades, so both facades will benefit from
redux-saga
, but the old facade will still load scan the complete list of addresses and after that, all calls to the facade methods will return instantlyScreens will be adapted to the new async interface on both facades, but since the methods on the old facade will return instantly (as the balances and history are already loaded locally), the loading shouldn't be noticeable
Reviewers: @msbrogli @pedroferreira1
The text was updated successfully, but these errors were encountered: