wallet: load scan data from the Store and make the syncer Store-mandatory#1271
Open
yyforyongyu wants to merge 8 commits into
Open
wallet: load scan data from the Store and make the syncer Store-mandatory#1271yyforyongyu wants to merge 8 commits into
yyforyongyu wants to merge 8 commits into
Conversation
Coverage Report for CI Build 28448458732Warning No base build found for commit Coverage: 56.386%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
6fd8846 to
6e49f06
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
449c243 to
e919fcb
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
6e49f06 to
ee88d41
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
e919fcb to
e8d5e1a
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
ee88d41 to
35428a1
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
e8d5e1a to
b7b60d7
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
35428a1 to
cb49f87
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
b7b60d7 to
a354b6c
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
cb49f87 to
a46bd24
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
2 times, most recently
from
0908fc0 to
91960b1
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
a46bd24 to
271e6c5
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
91960b1 to
edba65b
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
2 times, most recently
from
6e1c859 to
ba97a4a
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
2 times, most recently
from
a9ae4b2 to
7475577
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
ba97a4a to
edba198
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
7475577 to
66055e4
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
2 times, most recently
from
3108912 to
d1e0364
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
2 times, most recently
from
c0da057 to
f6a0187
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-4
branch
from
d1e0364 to
b822c2a
Compare
yyforyongyu
force-pushed
the
impl-runtime-store-5
branch
from
f6a0187 to
6650cba
Compare
yyforyongyu
commented
Jun 22, 2026
Collaborator
Author
|
No issues found by |
yyforyongyu
commented
Jun 22, 2026
yyforyongyu
commented
Jun 22, 2026
Collaborator
Author
|
No issues found by gpt-5.5 🦉 |
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by gpt-5.5 🐬 |
Collaborator
Author
|
No issues found by |
yyforyongyu
commented
Jun 23, 2026
yyforyongyu
commented
Jun 23, 2026
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by gpt-5.5 🦭 |
yyforyongyu
commented
Jun 24, 2026
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by |
1 similar comment
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by gpt-5.5 🐧 |
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by gpt-5.5 🦑 |
Collaborator
Author
|
No issues found by |
Collaborator
Author
|
No issues found by gpt-5.5 🐚 |
The Store-backed targeted rescan resolved each AccountScope by number in storeScanHorizons. Both Store backends mask an imported account's number to 0, so an imported-xpub target was indistinguishable from the default derived account 0 and could seed the wrong horizon. Resolve the public targets into identity-aware scanTargets up front through the legacy manager, which still keys accounts by their non-masked number. The keyless imported-address bucket is skipped before any Store lookup; every other target carries its durable AccountName, and the Store horizon load prefers the name, mirroring the ScanHorizon contract and the SQL GetHorizonAccount behavior. A number lookup is never issued for an imported target. The legacy walletdb path is unchanged: it still resolves by number, which it does not mask.
Add real-kvdb-backed coverage for the targeted rescan identity fix: a syncer wired over a real, unlocked waddrmgr-backed Store. The keyless imported-address bucket produces no horizon and issues no number lookup (a real by-number lookup would surface ErrAccountNotFound). A setup with the default derived account 0 and an imported-xpub account masked to number 0 resolves the imported target by its durable name, classifies it as imported, and withholds it, so it is never mis-resolved as the default derived account.
The syncer runtime helpers were routed through the transitional Store but kept an "if s.store == nil" branch that fell back to the legacy walletdb DB* helpers, plus two positive "if s.store != nil" scan-data conditionals that did the same. Production newSyncer is always constructed with the wallet's Store, so nil Store is impossible on these migrated paths and the fallback is dead, contradictory design: a migrated runtime path must always use the Store. Remove the nil-store fallbacks from rewindToBlock, syncedBlockHashes, unminedTxns, updateSyncTip, putTxNotifications, putBlockNotifications, putSyncBatch, putTargetedBatch, syncedTip, loadTargetedScanData, and loadWalletScanData so each helper always uses the Store, and rewrite the "store when configured, falling back to the legacy walletdb path" comments to describe the Store-only behavior. The legacy DB* helpers in db_ops.go are left in place for now; they are removed once no non-deferred call site needs them.
newSyncer kept the Store optional behind a variadic syncerStoreConfig, but the migrated runtime paths now always read and write through the Store, so a nil Store is impossible. Drop the variadic and take store and walletID as mandatory positional parameters. Update every newSyncer call site mechanically to pass the Store: production wiring in manager.go, plus the benchmark, db_ops, and syncer test setups. The store-backed test bodies are rewritten in the following commits.
Rewrite the rollback, initialization, run-loop, run-step, wait, and init-chain-sync tests to drive the syncer through the Store instead of the legacy addrStore/walletdb setup: replace setupTestDB and mockAddrStore wiring with walletmock.Store expectations, and add the expectSyncedTip helper that stubs the synced tip via Store.GetWallet. Drop TestLoadScanDataLegacyFallback and TestRewindToBlockFallsBackToLegacy, which covered the now-removed nil-store fallbacks.
Rewrite the scan-strategy, scan-batch, scan-target, advance-chain-sync, fetch-and-filter, dispatch, scan-data loading, and handle-scan-req tests to source horizons, addresses, watched outputs, and the synced tip from the Store via walletmock expectations, replacing the legacy addrStore and walletdb-backed setup.
Rewrite the handle-chain-update, process-chain-update, and broadcast unmined-txn tests to read the synced tip and unmined transactions from the Store via walletmock expectations, and add the serializeTx helper that encodes a transaction for stubbing TxInfo.SerializedTx.
resolveMasterFingerprint read the master HD public key with a direct walletdb.View against the address manager during wallet load. The master public key is public wallet metadata, so route it through Store.GetWallet instead, removing the last direct walletdb transaction for public-metadata reads from wallet/manager.go. To serve the read, add MasterHDPubKey to the waddrmgr.AddrStore interface (already implemented on *waddrmgr.Manager) and have kvdb's GetWallet fill WalletInfo.MasterPubKey from it, tolerating ErrNoExist for shell, watch-only, and pre-master-key wallets. The MasterHDPubKey read is extracted into a small readMasterPubKey helper to keep GetWallet within the complexity limit. The DBCreateWallet/DBLoadWallet create/load path is left as-is: kvdb's Store.CreateWallet is unimplemented and the Store does not return the legacy waddrmgr.Manager/wtxmgr.Store the runtime needs, so that remains justified runtime-construction scope. Passphrase, private-key, and signer flows are unchanged (deferred to ADR 0010). TestManagerLoadSuccess now asserts the loaded wallet's cached master fingerprint is non-zero and matches the create-time value, locking in the Store-routed read.
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
3 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.
Loads the syncer scan inputs (horizons, addresses, watch outputs) from the
runtime
db.Store, resolves targeted-rescan accounts by durable identity, thendrops the legacy DB fallbacks and makes the Store mandatory. Also makes the
syncer rollback/lifecycle/scan/chain-update/broadcast tests Store-only and
routes the manager master-fingerprint read through the Store.
Split out of the runtime Store PR for reviewability; content-preserving, no
commit changes. Stacked on
impl-runtime-store-4.