Skip to content

refactor(x/marker): cleanup decode/encode for pagination keys #2005

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.

Sign up for GitHub

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

Merged
troian merged 1 commit into from
Sep 17, 2025
Merged

refactor(x/marker): cleanup decode/encode for pagination keys #2005

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 62 additions & 70 deletions x/market/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ func (k Querier) Orders(c context.Context, req *types.QueryOrdersRequest) (*type

total := uint64(0)

for idx := range states {
state := types.Order_State(states[idx])
var err error
var idx int
var err error

for idx = range states {
state := types.Order_State(states[idx])
if idx > 0 {
req.Pagination.Key = nil
}
Expand Down Expand Up @@ -125,34 +126,27 @@ func (k Querier) Orders(c context.Context, req *types.QueryOrdersRequest) (*type
return nil, status.Error(codes.Internal, err.Error())
}

if len(pageRes.NextKey) > 0 {
nextKey := make([]byte, len(searchPrefix)+len(pageRes.NextKey))
copy(nextKey, searchPrefix)
copy(nextKey[len(searchPrefix):], pageRes.NextKey)

pageRes.NextKey = nextKey
}

req.Pagination.Limit -= count
total += count

if req.Pagination.Limit == 0 {
if len(pageRes.NextKey) > 0 {
pageRes.NextKey, err = query.EncodePaginationKey(states[idx:], searchPrefix, pageRes.NextKey, nil)
if err != nil {
pageRes.Total = total
return &types.QueryOrdersResponse{
Orders: orders,
Pagination: pageRes,
}, status.Error(codes.Internal, err.Error())
}
}

break
}
}

pageRes.Total = total
if pageRes != nil {
pageRes.Total = total

if len(pageRes.NextKey) > 0 {
pageRes.NextKey, err = query.EncodePaginationKey(states[idx:], searchPrefix, pageRes.NextKey, nil)
if err != nil {
return &types.QueryOrdersResponse{
Orders: orders,
Pagination: pageRes,
}, status.Error(codes.Internal, err.Error())
}
}
}
Comment on lines +137 to +149
Copy link

@coderabbitai coderabbitai bot Sep 15, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify backward compatibility of pagination keys and Total semantics.

Please confirm:

  • Old NextKey values still decode under the new scheme and lead to identical result sets.
  • Total is unchanged for clients when CountTotal is false.

Run:

Also applies to: 285-303, 442-460

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# 1) Find all Encode/DecodePaginationKey call sites
rg -nP --type=go 'EncodePaginationKey\(|DecodePaginationKey\(' -C2

# 2) Check for callers that rely on PageResponse.Total without CountTotal
rg -nP --type=go 'PageRequest\{[^}]*CountTotal:\s*false' -C3 || true
rg -nP --type=go 'Pagination:\s*&sdkquery\.PageRequest\{[^}]*CountTotal:\s*false' -C3 || true

# 3) Grep tests for pagination behavior
rg -nP --type=go '(NextKey|CountTotal|PageResponse\.Total)' -g '!**/vendor/**' -C3

Length of output: 7895

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== util/query/pagination.go =="
sed -n '1,240p' util/query/pagination.go || true

echo
echo "== util/query/pagination_test.go =="
sed -n '1,400p' util/query/pagination_test.go || true

echo
echo "== x/market/keeper/grpc_query.go (1..520) =="
sed -n '1,520p' x/market/keeper/grpc_query.go || true

echo
echo "== x/cert/keeper/grpc_query.go (1..240) =="
sed -n '1,240p' x/cert/keeper/grpc_query.go || true

echo
echo "== x/deployment/keeper/grpc_query.go (1..320) =="
sed -n '1,320p' x/deployment/keeper/grpc_query.go || true

echo
echo "== rg CountTotal occurrences =="
rg -n "CountTotal" || true

echo
echo "== rg pageRes.Total occurrences =="
rg -n "pageRes.Total" || true

echo
echo "== rg EncodePaginationKey / DecodePaginationKey definitions =="
rg -n "func (EncodePaginationKey|DecodePaginationKey|EncodePaginationKey|DecodePaginationKey)" util -S || true

Length of output: 34601

Critical: pagination key encoding breaks legacy NextKey and CountTotal semantics

  • Legacy NextKey incompatible — util/query/pagination.go::DecodePaginationKey requires the new checksum+structure and will return an error for raw NextKey values returned by older FilteredPaginate. Affected handlers: x/market/keeper/grpc_query.go (Decode at line ~55; Encode at ~141/294/452), x/cert/keeper/grpc_query.go (Decode at ~50; Encode at ~126), x/deployment/keeper/grpc_query.go (Decode at ~58; Encode at ~148). Action: add a backward-compatible fallback (e.g. on ErrInvalidPaginationKey treat the incoming key as legacy raw key and continue, or implement versioned keys).

  • CountTotal semantics violated — handlers set pageRes.Total = total unconditionally (x/market/... lines ~138/286/296/443/454; x/cert/... ~141; x/deployment/... ~162), so clients with CountTotal=false still receive totals. Action: only populate pageRes.Total when req.Pagination.CountTotal == true (or otherwise preserve legacy behavior).

🤖 Prompt for AI Agents 
In x/market/keeper/grpc_query.go around lines 137 to 149, the pagination
handling breaks compatibility: EncodePaginationKey produces a structured key
that DecodePaginationKey will reject if callers send legacy raw NextKey values,
and pageRes.Total is always set which violates CountTotal semantics. Fix by
catching the pagination decode/encode error (specifically treat
ErrInvalidPaginationKey as a legacy raw key and continue using the raw key bytes
instead of failing), and only populate pageRes.Total when req.Pagination != nil
&& req.Pagination.CountTotal == true (leave it unset otherwise) so
CountTotal=false preserves the legacy behavior.


return &types.QueryOrdersResponse{
Orders: orders,
Expand Down Expand Up @@ -196,7 +190,7 @@ func (k Querier) Bids(c context.Context, req *types.QueryBidsRequest) (*types.Qu
}
req.Pagination.Key = key

if unsolicited[1] == 1 {
if unsolicited[0] == 1 {
reverseSearch = true
}
} else if req.Filters.State != "" {
Expand All @@ -220,9 +214,11 @@ func (k Querier) Bids(c context.Context, req *types.QueryBidsRequest) (*types.Qu

total := uint64(0)

for idx := range states {
var idx int
var err error

for idx = range states {
state := types.Bid_State(states[idx])
var err error

if idx > 0 {
req.Pagination.Key = nil
Expand Down Expand Up @@ -278,41 +274,34 @@ func (k Querier) Bids(c context.Context, req *types.QueryBidsRequest) (*types.Qu
return nil, status.Error(codes.Internal, err.Error())
}

if len(pageRes.NextKey) > 0 {
nextKey := make([]byte, len(searchPrefix)+len(pageRes.NextKey))
copy(nextKey, searchPrefix)
copy(nextKey[len(searchPrefix):], pageRes.NextKey)

pageRes.NextKey = nextKey
}

req.Pagination.Limit -= count
total += count

if req.Pagination.Limit == 0 {
if len(pageRes.NextKey) > 0 {
unsolicited := make([]byte, 1)
unsolicited[0] = 0
if reverseSearch {
unsolicited[0] = 1
}
break
}
}

pageRes.NextKey, err = query.EncodePaginationKey(states[idx:], searchPrefix, pageRes.NextKey, unsolicited)
if err != nil {
pageRes.Total = total
return &types.QueryBidsResponse{
Bids: bids,
Pagination: pageRes,
}, status.Error(codes.Internal, err.Error())
}
if pageRes != nil {
pageRes.Total = total
if len(pageRes.NextKey) > 0 {
unsolicited := make([]byte, 1)
unsolicited[0] = 0
if reverseSearch {
unsolicited[0] = 1
}

break
pageRes.NextKey, err = query.EncodePaginationKey(states[idx:], searchPrefix, pageRes.NextKey, unsolicited)
if err != nil {
pageRes.Total = total
return &types.QueryBidsResponse{
Bids: bids,
Pagination: pageRes,
}, status.Error(codes.Internal, err.Error())
}
}
}

pageRes.Total = total

return &types.QueryBidsResponse{
Bids: bids,
Pagination: pageRes,
Expand Down Expand Up @@ -380,9 +369,10 @@ func (k Querier) Leases(c context.Context, req *types.QueryLeasesRequest) (*type

total := uint64(0)

for idx := range states {
var idx int
var err error
for idx = range states {
state := types.Lease_State(states[idx])
var err error

if idx > 0 {
req.Pagination.Key = nil
Expand Down Expand Up @@ -445,29 +435,31 @@ func (k Querier) Leases(c context.Context, req *types.QueryLeasesRequest) (*type
total += count

if req.Pagination.Limit == 0 {
if len(pageRes.NextKey) > 0 {
unsolicited := make([]byte, 1)
unsolicited[0] = 0
if reverseSearch {
unsolicited[0] = 1
}
break
}
}

pageRes.NextKey, err = query.EncodePaginationKey(states[idx:], searchPrefix, pageRes.NextKey, unsolicited)
if err != nil {
pageRes.Total = total
return &types.QueryLeasesResponse{
Leases: leases,
Pagination: pageRes,
}, status.Error(codes.Internal, err.Error())
}
if pageRes != nil {
pageRes.Total = total

if len(pageRes.NextKey) > 0 {
unsolicited := make([]byte, 1)
unsolicited[0] = 0
if reverseSearch {
unsolicited[0] = 1
}

break
pageRes.NextKey, err = query.EncodePaginationKey(states[idx:], searchPrefix, pageRes.NextKey, unsolicited)
if err != nil {
pageRes.Total = total
return &types.QueryLeasesResponse{
Leases: leases,
Pagination: pageRes,
}, status.Error(codes.Internal, err.Error())
}
}
}

pageRes.Total = total

return &types.QueryLeasesResponse{
Leases: leases,
Pagination: pageRes,
Expand Down
Loading