Add paginated VTXO listing with filter support#175
Conversation
Introduce offset-based pagination and filtering for VTXO listing. Merge ListVtxos and ListSpendableVtxos into a single ListVtxos method that accepts a Page (pageNum, pageSize) and VtxoFilter (all, spendable, spent, recoverable). - Add Page struct with MaxPageSize=200 and zero-value "return all" semantics - Add VtxoFilter enum (All, Spendable, Spent, Recoverable) - Implement SQL pagination via subquery on vtxo table to handle multi-asset VTXOs - Update all internal callers and e2e tests
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (12)
WalkthroughAdds paginated, filterable VTXO listing (Page, VtxoFilter), implements paginated SQL queries, repurposes store GetVtxos and adds GetVtxosByOutpoint, updates wallet/sdk APIs and call sites, and augments store and E2E tests to use the new API. ChangesPaginated and filterable VTXO listing API
Sequence Diagram(s)sequenceDiagram
participant Client
participant Wallet
participant VtxoStore
participant SQL
Client->>Wallet: ListVtxos(ctx, Page{}, VtxoFilterSpendable)
Wallet->>VtxoStore: GetVtxos(ctx, Page{}, VtxoFilterSpendable)
VtxoStore->>SQL: SelectSpendableVtxosPaginated(limit, offset)
SQL-->>VtxoStore: []AssetVtxoVw rows
VtxoStore-->>Wallet: []Vtxo (grouped by outpoint)
Wallet-->>Client: []Vtxo, error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
store/sql/vtxo_store.go (1)
448-467:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve row order when collapsing multi-asset VTXOs.
assetVtxoVwRowsToVtxosgroups into a map and then ranges that map, which randomizes the final slice order. Even with ordered SQL, callers will still see nondeterministic VTXO ordering across pages.Suggested fix
func assetVtxoVwRowsToVtxos(rows []queries.AssetVtxoVw) []clientTypes.Vtxo { - byOutpoint := make(map[string][]queries.AssetVtxoVw) + byOutpoint := make(map[string][]queries.AssetVtxoVw, len(rows)) + order := make([]string, 0, len(rows)) for _, row := range rows { key := fmt.Sprintf("%s:%d", row.Txid, row.Vout) + if _, ok := byOutpoint[key]; !ok { + order = append(order, key) + } byOutpoint[key] = append(byOutpoint[key], row) } - vtxos := make([]clientTypes.Vtxo, 0, len(byOutpoint)) - for _, group := range byOutpoint { - vtxo := assetVtxoVwGroupToVtxo(group) - vtxos = append(vtxos, vtxo) + vtxos := make([]clientTypes.Vtxo, 0, len(order)) + for _, key := range order { + vtxos = append(vtxos, assetVtxoVwGroupToVtxo(byOutpoint[key])) } return vtxos }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@store/sql/vtxo_store.go` around lines 448 - 467, The current assetVtxoVwRowsToVtxos groups rows into a map and then ranges it, which yields nondeterministic ordering; change it to preserve input order by recording outpoint keys as you encounter them (e.g., maintain a []string keys alongside the byOutpoint map or build groups in an ordered slice of groups) and then iterate that ordered keys/slice to call assetVtxoVwGroupToVtxo and append results; this keeps the final []clientTypes.Vtxo in the same order as the incoming rows while still collapsing multi-asset VTXOs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@init.go`:
- Line 219: The call to GetVtxos on w.store.VtxoStore() is too long for golines;
break the invocation across multiple lines so each argument is on its own line
(or introduce a short ctx variable) to reduce line length—e.g. assign ctx :=
context.Background() and call vtxos, err := w.store.VtxoStore().GetVtxos(ctx,
types.Page{}, types.VtxoFilterSpendable) with the arguments wrapped or split so
the line complies with golines while retaining the same variables and function
names (GetVtxos, VtxoStore, types.Page{}, types.VtxoFilterSpendable, vtxos,
err).
In `@sdk.go`:
- Line 87: The ListVtxos function signature is exceeding golines rules; reformat
the signature for ListVtxos so it complies with golines line-length/formatting
(e.g., break parameters onto separate lines and align the return types) while
preserving types: ListVtxos(ctx context.Context, page types.Page, filter
types.VtxoFilter) ([]clienttypes.Vtxo, error); ensure the new multi-line
signature matches the project's existing function signature style and compiles.
In `@store/sql/sqlc/queries/query_paginated.go`:
- Around line 19-26: The query using the IN(subquery) pattern (e.g.,
selectAllVtxosPaginated) relies on the subquery ORDER BY but IN does not
preserve that order; add an explicit outer ORDER BY clause to the main SELECT to
enforce "created_at DESC, txid ASC, vout ASC" and make page boundaries stable.
Apply the same change to the other paginated query constants referenced in the
file (the spendable/spent variants and selectSpentVtxos) so each top-level
SELECT has the same explicit ORDER BY sequence as the subquery.
In `@store/sql/vtxo_store.go`:
- Around line 323-364: The current code paginates via SelectAllVtxosPaginated
then applies IsRecoverable, which can return fewer than limit results per page;
instead, when filter == types.VtxoFilterRecoverable, fetch the full set using
v.querier.SelectAllVtxos (or SelectAllVtxos when allSpendable/allSpent/all are
not appropriate), convert with assetVtxoVwRowsToVtxos, apply IsRecoverable to
produce a filtered slice, and then apply pagination in Go using limit/offset
before returning; adjust the switch to route Recoverable to the non-paginated
SelectAllVtxos path and perform the manual slice pagination after filtering.
In `@types/types.go`:
- Around line 127-129: The comment for VtxoFilterSpendable is misleading about
"actively usable" — update the doc on the VtxoFilterSpendable constant/enum to
state it matches entries where !spent && !unrolled and therefore still includes
recoverable/expired or swept‑yet‑unspent VTXOs (i.e., not strictly guaranteed to
be usable for new off‑chain transactions); reference the VtxoFilterSpendable
symbol and explicitly mention the boolean semantics (!spent && !unrolled) so the
contract is clear.
---
Outside diff comments:
In `@store/sql/vtxo_store.go`:
- Around line 448-467: The current assetVtxoVwRowsToVtxos groups rows into a map
and then ranges it, which yields nondeterministic ordering; change it to
preserve input order by recording outpoint keys as you encounter them (e.g.,
maintain a []string keys alongside the byOutpoint map or build groups in an
ordered slice of groups) and then iterate that ordered keys/slice to call
assetVtxoVwGroupToVtxo and append results; this keeps the final
[]clientTypes.Vtxo in the same order as the incoming rows while still collapsing
multi-asset VTXOs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60575f72-6e0f-4682-b111-5d5892f7c892
📒 Files selected for processing (15)
funding.goinit.gosdk.gosend.gostore/service_test.gostore/sql/sqlc/queries/query_paginated.gostore/sql/vtxo_store.gotest/e2e/asset_test.gotest/e2e/exit_test.gotest/e2e/hd_wallet_test.gotest/e2e/restore_smoke_test.gotest/e2e/transaction_test.gotypes/interfaces.gotypes/types.gowallet.go
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arkana Code Review — feat/pagination-vtxo-list
Good refactor overall — unifying the VTXO query surface is the right move. But there are two bugs and one significant downstream breakage concern.
🔴 BUG: Result ordering within pages is non-deterministic
store/sql/vtxo_store.go — assetVtxoVwRowsToVtxos() (≈line 432)
The SQL subqueries correctly use ORDER BY created_at DESC, txid ASC, vout ASC to select which VTXOs land on which page. But assetVtxoVwRowsToVtxos groups rows into a map[string][]queries.AssetVtxoVw and then iterates the map:
for _, group := range byOutpoint {
vtxos = append(vtxos, assetVtxoVwGroupToVtxo(group))
}Go map iteration order is random. The ORDER BY from SQL is discarded. Callers paginating through results will see VTXOs in random order within each page — this defeats the purpose of pagination.
Fix: Use an ordered structure (e.g., a []string of keys preserving insertion order from the row scan) to iterate byOutpoint deterministically. Alternatively, sort the final vtxos slice by CreatedAt DESC before returning.
🔴 BUG: VtxoFilterRecoverable pagination is broken
store/sql/vtxo_store.go — GetVtxos() (≈line 350-355)
When filter == VtxoFilterRecoverable with pagination, the code fetches N rows from SelectAllVtxosPaginated, then filters Go-side:
if filter == types.VtxoFilterRecoverable {
filtered := make([]clientTypes.Vtxo, 0, len(vtxos))
for _, v := range vtxos {
if v.IsRecoverable() { filtered = append(filtered, v) }
}
return filtered, nil
}This means:
- Request
PageSize=10→ fetch 10 "all" VTXOs → filter → might return 0-10 recoverable VTXOs - Subsequent pages miss recoverable VTXOs that were counted as "all" but not recoverable on previous pages
- Pagination invariant is violated: no correct way to enumerate all recoverable VTXOs page-by-page
Fix options:
- Don't support pagination for recoverable (return error if
PageSize > 0 && filter == VtxoFilterRecoverable) - Move the recoverable check into SQL (e.g.,
WHERE swept = true AND spent = false AND expires_at < ?) — though this may not capture the fullIsRecoverable()logic - Document clearly that recoverable filter always returns all results (ignores pagination)
🟡 API Design: No total count or "has more" indicator
types/interfaces.go — GetVtxos returns ([]types.Vtxo, error)
A pagination API without a total count forces callers to:
- Keep requesting pages until they get an empty result
- Cannot render "page X of Y" or know the total VTXO count without a separate query
Consider returning a result struct: type VtxoPage struct { Vtxos []Vtxo; Total int; Page int } or at minimum a hasMore bool.
🟡 Cross-repo breakage (5+ repos affected)
This PR removes ListSpendableVtxos() and changes the ListVtxos() signature. The following downstream repos will break at compile time:
| Repo | File | Call |
|---|---|---|
bancod |
pkg/contract/taker.go:95 |
ListSpendableVtxos(ctx) |
bancod |
test/e2e/utils_test.go:144 |
ListVtxos(t.Context()) (old 3-return sig) |
demos |
asset/golang/main.go:100 |
ListSpendableVtxos(ctx) |
asset-demos |
asset/golang/main.go:100 |
ListSpendableVtxos(ctx) |
layerzero-usdt0-arkade-demo |
test/e2e_flow_test.go:519 |
ListVtxos(ctx) (old sig) |
introspector-review |
test/utils_test.go:540, contract_id_test.go:151 |
ListVtxos(ctx) (old sig) |
Please either:
- Open companion PRs for all affected repos, or
- Keep backward-compatible wrappers (deprecated) during transition
✅ What looks good
- Subquery-based pagination for multi-asset VTXOs is correct — paginates at the vtxo level, not the view level. Well-documented rationale in
query_paginated.go. pageToLimitOffsetcorrectly handles edge cases (PageNum=0 → 1, clamping to MaxPageSize).- Removing the manual
Spent || Unrolledcheck ingetOffchainBalanceis correct — the SQL filter now handles it. - Test coverage is solid —
TestVtxoPaginationcovers page boundaries, partial pages, multi-asset, MaxPageSize clamping, and spent filter. GetVtxosByOutpointrename is clearer than the overloadedGetVtxos.
Verdict: Request changes for the two bugs (ordering + recoverable pagination). The cross-repo breakage needs a migration plan before merge.
getOffchainBalance or getSpendableVtxos to return incomplete results if someone accidentally passes a Page with PageSize > 0 in internal callers. Current internal callers all use Page{} (fetch all), which is correct. Flagging for human review as the VTXO store interface is protocol-adjacent.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
There was a problem hiding this comment.
Arkana Follow-up Review — 32a3e2f lint
The new commit is formatting-only (line wrapping for linter). Both bugs from my previous review are still open:
🔴 Still open: Map iteration destroys SQL ordering
store/sql/vtxo_store.go:462-466 — assetVtxoVwRowsToVtxos still iterates byOutpoint map with for _, group := range byOutpoint, which randomizes the order that SQL carefully established. Pagination without deterministic ordering is broken.
🔴 Still open: VtxoFilterRecoverable + pagination returns wrong results
store/sql/vtxo_store.go:348-370 — Recoverable filter still fetches N "all" rows then filters Go-side, violating pagination invariants.
🟡 Still open: Cross-repo breakage plan needed
No companion PRs visible yet for downstream consumers (bancod, demos, etc.).
No new issues introduced by the lint commit. Maintaining request-changes until the two bugs are resolved.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
| ListVtxos( | ||
| ctx context.Context, | ||
| page types.Page, | ||
| filter types.VtxoFilter, |
There was a problem hiding this comment.
wouldn't be nice to keep having the option pattern so the user just calls (?):
ListVtxos(ctx) -> gets all vtxos, paginated by default if the response is too big
// Or
ListVtxos(ctx, WithPage(token)) -> gets a specific page given its token
// Or
ListVtxos(ctx, WithSpendableOnly()) -> gets only spendable vtxos, paginated by default if the response is too big
// Or
ListVtxos(ctx, WithSpendableOnly(), WithPage(token)) -> gets a specific page of only spendable vtxos
2 participants
Summary
Page{PageNum, PageSize}withMaxPageSize=200ListVtxosandListSpendableVtxosinto a singleListVtxos(ctx, page, filter)methodVtxoFilterenum:All,Spendable,Spent,RecoverablePage{}zero-value means "return all" — backward compatible for internal callersvtxotable (not theasset_vtxo_vwview) to correctly handle multi-asset VTXOs at page boundariesMotivation
Addresses the root cause behind ArkLabsHQ/fulmine#405, where unbounded
GetVtxosresponses exceed gRPC's default 4 MiB receive limiton wallets with large VTXO history (12k+ VTXOs observed in production). Raising the gRPC limit is a bandage — pagination is the proper fix.
API Changes (breaking)