sqlite: batch GetTxsWithTxids to stay under SQLITE_MAX_VARIABLE_NUMBER

[Tyagiquamar]

Problem

GetVirtualTxs fails on the sqlite backend when called with a large txid list: roundRepository.GetTxsWithTxids passes the entire slice to a single SelectTxs query, which expands it into three IN clauses (tx, offchain_tx, checkpoint_tx). With N txids the statement binds 3N variables, so anything beyond ~10,922 txids exceeds SQLITE_MAX_VARIABLE_NUMBER (32766 in modernc.org/sqlite) and the request dies with:

SQL logic error: too many SQL variables (1)

The gRPC response-size concern from the issue is already mitigated by result pagination (paginate + client-side default paging from #1011); the unbounded request-side SQL variable count is not.

Refs #1115.

Root cause

[internal/infrastructure/db/sqlite/round_repo.go]: GetTxsWithTxids → one SelectTxs call with Ids1/Ids2/Ids3 = txids, unbounded.

The postgres backend is unaffected (its SelectTxs binds a single pq.Array parameter) and is intentionally untouched.

Fix

Mirror the pattern already established in this package by getVtxoPubKeysByCommitmentTxidsBatched:

• New sqliteSelectTxsBatchSize = 5000 const (each query binds at most 3×5000 = 15000 params, generous headroom under 32766).
• GetTxsWithTxids delegates to an unexported getTxsWithTxidsBatched inner that chunks the txid slice, issues one query per chunk, and merges results in Go.
• Dedup keys on (txid, data) so the merged result preserves the single-query UNION semantics even when the input contains duplicated txids spanning two batches; first-seen order is preserved (no map-iteration randomness leaking into pagination).

For inputs up to 5000 txids the loop iterates once and behaviour is identical to today.

Regression test

New round_repo_batching_test.go:

• TestGetTxsWithTxidsBatched — drives the inner helper across all three tx tables with batch sizes 1, 2, 3, len-1, len, len+1, 0; asserts exact union, no duplicates, duplicated-input-txid dedup across batch boundaries, unknown txids ignored.
• TestGetTxsWithTxidsOverVariableLimit — seeds 12,000 rows and calls the public method. On current master this fails with too many SQL variables; with this change it returns all 12,000 rows.

Tests use per-test file-backed DBs (t.TempDir()) rather than the shared file::memory: name, and close the full service handle, so they neither pollute nor get polluted by other package tests.

Verification

• go test ./internal/infrastructure/db/sqlite/ -count=1 — full package green, incl. both new tests
• Same over-limit test against unmodified master — fails with too many SQL variables
• go vet ./internal/infrastructure/db/... — clean; gofmt clean on touched files
• go build ./... — clean
• internal/infrastructure/db/postgres tests require a live postgres and were not run locally; that backend is unchanged

Scope

Only the sqlite GetTxsWithTxids path. GetRoundsWithCommitmentTxids (single IN clause) and other slice-binding queries are unaffected by this issue's failure mode at realistic sizes and are left as-is; happy to follow up if you want them batched too.

Summary by CodeRabbit

• Bug Fixes

  • Improved transaction lookup reliability for large sets of transaction IDs by processing requests in manageable batches.
  • Preserved complete results while avoiding duplicate transaction data across batches.
  • Continued to ignore unknown transaction IDs without failing the overall lookup.

• Tests

  • Added coverage for varied batch sizes, duplicate IDs, unknown IDs, and large requests exceeding SQLite parameter limits.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 03931a54-5a6a-4a8a-adc4-f871d20a9060

📥 Commits

Reviewing files that changed from the base of the PR and between f863e48 and fcf4a35.

📒 Files selected for processing (3)

• internal/infrastructure/db/sqlite/export_test.go
• internal/infrastructure/db/sqlite/round_repo.go
• internal/infrastructure/db/sqlite/round_repo_batching_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

Walkthrough

Changes

SQLite transaction lookup batching

Layer / File(s)	Summary
Implement batched transaction lookup internal/infrastructure/db/sqlite/round_repo.go	GetTxsWithTxids now queries transaction IDs in batches of 5,000. The helper skips empty batches, ignores sql.ErrNoRows, and deduplicates results by transaction ID and data.
Validate batching across query sizes internal/infrastructure/db/sqlite/export_test.go, internal/infrastructure/db/sqlite/round_repo_batching_test.go	Tests cover multiple batch sizes, duplicate and unknown transaction IDs, all transaction tables, and 12,000 IDs that exceed SQLite's variable limit.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to fcf4a

This change fixes large SQLite lookups, but public transaction requests can still trigger unbounded database and memory work before response pagination, creating resource-exhaustion pressure. A request-size or transaction-count limit, suitable rate control, or explicit owner acceptance is needed before merge.

Suggested reviewers: bitcoin-coder-bob

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant roundRepository
  participant SelectTxs
  Caller->>roundRepository: GetTxsWithTxids(txids)
  roundRepository->>SelectTxs: Query each txid batch
  SelectTxs-->>roundRepository: Transaction rows or sql.ErrNoRows
  roundRepository-->>Caller: Deduplicated transaction data

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files.	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 clearly and concisely describes the main change: batching SQLite GetTxsWithTxids queries to remain below SQLITE_MAX_VARIABLE_NUMBER.
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.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[arkana-ai-bot]

PROTOCOL-CRITICAL: human review required.

This PR touches GetTxsWithTxids, which feeds the GetVirtualTxs path, the sweeper, and fraud detection. The batching logic appears semantically equivalent to the original for the common case, but all three callers on protocol-critical paths deserve a human sign-off.

---

Summary

The core fix is correct: chunking the txid slice into batches of 5000 keeps the per-statement parameter count at ≤ 15 000, well within SQLITE_MAX_VARIABLE_NUMBER = 32766. The pattern mirrors the existing getVtxoPubKeysByCommitmentTxidsBatched approach and is well-tested. Findings below are ordered by severity.

---

Findings

1. Per-batch withReadQuerier calls — snapshot drift between batches (low, noted for protocol reviewers)

internal/infrastructure/db/sqlite/round_repo.go:557

Each batch issues a separate withReadQuerier call (no wrapping transaction). For a 12 000-txid call this means N/5000 = 3 separate read statements. If a row is inserted or deleted between batch 1 and batch 2 the result set will be inconsistent across batches. In practice, tx/offchain_tx/checkpoint_tx rows are append-only once a round is finalised, so this is not an immediate correctness concern, but it is a departure from the single-snapshot semantics of the original single-query implementation. Worth a comment or a note in the function doc.

2. Dedup key uses a null-byte separator (negligible, note for future-proofing)

internal/infrastructure/db/sqlite/round_repo.go:605

key := row.Txid + "\x00" + row.Data

Txids are lowercase hex (no \x00); data in the tx/offchain_tx/checkpoint_tx tables appears to be a text-encoded transaction (hex or base64). There is no known case where the separator produces a collision today. A brief comment confirming that assumption (e.g. "txids are hex — no null bytes") would make future maintainers' lives easier if the schema ever takes binary-blob data.

3. seen map allocated without size hint (micro-performance, nitpick)

internal/infrastructure/db/sqlite/round_repo.go:571

seen := make(map[string]struct{})

The upper bound on unique results is len(txids). Pre-allocating make(map[string]struct{}, len(txids)) avoids multiple rehashes for the large-N case that this PR targets. Minor, but consistent with how the caller pre-sizes resp.

4. Nil vs empty slice for the all-miss case (very minor behavioral delta)

internal/infrastructure/db/sqlite/round_repo.go

The original code returned (nil, nil) when SelectTxs returned sql.ErrNoRows. The new code returns ([]string{}, nil) (a non-nil empty slice) when every batch produces zero rows. All known callers (indexer.go:963, fraud.go:137, sweeper.go:137, service.go:3729) check len(result) rather than result == nil, so this has no practical impact. Still worth logging as a deliberate change.

5. Result ordering for N > 5000 (informational — SDK already handles this)

internal/infrastructure/db/sqlite/round_repo.go

The original UNION issued one consistent unordered result set. The batched implementation preserves first-seen insertion order within each batch but may interleave results across batches differently from a single-query UNION. The dotnet SDK (NArk.Core/Services/VirtualTxService.cs:97) explicitly documents that GetVirtualTxs does not return hexes in request order and correctly keys by the txid parsed from each hex rather than by position, so this is not a correctness issue for any known consumer. Noted for completeness.

---

What looks good

• Batch size math: 5000 txids × 3 IN clauses = 15 000 parameters; limit is 32 766 — comfortable headroom and consistent with the PR description.
• Empty input guard (len(txids) == 0 → nil, nil): avoids a round-trip that the original code paid for (the generated SelectTxs substitutes NULL for empty slices, which scans nothing).
• batchSize <= 0 fallback: correctly degrades to a single batch, matching pre-patch behaviour for inputs ≤ 5000.
• SQL UNION semantics preserved: cross-batch dedup on (txid, data) matches exactly what UNION (not UNION ALL) would do in the single-query case.
• Test coverage: TestGetTxsWithTxidsBatched covers batch sizes 0, 1, 2, 3, N-1, N, N+1, plus cross-batch duplicate suppression and unknown-txid tolerance. TestGetTxsWithTxidsOverVariableLimit directly reproduces the failure on unmodified master. Both use isolated file-backed DBs via t.TempDir(), which is the right pattern for this package.
• Export shim in export_test.go follows the established package convention.
• Postgres and badger are untouched — correct scope.

[arkana-ai-bot]

Arkana review — 2026-09-02

Correctness fix for an easy-to-miss SQLite limit. Looks good.

Batching logic
getTxsWithTxidsBatched correctly splits the input slice into chunks and issues one query per chunk. The batchSize=5000 gives 15000 bound params per call (3 IN clauses × 5000), which is well within SQLITE_MAX_VARIABLE_NUMBER=32766. The comment in the const explains the three-field constraint clearly.

Deduplication
Using txid + "\x00" + data as the seen-map key is correct: it preserves the same semantics as a single-query UNION while still merging across batches. The null byte separator prevents a txid that is a prefix of another from causing false matches.

Test export / testability
The export_test.go approach (exposing the unexported batching function to test code only, via a file that is only compiled for tests) is idiomatic Go. No production binary impact.

One nit: batchSize <= 0 is handled as "no batching" (the whole slice as one call). This could panic or hit the limit on a very large input. Since the only public entry point passes the compile-time constant, this is low risk, but a comment explaining the intent (or clamping batchSize to the constant) would be helpful.

Overall: looks correct and ready to merge.

[arkana-ai-bot]

Arkana review — #1177 (sha fcf4a35)

Verdict: looks ready to merge — transparent SQLite-layer fix for SQLITE_MAX_VARIABLE_NUMBER.

What this does: Moves GetTxsWithTxids to chunk input at 5000 txids. With 3 IN clauses per call that is at most 15,000 bound params — well under SQLITE_MAX_VARIABLE_NUMBER=32766. Dedup is keyed on txid+data to match UNION semantics across batches.

Good:

• getTxsWithTxidsBatched is the testable inner exported via export_test.go — clean pattern.
• batchSize <= 0 fallback preserves pre-batching behaviour.
• TestGetTxsWithTxidsBatched covers off-by-one at sizes 1, 2, 3, len-1, len, len+1, 0 plus cross-batch dedup and unknown txids.
• The doc comment explaining the three Ids* fields receiving the same slice (sqlc generator limitation) is helpful.

Minor note: The seen key uses txid+null+data. If txid already uniquely maps to one data blob, txid alone suffices for dedup — non-blocking, but worth confirming the UNION source is injective.

Pairs well with #1178 (application-layer 250-chunk limit). Both can merge independently.

[arkana-ai-bot]

Arkana review — sqlite: batch GetTxsWithTxids to stay under SQLITE_MAX_VARIABLE_NUMBER

SHA reviewed: fcf4a35

Fixes a real hard crash: SelectTxs expands the txid slice into three IN clauses (tx, offchain_tx, checkpoint_tx), binding 3N parameters per call. SQLite caps at SQLITE_MAX_VARIABLE_NUMBER = 32766, so any call with >10,922 txids would fail.

Implementation

• sqliteSelectTxsBatchSize = 5000 → 15,000 params per call, leaving generous headroom.
• getTxsWithTxidsBatched merges results in Go with a seen map keyed on txid + "\x00" + data to preserve cross-batch dedup semantics equivalent to the single-query UNION.
• batchSize <= 0 falls through to no-batching mode (full slice in one call) — useful for tests and if the caller knows the input is small.
• The public GetTxsWithTxids delegates to the batching helper. The interface is unchanged.

Tests

• TestGetTxsWithTxidsBatched: drives the inner helper with batch sizes 1, 2, 3, len-1, len, len+1, and 0. Covers off-by-one at boundaries, cross-batch dedup, and unknown txids being silently dropped. Good.
• TestGetTxsWithTxidsOverVariableLimit: inserts 12,000 rows and calls the public method — this is the regression test for the actual failure mode (36,000 params over the limit without batching).

Relationship to #1178
This PR fixes the SQLite parameter limit at the infrastructure layer. PR #1178 adds a separate application-layer chunking in indexerService.getVirtualTxs with maxQueryChunkSize = 250. The two fixes address the same root cause at different levels. If #1177 is merged, #1178 becomes redundant for the SQLite limit issue (though it might be kept for other reasons like bounded query latency). Both PRs should be aware of each other.

Verdict: looks ready to merge. No security concerns.

[arkana-ai-bot]

Review: looks ready to merge — SQLite batch chunking for GetTxsWithTxids (avoids SQLITE_MAX_VARIABLE_NUMBER crash)

Problem fixed: GetTxsWithTxids expands the input slice into three IN-clause placeholders, binding 3N parameters per call. With large vtxo trees that easily exceeds SQLite's 32766-parameter hard cap, causing a runtime crash.

Fix is correct: getTxsWithTxidsBatched slices the input into chunks of 5000 (→ 15000 bound params max, well under the limit), runs one query per chunk, and merges deduplicated results using a txid+data composite key. The composite key is the right choice — it handles the case where the same txid could appear with different data across tables, and preserves UNION semantics without relying on query-level DISTINCT.

Batch size choice: 5000 is well-reasoned (the comment explains the 3N multiplier). It would be good to have this constant surfaced in a shared place if other queries in this file need the same bound, but that's a cleanup, not a blocker.

Tests: TestGetTxsWithTxidsBatched exercises batch sizes 1, 2, 3, len-1, len, len+1, and 0 against all three table types, including cross-batch duplicate input — solid coverage of the edge cases that matter for chunking logic.

Note on #1178 (CHANGES_REQUESTED): that sibling PR does the same thing for the indexer path. This PR fixes the round-repo path. Check that both paths are consistent in their batch-size constant and dedup logic before merging.

[arkana-ai-bot]

Arkana review — sha fcf4a35

SQLite parameter limit fix. Looks ready to merge.

What it does:
SQLite caps bound parameters at SQLITE_MAX_VARIABLE_NUMBER = 32766. SelectTxs expands a txid slice into three IN clauses (tx, offchain_tx, checkpoint_tx), so a single call binds 3N params. With large rounds this can exceed the limit and return a DB error. The fix batches the input at 5000 txids per call (15000 params, well within the cap) and deduplicates results across batches in Go.

Notes:

• The dedup key is txid + "\x00" + data — covers the case where the same txid appears in multiple tables. Correct.
• batchSize <= 0 falls through to no-batching (single call with the full slice) — consistent with the test covering batchSize=0.
• Tests drive batchSizes of 1, 2, 3, len-1, len, len+1, and 0, including a duplicated txid that crosses a batch boundary. Good coverage.
• The export test helper correctly avoids touching production binaries (_test.go suffix).

[arkana-ai-bot]

Arkana automated review — sha fcf4a35139

Straightforward correctness fix. GetTxsWithTxids previously issued a single query that bound the txid slice three times (once per table: tx, offchain_tx, checkpoint_tx), which would exceed SQLite's SQLITE_MAX_VARIABLE_NUMBER = 32766 for large inputs (the threshold is ~10922 txids). This PR splits the input into batches of 5000, keeping the bind count at 15000 per call with generous headroom.

The deduplication key of is correct — it preserves the single-query UNION semantics (same txid appearing in two batches produces one result, same txid in the same table only returns one row anyway due to primary key).

Tests (): drives the helper with batch sizes 1, 2, 3, , , , and 0 (no-batching fallback). Tests cross-batch deduplication with a duplicated txid and ignores unknown txids. The export helper in is the right pattern for white-box testing.

This looks like a companion to #1178 (reviewed 2026-09-05, same pattern). Looks ready to merge.

[arkana-ai-bot]

Arkana automated review — sha fcf4a35139

Straightforward correctness fix. GetTxsWithTxids previously issued a single query binding the txid slice three times (once per table: tx, offchain_tx, checkpoint_tx), which would exceed SQLite's SQLITE_MAX_VARIABLE_NUMBER = 32766 for inputs beyond ~10922 txids. This PR splits the input into batches of 5000, keeping the bind count at 15000 per call with generous headroom.

The deduplication key of txid + "\x00" + data is correct — it preserves single-query UNION semantics across batches (same txid in two batches produces one result), and the null-byte separator prevents accidental collisions between txid/data concatenations.

Tests (TestGetTxsWithTxidsBatched): drives the helper with batch sizes 1, 2, 3, len-1, len, len+1, and 0 (no-batching fallback), tests cross-batch deduplication with a duplicated txid, and verifies unknown txids are ignored. The export helper in export_test.go is the right pattern for white-box testing of unexported methods.

This is a companion to #1178 (reviewed 2026-09-05, same batching pattern). Looks ready to merge.