Refactor LCD endpoints

[faboweb]

Description:

We want to improve the LCD staking endpoints:

• to be more REST-ful
• to allow batch querying of stake per delegator

[faboweb]

Consider #879

[fedekunze]

I can work on this too @faboweb

[faboweb]

Current state:
query delegation /stake/{delegator}/delegation/{validator}
query undonding delegations /stake/{delegator}/ubd/{validator}
query redelegations /stake/{delegator}/red/{validator_src}/{validator_dst}
query candidates /stake/validators
post bonding/unbdonding/redelegation tx /stake/delegations

[fedekunze]

GET /stake/delegators/{addr}/txs/{id} // Get a specific tx from a delegator Not necessary
GET /stake/validators/{addr}/delegators Deleting since we would have to change the keys

• GET /stake/delegators/{addr} // Get delegation (i.e delegation and undelegation) information from a delegator

• GET /stake/delegators/{addr}/txs // Get all txs from a delegator

• GET /stake/delegators/{addr}/validators // Query all validators that a delegator is bonded to

• GET /stake/delegators/{addr}/validators/{addr} // Query a validator info that a delegator is bonded to

• GET /stake/delegators/{addr}/validators/{addr}/txs //

• GET /stake/validators/

• GET /stake/validators/{addr}

• POST /stake/delegators/{addr}/txs // Submit a tx

a Tx can be of type: Bond, Unbond or Redelegate, so you should be able to query according to type:
GET /stake/delegators/{addr}/validators/{addr}/txs?type=<bond/unbond/redelegate>

Validator params:

• address
• description
• revoked
• status
• tokens
• delegatorShares // Tokens that delegated by several delegators
• bondHeight
• totalStake (i.e stakedCoins + delegatedCoins)
• stakedCoins
• looseTokens // Tokens that are not staked
• commission
• commissionMax
• commissionChangeRate
• commissionChangeToday
• lastBondedTokens (????)
• moniker
• website
• keybase-sig

[fedekunze]

• Write tests on the SDK in Pact
• Change endpoints on the SDK
• Run the Pact tests in Voyager
• Implement on Voyager
• Change LCD client yaml
• Update documentation
• Notify changes on both teams

[faboweb]

Stretch goal could be implementing this in Voyager (should be handled in a new issue)

[fedekunze]

@faboweb Just had a quick talk with Ismail and we agreed on taking out the /stake word from the beginning of the endpoint. Please also check this article for API best practices. I'd like to get your thoughts on this

[fedekunze]

Also, TxRedelegation would be seen to both ValidatorFrom and ValidatorTo addresses by doing GET /stake/delegators/{addr}/validators/{ValidatorFrom}/txs?type=redelegate and GET /stake/delegators/{addr}/validators/{ValidatorTo}/txs?type=redelegate respectively

[NodeGuy]

• I read that article and agree with it.
• I re-ordered your to do list to write tests before implementation (TDD).
• I wrote one test in Pact for you to use as a template. You can run the test with go test ./client/lcd -run TestProvider
• I'm creating tests for the current behavior at: WIP: Refactor LCD endpoints cosmos/cosmos-sdk#1813.
• /stake/validators is returning a Content-Type of text/plain; charset=utf-8 but should be returning application/json. The other end points may be making the same mistake.
• The staking module specification may be a useful reference.

[faboweb]

@fedekunze

we agreed on taking out the /stake word from the beginning of the endpoint

Can you add some reasoning?

[fedekunze]

@faboweb because it's not actually needed and it doesn't add much value to the endpoint (as the article says). If we want to get the module an endpoint is coming from we could for example add Tags in the Results with module equal to stake (as in this case)

[faboweb]

This is not about mapping the results but about namespacing the endpoints coming from different modules (stake module, gov module) to not conflict endpoints. I read the article and it doesn't handle this case. As this is not the case for the bank module or the auth module we should decide on a convention. I am fine with having no namespacing, but let's contact the SDK team if we want to change this convention (pulling in @cwgoes ).
One more thought: Requesting data and only after receiving it getting to know what data I requested sounds impractical.

[cwgoes]

I am fine with having no namespacing, but let's contact the SDK team if we want to change this convention (pulling in @cwgoes ).

Hmm I understand the desire to simplify but I think this will lead to problems. We want standard REST endpoints for not only our module set within Gaia but any conceivable other SDK apps which use our standard modules - those apps may use their own modules too, which might have conflicting endpoint names. If we prefix all routes by the name of the module, downstream usage is much easier.

[faboweb]

I will close this issue, as this has been merged on the SDK side.