Skip to content

Commit a177bc5

Browse files
committed
Fixes
Signed-off-by: AkramBitar <akram@il.ibm.com>
1 parent 2b9b91d commit a177bc5

8 files changed

Lines changed: 52 additions & 28 deletions

File tree

docs/configuration.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -604,9 +604,9 @@ token:
604604

605605
### Optional: token.storage.maxPageSize
606606

607-
Maximum number of rows a single transaction-store read may return. It bounds the
607+
Maximum number of rows a single transaction-store read may return. It limits the
608608
pages accepted by transaction queries and caps token-request queries so an
609-
unbounded scan cannot monopolise database resources. When the key is absent, the
609+
unlimited scan cannot exhaust database resources. When the key is absent, the
610610
store default (1000) applies.
611611

612612
```yaml

docs/services/storage.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ off). `AddTokenRequest` measures the raw token request plus its metadata;
329329
transaction and movement records add up their field lengths instead, to keep the
330330
write path fast.
331331

332-
**Read size limits.** `QueryTransactions` needs a bounded page argument (`nil` and
332+
**Read size limits.** `QueryTransactions` needs a page with a size limit (`nil` and
333333
`pagination.None()` are rejected; callers page through the full result set).
334334
`QueryTokenRequests` adds a hard `LIMIT` equal to `maxPageSize` (default 1000).
335335
`QueryMovements` is left uncapped on purpose: it feeds balance totals, and dropping

integration/token/fungible/views/history.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,18 @@ import (
2323
"github.com/hyperledger-labs/fabric-smart-client/platform/view/view"
2424
)
2525

26-
// historyPageSize bounds each page when listing all transactions; the storage
27-
// layer rejects unbounded queries, so these views page through the full set.
26+
// historyPageSize is the number of rows fetched per page when listing all
27+
// transactions. It must stay <= the store's max page size.
2828
const historyPageSize = 100
2929

3030
// collectAllTransactions pages through queryPage (one page per call) and returns
3131
// every record. queryPage runs the underlying query for the given pagination.
32+
//
33+
// The storage layer now rejects unlimited queries, so these trusted views can no
34+
// longer fetch everything in one call. This helper is only that adaptation: it
35+
// still accumulates the full result set in memory, so it is not itself a memory
36+
// safeguard. The actual DoS protection (rejecting unlimited scans, hard LIMITs)
37+
// lives in the storage layer; these views legitimately need the complete list.
3238
func collectAllTransactions(queryPage func(driver2.Pagination) (iterators.Iterator[*ttxdb.TransactionRecord], error)) ([]*ttxdb.TransactionRecord, error) {
3339
var page driver2.Pagination
3440
page, err := pagination.Offset(0, historyPageSize)

token/services/storage/db/common/checks.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func (a *DefaultCheckers) CheckTransactions(ctx context.Context) ([]string, erro
120120
return nil, errors.WithMessagef(err, "failed to get ledger [%s]", tms.ID())
121121
}
122122

123-
// Iterate over transactions one bounded page at a time. A single unbounded
123+
// Iterate over transactions one limited page at a time. A single unlimited
124124
// query is rejected by the storage layer, so we page through the whole set
125125
// using offset pagination and stop once a page comes back short.
126126
var page driver2.Pagination

token/services/storage/db/sql/common/transactions.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func (db *TransactionStore) QueryMovements(ctx context.Context, params dbdriver.
235235
}
236236

237237
func (db *TransactionStore) QueryTransactions(ctx context.Context, params dbdriver.QueryTransactionsParams, pagination driver3.Pagination) (*driver3.PageIterator[*dbdriver.TransactionRecord], error) {
238-
if err := paginationutil.ValidateBounded(pagination, db.maxPageSize); err != nil {
238+
if err := paginationutil.ValidateLimited(pagination, db.maxPageSize); err != nil {
239239
return nil, err
240240
}
241241
transactionsTable, requestsTable := q.Table(db.table.Transactions), q.Table(db.table.Requests)
@@ -314,7 +314,7 @@ func (db *TransactionStore) Notifier() (dbdriver.TransactionNotifier, error) {
314314
}
315315

316316
// QueryTokenRequests returns an iterator over the token requests matching params,
317-
// capped at the store's max page size to bound the scan.
317+
// capped at the store's max page size to limit the scan.
318318
func (db *TransactionStore) QueryTokenRequests(ctx context.Context, params dbdriver.QueryTokenRequestsParams) (dbdriver.TokenRequestIterator, error) {
319319
query, args := q.Select().
320320
FieldsByName("tx_id", "request", "status").

token/services/storage/db/sql/common/transactions_bounds_test.go renamed to token/services/storage/db/sql/common/transactions_limits_test.go

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"github.com/onsi/gomega"
2020
)
2121

22-
func boundsTestStore(db *sql.DB, opts ...TransactionStoreOption) *TransactionStore {
22+
func limitsTestStore(db *sql.DB, opts ...TransactionStoreOption) *TransactionStore {
2323
store, _ := NewOwnerTransactionStore(db, db, TableNames{
2424
Movements: "MOVEMENTS",
2525
Transactions: "TRANSACTIONS",
@@ -37,7 +37,7 @@ func TestWritePayloadSizeLimit(t *testing.T) {
3737
db, mockDB, err := sqlmock.New()
3838
gomega.Expect(err).ToNot(gomega.HaveOccurred())
3939

40-
store := boundsTestStore(db, WithMaxPayloadSize(20))
40+
store := limitsTestStore(db, WithMaxPayloadSize(20))
4141
mockDB.ExpectBegin()
4242
w, err := store.NewTransactionStoreTransaction()
4343
gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -73,7 +73,7 @@ func TestWritePayloadSizeDisabled(t *testing.T) {
7373
db, mockDB, err := sqlmock.New()
7474
gomega.Expect(err).ToNot(gomega.HaveOccurred())
7575

76-
store := boundsTestStore(db, WithMaxPayloadSize(0))
76+
store := limitsTestStore(db, WithMaxPayloadSize(0))
7777
mockDB.ExpectBegin()
7878
mockDB.ExpectExec("INSERT INTO REQUESTS").WillReturnResult(sqlmock.NewResult(1, 1))
7979
w, err := store.NewTransactionStoreTransaction()
@@ -84,18 +84,36 @@ func TestWritePayloadSizeDisabled(t *testing.T) {
8484
gomega.Expect(mockDB.ExpectationsWereMet()).To(gomega.Succeed())
8585
}
8686

87-
// TestQueryTransactionsRejectsUnbounded verifies the read guard rejects nil and
88-
// unbounded (None) pagination without querying the database.
89-
func TestQueryTransactionsRejectsUnbounded(t *testing.T) {
87+
// TestQueryTransactionsRejectsUnlimited verifies the read guard rejects nil and
88+
// unlimited (None) pagination without querying the database.
89+
func TestQueryTransactionsRejectsUnlimited(t *testing.T) {
9090
gomega.RegisterTestingT(t)
9191
db, _, err := sqlmock.New()
9292
gomega.Expect(err).ToNot(gomega.HaveOccurred())
9393

94-
store := boundsTestStore(db)
94+
store := limitsTestStore(db)
9595

9696
_, err = store.QueryTransactions(t.Context(), driver.QueryTransactionsParams{}, nil)
9797
gomega.Expect(err).To(gomega.HaveOccurred())
9898

9999
_, err = store.QueryTransactions(t.Context(), driver.QueryTransactionsParams{}, pagination.None())
100100
gomega.Expect(err).To(gomega.HaveOccurred())
101101
}
102+
103+
// TestQueryTokenRequestsAppliesLimit verifies the token-request scan is capped at
104+
// the store's max page size via a LIMIT clause.
105+
func TestQueryTokenRequestsAppliesLimit(t *testing.T) {
106+
gomega.RegisterTestingT(t)
107+
db, mockDB, err := sqlmock.New()
108+
gomega.Expect(err).ToNot(gomega.HaveOccurred())
109+
110+
store := limitsTestStore(db, WithMaxPageSize(500))
111+
mockDB.ExpectQuery("SELECT .* FROM REQUESTS .*LIMIT").
112+
WithArgs(500).
113+
WillReturnRows(sqlmock.NewRows([]string{"tx_id", "request", "status"}))
114+
115+
it, err := store.QueryTokenRequests(t.Context(), driver.QueryTokenRequestsParams{})
116+
gomega.Expect(err).ToNot(gomega.HaveOccurred())
117+
it.Close()
118+
gomega.Expect(mockDB.ExpectationsWereMet()).To(gomega.Succeed())
119+
}

token/services/storage/db/sql/query/pagination/validate.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ import (
1111
"github.com/hyperledger-labs/fabric-smart-client/platform/common/driver"
1212
)
1313

14-
// ValidateBounded rejects pagination that would run an unbounded scan: nil,
14+
// ValidateLimited rejects pagination that would run an unlimited scan: nil,
1515
// None(), or an offset whose page size is non-positive or exceeds maxPageSize
16-
// (capped only when maxPageSize > 0). Empty and keyset pagination are allowed.
17-
func ValidateBounded(p driver.Pagination, maxPageSize int) error {
16+
// (limited only when maxPageSize > 0). Empty and keyset pagination are allowed.
17+
func ValidateLimited(p driver.Pagination, maxPageSize int) error {
1818
switch v := p.(type) {
1919
case nil:
20-
return errors.New("pagination is required: a bounded page must be provided")
20+
return errors.New("pagination is required: a page-size limit must be provided")
2121
case *none:
22-
return errors.New("unbounded pagination (None) is not allowed")
22+
return errors.New("unlimited pagination (None) is not allowed")
2323
case *empty:
2424
return nil
2525
case *offset:

token/services/storage/db/sql/query/pagination/validate_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ package pagination
88

99
import "testing"
1010

11-
func TestValidateBounded(t *testing.T) {
11+
func TestValidateLimited(t *testing.T) {
1212
off, err := Offset(0, 10)
1313
if err != nil {
1414
t.Fatalf("unexpected error building offset: %v", err)
@@ -27,13 +27,13 @@ func TestValidateBounded(t *testing.T) {
2727
build func() error
2828
wantErr bool
2929
}{
30-
{"nil is rejected", func() error { return ValidateBounded(nil, 50) }, true},
31-
{"None is rejected", func() error { return ValidateBounded(None(), 50) }, true},
32-
{"Empty is allowed", func() error { return ValidateBounded(Empty(), 50) }, false},
33-
{"bounded offset is allowed", func() error { return ValidateBounded(off, 50) }, false},
34-
{"offset over max is rejected", func() error { return ValidateBounded(bigOff, 50) }, true},
35-
{"zero page size is rejected", func() error { return ValidateBounded(zeroOff, 50) }, true},
36-
{"no max disables the cap", func() error { return ValidateBounded(bigOff, 0) }, false},
30+
{"nil is rejected", func() error { return ValidateLimited(nil, 50) }, true},
31+
{"None is rejected", func() error { return ValidateLimited(None(), 50) }, true},
32+
{"Empty is allowed", func() error { return ValidateLimited(Empty(), 50) }, false},
33+
{"offset within the limit is allowed", func() error { return ValidateLimited(off, 50) }, false},
34+
{"offset over max is rejected", func() error { return ValidateLimited(bigOff, 50) }, true},
35+
{"zero page size is rejected", func() error { return ValidateLimited(zeroOff, 50) }, true},
36+
{"no max disables the cap", func() error { return ValidateLimited(bigOff, 0) }, false},
3737
}
3838
for _, c := range cases {
3939
t.Run(c.name, func(t *testing.T) {

0 commit comments

Comments
 (0)