From 74435137e9bcfeba7c2022b7aafe176bb632ccc5 Mon Sep 17 00:00:00 2001 From: Amaury Date: Tue, 24 Nov 2020 11:28:12 +0100 Subject: [PATCH] Show height on legacy REST endpoints (#7997) * Fix showing height in rest endpoints * Fix query height * Manually inject req.Height * Small tweaks * Use withHeight Co-authored-by: Aleksandr Bezobchuk Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- baseapp/abci.go | 10 +++---- client/errors.go | 22 ---------------- client/rpc/block.go | 3 +-- x/bank/client/rest/query_test.go | 45 +++++++++++++++++++++++++------- 4 files changed, 41 insertions(+), 39 deletions(-) delete mode 100644 client/errors.go diff --git a/baseapp/abci.go b/baseapp/abci.go index ada95acf1f6..d988ead0fcf 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -382,6 +382,11 @@ func (app *BaseApp) snapshot(height int64) { func (app *BaseApp) Query(req abci.RequestQuery) abci.ResponseQuery { defer telemetry.MeasureSince(time.Now(), "abci", "query") + // when a client did not provide a query height, manually inject the latest + if req.Height == 0 { + req.Height = app.LastBlockHeight() + } + // handle gRPC routes first rather than calling splitPath because '/' characters // are used as part of gRPC paths if grpcHandler := app.grpcQueryRouter.Route(req.Path); grpcHandler != nil { @@ -742,11 +747,6 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.R req.Path = "/" + strings.Join(path[1:], "/") - // when a client did not provide a query height, manually inject the latest - if req.Height == 0 { - req.Height = app.LastBlockHeight() - } - if req.Height <= 1 && req.Prove { return sdkerrors.QueryResult( sdkerrors.Wrap( diff --git a/client/errors.go b/client/errors.go deleted file mode 100644 index d7f7e29664c..00000000000 --- a/client/errors.go +++ /dev/null @@ -1,22 +0,0 @@ -package client - -import ( - "fmt" - - sdk "github.com/cosmos/cosmos-sdk/types" -) - -// ErrInvalidAccount returns a standardized error reflecting that a given -// account address does not exist. -func ErrInvalidAccount(addr sdk.AccAddress) error { - return fmt.Errorf(`no account with address %s was found in the state. -Are you sure there has been a transaction involving it?`, addr) -} - -// ErrVerifyCommit returns a common error reflecting that the blockchain commit at a given -// height can't be verified. The reason is that the base checkpoint of the certifier is -// newer than the given height -func ErrVerifyCommit(height int64) error { - return fmt.Errorf(`the height of base truststore in the light client is higher than height %d. -Can't verify blockchain proof at this height. Please set --trust-node to true and try again`, height) -} diff --git a/client/rpc/block.go b/client/rpc/block.go index a6a42967ce9..8bf7f12af8b 100644 --- a/client/rpc/block.go +++ b/client/rpc/block.go @@ -11,7 +11,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" - "github.com/cosmos/cosmos-sdk/codec/legacy" "github.com/cosmos/cosmos-sdk/types/rest" ) @@ -68,7 +67,7 @@ func getBlock(clientCtx client.Context, height *int64) ([]byte, error) { return nil, err } - return legacy.Cdc.MarshalJSON(res) + return clientCtx.LegacyAmino.MarshalJSON(res) } // get the current blockchain height diff --git a/x/bank/client/rest/query_test.go b/x/bank/client/rest/query_test.go index bbd803ba752..5ca3a15be50 100644 --- a/x/bank/client/rest/query_test.go +++ b/x/bank/client/rest/query_test.go @@ -29,7 +29,7 @@ func (s *IntegrationTestSuite) SetupSuite() { s.cfg = cfg s.network = network.New(s.T(), cfg) - _, err := s.network.WaitForHeight(1) + _, err := s.network.WaitForHeight(2) s.Require().NoError(err) } @@ -43,14 +43,26 @@ func (s *IntegrationTestSuite) TestQueryBalancesRequestHandlerFn() { baseURL := val.APIAddress testCases := []struct { - name string - url string - respType fmt.Stringer - expected fmt.Stringer + name string + url string + expHeight int64 + respType fmt.Stringer + expected fmt.Stringer }{ { "total account balance", + fmt.Sprintf("%s/bank/balances/%s", baseURL, val.Address), + -1, + &sdk.Coins{}, + sdk.NewCoins( + sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), s.cfg.AccountTokens), + sdk.NewCoin(s.cfg.BondDenom, s.cfg.StakingTokens.Sub(s.cfg.BondedTokens)), + ), + }, + { + "total account balance with height", fmt.Sprintf("%s/bank/balances/%s?height=1", baseURL, val.Address), + 1, &sdk.Coins{}, sdk.NewCoins( sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), s.cfg.AccountTokens), @@ -59,13 +71,15 @@ func (s *IntegrationTestSuite) TestQueryBalancesRequestHandlerFn() { }, { "total account balance of a specific denom", - fmt.Sprintf("%s/bank/balances/%s?height=1&denom=%s", baseURL, val.Address, s.cfg.BondDenom), + fmt.Sprintf("%s/bank/balances/%s?denom=%s", baseURL, val.Address, s.cfg.BondDenom), + -1, &sdk.Coin{}, sdk.NewCoin(s.cfg.BondDenom, s.cfg.StakingTokens.Sub(s.cfg.BondedTokens)), }, { "total account balance of a bogus denom", - fmt.Sprintf("%s/bank/balances/%s?height=1&denom=foobar", baseURL, val.Address), + fmt.Sprintf("%s/bank/balances/%s?denom=foobar", baseURL, val.Address), + -1, &sdk.Coin{}, sdk.NewCoin("foobar", sdk.ZeroInt()), }, @@ -74,12 +88,23 @@ func (s *IntegrationTestSuite) TestQueryBalancesRequestHandlerFn() { for _, tc := range testCases { tc := tc s.Run(tc.name, func() { - resp, err := rest.GetRequest(tc.url) + respJSON, err := rest.GetRequest(tc.url) s.Require().NoError(err) - bz, err := rest.ParseResponseWithHeight(val.ClientCtx.LegacyAmino, resp) + var resp = rest.ResponseWithHeight{} + err = val.ClientCtx.LegacyAmino.UnmarshalJSON(respJSON, &resp) s.Require().NoError(err) - s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(bz, tc.respType)) + + // Check height. + if tc.expHeight >= 0 { + s.Require().Equal(resp.Height, tc.expHeight) + } else { + // To avoid flakiness, just test that height is positive. + s.Require().Greater(resp.Height, int64(0)) + } + + // Check result. + s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(resp.Result, tc.respType)) s.Require().Equal(tc.expected.String(), tc.respType.String()) }) }