Add API endpoint to migrate funds from a legacy wallet. #779

jonathanknowles · 2019-10-01T05:45:52Z

Context

This task will provide an API endpoint that allows users to migrate their funds from a Byron wallet to a new-style wallet.

Usage

The basic sequence of a complete migration would be:

Use postByronWallet to restore a Byron wallet.
Use postWallet to create a new-style wallet (if one does not already exist).
Use migrateByronWallet to migrate funds from the Byron wallet to the new-style wallet.
Use deleteByronWallet to delete the now-empty Byron wallet.

Decision

The API will provide an endpoint similar to the following:

/byron/wallets/{byronWalletId}/migrate/{walletId}:
  post:
    operationId: migrateByronWallet
    tags: ["Wallets"]   
    summary: Migrate  
    description: |
      Migrate all funds from a Byron wallet to a new-style wallet.
    parameters:
      - *parametersByronWalletId
      - *parametersWalletId
      - <<: *parametersBody 
        schema: *ApiMigrateByronWalletData 
    responses: *responsesMigrateByronWallet

With schema definitions similar to the following:

ApiMigrateByronWalletData: &ApiMigrateByronWalletData 
  type: object
  required:
    - passphrase
  properties: 
    passphrase:
      <<: *walletPassphrase
      description: The wallet's master passphrase.

Acceptance Criteria

Within the list of criteria below, a ticked criterion indicates that there is test coverage in the master branch to demonstrate that this criterion is satisfied.

The API must provide a simple operation to migrate funds from a Byron wallet to a new-style wallet (the migrateByronWallet operation).
If the user erroneously specifies a Shelly wallet ID as the source wallet when calling migrateByronWallet, the operation must fail with an error. The error should include a user-friendly explanation informing them that they have specified the wrong type of wallet as a source wallet.
If the user erroneously specifies a Byron wallet ID as the target wallet when calling migrateByronWallet, the operation must fail with an error. The error should include a user-friendly explanation informing them that they have specified the wrong type of wallet as a target wallet.
The actual cost incurred by the migration must exactly match the cost returned by the getByronWalletMigrateInfo endpoint. Therefore, the balance of the new wallet must be equal to b - c, where b was the original balance of the legacy wallet, and c is the cost returned by getByronWalletMigrateInfo.
After a successful migration, data relating to the original wallet should still be present in the database.
After a successful migration, calling getByronWallet with the migrated wallet ID should still succeed.
After a successful migration, calling getByronWallet with the migrated wallet ID must report a balance of 0.
After a successful migration, the listByronWallets operation should still include the migrated wallet in the list it returns.

PR

Number	Base
#795	master
#806	master
#871	master
#890	master
#950	master

QA

Build integration tests to verify that all acceptance criteria have been satisfied.

The text was updated successfully, but these errors were encountered:

795: Add skeletal Byron wallet API endpoints relating to migration. r=jonathanknowles a=jonathanknowles # Issue Number #778 #779 # Overview This PR adds skeletal migration-related API endpoints for Byron-style wallets. Further PRs will add actual implementations for these endpoints. Co-authored-by: Jonathan Knowles <[email protected]>

806: Add test example values for Byron API types. r=jonathanknowles a=jonathanknowles # Issue Number #778 #779 # Overview This PR adds example test values for the following Byron API types: * `ApiMigrateByronWalletData` * `ApiByronWalletMigrationInfo` Co-authored-by: Jonathan Knowles <[email protected]>

798: Stake pools changes to DB r=KtorZ a=paweljakubas # Issue Number  #712 # Overview  - [x] I have added separate SQL schema - [x] I have added DBPoolLayer - [x] I have added impl of MVar version of DBPoolLayer - [x] I have added impl of Sqlite version of DBPoolLayer # Comments  Properties to check in the next PR: 1. `putPoolProduction` of sequence of `(poolId, SlotIdN)` where `N` is incrementing in Sqlite gives the same result upon `readPoolProduction` as when MVar impl 2. `putPoolProduction` slots being in given epoch and `readPoolProduction (Just epoch)` with this epoch should give proper results 3. `readPoolProduction (Just epoch)` for any epoch always should always only give values from a given epoch 4. When we `putPoolProduction` pools and non-duplicating slots in increasing order then we should have all pool values and all slot values when `readPoolProduction Nothing`. 5. When we `putPoolProduction` pools and non-duplicating slots in increasing order then for each pool id we have slots value that are diminishing in time 6. `rollbackTo slotIdN` should not have slot values later than slotIdN when `readPoolProduction Nothing) 7. `rollbackTo slotIdN` should not affect slots not later than slotIdN 8. `readPoolProduction` should not return pools with no slots, with or without rollback  811: Provisional integration test code for migrate Byron wallets r=piotr-iohk a=piotr-iohk # Issue Number #778 #779 # Overview  - [ ] Provisional integration test code for migrate Byron wallets + small swagger.yaml update (label `Estimate Migration Cost` for endpoint that estimates cost) # Comments   Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]>

837: Change the return type of `migrateByronWallet` to be a list of transactions. r=KtorZ a=jonathanknowles # Issue Number #779 # Overview The result of migrating funds from a Byron wallet to a new-style wallet will be the creation of _one or more_ transactions. Therefore, it makes sense to report this set of transactions as the result of the `migrateByronWallet` call. This PR merely changes the return type of the call, in order to unblock testing work. Further PRs will actually implement this call. Co-authored-by: Jonathan Knowles <[email protected]>

@piotr-iohk

838: Add --listen-address=127.0.0.1 option r=KtorZ a=rvl Relates to #833 # Overview The host option follows the specification here: http://hackage.haskell.org/package/streaming-commons-0.2.1.0/docs/Data-Streaming-Network.html#t:HostPreference # Comments CLI client can still only connect to localhost. 844: Add and 'implement' "listByronTransactions" to the API r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have added a `GET /byron-wallets/{walletId}/transactions` to get the list of transactions for a byron wallet - [x] The implementation is actually the same as the one for sequential wallets, so we could consider testing done here 😅 ? @piotr-iohk ? # Comments   859: Relocate C.W.Jormungandr.BlockHeaders to C.W.Network.BlockHeaders r=KtorZ a=Anviking # Issue Number #711 # Overview - [x] Relocate Cardano.Wallet.Jormungandr.BlockHeaders to Cardano.Wallet.Network.BlockHeaders. `Cardano.Pool.Metrics` needs them! # Comments   Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

@piotr-iohk

844: Add and 'implement' "listByronTransactions" to the API r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have added a `GET /byron-wallets/{walletId}/transactions` to get the list of transactions for a byron wallet - [x] The implementation is actually the same as the one for sequential wallets, so we could consider testing done here 😅 ? @piotr-iohk ? # Comments   859: Relocate C.W.Jormungandr.BlockHeaders to C.W.Network.BlockHeaders r=KtorZ a=Anviking # Issue Number #711 # Overview - [x] Relocate Cardano.Wallet.Jormungandr.BlockHeaders to Cardano.Wallet.Network.BlockHeaders. `Cardano.Pool.Metrics` needs them! # Comments   Co-authored-by: KtorZ <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

@piotr-iohk

844: Add and 'implement' "listByronTransactions" to the API r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have added a `GET /byron-wallets/{walletId}/transactions` to get the list of transactions for a byron wallet - [x] The implementation is actually the same as the one for sequential wallets, so we could consider testing done here 😅 ? @piotr-iohk ? # Comments   859: Relocate C.W.Jormungandr.BlockHeaders to C.W.Network.BlockHeaders r=KtorZ a=Anviking # Issue Number #711 # Overview - [x] Relocate Cardano.Wallet.Jormungandr.BlockHeaders to Cardano.Wallet.Network.BlockHeaders. `Cardano.Pool.Metrics` needs them! # Comments   Co-authored-by: KtorZ <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

@piotr-iohk

844: Add and 'implement' "listByronTransactions" to the API r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have added a `GET /byron-wallets/{walletId}/transactions` to get the list of transactions for a byron wallet - [x] The implementation is actually the same as the one for sequential wallets, so we could consider testing done here 😅 ? @piotr-iohk ? # Comments   Co-authored-by: KtorZ <[email protected]>

@jonathanknowles

871: Special coin selection function for handling migration of legacy wallets r=jonathanknowles a=KtorZ # Issue Number  #778 #779 # Overview  - [x] I have added a function to build a coin selection from an existing UTxO. The function yields multiple coin selections. Making transaction corresponding to these selection should empty the wallet. - [x] @jonathanknowles has implemented a series of property tests to check the correctness of the function. # Comments  I've measured how "good" the migration algorithm is by comparing the sum of all funds migrated (without taking fee into account) to the initial UTxO value. So for instance `99%` means that 99% of the wallet's value was selected for migration and 1% is left behind and won't be migrated. This has been tried for various dust ratio (1% of your wallet is dust, 5% is dust, 25% is dust etc...) and, with realistic values for the fee policy. (see https://github.com/input-output-hk/cardano-wallet/pull/871/files#diff-d067d3d9dbdfa81e932cc6fee6a9748cR66-R93) Then, I've tried using different approaches with regards to the way we arrange the UTxO set in the migration function: - The default one which just uses the UTxO in an arbitrary order - Sorting the UTxO with biggest coins first - Interleaving small and big coins Aaaaaand.. despite our intuition, here are the results: ``` Arbitrary Order Biggest UTxO first Interleaving of big and small dust=1% dust=1% dust=1% 93.5% PERFECT (== 100%) 46.5% OKAY (> 99%) 93.8% PERFECT (== 100%) 4.4% GREAT (> 99%) 43.5% PERFECT (== 100%) 4.9% OKAY (> 99%) 2.1% MEDIOCRE (<= 99%) 9.9% MEDIOCRE (<= 99%) 1.3% MEDIOCRE (<= 99%) dust=5% dust=5% dust=5% 93.3% PERFECT (== 100%) 58.7% OKAY (> 99%) 93.6% PERFECT (== 100%) 4.4% GREAT (> 99%) 32.9% PERFECT (== 100%) 5.0% OKAY (> 99%) 2.3% MEDIOCRE (<= 99%) 8.4% MEDIOCRE (<= 99%) 1.4% MEDIOCRE (<= 99%) dust=10% dust=10% dust=10% 92.8% PERFECT (== 100%) 66.4% OKAY (> 99%) 93.1% PERFECT (== 100%) 4.6% GREAT (> 99%) 24.1% PERFECT (== 100%) 5.1% OKAY (> 99%) 2.6% MEDIOCRE (<= 99%) 9.5% MEDIOCRE (<= 99%) 1.8% MEDIOCRE (<= 99%) dust=25% dust=25% dust=25% 91.5% PERFECT (== 100%) 75.0% OKAY (> 99%) 89.9% PERFECT (== 100%) 5.0% GREAT (> 99%) 14.4% PERFECT (== 100%) 6.1% OKAY (> 99%) 3.5% MEDIOCRE (<= 99%) 10.6% MEDIOCRE (<= 99%) 4.0% MEDIOCRE (<= 99%) dust=50% dust=50% dust=50% 86.9% PERFECT (== 100%) 74.4% OKAY (> 99%) 38.0% OKAY (> 99%) 8.2% MEDIOCRE (<= 99%) 14.8% MEDIOCRE (<= 99%) 34.7% PERFECT (== 100%) 4.9% GREAT (> 99%) 10.8% PERFECT (== 100%) 27.3% MEDIOCRE (<= 99%) ``` Sooooo... It seems that sometimes, doing less is better 😄 ... And it's already quite sufficient do not do anything and leave the UTxO in an arbitrary order). Now, if I play a bit more with the value for the `dust`, and use as a `dust` value, the constant part for the fees (the `b` in `a * x + b` in the fee equation), we end up with: ``` dust=1% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=5% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=10% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=25% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=50% +++ OK, passed 1000 tests: 99.6% PERFECT (== 100%) 0.4% OKAY (> 99%) ``` Which clearly gives us some indication about what a good value for the `dustThreshold` should be 😄 > ℹ️ It would be interesting to take the price of the migration into account too in the comparison. Perhaps one selects less coins, but, also generate less transactions which results in more money being transferred 🤔 ?  Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: KtorZ <[email protected]>

@jonathanknowles

871: Special coin selection function for handling migration of legacy wallets r=KtorZ a=KtorZ # Issue Number  #778 #779 # Overview  - [x] I have added a function to build a coin selection from an existing UTxO. The function yields multiple coin selections. Making transaction corresponding to these selection should empty the wallet. - [x] @jonathanknowles has implemented a series of property tests to check the correctness of the function. # Comments  I've measured how "good" the migration algorithm is by comparing the sum of all funds migrated (without taking fee into account) to the initial UTxO value. So for instance `99%` means that 99% of the wallet's value was selected for migration and 1% is left behind and won't be migrated. This has been tried for various dust ratio (1% of your wallet is dust, 5% is dust, 25% is dust etc...) and, with realistic values for the fee policy. (see https://github.com/input-output-hk/cardano-wallet/pull/871/files#diff-d067d3d9dbdfa81e932cc6fee6a9748cR66-R93) Then, I've tried using different approaches with regards to the way we arrange the UTxO set in the migration function: - The default one which just uses the UTxO in an arbitrary order - Sorting the UTxO with biggest coins first - Interleaving small and big coins Aaaaaand.. despite our intuition, here are the results: ``` Arbitrary Order Biggest UTxO first Interleaving of big and small dust=1% dust=1% dust=1% 93.5% PERFECT (== 100%) 46.5% OKAY (> 99%) 93.8% PERFECT (== 100%) 4.4% GREAT (> 99%) 43.5% PERFECT (== 100%) 4.9% OKAY (> 99%) 2.1% MEDIOCRE (<= 99%) 9.9% MEDIOCRE (<= 99%) 1.3% MEDIOCRE (<= 99%) dust=5% dust=5% dust=5% 93.3% PERFECT (== 100%) 58.7% OKAY (> 99%) 93.6% PERFECT (== 100%) 4.4% GREAT (> 99%) 32.9% PERFECT (== 100%) 5.0% OKAY (> 99%) 2.3% MEDIOCRE (<= 99%) 8.4% MEDIOCRE (<= 99%) 1.4% MEDIOCRE (<= 99%) dust=10% dust=10% dust=10% 92.8% PERFECT (== 100%) 66.4% OKAY (> 99%) 93.1% PERFECT (== 100%) 4.6% GREAT (> 99%) 24.1% PERFECT (== 100%) 5.1% OKAY (> 99%) 2.6% MEDIOCRE (<= 99%) 9.5% MEDIOCRE (<= 99%) 1.8% MEDIOCRE (<= 99%) dust=25% dust=25% dust=25% 91.5% PERFECT (== 100%) 75.0% OKAY (> 99%) 89.9% PERFECT (== 100%) 5.0% GREAT (> 99%) 14.4% PERFECT (== 100%) 6.1% OKAY (> 99%) 3.5% MEDIOCRE (<= 99%) 10.6% MEDIOCRE (<= 99%) 4.0% MEDIOCRE (<= 99%) dust=50% dust=50% dust=50% 86.9% PERFECT (== 100%) 74.4% OKAY (> 99%) 38.0% OKAY (> 99%) 8.2% MEDIOCRE (<= 99%) 14.8% MEDIOCRE (<= 99%) 34.7% PERFECT (== 100%) 4.9% GREAT (> 99%) 10.8% PERFECT (== 100%) 27.3% MEDIOCRE (<= 99%) ``` Sooooo... It seems that sometimes, doing less is better 😄 ... And it's already quite sufficient do not do anything and leave the UTxO in an arbitrary order). Now, if I play a bit more with the value for the `dust`, and use as a `dust` value, the constant part for the fees (the `b` in `a * x + b` in the fee equation), we end up with: ``` dust=1% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=5% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=10% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=25% +++ OK, passed 1000 tests (100.0% PERFECT (== 100%)). dust=50% +++ OK, passed 1000 tests: 99.6% PERFECT (== 100%) 0.4% OKAY (> 99%) ``` Which clearly gives us some indication about what a good value for the `dustThreshold` should be 😄 > ℹ️ It would be interesting to take the price of the migration into account too in the comparison. Perhaps one selects less coins, but, also generate less transactions which results in more money being transferred 🤔 ?  Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: KtorZ <[email protected]>

@disassembler

862: jormungandr launch: use --rest-listen instead of generating config.yaml r=KtorZ a=rvl Relates to #832 and #848. Supersedes #850. # Overview `cardano-wallet-jormungandr launch` specifies the REST API port and storage directory. The user provides the rest of the Jörmungandr configuration (e.g. trusted peers). # Comments The Jörmungandr P2P listen address, port, and log level are no longer configured by cardano-wallet. 864: nix: Provide derivations for Daedalus installer r=KtorZ a=rvl Relates to #863. Based on #828. # Overview - @disassembler @cleverca22 It's not exactly the same as before but should work ok I think. - Adds source filtering to avoid unnecessary rebuilds. # Comments To build: ``` nix-build -A cardano-wallet-jormungandr nix-build release.nix -A daedalus-jormungandr.windows -o daedalus-jormungandr-windows nix-build release.nix -A daedalus-jormungandr.macos -o daedalus-jormungandr-macos nix-build release.nix -A daedalus-jormungandr.linux -o daedalus-jormungandr-linux ``` Note that `daedalus-jormungandr.{windows,macos,linux}` from `release.nix` reference the same `cardano-wallet-jormungandr` derivation, only with different `system` or `crossSystem` arguments. So Daedalus may also import from `default.nix` rather than `release.nix`. 878: Support for legacy UTxO witness in Jörmungandr r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have extended `mkStdTx` so that it would properly handle transactions coming from a `RndKey` and construct `legacy-utxo` witnesses for it. # Comments  Jörmungandr doesn't implement creating legacy utxo witnesses from jcli ... [jcli/src/jcli_app/transaction/mk_witness.rs](https://github.com/input-output-hk/jormungandr/blob/master/jcli/src/jcli_app/transaction/mk_witness.rs#L78-L82) ```rust WitnessType::OldUTxO => { // TODO unimplemented!() let _secret_key: SecretKey<Ed25519Bip32> = self.secret()?; Err(Error::MakeWitnessLegacyUtxoUnsupported)?; unimplemented!() } ``` so I had to construct them by hand according to: - The format defined in [chain-impl-mockchain#witnesses](https://github.com/input-output-hk/chain-libs/blob/incentive/chain-impl-mockchain/doc/format.md#witnesses) - The format from the source (double checking it matches the doc..) [chain-impl-mockchain/src/transaction/witness.rs](https://github.com/input-output-hk/chain-libs/blob/incentive/chain-impl-mockchain/src/transaction/witness.rs#L173-L176) It'd be nice to review our golden if / when the constructor for the witness type gets updated.  879: Porting the rest of forget pending tx integration tests to CLI r=KtorZ a=paweljakubas # Issue Number  #836 # Overview  - [x] I have ported the rest of forgetting pending tx integration tests to CLI # Comments   Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]>

@disassembler

862: jormungandr launch: use --rest-listen instead of generating config.yaml r=KtorZ a=rvl Relates to #832 and #848. Supersedes #850. # Overview `cardano-wallet-jormungandr launch` specifies the REST API port and storage directory. The user provides the rest of the Jörmungandr configuration (e.g. trusted peers). # Comments The Jörmungandr P2P listen address, port, and log level are no longer configured by cardano-wallet. 864: nix: Provide derivations for Daedalus installer r=KtorZ a=rvl Relates to #863. Based on #828. # Overview - @disassembler @cleverca22 It's not exactly the same as before but should work ok I think. - Adds source filtering to avoid unnecessary rebuilds. # Comments To build: ``` nix-build -A cardano-wallet-jormungandr nix-build release.nix -A daedalus-jormungandr.windows -o daedalus-jormungandr-windows nix-build release.nix -A daedalus-jormungandr.macos -o daedalus-jormungandr-macos nix-build release.nix -A daedalus-jormungandr.linux -o daedalus-jormungandr-linux ``` Note that `daedalus-jormungandr.{windows,macos,linux}` from `release.nix` reference the same `cardano-wallet-jormungandr` derivation, only with different `system` or `crossSystem` arguments. So Daedalus may also import from `default.nix` rather than `release.nix`. 878: Support for legacy UTxO witness in Jörmungandr r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have extended `mkStdTx` so that it would properly handle transactions coming from a `RndKey` and construct `legacy-utxo` witnesses for it. # Comments  Jörmungandr doesn't implement creating legacy utxo witnesses from jcli ... [jcli/src/jcli_app/transaction/mk_witness.rs](https://github.com/input-output-hk/jormungandr/blob/master/jcli/src/jcli_app/transaction/mk_witness.rs#L78-L82) ```rust WitnessType::OldUTxO => { // TODO unimplemented!() let _secret_key: SecretKey<Ed25519Bip32> = self.secret()?; Err(Error::MakeWitnessLegacyUtxoUnsupported)?; unimplemented!() } ``` so I had to construct them by hand according to: - The format defined in [chain-impl-mockchain#witnesses](https://github.com/input-output-hk/chain-libs/blob/incentive/chain-impl-mockchain/doc/format.md#witnesses) - The format from the source (double checking it matches the doc..) [chain-impl-mockchain/src/transaction/witness.rs](https://github.com/input-output-hk/chain-libs/blob/incentive/chain-impl-mockchain/src/transaction/witness.rs#L173-L176) It'd be nice to review our golden if / when the constructor for the witness type gets updated.  879: Porting the rest of forget pending tx integration tests to CLI r=KtorZ a=paweljakubas # Issue Number  #836 # Overview  - [x] I have ported the rest of forgetting pending tx integration tests to CLI # Comments   Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]>

878: Support for legacy UTxO witness in Jörmungandr r=KtorZ a=KtorZ # Issue Number  #779 # Overview  - [x] I have extended `mkStdTx` so that it would properly handle transactions coming from a `RndKey` and construct `legacy-utxo` witnesses for it. # Comments  Jörmungandr doesn't implement creating legacy utxo witnesses from jcli ... [jcli/src/jcli_app/transaction/mk_witness.rs](https://github.com/input-output-hk/jormungandr/blob/master/jcli/src/jcli_app/transaction/mk_witness.rs#L78-L82) ```rust WitnessType::OldUTxO => { // TODO unimplemented!() let _secret_key: SecretKey<Ed25519Bip32> = self.secret()?; Err(Error::MakeWitnessLegacyUtxoUnsupported)?; unimplemented!() } ``` so I had to construct them by hand according to: - The format defined in [chain-impl-mockchain#witnesses](https://github.com/input-output-hk/chain-libs/blob/incentive/chain-impl-mockchain/doc/format.md#witnesses) - The format from the source (double checking it matches the doc..) [chain-impl-mockchain/src/transaction/witness.rs](https://github.com/input-output-hk/chain-libs/blob/incentive/chain-impl-mockchain/src/transaction/witness.rs#L173-L176) It'd be nice to review our golden if / when the constructor for the witness type gets updated.  Co-authored-by: KtorZ <[email protected]>

882: Add handlers for Byron migration r=KtorZ a=paweljakubas # Issue Number #778 (`migrateByronWallet`) #779 (`getByronWalletMigrationInfo`) # Overview This PR: - [x] Adds a handler for `getByronWalletMigrationInfo`. - [x] Adds a handler for `migrateByronWallet`. # Not included in this PR (reserved for future PRs) - More user-friendly error handling for the case where a user attempts to migrate _**from**_ a known Shelley wallet, or _**to**_ a known Byron wallet. (We can only migrate from Byron to Shelley.) - A complete set of integration tests. We can add these in a further PR.   884: More integration tests for Byron wallets r=KtorZ a=piotr-iohk # Issue Number  # Overview  - [ ] More integration tests for Byron wallets ensuring that Shelley endpoints/CLI cannot manage them # Comments   Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]>

882: Add handlers for Byron migration r=KtorZ a=paweljakubas # Issue Number #778 (`migrateByronWallet`) #779 (`getByronWalletMigrationInfo`) # Overview This PR: - [x] Adds a handler for `getByronWalletMigrationInfo`. - [x] Adds a handler for `migrateByronWallet`. # Not included in this PR (reserved for future PRs) - More user-friendly error handling for the case where a user attempts to migrate _**from**_ a known Shelley wallet, or _**to**_ a known Byron wallet. (We can only migrate from Byron to Shelley.) - A complete set of integration tests. We can add these in a further PR.   Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Jonathan Knowles <[email protected]>

849: Enable faucet random wallets r=piotr-iohk a=KtorZ # Issue Number  #779 # Overview  - [x] I have added faucet random wallets for Jörmungandr - [x] Faucet wallets can be restored via `fixtureByronWallet`, although, we can't make transactions with them. # Comments  :warning: Somehow, I can't generate the genesis block from the genesis configuration, jcli isn't happy with the `legacy_fund`, am I missing something?  Co-authored-by: KtorZ <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]>

916: Byron migration integration tests r=KtorZ a=jonathanknowles # Issue Number #778 #779 # Overview This PR adds integration tests to verify that: - [x] Calculating the migration cost of an empty wallet (with [`getByronWalletMigrateInfo`](#778)) returns a value of `0`. - [x] Migrating an empty wallet (with the [`migrateByronWallet`](#779) endpoint) should not generate any transactions. - [x] The actual cost incurred by a migration (with the [`migrateByronWallet`](#779) endpoint) **exactly** matches the cost returned by the [`getByronWalletMigrateInfo`](#778) endpoint. # Comments   Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: KtorZ <[email protected]>

950: Test that `migrateByronWallet` depletes all funds from a source wallet. r=KtorZ a=jonathanknowles # Issue Number #779 # Overview This PR tests that after a successful call to `migrateByronWallet`, all funds are debited from the source wallet, leaving a balance of `0`. # Further work A further PR will test that funds are appropriately credited to the target wallet. Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

950: Test that `migrateByronWallet` depletes all funds from a source wallet. r=KtorZ a=jonathanknowles # Issue Number #779 # Overview This PR tests that after a successful call to `migrateByronWallet`, all funds are debited from the source wallet, leaving a balance of `0`. # Further work A further PR will test that funds are appropriately credited to the target wallet. Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: KtorZ <[email protected]>

950: Test that `migrateByronWallet` migrates funds correctly. r=jonathanknowles a=jonathanknowles # Issue Number #779 # Overview This PR tests that after a successful call to `migrateByronWallet`: - [x] all funds are debited from the source wallet, leaving a balance of `0`. - [x] all funds are eventually credited to the target wallet, minus the fee. - [x] the resultant fee is the same as the fee predicted by `getByronWalletMigrateInfo`. Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: KtorZ <[email protected]>

piotr-iohk · 2019-11-06T11:25:08Z

I have reviewed integration tests, they are quite nice.

I would like to add few more mostly around negative cases (BYRON_MIGRATE and BYRON_CALCULATE in the file): https://docs.google.com/spreadsheets/d/1fadIA_j4nCd3FbylPo5J8K9Fy6tixSC2T9V3xHKuUvU/edit#gid=0
I don't see many tests for caluclating migration cost too. (they are touched in the BYRON_MIGRATE suite)

Also testing of of big wallet with many transactions and fragmented utxo from genesis file would be beneficial.

Also response codes might be not complete for byron migrate (https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/migrateByronWallet) I think 415 might be missing.

966: More tests for byron wallets migration, in particular migrating from big wallets r=piotr-iohk a=KtorZ # Issue Number  #779 # Overview  - [x] I have added an extra integration scenario to test migrating a big wallet with a lot of dust - [x] I have revised byron migrate response behavior when trying to migrate an empty wallet to return a 403. Two reasons for this: this is more semantically correct. It prevents some extra code needed to handle possible errors that could have occurred if we indeed tried migrating (like, an invalid passphrase). - [x] enable a currently pending test in BYRON_TX_LIST which required faucet - [x] Added extra negative scenario to check migrating with a wrong passphrase - [x] Revised endpoints status in swagger.yaml + added missing error code in migration response # Comments   Co-authored-by: KtorZ <[email protected]>

jonathanknowles · 2019-11-07T02:27:31Z

@piotr-iohk wrote:

I don't see many tests for calculating migration cost too.

Good point. It's true that we don't have many tests to trigger a migration cost calculation in the integration layer.

Though have you seen the property tests for migration coin selection?

The GetByronWalletMigrationInfo endpoint is basically a wrapper around the selectCoinsForMigration function.

Can you see any deficiencies in the set of property tests above? If so, perhaps we can flesh them out a bit more.

973: Make large numbers more readable. r=Anviking a=jonathanknowles # Issue Number Related to #779. # Overview Just a tiny PR to make some large numbers more readable. Co-authored-by: Jonathan Knowles <[email protected]>

piotr-iohk · 2019-11-08T17:25:53Z

@jonathanknowles I think property tests look good. I have added a few more integration tests in #989.

989: re-organize and add few new integration tests for byron calculate/migrate r=KtorZ a=piotr-iohk # Issue Number #779 # Overview  - [ ] I have re-organized a bit and added few new integration test cases for byron migration/fee calculation to match with https://docs.google.com/spreadsheets/d/1fadIA_j4nCd3FbylPo5J8K9Fy6tixSC2T9V3xHKuUvU # Comments   Co-authored-by: Piotr Stachyra <[email protected]>

piotr-iohk · 2019-11-12T10:09:39Z

I have added yet additional tests in #1008 and an issue #1007 that actually was inspired by this comment #989 (comment).

Closing this one.

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  - [ ] Added additional tests for attempting to migrate and calculate migration cost for dust wallets. Those special wallets added to `genesis.yaml` # Comments   Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]>

916: Byron migration integration tests r=KtorZ a=jonathanknowles # Issue Number #778 #779 # Overview This PR adds integration tests to verify that: - [x] Calculating the migration cost of an empty wallet (with [`getByronWalletMigrateInfo`](cardano-foundation/cardano-wallet#778)) returns a value of `0`. - [x] Migrating an empty wallet (with the [`migrateByronWallet`](cardano-foundation/cardano-wallet#779) endpoint) should not generate any transactions. - [x] The actual cost incurred by a migration (with the [`migrateByronWallet`](cardano-foundation/cardano-wallet#779) endpoint) **exactly** matches the cost returned by the [`getByronWalletMigrateInfo`](cardano-foundation/cardano-wallet#778) endpoint. # Comments   Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: KtorZ <[email protected]>

jonathanknowles added this to the Legacy Wallet Support milestone Oct 1, 2019

jonathanknowles changed the title ~~Add API endpoint to migrate funds from a legacy wallet to a new wallet.~~ Add API endpoint to migrate funds from a legacy wallet. Oct 1, 2019

jonathanknowles self-assigned this Oct 4, 2019

jonathanknowles mentioned this issue Oct 4, 2019

Add skeletal Byron wallet API endpoints relating to migration. #795

Merged

jonathanknowles mentioned this issue Oct 10, 2019

Add test example values for Byron API types. #806

Merged

jonathanknowles mentioned this issue Oct 11, 2019

Fork API server context into random and sequential variants. #808

Closed

6 tasks

piotr-iohk mentioned this issue Oct 11, 2019

Provisional integration test code for migrate Byron wallets #811

Merged

1 task

jonathanknowles mentioned this issue Oct 16, 2019

Change the return type of migrateByronWallet to be a list of transactions. #837

Merged

This was referenced Oct 16, 2019

Add and 'implement' "listByronTransactions" to the API #844

Merged

Enable faucet random wallets #849

Merged

jonathanknowles mentioned this issue Oct 17, 2019

Add API endpoint to calculate the cost of migrating from a legacy wallet. #778

Closed

2 tasks

This was referenced Oct 18, 2019

Special coin selection function for handling migration of legacy wallets #871

Merged

Support for legacy UTxO witness in Jörmungandr #878

Merged

jonathanknowles assigned paweljakubas Oct 23, 2019

This was referenced Oct 23, 2019

Add handlers for Byron migration #882

Merged

Add deleteByronTransaction endpoint. #890

Merged

jonathanknowles mentioned this issue Oct 25, 2019

Byron migration integration tests #916

Merged

3 tasks

KtorZ mentioned this issue Oct 29, 2019

Add Jörmungandr binary decoders for legacy funds #934

Closed

2 tasks

This was referenced Nov 4, 2019

Test that migrateByronWallet migrates funds correctly. #950

Merged

Test that migrateByronWallet appropriately deposits funds into a target wallet. #960

Merged

KtorZ mentioned this issue Nov 6, 2019

More tests for byron wallets migration, in particular migrating from big wallets #966

Merged

5 tasks

jonathanknowles mentioned this issue Nov 7, 2019

Make large numbers more readable. #973

Merged

piotr-iohk mentioned this issue Nov 8, 2019

re-organize and add few new integration tests for byron calculate/migrate #989

Merged

1 task

piotr-iohk mentioned this issue Nov 12, 2019

Additional tests for migrating and calculating migration cost for wallets containing only dust #1008

Merged

1 task

piotr-iohk closed this as completed Nov 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API endpoint to migrate funds from a legacy wallet. #779

Add API endpoint to migrate funds from a legacy wallet. #779

jonathanknowles commented Oct 1, 2019 •

edited

Loading

piotr-iohk commented Nov 6, 2019 •

edited

Loading

jonathanknowles commented Nov 7, 2019 •

edited

Loading

piotr-iohk commented Nov 8, 2019

piotr-iohk commented Nov 12, 2019

Add API endpoint to migrate funds from a legacy wallet. #779

Add API endpoint to migrate funds from a legacy wallet. #779

Comments

jonathanknowles commented Oct 1, 2019 • edited Loading

Context

Usage

Decision

Acceptance Criteria

PR

QA

piotr-iohk commented Nov 6, 2019 • edited Loading

jonathanknowles commented Nov 7, 2019 • edited Loading

piotr-iohk commented Nov 8, 2019

piotr-iohk commented Nov 12, 2019

jonathanknowles commented Oct 1, 2019 •

edited

Loading

piotr-iohk commented Nov 6, 2019 •

edited

Loading

jonathanknowles commented Nov 7, 2019 •

edited

Loading