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

Transaction metadata in swagger API spec #2087

Merged
merged 5 commits into from
Aug 31, 2020
Merged

Transaction metadata in swagger API spec #2087

merged 5 commits into from
Aug 31, 2020

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Aug 28, 2020

Issue Number

ADP-307 / #2073 / #2074

Overview

Updates the swagger spec for the new metadata field when:

  • listing transactions (metadata field is always present but may be null)
  • posting a transaction (metadata field is optional)
  • estimating fee (as above)

Comments

Rendered spec

@rvl rvl self-assigned this Aug 28, 2020
@rvl rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Aug 28, 2020
@rvl rvl added this to the (ADP-307) Transaction metadata milestone Aug 28, 2020
@rvl rvl requested review from KtorZ and jonathanknowles August 28, 2020 06:19
@rvl
Copy link
Contributor Author

rvl commented Aug 28, 2020

I found it difficult to debug swagger validation errors, so wrote this wiki doc: Swagger Development.

@rvl rvl mentioned this pull request Aug 28, 2020
@rvl
Copy link
Contributor Author

rvl commented Aug 28, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 28, 2020
2066: Bump cardano-addresses r=rvl a=Anviking

# Issue Number

Release.


# Overview

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

- [x] Bump cardano-addresses


# Comments

- I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release

```
Bump includes:
- [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53)
- [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56)
- [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58)
- [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59)
```

<!-- 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
-->


2071: Minor nix cleanups r=rvl a=rvl

### Issue Number

#2046 #2054

### Overview

- Minor cleanups of nix code for migration tests and latency benchmark.

### Comments

Tested with:
```
nix-build -A benchmarks.cardano-wallet.latency
nix-build nix/migration-tests.nix
```


2080: Also trace the times of stake distribution LSQ queries r=rvl a=Anviking

# Issue Number

#2005 / new issue

# Overview

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

- [x] Also measure and trace the times of the LSQ queries in the `stakeDistribution` function


# 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
-->


2087: Transaction metadata in swagger API spec r=rvl a=rvl

### Issue Number

ADP-307 / #2073 / #2074 

### Overview

Updates the swagger spec for the new metadata field when:
- listing transactions (metadata field is always present but may be null)
- posting a transaction (metadata field is optional)
- estimating fee (as above)

### Comments

[Rendered spec](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/rvl/2073/swagger/specifications/api/swagger.yaml)


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

iohk-bors bot commented Aug 28, 2020

Build failed (retrying...)

iohk-bors bot added a commit that referenced this pull request Aug 28, 2020
2080: Also trace the times of stake distribution LSQ queries r=rvl a=Anviking

# Issue Number

#2005 / new issue

# Overview

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

- [x] Also measure and trace the times of the LSQ queries in the `stakeDistribution` function


# 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
-->


2087: Transaction metadata in swagger API spec r=rvl a=rvl

### Issue Number

ADP-307 / #2073 / #2074 

### Overview

Updates the swagger spec for the new metadata field when:
- listing transactions (metadata field is always present but may be null)
- posting a transaction (metadata field is optional)
- estimating fee (as above)

### Comments

[Rendered spec](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/rvl/2073/swagger/specifications/api/swagger.yaml)


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

iohk-bors bot commented Aug 28, 2020

Build failed (retrying...)

iohk-bors bot added a commit that referenced this pull request Aug 28, 2020
2087: Transaction metadata in swagger API spec r=rvl a=rvl

### Issue Number

ADP-307 / #2073 / #2074 

### Overview

Updates the swagger spec for the new metadata field when:
- listing transactions (metadata field is always present but may be null)
- posting a transaction (metadata field is optional)
- estimating fee (as above)

### Comments

[Rendered spec](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/rvl/2073/swagger/specifications/api/swagger.yaml)


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

iohk-bors bot commented Aug 28, 2020

Build failed

@iohk-bors iohk-bors bot mentioned this pull request Aug 30, 2020
1 task
Copy link
Contributor

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

The description of transactionMetadata is very readable.

I do have one question (see review comments) relating to accessibility of the spec.


See the example on the right to get an overview and the
equivalence table below to understand how metadata are mapped to
on-chain data types.
Copy link
Contributor

@jonathanknowles jonathanknowles Aug 31, 2020

Choose a reason for hiding this comment

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

Just out of interest, what does "on the right" refer to here?

Suggestion: if this wording relies on a particular rendering style, perhaps it's better to use a layout-neutral method to refer to examples? (I'm guessing that not everyone will be viewing the API spec through the web tool that we provide.)

Copy link
Member

Choose a reason for hiding this comment

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

It refers to our current documentation layout indeed.

@KtorZ
Copy link
Member

KtorZ commented Aug 31, 2020

@rvl I'll look at the swagger failure above, this is a bit tricky because we use OpenAPI 3.0 but the Haskell library we use expects swagger 2.0 format, so there are some subtleties...

@rvl
Copy link
Contributor Author

rvl commented Aug 31, 2020

Oh ok - I assumed it was maybe because the endpoints in the swagger file need to be added to the servant API.

@KtorZ KtorZ force-pushed the rvl/2073/swagger branch from 2ffe45e to 10d022d Compare August 31, 2020 13:27
@KtorZ
Copy link
Member

KtorZ commented Aug 31, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 31, 2020
2087: Transaction metadata in swagger API spec r=KtorZ a=rvl

### Issue Number

ADP-307 / #2073 / #2074 

### Overview

Updates the swagger spec for the new metadata field when:
- listing transactions (metadata field is always present but may be null)
- posting a transaction (metadata field is optional)
- estimating fee (as above)

### Comments

[Rendered spec](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/rvl/2073/swagger/specifications/api/swagger.yaml)


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

iohk-bors bot commented Aug 31, 2020

Build failed

@KtorZ KtorZ force-pushed the rvl/2073/swagger branch from 10d022d to 81b6a23 Compare August 31, 2020 15:54
@KtorZ
Copy link
Member

KtorZ commented Aug 31, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 31, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 2b76b50 into master Aug 31, 2020
@iohk-bors iohk-bors bot deleted the rvl/2073/swagger branch August 31, 2020 17:42
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