Make `createMigrationPlan` accept a list of addresses #2637

jonathanknowles · 2021-05-03T07:18:08Z

Issue Number

Overview

This PR adjusts the following API operations to accept a non-empty list of addresses:

createByronWalletMigrationPlan
createShelleyWalletMigrationPlan

These operations need addresses in order to construct the coin selections required for a migration plan, as every coin selection output must have an address, and these can only be provided by the caller.

If the caller supplies n addresses, but the migration planner needs to create m outputs where m > n, then the server implementations of the above endpoints will reuse the given addresses in a cyclic fashion. (This was already the case in the old migration implementation.)

The migration planning endpoint will need to accept some data: the set of addresses to which the wallet balance should be migrated. Without a set of addresses, it's impossible to construct the coin selections required for a migration plan, as every coin selection output must have an address: these can only be provided by the caller. However, the "migrateByronWallet" and "migrateShelleyWallet" operations at the "migrations" endpoint already accept POST data. We need a way to discriminate between the following operations: 1. Creating a plan. 2. Creating a plan and executing it immediately. Assuming both types of operation require data to be POSTed, this means we need different endpoint paths.

This family of API operations will eventually change from HTTP GET to HTTP POST, as each operation requires a list of addresses to be sent within the request body. Therefore, it no longer makes sense to use the `get` prefix. Additionally, since the result returned is an entire migration plan and not just a few pieces of information, we might as well change the "Info" suffix to a "Plan" suffix. (This naming is consistent with the already-established concept of creating a migration plan.)

This change adjusts each of the following API operations to require a non-empty list of addresses as an HTTP POST request body parameter: - `createByronWalletMigrationPlan` - `createShelleyWalletMigrationPlan`

Anviking

👌

jonathanknowles · 2021-05-03T10:46:22Z

bors r+

2637: Make `createMigrationPlan` accept a list of addresses r=jonathanknowles a=jonathanknowles # Issue Number ADP-840 # Overview This PR adjusts the following API operations to accept a non-empty list of addresses: - `createByronWalletMigrationPlan` - `createShelleyWalletMigrationPlan` These operations need addresses in order to construct the coin selections required for a migration plan, as every coin selection output must have an address, and these can only be provided by the caller. If the caller supplies **_n_** addresses, but the migration planner needs to create **_m_** outputs where **_m_** > **_n_**, then the server implementations of the above endpoints will reuse the given addresses in a cyclic fashion. (This was already the case in the old migration implementation.) Co-authored-by: Jonathan Knowles <[email protected]>

iohk-bors · 2021-05-03T11:21:10Z

Build failed:

ci/hydra-build:required

Log extract:


  src/Test/Integration/Scenario/API/Byron/Wallets.hs:187:59: 
  1) API Specifications, BYRON_WALLETS, BYRON_RESTORE_01, GET_01, LIST_01 - Restore a wallet, random 24 words
       While verifying (Status {statusCode = 200, statusMessage = "OK"},Right (ApiByronWallet {id = ApiT {getApiT = WalletId {getWalletId = 4d677c26b0a9888a750258f1f30fc6641841f697}}, balance = ApiByronWalletBalance {available = Quantity {getQuantity = 0}, total = Quantity {getQuantity = 0}}, assets = ApiWalletAssetsBalance {available = ApiT {getApiT = TokenMap (fromList [])}, total = ApiT {getApiT = TokenMap (fromList [])}}, discovery = DiscoveryRandom, name = ApiT {getApiT = WalletName {getWalletName = "Empty Byron Wallet"}}, passphrase = Just (ApiWalletPassphraseInfo {lastUpdatedAt = 2021-05-03 11:00:55.567681503 UTC}), state = ApiT {getApiT = Syncing (Quantity {getQuantity = Percentage {getPercentage = 0 % 1}})}, tip = ApiBlockReference {absoluteSlotNumber = ApiT {getApiT = SlotNo 0}, slotId = ApiSlotId {epochNumber = ApiT {getApiT = EpochNo {unEpochNo = 0}}, slotNumber = ApiT {getApiT = SlotInEpoch {unSlotInEpoch = 0}}}, time = 2021-05-03 10:53:16 UTC, block = ApiBlockInfo {height = Quantity {getQuantity = 0}}}}))
       Waited longer than 90s to resolve action: "wallet is available and ready".
       expected: Ready
        but got: Syncing (Quantity {getQuantity = Percentage {getPercentage = 0 % 1}})

  To rerun use: --match "/API Specifications/BYRON_WALLETS/BYRON_RESTORE_01, GET_01, LIST_01 - Restore a wallet/random 24 words/"

Randomized with seed 228433057

#2565

jonathanknowles · 2021-05-03T11:44:59Z

bors r+

2637: Make `createMigrationPlan` accept a list of addresses r=jonathanknowles a=jonathanknowles # Issue Number ADP-840 # Overview This PR adjusts the following API operations to accept a non-empty list of addresses: - `createByronWalletMigrationPlan` - `createShelleyWalletMigrationPlan` These operations need addresses in order to construct the coin selections required for a migration plan, as every coin selection output must have an address, and these can only be provided by the caller. If the caller supplies **_n_** addresses, but the migration planner needs to create **_m_** outputs where **_m_** > **_n_**, then the server implementations of the above endpoints will reuse the given addresses in a cyclic fashion. (This was already the case in the old migration implementation.) Co-authored-by: Jonathan Knowles <[email protected]>

iohk-bors · 2021-05-03T12:13:12Z

Build failed:

ci/hydra-build:required

Timed out here:

    sparseCheckpoints
    k=2160, h=42
    k=2160, h=2414
    k=2160, h=2414
    The tip is always a checkpoint
      +++ OK, passed 100 tests.
    There's at least (min h edgeSize) checkpoints
      +++ OK, passed 100 tests.
    ∀ cfg. sparseCheckpoints (cfg { edgeSize = 0 }) ⊆ sparseCheckpoints cfg
      +++ OK, passed 100 tests.
    Checkpoints are eventually stored in a sparse manner
      +++ OK, passed 100 tests; 504 discarded.
Cardano.Wallet.DB.Sqlite.Types
Values can be persisted and unpersisted successfully
  can persist and unpersist values of type 'SlotNo'
    +++ OK, passed 100 tests.
  can persist and unpersist values of type 'NominalDiffTime'
    +++ OK, passed 100 tests.
  can persist and unpersist values of type 'TokenQuantity'
    +++ OK, passed 100 tests.
Coverage checks for generators
  TokenQuantity
building of '/nix/store/pn8q7cxcxf4wkz7sw73xw8p3ql8qdj7s-cardano-wallet-core-test-unit-2021.4.28-check' timed out after 900 seconds of silence

Looks very much like:

#2436

jonathanknowles · 2021-05-03T12:14:18Z

bors r+

2637: Make `createMigrationPlan` accept a list of addresses r=jonathanknowles a=jonathanknowles # Issue Number ADP-840 # Overview This PR adjusts the following API operations to accept a non-empty list of addresses: - `createByronWalletMigrationPlan` - `createShelleyWalletMigrationPlan` These operations need addresses in order to construct the coin selections required for a migration plan, as every coin selection output must have an address, and these can only be provided by the caller. If the caller supplies **_n_** addresses, but the migration planner needs to create **_m_** outputs where **_m_** > **_n_**, then the server implementations of the above endpoints will reuse the given addresses in a cyclic fashion. (This was already the case in the old migration implementation.) Co-authored-by: Jonathan Knowles <[email protected]>

iohk-bors · 2021-05-03T14:40:14Z

Build failed:

ci/hydra-build:required

Anviking · 2021-05-03T14:44:20Z

bors r+

iohk-bors · 2021-05-03T15:29:19Z

Build succeeded:

2638: Update e2e tests with migration plan endpoint r=rvl a=piotr-iohk # Issue Number ADP-840 # Overview  - [ ] Updated e2e tests to comply with changes from #2637 # Comments In #2637 migration cost ep was removed in favor of migrations/plan. Updating tests to comply with this change. 2641: update migration-tests.nix r=rvl a=piotr-iohk # Issue Number  # Overview  - [ ] I have updated migration-tests.nix... # Comments ... Co-authored-by: Piotr Stachyra <[email protected]>

2638: Update e2e tests with migration plan endpoint r=rvl a=piotr-iohk # Issue Number ADP-840 # Overview  - [ ] Updated e2e tests to comply with changes from #2637 # Comments In #2637 migration cost ep was removed in favor of migrations/plan. Updating tests to comply with this change. Co-authored-by: Piotr Stachyra <[email protected]>

2638: Update e2e tests with migration plan endpoint r=piotr-iohk a=piotr-iohk # Issue Number ADP-840 # Overview  - [ ] Updated e2e tests to comply with changes from #2637 # Comments In #2637 migration cost ep was removed in favor of migrations/plan. Updating tests to comply with this change. Co-authored-by: Piotr Stachyra <[email protected]>

jonathanknowles added 4 commits May 3, 2021 03:38

[Formatting] Fix migration-related formatting in Api.Types.

efdd786

Adjust createMigrationPlan operations to require address lists.

07aa380

This change adjusts each of the following API operations to require a non-empty list of addresses as an HTTP POST request body parameter: - `createByronWalletMigrationPlan` - `createShelleyWalletMigrationPlan`

jonathanknowles self-assigned this May 3, 2021

jonathanknowles requested a review from Anviking May 3, 2021 07:21

jonathanknowles changed the title ~~Adjust createMigrationPlan operations to accept a list of addresses~~ Adjust createMigrationPlan to accept a list of addresses May 3, 2021

jonathanknowles changed the title ~~Adjust createMigrationPlan to accept a list of addresses~~ Make createMigrationPlan accept a list of addresses May 3, 2021

Apply hlint suggestion (data to newtype).

96efb30

Anviking approved these changes May 3, 2021

View reviewed changes

iohk-bors bot merged commit 9101a8e into master May 3, 2021

iohk-bors bot deleted the jonathanknowles/multi-asset-migration-api-tweaks branch May 3, 2021 15:29

piotr-iohk mentioned this pull request May 4, 2021

Update e2e tests with migration plan endpoint #2638

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `createMigrationPlan` accept a list of addresses #2637

Make `createMigrationPlan` accept a list of addresses #2637

jonathanknowles commented May 3, 2021 •

edited

Loading

Anviking left a comment

jonathanknowles commented May 3, 2021

iohk-bors bot commented May 3, 2021 •

edited by jonathanknowles

Loading

jonathanknowles commented May 3, 2021

iohk-bors bot commented May 3, 2021 •

edited by jonathanknowles

Loading

jonathanknowles commented May 3, 2021

iohk-bors bot commented May 3, 2021

Anviking commented May 3, 2021

iohk-bors bot commented May 3, 2021

Make createMigrationPlan accept a list of addresses #2637

Make createMigrationPlan accept a list of addresses #2637

Conversation

jonathanknowles commented May 3, 2021 • edited Loading

Issue Number

Overview

Anviking left a comment

Choose a reason for hiding this comment

jonathanknowles commented May 3, 2021

iohk-bors bot commented May 3, 2021 • edited by jonathanknowles Loading

jonathanknowles commented May 3, 2021

iohk-bors bot commented May 3, 2021 • edited by jonathanknowles Loading

jonathanknowles commented May 3, 2021

iohk-bors bot commented May 3, 2021

Anviking commented May 3, 2021

iohk-bors bot commented May 3, 2021

Make `createMigrationPlan` accept a list of addresses #2637

Make `createMigrationPlan` accept a list of addresses #2637

jonathanknowles commented May 3, 2021 •

edited

Loading

iohk-bors bot commented May 3, 2021 •

edited by jonathanknowles

Loading

iohk-bors bot commented May 3, 2021 •

edited by jonathanknowles

Loading