Skip to content

Cursor-based pagination#178

Open
altafan wants to merge 2 commits into
masterfrom
pagination
Open

Cursor-based pagination#178
altafan wants to merge 2 commits into
masterfrom
pagination

Conversation

@altafan
Copy link
Copy Markdown
Contributor

@altafan altafan commented May 15, 2026

This adds cursor-based pagination option to ListVtxos:

  • the cursor represents the first vtxo of the very next page.
  • page size (Limit) can be customized at will between pages. Defaults to max value (1000) and can assume any value in range [1, 1000].
  • to correctly implement pagination, the db always orders vtxos by creation time (+ vtxo oupoint to resolve deterministically if many vtxos have the same creation time) DESC, and it's non-sensitive to insertions happening while retrieving pages.
  • adds tests for the new pagination system

Please @sekulicd review

Summary by CodeRabbit

  • New Features

    • Added filtering options for VTXO queries: filter by spendable/spent status and asset ID.
    • Introduced pagination support with cursor-based navigation for VTXO listings.
  • Breaking Changes

    • Updated ListVtxos API to accept filter options and return pagination cursors; previously returned separate spendable/spent lists.
    • Removed ListSpendableVtxos method; use ListVtxos with spendable filter option instead.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Walkthrough

This PR refactors VTXO listing from a tuple-based API returning separate spendable/spent slices to a new option-driven filtering and pagination model with cursor support. New pagination types, SQL keyset-pagination query, store method reorganization, and comprehensive test coverage enable efficient VTXO querying by status, asset, and page boundary.

Changes

VTXO pagination and filtering refactor

Layer / File(s) Summary
Pagination types and option framework
types/interfaces.go, funding_opts.go, funding_opts_test.go
New VtxoStatusFilter, GetVtxoFilter, Cursor, and VtxoPageResult types. Option constructors WithSpendableOnly, WithSpentOnly, WithAssetID, WithLimit, WithCursor with validation, cursor encoding/decoding, and filter-hash binding. Unit tests verify option behavior, cursor round-trip, and filter-hash determinism.
SQL query layer and pagination index
store/sql/migration/20260514120000_add_vtxo_pagination_index.{up,down}.sql, store/sql/sqlc/query.sql, store/sql/sqlc/queries/query.sql.go
New index on (created_at DESC, txid DESC, vout DESC) for keyset pagination. GetVtxos query uses two-stage CTE hydration with status/asset/cursor filters and limit-plus-one sentinel. Generated methods include new paginated GetVtxos with GetVtxosParams, remove SelectAllVtxos, and rename SelectSpendableVtxos to SelectSpendableOrRecoverableVtxos.
VtxoStore implementation refactor
store/sql/vtxo_store.go
Write paths (AddVtxos, SpendVtxos, SweepVtxos, UnrollVtxos, SettleVtxos) now route through new GetVtxosByOutpoints helper and use clienttypes consistently. Read methods: removed GetAllVtxos/GetSpendableVtxos, added GetSpendableOrRecoverableVtxos and GetVtxosByOutpoints, replaced old GetVtxos with filter-driven paginated version returning cursor. Conversion helpers updated for clienttypes and order preservation.
SDK interface and wallet implementation
sdk.go, funding.go, wallet.go, init.go, send.go
Wallet.ListVtxos interface signature changed to ListVtxos(ctx, opts ...ListVtxosOption) ([]Vtxo, string, error) with ListSpendableVtxos removed. Implementation applies options, validates cursor filter hash, queries store, and returns encoded cursor. Transaction handlers, refresh, and scheduling updated to use GetVtxosByOutpoints and GetSpendableOrRecoverableVtxos.
VtxoStore comprehensive test suite
store/vtxo_store_test.go
10 test functions covering AddVtxos, SpendVtxos, SettleVtxos, SweepVtxos, UnrollVtxos, GetSpendableOrRecoverableVtxos, GetVtxosByOutpoints, paginated GetVtxos with cursor/status/asset filters and deterministic tiebreaker, Clean, and GetEventChannel event emission. Fixtures and helpers seed test data and compare results by outpoint.
E2E test updates and service test cleanup
test/e2e/asset_test.go, test/e2e/exit_test.go, test/e2e/hd_wallet_test.go, test/e2e/restore_smoke_test.go, test/e2e/transaction_test.go, store/service_test.go
Service tests remove VTXO fixtures and helpers; spend-key assertions corrected. E2E tests updated: asset_test.go uses WithSpendableOnly(), exit_test.go uses WithSpentOnly(), hd_wallet_test.go splits unfiltered calls into separate spendable/spent queries, restore_smoke_test.go uses WithSpendableOnly() and accesses returned page.Vtxos, transaction_test.go adds import and uses WithSpendableOnly().

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • arkade-os/go-sdk#174: Introduces ListSpendableVtxos/ListVtxos methods that this PR removes and replaces with option-based filtering.
  • arkade-os/go-sdk#49: Adds ListSpendableVtxos/GetSpendableVtxos that this PR removes in favor of WithSpendableOnly() option and GetSpendableOrRecoverableVtxos store method.
  • arkade-os/go-sdk#6: Earlier VTXO listing API changes that this PR's ListVtxos refactor directly supersedes.

Suggested reviewers

  • sekulicd
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Cursor-based pagination' accurately describes the primary change: introducing cursor-based pagination to the ListVtxos API.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pagination

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@arkanaai arkanaai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arkana Code Review — #178

Overall: well-designed pagination. Keyset pagination on (created_at DESC, txid DESC, vout DESC) with the limit+1 sentinel pattern is the right approach. The CTE-then-JOIN strategy to avoid cutting multi-asset VTXOs mid-page is smart. Filter hash binding the cursor to its filter set is a nice safety net.

However, there are blocking issues that must be fixed before merge.


🔴 BLOCKING — Compile Error

test/e2e/restore_smoke_test.go:50,94 — These lines will not compile.

page, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())
vtxos := page.Vtxos

ListVtxos returns ([]clienttypes.Vtxo, string, error) — three return values, not two. Assigning 3 returns to 2 variables is a Go compile error. And page would be a []Vtxo slice, so page.Vtxos is nonsensical.

This test was clearly written for a VtxoPageResult-based return type that was abandoned in favor of the (slice, cursor_string, error) tuple. Fix: use the same pattern as the other e2e tests:

vtxos, _, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())

🔴 BLOCKING — Test Assertion Bug

funding_opts_test.go:91-94 — This test will fail:

out, err := decodeCursor("")
require.NoError(t, err)
require.Equal(t, vtxoCursor{}, out)  // ← FAILS

decodeCursor("") returns (*vtxoCursor)(nil), nil. The assertion compares a vtxoCursor value with a *vtxoCursor nil pointer — reflect.DeepEqual returns false because the types differ. Fix either:

  • Change assertion to require.Nil(t, out), or
  • Change decodeCursor to return &vtxoCursor{}, nil for empty input (though nil is the better semantic for "no cursor").

🟡 Dead Type

types/interfaces.go:29-33VtxoPageResult is defined but never referenced anywhere in the PR. It's a leftover from the earlier design that used a struct return. Remove it to avoid confusion.


🟡 Cross-Repo Breakage — Downstream Consumers

This PR removes ListSpendableVtxos and changes the ListVtxos signature on the public Wallet interface. The following downstream repos will break at compile time on their next go get:

Repo Files Old API used
bancod pkg/contract/taker.go, test/e2e/utils_test.go ListSpendableVtxos, ListVtxos (old sig)
introspector-review test/utils_test.go, test/contract_id_test.go ListVtxos (old sig)
demos / asset-demos asset/golang/main.go ListSpendableVtxos
layerzero-usdt0-arkade-demo test/e2e_flow_test.go ListVtxos (old sig)

Recommendation: Open companion PRs in affected repos (at minimum bancod since it's production) before or alongside this merge, and pin the go.mod upgrade together.


✅ Protocol Correctness Notes

  • getOffchainBalance switch from GetAllVtxos + in-memory filter to GetSpendableOrRecoverableVtxos is logically equivalent — the SQL WHERE spent = false AND unrolled = false matches the old if vtxo.Spent || vtxo.Unrolled { continue } guard. Correct.
  • getSpendableVtxos in send.go and scheduleNextSettlement in init.go — same rename, same SQL, no behavioral change. Correct.
  • wallet.go changes from GetVtxos(ctx, outpoints)GetVtxosByOutpoints(ctx, outpoints) are pure renames. Correct.
  • The assetVtxoVwRowsToVtxos fix to preserve insertion order via the order slice is necessary for pagination to work — without it, map iteration randomizes page order. Good catch.

✅ SQL Review

  • Composite index idx_vtxo_created_at_txid_vout ON vtxo (created_at DESC, txid DESC, vout DESC) matches the ORDER BY and cursor predicate exactly. The keyset scan is O(log n + limit). Good.
  • Row-value comparison (created_at, txid, vout) < (?, ?, ?) is supported in SQLite ≥ 3.15.0 (2016). Safe.
  • The EXISTS subquery for asset filtering avoids inflating the row count. Correct.
  • The CAST wrappers for nullable params are the right sqlc idiom.

✅ Pagination Design

  • Limit+1 sentinel pattern: clean, standard.
  • Cursor encodes as base64url(JSON) with a filter hash — prevents cursor reuse across different filter sets. Good.
  • Filter hash is sha256[:8] of status+asset — 64 bits is ample for accidental mismatch detection (this isn't a security boundary).
  • encodeCursor ignoring the json.Marshal error is fine for a struct of primitives.

⚠️ VTXO Handling — Human Review Required

This PR modifies the VTXO listing and storage interfaces. While the changes are well-structured and the pagination logic is sound, these are protocol-critical interfaces per project rules. A human reviewer must sign off before merge.

Summary: Fix the two compile/test bugs in restore_smoke_test.go and funding_opts_test.go, remove the dead VtxoPageResult type, and coordinate downstream repo updates. The core pagination design is solid.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
store/sql/sqlc/query.sql (1)

50-52: 💤 Low value

Clarify which row becomes the cursor.

The header reads "the caller trims the last one and uses its outpoint as the next cursor". Taken literally, "its" refers to the trimmed extra row; combined with the strict < cursor predicate at lines 73-78, that would skip the extra row on the next page (data loss). The store code in vtxo_store.go (last := vtxos[len(vtxos)-1] after vtxos = vtxos[:q.Limit]) actually uses the last row of the returned page, which is the correct pairing with <. Tighten the wording so a future maintainer doesn't "fix" one side or the other into a row-skipping bug.

📝 Suggested wording
 -- The caller passes (user_limit + 1) as limit_plus_one. The extra row is
--- a has-more sentinel; if SQL returns more than user_limit rows the caller
--- trims the last one and uses its outpoint as the next cursor.
+-- a has-more sentinel; if SQL returns more than user_limit rows the caller
+-- drops the trailing sentinel and uses the outpoint of the LAST RETAINED
+-- row as the next cursor. Paired with the strict `<` predicate below, the
+-- sentinel becomes the first row of the next page.
🤖 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/sqlc/query.sql` around lines 50 - 52, The SQL header comment for
the pagination query is ambiguous about which row becomes the next cursor and
could mislead maintainers into causing off-by-one skips; update the comment to
state that the caller supplies limit_plus_one (user_limit + 1), trims the extra
sentinel row to present exactly user_limit rows, and uses the last row of the
returned page (e.g., the element referenced by last := vtxos[len(vtxos)-1] after
vtxos = vtxos[:q.Limit] in vtxo_store.go) as the next cursor so it pairs
correctly with the strict "<" cursor predicate; keep mention of the sentinel but
explicitly say the cursor comes from the retained last page row, not the trimmed
sentinel.
types/interfaces.go (1)

35-40: 💤 Low value

VtxoPageResult is defined but never used by the interface.

The doc comment states VtxoPageResult is "the typed page response from VtxoStore.GetVtxos", but GetVtxos at Line 99 returns ([]types.Vtxo, *Cursor, error) rather than (*VtxoPageResult, error). Either drop the type, or change the interface signature so the returned page is a single value (which also makes the API a bit harder to misuse — callers can't accidentally read the cursor when an error is returned).

Option A — drop the unused type
-// VtxoPageResult is the typed page response from VtxoStore.GetVtxos.
-// Next is nil when there is no further page.
-type VtxoPageResult struct {
-	Vtxos []types.Vtxo
-	Next  *Cursor
-}
-
Option B — use it as the GetVtxos return value
-	GetVtxos(ctx context.Context, q GetVtxoFilter) ([]types.Vtxo, *Cursor, error)
+	GetVtxos(ctx context.Context, q GetVtxoFilter) (*VtxoPageResult, error)

Also applies to: 99-99

🤖 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 `@types/interfaces.go` around lines 35 - 40, VtxoPageResult is declared but not
used by the VtxoStore.GetVtxos signature; update the API to be consistent by
either removing VtxoPageResult or changing GetVtxos to return (*VtxoPageResult,
error). To fix, pick one option: (A) delete the VtxoPageResult type and keep
GetVtxos returning ([]types.Vtxo, *Cursor, error), or (B) change the
VtxoStore.GetVtxos method signature to func GetVtxos(...) (*VtxoPageResult,
error) and update all callers to construct/consume VtxoPageResult (use
VtxoPageResult.Vtxos and VtxoPageResult.Next), ensuring error handling no longer
expects a cursor when error != nil.
🤖 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 `@funding_opts_test.go`:
- Around line 61-74: The tests in TestVtxoCursorCodec compare a vtxoCursor value
to the pointer returned by decodeCursor, causing type-mismatch failures; update
the "roundtrip" subtest to call decodeCursor and dereference the returned
*vtxoCursor (e.g., outPtr -> *outPtr) before comparing to the expected
vtxoCursor, and update the "empty cursor" subtest to assert decodeCursor("")
returns nil (or check out == nil) and then compare the zero value by either
ensuring out is nil or by dereferencing a non-nil pointer to compare to
vtxoCursor{}; touch the TestVtxoCursorCodec, encodeCursor, and decodeCursor
usages accordingly.

In `@store/sql/vtxo_store.go`:
- Around line 334-392: GetVtxos currently panics when q.Limit <= 0 or negative;
add a guard at the start of GetVtxos that validates/coerces q.Limit into a safe
range (e.g. if q.Limit <= 0 set q.Limit = 1; optionally clamp above to 1000),
then recompute params.LimitPlusOne from the coerced q.Limit before calling
v.querier.GetVtxos; this ensures the later slice logic (vtxos[:q.Limit], last :=
vtxos[len(vtxos)-1]) and cursor creation use a non-zero positive limit and avoid
index-out-of-range panics.

In `@store/vtxo_store_test.go`:
- Around line 19-132: The tests reference a missing fixture variable named
settledBy causing compilation failures; add a new string variable/constant
called settledBy in the fixtures block (alongside arkTxid/testVtxo* definitions)
with the expected value used by the tests (e.g. a txid-like hex string) and
ensure its type matches usages (string) so references to settledBy in the tests
compile.

In `@test/e2e/restore_smoke_test.go`:
- Around line 50-52: The test incorrectly unpacks ListVtxos (which returns
[]Vtxo, string, error) into two variables; change the call to capture three
returns (e.g., restoredVtxos, _, err := alice.ListVtxos(ctx,
arksdk.WithSpendableOnly())) and then use the first returned slice (replace
page.Vtxos uses with the new restoredVtxos variable) so the values and error are
handled correctly.
- Around line 95-97: The call to ListVtxos in the restored path is unpacking the
wrong number of return values—ListVtxos returns ([]Vtxo, string, error). Update
the call to capture three values (e.g., vtxosRestored, nextCursorRestored, err)
instead of two, then use require.NoError(t, err, ...) and set vtxosRestored :=
vtxosRestored (or use the captured vtxos variable) and/or pass
nextCursorRestored where appropriate so the returned page token is not lost;
locate the call to ListVtxos and the variables pageRestored/vtxosRestored to
make this change.

---

Nitpick comments:
In `@store/sql/sqlc/query.sql`:
- Around line 50-52: The SQL header comment for the pagination query is
ambiguous about which row becomes the next cursor and could mislead maintainers
into causing off-by-one skips; update the comment to state that the caller
supplies limit_plus_one (user_limit + 1), trims the extra sentinel row to
present exactly user_limit rows, and uses the last row of the returned page
(e.g., the element referenced by last := vtxos[len(vtxos)-1] after vtxos =
vtxos[:q.Limit] in vtxo_store.go) as the next cursor so it pairs correctly with
the strict "<" cursor predicate; keep mention of the sentinel but explicitly say
the cursor comes from the retained last page row, not the trimmed sentinel.

In `@types/interfaces.go`:
- Around line 35-40: VtxoPageResult is declared but not used by the
VtxoStore.GetVtxos signature; update the API to be consistent by either removing
VtxoPageResult or changing GetVtxos to return (*VtxoPageResult, error). To fix,
pick one option: (A) delete the VtxoPageResult type and keep GetVtxos returning
([]types.Vtxo, *Cursor, error), or (B) change the VtxoStore.GetVtxos method
signature to func GetVtxos(...) (*VtxoPageResult, error) and update all callers
to construct/consume VtxoPageResult (use VtxoPageResult.Vtxos and
VtxoPageResult.Next), ensuring error handling no longer expects a cursor when
error != nil.
🪄 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: 424356ef-7d06-4699-ae94-8763769215c9

📥 Commits

Reviewing files that changed from the base of the PR and between 625a2e2 and d81cd4a.

📒 Files selected for processing (20)
  • funding.go
  • funding_opts.go
  • funding_opts_test.go
  • init.go
  • sdk.go
  • send.go
  • store/service_test.go
  • store/sql/migration/20260514120000_add_vtxo_pagination_index.down.sql
  • store/sql/migration/20260514120000_add_vtxo_pagination_index.up.sql
  • store/sql/sqlc/queries/query.sql.go
  • store/sql/sqlc/query.sql
  • store/sql/vtxo_store.go
  • store/vtxo_store_test.go
  • test/e2e/asset_test.go
  • test/e2e/exit_test.go
  • test/e2e/hd_wallet_test.go
  • test/e2e/restore_smoke_test.go
  • test/e2e/transaction_test.go
  • types/interfaces.go
  • wallet.go

Comment thread funding_opts_test.go
Comment on lines +61 to +74
func TestVtxoCursorCodec(t *testing.T) {
t.Run("roundtrip", func(t *testing.T) {
in := vtxoCursor{CreatedAt: 1234567, Txid: "abc", VOut: 7, FilterHash: "deadbeef"}
s := encodeCursor(in)
out, err := decodeCursor(s)
require.NoError(t, err)
require.Equal(t, in, out)
})

t.Run("empty cursor decodes to zero value with no error", func(t *testing.T) {
out, err := decodeCursor("")
require.NoError(t, err)
require.Equal(t, vtxoCursor{}, out)
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm decodeCursor signature so the type mismatch is unambiguous.
rg -nP 'func\s+decodeCursor\s*\(' 
# Confirm there are no compatibility helpers or shims wrapping decodeCursor
# that might massage the return value.
rg -nP '\bdecodeCursor\s*\(' -C2

Repository: arkade-os/go-sdk

Length of output: 2270


🌐 Web query:

testify require.Equal compare struct value against pointer reflect.DeepEqual behavior

💡 Result:

The testify library's require.Equal (and assert.Equal) function relies on ObjectsAreEqual to determine equality [1][2]. ObjectsAreEqual, in turn, uses reflect.DeepEqual for most types [1]. Because it uses reflect.DeepEqual, testify's equality behavior for pointers is identical to that of the standard library's reflect.DeepEqual [1]. Specifically: 1. Pointer Comparison: reflect.DeepEqual considers two pointer values to be deeply equal if they are equal using Go's == operator (i.e., they point to the same memory address) OR if they point to values that are themselves deeply equal [3][4]. This means that if you compare a struct value to a pointer to that same struct, they will generally not be considered equal because their types differ (a struct type is not the same as a pointer-to-struct type) [5][3]. 2. Struct vs. Pointer: If you are comparing a struct value (e.g., MyStruct{...}) against a pointer to a struct (e.g., &MyStruct{...}), reflect.DeepEqual will return false because the types are not identical [3]. To compare them, you must manually dereference the pointer (e.g., require.Equal(t, expectedStruct, *actualPointer)) [6][7]. 3. Limitations: Neither testify's ObjectsAreEqual nor reflect.DeepEqual can automatically dereference pointers when comparing different types [6]. If you need to compare a struct value against a pointer, you must dereference the pointer yourself [6]. For more complex comparison needs—such as ignoring specific fields, handling unexported fields, or providing better diff output when comparisons fail—the Go community often recommends using the google/go-cmp library instead of reflect.DeepEqual or testify's default equality assertions [8][9].

Citations:


TestVtxoCursorCodec assertions compare incompatible types — both subtests will fail.

decodeCursor returns (*vtxoCursor, error), so out is always a pointer. However, both test assertions compare it directly to vtxoCursor values using require.Equal, which relies on reflect.DeepEqual. Since reflect.DeepEqual returns false when comparing different types (a struct value cannot equal a pointer to that struct), both assertions fail unconditionally:

  • Line 67: require.Equal(t, in, out)in is vtxoCursor, out is *vtxoCursor. Type mismatch.
  • Line 73: require.Equal(t, vtxoCursor{}, out) — comparing a struct value to a nil pointer. Type mismatch.

Fix by dereferencing the pointer in the roundtrip test and using explicit nil checks for the empty-cursor case:

Proposed fix
 	t.Run("roundtrip", func(t *testing.T) {
 		in := vtxoCursor{CreatedAt: 1234567, Txid: "abc", VOut: 7, FilterHash: "deadbeef"}
 		s := encodeCursor(in)
 		out, err := decodeCursor(s)
 		require.NoError(t, err)
-		require.Equal(t, in, out)
+		require.NotNil(t, out)
+		require.Equal(t, in, *out)
 	})
 
-	t.Run("empty cursor decodes to zero value with no error", func(t *testing.T) {
+	t.Run("empty cursor decodes to nil with no error", func(t *testing.T) {
 		out, err := decodeCursor("")
 		require.NoError(t, err)
-		require.Equal(t, vtxoCursor{}, out)
+		require.Nil(t, out)
 	})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestVtxoCursorCodec(t *testing.T) {
t.Run("roundtrip", func(t *testing.T) {
in := vtxoCursor{CreatedAt: 1234567, Txid: "abc", VOut: 7, FilterHash: "deadbeef"}
s := encodeCursor(in)
out, err := decodeCursor(s)
require.NoError(t, err)
require.Equal(t, in, out)
})
t.Run("empty cursor decodes to zero value with no error", func(t *testing.T) {
out, err := decodeCursor("")
require.NoError(t, err)
require.Equal(t, vtxoCursor{}, out)
})
func TestVtxoCursorCodec(t *testing.T) {
t.Run("roundtrip", func(t *testing.T) {
in := vtxoCursor{CreatedAt: 1234567, Txid: "abc", VOut: 7, FilterHash: "deadbeef"}
s := encodeCursor(in)
out, err := decodeCursor(s)
require.NoError(t, err)
require.NotNil(t, out)
require.Equal(t, in, *out)
})
t.Run("empty cursor decodes to nil with no error", func(t *testing.T) {
out, err := decodeCursor("")
require.NoError(t, err)
require.Nil(t, out)
})
🤖 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 `@funding_opts_test.go` around lines 61 - 74, The tests in TestVtxoCursorCodec
compare a vtxoCursor value to the pointer returned by decodeCursor, causing
type-mismatch failures; update the "roundtrip" subtest to call decodeCursor and
dereference the returned *vtxoCursor (e.g., outPtr -> *outPtr) before comparing
to the expected vtxoCursor, and update the "empty cursor" subtest to assert
decodeCursor("") returns nil (or check out == nil) and then compare the zero
value by either ensuring out is nil or by dereferencing a non-nil pointer to
compare to vtxoCursor{}; touch the TestVtxoCursorCodec, encodeCursor, and
decodeCursor usages accordingly.

Comment thread store/sql/vtxo_store.go
Comment on lines +334 to +392
func (v *vtxoRepository) GetVtxos(
ctx context.Context, q types.GetVtxoFilter,
) ([]clienttypes.Vtxo, *types.Cursor, error) {
// We ask SQL for q.Limit+1 rows. If the database returns more than q.Limit rows,
// we know there's at least one more page, and the extra row's outpoint becomes the cursor
// for the next page. If it returns ≤ q.Limit, we've hit the end and Next stays nil.
params := queries.GetVtxosParams{
LimitPlusOne: int64(q.Limit) + 1,
}

// Translate the typed status filter into the string the SQL query expects.
// VtxoStatusAll leaves StatusFilter.Valid = false, which the WHERE clause
// interprets as "no status filter."
switch q.Status {
case types.VtxoStatusSpendable:
params.StatusFilter = sql.NullString{String: "spendable", Valid: true}
case types.VtxoStatusSpent:
params.StatusFilter = sql.NullString{String: "spent", Valid: true}
case types.VtxoStatusAll:
// No status filter at the SQL layer.
}

if q.AssetID != "" {
params.AssetID = sql.NullString{String: q.AssetID, Valid: true}
}

// Cursor position: SQL resumes from rows strictly less than
// (cursor_created_at, cursor_txid, cursor_vout) in the descending sort
// order. Nil After = first page; the cursor params stay invalid and the
// query's cursor predicate is a no-op.
if q.After != nil {
params.CursorCreatedAt = sql.NullInt64{Int64: q.After.CreatedAt, Valid: true}
params.CursorTxid = sql.NullString{String: q.After.Txid, Valid: true}
params.CursorVout = sql.NullInt64{Int64: int64(q.After.VOut), Valid: true}
}

rows, err := v.querier.GetVtxos(ctx, params)
if err != nil {
return nil, nil, err
}
vtxos := assetVtxoVwRowsToVtxos(rows)

// If we got back more than the caller asked for, the extra row is the
// "has more" sentinel: drop it and use its outpoint to build the cursor
// for the cursor page. Otherwise we hit the end of the dataset and Next
// stays nil, signaling no further pages.
var cursor *types.Cursor
if len(vtxos) > q.Limit {
vtxos = vtxos[:q.Limit]
last := vtxos[len(vtxos)-1]
cursor = &types.Cursor{
CreatedAt: last.CreatedAt.Unix(),
Txid: last.Txid,
VOut: last.VOut,
}
}

return vtxos, cursor, nil
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against q.Limit <= 0 to avoid an index-out-of-range panic.

GetVtxos is exported via the VtxoStore interface and reachable by callers that don't go through WithLimit (which enforces [1, 1000]). If q.Limit == 0:

  • LimitPlusOne is 1, SQL returns up to one row.
  • len(vtxos) > q.Limit (1 > 0) is true.
  • vtxos = vtxos[:0] → empty.
  • last := vtxos[len(vtxos)-1]vtxos[-1] → panic.

And q.Limit < 0 panics earlier on vtxos[:q.Limit]. Either reject the bad input or coerce to a sensible default.

🛡️ Suggested guard
 func (v *vtxoRepository) GetVtxos(
 	ctx context.Context, q types.GetVtxoFilter,
 ) ([]clienttypes.Vtxo, *types.Cursor, error) {
+	if q.Limit <= 0 {
+		return nil, nil, fmt.Errorf("limit must be > 0, got %d", q.Limit)
+	}
 	// We ask SQL for q.Limit+1 rows. If the database returns more than q.Limit rows,

Based on learnings (PR 118): swept-but-not-spent VTXOs are intentionally kept as spendable, so SelectSpendableOrRecoverableVtxos correctly omits a swept = false predicate — that part is fine.

🤖 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 334 - 392, GetVtxos currently panics
when q.Limit <= 0 or negative; add a guard at the start of GetVtxos that
validates/coerces q.Limit into a safe range (e.g. if q.Limit <= 0 set q.Limit =
1; optionally clamp above to 1000), then recompute params.LimitPlusOne from the
coerced q.Limit before calling v.querier.GetVtxos; this ensures the later slice
logic (vtxos[:q.Limit], last := vtxos[len(vtxos)-1]) and cursor creation use a
non-zero positive limit and avoid index-out-of-range panics.

Comment thread store/vtxo_store_test.go
Comment on lines +19 to +132
var (
testVtxoAsset1 = clientTypes.Asset{
AssetId: asset.AssetId{
Txid: [32]byte{
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0a,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
},
Index: 12,
}.String(),
Amount: 123456789,
}

testVtxoAsset2 = clientTypes.Asset{
AssetId: asset.AssetId{
Txid: [32]byte{
0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0a,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
},
Index: 0,
}.String(),
Amount: 987654321,
}

testVtxos = []clientTypes.Vtxo{
{
Outpoint: clientTypes.Outpoint{
Txid: "0000000000000000000000000000000000000000000000000000000000000000",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 1000,
CommitmentTxids: []string{
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
Preconfirmed: true,
},
{
Outpoint: clientTypes.Outpoint{
Txid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 2000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
},
{
Outpoint: clientTypes.Outpoint{
Txid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 3000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
// vtxo with multiple assets
Assets: []clientTypes.Asset{testVtxoAsset1, testVtxoAsset2},
},
{
Outpoint: clientTypes.Outpoint{
Txid: "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 3000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
// vtxo with single asset
Assets: []clientTypes.Asset{testVtxoAsset1},
},
}

testVtxoKeys = []clientTypes.Outpoint{
{
Txid: "0000000000000000000000000000000000000000000000000000000000000000",
VOut: 0,
},
{
Txid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
VOut: 0,
},
{
Txid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
VOut: 0,
},
{
Txid: "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
VOut: 0,
},
}

testSpendVtxoKeys = map[clientTypes.Outpoint]string{
testVtxoKeys[0]: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
}

testSettleVtxoKeys = map[clientTypes.Outpoint]string{
testVtxoKeys[1]: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
}

arkTxid = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Define missing settledBy constant.

The variable settledBy is referenced on lines 279 and 802 but never defined in the fixtures section. This will cause a compilation error.

🐛 Proposed fix

Add the missing constant in the fixtures section:

 	arkTxid = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
+	settledBy = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var (
testVtxoAsset1 = clientTypes.Asset{
AssetId: asset.AssetId{
Txid: [32]byte{
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0a,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
},
Index: 12,
}.String(),
Amount: 123456789,
}
testVtxoAsset2 = clientTypes.Asset{
AssetId: asset.AssetId{
Txid: [32]byte{
0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0a,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
},
Index: 0,
}.String(),
Amount: 987654321,
}
testVtxos = []clientTypes.Vtxo{
{
Outpoint: clientTypes.Outpoint{
Txid: "0000000000000000000000000000000000000000000000000000000000000000",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 1000,
CommitmentTxids: []string{
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
Preconfirmed: true,
},
{
Outpoint: clientTypes.Outpoint{
Txid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 2000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
},
{
Outpoint: clientTypes.Outpoint{
Txid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 3000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
// vtxo with multiple assets
Assets: []clientTypes.Asset{testVtxoAsset1, testVtxoAsset2},
},
{
Outpoint: clientTypes.Outpoint{
Txid: "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 3000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
// vtxo with single asset
Assets: []clientTypes.Asset{testVtxoAsset1},
},
}
testVtxoKeys = []clientTypes.Outpoint{
{
Txid: "0000000000000000000000000000000000000000000000000000000000000000",
VOut: 0,
},
{
Txid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
VOut: 0,
},
{
Txid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
VOut: 0,
},
{
Txid: "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
VOut: 0,
},
}
testSpendVtxoKeys = map[clientTypes.Outpoint]string{
testVtxoKeys[0]: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
}
testSettleVtxoKeys = map[clientTypes.Outpoint]string{
testVtxoKeys[1]: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
}
arkTxid = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
)
var (
testVtxoAsset1 = clientTypes.Asset{
AssetId: asset.AssetId{
Txid: [32]byte{
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0a,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
},
Index: 12,
}.String(),
Amount: 123456789,
}
testVtxoAsset2 = clientTypes.Asset{
AssetId: asset.AssetId{
Txid: [32]byte{
0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0a,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
},
Index: 0,
}.String(),
Amount: 987654321,
}
testVtxos = []clientTypes.Vtxo{
{
Outpoint: clientTypes.Outpoint{
Txid: "0000000000000000000000000000000000000000000000000000000000000000",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 1000,
CommitmentTxids: []string{
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
Preconfirmed: true,
},
{
Outpoint: clientTypes.Outpoint{
Txid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 2000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
},
{
Outpoint: clientTypes.Outpoint{
Txid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 3000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
// vtxo with multiple assets
Assets: []clientTypes.Asset{testVtxoAsset1, testVtxoAsset2},
},
{
Outpoint: clientTypes.Outpoint{
Txid: "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
VOut: 0,
},
Script: "0000000000000000000000000000000000000000000000000000000000000001",
Amount: 3000,
CommitmentTxids: []string{
"0000000000000000000000000000000000000000000000000000000000000000",
},
ExpiresAt: time.Unix(1748143068, 0),
CreatedAt: time.Unix(1746143068, 0),
// vtxo with single asset
Assets: []clientTypes.Asset{testVtxoAsset1},
},
}
testVtxoKeys = []clientTypes.Outpoint{
{
Txid: "0000000000000000000000000000000000000000000000000000000000000000",
VOut: 0,
},
{
Txid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
VOut: 0,
},
{
Txid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
VOut: 0,
},
{
Txid: "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
VOut: 0,
},
}
testSpendVtxoKeys = map[clientTypes.Outpoint]string{
testVtxoKeys[0]: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
}
testSettleVtxoKeys = map[clientTypes.Outpoint]string{
testVtxoKeys[1]: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
}
arkTxid = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
settledBy = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
)
🤖 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/vtxo_store_test.go` around lines 19 - 132, The tests reference a
missing fixture variable named settledBy causing compilation failures; add a new
string variable/constant called settledBy in the fixtures block (alongside
arkTxid/testVtxo* definitions) with the expected value used by the tests (e.g. a
txid-like hex string) and ensure its type matches usages (string) so references
to settledBy in the tests compile.

Comment on lines +50 to +52
page, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())
require.NoError(t, err, "❌ funding failed: expected no error on getting vtxos, got %w", err)
vtxos := page.Vtxos
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Type mismatch: incorrect unpacking of ListVtxos return values.

The ListVtxos method signature returns ([]Vtxo, string, error) (vtxos, cursor, error), but this code assigns the return to only two variables (page, err). Go does not allow assigning three return values to two variables—this should fail to compile.

Comparing with the other test files in this PR:

  • hd_wallet_test.go line 103: restoredSpendable, _, err := aliceClientHD.ListVtxos(ctx, sdk.WithSpendableOnly())
  • transaction_test.go line 54: bobVtxos, _, err := bob.ListVtxos(ctx, arksdk.WithSpendableOnly())

Both correctly unpack three values.

🐛 Proposed fix
-page, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())
+vtxos, _, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())
 require.NoError(t, err, "❌ funding failed: expected no error on getting vtxos, got %w", err)
-vtxos := page.Vtxos
 require.Len(
 	t, vtxos, expectedVtxos,
 	"❌ funding failed: got %d vtxos, expected %d", len(vtxos), expectedVtxos,
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
page, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())
require.NoError(t, err, "❌ funding failed: expected no error on getting vtxos, got %w", err)
vtxos := page.Vtxos
vtxos, _, err := alice.ListVtxos(ctx, arksdk.WithSpendableOnly())
require.NoError(t, err, "❌ funding failed: expected no error on getting vtxos, got %w", err)
🤖 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 `@test/e2e/restore_smoke_test.go` around lines 50 - 52, The test incorrectly
unpacks ListVtxos (which returns []Vtxo, string, error) into two variables;
change the call to capture three returns (e.g., restoredVtxos, _, err :=
alice.ListVtxos(ctx, arksdk.WithSpendableOnly())) and then use the first
returned slice (replace page.Vtxos uses with the new restoredVtxos variable) so
the values and error are handled correctly.

Comment on lines +95 to +97
pageRestored, err := aliceRestored.ListVtxos(ctx, arksdk.WithSpendableOnly())
require.NoError(t, err, "❌ restore failed: expected no error, got %w", err)
vtxosRestored := pageRestored.Vtxos
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Type mismatch: incorrect unpacking of ListVtxos return values.

Same issue as lines 50-52: the return signature is ([]Vtxo, string, error) but only two variables are used to unpack it.

🐛 Proposed fix
-pageRestored, err := aliceRestored.ListVtxos(ctx, arksdk.WithSpendableOnly())
+vtxosRestored, _, err := aliceRestored.ListVtxos(ctx, arksdk.WithSpendableOnly())
 require.NoError(t, err, "❌ restore failed: expected no error, got %w", err)
-vtxosRestored := pageRestored.Vtxos
 require.Len(
 	t, vtxosRestored, len(vtxos),
 	"❌ restore failed: got %d restored vtxos, expected %d",
 	len(vtxosRestored), len(vtxos),
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pageRestored, err := aliceRestored.ListVtxos(ctx, arksdk.WithSpendableOnly())
require.NoError(t, err, "❌ restore failed: expected no error, got %w", err)
vtxosRestored := pageRestored.Vtxos
vtxosRestored, _, err := aliceRestored.ListVtxos(ctx, arksdk.WithSpendableOnly())
require.NoError(t, err, "❌ restore failed: expected no error, got %w", err)
🤖 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 `@test/e2e/restore_smoke_test.go` around lines 95 - 97, The call to ListVtxos
in the restored path is unpacking the wrong number of return values—ListVtxos
returns ([]Vtxo, string, error). Update the call to capture three values (e.g.,
vtxosRestored, nextCursorRestored, err) instead of two, then use
require.NoError(t, err, ...) and set vtxosRestored := vtxosRestored (or use the
captured vtxos variable) and/or pass nextCursorRestored where appropriate so the
returned page token is not lost; locate the call to ListVtxos and the variables
pageRestored/vtxosRestored to make this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant