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

Add missing APIs for multi-asset #2489

Merged
merged 10 commits into from
Feb 8, 2021
Merged

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Feb 4, 2021

Issue Number

ADP-698

Overview

  • Adds assets to the responses of the coin selections endpoints.
  • Adds assets to the /byron-wallets endpoints.

Comments

@rvl rvl self-assigned this Feb 4, 2021
@rvl rvl force-pushed the rvl/adp-698/ma-missing-apis branch from d870ffc to 0ff5654 Compare February 4, 2021 08:41
@KtorZ
Copy link
Member

KtorZ commented Feb 4, 2021

@rvl shall we also tackle this quick one at the same time: #2476 (comment)

@rvl
Copy link
Contributor Author

rvl commented Feb 4, 2021

Yes, coming up...

@@ -213,6 +215,8 @@ instance PartialOrd TokenMap where
(getAssets m1 `Set.union` getAssets m2)

instance NFData TokenMap
instance Hashable TokenMap where
hashWithSalt = hashUsing (Map.toList . unTokenMap)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't GeneralizedNewtypeDeriving or DerivingVia handle that just fine 🤔 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't seem to work - I think perhaps Map didn't have a Hashable instance.

@@ -996,6 +996,7 @@ x-transactionResolvedInputs: &transactionResolvedInputs
properties:
address: *addressId
amount: *amount
assets: *walletAssets
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

:> Capture "walletId" (ApiT WalletId)
:> "assets"
:> Capture "policyId" (ApiT TokenPolicyId)
:> Get '[JSON] ApiAsset
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@rvl rvl force-pushed the rvl/adp-698/ma-missing-apis branch from 98a29bc to 1839acd Compare February 4, 2021 12:14
@rvl rvl requested a review from piotr-iohk February 4, 2021 12:19
@rvl
Copy link
Contributor Author

rvl commented Feb 4, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 4, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 4, 2021

Build failed:

== Section test:integration ==
--
  | Redundant build-depends entry
  | * unordered-containers

#expected

@piotr-iohk
Copy link
Contributor

piotr-iohk commented Feb 4, 2021

As far as I see assets are now present on inputs and outputs on the coin selection response, but they are not on change. Should there be on change too @rvl?

Exemplary response:

{
  "inputs": [
    {
      "amount": {
        "quantity": 353684111,
        "unit": "lovelace"
      },
      "address": "addr_test1qzvw02vau2jf8vmwav657w5j9grstsx3sp4cgnqh9agnz48l0dw5r75vk42mv3ykq8vyjeaanvpytg79xqzymqy5acmqy0h2p9",
      "id": "b2c4b2725a1cbe6a46b843368f525b87d3a4e1388834a18b39f2d0db7d5c069e",
      "derivation_path": [
        "1852H",
        "1815H",
        "0H",
        "1",
        "287"
      ],
      "assets": [
        {
          "asset_name": "6861707079636f696e",
          "quantity": 4000,
          "policy_id": "789ef8ae89617f34c07f7f6a12e4d65146f958c0bc15a97b4ff169f1"
        },
        {
          "asset_name": "7375706572636f696e",
          "quantity": 4000,
          "policy_id": "789ef8ae89617f34c07f7f6a12e4d65146f958c0bc15a97b4ff169f1"
        }
      ],
      "index": 3
    }
  ],
  "deposits": [],
  "change": [
    {
      "amount": {
        "quantity": 351505014,
        "unit": "lovelace"
      },
      "address": "addr_test1qqes885m0524cgxk84awhlhp38qf9q2p9c6t99zs0smdy3ll0dw5r75vk42mv3ykq8vyjeaanvpytg79xqzymqy5acmqx4x7rl",
      "derivation_path": [
        "1852H",
        "1815H",
        "0H",
        "1",
        "288"
      ]
    }
  ],
  "outputs": [
    {
      "amount": {
        "quantity": 2000000,
        "unit": "lovelace"
      },
      "address": "37btjrVyb4KF6KtvXYUxDHqjAwfKAuDzP71UEuG6vh87rjstvB8o1Muu6KaeGAF8xZro63Zc5VAYNnQtcci4vT8jhLuNtwku9x1YgS18LHFAhxxTi8",
      "assets": [
        {
          "asset_name": "6861707079636f696e",
          "quantity": 1000,
          "policy_id": "789ef8ae89617f34c07f7f6a12e4d65146f958c0bc15a97b4ff169f1"
        },
        {
          "asset_name": "7375706572636f696e",
          "quantity": 1000,
          "policy_id": "789ef8ae89617f34c07f7f6a12e4d65146f958c0bc15a97b4ff169f1"
        }
      ]
    }
  ]
}

@rvl rvl force-pushed the rvl/adp-698/ma-missing-apis branch from 64b0955 to 46f3009 Compare February 4, 2021 15:10
@rvl
Copy link
Contributor Author

rvl commented Feb 4, 2021

Ah yes ... fixed now. I think that's the last one.

ApiTransaction doesn't have an assets field corresponding to the amount field, but that's a known limitation (ADP-683).

@rvl
Copy link
Contributor Author

rvl commented Feb 5, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 5, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



Co-authored-by: Rodney Lorrimar <[email protected]>
@rvl
Copy link
Contributor Author

rvl commented Feb 5, 2021

bors retry

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 5, 2021

Already running a review

@rvl
Copy link
Contributor Author

rvl commented Feb 5, 2021

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 5, 2021

Canceled.

(The integration tests were failing on macOS - really slowly.)

@rvl
Copy link
Contributor Author

rvl commented Feb 5, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 5, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 5, 2021

Build failed:

     TRACE /wallets/0000000000000000000000000000000000000000/statistics/utxos
     OPTIONS /wallets/0000000000000000000000000000000000000000/statistics/utxos
     PUT /wallets/0000000000000000000000000000000000000000/transactions
     PATCH /wallets/0000000000000000000000000000000000000000/transactions
     DELETE /wallets/0000000000000000000000000000000000000000/transactions
building of '/nix/store/pvlwybm8741ir5lzpqb4vwfij4b7mx76-cardano-wallet-core-test-unit-2021.1.28-check' timed out after 900 seconds of silence

#2472

@rvl rvl force-pushed the rvl/adp-698/ma-missing-apis branch from 46f3009 to b458c8c Compare February 5, 2021 11:35
@rvl
Copy link
Contributor Author

rvl commented Feb 5, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 5, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



2492: API: get and list only assets associated with the wallet r=rvl a=rvl

### Issue Number

ADP-603 / ADP-604

### Overview

- The list assets endpoint now returns all assets of the wallet, not just available assets.
- The get asset endpoint checks that the asset id really is associated with the wallet.



2493: Let tests and cluster run in pre-mary eras again r=rvl a=rvl

### Issue Number

ADP-695

### Overview

Conditionally fund the assets faucet based on era.


Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 5, 2021

Build failed (retrying...):

iohk-bors bot added a commit that referenced this pull request Feb 5, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 5, 2021

Build failed:

src/Test/Integration/Scenario/API/Shelley/Addresses.hs:96:5:
--
  | 1) API Specifications, SHELLEY_ADDRESSES, BYRON_ADDRESS_LIST - Byron wallet on Shelley ep
  | uncaught exception: ProcessHasExited
  | ProcessHasExited "cluster didn't start correctly: [Left user error (Waited too long for: pool registration),Left user error (Waited too long for: pool registration),Left user error (Waited too long for: pool registration),Left user error (Waited too long for: pool registration)]" (ExitFailure 1)
  |  
  | To rerun use: --match "/API Specifications/SHELLEY_ADDRESSES/BYRON_ADDRESS_LIST - Byron wallet on Shelley ep/"
  |  
  | 2) afterAll-hook
  | uncaught exception: ResultStatus
  | Pending Nothing (Just "exception in beforeAll-hook (see previous failure)")
  |  
  | To rerun use: --match "/afterAll-hook/"
  |  

@rvl
Copy link
Contributor Author

rvl commented Feb 6, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 6, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



2492: API: get and list only assets associated with the wallet r=rvl a=rvl

### Issue Number

ADP-603 / ADP-604

### Overview

- The list assets endpoint now returns all assets of the wallet, not just available assets.
- The get asset endpoint checks that the asset id really is associated with the wallet.



2493: Let tests and cluster run in pre-mary eras again r=rvl a=rvl

### Issue Number

ADP-695

### Overview

Conditionally fund the assets faucet based on era.


Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 6, 2021

Build failed (retrying...):

#2467

iohk-bors bot added a commit that referenced this pull request Feb 6, 2021
2489: Add missing APIs for multi-asset r=rvl a=rvl

## Issue Number

ADP-698

## Overview

- Adds assets to the responses of the coin selections endpoints.
- Adds assets to the `/byron-wallets` endpoints.

## Comments



Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 6, 2021

Build failed:

#2472

     PUT /wallets/0000000000000000000000000000000000000000/statistics/utxos
     POST /wallets/0000000000000000000000000000000000000000/statistics/utxos
     PATCH /wallets/0000000000000000000000000000000000000000/statistics/utxos
     DELETE /wallets/0000000000000000000000000000000000000000/statistics/utxos
     CONNECT /wallets/0000000000000000000000000000000000000000/statistics/utxos
     TRACE /wallets/0000000000000000000000000000000000000000/statistics/utxos
     OPTIONS /wallets/0000000000000000000000000000000000000000/statistics/utxos
     PUT /wallets/0000000000000000000000000000000000000000/transactions
     PATCH /wallets/0000000000000000000000000000000000000000/transactions
     DELETE /wallets/0000000000000000000000000000000000000000/transactions
building of '/nix/store/5qpdb8ap9d1hf1x4y8nkhwfnd73x0apc-cardano-wallet-core-test-unit-2021.1.28-check' timed out after 900 seconds of silence

@KtorZ
Copy link
Member

KtorZ commented Feb 8, 2021

bors retry

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 8, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit b301e3a into master Feb 8, 2021
@iohk-bors iohk-bors bot deleted the rvl/adp-698/ma-missing-apis branch February 8, 2021 09:53
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

Successfully merging this pull request may close these issues.

3 participants