Skip to content

Commit eea0b48

Browse files
amir-derisclaude
andauthored
fix(query): fix off-by-one when iteration budget exhausts at offset (#4629)
Untrusted unfiltered offset queries were rejected with `InvalidArgument` ("scanned more than N entries before reaching offset") when `max-iterations` equalled the requested offset, even though the offset had been fully skipped at that point. The correct response is an empty page whose `next_key` resumes at the first entry of the page. Defaults cannot reach this (`max-iterations` 11000 > `max-offset` 10000), and `ValidateQueryConfig` already warns about such configs, so it needs an operator who ignores that warning. Follow-up to #3984 review; fixes PLT-1485. The cause is that `offsetScanCursor.phase()` meant different things at its two call sites. The loop body calls it after `beginIteration` increments `scanned`, where `scanned <= offset` correctly means "this entry is skipped". The budget-exhaust check in `beginIteration` calls it before the increment, where the same predicate is off by one. Rather than overload `phase()` further, the exhaust check now uses a new `offsetConsumed()` predicate, so `phase()` keeps a single meaning. The filtered path is unaffected: `offsetConsumed()` reduces to the existing `hits < offset` check there. No consensus, state, or wire-format impact; this only changes the response to a narrow class of ABCI queries from an error to a valid page. `TestUntrustedQueryUnfilteredBudgetExhaustAtOffsetReturnsResumeKey` covers the boundary and its resume, and fails without the fix. The existing `TestUntrustedQueryRejectsBudgetExhaustBeforeOffset{Filtered,Unfiltered}` tests pass unchanged. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 415e691 commit eea0b48

5 files changed

Lines changed: 73 additions & 48 deletions

File tree

‎sei-cosmos/server/config/config.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,8 @@ func GetConfig(v *viper.Viper) (Config, error) {
418418
StreamImport: v.GetBool("genesis.stream-import"),
419419
GenesisStreamFile: v.GetString("genesis.genesis-stream-file"),
420420
},
421+
// The query.* keys are read at runtime by baseapp.readQueryConfig; nothing consumes
422+
// this field, so parsing them here would add a second reader that could drift.
421423
Query: DefaultQueryConfig(),
422424
}
423425

‎sei-cosmos/server/config/toml.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,13 @@ max-limit = {{ .Query.MaxLimit }}
119119
max-offset = {{ .Query.MaxOffset }}
120120
121121
# MaxIterations is the maximum store entries a single untrusted query may scan. 0 uses
122-
# the default. When the budget is exhausted the response is a partial page with next_key;
123-
# total stays 0 even when the client set count_total, so next_key != nil with total == 0
124-
# means the total is unknown rather than zero results.
122+
# the default. When the budget is exhausted after the offset is reached, the response is
123+
# a partial page with next_key; total stays 0 even when the client set count_total, so
124+
# next_key != nil with total == 0 means the total is unknown rather than zero results.
125+
# When the budget is exhausted before the offset is reached, the query fails with
126+
# InvalidArgument. Filtered queries (for example gov proposals with a status filter)
127+
# can hit this on defaults, because entries the filter rejects still count against
128+
# the budget but not toward the offset.
125129
max-iterations = {{ .Query.MaxIterations }}
126130
`
127131

‎sei-cosmos/types/query/filtered_pagination.go‎

Lines changed: 28 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -211,50 +211,6 @@ func GenericFilteredPaginate[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
211211
onResult func(key []byte, value T) (F, error),
212212
constructor func() T,
213213
) ([]F, *PageResponse, error) {
214-
return genericFilteredPaginate(ctx, cdc, prefixStore, pageRequest, onResult, constructor, false)
215-
}
216-
217-
// GenericFilteredPaginateForContext applies GenericFilteredPaginate on the ABCI query path
218-
// and GenericFilteredPaginateV66 during consensus execution.
219-
func GenericFilteredPaginateForContext[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
220-
ctx sdk.Context,
221-
cdc codec.BinaryCodec,
222-
prefixStore types.KVStore,
223-
pageRequest *PageRequest,
224-
onResult func(key []byte, value T) (F, error),
225-
constructor func() T,
226-
) ([]F, *PageResponse, error) {
227-
if ctx.IsABCIQuery() {
228-
return GenericFilteredPaginate(ctx, cdc, prefixStore, pageRequest, onResult, constructor)
229-
}
230-
return GenericFilteredPaginateV66(cdc, prefixStore, pageRequest, onResult, constructor)
231-
}
232-
233-
// GenericFilteredPaginateV66 preserves release/v6.6 behavior for EVM
234-
// precompiles. Node-local LCD/gRPC queries must use GenericFilteredPaginate.
235-
func GenericFilteredPaginateV66[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
236-
cdc codec.BinaryCodec,
237-
prefixStore types.KVStore,
238-
pageRequest *PageRequest,
239-
onResult func(key []byte, value T) (F, error),
240-
constructor func() T,
241-
) ([]F, *PageResponse, error) {
242-
return genericFilteredPaginate(sdk.Context{}, cdc, prefixStore, pageRequest, onResult, constructor, true)
243-
}
244-
245-
func genericFilteredPaginate[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
246-
ctx sdk.Context,
247-
cdc codec.BinaryCodec,
248-
prefixStore types.KVStore,
249-
pageRequest *PageRequest,
250-
onResult func(key []byte, value T) (F, error),
251-
constructor func() T,
252-
useV66 bool,
253-
) ([]F, *PageResponse, error) {
254-
if useV66 {
255-
return genericFilteredPaginateV66(cdc, prefixStore, pageRequest, onResult, constructor)
256-
}
257-
258214
var results []F
259215
pageRes, err := FilteredPaginate(ctx, prefixStore, pageRequest, func(key []byte, value []byte, accumulate bool) (bool, error) {
260216
protoMsg := constructor()
@@ -284,6 +240,34 @@ func genericFilteredPaginate[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
284240
return results, pageRes, nil
285241
}
286242

243+
// GenericFilteredPaginateForContext applies GenericFilteredPaginate on the ABCI query path
244+
// and GenericFilteredPaginateV66 during consensus execution.
245+
func GenericFilteredPaginateForContext[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
246+
ctx sdk.Context,
247+
cdc codec.BinaryCodec,
248+
prefixStore types.KVStore,
249+
pageRequest *PageRequest,
250+
onResult func(key []byte, value T) (F, error),
251+
constructor func() T,
252+
) ([]F, *PageResponse, error) {
253+
if ctx.IsABCIQuery() {
254+
return GenericFilteredPaginate(ctx, cdc, prefixStore, pageRequest, onResult, constructor)
255+
}
256+
return GenericFilteredPaginateV66(cdc, prefixStore, pageRequest, onResult, constructor)
257+
}
258+
259+
// GenericFilteredPaginateV66 preserves release/v6.6 behavior for EVM
260+
// precompiles. Node-local LCD/gRPC queries must use GenericFilteredPaginate.
261+
func GenericFilteredPaginateV66[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
262+
cdc codec.BinaryCodec,
263+
prefixStore types.KVStore,
264+
pageRequest *PageRequest,
265+
onResult func(key []byte, value T) (F, error),
266+
constructor func() T,
267+
) ([]F, *PageResponse, error) {
268+
return genericFilteredPaginateV66(cdc, prefixStore, pageRequest, onResult, constructor)
269+
}
270+
287271
func genericFilteredPaginateV66[T codec.ProtoMarshaler, F codec.ProtoMarshaler](
288272
cdc codec.BinaryCodec,
289273
prefixStore types.KVStore,

‎sei-cosmos/types/query/paginate_driver.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,17 @@ func (c *offsetScanCursor) phase() scanPhase {
159159
}
160160
}
161161

162+
// offsetConsumed reports whether the requested offset has been fully skipped.
163+
func (c *offsetScanCursor) offsetConsumed() bool {
164+
if c.filtered {
165+
return c.hits >= c.req.offset
166+
}
167+
return c.scanned >= c.req.offset
168+
}
169+
162170
func (c *offsetScanCursor) beginIteration(key []byte) (stop bool, err error) {
163171
if resumeKey, exhausted := c.budget.begin(key); exhausted {
164-
if c.phase() == scanPhaseSkip {
172+
if !c.offsetConsumed() {
165173
return true, c.budget.offsetNotReachedError()
166174
}
167175
if c.nextKey == nil {

‎sei-cosmos/types/query/scan_limit_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,33 @@ func TestUntrustedQueryRejectsBudgetExhaustBeforeOffsetUnfiltered(t *testing.T)
8282
require.Contains(t, err.Error(), "scanned more than 50 entries before reaching offset")
8383
}
8484

85+
func TestUntrustedQueryUnfilteredBudgetExhaustAtOffsetReturnsResumeKey(t *testing.T) {
86+
const maxIterations = 50
87+
ctx := sdk.Context{}.WithIsABCIQuery(true).WithPaginationLimits(sdk.UntrustedPaginationLimits(1000, 10_000, maxIterations))
88+
store := newTestKVStore(t)
89+
90+
for i := 0; i < 200; i++ {
91+
store.Set([]byte(fmt.Sprintf("%03d", i)), []byte("v"))
92+
}
93+
94+
var keys [][]byte
95+
collect := func(key, _ []byte) error {
96+
keys = append(keys, key)
97+
return nil
98+
}
99+
100+
res, err := query.Paginate(ctx, store, &query.PageRequest{Offset: maxIterations, Limit: 10}, collect)
101+
require.NoError(t, err)
102+
require.Empty(t, keys)
103+
require.Equal(t, "050", string(res.NextKey))
104+
105+
_, err = query.Paginate(ctx, store, &query.PageRequest{Key: res.NextKey, Limit: 10}, collect)
106+
require.NoError(t, err)
107+
require.Len(t, keys, 10)
108+
require.Equal(t, []byte("050"), keys[0])
109+
require.Equal(t, []byte("059"), keys[9])
110+
}
111+
85112
func TestUntrustedQueryPartialPageWhenBudgetExhaustDuringCollect(t *testing.T) {
86113
ctx := sdk.Context{}.WithIsABCIQuery(true).WithPaginationLimits(sdk.UntrustedPaginationLimits(1000, 10_000, 25))
87114
store := newTestKVStore(t)

0 commit comments

Comments
 (0)