[instant] Request account address and balance at mount #1232
Conversation
| store.dispatch(actions.setAccountStateError()); | ||
| return; | ||
| } | ||
| if (!_.isEmpty(availableAddresses)) { |
There was a problem hiding this comment.
super nit (not necessary to change) but I'd prefer for this to follow the return pattern above, i.e.
if (_.isEmpty(availableAddresses)) {
store.dispatch(actions.setAccountStateLocked());
return;
}
... setAccountStateReady code ..
| store.dispatch(actions.setAccountStateLocked()); | ||
| } | ||
| }, | ||
| fetchAccountBalanceAndDispatchToStore: async (store: Store) => { |
There was a problem hiding this comment.
Is it possible that we'd have a concurrency issue in which we'd dispatch a balance for an account that they have since changed? I feel like we may need a similar check here as we have for the buy quote.
Alternatively, part of me feels like balance fetching could be paired with address fetching. This could be easier to reason about -- we always get the address and the balance together, or the whole thing is considered "failed". I'm not married to this though, what do you think?
There was a problem hiding this comment.
Yes, you are correct wrt to the balance update coming late issue
There was a problem hiding this comment.
:megusta:
@steveklebanoff's last comment is interesting. It does feel like it would simplify things... but we rely on this account notion to distinguish between different "not ready" states right?
|
@fragosti im inclined to leave the balance as optional in the AccountReady state because it is not core to the functionality of instant. Even if we did not have the balance, we would still be able to function normally, we would just not be able to do the "do you have enough ETH" validation, which is not a big deal because it will most likely be caught at the wallet layer |
* development: [instant] Viewport specific errors (#1228) Added more comments Include wholeNumberSchema in sra-spec schemas chore(instant): fix linter Fix isNode fix(instant): update buy quote at start up in the case of default amount fix: apply css reset to overlay as well fix(website): turn off production flag when building locally chore: linter fix: progress bar fix: use fontSize prop in button feat: make instant resistant to external styles feat: add faux externall css file Add upstream issue Use const require instead of import Fix tslint issues Use detect-node Set curstom inspect printer in BigNumber
|
Thanks for the concurrency fix 🎉 ! |
Description
asyncData.fetchAccountInfoAndDispatchToStore()that kicks off a request for the account address and balancestate.providerState.accountobject in reduxTesting instructions
Types of changes
Checklist:
[WIP]if necessary.[sol-cov] Fixed bug.