LCD and API Specification / Documentation #1314
Conversation
ICS0 - TendermintAPI ICS1 - KeyAPI ICS20 - TokenAPI ICS21 - StakingAPI ICS22 - GovernanceAPI The LCD exposes all these APIs and is the only way clients should interact with the network. Optionally, the LCD can verify proofs from fullnodes.
Codecov Report
@@ Coverage Diff @@
## develop #1314 +/- ##
========================================
Coverage 63.71% 63.71%
========================================
Files 122 122
Lines 6774 6774
========================================
Hits 4316 4316
Misses 2211 2211
Partials 247 247 |
|
Well done!
|
|
Is this PR ready for review or still WIP? Please tag |
|
@adrianbrink |
|
This is related: |
|
@nylira @jolesbi @faboweb It would be great to get your input on the apis exposed by the LCD. Do you need different endpoints or completely new ones? Splitting up the APIs was done on purpose so that none of them depend on each other. For example, it is on purpose that ICS20 (TokenAPI) does not allow you to directly send someone some coins, but instead requires that you generate a transaction using it and then sign it using ICS1 (KeyAPI) and eventually submit it using ICS0 (TendermintAPI). Lastly, there is an old implementation of the LCD here. We can potentially reuse some of the logic/code from it. @HaoyangLiu @haifeng |
|
This was discussed in depth before. The decision was that we will split out the KeyAPI softly, meaning we keep it in for now and add transaction building endpoints and a broadcast endpoint to allow signing with an external KMS. Then later we deprecate the build -> sign -> send endpoints if desired. |
This is done so that the time spent on bcrypt during test cases can be reduced. This change reduces the amount of time lcd tests spend on bcrypt from 76% to 40%. (We need to reduce the number of calls to bcrypt in a seperate PR, along with fixing other sources of slowness) Making the bcrypt security parameter a var shouldn't be a security issue: One can't verify an invalid key by maliciously changing the bcrypt parameter during a runtime vulnerability. The main security threat this then exposes would be something that changes this during runtime before the user creates their key. This vulnerability must succeed to update this to that same value before every subsequent call to gaiacli keys in future startups / or the attacker must get access to the filesystem. However, with this same threat model (changing variables in runtime), one can cause the user to sign a different tx than what they see, which is a significantly cheaper attack then breaking a bcrypt hash. (Recall that the nonce still exists to break rainbow tables)
This adds a command to automatically fix gofmt and misspell errors.
* x/stake: Limit the size of rationals from user input This commit sets the maximum number of decimal points that can be passed in from messages. This is enforced on the validate basic of MsgBeginUnbonding and MsgBeginRedelegation. The cli has been updated to truncate the user input to the specified precision. This also updates types/rational to return big ints for Num() and Den(). Closes #887 * Switch NewFromDecimal to error instead of truncating
* tools: Add unparam linter unparam detects unused parameters in functions, and a parameter to a function which only ever takes on one value. The latter is an indication that more tests are required. There are many nolints in this PR, as I believe that writing tests to fix alot of these situations is out of scope for this PR / it will be changed in future commits. There are some nolints for when we have to comply to normal api's. * crypto/keys no longer used by x/gov/client/rest/rest.go
* gaiacli: Make recovery allow new keys * Move create key to a temporary method, restore create fundraiser key
* meta: Switch the majority of asserts to require Switch most assert statements to require, to ease debugging. Closes #1418 * Fix imports
* Merge pull request #1422: Add Contributing Guidelines * cwgoes comments
* client/lcd: fix tests * circle: drop test_unit. store artifacts in test_cover * hack fix in TestUnrevoke
Adds sensible return data on success to submitting transactions.
|
This is ready for review. I'm not sure why all these commits ended up in this branch. My plan was to update to the latest state of |
|
Can we address #1506 in this spec please? |
|
Yikes - can we just get this PR wrapped up as a single commit on top of latest develop with all the new files? |
|
Closing in favor of #1617. |
@kidinamoto01 I've merged your PR into mine. It looks great. You can open future PRs against this branch. I think we should try and merge smaller changes faster. I'm not super familiar with working with forks on github. Sorry :-(
@gamarin2 @HaoyangLiu @haifengxi Let's work from this branch.
The high-level overview of the LCD and API. The LCD specification describes how the LCD works and how other people would go about implementing it. It also explains how to add extra modules to it, that will then get exposed in the API. We'll make it more specific over time. Also, we should pull in more of the LCD specification here.
The API documentation describes the different APIs that are exposed by the LCD for the Cosmos Hub (Gaia). We can split these documents, once we split the Cosmos Hub out of the sdk repo.
The LCD will include the following APIs:
Related issues:
This PR supersedes #1297 and #1092 and #1139 .
This PR closes #1020 and #1320 and #1312 and #1311 and #1272