Skip to content

kvdb: implement the ApplyScanBatch scan engine#1256

Open
yyforyongyu wants to merge 9 commits into
prep-runtime-3from
impl-runtime-store
Open

kvdb: implement the ApplyScanBatch scan engine#1256
yyforyongyu wants to merge 9 commits into
prep-runtime-3from
impl-runtime-store

Conversation

@yyforyongyu

@yyforyongyu yyforyongyu commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Implements the ApplyScanBatch scan engine across kvdb and SQLite/PostgreSQL
behind the db.Store interface, with the scan / tx-batch unit tests + itests and
the RollbackToBlock sync-tip atomicity.

The runtime query/batch methods and scan scaffolding this builds on were split
out into prep-runtime-1 + prep-runtime-2 for reviewability. Stacked on
prep-runtime-2.

@coveralls

coveralls commented Jun 3, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 28448439256

Warning

No base build found for commit a0e618a on prep-runtime-3.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 55.49%

Details

  • Patch coverage: 156 uncovered changes across 6 files (330 of 486 lines covered, 67.9%).

Uncovered Changes

File Changed Covered %
wallet/internal/db/kvdb/txstore.go 191 123 64.4%
wallet/internal/db/pg/txstore_applyscanbatch.go 63 40 63.49%
wallet/internal/db/sqlite/txstore_applyscanbatch.go 61 40 65.57%
wallet/internal/db/pg/txstore_batch.go 73 54 73.97%
wallet/internal/db/sqlite/txstore_batch.go 73 54 73.97%
wallet/internal/bwtest/mock/store.go 6 0 0.0%
Total (7 files) 486 330 67.9%

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 58045
Covered Lines: 32209
Line Coverage: 55.49%
Coverage Strength: 13349.83 hits per line

💛 - Coveralls

@yyforyongyu yyforyongyu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

LGTM (deepseek-v4-pro)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Implements additional runtime and scan-batch Store surface area for the legacy kvdb backend by adapting operations onto existing waddrmgr and wtxmgr paths, along with adding the associated request/parameter types and tests.

Changes:

  • Add kvdb.Store.ListSyncedBlocks and a shared helper for converting db.Block to legacy waddrmgr.BlockStamp.
  • Add kvdb implementations for recovery/runtime UTXO helpers (ListOutputsToWatch, DeleteExpiredLeases).
  • Add batch-oriented tx write paths (ApplyTxBatch, ApplyScanBatch) plus RollbackToBlock, with new unit tests and new db parameter types.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
wallet/internal/db/kvdb/walletstore.go Adds synced-block listing and a block conversion helper for legacy sync/scan paths.
wallet/internal/db/kvdb/walletstore_test.go Adds unit test coverage for ListSyncedBlocks.
wallet/internal/db/kvdb/utxostore.go Implements outputs-to-watch listing and expired-lease deletion via legacy wtxmgr.
wallet/internal/db/kvdb/utxostore_test.go Adds tests for expired lease deletion and watch-list behavior with leased credits.
wallet/internal/db/kvdb/txstore.go Implements tx/scan batch application and rollback via legacy managers.
wallet/internal/db/kvdb/txstore_test.go Adds tests for tx batch, scan batch, and rollback behavior.
wallet/internal/db/data_types.go Introduces query/params types for synced-block listing and batch apply operations.

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

Comment thread wallet/internal/db/kvdb/walletstore.go Outdated
Comment thread wallet/internal/db/kvdb/walletstore.go Outdated
Comment thread wallet/internal/db/kvdb/txstore.go Outdated
@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

LGTM 🐳 (gemini-3.5-flash)

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

LGTM 🧱
(claude-opus-4-8[1m])

Comment thread wallet/internal/db/kvdb/txstore.go Outdated

@yyforyongyu yyforyongyu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

LGTM (deepseek-v4-pro)

@yyforyongyu yyforyongyu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

LGTM (deepseek-v4-pro)

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

LGTM 🪨
(claude-opus-4-8[1m])

Comment thread wallet/internal/db/kvdb/txstore.go Outdated
Comment thread wallet/internal/db/kvdb/txstore.go
@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

Branch hygiene: this PR was updated via merge this round rather than rebased — its range carries 2 Merge branch 'impl-psbt-store' into 'impl-runtime-store' commits. Prior rounds it was a clean linear rebase, and the project preserves commit history (no squash), so these merge commits would land on the target branch.

Please rebase onto the base to drop the Merge branch … commits and keep the stack linear. (See the 1258 thread — the same merge-based update also produced a duplicated stop precreating commit there.) The code changes themselves look correct.

(claude-opus-4-8[1m])

Comment thread wallet/internal/db/txstore_common.go
Comment thread wallet/internal/db/kvdb/txstore.go
Comment thread wallet/internal/db/kvdb/txstore.go
Comment thread wallet/internal/db/txstore_common.go
@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by gpt-5.5 🦉

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by claude-opus-4-8[1m] ⚙️

Comment thread wallet/internal/db/kvdb/txstore.go Outdated
Comment thread wallet/internal/db/sqlite/txstore_applyscanbatch.go Outdated
Comment thread wallet/internal/db/pg/txstore_applyscanbatch.go Outdated
Comment thread wallet/internal/db/kvdb/txstore.go Outdated
Comment thread wallet/internal/db/kvdb/txstore.go Outdated
Comment thread wallet/internal/db/kvdb/txstore.go
Comment thread wallet/internal/db/kvdb/txstore.go
@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by claude-opus-4-8[1m]

Comment thread wallet/internal/db/kvdb/txstore.go
Comment thread wallet/internal/db/kvdb/txstore.go
@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by gpt-5.5 🧩

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by claude-opus-4-8[1m]

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by gpt-5.5 xhigh

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by claude-opus-4-8[1m] 🧱

@yyforyongyu

Copy link
Copy Markdown
Collaborator Author

No issues found by gpt-5.5 🐙

Add the kvdb scan-horizon machinery: classifyLegacyHorizonBranch and
resolveLegacyHorizonAccount validate a horizon and resolve its real
account by scope-unique name, applyLegacyScanHorizons extends each branch
through the legacy ExtendAddresses path and records a scanHorizonRollback
range, and applyLegacySyncedBlocks connects discovered blocks.
Add ApplyScanBatch, extending horizons before recording the tx batch so
freshly derived credit addresses are persisted first, then connecting
discovered blocks, all in one walletdb write. On failure
rollbackScanBatchCaches evicts the derived addresses, invalidates the
account caches, and restores the synced tip so no rolled-back in-memory
advance stays observable.
Add CanSkipCreateTxDuplicate, reporting whether a batch insert may treat
an ErrTxAlreadyExists result as an idempotent no-op: only an exact
current-shape duplicate (same live status and block) is skippable, while
terminal history rows must not be skipped.
Refactor the sqlite ApplyTxBatch tx loop into applyBatchTransaction and
canSkipBatchDuplicate so the upcoming ApplyScanBatch shares one
duplicate-tolerant tx writer, treating an exact-shape duplicate as an
idempotent retry via CanSkipCreateTxDuplicate.
Refactor the postgres ApplyTxBatch tx loop into applyBatchTransaction
and canSkipBatchDuplicate, mirroring the sqlite shared duplicate-tolerant
tx writer used by ApplyScanBatch.
Add the SQLite ApplyScanBatch, extending every reported horizon, then
connecting discovered synced blocks before recording transactions (so a
tx confirmed in a discovered block finds its block row), all in one
write transaction via applyBatchSyncedBlocks and applyBatchTransaction.
Add the postgres ApplyScanBatch, mirroring the sqlite ordering: extend
horizons, connect discovered synced blocks before recording
transactions, all in one write transaction via applyBatchSyncedBlocks
and applyBatchTransaction.
Expose ApplyScanBatch on the TxStore interface now that the kvdb and SQL
backends implement it, and wire the mock store through to its testify
expectations.
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.

4 participants