From 2e4932d3c971177588b1fdc0605b4c838a261cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 9 Aug 2021 12:06:55 +0200 Subject: [PATCH 1/7] fix, update ABCI query to use request height fix ABCI Query to use request height instead of context height Add tests to test query height change --- client/query.go | 2 +- client/query_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 client/query_test.go diff --git a/client/query.go b/client/query.go index e79779ce571b..c37671246cba 100644 --- a/client/query.go +++ b/client/query.go @@ -77,7 +77,7 @@ func (ctx Context) queryABCI(req abci.RequestQuery) (abci.ResponseQuery, error) } opts := rpcclient.ABCIQueryOptions{ - Height: ctx.Height, + Height: req.Height, Prove: req.Prove, } diff --git a/client/query_test.go b/client/query_test.go new file mode 100644 index 000000000000..6dd74fcd9ca6 --- /dev/null +++ b/client/query_test.go @@ -0,0 +1,35 @@ +// +build norace + +package client_test + +import ( + "fmt" + + abci "github.com/tendermint/tendermint/abci/types" + + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" +) + +func (s *IntegrationTestSuite) TestQueryABCIHeight() { + // test ABCI query uses request height argument + // instead of client context height + contextHeight := 1 // query at height 1 or 2 would cause an error + reqHeight := int64(10) + + val := s.network.Validators[0] + clientCtx := val.ClientCtx + clientCtx = clientCtx.WithHeight(contextHeight) + + req := abci.RequestQuery{ + Path: fmt.Sprintf("store/%s/key", banktypes.StoreKey), + Height: reqHeight, + Data: banktypes.CreateAccountBalancesPrefix(val.Address), + Prove: true, + } + + res, err := clientCtx.QueryABCI(req) + s.Require().NoError(err) + + s.Require().Equal(reqHeight, res.Height) + +} From 15c227069a01c2d586f25c4edc102423aa2c9fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 9 Aug 2021 12:09:52 +0200 Subject: [PATCH 2/7] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 630275aa8a08..d58261ca3acb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Client Breaking Changes +* [\#9879](https://github.com/cosmos/cosmos-sdk/pull/9879) Modify ABCI Queries to use `abci.QueryRequest` Height field instead of context height. * [\#9859](https://github.com/cosmos/cosmos-sdk/pull/9859) The `default` pruning strategy now keeps the last 362880 blocks instead of 100. 362880 equates to roughly enough blocks to cover the entire unbonding period assuming a 21 day unbonding period and 5s block time. * [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) Remove legacy REST API. Please see the [REST Endpoints Migration guide](https://docs.cosmos.network/master/migrations/rest.html) to migrate to the new REST endpoints. * [\#9781](https://github.com/cosmos/cosmos-sdk/pull/9781) Improve`withdraw-all-rewards` UX when broadcast mode `async` or `async` is used. From a385b75446bb3f5838654715f501732fa8353ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 9 Aug 2021 15:55:03 +0200 Subject: [PATCH 3/7] Update client/query_test.go Co-authored-by: Aleksandr Bezobchuk --- client/query_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/query_test.go b/client/query_test.go index 6dd74fcd9ca6..653335fc7aae 100644 --- a/client/query_test.go +++ b/client/query_test.go @@ -31,5 +31,4 @@ func (s *IntegrationTestSuite) TestQueryABCIHeight() { s.Require().NoError(err) s.Require().Equal(reqHeight, res.Height) - } From e4f28fdd2b073b76f7493bd0d9e94247daa9f52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 9 Aug 2021 17:09:48 +0200 Subject: [PATCH 4/7] fix tests --- client/query_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/query_test.go b/client/query_test.go index 653335fc7aae..eba3beaadf36 100644 --- a/client/query_test.go +++ b/client/query_test.go @@ -13,10 +13,12 @@ import ( func (s *IntegrationTestSuite) TestQueryABCIHeight() { // test ABCI query uses request height argument // instead of client context height - contextHeight := 1 // query at height 1 or 2 would cause an error - reqHeight := int64(10) + contextHeight := int64(1) // query at height 1 or 2 would cause an error + reqHeight := int64(3) + s.network.WaitForHeight(reqHeight) val := s.network.Validators[0] + clientCtx := val.ClientCtx clientCtx = clientCtx.WithHeight(contextHeight) From 860572c5d48616fd206ff2adc229540a92bae012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 11 Aug 2021 17:35:29 +0200 Subject: [PATCH 5/7] apply review suggestions from Bez Set request height in abciReq for RunGRPCQuery Only use the request height in the ABCI Query if it is set, otherwise fallback to context height --- client/grpc_query.go | 5 +++-- client/query.go | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/client/grpc_query.go b/client/grpc_query.go index 011523944c54..cbaba73caa2a 100644 --- a/client/grpc_query.go +++ b/client/grpc_query.go @@ -112,8 +112,9 @@ func RunGRPCQuery(ctx Context, grpcCtx gocontext.Context, method string, req int } abciReq := abci.RequestQuery{ - Path: method, - Data: reqBz, + Path: method, + Data: reqBz, + Height: ctx.Height, } abciRes, err := ctx.QueryABCI(abciReq) diff --git a/client/query.go b/client/query.go index c37671246cba..be603dfbf6e2 100644 --- a/client/query.go +++ b/client/query.go @@ -50,7 +50,9 @@ func (ctx Context) QueryStore(key tmbytes.HexBytes, storeName string) ([]byte, i } // QueryABCI performs a query to a Tendermint node with the provide RequestQuery. -// It returns the ResultQuery obtained from the query. +// It returns the ResultQuery obtained from the query. The height used to perform +// the query is the RequestQuery Height if it is non-zero, otherwise the context +// height is used. func (ctx Context) QueryABCI(req abci.RequestQuery) (abci.ResponseQuery, error) { return ctx.queryABCI(req) } @@ -76,8 +78,16 @@ func (ctx Context) queryABCI(req abci.RequestQuery) (abci.ResponseQuery, error) return abci.ResponseQuery{}, err } + var queryHeight int64 + if req.Height != 0 { + queryHeight = req.Height + } else { + // fallback on the context height + queryHeight = ctx.Height + } + opts := rpcclient.ABCIQueryOptions{ - Height: req.Height, + Height: queryHeight, Prove: req.Prove, } From ac4edc5f0091387ed07b83b21a9fb0780a13cfa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 11 Aug 2021 17:52:15 +0200 Subject: [PATCH 6/7] update tests Use table test format and adding more test cases --- client/query_test.go | 65 +++++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/client/query_test.go b/client/query_test.go index eba3beaadf36..14cc25ba9d97 100644 --- a/client/query_test.go +++ b/client/query_test.go @@ -11,26 +11,53 @@ import ( ) func (s *IntegrationTestSuite) TestQueryABCIHeight() { - // test ABCI query uses request height argument - // instead of client context height - contextHeight := int64(1) // query at height 1 or 2 would cause an error - reqHeight := int64(3) - s.network.WaitForHeight(reqHeight) - - val := s.network.Validators[0] - - clientCtx := val.ClientCtx - clientCtx = clientCtx.WithHeight(contextHeight) - - req := abci.RequestQuery{ - Path: fmt.Sprintf("store/%s/key", banktypes.StoreKey), - Height: reqHeight, - Data: banktypes.CreateAccountBalancesPrefix(val.Address), - Prove: true, + + testCases := []struct { + name string + reqHeight int64 + ctxHeight int64 + expHeight int64 + }{ + { + name: "non zero request height", + reqHeight: 3, + ctxHeight: 1, // query at height 1 or 2 would cause an error + expHeight: 3, + }, + { + name: "empty request height - use context height", + reqHeight: 0, + ctxHeight: 3, + expHeight: 3, + }, + { + name: "empty request height and context height - use latest height", + reqHeight: 0, + ctxHeight: 0, + expHeight: 4, + }, } - res, err := clientCtx.QueryABCI(req) - s.Require().NoError(err) + for _, tc := range testCases { + s.Run(tc.name, func() { + s.network.WaitForHeight(tc.expHeight) + + val := s.network.Validators[0] + + clientCtx := val.ClientCtx + clientCtx = clientCtx.WithHeight(tc.ctxHeight) - s.Require().Equal(reqHeight, res.Height) + req := abci.RequestQuery{ + Path: fmt.Sprintf("store/%s/key", banktypes.StoreKey), + Height: tc.reqHeight, + Data: banktypes.CreateAccountBalancesPrefix(val.Address), + Prove: true, + } + + res, err := clientCtx.QueryABCI(req) + s.Require().NoError(err) + + s.Require().Equal(tc.expHeight, res.Height) + }) + } } From 2139665d5b9f4e5b0843be1b2253a987546175ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 11 Aug 2021 17:54:19 +0200 Subject: [PATCH 7/7] remove unnecessary changelog entry --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf4cfe908c51..296baebb8cbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,7 +70,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Client Breaking Changes * [\#9879](https://github.com/cosmos/cosmos-sdk/pull/9879) Modify ABCI Queries to use `abci.QueryRequest` Height field if it is non-zero, otherwise continue using context height. -* [\#9859](https://github.com/cosmos/cosmos-sdk/pull/9859) The `default` pruning strategy now keeps the last 362880 blocks instead of 100. 362880 equates to roughly enough blocks to cover the entire unbonding period assuming a 21 day unbonding period and 5s block time. * [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) Remove legacy REST API. Please see the [REST Endpoints Migration guide](https://docs.cosmos.network/master/migrations/rest.html) to migrate to the new REST endpoints. ### CLI Breaking Changes