[DDW-827] Show helpful message when trying to empty wallet containing native tokens #2783

mchappell · 2021-12-10T13:58:58Z

This PR shows a more helpful error message when trying to an amount of funds out of the wallet, which would not leave enough funds left to support native tokens in a wallet.

Screenshots

Testing Checklist

Testing Scenarios

Scenario 1: Sending max allowable ADA but not native tokens
Given that I have a wallet with ADA and native tokens
When I attempt to send max allowable ADA to be sent
Then the following message is displayed:

**Insufficient funds to support tokens. A minimum of {adaToRemain} ADA must remain in the wallet after this transaction.
トークンをサポートする資金が不足しています。このトランザクションの後に、ウォレットに{adaToRemain} ADA以上残るようにしてください。**

And Daedalus will not proceed with the transaction

Scenario 2: Sending max allowable ADA and all native tokens
Given that I have a wallet with ADA and native tokens
When I attempt to send max allowable ADA to be sent and all native tokens 
Then the transaction will go through as expected

Scenario 3: Sending (max allowable ADA - 1ADA) and all native tokens (Note: adaToRemain is currently 1ADA)
Given that I have a wallet with ADA and native tokens
When I attempt to send (max allowable ADA to be sent - 1ADA) and all native tokens 
Then the transaction will go through as expected

Scenario 4: Sending (max allowable ADA -  < 1ADA) but not native tokens (Note: adaToRemain is currently 1ADA)
Given that I have a wallet with ADA and native tokens
When I attempt to send (max allowable ADA to be sent - < 1ADA)
Then the following message is displayed:

**Insufficient funds to support tokens. A minimum of {adaToRemain} ADA must remain in the wallet after this transaction.
トークンをサポートする資金が不足しています。このトランザクションの後に、ウォレットに{adaToRemain} ADA以上残るようにしてください。**

And Daedalus will not proceed with the transaction

Scenario 5: Message displayed when API returns 'cannot_cover_fees' error (Note: This also covers scenario 3 and 4 but adding it to cover additional scenarios that we can think of during exploratory testing)
Given I have a wallet with ada and native tokens
When I try to send a large amount of ada but not tokens
When the API returns a 'cannot_cover_fees' error
Then the following message is displayed:

**Insufficient funds to support tokens. A minimum of {adaToRemain} ADA must remain in the wallet after this transaction.
トークンをサポートする資金が不足しています。このトランザクションの後に、ウォレットに{adaToRemain} ADA以上残るようにしてください。**

And Daedalus will not proceed with the transaction

Slack QA thread
Test

Review Checklist

Basics

Code Quality

Important parts of the code are properly commented and documented
Code is properly typed with flow
React components are split-up enough to avoid unnecessary re-renderings
Any code that only works in main process is neatly separated from components

Testing

New feature/change is covered by acceptance tests
New feature/change is manually tested and approved by QA team
All existing acceptance tests are still up-to-date
New feature/change is covered by Daedalus Testing scenario
All existing Daedalus Testing scenarios are still up-to-date

After Review

Merge the PR
Delete the source branch
Move the ticket to done column on the YouTrack board
Update Slack QA thread by marking it with a green checkmark

source/renderer/app/api/api.js

# Conflicts: # CHANGELOG.md

source/renderer/app/components/wallet/send-form/WalletSendConfirmationDialog.js

miorsufianiohk · 2022-01-25T10:13:51Z

Hi @mchappell ,

On build 20533, I noticed when:

I have ada and native tokens
I only sent ada but leaving ada balance of < 1 ADA

Observation: The error message contained NaN ADA. Please see screenshot

Note: This is new which didn't exist in the last few builds during testing.

… cases

gabriela-ponce · 2022-01-25T19:44:45Z

@mchappell This looks good overall, but I noticed some strange scenarios:

I was able to leave less than 2 ADA on a wallet, and I couldn't move the tokens this wallet had with the remaining balance. Check this video, you can see first an attempt of emptying the wallet (having less than 2 ADA) and the message reads that 2 ADA are required. Then you see the last transaction (made on the same build a few minutes before) where I was able to move almost all the balance from this wallet.
On the same wallet (containing the same tokens and more ADA) I got the message that informs that 1 ADA is required (instead of 2). Then when I left 1.9 ADA on the wallet, I was able to move the tokens, but more than 1 ADA was required. Check this video (ignore from 1:02 to 1:44, as I couldn't proceed because of a pending transaction).

miorsufianiohk · 2022-02-11T10:31:30Z

Hi @mchappell ,

This is an interim report for build 20898 as test is still ongoing (as discussed) and for ease of reference for everybody

When I typed wrong password, I received the error message that you created for this PR. See video
Also, when I attempted to send all allowable ada with all native tokens, I still got the same error message displayed. See video

mchappell · 2022-02-11T12:26:38Z

Hi @mchappell ,

This is an interim report for build 20898 as test is still ongoing (as discussed) and for ease of reference for everybody

When I typed wrong password, I received the error message that you created for this PR. See video

Also, when I attempted to send all allowable ada with all native tokens, I still got the same error message displayed. See video

Hey @miorsufianiohk, thanks for testing this

Can you confirm if 1. is a behaviour that exists in develop, if so it's out of scope for this PR but please discuss future improvement with the team
2. This has now been fixed and ready to re-check

Thanks for being really rigorous with this one 👍

przemyslaw-wlodek · 2022-02-11T13:11:33Z

source/main/ipc/downloadManagerChannel.ts

+> => {
  // @ts-ignore ts-migrate(2322) FIXME: Type 'unknown' is not assignable to type 'Download... Remove this comment to see the full error message
-> => localStorage.getAll();
+  return localStorage.getAll();
+};


Apologies, but is there any reason to change that? :) I assume that previous ts-ignore was working.

git merge didn't work out the right way and reverted to a previous configuration. That's the sad part about long lived branches with multiple migrations run

gabriela-ponce

This looks ok, good job 💯

Regarding the issues mentioned here, we think the API is returning a smaller value than is actually required to stay in the wallet to move the tokens. As it's out of the scope of this PR we'll check this with the Adrestia team and if required, address it on another card.

miorsufianiohk

Hi @mchappell,

LGTM. I could confirm that issue 1 exists on develop too of which I have created a Jira ticket for it. Issue 2 is now fixed. Great work 👍 . Tested on 20909

marcin-mazurek · 2022-02-14T12:41:39Z

source/renderer/app/api/api.ts

  GetNetworkParametersApiResponse,
 } from './network/types';
 // Transactions Types
+// @ts-ignore ts-migrate(2307) FIXME: Cannot find module './transactions/types' or its c... Remove this comment to see the full error message


Not sure how it happened, is it a result of running ts-migrate, but could we ideally please not introduce new ts-ignore's?

source/renderer/app/api/utils/apiHelpers.spec.ts

source/renderer/app/api/utils/apiHelpers.ts

source/renderer/app/api/utils/apiHelpers.spec.ts

source/renderer/app/components/status/DaedalusDiagnostics.tsx

source/renderer/app/components/wallet/send-form/WalletSendAssetsConfirmationDialog.tsx

source/renderer/app/components/wallet/send-form/WalletSendConfirmationDialog.tsx

danielmain

Great job @mchappell I have some questions if you still have some minutes for us :)

danielmain · 2022-02-16T12:16:21Z

source/renderer/app/actions/lib/Action.ts

  }

-  trigger(params: Params) {
+  trigger(params?: Params) {


If this is optional, why not checking it its null the next line?

source/renderer/app/api/utils/apiHelpers.ts

CHANGELOG.md

…pty-wallet-with-tokens

… native tokens (#2783) Co-authored-by: Daniel Main <daniel.main.cernhoff@icloud.com> Co-authored-by: Marcin Mazurek <marcin.mazurek@iohk.io> Co-authored-by: Renan Ferreira <renan.ferreira@iohk.io> Co-authored-by: Szymon Masłowski <szymon.maslowski@iohk.io>

[DDW-827] show helpful message

4ad3c05

mchappell added feature WIP labels Dec 10, 2021

mchappell self-assigned this Dec 10, 2021

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

3e5e5c5

danielmain added the on-hold label Dec 14, 2021

mchappell added 2 commits January 18, 2022 09:11

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

469988a

[DDW-827] updated error message and translations

160e7c0

mchappell removed the on-hold label Jan 18, 2022

mchappell added 3 commits January 18, 2022 14:46

[DDW-827] fixed flow types for sendMoney

56af6df

[DDW-827] updated CHANGELOG

2b253a3

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

825a6fe

mchappell added ⏳release-vNext and removed WIP labels Jan 21, 2022

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

9815f89

miorsufianiohk self-requested a review January 21, 2022 10:55

mchappell requested review from a team January 21, 2022 12:29

lucas-barros reviewed Jan 21, 2022

View reviewed changes

source/renderer/app/api/api.js Outdated Show resolved Hide resolved

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

fae171e

# Conflicts: # CHANGELOG.md

renanvalentin reviewed Jan 24, 2022

View reviewed changes

source/renderer/app/components/wallet/send-form/WalletSendConfirmationDialog.js Show resolved Hide resolved

mchappell added 2 commits January 24, 2022 19:08

[DDW-827] Addressed PR comments

55c079a

[DDW-827] updated regex

93dd0e2

mchappell added 2 commits January 25, 2022 16:06

[DDW-827] updated regex and moved to testable helper method with test…

0f84bcc

… cases

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

80a2bae

mchappell added 3 commits January 27, 2022 11:33

[DDW-827] prevent resubmission in the case of error

c9ddad9

[DDW-827] updated unit test and type for helper

e7bad15

[DDW-827] temporarily removed husky pre-commit

5ed7eb0

[DDW-827] prevented submission using keyboard if form error exists

7edda20

[DDW-827] updated api thrown error

3719798

przemyslaw-wlodek approved these changes Feb 11, 2022

View reviewed changes

gabriela-ponce approved these changes Feb 11, 2022

View reviewed changes

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

963db65

miorsufianiohk approved these changes Feb 14, 2022

View reviewed changes

marcin-mazurek suggested changes Feb 14, 2022

View reviewed changes

renanvalentin reviewed Feb 14, 2022

View reviewed changes

source/renderer/app/api/utils/apiHelpers.spec.ts Outdated Show resolved Hide resolved

renanvalentin reviewed Feb 14, 2022

View reviewed changes

source/renderer/app/components/status/DaedalusDiagnostics.tsx Outdated Show resolved Hide resolved

renanvalentin reviewed Feb 14, 2022

View reviewed changes

source/renderer/app/components/wallet/send-form/WalletSendAssetsConfirmationDialog.tsx Show resolved Hide resolved

renanvalentin reviewed Feb 14, 2022

View reviewed changes

source/renderer/app/components/wallet/send-form/WalletSendConfirmationDialog.tsx Show resolved Hide resolved

Marcin Mazurek and others added 4 commits February 14, 2022 16:42

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

43a2225

[DDW-827] Code review changes

c05d7f2

Merge branch 'develop' into feature/ddw-827-empty-wallet-with-tokens

71edff6

[DDW-827] Updated CHANGELOG.md and run yarn manage:translations

125791b

danielmain suggested changes Feb 16, 2022

View reviewed changes

[DDW-827] Remove ts-ignore

b2965b3

szymonmaslowski added the WIP label Feb 23, 2022

szymonmaslowski added 2 commits February 23, 2022 10:54

Merge remote-tracking branch 'origin/develop' into feature/ddw-827-em…

3f6a5f5

…pty-wallet-with-tokens

[DDW-827] Update defaultMessages.json file

fee9423

danielmain merged commit c515b62 into develop Feb 23, 2022

danielmain deleted the feature/ddw-827-empty-wallet-with-tokens branch February 23, 2022 11:14

danielmain removed the WIP label Feb 23, 2022

marcin-mazurek mentioned this pull request Mar 23, 2022

Behavior when trying to transfer money from a wallet containing tokens so that the balance is less than 1 ADA #2932

Closed

danielmain added release-4.10.0 and removed ⏳release-vNext labels May 16, 2022

[DDW-827] Show helpful message when trying to empty wallet containing native tokens #2783

[DDW-827] Show helpful message when trying to empty wallet containing native tokens #2783

Uh oh!

Conversation

mchappell commented Dec 10, 2021 • edited by miorsufianiohk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

Testing Checklist

Testing Scenarios

Review Checklist

Basics

Code Quality

Testing

After Review

Uh oh!

Uh oh!

Uh oh!

miorsufianiohk commented Jan 25, 2022

Uh oh!

gabriela-ponce commented Jan 25, 2022

Uh oh!

miorsufianiohk commented Feb 11, 2022

Uh oh!

mchappell commented Feb 11, 2022

Uh oh!

przemyslaw-wlodek Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

mchappell Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

gabriela-ponce left a comment

Choose a reason for hiding this comment

Uh oh!

miorsufianiohk left a comment

Choose a reason for hiding this comment

Uh oh!

marcin-mazurek Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielmain left a comment

Choose a reason for hiding this comment

Uh oh!

danielmain Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

mchappell commented Dec 10, 2021 •

edited by miorsufianiohk

Loading