Skip to content
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

Calculating migration cost of the wallet that cannot be migrated should return 403 #1007

Closed
piotr-iohk opened this issue Nov 12, 2019 · 1 comment

Comments

@piotr-iohk
Copy link
Contributor

piotr-iohk commented Nov 12, 2019

Release Operating System Cause
next Windows & OSX & Linux) Code v Configuration v Environment v Human v Unknown

Context

#989 (comment)

We are returning 403 and error message when trying to migrate the wallets that are empty or contain only dust, however when calculating migration cost for such wallets we return 200 and migration cost = 0. This is somewhat inconsistent. We should be returning 403 and error message when calculating such migration cost too.

Note: for instance for estimating cost of transaction that is impossible to make due to insufficient funds, we return 403 -> https://github.com/input-output-hk/cardano-wallet/blob/master/lib/core-integration/src/Test/Integration/Scenario/API/Transactions.hs#L930-L951

Steps to Reproduce

  1. Run BYRON_CALCULATE_02 integration test -> https://github.com/input-output-hk/cardano-wallet/blob/master/lib/core-integration/src/Test/Integration/Scenario/API/ByronWallets.hs#L154-L163

Expected behavior

We should return 403 and message (perhaps the same as when migrating such wallet):

I can't migrate the wallet with the given id: <wid>, because it's either empty or full of small coins which wouldn't be worth migrating.

Actual behavior

Currently it passes, asserts for 0 calculated cost.


Resolution Plan

PR

Number Base
#1008 master

QA

BYRON_CALCULATE_02 integration tests.

@piotr-iohk piotr-iohk added this to the Byron Wallet Support milestone Nov 12, 2019
iohk-bors bot added a commit that referenced this issue Nov 12, 2019
1008: Additional tests for migrating and calculating migration cost for wallets containing only dust r=piotr-iohk a=piotr-iohk



# Issue Number

#779 
#1007

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [ ] Added additional tests for attempting to migrate and calculate migration cost for dust wallets. Those special wallets added to `genesis.yaml`


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
Co-authored-by: Pawel Jakubas <[email protected]>
@piotr-iohk
Copy link
Contributor Author

lgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant