Cosmos-sdk: Implement Gaia-lite LCD Spec

[jackzampolin]

This issue is to track implementation of the newly accepted LCD API spec (#1979). Each of the interchain standards needs to be checked to ensure that it is brought up to spec. The input, return and error values in the spec also need to be updated for each endpoint. Those updates should accompany code changes. Below is a list of the ICS and their endpoints to track progress.

• [x] ICS0
  
  
  
  • [x] POST /txs
  
  
  • [x] POST /tx/sign
  
  
  • [x] POST /tx/broadcast
  
  
• [x] ICS1
  
  
  
  • [x] GET /keys
  
  
  • [x] POST /keys
  
  
  • [x] GET /keys/seed
  
  
  • [x] GET /keys/{name}
  
  
  • [x] PUT /keys/{name}
  
  
  • [x] DELETE /keys/{name}
  
  
  • [x] POST /keys/{name}/recover
  
  
  • [x] GET /auth/accounts/{address}
  
  
• [x] ICS20
  
  
  
  • [x] GET /bank/balances/{address}
  
  
  • [x] POST /bank/accounts/{address}/transfers
  
  
• [x] ICS 21
  
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/validators
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/validators/{validatorAddr}
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/txs
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/delegations #2258
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/delegations/{validatorAddr}
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/unbonding_delegations #2258
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/unbonding_delegations/{validatorAddr}
  
  
  • [x] GET /stake/delegators/{delegatorAddr}/redelegations #2258
  
  
  • [x] GET /stake/redelegations? #2182
  
  
  • [x] POST /stake/delegators/{delegatorAddr}/delegations #2191
  
  
  • [x] POST /stake/delegators/{delegatorAddr}/unbonding_delegations #2191
  
  
  • [x] POST /stake/delegators/{delegatorAddr}/redelegations #2191
  
  
  • [x] GET /stake/validators
  
  
  • [x] GET /stake/validators/{validatorAddr}
  
  
  • [x] GET /stake/validators/{validatorAddr}/delegations #2027
  
  
  • [x] GET /stake/validators/{validatorAddr}/unbonding_delegations #2477
  
  
  • [x] GET /stake/validators/{validatorAddr}/redelegations #2477
  
  
  • [x] GET /stake/parameters
  
  
  • [x] GET /stake/pool
  
  
• [x] ICS 22
  
  
  
  • [x] GET /gov/proposals
  
  
  • [x] GET /gov/proposals/{proposalId}
  
  
  • [x] GET /gov/proposals/{proposalId}/deposits #2478
  
  
  • [x] GET /gov/proposals/{proposalId}/deposits/{address}
  
  
  • [x] GET /gov/proposals/{proposalId}/votes
  
  
  • [x] GET /gov/proposals/{proposalId}/votes/{address}
  
  
  • [x] GET /gov/parameters #2479
  
  
  • [x] POST /gov/proposals
  
  
  • [x] POST /gov/proposals/{proposalId}/deposits
  
  
  • [x] POST /gov/proposals/{proposalId}/votes
  
  
• [x] ICS 23
  
  
  
  • [x] GET /slashing/validators/{validatorPubKey}/signing-info #2462
  
  
  • [x] GET /slashing/parameters #2399
  
  
  • [x] POST /slashing/validators/{validatorAddr}/unjail #2120 #2462
  
  

cc @fedekunze

[gamarin2]

Also needed prelaunch, but not in the current spec: signing endpoint in key API.

cc @adrianbrink

[jackzampolin]

Can we open a PR on the spec for that @gamarin2 ?

[gamarin2]

@adrianbrink is supposed to do it

[faboweb]

GET /bank/balance/{account} should be GET /bank/balances/{account}?
GET /slashing/validator/{validatorAddr}/signing-info should be GET /slashing/validators/{validatorAddr}/signing-info?

[jackzampolin]

Thank you for checking my work there @faboweb

[jackzampolin]

I just added one here: POST /keys/{name}/sign to sign an unsigned transaction. To accompany this and mirror the CLI, each of the routes that produce transactions need to have a ?generate= param that defaults to false, which when set to true returns only the sign bytes for the transaction.

[siman]

Sorry for off-topic, but what does an LCD abbreviation mean?

[alexanderbez]

@siman iirc, light (lite?) client daemon 👍

[fedekunze]

Just realized we missed GET /stake/delegators/{delegatorAddr}/redelegations/{validatorAddr} to the list that still needs to be implemented and added to the documentation

[fedekunze]

Also adding GET stake/validators/{validatorAddr}/delegators and GET stake/validators/{validatorAddr}/delegations as discussed in #2027

[fedekunze]

Proposal for splitting the POST delegation endpoint: #2191 . CC: @jackzampolin

[fedekunze]

I just added one here: POST /keys/{name}/sign to sign an unsigned transaction. To accompany this and mirror the CLI, each of the routes that produce transactions need to have a ?generate= param that defaults to false, which when set to true returns only the sign bytes for the transaction.

@jackzampolin it was renamed to tx/sign. Should rename it ? Personally I'm in favour of it

[HaoyangLiu]

[ ] ICS0

• [ ] POST /txs

[ ] ICS1

• [x] GET /keys
• [x] POST /keys
• [x] GET /keys/seeds Do we really need this endpoint?
• [x] GET /keys/{name}
• [x] PUT /keys/{name}
• [x] DELETE /keys/{name}
• [ ] POST /keys/{name}/recover
• [ ] GET /auth/accounts/{address}
• [ ] POST /keys/{name}/sign *
• [x] POST /tx/sign
• [x] POST /tx/broadcast

[ ] ICS20

• [ ] GET /bank/balances/{account}
• [ ] POST /bank/transfers

@jackzampolin
I have some thoughts above the following APIs:

• POST /txs and POST /tx/broadcast: They are duplicate.
• POST /tx/sign and POST /keys/{name}/sign : I think the only thing need to be signed is transaction. So only POST /tx/sign is enough.
• GET /bank/balances/{account} and GET /auth/accounts/{address}: They are duplicate.
• POST /keys/{name}/recover and POST /keys: POST /keys can add new key. The post body is:

type NewKeyBody struct {
    Name     string `json:"name"`
    Password string `json:"password"`
    Seed     string `json:"seed"`
}

If seed field is not empty, then it will be used to recover a key. So POST /keys/{name}/recover is not necessary.

• POST /accounts/{address}/send and POST /bank/transfers: Should we rename POST /accounts/{address}/send to POST /bank/transfers?

[HaoyangLiu]

@jackzampolin @cwgoes
Why we need both gaiacli rest-server and Gaia-lite LCD? I think their functionalities are duplicated. Could you clarify this?

[faboweb]

As far as I know: gaiacli rest-server is the command Gaia-lite LCD or better just Gaia-lite is the name for the product/feature.

[faboweb]

POST /txs and POST /tx/broadcast: They are duplicate.

Let's keep both. POST /txs is more restful, POST /tx/broadcast feels more familiar to the blockchain domain.

POST /tx/sign and POST /keys/{name}/sign

You are correct. Let's stick with just POST /tx/sign

GET /bank/balances/{account} and GET /auth/accounts/{address}

Let's go with both where GET /bank/balances/{account} returns only balances (not sequence etc.) for the sake of consistency with the token transfer POST and to show the bank module is used.

POST /keys/{name}/recover and POST /keys

Let's use the explicit POST /keys/{name}/recover endpoint for recovery and keep POST /keys for key creation which fails if a seed is provided.

POST /accounts/{address}/send and POST /bank/transfers

I propose to name this endpoint POST /bank/accounts/{address} to stay consistent with the GET request and to make it clear the bank module is used for both requests.

[fedekunze]

cc: @faboweb @HaoyangLiu

Let's keep both. POST /txs is more restful, POST /tx/broadcast feels more familiar to the blockchain domain.

The original idea @jackzampolin and I discussed (I don't recall the exact date) is that we should would leave it as POST /txs and have a return parameter in the request body with the desired output: return=<sync|async|block>. But since we are also keeping with POST /tx/sign let's keep the 3 of them

Let's go with both where GET /bank/balances/{account} returns only balances (not sequence etc.) for the sake of consistency with the token transfer POST and to show the bank module is used.

Agree, but lets change GET /bank/balances/{account} to use an /{address} instead

Let's use the explicit POST /keys/{name}/recover endpoint for recovery and keep POST /keys for key creation which fails if a seed is provided.

👍

I propose to name this endpoint POST /bank/accounts/{address} to stay consistent with the GET request and to make it clear the bank module is used for both requests.

From just looking at the endpoint POST /bank/accounts/{address} someone could think that you're creating a new bank account to tied to your {address} (?). What about POST /bank/accounts/{address}/transfers

[faboweb]

return parameter in the request body with the desired output: return=

I would put them into the request URL and not into the body, so the data stays the same but the handling changes. Plus: Let's do that later.

POST /bank/accounts/{address}/transfers

👍

Agree, but lets change GET /bank/balances/{account} to use an /{address} instead

👍

[fedekunze]

Updated list of endpoints according to the most recent discussion

[jackzampolin]

Update list.

[faboweb]

Follow up of https://github.com/cosmos/cosmos-sdk/pull/2336

I propose that we don't bloat the API and remove:

• GET /stake/delegators/{delegatorAddr}/redelegations/validator_from/{validatorSrcAddr}/validator_to/{validatorDstAddr}

I don't see a use case for this. I see a usecase making this more general purpose:

• GET /stake/delegators/{delegatorAddr}/redelegations/{validatorSrcAddr}

[fedekunze]

@faboweb I get your point and I think that would be actually more useful. I even thought initially of the same idea but with GET /stake/delegators/{delegatorAddr}/redelegations/{validatorAddr} (where validatorAddr could be either src or dst).

The thing is that that functionality is not implemented since there's no key for it. The one that's similar is getting the redelegations from a validatorSrcAddr, i.e something like the following:

GET /stake/validators/{validatorSrcAddr}/redelegations

Here's the permalink to the function:

https://github.com/cosmos/cosmos-sdk/blob/c653053cfb7040b20328eb0d1f9378ddcca0058d/x/stake/keeper/delegation.go#L245-L258

Am I right @cwgoes ?

[fedekunze]

I think this is done, @jackzampolin can we close this ?

[jackzampolin]

@fedekunze Can you do the honor as this has been your baby!?

[fedekunze]

@fedekunze Can you do the honor as this has been your baby!?

😆 closing this ! 🎊