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

Standardize tokens output across modules' REST APIs #4783

Closed
4 tasks
RiccardoM opened this issue Jul 25, 2019 · 4 comments
Closed
4 tasks

Standardize tokens output across modules' REST APIs #4783

RiccardoM opened this issue Jul 25, 2019 · 4 comments

Comments

@RiccardoM
Copy link
Contributor

Summary

Allows different modules' connected REST APIs to output token values always as a list of StdCoin.

Problem Definition

Currently when connecting to the LCD REST APIs of the distribution and staking module, we receive token values as two different types:

  1. Inside the distribution module they are presented as a list of StdCoin .
  2. Inside the staking module they are presented as strings.

This results in having to perform strange assumptions if we want to create something that puts together those data in a chain that has multiple supported coins.

As an example, let's take Regen.network that supports both seeds and trees tokens.

Now, let's take two API calls to two different endpoints.

1. Delegator rewards.
Link: https://xrn-us-east-1.regen.network/distribution/delegators/xrn:1dy39q7t3ja893qwnhr9hgllpd966npkeeyqzmt/rewards

Response

{
  "rewards": [
    {
      "validator_address": "xrn:valoper1dy39q7t3ja893qwnhr9hgllpd966npke003uud",
      "reward": [
        {
          "denom": "seed",
          "amount": "96.114571695532999000"
        },
        {
          "denom": "tree",
          "amount": "307692.078229836368667000"
        }
      ]
    }
  ],
  "total": [
    {
      "denom": "seed",
      "amount": "96.114571695532999000"
    },
    {
      "denom": "tree",
      "amount": "307692.078229836368667000"
    }
  ]
}

2. Delegator's delegations
Link: https://xrn-us-east-1.regen.network/staking/delegators/xrn:1dy39q7t3ja893qwnhr9hgllpd966npkeeyqzmt/delegations

Response

[
  {
    "delegator_address": "xrn:1dy39q7t3ja893qwnhr9hgllpd966npkeeyqzmt",
    "validator_address": "xrn:valoper1dy39q7t3ja893qwnhr9hgllpd966npke003uud",
    "shares": "1007070.707070707070707070",
    "balance": "997000"
  }
]

Now, what are those shares? And what tokens are those balance? Are those seed or tree?

Proposal

The actual proposal is quite simple: when outputting a value that represents a token, always make it as a list of StdCoin so that multiple coins chain can output the data correctly and wallets an/or explorers can show the data in a better and more significative way than the one that is currently possibile.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

@RiccardoM there is no such type as a StdCoin but I think I understand what you're stating. The difference in types you're expressing is sdk.Coin/s vs sdk.Int. The difference being the former has a denomination.

I agree that within staking and other modules we can/should use sdk.Coin instead of sdk.Int, although it provides no functional difference. I would greatly appreciate to see this as a community contribution PR 👍

@RiccardoM
Copy link
Contributor Author

I think this can be expanded by using sdk.Coins in order to allow multi-coins chains. That's why I started #4794.

What do you think @alexanderbez?

@RiccardoM
Copy link
Contributor Author

@alexanderbez Now that balance have been converted to sdk.Coin, do you think we should convert shares to sdk.CoinDec?

@alexanderbez
Copy link
Contributor

No, not necessarily because shares are really meant to be abstracted away from end users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants