wallet: implement store pagination

[GustavoStingelin]

This PR adds pagination support to list queries that may return very large result sets, such as addresses.

• List queries are now paginated by default. They accept an optional cursor parameter, which preserves compatibility with the existing contract.

• New interface methods named IterSomething were added. These provide seamless iteration over all results without requiring the caller to manage pagination, using the iter.Seq2 API.

[Copilot]

Pull request overview

This PR introduces cursor-based pagination for database “list” queries (wallets, accounts, addresses) and adds iterator-style APIs (Iter*) to transparently traverse all pages via iter.Seq2, preventing large result sets from being loaded at once.

Changes:

• Add a page utility package (Options, Iter) and shared paging helpers to implement cursor-based pagination consistently.
• Update wallet/account/address store interfaces and implementations to return paged results by default and provide Iter* helpers for full traversal.
• Regenerate sqlc queries (first-page/next-page) for SQLite and Postgres, and expand integration tests to cover paging + iterators.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
wallet/internal/db/wallets_common.go	Shared wallet pagination helpers (cursor advancement, row conversion).
wallet/internal/db/wallet_sqlite.go	SQLite wallet listing now paginated; adds IterWallets.
wallet/internal/db/wallet_pg.go	Postgres wallet listing now paginated; adds IterWallets.
wallet/internal/db/sqlc/sqlite/wallets.sql.go	sqlc-generated SQLite wallet queries split into first/next page.
wallet/internal/db/sqlc/sqlite/querier.go	Updates SQLite sqlc Querier interface for paged list methods.
wallet/internal/db/sqlc/sqlite/db.go	Prepares/closes new SQLite prepared statements for paged queries.
wallet/internal/db/sqlc/sqlite/addresses.sql.go	sqlc-generated SQLite address list split into first/next page.
wallet/internal/db/sqlc/sqlite/accounts.sql.go	sqlc-generated SQLite account lists split into first/next page variants.
wallet/internal/db/sqlc/postgres/wallets.sql.go	sqlc-generated Postgres wallet queries split into first/next page.
wallet/internal/db/sqlc/postgres/querier.go	Updates Postgres sqlc Querier interface for paged list methods.
wallet/internal/db/sqlc/postgres/db.go	Prepares/closes new Postgres prepared statements for paged queries.
wallet/internal/db/sqlc/postgres/addresses.sql.go	sqlc-generated Postgres address list split into first/next page.
wallet/internal/db/sqlc/postgres/accounts.sql.go	sqlc-generated Postgres account lists split into first/next page variants.
wallet/internal/db/queries/sqlite/wallets.sql	Source SQL for SQLite wallet paging queries.
wallet/internal/db/queries/sqlite/addresses.sql	Source SQL for SQLite address paging queries.
wallet/internal/db/queries/sqlite/accounts.sql	Source SQL for SQLite account paging queries.
wallet/internal/db/queries/postgres/wallets.sql	Source SQL for Postgres wallet paging queries.
wallet/internal/db/queries/postgres/addresses.sql	Source SQL for Postgres address paging queries.
wallet/internal/db/queries/postgres/accounts.sql	Source SQL for Postgres account paging queries.
wallet/internal/db/pagination_common.go	Shared generic helper to fetch either first or next page by cursor.
wallet/internal/db/page/options_test.go	Unit tests for pagination options normalization/immutability.
wallet/internal/db/page/options.go	Pagination Options type (size/cursor, normalization).
wallet/internal/db/page/iter_test.go	Unit tests for iterator paging traversal, errors, cancellation, break.
wallet/internal/db/page/iter.go	Core paging iterator (Iter) producing iter.Seq2.
wallet/internal/db/page/doc.go	Package documentation for page.
wallet/internal/db/itest/wallet_store_test.go	Adds wallet paging + iterator integration tests.
wallet/internal/db/itest/fixtures_common_test.go	Adds shared-name fixtures and helpers used by pagination tests.
wallet/internal/db/itest/address_store_test.go	Adds address paging + iterator integration tests.
wallet/internal/db/itest/account_store_test.go	Adds account paging + iterator integration tests and cursor helpers.
wallet/internal/db/interface.go	Store interfaces updated: paged List* and new Iter* methods.
wallet/internal/db/data_types.go	Adds list query types + pagination cursor types (wallets/accounts/addresses).
wallet/internal/db/addresses_sqlite.go	SQLite address listing now paginated; adds IterAddresses.
wallet/internal/db/addresses_pg.go	Postgres address listing now paginated; adds IterAddresses.
wallet/internal/db/addresses_common.go	Shared address paging helpers (cursor advancement + paging adapter).
wallet/internal/db/accounts_sqlite.go	SQLite account listing now paginated; adds IterAccounts.
wallet/internal/db/accounts_pg.go	Postgres account listing now paginated; adds IterAccounts.
wallet/internal/db/accounts_common.go	Shared account paging helpers (cursor advancement + stable tie-breaker).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[GustavoStingelin]

I am thinking that, since we will always rely on the automatic pagination provided by the iterator, it may be better to drop the list method entirely, or change its signature to return iter.Seq2.

This would expose a single way to query these results and help prevent misuse.

[yyforyongyu]

Hmm I think the new API exposes too many details to the caller, so ask a few LLMs on how to improve it plus a few of my own thoughts, I think the design can be improved.

[yyforyongyu]

None of these tests have docs?

[yyforyongyu]

Add some inline docs to explain the noqa?

[yyforyongyu]

it seems to me that a lot of the new methods are not documented, maybe we could point out the docs @docs/developer/ENGINEERING_GUIDE.md and @docs/developer/contribution_guidelines.md so agents will follow.

[yyforyongyu]

This check is redundant - I have also seen it multiple times that LLMs turn to have this pattern, weird...but fetchPage below will already handle it if the context is done. Plus once we are in the for _, item := range items { loop, the cancellation won't stop the yield emission.

[yyforyongyu]

I think the current naming is a bit hard to follow, so suggest sth like,

func Iter[Query, Item any](
	ctx context.Context,
	initial Query,
	fetchPage func(context.Context, Query) ([]Item, error),
	nextQuery func(Query, []Item) Query,
) iter.Seq2[Item, error]

[yyforyongyu]

I guess if we wanna catch the cancel signal, we need to perform it here.

[yyforyongyu]

Does it mean this is mutable?

[yyforyongyu]

to keep it immutable, i think sth like,

c := cursor
o.cursor = &c

This is not a deep copy, but it should prevent direct mutation through opts.Cursor.

[yyforyongyu]

Good that we have these immutability tests, but it seems like we only test against uint32 while we support Options[C any]. I think we need to add more tests.

[yyforyongyu]

That said, I think it's better to keep the cursor unexported - maybe add a new method Cursor and return the copy for reading?

[yyforyongyu]

If we adopt the new design, the call will instead be,

res, err := store.ListAccounts(ctx, q)
if err != nil { ... }
for _, acc := range res.Items {
	...
}
if res.Next != nil {
	q.Page.After = res.Next
}

[yyforyongyu]

Yes — here’s the redesign I’d recommend.

The short version:

• List* should return a page object, not a bare slice
• the store should return the next cursor explicitly
• callers should never derive the next cursor from the last item
• Iter* should be a thin convenience wrapper over List*
• use LIMIT n+1 internally so you know whether another page exists without an extra empty fetch
• simplify SQL where it is safe, but do not let SQL unification block the API redesign

Goals

The redesigned API should make these things true:

• the caller asks for a page and gets both Items and Next
• the store owns sort order and cursor derivation
• page requests are simple data, not builder objects
• iteration over all results is easy
• implementation stays boring and local, not spread across several generic helper layers

What’s wrong with the current shape

The current design has a few structural problems:

• List* returns []Item, so the caller must know how to derive the next cursor
• that leaks ordering logic into item types like AccountInfo.Cursor()
• page.Options adds ceremony without really making the API safer
• page.Iter is generic, but the stores still have to do a lot of setup around it
• splitting SQL into FirstPage and NextPage doubles row/param types and causes a lot of boilerplate
• the iterator needs an extra empty fetch to know it is done

The main design mistake is ownership: the store knows pagination semantics, but the caller is currently forced to participate in them.

Proposed API

I would replace page.Options with a plain request type, and replace bare slice returns with a page result.

package page

const (
  DefaultLimit = 100
  MaxLimit     = 1000
)

type Request[C any] struct {
  Limit uint32
  After *C
}

func (r Request[C]) EffectiveLimit() uint32 {
  switch {
  case r.Limit == 0:
    return DefaultLimit
  case r.Limit > MaxLimit:
    return MaxLimit
  default:
    return r.Limit
  }
}

type Result[T any, C any] struct {
  Items []T
  Next  *C
}

This is a much better fit:

• Request is just input data
• Result is just output data
• Next is owned by the store, not reconstructed by the caller

Query types

Then the store query types become simple and obvious.

type ListWalletsQuery struct {
  Page page.Request[uint32]
}

type AccountCursor struct {
  Imported      bool
  AccountNumber uint32
  RowID         uint32
}

type ListAccountsQuery struct {
  WalletID uint32
  Scope    *KeyScope
  Name     *string
  Page     page.Request[AccountCursor]
}

type ListAddressesQuery struct {
  WalletID    uint32
  Scope       KeyScope
  AccountName string
  Page        page.Request[uint32]
}

This reads better than:

page.Options[uint32]{}.WithSize(50).WithCursor(99)

and it is easier to debug because you can just print the struct.

Store interfaces

The store interfaces should return page.Result[...].

type WalletStore interface {
  ListWallets(ctx context.Context, query ListWalletsQuery) (
    page.Result[WalletInfo, uint32], error,
  )

  IterWallets(ctx context.Context, query ListWalletsQuery) iter.Seq2[WalletInfo, error]
}

type AccountStore interface {
  ListAccounts(ctx context.Context, query ListAccountsQuery) (
    page.Result[AccountInfo, AccountCursor], error,
  )

  IterAccounts(ctx context.Context, query ListAccountsQuery) iter.Seq2[AccountInfo, error]
}

type AddressStore interface {
  ListAddresses(ctx context.Context, query ListAddressesQuery) (
    page.Result[AddressInfo, uint32], error,
  )

  IterAddresses(ctx context.Context, query ListAddressesQuery) iter.Seq2[AddressInfo, error]
}

This is the key public API improvement.

Caller experience

Current style:

items, err := store.ListAccounts(ctx, q)
if err != nil {
  return err
}

for _, item := range items {
  use(item)
}

if len(items) > 0 {
  q.Page = q.Page.WithCursor(items[len(items)-1].Cursor())
}

Redesigned style:

res, err := store.ListAccounts(ctx, q)
if err != nil {
  return err
}

for _, item := range res.Items {
  use(item)
}

if res.Next != nil {
  q.Page.After = res.Next
}

That is much easier to use, and much harder to misuse.

The store computes Next. The caller just forwards it.

Why Result.Next is such a big improvement

This removes pagination knowledge from item types.

Right now something like AccountInfo.Cursor() leaks internal ordering assumptions:

• imported accounts sort after derived
• imported state is inferred from Origin
• RowID is part of cursor semantics

That logic should not live in the public AccountInfo API at all. It belongs inside ListAccounts.

With page.Result.Next, you can keep cursor derivation private:

func accountCursorFromInfo(info AccountInfo) AccountCursor {
  return AccountCursor{
    Imported:      info.Origin == ImportedAccount,
    AccountNumber: info.AccountNumber,
    RowID:         info.ID,
  }
}

That helper stays private to the store package.

Store implementation pattern

For each List* method, use this flow:

1. normalize page size
2. fetch limit + 1 rows
3. convert rows to infos
4. if more than limit, compute Next from the last retained item and trim
5. return page.Result

That removes the need for generic nextQuery plumbing and avoids the extra terminal fetch.

Example: ListWallets

This is the ideal simple case because the cursor is just wallet.id.

func (s *SqliteStore) ListWallets(
  ctx context.Context,
  query ListWalletsQuery,
) (page.Result[WalletInfo, uint32], error) {

  limit := query.Page.EffectiveLimit()
  fetchLimit := int64(limit) + 1

  rows, err := s.queries.ListWallets(ctx, sqlc.ListWalletsParams{
    AfterID: nullUint32(query.Page.After),
    Limit:   fetchLimit,
  })
  if err != nil {
    return page.Result[WalletInfo, uint32]{}, fmt.Errorf("list wallets: %w", err)
  }

  items, err := walletInfosFromRows(rows)
  if err != nil {
    return page.Result[WalletInfo, uint32]{}, fmt.Errorf("convert wallets: %w", err)
  }

  var next *uint32
  n := int(limit)
  if len(items) > n {
    cursor := items[n-1].ID
    next = &cursor
    items = items[:n]
  }

  return page.Result[WalletInfo, uint32]{
    Items: items,
    Next:  next,
  }, nil
}

The important detail is this line:

cursor := items[n-1].ID

When you fetch limit+1, the cursor should point to the last returned item, not the discarded extra row.

Example: IterWallets

With the new ListWallets shape, the iterator becomes trivial.

func (s *SqliteStore) IterWallets(
  ctx context.Context,
  query ListWalletsQuery,
) iter.Seq2[WalletInfo, error] {
  return func(yield func(WalletInfo, error) bool) {
    for {
      res, err := s.ListWallets(ctx, query)
      if err != nil {
        var zero WalletInfo
        _ = yield(zero, err)
        return
      }

      for _, item := range res.Items {
        select {
        case <-ctx.Done():
          var zero WalletInfo
          _ = yield(zero, ctx.Err())
          return
        default:
        }

        if !yield(item, nil) {
          return
        }
      }

      if res.Next == nil {
        return
      }

      next := *res.Next
      query.Page.After = &next
    }
  }
}

I would handwrite IterWallets, IterAccounts, and IterAddresses instead of keeping a generic page.Iter. With only three call sites, that is clearer and easier to maintain.

If you still want a generic helper, it should operate on page.Result, not on (fetchPage, nextQuery).

A better generic iterator, if you keep one

If you really want a helper, make it work with explicit page metadata.

func Iter[Q, T, C any](
  ctx context.Context,
  query Q,
  fetch func(context.Context, Q) (page.Result[T, C], error),
  withAfter func(Q, *C) Q,
) iter.Seq2[T, error] {
  return func(yield func(T, error) bool) {
    for {
      res, err := fetch(ctx, query)
      if err != nil {
        var zero T
        _ = yield(zero, err)
        return
      }

      for _, item := range res.Items {
        select {
        case <-ctx.Done():
          var zero T
          _ = yield(zero, ctx.Err())
          return
        default:
        }

        if !yield(item, nil) {
          return
        }
      }

      if res.Next == nil {
        return
      }

      query = withAfter(query, res.Next)
    }
  }
}

That is much better than the current nextQuery func(Q, []R) Q design.

But again, I would still prefer handwritten iterators here.

SQL design

There are two parts here:

• simple cursors: wallets, addresses
• composite cursors: accounts

The API redesign should happen regardless of whether all SQL is unified.

Wallets and addresses: use one query

These are the easy cases. Use one query with a nullable cursor and LIMIT n+1.

Pseudo-SQL:

SELECT ...
FROM wallets w
WHERE (:after_id IS NULL OR w.id > :after_id)
ORDER BY w.id
LIMIT :limit_plus_one;

and similarly for addresses:

SELECT ...
FROM addresses a
WHERE
   a.account_id = :account_id
   AND (:after_id IS NULL OR a.id > :after_id)
ORDER BY a.id
LIMIT :limit_plus_one;

This gives you:

• one SQL query
• one sqlc row type
• one sqlc params type
• much less Go boilerplate

Accounts: two acceptable options

Accounts are more complicated because the sort key is composite:

• derived accounts first, ordered by (account_number, id)
• imported accounts after, ordered by id

You have two reasonable choices.

Option A: one unified query

Use a single query with nullable cursor fields.

Pseudo-SQL:

SELECT ...
FROM accounts a
WHERE
   a.wallet_id = :wallet_id
   AND (:scope_purpose IS NULL OR a.purpose = :scope_purpose)
   AND (:scope_coin_type IS NULL OR a.coin_type = :scope_coin_type)
   AND (:name IS NULL OR a.account_name = :name)
   AND (
     :after_id IS NULL
     OR (
       (
         NOT :after_imported
         AND a.account_number IS NOT NULL
         AND (
           a.account_number > :after_account_number
           OR (
             a.account_number = :after_account_number
             AND a.id > :after_id
           )
         )
       )
       OR (
         a.account_number IS NULL
         AND (
           NOT :after_imported
           OR a.id > :after_id
         )
       )
     )
   )
ORDER BY a.account_number NULLS LAST, a.id
LIMIT :limit_plus_one;

That is still complicated SQL, but the complexity is in one place instead of duplicated across first-page and next-page versions.

This is the cleanest overall design if the query planner behaves well.

Option B: keep split SQL for accounts only

If EXPLAIN shows that the unified query produces worse plans, keep the first-page / next-page split for accounts only.

That is fine.

The important point is: the Go API does not depend on SQL unification.

You can still ship the better page.Request / page.Result design even if accounts continue using two separate SQL statements internally.

That is probably the safest rollout path.

Query planner caution

This is the one place where I would be pragmatic instead of ideological.

For wallets and addresses, one nullable-cursor query is obviously worth it.

For accounts, I would require plan verification:

• PostgreSQL: EXPLAIN (ANALYZE, BUFFERS) for first-page and next-page cases
• SQLite: EXPLAIN QUERY PLAN
• test with realistic data and the actual ordering/index strategy

If the unified account query regresses, do not force it. Keep two queries there.

Optional tiny helper for limit+1 trimming

If the trimming logic repeats too much, add one very small helper.

func BuildResult[T any, C any](
  items []T,
  limit uint32,
  cursorOf func(T) C,
) page.Result[T, C] {
  n := int(limit)
  if len(items) <= n {
    return page.Result[T, C]{Items: items}
  }

  cursor := cursorOf(items[n-1])
  return page.Result[T, C]{
    Items: items[:n],
    Next:  &cursor,
  }
}

That is the kind of generic helper that helps without taking over the design.

This is very different from the current multi-layer generic abstraction.

What the API should not do

I would explicitly avoid these patterns:

• do not make callers compute the next cursor from the last item
• do not put Cursor() methods on public info types
• do not hide plain data behind builder methods like WithSize().WithCursor()
• do not introduce a generic pagination helper that is more complicated than the call sites
• do not require an extra empty fetch just to detect completion
• do not let SQLC-generated first/next row types dictate the public API

Testing strategy

Once redesigned, the tests should validate the actual contract.

At the page type level:

• Limit == 0 uses DefaultLimit
• Limit > MaxLimit clamps to MaxLimit

At the store level:

• zero-value query returns the first page with DefaultLimit
• explicit Limit is honored
• Next == nil when the result fits in one page
• Next != nil when more items exist
• following Next yields the remaining items without duplicates or gaps
• accounts cross the derived/imported boundary correctly
• scope and name filters still page correctly
• iteration matches concatenated page traversal

At the iterator level:

• stop on consumer break
• surface errors once
• if you support it, stop promptly on mid-page cancellation
• do not do the old extra empty fetch

That last one is important: with LIMIT+1, exact-multiple traversals should not need one final empty-page round trip.

Migration plan

I would do this in stages.

1. Introduce page.Request and page.Result
2. Convert ListWallets first, because it is the simplest case
3. Convert IterWallets to use the new result type
4. Repeat for addresses
5. Convert accounts last
6. Remove AccountInfo.Cursor() and similar cursor-derivation helpers from public types
7. Delete page.Options and the current generic page.Iter
8. Optionally unify SQL where planner behavior is acceptable

That gives you a clean incremental path.

Recommended final shape

If I had to boil it down to one target design, it would be this:

package page

type Request[C any] struct {
  Limit uint32
  After *C
}

type Result[T any, C any] struct {
  Items []T
  Next  *C
}

type AccountStore interface {
  ListAccounts(ctx context.Context, query ListAccountsQuery) (
    page.Result[AccountInfo, AccountCursor], error,
  )
  IterAccounts(ctx context.Context, query ListAccountsQuery) iter.Seq2[AccountInfo, error]
}

res, err := store.ListAccounts(ctx, query)
if err != nil {
  return err
}

for _, item := range res.Items {
  ...
}

if res.Next != nil {
  query.Page.After = res.Next
}

That is the core redesign.

It fixes the ergonomics, keeps pagination ownership in the right layer, and lets you simplify the implementation without over-abstracting it.

[GustavoStingelin]

Regarding returning a page struct with the list inside, that was my initial approach. However, after looking at LND, I noticed it returns only the slice, with the cursor handled externally. Since termination can be inferred from an empty list, this seemed simpler at first.

That said, I missed an important point you raised: this approach shifts the responsibility to the caller to understand how to assemble the cursor correctly. Because of that, I am now including the page struct to make the contract more explicit and reduce the risk of misuse.

[saubyk]

is this pr still in draft?

[GustavoStingelin]

is this pr still in draft?

yes, will be ready soon

[GustavoStingelin]

I ran benchmarks comparing the split queries, with and without cursor, against the unified approach. The results show the split version is generally about 7% faster. For PostgreSQL, the results are mixed, sometimes slightly faster, sometimes slightly slower, within a similar range.

Full benchmark details are available here: https://github.com/GustavoStingelin/btcwallet/blob/6eb7ab030a1899aba0a92e5f12ea2c8ad1763caf/wallet/internal/db/itest/PAGINATION_REPORT.md

Given that the performance difference is relatively small, I agree it is worth simplifying the implementation by using a single conditional cursored query, and I will do this.