wallet: add manager transaction boundary - #1294
Draft
Roasbeef wants to merge 3 commits into
Draft
Conversation
In this commit, we introduce the narrow transaction contracts used to move the address and transaction managers away from walletdb buckets. The first slice only covers the chain reorg path: block-hash lookup, sync-tip updates, and transaction rollback. The wallet still owns the outer View or Update callback, so both managers can participate in one atomic transaction. Backend handles stay behind the adapter, and the interfaces can grow alongside the SQL port instead of requiring either backend to implement the full wallet surface up front. Extracted-from: f94ec68 Extracted-from: c8c1ecd
In this commit, we bind the existing waddrmgr.Manager and wtxmgr.Store to the new transaction contracts. The adapter captures the two legacy buckets once, then each domain method forwards directly to the existing implementation. Unlike the earlier kvdb Store, these methods do not open their own transactions or reproduce manager logic. The tests show that address sync and transaction rollback writes commit together, roll back together, and retain the callback reset behavior needed by the SQL executor. Extracted-from: 418c8efd5300569f1b40fd047f6f6112971111e4 Extracted-from: 1cad0d8
In this commit, we move the block-disconnect path onto the new manager transaction boundary. The callback still checks the same block hashes, rewinds the address-manager sync tip, and rolls back wtxmgr at the same height. The wallet now constructs the legacy adapter when it opens, while the body of the reorg closure no longer touches walletdb buckets. This gives us one real cross-manager path that an SQL transaction can implement next without changing the surrounding notification control flow.
Roasbeef
pushed a commit
that referenced
this pull request
Jul 14, 2026
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>
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
2 participants
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 add the first backend-neutral transaction boundary in front of
waddrmgrandwtxmgr. The cut is deliberately narrow: it covers the block-disconnect path first, where the wallet needs to read + update address-manager chain state and roll back the transaction store in one atomic transaction.The wallet still owns the outer
View/Updatecallback. For the legacy backend, the new adapter captures the existingwalletdbbuckets and forwards each call directly to*waddrmgr.Manageror*wtxmgr.Store. It doesn't reproduce either manager's logic, and it doesn't open a transaction per method.This is the boundary PR that needs to precede #1293. The SQL store in that draft can then be reshaped to implement the same transaction-scoped interfaces, instead of introducing a second generic store contract and a second KVDB implementation.
Transaction shape
db.Storeexposes read-only and read/write callbacks. The transaction passed to a write callback providesAddr()andTx()domain views, so both managers share the same underlying Bolt or SQL transaction. Neither the wallet callback nor the public manager-facing contract seeswalletdb.ReadBucket,walletdb.ReadWriteBucket,database/sql, or generated sqlc types.We start with
BlockHash,SetSyncedTo, andRollback. The interfaces can grow with each ported behavior, which keeps the next SQL PR reviewable and avoids requiring a giant address + transaction interface before any path can run end-to-end.Verification
The adapter tests exercise commit, rollback, reset, read-only access, and canceled contexts. In particular, the rollback vector writes through both manager adapters, returns an error from the outer callback, and verifies that neither namespace commits.
The following focused checks pass:
The full
go test ./...run reached the affected packages successfully. Its only failures were the twochain/TestBitcoindEventsvariants after their ephemeral local bitcoind RPC endpoints refused connections.Stack
This PR is based on #1290. PR #1293 stays draft while we restack its SQL metadata implementation on top of this boundary.
See each commit message for a detailed description w.r.t the incremental changes.