fix(storage): don't row-cap reads that cannot be paged

The guard layer applied LimitIterator to QueryValidations,
QueryTokenRequests and IteratorConfigurations. None of those three
accepts a pagination argument, so the error they raised once the cap was
exceeded ("must paginate") asked for something the API cannot do.

IteratorConfigurations made this a functional regression rather than
just misleading advice: LocalMembership.storedIdentityConfigurations
drains it in full via collections.ReadAll on the identity Load path, so
a node holding more than maxPageSize (default 1000) stored identity
configurations of one type failed to load identities, recoverable only
by raising or disabling the limit globally.

Remove the cap from all three and record at each site why the read is
not bounded here, matching the reasoning already applied to the
token-store iterators. Row-capping these reads needs a SQL-level LIMIT
or paging in their signatures, which the docs now track as follow-up.
LimitIterator has no remaining callers, so it and its tests are removed
rather than left as dead code.

maxPageSize continues to bound QueryTransactions on the owner and audit
transaction stores, which does take a pagination argument, so rejecting
an unbounded page there is actionable. Write payload limits are
unchanged.

Also correct the docs, which claimed the token store's
unspent/spendable/unsupported iterators were row-capped while
guard/token.go explicitly did not cap them and overrode no read method.

Signed-off-by: AkramBitar <akram@il.ibm.com>

Author: AkramBitar

docs/configuration.md

@@ -720,18 +720,22 @@ token:
 
 ### Optional: token.storage.maxPageSize
 
-Maximum number of rows a single storage-service read may return. It bounds the pages
-accepted by paginated queries and caps streaming iterator reads so an unlimited scan
-cannot exhaust database resources. When the key is absent, the default (1000) applies.
+Maximum page size a paginated storage-service read may request, so an unlimited scan
+cannot exhaust database resources. A query that asks for an unbounded page (`nil` or
+`pagination.None()`) or a page larger than this value is rejected; callers page
+through the full result set instead. When the key is absent, the default (1000)
+applies.
 
 ```yaml
 token:
   storage:
     maxPageSize: 1000
 ```
 
-See [Storage API Limits](services/storage.md#storage-api-limits) for details,
-including why movement/balance queries are intentionally not row-capped.
+This bounds the paginated reads only. Streaming iterator reads are deliberately not
+row-capped, because they accept no page size for the caller to comply with — see
+[Storage API Limits](services/storage.md#storage-api-limits) for the full list and
+the reasoning, including why movement/balance queries are never row-capped.
 
 ---
 

docs/services/storage.md

@@ -367,25 +367,38 @@ writes include the transaction store's `AddTokenRequest` / `AddTransaction` /
 `RegisterIdentityDescriptor` / `AddConfiguration`, and the wallet store's
 `StoreIdentity`.
 
-**Read size limits.**
-- Paginated reads (`QueryTransactions`) require a bounded page: `nil` and
-  `pagination.None()` are rejected, and a page size larger than `maxPageSize`
-  (default 1000) is rejected. Callers page through the full result set.
-- Streaming iterator reads are capped at `maxPageSize` rows via a limiting iterator
-  that errors (rather than silently truncating) once the cap is exceeded. Covered
-  reads include the token store's unspent/spendable/unsupported iterators, the
-  endorser store's `QueryValidations`, the transaction store's `QueryTokenRequests`,
-  and the identity store's `IteratorConfigurations`.
-- `QueryMovements` is left uncapped on purpose: it feeds balance totals, and dropping
-  rows there would quietly return wrong balances. It is already narrowed by its
+**Read size limit.** `maxPageSize` (default 1000) bounds the paginated reads —
+currently `QueryTransactions` on the owner and audit transaction stores. Those
+reads require a bounded page: `nil` and `pagination.None()` are rejected, as is a
+page size larger than `maxPageSize`. Callers page through the full result set.
+
+Rejecting the request is only a safe way to bound a read when the caller has a way
+to comply, so the cap is applied exactly to the reads that accept a pagination
+argument. Streaming iterator reads are **not** row-capped:
+
+- The token store's unspent/spendable/unsupported iterators, the endorser store's
+  `QueryValidations`, the transaction store's `QueryTokenRequests`, and the identity
+  store's `IteratorConfigurations` all take no page size or cursor, and their
+  consumers drain them in full — the selectors and integrity checks for the token
+  iterators, and `LocalMembership.storedIdentityConfigurations` on the identity
+  `Load` path for `IteratorConfigurations`. Capping any of them would turn a large
+  but legitimate dataset into a hard failure that the caller cannot page around.
+- `QueryMovements` is uncapped for a second reason: it feeds balance totals, so
+  dropping rows would quietly return wrong balances. It is already narrowed by its
   required filters.
 
 You can override the limits in configuration (`token.storage.maxPayloadSize` /
 `token.storage.maxPageSize`) — see the [Configuration Guide](../configuration.md).
 
-**Not yet covered (tracked follow-up).** Reads that materialise a full slice/map
-before returning (e.g. `ListUnspentTokens`, `QueryTokenDetails`,
-`ConfigurationsByID`, `GetWalletIDs`) cannot be bounded by a wrapper alone — the SQL
-has already loaded everything — so they need a SQL-level `LIMIT` or conversion to
-iterators. The opaque `Keystore.Put` value and input-size caps for variadic id lists
+**Not yet covered (tracked follow-up).** Two groups of reads cannot be bounded by a
+wrapper at all, and need query-level work instead:
+
+- The streaming iterators listed above. Bounding them requires a SQL-level `LIMIT`
+  on the query (with a defined order and documented truncation) or adding paging to
+  their signatures, so the caller can ask for the next page rather than fail.
+- Reads that materialise a full slice or map before returning (e.g.
+  `ListUnspentTokens`, `QueryTokenDetails`, `ConfigurationsByID`, `GetWalletIDs`) —
+  the SQL has already loaded everything by the time the decorator sees the result.
+
+The opaque `Keystore.Put` value and input-size caps for variadic id lists
 (`DeleteTokens`, `GetTokens`) are in the same follow-up.

token/services/storage/db/guard/endorser.go

@@ -27,14 +27,10 @@ type guardedEndorserStore struct {
 	policy Policy
 }
 
-func (g *guardedEndorserStore) QueryValidations(ctx context.Context, params driver.QueryValidationRecordsParams) (driver.ValidationRecordsIterator, error) {
-	it, err := g.EndorserStore.QueryValidations(ctx, params)
-	if err != nil {
-		return nil, err
-	}
-
-	return LimitIterator(it, g.policy.MaxPageSize, "QueryValidations"), nil
-}
+// QueryValidations is intentionally not row-capped and delegates to the embedded
+// store unchanged: QueryValidationRecordsParams carries no page-size or cursor,
+// so a caller that hits a cap has no way to page around it. Bounding this read
+// needs a SQL-level LIMIT on the query itself (tracked follow-up), not a wrapper.
 
 func (g *guardedEndorserStore) NewEndorserStoreTransaction() (driver.EndorserStoreTransaction, error) {
 	w, err := g.EndorserStore.NewEndorserStoreTransaction()

token/services/storage/db/guard/identity.go

@@ -28,14 +28,12 @@ type guardedIdentityStore struct {
 	policy Policy
 }
 
-func (g *guardedIdentityStore) IteratorConfigurations(ctx context.Context, configurationType string) (idriver.IdentityConfigurationIterator, error) {
-	it, err := g.IdentityStore.IteratorConfigurations(ctx, configurationType)
-	if err != nil {
-		return nil, err
-	}
-
-	return LimitIterator(it, g.policy.MaxPageSize, "IteratorConfigurations"), nil
-}
+// IteratorConfigurations is intentionally not row-capped and delegates to the
+// embedded store unchanged. It takes no pagination argument, and
+// LocalMembership.storedIdentityConfigurations drains it in full on the identity
+// Load path, so a cap here would turn "this node has many registered identities"
+// into a hard identity-loading failure with no way for the caller to page around
+// it. Bounding this read needs a SQL-level LIMIT (tracked follow-up).
 
 func (g *guardedIdentityStore) AddConfiguration(ctx context.Context, wp idriver.IdentityConfiguration) error {
 	size := len(wp.ID) + len(wp.Type) + len(wp.URL) + len(wp.Config) + len(wp.Raw)

token/services/storage/db/guard/iterator.go

@@ -48,14 +48,12 @@ func (g *guardedOwnerTx) QueryTransactions(ctx context.Context, params driver.Qu
 	return g.TokenTransactionStore.QueryTransactions(ctx, params, p)
 }
 
-func (g *guardedOwnerTx) QueryTokenRequests(ctx context.Context, params driver.QueryTokenRequestsParams) (driver.TokenRequestIterator, error) {
-	it, err := g.TokenTransactionStore.QueryTokenRequests(ctx, params)
-	if err != nil {
-		return nil, err
-	}
-
-	return LimitIterator(it, g.policy.MaxPageSize, "QueryTokenRequests"), nil
-}
+// QueryTokenRequests is intentionally not row-capped on either transaction store
+// and delegates to the embedded store unchanged: QueryTokenRequestsParams carries
+// only a status filter, with no page size or cursor, so a caller that hits a cap
+// cannot page around it. Bounding this read needs a SQL-level LIMIT on the query
+// itself (tracked follow-up). QueryTransactions above is capped instead, because
+// it does take a pagination argument, so rejecting an unbounded page is actionable.
 
 func (g *guardedOwnerTx) AddTransactionEndorsementAck(ctx context.Context, txID string, endorser token.Identity, sigma []byte) error {
 	if err := CheckPayload("AddTransactionEndorsementAck", len(txID)+len(endorser)+len(sigma), g.policy.MaxPayloadSize); err != nil {
@@ -87,15 +85,6 @@ func (g *guardedAuditTx) QueryTransactions(ctx context.Context, params driver.Qu
 	return g.AuditTransactionStore.QueryTransactions(ctx, params, p)
 }
 
-func (g *guardedAuditTx) QueryTokenRequests(ctx context.Context, params driver.QueryTokenRequestsParams) (driver.TokenRequestIterator, error) {
-	it, err := g.AuditTransactionStore.QueryTokenRequests(ctx, params)
-	if err != nil {
-		return nil, err
-	}
-
-	return LimitIterator(it, g.policy.MaxPageSize, "QueryTokenRequests"), nil
-}
-
 func (g *guardedAuditTx) NewTransactionStoreTransaction() (driver.TransactionStoreTransaction, error) {
 	w, err := g.AuditTransactionStore.NewTransactionStoreTransaction()
 	if err != nil {