feat: implement pagination for x/wasm query #85

shiki-tak · 2021-03-10T11:36:32Z

Closes: https://github.com/line/link-modules/issues/85
Ref: https://github.com/line/link-modules/pull/87

Description

I changed the structure of contract_history store and added pagination query.
In cosmos-sdk v0.40.0 and later, the sdk defines a flag and protobuf for pagination, but this is not implemented in the current version, so I added my own flag by referring to the following PR.

FYI: Add pagination to grpc queries CosmWasm/wasmd#317
- Use FilteredPaginate
  https://github.com/cosmos/cosmos-sdk/blob/f3e4964f2f9cdf5d65c819d689950bffec728c65/types/query/filtered_pagination.go#L17
- Moved contract_history queries prior to v0.40.0 that use path to legacy_querier.go.
  - However, the current version of cosmos-sdk in link-modules is 0.39.x, so it is not possible to define queries with protobufs.
  - Therefore, the new query is prepared in querier.go with the name queryContractHistory2, and the newly defined GetContractHistory is called from there.
    - https://github.com/line/link-modules/pull/87/files#diff-0b0d34c72a5d14e64c41fb93a58eb475a8213cf4b8fa7cd3e50f869ea6886b9bR167

Format

# not use flag
❯ linkcli query wasm contract-history $CONTRACT
{
  "Entries": [
    {
      "operation": "Init",
      "code_id": "1",
      "msg": {}
    },
    {
      "operation": "Migrate",
      "code_id": "2",
      "msg": {
        "payout": "link1vrgaj0qzjpjfsmanjzljdkwclas8zatcmnrqfg"
      }
    }
  ],
  "Pagination": {
    "NextKey": null,
    "Total": "2"
  }
}

# use flag
❯ linkcli query wasm contract-history $CONTRACT --page 2 --limit 1 --count-total=true
{
  "Entries": [
    {
      "operation": "Migrate",
      "code_id": "2",
      "msg": {
        "payout": "link1vrgaj0qzjpjfsmanjzljdkwclas8zatcmnrqfg"
      }
    }
  ],
  "Pagination": {
    "NextKey": null,
    "Total": "2"
  }
}

Motivation and context

How has this been tested?

Checklist:

I followed the contributing guidelines.
I have updated the documentation accordingly.
I have added tests to cover my changes.

LINK-Network-Dev · 2021-03-10T11:42:22Z

tps:389

loloicci · 2021-03-11T03:13:07Z

x/wasm/client/utils/utils.go

+		if err != nil {
+			return pageKey, offset, limit, page, countTotal, err
+		}
+		if offset <= 0 {


other than here, used else if

FYI, https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow

whylee259 · 2021-03-11T09:09:59Z

x/wasm/client/cli/query.go

+			if err != nil {
+				return err
+			}
+
 			route := fmt.Sprintf("custom/%s/%s/%s", types.QuerierRoute, keeper.QueryContractHistory, addr.String())


I think address information is now included in the data field so that we can diet route path.

yes! it removed.

whylee259 · 2021-03-11T09:17:32Z

x/wasm/client/rest/query.go

+		data := &types.QueryCodesRequest{
+			Pagination: pageReq,
+		}
+		bs, err := cliCtx.Codec.MarshalJSON(data)
+		if err != nil {
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+			return
+		}
+
+		res, height, err := cliCtx.QueryWithData(route, bs)


Can we reduce the duplicate codes?
cli/query.go#L60-L68

It is summarized in utils.

whylee259 · 2021-03-11T09:17:55Z

x/wasm/client/rest/query.go

+			return
+		}
+
+		data := &types.QueryContractsByCodeRequest{


whylee259 · 2021-03-11T09:18:09Z

x/wasm/client/rest/query.go

+			return
+		}
+
+		data := &types.QueryContractHistoryRequest{


whylee259 · 2021-03-11T09:22:59Z

x/wasm/client/utils/utils.go

+	}, nil
+}
+
+func ParseHTTPArgs(r *http.Request) (pageKey string, offset, limit, page uint64, countTotal bool, err error) {


The pagination utility for HTTP args does already exist. Can we use it for API consistency?

https://github.com/line/lbm-sdk/blob/develop/types/rest/rest.go#L382

The limit and page could be parseed from the already existing ParseHTTPArgs.

LINK-Network-Dev · 2021-03-11T12:20:46Z

tps:309

whylee259

I have left a minor comment.

whylee259 · 2021-03-12T05:18:49Z

x/wasm/internal/keeper/keeper.go

+
+// NOTE: This function is implemented in cosmos-sdk v0.40.0.
+// If you want to update cosmos-sdk to 0.40.0 or later, you can use the sdk function.
+func FilteredPaginate(


I think this function is part of the query feature, so seems to be better to move it to keeper/querier.go

LINK-Network-Dev · 2021-03-12T06:10:39Z

tps:304

whylee259 · 2021-03-12T06:24:30Z

x/wasm/client/utils/utils.go

+		page, err = strconv.ParseUint(pageStr, 10, 64)
+		if err != nil {
+			return pageKey, offset, limit, page, countTotal, err
+		} else if page <= 0 {


Minor comment. As the previous doc says, the following suggestion is better.

Suggested change

} else if page <= 0 {

}

if page <= 0 {

Question: does page index start from 1, not 0?

whylee259 · 2021-03-12T06:25:08Z

x/wasm/client/utils/utils.go

+		limit, err = strconv.ParseUint(limitStr, 10, 64)
+		if err != nil {
+			return pageKey, offset, limit, page, countTotal, err
+		} else if limit <= 0 {


Suggested change

} else if limit <= 0 {

}

if limit <= 0 {

ditto

x/wasm/client/utils/utils.go

LINK-Network-Dev · 2021-03-12T07:58:46Z

tps:364

LINK-Network-Dev · 2021-03-12T08:11:56Z

tps:340

LINK-Network-Dev · 2021-03-15T01:54:53Z

tps:358

The original is the PR #85. cherry-pick from v1/develop by Jiyong Ha<[email protected]>

* feat(wasm): add pagination for rest api The original is the PR #85. cherry-pick from v1/develop by Jiyong Ha<[email protected]> * feat(wasm): make hardcoded params to gov params The original is the PR #86. with excessive gas consumption issue needs further modification. cherry-pick from v1/develop by Jiyong Ha<[email protected]> * test(wasm): adjust gas by gov params Limit values are greatly eased due to the known issue of gas consumption. * chore: go fmt * chore: fix typo Co-authored-by: shiki <[email protected]>

shiki-tak added 8 commits March 10, 2021 18:04

feat: add legacy_querier

5eaac52

feat: add pagination query for contract history

0421c56

feat: add pagination for rest api

fda259e

fix: modify unit test

1f0c296

fix: apply review

87b8e1b

feat: add pagination for queryContractListByCode, queryCodeList

9bf9b68

fix: remove legacy_querier

37abfeb

fix: fix integration test error

d270cf6

shiki-tak added the C:WASM label Mar 10, 2021

shiki-tak requested review from whylee259, loloicci and brew0722 March 10, 2021 11:36

shiki-tak self-assigned this Mar 10, 2021

loloicci reviewed Mar 11, 2021

View reviewed changes

whylee259 reviewed Mar 11, 2021

View reviewed changes

fix: apply review

b3f69cf

whylee259 reviewed Mar 12, 2021

View reviewed changes

whylee259 approved these changes Mar 12, 2021

View reviewed changes

refactor: move filteredPaginate to keeper/querier.go

d037d18

whylee259 reviewed Mar 12, 2021

View reviewed changes

brew0722 reviewed Mar 12, 2021

View reviewed changes

x/wasm/client/utils/utils.go Show resolved Hide resolved

fix: fix offset condition

c35995a

fix: fix lint error

0d36346

brew0722 approved these changes Mar 15, 2021

View reviewed changes

Merge branch 'develop' of github.com:line/lbm-sdk into issues/85

365b485

shiki-tak merged commit d06d4ee into Finschia:develop Mar 15, 2021

brew0722 pushed a commit that referenced this pull request Apr 27, 2021

feat(wasm): add pagination for rest api

3c41271

The original is the PR #85. cherry-pick from v1/develop by Jiyong Ha<[email protected]>

brew0722 mentioned this pull request Apr 27, 2021

feat(wasm): porting our modifications after 0.12.0-0.1.0 #139

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement pagination for x/wasm query #85

feat: implement pagination for x/wasm query #85

shiki-tak commented Mar 10, 2021 •

edited

Loading

LINK-Network-Dev commented Mar 10, 2021

loloicci Mar 11, 2021

whylee259 Mar 11, 2021

whylee259 Mar 11, 2021

shiki-tak Mar 11, 2021

whylee259 Mar 11, 2021

shiki-tak Mar 11, 2021

whylee259 Mar 11, 2021

shiki-tak Mar 11, 2021

whylee259 Mar 11, 2021

shiki-tak Mar 11, 2021

whylee259 Mar 11, 2021

shiki-tak Mar 11, 2021

LINK-Network-Dev commented Mar 11, 2021

whylee259 left a comment

whylee259 Mar 12, 2021

LINK-Network-Dev commented Mar 12, 2021

whylee259 Mar 12, 2021

loloicci Mar 12, 2021

whylee259 Mar 12, 2021

LINK-Network-Dev commented Mar 12, 2021

LINK-Network-Dev commented Mar 12, 2021

LINK-Network-Dev commented Mar 15, 2021

feat: implement pagination for x/wasm query #85

feat: implement pagination for x/wasm query #85

Conversation

shiki-tak commented Mar 10, 2021 • edited Loading

Description

Motivation and context

How has this been tested?

Checklist:

LINK-Network-Dev commented Mar 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LINK-Network-Dev commented Mar 11, 2021

whylee259 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LINK-Network-Dev commented Mar 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LINK-Network-Dev commented Mar 12, 2021

LINK-Network-Dev commented Mar 12, 2021

LINK-Network-Dev commented Mar 15, 2021

shiki-tak commented Mar 10, 2021 •

edited

Loading