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

Expect bank/balances use latest height when height param not specified #4903

Closed
4 tasks
whunmr opened this issue Aug 14, 2019 · 4 comments · Fixed by #4905
Closed
4 tasks

Expect bank/balances use latest height when height param not specified #4903

whunmr opened this issue Aug 14, 2019 · 4 comments · Fixed by #4905
Labels

Comments

@whunmr
Copy link

whunmr commented Aug 14, 2019

Summary of Bug

When height param not specified, do not using the latest height in REST API bank/balances
and returns error.

related with #4891

BJ00609:~$ curl http://localhost:1317/bank/balances/cosmos1w06eekstpe4njyrvx94ehal7yttcs5lm06a0sl?height=0
{"error":"cannot query with proof when height \u003c= 1; please provide a valid height"}
BJ00609:~$ curl http://localhost:1317/bank/balances/cosmos1w06eekstpe4njyrvx94ehal7yttcs5lm06a0sl?height=1
{"error":"cannot query with proof when height \u003c= 1; please provide a valid height"}
BJ00609:~$ curl http://localhost:1317/bank/balances/cosmos1w06eekstpe4njyrvx94ehal7yttcs5lm06a0sl
{"error":"cannot query with proof when height \u003c= 1; please provide a valid height"}
BJ00609:~$ curl http://localhost:1317/bank/balances/cosmos1w06eekstpe4njyrvx94ehal7yttcs5lm06a0sl?height=2
{"height":"2","result":[
  {
    "denom": "stake",
    "amount": "100000000"
  },
  {
    "denom": "validatortoken",
    "amount": "900000000"
  }
]}
BJ00609:~$ curl http://localhost:1317/bank/balances/cosmos1w06eekstpe4njyrvx94ehal7yttcs5lm06a0sl?height=2000
{"error":"{\"codespace\":\"sdk\",\"code\":1,\"message\":\"failed to load state at height 2000; version does not exist (latest height: 25)\"}"}

Expected behave

  • when height not specified, using the latest block height

Version

gaia v1.0.0

Steps to Reproduce

  • start gaiad
$ gaiad init --chain-id=testing testing
$ gaiacli keys add validator
$ gaiad add-genesis-account $(gaiacli keys show validator -a) 200000000stake,900000000validatortoken
$ gaiad gentx --name validator
$ gaiad collect-gentxs
$ gaiad start
  • start rest-server with trust-node=false
gaiacli rest-server --chain-id=testing  --laddr=tcp://localhost:1317  --node tcp://localhost:26657 --trust-node=false
  • curl request in Summary of Bug section

more context

when height param not specified in rest query,
ParseQueryHeightOrReturnBadRequest will set the default value as 0

func ParseQueryHeightOrReturnBadRequest(...) {
	heightStr := r.FormValue("height")
	if heightStr != "" {
...
	} else {
		cliCtx = cliCtx.WithHeight(0)
	}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@whunmr
Copy link
Author

whunmr commented Aug 14, 2019

In handleQueryCustom, when height==0, the height will be assigned as app.LastBlockHeight()

func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
	
	// when a client did not provide a query height, manually inject the latest
	if req.Height == 0 {
		req.Height = app.LastBlockHeight()
	}

But do not have similar logic in handleQueryStore which is called when we query
gaiacli query staking validator in #4891

=> 509:	func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
(dlv) args
req = github.com/tendermint/tendermint/abci/types.RequestQuery {Data: []uint8 len: 21, cap: 48, [...], Path: "/store/staking/key", Height: 0,...+4 more}

maybe we should add similar logic (assign height to app.LastBlockHeight when height == 0) in handleQueryStore

Then maybe we can remove the height restriction in CLIContext#query

func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, height int64, err error) {

--	if ctx.Height <= 1 && !ctx.TrustNode {
--		return res, height, errors.New("cannot query with proof when height <= 1; please provide a valid height")
--	}

@alexanderbez
Copy link
Contributor

Thanks @whunmr; the solution is quite simple. For now, just run your node with --trust-node until we release v0.36.1 with this fix.

@whunmr
Copy link
Author

whunmr commented Aug 14, 2019

Thanks for the fix and quick response :) @alexanderbez

@alexanderbez
Copy link
Contributor

Thank you!

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

Successfully merging a pull request may close this issue.

2 participants