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

[R4R]: add swagger-ui for gov, stake and slashing #2462

Merged
merged 28 commits into from
Oct 24, 2018
Merged

[R4R]: add swagger-ui for gov, stake and slashing #2462

merged 28 commits into from
Oct 24, 2018

Conversation

HaoyangLiu
Copy link

Main changes:

  • add swagger-ui for gov, stake and slashing
  • refactor swagger.yaml

@codecov
Copy link

codecov bot commented Oct 10, 2018

Codecov Report

Merging #2462 into develop will decrease coverage by 1.24%.
The diff coverage is 12.5%.

@@             Coverage Diff             @@
##           develop    #2462      +/-   ##
===========================================
- Coverage    60.08%   58.84%   -1.25%     
===========================================
  Files          150      152       +2     
  Lines         8704     9413     +709     
===========================================
+ Hits          5230     5539     +309     
- Misses        3116     3504     +388     
- Partials       358      370      +12

@HaoyangLiu
Copy link
Author

My changes seems have no connection with the test failures. The test_cover failure can't be reproduced in my local machine.

@HaoyangLiu HaoyangLiu changed the title [WIP]: add swagger-ui for gov, stake and slashing [R4R]: add swagger-ui for gov, stake and slashing Oct 10, 2018
public_key:
type: string
sequence:
type: string
404:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we choose to make this a 404? We don't know if someone holds that address but hasn't don't any transaction yet. Querying such an account would result in an error. I propose we do either serve a 400 or serve a default response with an empty account and a 200.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rechecked the code. If there is no content about this account address, 204 error code will be returned.

if len(res) == 0 {
   w.WriteHeader(http.StatusNoContent)
   return
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use this then for now. There was a discussion with @fedekunze about using 200 and a default response.

description: OK
schema:
$ref: "#/definitions/BroadcastTxCommitResult"
404:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same: We don't know if the delegator doesn't exist or hasn't delegated yet.

200:
description: OK
404:
description: Not Found
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same: We don't know if the delegator doesn't exist or hasn't delegated yet.

200:
description: OK
404:
description: Not Found
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same: We don't know if the delegator doesn't exist or hasn't delegated yet.

schema:
$ref: "#/definitions/TxQuery"
404:
description: Not Found
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same: We don't know if the delegator doesn't exist or hasn't delegated yet.

200:
description: OK
404:
description: Not Found
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would there be a not found error here?

200:
description: OK
schema:
$ref: "#/definitions/TxQuery"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an array not a single object

@fedekunze
Copy link
Collaborator

fedekunze commented Oct 19, 2018

@HaoyangLiu about your #2462 (comment), there's an open issue for that #2007

@HaoyangLiu
Copy link
Author

HaoyangLiu commented Oct 19, 2018

@fedekunze
Thanks very much for your suggestions.

@jackzampolin @faboweb
I have synced the latest change to the demo, please refer to this url: https://111.231.70.168:1317/swagger-ui/#/ for further review.

@cwgoes
Copy link
Contributor

cwgoes commented Oct 19, 2018

Needs a rebase since #2444 was merged (also, let's ensure that we're consistent in this PR with the updated error responses introduced in that one).

@@ -0,0 +1,45 @@
package client

// NormalizeVoteOption - normalize user specified vote option
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I understand the intent but I wonder if it's more obvious to just require exact strings - thoughts @fedekunze ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@faboweb and @jackzampolin suggested that the API should be independent from the backend implementation. So I added the normalization functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm OK, I don't have a strong opinion

@@ -256,3 +257,19 @@ func PostProcessResponse(w http.ResponseWriter, cdc *codec.Codec, response inter
w.Header().Set("Content-Type", "application/json")
w.Write(output)
}

// IsKeyNotFoundErr - check if the error means that the specified key doesn't exist
func IsKeyNotFoundErr(err error, name string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we export references to the exact error types instead of this sort of logic? It introduces technical debt.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have thought about this idea. But this will change key base implementaion. To avoid too much impact to the code, I chosen this implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HaoyangLiu How so? Ideally the client documentation shows the exact return types.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can check by the code or abci code I guess

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key base should export the error types directly - if it doesn't, let's fix it - all the relevant code should be in this repo I think.

Copy link
Author

@HaoyangLiu HaoyangLiu Oct 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry for misleading you. All code about key base implementation are in this repo too. My point is to minimize the change and limit the change to client code. If this is not a problem, I will change to code according to your suggestions.
@fedekunze
Currently the error has no error code or abci code. I will add error code in it.

@fedekunze
Copy link
Collaborator

Let's get a final review here @cwgoes @faboweb @NodeGuy @jackzampolin

@jackzampolin
Copy link
Member

Gets a LGTM 👍 from me!

@fedekunze
Copy link
Collaborator

@HaoyangLiu could you add the latest changes from #2258 ? 🙏

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 23, 2018

This PR is too big (and has 98 comments). Please break it up into smaller PRs.

@cwgoes
Copy link
Contributor

cwgoes commented Oct 23, 2018

This PR is too big (and has 98 comments). Please break it up into smaller PRs.

It is, but at this point I think that might be more work.

@HaoyangLiu
Copy link
Author

@fedekunze
OK. I will add the endpoints soon.

@HaoyangLiu
Copy link
Author

HaoyangLiu commented Oct 24, 2018

I have added the latest changes from #2258
I also synced the latest to the demo: https://111.231.70.168:1317/swagger-ui/

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@@ -0,0 +1,45 @@
package client

// NormalizeVoteOption - normalize user specified vote option
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm OK, I don't have a strong opinion

@cwgoes cwgoes merged commit 9ee9e28 into cosmos:develop Oct 24, 2018
MarcelMWS added a commit to MarcelMWS/cosmos-sdk that referenced this pull request Nov 15, 2018
* Back to 50 initially bonded

* Add query deposits cmds

* Update test

* Update PENDING.md

* Back to on-operation=false, update PENDING.md

* Remove unnecessary change, clarify amt in sim_test.go

* Cleanup, additional item in PENDING.md

* Update PENDING.md

Co-Authored-By: alessio <quadrispro@ubuntu.com>

* Update client/keys/utils.go

Co-Authored-By: alessio <quadrispro@ubuntu.com>

* update depositer addr

* Address @rigelrozanski comments

* Make linter happy

* Update PENDING.md

* Remove shorthand

* Make "multi" const

* Store last total power as sdk.Int, not sdk.Dec

* Merge PR cosmos#2553: Renamed msg.Name() and msg.Type() to msg.Type() and msg.Route()

* Fix stupid mistake

* s/number/weight/

* LastValidatorPower is also an Int

* Merge PR cosmos#2462: Add swagger-ui for gov, stake and slashing

* 'make format'

* Move PENDING to CHANGELOG

* Linkify changelog

* Fix db write perm

* Additional cleanup

* Remove logs from prior debugging

* Merge PR cosmos#2599 from cosmos/jae/dist_refactor

* Rename Pool -> DelRewards; PoolCommission -> ValCommision
* FeePool.Pool -> FeePool.ValPool
* WithdrawalHeight->DelPoolWithdrawalHeight
* OnValidatorBeginUnbonding
* Caught the bug's tail
* Update vi.FeePoolWithdrawalHeight upon bonding
* Fix staking slashUnbondingDelegation bug; fixes simulator failure cosmos#9

* Merge PR cosmos#2597: Add distribution accum invariants

* PENDING.md => CHANGELOG.md

* Manually linkify

* Manually fix some links

* Docs fixes in progress while running through the release process

* More docs fixes

* '--voter' is no longer required

* Rectify validator setup documentation

* Merge PR cosmos#2596: Cmds for validator unbondings and redelegations

* Make simulation use a transition matrix for block size

This enables simulating periods of high load, and periods of low to no load.
(low load because future ops will still terminate in that time frame)

* address bez's comments

* fix flags in docs, closes cosmos#2530

* Merge PR cosmos#2616: Block redelegations to the same validator

* Merge PR cosmos#2623: Speedup simulator by switching to goleveldb

Due to requiring app.Commit() at the moment, golevel db is significantly faster than a memdb

* fix block offsets in printing simulation block number

* Merge PR cosmos#2644: Simulation: Print last block when there is an error

There was an off by one error in the log printing function previously

* Merge PR cosmos#2642: Add todo diagrams

* Various sign command improvements

- Exit with error if the user is attempting to sign with a key
  whose address is not among those who are expected to sign
  the transaction.

- Add --print-signature-only to output only the generated
  signature.

* Check sanity of signatures and report errors when run with --print-sigs

* Improve errors reporting

* Improve online docs

* Refresh PENDING.md

* Find better name for --print-signature-only

* Fix integration tests

* Validate --name

* Fix integration tests

* s/--print-sigs/--validate-signatures/

* s/--sig-only/--signature-only/

* Docs updated

* Update PENDING.md

* Rename append, it's go builtin

* Set success = false when it fails

* Apply suggestions from bez

* Nest switches

* Fix rebase

* Document what --validate-signatures does

* perform minor doc and function cleanup

* move typedef

* Merge PR cosmos#2614: Configurable Bech32 prefix for SDK users

* Merge PR cosmos#2643: AppendTag function usage error. append elements do not work

* simulation: Make validator choice use validator set

This also had to change the default seed, since with the previous one it
actually got into a state where there were no validators left bonded, lol.

This also changes Unbond msgs from failing with almost 100% probability to now
only failing with 33% probability.
Thus more of the state machine is getting tested!

* Update changelog

* Merge PR cosmos#2657: Fix config.js

* Merge PR cosmos#2589: Update Vesting Spec

* Merge PR cosmos#2656: Revert read-only leveldb database

* Revert read-only leveldb database

Waiting on a fix for syndtr/goleveldb#240.

* Update client/keys/utils.go

* Include DNS alt name in certificate

Closes: cosmos#2664

* Gaialite signal handling is broken, repair it

* Merge PR cosmos#2665: simulation: Remove header from Invariant

This got introduced recently, but wasn't actually needed, hence the reversion

* Merge PR cosmos#2653: Add benchmark for get and set account

* Fix test

* Refactor TrapSignal

* Fix lint

* enforcing @jaekwon mergemaster

* added querier redelegation

* added validatorDelegations querier endpoint

* LCD and CLI

* cli fixes

* removed redelegation stuff

* address other comments

* rebased

* addressed comments

* Make the simulator create the new comission rate sensibly

* Update to TM v0.26.0 - Part I (cosmos#2679)

* Update to TM v0.26.0

* Bez/tm0.26 update pt 2 redux (cosmos#2684)

* Update to TM v0.26.0
* Update TODOs
* Proof and verification updates
* Fix linting
* Fix key path creation
* Temporarily fix tendermint revision to make tests pass

* Fix merge conflict bug; Update PENDING

* New genesis workflow (cosmos#2602)

New genesis workflow:
* `gaiad init` is now used to generate an empty `genesis.json`.
* Genesis accounts need to be populated manually before running
  `gaiad collect-gentxs`.
* This should support starfish too, see cosmos#2615 for more info.
* Closes: cosmos#2596 cosmos#2615
* Validate validator address and address against respective account ex ante
* Fix local testnet failures
* New genesis tests
* Run make format
* Add --pubkey flag
* gaiad collect-gentxs takes no args

* Simulation improvements (logging fix, random genesis parameters) (cosmos#2617)

* Print out initial update on every block
* Randomize simulation parameters
* Randomize initial liveness weightings
* Randomize genesis parameters
* fixed power store invariant
* IterateValidatorsBonded -> IterateBondedValidatorsByPower
* WriteValidators uses IterateLastValidators rather than IterateBondedValidatorsByPower
* fixed democoin interface

Closes cosmos#2556
Closes cosmos#2396

Via cosmos#2671:
closes cosmos#2669
closes cosmos#2670
closes cosmos#2620

Offshoot issues:
cosmos#2618
cosmos#2619
cosmos#2620
cosmos#2661

* Fix simulation bugs; Incorprates cosmos#2676 from Sunny (cosmos#2677)

* Fix simulation bugs; Incorprates cosmos#2676 from Sunny
* Address review feedback; Update PENDING

* 'make format'

* Revert "enforcing @jaekwon mergemaster"

This reverts commit 15c2093.

* Update x/stake/client/rest/query.go

Co-Authored-By: sunnya97 <sunnya97@gmail.com>

* addressed fede's comment

* Switch gov proposal-queues to use iterators (cosmos#2638)

* switched gov proposals queue to use iterators
* update gov spec
* update proposal.Equal
* Amino api change
* switched proposalID to uint64
* renamed Gov Procedures to Params
* s/ActiveProposalQueueProposalKey/KeyActiveProposalQueueProposal/g
* numLatestProposals -> Limit
* fixed staking invariant breakage because of gov deposits
* Send deposits to DepositedCoinsAccAddr or BurnedDepositCoinsAccAddr

* Add general merkle absence proof (also for empty substores) (cosmos#2685)

* Fix coins.IsLT() impl (cosmos#2686)

* Fix coins.IsLT() impl
* Fix coin.IsLT() impl
* Coins.IsLT -> Coins.IsAllLT etc

* Update testnet to use canonical genesis time (cosmos#2692)

* Update testnet to use canonical genesis time
* Fix linting in genesis test

* Do not allow nil values to be set in CacheKVStore (cosmos#2708)

* Do not allow nil values to be set in CacheKVStore

* Makefile OS compatibility update

* Merge PR cosmos#2714: Add commission data to MsgCreateValidator signature bytes

* PENDING => CHANGELOG

* Linkify changelog

* Cleanup bank keeper

* whitespacing

* rand utile

...

* moving stuff around a bit, trying to get rid of types

* reorganize more

* rename ambig naming of queueOperations

* minimizing indentation

* fix some duplicate to get passing

* Address style comments

* Reorganize CLI command structure. Fixes cosmos#2575

* Fix missing flags issue

* Address linting issues

* Fix gobash CLI testing

* Fix typo

* Cross-compiling get_tools Makefile added

* operations functions

* assertAllInvarients changes, Operation reorg

* mock tendermint

* util cleanup

* event stats object, more general cleanup

* compiling

* pending

* Removed comment from Makefile as per bez's request

* val comments

* Address PR comments

* Update cmd/gaia/cmd/gaiacli/main.go

Co-Authored-By: jackzampolin <jack.zampolin@gmail.com>

* PENDING

* Fix state export/import, add to CI (cosmos#2690)

* Update slashing import/export
* More slashing.WriteGenesis
* Add test import/export to CI
* Store equality comparison.
* Fix validator bond intra-tx counter
* Set timeslices for unbonding validators
* WriteGenesis => ExportGenesis
* Delete validators from unbonding queue when re-bonded
* Hook for validator deletion, fix staking genesis tests

* Merge 0.26.0 back to develop (cosmos#2718)

* PENDING => CHANGELOG
* Linkify changelog
* Merge PR cosmos#2716: Temporarily disable gaia lite insecure mode
* TODO: need to update CHANGELOG w/ import-export PR cosmos#2690

* Update CHANGELOG/PENDING for straggling PR cosmos#2690

* Add small utility to add account to genesis.json after gaiad init

* Update CHANGELOG.md

* s/WriteGenesisFile/ExportGenesisFile/

* Update PENDING.md

* Add --chain-id to testnet command

* Address remaining comments from cosmos#2690

* Update PENDING.md

* add back in PeriodicInvariant

* Linter fix

* Fix TimeoutCommit (cosmos#2743)

* Fix TimeoutCommit to 5 seconds instead of whatever it was before which was too short.

* Gaia-9000: Update to TM 0.26.1-rc2 (cosmos#2753)

* Update to tm 0.26.1-rc2 to fix prometheus issue and node disconnect issue.

* Gaia-9000: Update to TM 0.26.1-rc3 -- pex SeedMode fix

* fix typo

I think it might be a spelling mistake

* Slight distribution spec cleanup

* More cleanup

* use defer

* Use correct Bech32 prefix for show-address command (cosmos#2746)

* Use consensus address bech32 prefix
* Update show-address CLI description

* Generate random moniker when missing

* Update moniker prefix

* Require moniker instead of generating a random one

* update to tendermint v0.26.1

* Fix test coverage

* Correctly set return code

* Fix date to be cross platform

* Merge PR cosmos#2752: Don't hardcode bondable denom

* R4R: Fix unbonding command flow (cosmos#2727)

* Fix required flag

* Fix redelegation command

* Add pending entry

* update swagger.yaml

* use newQuery...Params

* Link to issue

* Fix DiffKVStore

* Address PR review

* Working on stake import/export

* Only apply validator set updates on initial genesis

* Clarify comment

* Fix failing test

* add back in CLI command after rebase

* Fix CLI tests

* update to amino 0.14.1

* pending

* R4R:  Query Gov Params (cosmos#2576)

* gov query params

* Merge PR cosmos#2744: Fix Makefile targets dependencies

* Fix Makefile targets dependencies
* Remove unnecessary build deps from install targets
* Create a rule for each tool
* Don't dep test_lint on tools

* Update docs/spec/distribution/overview.md

Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>

* Update docs/spec/distribution/overview.md

Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>

* Update docs/spec/distribution/overview.md

Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>

* Update overview.md

* Documentation Structure Change and Cleanup (cosmos#2808)

* Update docs/sdk/clients.md
* organize ADR directory like tendermint
* docs: move spec-proposals into spec/
* remove lotion, moved to website repo
* move getting-started to cosmos-hub, and voyager to website
* docs: move lite/ into clients/lite/
* move introduction/ content to website repo
* move resources/ content to website repo
* mv sdk/clients.md to clients/clients.md
* mv validators to cosmos-hub/validators
* move deprecated sdk/ content to _attic
* sdk/modules.md is duplicate with modules/README.md
* consolidate remianing sdk/ files into a single sdk.md
* move examples/ to docs/examples/
* mv docs/cosmos-hub to docs/gaia
* Add keys/accounts section to localnet docs

* Bring back banner (cosmos#2814)

* Build docs in CircleCI  (cosmos#2810)

* error checking the API call
* added docs build trigger to circleci job

* Update contributing.md with new merge policy (cosmos#2789)

* Update contribuiting.md with new merge policy

* deleted obsolete file (cosmos#2817)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants