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

(ADP-257) Add Shelley migration endpoints and Byron (haskell impl) support of migrateWallet #1675

Closed
paweljakubas opened this issue May 20, 2020 · 10 comments
Assignees

Comments

@paweljakubas
Copy link
Contributor

paweljakubas commented May 20, 2020

Context

As an API Client,
I want to be able to empty a wallet and send all funds to a list of addresses,
So that I can easily move funds around and have the wallet dealing with fees for me.

Decision

We are adding migration endpoints as done in #1658 in Shelley

Acceptance Criteria

  1. Shelley has migration sendAll endpoints and cost estimation endpoint for this.
  2. We have Shelley migration post data unit tests.
  3. Corresponding changes in swagger are done.
  4. Migration tests are added to cardano-node's byron integration tests.

Comment: as transaction are not available for shelley we cannot check it at this moment in integration test mode.


Development

QA

  1. migrateWallet was enabled for byron (haskell impl) and several integration tests were added in lib/byron.
  2. The subtle bug was found in Transaction layer of byron (haskell impl) and TransactionSpec was adjusted.
  3. migrateWallet was enabled also for shelley (haskell impl), but at this moment it was not impossible to test it as this lib misses transaction layer impl (although it should go very soon as Add Shelley TransactionLayer #1673 - so it would be worth testing this (maybe copy integration tests from byron lib concerning migration)
@paweljakubas paweljakubas added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label May 20, 2020
@paweljakubas paweljakubas self-assigned this May 20, 2020
@KtorZ
Copy link
Member

KtorZ commented May 20, 2020

When reviewing #1658 I noticed that the current migration endpoints are actually associated with a /migrations namespace / resource. So, perhaps it makes sense to keep the separation between transactions / migrations in our API types as well? @paweljakubas

@paweljakubas paweljakubas changed the title (ADP-257) Add Shelley migration endpoints and relocate all migration endpoints (ADP-257) Add Shelley migration endpoints May 21, 2020
@KtorZ KtorZ removed the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label May 22, 2020
iohk-bors bot added a commit that referenced this issue May 26, 2020
1678: Migration endpoints for shelley and cardano-node's byron support r=paweljakubas a=paweljakubas

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->
#1675 

# Overview

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

- [x] I have make ApiWalletMigrationPostData more flexible
- [x] I have added ShelleyMigration to API enpoints
- [x] I have added unit tests for the changed ApiWalletMigrationPostData
- [x] I have added integration tests to cardano-node's byron suite


# Comments


<!-- 
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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@paweljakubas paweljakubas changed the title (ADP-257) Add Shelley migration endpoints (ADP-257) Add Shelley migration endpoints and Byron (haskell impl) support of migrateWallet May 26, 2020
@piotr-iohk
Copy link
Contributor

I have updated swagger spec -> #1687.
It would be nice to also address this -> #1682.

iohk-bors bot added a commit that referenced this issue May 26, 2020
1687: Display Shelley migrations in Api doc r=piotr-iohk a=piotr-iohk

# Issue Number

#1675

# Overview

- 1867266
  Display Shelley migrations in swagger spec.
  
- 508e41b
  Reword migration description according to review suggestions



# Comments

Shelley migrations, although included in `swagger.yaml`, were not being displayed in the [API doc](https://input-output-hk.github.io/cardano-wallet/api/edge/). This pr fixes this. Also includes minor rewording of the migration endpoints descriptions.

![Screenshot from 2020-05-26 16-38-13](https://user-images.githubusercontent.com/42900201/82914129-6c934b00-9f6f-11ea-91e3-d92d336498f5.png)




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

I'll leave it in QA for now because Shelley migrations are not quite testable currently without transactions.

@piotr-iohk
Copy link
Contributor

I encounter a 500 response when running migrations from Byron wallet on F&F testnet (for Shelley wallets migrations seem to work fine).

Steps to reproduce:

  1. Run migration on Byron wallet having funds and as a result you'll have 500 response:
POST http://localhost:8090/v2/byron-wallets/94c0af1034914f4455b7eb795ebea74392deafe9/migrations

The response was:
Code = 500,
Message = {"code":"created_invalid_transaction","message":"That's embarrassing. It looks like I've created an invalid transaction that could not be parsed by the node. Here's an error message that may help with debugging: ApplyTxError [LedgerFailure (UtxowFailure (MissingVKeyWitnessesUTXOW {pfUTXOmissingWitnesses = fromList [KeyHash 03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314]}))]"}

In the wallet.log there is:

[cardano-wallet.api-server:Info:630] [2020-06-01 11:44:52.51 UTC] [RequestId 99] [POST] /v2/byron-wallets/94c0af1034914f4455b7eb795ebea74392deafe9/migrations
[cardano-wallet.network:Info:18] [2020-06-01 11:44:52.61 UTC] Send MsgSubmitTx Tx' {_body' = TxBody' {_inputs' = fromList [TxIn (TxId {_TxId = 1594e75578ceb053ac3bfa6d9da3fa2de4e2f89df1c269984a6fcc4b935cb8a1}) 0], _outputs' = StrictSeq {getSeq = fromList [TxOut (AddrBootstrap (Address {addrRoot = cf9aac64ca146b7812c623d40d1a6b429f92684e9c801078fc4f149d, addrAttributes = Attributes { data: AddrAttributes {aaVKDerivationPath = Just (HDAddressPayload {getHDAddressPayload = "s\212\ta\138\197\196\231J(}\146d\150\191\v6\232J\223\255\218\150\251\210@\DLE\211"}), aaNetworkMagic = NetworkTestnet 42} }, addrType = ATVerKey})) (Coin 999834719)]}, _certs' = StrictSeq {getSeq = fromList []}, _wdrls' = Wdrl {unWdrl = fromList []}, _txfee' = Coin 165281, _ttl' = SlotNo {unSlotNo = 686680}, _txUpdate' = SNothing, _mdHash' = SNothing, bodyBytes = "\164\NUL\129\130X \NAK\148\231Ux\206\176S\172;\250m\157\163\250-\228\226\248\157\241\194i\152Jo\204K\147\\\184\161\NUL\SOH\129\130XP\130\216\CANXF\131X\FS\207\154\172d\202\DC4kx\DC2\198#\212\r\SUBkB\159\146hN\156\128\DLEx\252O\DC4\157\162\SOHX\RSX\FSs\212\ta\138\197\196\231J(}\146d\150\191\v6\232J\223\255\218\150\251\210@\DLE\211\STXB\CAN*\NUL\SUB\NUL\196\250\143\SUB;\152D_\STX\SUB\NUL\STX\133\161\ETX\SUB\NUL\nzX", extraSize = 131}, _witnessVKeySet' = fromList [WitVKey' {wvkKey' = VKey (VerKeyEd25519DSIGN (PublicKey "\b\171\238\224\175\154\207\221M3S\SO\224\253\132\ETB|\210\214\251d(\DC4\200\202c\191\172`\238\206\170")), wvkSig' = SignedDSIGN (SigEd25519DSIGN (Signature "\243\241\133\254\186\150G\191\161\133\189\176\246L\ACK\DC4\167\254%T\192\241,\213b\146?\185%\231v@C\221U\143\199\185\233e\188\229\EMo\130\216\202@\184+2\241\191~\242\DEL\233\CAN\209c\f\245\152\ACK")), wvkBytes = "\130X \b\171\238\224\175\154\207\221M3S\SO\224\253\132\ETB|\210\214\251d(\DC4\200\202c\191\172`\238\206\170X@\243\241\133\254\186\150G\191\161\133\189\176\246L\ACK\DC4\167\254%T\192\241,\213b\146?\185%\231v@C\221U\143\199\185\233e\188\229\EMo\130\216\202@\184+2\241\191~\242\DEL\233\CAN\209c\f\245\152\ACK"}], _witnessMSigMap' = fromList [], _metadata' = SNothing, txWitsBytes = "\161\NUL\129\130X \b\171\238\224\175\154\207\221M3S\SO\224\253\132\ETB|\210\214\251d(\DC4\200\202c\191\172`\238\206\170X@\243\241\133\254\186\150G\191\161\133\189\176\246L\ACK\DC4\167\254%T\192\241,\213b\146?\185%\231v@C\221U\143\199\185\233e\188\229\EMo\130\216\202@\184+2\241\191~\242\DEL\233\CAN\209c\f\245\152\ACK", txFullBytes = "\131\164\NUL\129\130X \NAK\148\231Ux\206\176S\172;\250m\157\163\250-\228\226\248\157\241\194i\152Jo\204K\147\\\184\161\NUL\SOH\129\130XP\130\216\CANXF\131X\FS\207\154\172d\202\DC4kx\DC2\198#\212\r\SUBkB\159\146hN\156\128\DLEx\252O\DC4\157\162\SOHX\RSX\FSs\212\ta\138\197\196\231J(}\146d\150\191\v6\232J\223\255\218\150\251\210@\DLE\211\STXB\CAN*\NUL\SUB\NUL\196\250\143\SUB;\152D_\STX\SUB\NUL\STX\133\161\ETX\SUB\NUL\nzX\161\NUL\129\130X \b\171\238\224\175\154\207\221M3S\SO\224\253\132\ETB|\210\214\251d(\DC4\200\202c\191\172`\238\206\170X@\243\241\133\254\186\150G\191\161\133\189\176\246L\ACK\DC4\167\254%T\192\241,\213b\146?\185%\231v@C\221U\143\199\185\233e\188\229\EMo\130\216\202@\184+2\241\191~\242\DEL\233\CAN\209c\f\245\152\ACK\246"}
[cardano-wallet.network:Info:18] [2020-06-01 11:44:52.64 UTC] Recv MsgRejectTx (ApplyTxError [LedgerFailure (UtxowFailure (MissingVKeyWitnessesUTXOW {pfUTXOmissingWitnesses = fromList [KeyHash 03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314]}))])
[cardano-wallet.api-server:Error:630] [2020-06-01 11:44:52.64 UTC] [RequestId 99] 500 Internal Server Error in 0.128202956s

Byron wallet on F&F testnet with funds to reproduce ->

purchase carbon forest frog robot actual used news broken start plunge family

@piotr-iohk
Copy link
Contributor

Shelley migrations are also lacking any integration tests at this moment.

iohk-bors bot added a commit that referenced this issue Jun 12, 2020
1739: Migration tests clean up r=piotr-iohk a=piotr-iohk

# Issue Number

#1675

# Overview

- 78a6a1c
  Extract Migrations for byron to separate scenario file
  
- 64a0416
  Extract Migration tests for byron in core-integration
  
- 96bd585
  Migration tests for Shelley
  
- 90853c3
  Remove `pending` from byron migration tests
  
- 5e2f26c
  Make `SHELLEY_MIGRATE_01_big_wallet` test pending due to issue #1751
  
- 5c25b01
  - Add ByronWallets tests to Shelley integration suite.  
  - Make `Todo comment` which tests to enable when Byron addresses/transactions are supported on the cardano-node.  
  - temporarily move BYRON_RESTORE special cases involving restoring 'funky' ledger and icarus wallets to Transactions so we can run ByronWallet tests on Shelley integration suite




# Comments

Attempting to "clean-up" migration tests and add migration tests for Shelley and Byron.

~I have added some byron (random + icarus) wallets into genesis.yaml, but... It doesn't really work, node crashes... 🤔~ ... because they are not supported yet.




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

For the record this is still the case on cardano-node 1.14.1 -> #1675 (comment)

@KtorZ
Copy link
Member

KtorZ commented Jun 26, 2020

@piotr-iohk we can't yet do transactions from Byron addresses. We need to acknowledge for Byron witnesses in transactions, which only landed recently on cardano-node. We can keep this ticket open perhaps until we solve this.

@paweljakubas
Copy link
Contributor Author

@piotr-iohk now this can be re-looked

@paweljakubas
Copy link
Contributor Author

this one is in QA #1844 and related

@piotr-iohk
Copy link
Contributor

lgtm, closing. Migration tests enabled in #1900.

iohk-bors bot added a commit that referenced this issue Jul 13, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 14, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 17, 2020
1900: Testing byron txs and migrations in Shelley r=rvl a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 18, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 20, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 21, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 22, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 23, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Jul 24, 2020
1900: Testing byron txs and migrations in Shelley r=piotr-iohk a=piotr-iohk

# Issue Number

#1844, #1675

# Overview

- 66e5151
  Enable more integration tests for byron transactions, migrations, addresses and HW wallets in shelley
  
- d773d95
  Add some special Byron and Icarus wallets to genesis for testing migration scenarios
  
- 834923e
  Use regular HSpec.it for tests that use explicit/special mnemonics, otherwise in case they fail, on repeat there will be a 409 as such wallet would exist
  
- 18d1c34
  Don't run Byron tx test on Jormungandr

  



# 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
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

3 participants