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 Coingecko module to expose useful APIs directly from nodes #773

Closed
leobragaz opened this issue Mar 14, 2022 · 8 comments · Fixed by #782
Closed

Add Coingecko module to expose useful APIs directly from nodes #773

leobragaz opened this issue Mar 14, 2022 · 8 comments · Fixed by #782
Labels
kind/new-feature Propose the addition of a new feature that does not yet exist

Comments

@leobragaz
Copy link
Contributor

In order to make it easy to keep all the $DSM info update on coingecko page
it would be useful to have a x/coingecko modules exposing useful APIs (GRPC and REST) that can be extended in time and provided by all the full-nodes.

The module will use Cosmos-SDK modules (x/auth, x/bank, x/distribution) in order to retrieve supply and vesting accounts informations to properly calculate the most updated circulating supply at any time.

The circulating-supply API will be exposed by the module through both GRPC and REST endpoints as follow:

/desmos/coingecko/v1/circulating-supply/{denom}

With it, it will be possible to query circulating supply of any kind of assets available in the chain.
The above proposed implementation can already be tested by compiling and executing a chain from the following branch:
https://github.com/desmos-labs/desmos/tree/leonardo/coingecko-APIs

@leobragaz leobragaz added the kind/new-feature Propose the addition of a new feature that does not yet exist label Mar 14, 2022
@leobragaz leobragaz changed the title Add Coingecko APIs Add Coingecko module to expose useful APIs directly from nodes Mar 14, 2022
@dadamu
Copy link
Contributor

dadamu commented Mar 15, 2022

It is the oracle move. From my side, it should be implemented with other oracle provider like Chainlink or Band Protocol to solve the oracle problem if it would be integrated with smart contract in the future. Building oracle on desmos directly is not a desmos goal as I know. In addition, the feature seems for convenience, could/should be done with bdjuno. Personally, I trend to reject this feature.
What do you think? @RiccardoM

@kwunyeung
Copy link
Contributor

I tend to agree with @dadamu and I think this should be done with BDJuno. We already have the vesting account data indexed in the database. It seems easier and straightforward to do with BDJuno.

@RiccardoM
Copy link
Contributor

RiccardoM commented Mar 15, 2022

@dadamu Honestly I don't understand what would be the problem here. We are not talking about getting the price of the tokens but only some on-chain data (es. total supply, circulating supply) which are already present on-chain

@kwunyeung I don't see what's the problem in implementing this endpoint directly inside the node. We already have all the data there without relying on an external (potentially unstable) thing such as BDJuno. It would only require us to implement a couple of endpoints that can be accessed by all data aggregators (CoinMarketCap, CoinGecko, etc) and relies on other modules (bank, auth, etc). Also, once done we could simply upstream this to the Cosmos SDK for easier of usage

@dadamu
Copy link
Contributor

dadamu commented Mar 15, 2022

@bragaz @RiccardoM Ah my bad, I thought it will gather coingecko price to provide price related data for the user. It is the useful api to let coingecko to query it. Then this feature is fine with me.

@leobragaz
Copy link
Contributor Author

@dadamu Maybe I wrote it a little bit bad. To clarify, it will be a module that exposes useful queries that coingecko (and coinmarketcap) will use to update info such as the circulating supply and the total supply.

@dadamu
Copy link
Contributor

dadamu commented Mar 16, 2022

@bragaz Maybe we can call this module x/supply or something else.

@leobragaz
Copy link
Contributor Author

@dadamu I was thinking the same. x/supply was also the name of an old module of the SDK that has later been merged into bank 😆. Btw I think the name explains the purpose of it pretty well so why not.
@RiccardoM @kwunyeung comments?

@kwunyeung
Copy link
Contributor

Ah I see. I thought it would be something state breaking. If it's only a few endpoints then we can just update our nodes and serve the new APIs.

@mergify mergify bot closed this as completed in #782 Apr 8, 2022
mergify bot pushed a commit that referenced this issue Apr 8, 2022
## Description

Closes: #773 .

This PR implements the `x/supply` module which serves some useful APIs to query both `TotalSupply` (converted from millionth)  and `CirculatingSupply`.

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration [tests](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [x] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [x] reviewed documentation is accurate
- [x] reviewed tests and test coverage
- [x] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/new-feature Propose the addition of a new feature that does not yet exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants