-
Notifications
You must be signed in to change notification settings - Fork 295
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
Feature/ddw 220 Implement async wallet restore #849
Feature/ddw 220 Implement async wallet restore #849
Conversation
…ub.com:input-output-hk/daedalus into feature/ddw-220-implement-async-wallet-restore
…ub.com:input-output-hk/daedalus into feature/ddw-220-implement-async-wallet-restore
@darko-mijic @DominikGuzei @tomothespian this PR is now ready for the review! |
PS. I will post details of how to run Cardano-SL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @nikolaglumac 👍
// Reject errors | ||
response.on('error', (error) => reject(error)); | ||
// Resolve JSON results and handle weird backend behavior | ||
// of "Left" (for errors) and "Right" (for success) properties |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment should be removed as it is not the case here anymore!
if (!wallets.active) return <MainLayout><LoadingSpinner /></MainLayout>; | ||
|
||
const isRestoreActive = get(wallets.active, 'syncState.tag') === 'restoring'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to refactor this into proper api TYPES instead of magic constants.
@@ -30,6 +31,8 @@ export default class WalletSendPage extends Component<Props> { | |||
// Guard against potential null values | |||
if (!activeWallet) throw new Error('Active wallet required for WalletSendPage.'); | |||
|
|||
const isRestoreActive = get(activeWallet, 'syncState.tag') === 'restoring'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here -> using proper api types instead of 'restoring'
string
@@ -51,7 +52,9 @@ export default class WalletSummaryPage extends Component<Props> { | |||
let walletTransactions = null; | |||
const noTransactionsLabel = intl.formatMessage(messages.noTransactions); | |||
|
|||
if (recentTransactionsRequest.isExecutingFirstTime || hasAny) { | |||
const isRestoreActive = get(wallet, 'syncState.tag') === 'restoring'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
@DominikGuzei all your comments are now fixed! |
This PR contains the implementation of asynchronous wallet restoration and import features.
This is a
major
improvement as the wallet which is being restored/imported is momentarily available in the UI. User is able to straight away see the wallet's balance and existing wallet addresses while wallet transaction history is being fetched in the background. During the restoration process details are shown: restoration progress percentage and estimated time of completion. It is not possible to send new transactions during the entire restoration process.TODOs:
Summary screen
Send screen
Transactions screen