wallet/sql: implement manager transaction store - #1293
Draft
Roasbeef wants to merge 4 commits into
Draft
Conversation
Roasbeef
force-pushed
the
sql-port-stage2-store-contract
branch
from
July 14, 2026 19:43
49c7015 to
77a2148
Compare
Roasbeef
marked this pull request as draft
July 14, 2026 20:00
5 tasks
In this commit, we separate the birthday block verification bit from the optional birthday block itself. The legacy address manager keeps the bit when the block is deleted, so the SQL schema needs to retain the same state instead of clearing it as a side effect. Both backends carry the migration in both directions. The down migration clears only the states that the old constraint cannot represent. Extracted-from: PR #1125 (d9bd945) Extracted-from: PR #1134 (e773de5) Co-authored-by: Mohamed Awnallah <mohamedmohey2352@gmail.com>
In this commit, we add the narrow SQL query surface needed by the address and transaction manager boundary. The address side can read or replace one block stamp, restore the start block for SetSyncedTo(nil), and update only the wallet sync tip. The transaction side lists mined incidences in rollback order, detaches non-coinbase rows, clears mined spend edges, and finds unmined descendants of a disconnected coinbase. The generated SQLite and PostgreSQL bindings stay in the same commit as their source queries. Extracted-from: PR #1125 (21a8f2f) Extracted-from: PR #1125 (ad53b24) Extracted-from: PR #1125 (70e7262) Extracted-from: PR #1134 (e773de5) Co-authored-by: Mohamed Awnallah <mohamedmohey2352@gmail.com>
In this commit, we implement the #1294 manager boundary for SQLite and PostgreSQL on top of the released lnd/sqldb v1.0.13 transaction executor. Each callback receives address and transaction views bound to the same SQL transaction, while context, database/sql handles, and generated query types remain behind the backend adapters. The address view preserves BlockHash and SetSyncedTo behavior, including the nil reset to the wallet start block. The transaction view moves disconnected non-coinbase incidences back to the unmined set, removes coinbase rows and their unmined descendants, clears mined spend edges, and keeps the active credit on the surviving incidence when duplicate history exists. Extracted-from: PR #1125 (d9bd945) Extracted-from: PR #1125 (8070803) Extracted-from: PR #1125 (70e7262) Co-authored-by: yyforyongyu <yong2452@gmail.com> Co-authored-by: Mohamed Awnallah <mohamedmohey2352@gmail.com>
In this commit, we run one manager-store conformance suite against SQLite and PostgreSQL. The suite covers callback reset and cancellation, missing block errors, block replacement, SetSyncedTo(nil), and the independent birthday verification state. The rollback cases exercise the state transitions that are easy to lose in a mechanical port: address and transaction rewinds commit or abort together, non-coinbase incidences return to the unmined set, mined spend edges are cleared, coinbase descendants are removed recursively, and duplicate incidences leave the active credit attached to the surviving row. Extracted-from: PR #1125 (e9223af) Extracted-from: PR #1125 (202ac9c) Extracted-from: PR #1262 (21a8f2f) Co-authored-by: yyforyongyu <yong2452@gmail.com>
Roasbeef
force-pushed
the
sql-port-stage2-store-contract
branch
from
July 14, 2026 20:53
77a2148 to
e0622ab
Compare
Roasbeef
changed the base branch from
sql-port-stage1-transactions
to
sql-port-stage2-manager-boundary
July 14, 2026 20:54
This was referenced Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this PR, we implement the SQL side of the manager transaction boundary introduced in #1294. This replaces the earlier generic wallet metadata contract with the actual
waddrmgrandwtxmgroperations the wallet already uses for a block disconnect:BlockHash,SetSyncedTo, andRollback.Both SQLite and PostgreSQL bind the address and transaction views to one
lnd/sqldbv1.0.13 transaction. The wallet keeps its existing callback-oriented control flow, whiledatabase/sql, generated sqlc types, and backend integer widths remain behind the SQL adapters. The KV path stays in #1294 as a thin adapter over the existing managers, so we don't gain a second KV implementation here.Manager Semantics
The address view preserves the existing missing-block error and
SetSyncedTo(nil)reset to the wallet start block. A replacement block stamp updates the shared block row and the wallet sync tip inside the same transaction.The transaction view ports the existing rollback behavior onto the Stage 1 transaction schema. Non-coinbase incidences at the rollback height and above return to the unmined set, mined spend edges are cleared, disconnected coinbase rows are removed, and unmined descendants that spend those coinbase outputs are removed recursively. If the same transaction has multiple mined incidences, the active credit moves to the incidence that survives as unmined.
The address rewind and transaction rollback commit or abort together. This is the cross-domain atomicity the legacy wallet gets from one
walletdb.Updateclosure, and it's the main reason the SQL backend sits behind the transaction boundary instead of opening one transaction per manager method.Birthday State
The final Stage 1 wallet schema coupled
birthday_block_verifiedto the presence ofbirthday_block_height, but the legacy address manager retains the verification bit when the birthday block is deleted. The migration in this PR separates those fields for SQLite and PostgreSQL, including the reverse migration back to the older constraint.Stack
This draft is stacked on #1294, which is itself stacked on #1290. It does not route a production wallet through SQL yet. Wallet construction, KV-to-SQL migration, and the full integration test remain later slices after the manager surface has been moved behind the same boundary.
Provenance
This work is salvaged from the long-running SQL series. The four commits retain the original authors and record the source PRs and full commits in their trailers, including work from #1125, #1134, and #1262.
See each commit message for the detailed lineage w.r.t the migration, query layer, SQL manager implementation, and shared conformance suite.
Verification
go test ./wallet ./wallet/internal/db/... ./wallet/internal/sql/...go test -tags test_db_postgres ./wallet/internal/db/itest -run TestPostgresManagerStore -count=1golangci-lintover./wallet/internal/db/...and./wallet/internal/sql/...make sqlcleaves the worktree unchanged