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

OpenApi Definition for Electrs Rest Api #103

Open
wants to merge 4 commits into
base: new-index
Choose a base branch
from

Conversation

cjrutherford
Copy link

Fully validated requests with the exception of the broadcast and tx routes to post a transaction to the chain.

everything else has been documented with the correct shape of the response.

work on request body is dependent on testing of the broadcast and tx routes as mentioned above.

all other routes tested properly.

@RCasatta
Copy link
Collaborator

I like the OpenAPI spec, Ideally we should have some automatic way of checking it's in sync with the real API. This seem not quickly doable but IIRC you can build "tester web site " out of the spec, can we expose/have instruction to build it locally so that at least one can manual test it?

There is some naming confusion I would fix, like btcd instead of bitcoin in the file name (btcd recall a different software, the node written in go) and also a typo in the commit message (lnd instead of liquid)

tags:
- block
summary: Returns up to 25 transactions based on an optional starting index.
description: Responses for this endpoint can be cached indefinitely.
Copy link
Collaborator

Choose a reason for hiding this comment

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

see comments about similar description above

summary: Merkle Inclusing Proof (Bitcoind merkleblock)
description: |
Returns a Merkle inclusion proof for the transaction using
[bitcoind's merkelblock](https://bitcoin.org/en/glossary/merkle-block) format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

link is 404

Copy link
Author

Choose a reason for hiding this comment

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

it appears that the entire glossary section is gone looking into alternatives.

Copy link
Author

Choose a reason for hiding this comment

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

summary: Merkle Inclusing Proof (Bitcoind merkleblock)
description: |
Returns a Merkle inclusion proof for the transaction using
[bitcoind's merkelblock](https://bitcoin.org/en/glossary/merkle-block) format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

type "merkelblock"

required: true
schema:
type: string
responses:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this response content has been swapped for merkle-proof.

The merkle-proof resource is a hex string, whereas merkle-block-proof is the electrumx object

Copy link
Author

Choose a reason for hiding this comment

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

swapped them around and confirmed response schemas

Copy link
Collaborator

@philippem philippem left a comment

Choose a reason for hiding this comment

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

see comments above

@philippem
Copy link
Collaborator

I like the OpenAPI spec, Ideally we should have some automatic way of checking it's in sync with the real API. This seem not quickly doable but IIRC you can build "tester web site " out of the spec, can we expose/have instruction to build it locally so that at least one can manual test it?

There is some naming confusion I would fix, like btcd instead of bitcoin in the file name (btcd recall a different software, the node written in go) and also a typo in the commit message (lnd instead of liquid)

I like the OpenAPI spec, Ideally we should have some automatic way of checking it's in sync with the real API. This seem not quickly doable but IIRC you can build "tester web site " out of the spec, can we expose/have instruction to build it locally so that at least one can manual test it?

There is some naming confusion I would fix, like btcd instead of bitcoin in the file name (btcd recall a different software, the node written in go) and also a typo in the commit message (lnd instead of liquid)

we can do a validation with a generated client (e.g. https://openapi-generator.tech/)

…or each BTC and LND

fixed several typos and corrections based on review feedback.
updated liquid version to match
@cjrutherford
Copy link
Author

I like the OpenAPI spec, Ideally we should have some automatic way of checking it's in sync with the real API. This seem not quickly doable but IIRC you can build "tester web site " out of the spec, can we expose/have instruction to build it locally so that at least one can manual test it?
There is some naming confusion I would fix, like btcd instead of bitcoin in the file name (btcd recall a different software, the node written in go) and also a typo in the commit message (lnd instead of liquid)

I like the OpenAPI spec, Ideally we should have some automatic way of checking it's in sync with the real API. This seem not quickly doable but IIRC you can build "tester web site " out of the spec, can we expose/have instruction to build it locally so that at least one can manual test it?
There is some naming confusion I would fix, like btcd instead of bitcoin in the file name (btcd recall a different software, the node written in go) and also a typo in the commit message (lnd instead of liquid)

we can do a validation with a generated client (e.g. https://openapi-generator.tech/)

should I create a generated client and set it up to auto test?

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