Skip to content

Commit 1f34619

Browse files
committed
fix(x/auth/tx): honor deprecated Pagination field in GetTxsEvent
GetTxsEvent only consumed the top-level Page/Limit/OrderBy fields and silently ignored req.Pagination. REST clients hitting the gRPC-gateway endpoint with pagination.limit / pagination.offset / pagination.reverse got DefaultLimit-sized ascending pages no matter what they passed, which is what made paginated transfer.recipient queries appear to drop matching txs. Treat the deprecated fields as a fallback when the top-level fields are unset: limit -> req.Pagination.Limit, page -> Offset/Limit + 1 (with an InvalidArgument when Offset is not a multiple of Limit), and Reverse maps to OrderBy_DESC. Top-level fields still win when both are set. Adds a service_test.go with a recording TxSearch client covering each fallback path plus the precedence rule. Closes #25886
1 parent 0027c36 commit 1f34619

2 files changed

Lines changed: 168 additions & 2 deletions

File tree

x/auth/tx/service.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,39 @@ func (s txServer) GetTxsEvent(_ context.Context, req *txtypes.GetTxsEventRequest
4848
return nil, status.Error(codes.InvalidArgument, "request cannot be nil")
4949
}
5050

51-
orderBy := parseOrderBy(req.OrderBy)
51+
page, limit, orderBy := req.Page, req.Limit, parseOrderBy(req.OrderBy)
5252

53-
result, err := QueryTxsByEvents(s.clientCtx, int(req.Page), int(req.Limit), req.Query, orderBy)
53+
// Backwards-compatibility: the top-level page/limit/order_by fields supersede
54+
// the deprecated Pagination message, but if a caller only set the deprecated
55+
// fields (common when consuming the gRPC-gateway REST endpoint with
56+
// pagination.limit / pagination.offset / pagination.reverse query params),
57+
// fall back to those values rather than silently ignoring them. See #25886.
58+
if req.Pagination != nil {
59+
if limit == 0 && req.Pagination.Limit > 0 {
60+
limit = req.Pagination.Limit
61+
}
62+
if page == 0 {
63+
effectiveLimit := limit
64+
if effectiveLimit == 0 {
65+
effectiveLimit = query.DefaultLimit
66+
}
67+
if req.Pagination.Offset > 0 {
68+
if req.Pagination.Offset%effectiveLimit != 0 {
69+
return nil, status.Errorf(
70+
codes.InvalidArgument,
71+
"pagination.offset (%d) must be a multiple of pagination.limit (%d)",
72+
req.Pagination.Offset, effectiveLimit,
73+
)
74+
}
75+
page = req.Pagination.Offset/effectiveLimit + 1
76+
}
77+
}
78+
if orderBy == "" && req.Pagination.Reverse {
79+
orderBy = "desc"
80+
}
81+
}
82+
83+
result, err := QueryTxsByEvents(s.clientCtx, int(page), int(limit), req.Query, orderBy)
5484
if err != nil {
5585
return nil, status.Error(codes.Internal, err.Error())
5686
}

x/auth/tx/service_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
package tx_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
coretypes "github.com/cometbft/cometbft/rpc/core/types"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/cosmos/cosmos-sdk/client"
11+
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
12+
"github.com/cosmos/cosmos-sdk/types/query"
13+
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
14+
"github.com/cosmos/cosmos-sdk/x/auth/tx"
15+
)
16+
17+
// recordingTxSearchClient captures the page/limit/orderBy passed to TxSearch
18+
// so tests can assert how GetTxsEvent translated the request.
19+
type recordingTxSearchClient struct {
20+
client.MockClient
21+
22+
page *int
23+
limit *int
24+
orderBy string
25+
}
26+
27+
func (c *recordingTxSearchClient) TxSearch(
28+
_ context.Context,
29+
_ string,
30+
_ bool,
31+
page, limit *int,
32+
orderBy string,
33+
) (*coretypes.ResultTxSearch, error) {
34+
if page != nil {
35+
v := *page
36+
c.page = &v
37+
}
38+
if limit != nil {
39+
v := *limit
40+
c.limit = &v
41+
}
42+
c.orderBy = orderBy
43+
return &coretypes.ResultTxSearch{Txs: nil, TotalCount: 0}, nil
44+
}
45+
46+
func newRecordingCtx() (*recordingTxSearchClient, client.Context) {
47+
rc := &recordingTxSearchClient{}
48+
ctx := client.Context{}.
49+
WithClient(rc).
50+
WithCmdContext(context.Background()).
51+
WithInterfaceRegistry(codectypes.NewInterfaceRegistry())
52+
return rc, ctx
53+
}
54+
55+
// TestGetTxsEvent_HonorsDeprecatedPagination ensures that when callers only
56+
// populate the deprecated Pagination field (e.g. REST clients passing
57+
// pagination.limit / pagination.offset / pagination.reverse), the values are
58+
// forwarded to TxSearch instead of being silently dropped. Regression for
59+
// https://github.com/cosmos/cosmos-sdk/issues/25886.
60+
func TestGetTxsEvent_HonorsDeprecatedPagination(t *testing.T) {
61+
cases := []struct {
62+
name string
63+
req *txtypes.GetTxsEventRequest
64+
wantPage int
65+
wantLimit int
66+
wantOrder string
67+
wantErrMsg string
68+
}{
69+
{
70+
name: "deprecated limit honored when top-level limit is zero",
71+
req: &txtypes.GetTxsEventRequest{
72+
Query: "tx.height=1",
73+
Pagination: &query.PageRequest{Limit: 250},
74+
},
75+
wantPage: 1,
76+
wantLimit: 250,
77+
},
78+
{
79+
name: "deprecated offset translates to page",
80+
req: &txtypes.GetTxsEventRequest{
81+
Query: "tx.height=1",
82+
Pagination: &query.PageRequest{Limit: 100, Offset: 200},
83+
},
84+
wantPage: 3,
85+
wantLimit: 100,
86+
},
87+
{
88+
name: "deprecated reverse maps to desc order",
89+
req: &txtypes.GetTxsEventRequest{
90+
Query: "tx.height=1",
91+
Pagination: &query.PageRequest{Limit: 50, Reverse: true},
92+
},
93+
wantPage: 1,
94+
wantLimit: 50,
95+
wantOrder: "desc",
96+
},
97+
{
98+
name: "top-level page/limit win over deprecated fields",
99+
req: &txtypes.GetTxsEventRequest{
100+
Query: "tx.height=1",
101+
Page: 2,
102+
Limit: 25,
103+
Pagination: &query.PageRequest{Limit: 999, Offset: 9000},
104+
},
105+
wantPage: 2,
106+
wantLimit: 25,
107+
},
108+
{
109+
name: "non-multiple offset rejected",
110+
req: &txtypes.GetTxsEventRequest{
111+
Query: "tx.height=1",
112+
Pagination: &query.PageRequest{Limit: 100, Offset: 150},
113+
},
114+
wantErrMsg: "must be a multiple of",
115+
},
116+
}
117+
118+
for _, tc := range cases {
119+
t.Run(tc.name, func(t *testing.T) {
120+
rc, ctx := newRecordingCtx()
121+
srv := tx.NewTxServer(ctx, nil, codectypes.NewInterfaceRegistry())
122+
_, err := srv.GetTxsEvent(context.Background(), tc.req)
123+
if tc.wantErrMsg != "" {
124+
require.Error(t, err)
125+
require.Contains(t, err.Error(), tc.wantErrMsg)
126+
return
127+
}
128+
require.NoError(t, err)
129+
require.NotNil(t, rc.page, "page not forwarded to TxSearch")
130+
require.NotNil(t, rc.limit, "limit not forwarded to TxSearch")
131+
require.Equal(t, tc.wantPage, *rc.page)
132+
require.Equal(t, tc.wantLimit, *rc.limit)
133+
require.Equal(t, tc.wantOrder, rc.orderBy)
134+
})
135+
}
136+
}

0 commit comments

Comments
 (0)