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

API spec: First revision of multi-asset changes #2446

Closed
wants to merge 5 commits into from

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Jan 14, 2021

Issue Number

ADP-603

Overview

Adjusts the OpenAPI specification to include multi-asset amounts in transactions and wallet balances.

Changes are designed to preserve API backwards compatibility, if users only care about Ada amounts.

A primary consideration was how multi-asset values would be presented and manipulated in user interfaces such as Daedalus.

Comments

@rvl rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Jan 14, 2021
@rvl rvl self-assigned this Jan 14, 2021
@rvl rvl force-pushed the rvl/adp-603/api-spec-ma branch 5 times, most recently from 80f4f08 to c53bae4 Compare January 14, 2021 07:53
@rvl rvl mentioned this pull request Jan 14, 2021
7 tasks
# fixme: (remove this note) I changed it to hex from base64, the reason is below.
format: hex
maxLength: 64
# fixme: (remove this note) multiple examples seem not to validate
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately no, OpenAPI does not support multiple examples... I think there's a Redoc extension we could use if we really wanted that, but it's a vendor-specific extension so not a pure OpenAPI primitive.

@rvl rvl marked this pull request as ready for review January 14, 2021 11:41
@rvl rvl force-pushed the rvl/adp-603/api-spec-ma branch 3 times, most recently from 9a424a6 to 2994e6e Compare January 19, 2021 12:23
@KtorZ
Copy link
Member

KtorZ commented Jan 19, 2021

@rvl I've been capturing the metadata in a confluence document: https://input-output.atlassian.net/wiki/spaces/AD/pages/2137456733/Token+Identity+Metadata

I've added a few (sensible) constraints / validations not yet present in our swagger specs (e.g. maxLength for the token's name)

@@ -236,7 +236,7 @@ x-syncClockProgress: &syncClockProgress
status: pending

x-amount: &amount
description: Coins, in Lovelace
description: Coins, in Lovelace. Only relates to 'Ada'. Refer to `assets` for multi-assets wallets instead.
Copy link
Member

Choose a reason for hiding this comment

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

Q: Can a user specify to send 0 ada, and 1 Apple asset?

I'm guessing: yes, but the coin selection will automatically send the minimum required ada along in the token bundle?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed. And also assign the minimum ada value needed for the multi asset UTxO.

@rvl
Copy link
Contributor Author

rvl commented Jan 20, 2021

Thanks @KtorZ I shall apply that json schema to the swagger.yaml.

I have one issue with the metadata schema.
Acronynm should not be a required field. For some assets it would not make sense.
We could say that if acronym is not present, then the name will be used, but UIs might truncate the text if there isn't enough space.

@KtorZ
Copy link
Member

KtorZ commented Jan 20, 2021

@rvl good point. We can still change that.

@rvl rvl force-pushed the rvl/adp-603/api-spec-ma branch from 24466c7 to cad01cd Compare January 21, 2021 11:11
iohk-bors bot added a commit that referenced this pull request Jan 25, 2021
2447: Multi-asset API extensions r=rvl a=rvl

### Issue Number

ADP-604

### Overview

Starting implementation of API spec in #2446.

- [x] listAssets endpoint
- [x] getAsset endpoint
- [x] Added `ApiWalletAssetsBalance` which provides the balance of non-ade assets.
- [x] Removed WalletBalance from Primitive.Types and replaced `ApiT WalletBalance` with `ApiWalletBalance`.
- [ ] Some kind of integration test for these endpoints.
- [x] Remove `Quantity "lovelace" a` where possible, except for the API.
- [x] TokenName inner type is now ByteString, add length validation.

### Comments
- This branch is based on top of the PR #2446 branch.



Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this pull request Jan 25, 2021
2447: Multi-asset API extensions r=Anviking a=rvl

### Issue Number

ADP-604

### Overview

Starting implementation of API spec in #2446.

- [x] listAssets endpoint
- [x] getAsset endpoint
- [x] Added `ApiWalletAssetsBalance` which provides the balance of non-ade assets.
- [x] Removed WalletBalance from Primitive.Types and replaced `ApiT WalletBalance` with `ApiWalletBalance`.
- [ ] Some kind of integration test for these endpoints.
- [x] Remove `Quantity "lovelace" a` where possible, except for the API.
- [x] TokenName inner type is now ByteString, add length validation.

### Comments
- This branch is based on top of the PR #2446 branch.



Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this pull request Jan 25, 2021
2447: Multi-asset API extensions r=Anviking a=rvl

### Issue Number

ADP-604

### Overview

Starting implementation of API spec in #2446.

- [x] listAssets endpoint
- [x] getAsset endpoint
- [x] Added `ApiWalletAssetsBalance` which provides the balance of non-ade assets.
- [x] Removed WalletBalance from Primitive.Types and replaced `ApiT WalletBalance` with `ApiWalletBalance`.
- [ ] Some kind of integration test for these endpoints.
- [x] Remove `Quantity "lovelace" a` where possible, except for the API.
- [x] TokenName inner type is now ByteString, add length validation.

### Comments
- This branch is based on top of the PR #2446 branch.



Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this pull request Jan 25, 2021
2447: Multi-asset API extensions r=Anviking a=rvl

### Issue Number

ADP-604

### Overview

Starting implementation of API spec in #2446.

- [x] listAssets endpoint
- [x] getAsset endpoint
- [x] Added `ApiWalletAssetsBalance` which provides the balance of non-ade assets.
- [x] Removed WalletBalance from Primitive.Types and replaced `ApiT WalletBalance` with `ApiWalletBalance`.
- [ ] Some kind of integration test for these endpoints.
- [x] Remove `Quantity "lovelace" a` where possible, except for the API.
- [x] TokenName inner type is now ByteString, add length validation.

### Comments
- This branch is based on top of the PR #2446 branch.



Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@KtorZ
Copy link
Member

KtorZ commented Jan 27, 2021

closing since #2447 has been merged and was already including this chunk of work.

@KtorZ KtorZ closed this Jan 27, 2021
@rvl rvl deleted the rvl/adp-603/api-spec-ma branch January 28, 2021 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants