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

Rename Gaia-lite endpoints to be more RESTful #1979

Closed
4 tasks
fedekunze opened this issue Aug 10, 2018 · 20 comments
Closed
4 tasks

Rename Gaia-lite endpoints to be more RESTful #1979

fedekunze opened this issue Aug 10, 2018 · 20 comments

Comments

@fedekunze
Copy link
Collaborator

fedekunze commented Aug 10, 2018

Summary

Rename some of the ICS/Gaia-lite/LCD endpoints to better describe what they do and be more RESTful.

CC: @cosmos/cosmos-ui @jackzampolin

Problem Definition

Same as above.

Proposal

ICS0 - TendermintAPI

POST /broadcast_tx_sync --> POST /txs body: {return: sync, ...items}
POST /broadcast_tx_async --> POST /txs body: {return: async, ...items}
POST /broadcast_tx_commit --> POST /txs body: {return: block, ...items}

ICS1 - KeyAPI

POST /keys/create --> POST /keys
POST /keys/recover --> POST /keys/{name}/recover

ICS20 - TokenAPI

POST /bank/create_transfer --> POST /bank/transfers

Other

GET /accounts/{address} --> GET /auth/accounts/{address}
POST /ibc/{destchain}/{address}/send --> POST /ibc/txs
GET /slashing/signing_info/{validatorPubKey} --> GET /slashing/validator/{validatorAddr}/signing-info
POST /slashing/unjail --> POST /slashing/validator/{validatorAddr}/unjail


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@fedekunze fedekunze changed the title Rename Gaia-lite endpoints to be more RESTfull Rename Gaia-lite endpoints to be more RESTful Aug 10, 2018
@NodeGuy
Copy link
Contributor

NodeGuy commented Aug 10, 2018

Thank you, Fede. If we're going to do some renaming then please, PLEASE, for the love of God rename txs to transactions. Thr's no crtcal shrtge of keystrks nymr.

@alexanderbez
Copy link
Contributor

💯 %

@jackzampolin
Copy link
Member

All of this! How do people feel about txs vs transactions?

@fedekunze
Copy link
Collaborator Author

@NodeGuy @jackzampolin I'm down as long as we rename all the txs routes of the SDK

@jackzampolin
Copy link
Member

I kinda like /txs personally, but feels like I'm outvoted here...

@fedekunze
Copy link
Collaborator Author

I actually like /txs more ... but it won't bother me if we change it. Let's see what @ebuchman @cwgoes and @rigelrozanski think

@fedekunze
Copy link
Collaborator Author

Actually, POST /bank/create_transfer in the SDK is implemented as POST /accounts/{address}/send (see L20). I thought having the module name before was a standard ... Would love to hear your thoughts about this

@jbibla
Copy link
Contributor

jbibla commented Aug 13, 2018

would hate for txs to be confused for taxes or taxis

@cwgoes
Copy link
Contributor

cwgoes commented Aug 13, 2018

would hate for txs to be confused for taxes or taxis

Why would you confuse it with either of those? Neither is a likely endpoint in a blockchain-related API.

@jbibla
Copy link
Contributor

jbibla commented Aug 13, 2018

sorry, i was being facetious.

but in general, i do think using the full word as @NodeGuy suggested is the right choice.

@NodeGuy
Copy link
Contributor

NodeGuy commented Aug 13, 2018

What is it that you guys like about txs?

@fedekunze
Copy link
Collaborator Author

I personally like txs more than transactions

@jackzampolin
Copy link
Member

I think txs is a common short hand and is less verbose. Thats really just a preference for me tho.

@ValarDragon
Copy link
Contributor

ValarDragon commented Aug 13, 2018

I prefer txs personally, but I think that is independent of the core suggestion of this issue, and perhaps should be relegated to a separate issue.

I think the original proposal is a great idea, and we should go through with it. (From what I understand from the above comments, no one actually has a problem with the original issue, and there was only agreement or a lack of opinion on it)

I'm unclear on the actual ICS numbers we've reserved, so I assume what you've suggested follows the most up to date plan for the numbering.

@fedekunze
Copy link
Collaborator Author

Note: Updating txs endpoints to move the return query param to request body as stated in:

The same encoding is used by default when the submission method is POST, but the result is submitted as the HTTP request body rather than being included in a modified URL.[1]
Source

@jackzampolin
Copy link
Member

@fedekunze that would require an issue to be opened on tendermint/tendermint right?

@fedekunze
Copy link
Collaborator Author

Note: added renaming suggestion for POST /slashing/unrevoke as well

@jbibla
Copy link
Contributor

jbibla commented Aug 17, 2018

@fedekunze wrt slashing/unrevoke let's wait to see what happens with the revoked / jailed conversation #1305

@cwgoes
Copy link
Contributor

cwgoes commented Aug 20, 2018

#1305 has concluded in favor of "jailed", with "penalty" as a higher-level UX concept.

@jackzampolin
Copy link
Member

Going to go ahead and close this as we have addressed this in a number of different PRs.

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

No branches or pull requests

7 participants