Skip to content

Commit e21896f

Browse files
committed
wallet: route manager master fingerprint read through Store
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.
1 parent be1215a commit e21896f

6 files changed

Lines changed: 106 additions & 112 deletions

File tree

bwtest/mock/addr_store.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,17 @@ func (m *AddrStore) BirthdayBlock(
189189
return args.Get(0).(waddrmgr.BlockStamp), args.Bool(1), args.Error(2)
190190
}
191191

192+
// MasterHDPubKey returns the plaintext master HD public key bytes persisted
193+
// for the wallet.
194+
func (m *AddrStore) MasterHDPubKey(ns walletdb.ReadBucket) ([]byte, error) {
195+
args := m.Called(ns)
196+
if raw, ok := args.Get(0).([]byte); ok {
197+
return raw, args.Error(1)
198+
}
199+
200+
return nil, args.Error(1)
201+
}
202+
192203
// IsWatchOnlyAccount determines if the account with the given key scope
193204
// is set up as watch-only.
194205
func (m *AddrStore) IsWatchOnlyAccount(ns walletdb.ReadBucket,

waddrmgr/interface.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ type AddrStore interface {
151151
// BirthdayBlock returns the birthday block of the address store.
152152
BirthdayBlock(ns walletdb.ReadBucket) (BlockStamp, bool, error)
153153

154+
// MasterHDPubKey returns the plaintext master HD public key bytes
155+
// persisted for the wallet, or ErrNoExist when none is persisted (shell,
156+
// watch-only, or pre-master-key wallets).
157+
MasterHDPubKey(ns walletdb.ReadBucket) ([]byte, error)
158+
154159
// IsWatchOnlyAccount determines if the account with the given key scope
155160
// is set up as watch-only.
156161
IsWatchOnlyAccount(ns walletdb.ReadBucket, keyScope KeyScope,

wallet/db_ops_test.go

Lines changed: 23 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
bwmock "github.com/btcsuite/btcwallet/bwtest/mock"
1212
"github.com/btcsuite/btcwallet/waddrmgr"
1313
walletmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock"
14+
"github.com/btcsuite/btcwallet/wallet/internal/db"
1415
"github.com/btcsuite/btcwallet/walletdb"
1516
_ "github.com/btcsuite/btcwallet/walletdb/bdb"
1617
"github.com/btcsuite/btcwallet/wtxmgr"
@@ -531,66 +532,40 @@ func TestDBGetScanData(t *testing.T) {
531532
require.Empty(t, initialUnspent)
532533
}
533534

534-
// TestLoadWalletScanDataKeepsImportedXpubHorizons verifies that full-scan setup
535-
// builds its horizon targets from the legacy address manager's active accounts.
536-
// Imported xpub accounts have real waddrmgr account numbers that must remain in
537-
// the lookahead set even though SQL account metadata may not expose a BIP44
538-
// account number for them.
535+
// TestLoadWalletScanDataKeepsImportedXpubHorizons verifies full-scan setup
536+
// preserves imported xpub recovery horizons under their non-masked waddrmgr
537+
// account number while still skipping the keyless imported-address bucket.
539538
func TestLoadWalletScanDataKeepsImportedXpubHorizons(t *testing.T) {
540539
t.Parallel()
541540

542-
w, mocks := createTestWalletWithMocks(t)
543-
s := newSyncer(w.cfg, w.addrStore, w.txStore, nil, &walletmock.Store{}, 0)
544-
545-
const importedAccount = uint32(9)
546-
541+
s, mgr := newStoreScanSyncer(t)
547542
scope := waddrmgr.KeyScopeBIP0084
548-
props := &waddrmgr.AccountProperties{
549-
AccountNumber: importedAccount,
550-
AccountName: "imported-xpub",
551-
ExternalKeyCount: 5,
552-
InternalKeyCount: 2,
553-
KeyScope: scope,
554-
IsWatchOnly: true,
555-
}
556-
557-
scopedMgr := &bwmock.AccountStore{}
558-
mocks.addrStore.On("ActiveScopedKeyManagers").Return(
559-
[]waddrmgr.AccountStore{scopedMgr},
560-
).Once()
561-
562-
scopedMgr.On("ActiveAccounts").Return([]uint32{importedAccount}).Once()
563-
scopedMgr.On("Scope").Return(scope).Once()
564-
565-
mocks.addrStore.On("FetchScopedKeyManager", scope).Return(
566-
scopedMgr, nil,
567-
).Once()
568-
569-
scopedMgr.On("AccountProperties", mock.Anything, importedAccount).Return(
570-
props, nil,
571-
).Once()
572-
573-
mocks.addrStore.On("ForEachRelevantActiveAddress", mock.Anything,
574-
mock.Anything,
575-
).Return(nil).Once()
576-
577-
mocks.txStore.On("OutputsToWatch", mock.Anything).Return(
578-
[]wtxmgr.Credit(nil), nil,
579-
).Once()
543+
importedNumber := createImportedXpubAccount(
544+
t, s, mgr, scope, "imported-xpub",
545+
)
580546

581547
horizonData, initialAddrs, initialUnspent, err := s.loadWalletScanData(
582548
t.Context(),
583549
)
584550

585551
require.NoError(t, err)
586-
require.Len(t, horizonData, 1)
587-
require.Equal(t, props, horizonData[0])
588-
require.Equal(t, importedAccount, horizonData[0].AccountNumber)
552+
553+
var importedProps *waddrmgr.AccountProperties
554+
for _, props := range horizonData {
555+
require.NotEqual(t, db.DefaultImportedAccountName,
556+
props.AccountName)
557+
558+
if props.AccountName == "imported-xpub" {
559+
importedProps = props
560+
}
561+
}
562+
563+
require.NotNil(t, importedProps)
564+
require.Equal(t, importedNumber, importedProps.AccountNumber)
565+
require.Equal(t, scope, importedProps.KeyScope)
566+
require.True(t, importedProps.IsWatchOnly)
589567
require.Empty(t, initialAddrs)
590568
require.Empty(t, initialUnspent)
591-
592-
mocks.addrStore.AssertExpectations(t)
593-
scopedMgr.AssertExpectations(t)
594569
}
595570

596571
// TestDBGetSyncedBlocks verifies that the wallet can successfully retrieve a

wallet/internal/db/kvdb/walletstore.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,25 @@ func (s *Store) CreateWallet(ctx context.Context,
3939
return nil, notImplemented(ctx, "CreateWallet")
4040
}
4141

42+
// readMasterPubKey returns the plaintext master HD public key persisted for
43+
// the wallet, or nil for shell, watch-only, and pre-master-key wallets, which
44+
// persist none and surface ErrNoExist.
45+
func readMasterPubKey(addrStore waddrmgr.AddrStore,
46+
ns walletdb.ReadBucket) ([]byte, error) {
47+
48+
pubKey, err := addrStore.MasterHDPubKey(ns)
49+
switch {
50+
case err == nil:
51+
return pubKey, nil
52+
53+
case waddrmgr.IsError(err, waddrmgr.ErrNoExist):
54+
return nil, nil
55+
56+
default:
57+
return nil, fmt.Errorf("get master HD pubkey: %w", err)
58+
}
59+
}
60+
4261
// GetWallet reads wallet runtime metadata from the legacy address manager.
4362
//
4463
// NOTE: kvdb is a single-wallet legacy backend. The supplied name is not
@@ -55,14 +74,24 @@ func (s *Store) GetWallet(_ context.Context,
5574

5675
addrStore := s.addrStore
5776

58-
var birthdayBlock *db.Block
77+
var (
78+
birthdayBlock *db.Block
79+
masterPubKey []byte
80+
)
5981

6082
err := walletdb.View(s.db, func(tx walletdb.ReadTx) error {
6183
ns := tx.ReadBucket(waddrmgr.NamespaceKey)
6284
if ns == nil {
6385
return errMissingAddrmgrNamespace
6486
}
6587

88+
var err error
89+
90+
masterPubKey, err = readMasterPubKey(addrStore, ns)
91+
if err != nil {
92+
return err
93+
}
94+
6695
block, verified, err := addrStore.BirthdayBlock(ns)
6796
if err != nil {
6897
if waddrmgr.IsError(err, waddrmgr.ErrBirthdayBlockNotSet) {
@@ -96,6 +125,7 @@ func (s *Store) GetWallet(_ context.Context,
96125
BirthdayBlock: birthdayBlock,
97126
SyncedTo: syncedTo,
98127
IsWatchOnly: addrStore.WatchOnly(),
128+
MasterPubKey: masterPubKey,
99129
}, nil
100130
}
101131

wallet/manager.go

Lines changed: 30 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import (
1010

1111
"github.com/btcsuite/btcd/btcutil/v2/hdkeychain"
1212
"github.com/btcsuite/btcwallet/waddrmgr"
13+
"github.com/btcsuite/btcwallet/wallet/internal/db"
1314
kvdb "github.com/btcsuite/btcwallet/wallet/internal/db/kvdb"
1415
"github.com/btcsuite/btcwallet/wallet/internal/keyvault"
15-
"github.com/btcsuite/btcwallet/walletdb"
1616
)
1717

1818
var (
@@ -323,18 +323,22 @@ func (m *Manager) Load(cfg Config) (*Wallet, error) {
323323
<-lockTimer.C
324324
}
325325

326+
store := kvdb.NewStore(cfg.DB, txMgr, addrMgr)
327+
326328
// Cache the wallet's master HD fingerprint up-front, before any
327329
// context/cancel is set up so an error here doesn't leak a
328-
// cancellable context.
329-
masterFingerprint, err := resolveMasterFingerprint(cfg.DB, addrMgr)
330+
// cancellable context. The master public key is public wallet metadata,
331+
// so it is read through the Store rather than a direct database
332+
// transaction.
333+
masterFingerprint, err := resolveMasterFingerprint(
334+
context.Background(), store, cfg.Name,
335+
)
330336
if err != nil {
331337
return nil, fmt.Errorf("cache master fingerprint: %w", err)
332338
}
333339

334340
lifetimeCtx, cancel := context.WithCancel(context.Background())
335341

336-
store := kvdb.NewStore(cfg.DB, txMgr, addrMgr)
337-
338342
w := &Wallet{
339343
cfg: cfg,
340344
id: walletID,
@@ -364,72 +368,35 @@ func (m *Manager) Load(cfg Config) (*Wallet, error) {
364368
return w, nil
365369
}
366370

367-
// errMissingWaddrmgrNamespace is returned when the waddrmgr namespace
368-
// bucket is missing from the wallet database. DBLoadWallet would have
369-
// already failed if this were a real wallet, so encountering it here
370-
// indicates a wiring bug.
371-
var errMissingWaddrmgrNamespace = errors.New(
372-
"missing waddrmgr namespace",
373-
)
374-
375-
// resolveMasterFingerprint reads the persisted master HD pubkey via
376-
// addrMgr, parses it, and returns its BIP32 fingerprint. Shell,
377-
// watch-only, and pre-master-key wallets have no pubkey persisted —
371+
// resolveMasterFingerprint reads the wallet's persisted master HD public key
372+
// through the Store, parses it, and returns its BIP32 fingerprint. Shell,
373+
// watch-only, and pre-master-key wallets have no pubkey persisted, so the
378374
// fingerprint is zero.
379-
func resolveMasterFingerprint(db walletdb.DB,
380-
addrMgr *waddrmgr.Manager) (uint32, error) {
381-
382-
var fingerprint uint32
383-
384-
err := walletdb.View(db, func(tx walletdb.ReadTx) error {
385-
ns := tx.ReadBucket(waddrmgr.NamespaceKey)
386-
if ns == nil {
387-
return errMissingWaddrmgrNamespace
388-
}
389-
390-
masterHDPubKey, err := addrMgr.MasterHDPubKey(ns)
391-
switch {
392-
case err == nil:
393-
// Continue to parse below.
394-
395-
case waddrmgr.IsError(err, waddrmgr.ErrNoExist):
396-
// Shell / watch-only / pre-master-key wallets:
397-
// no pubkey persisted. Leave fingerprint at zero
398-
// and continue.
399-
return nil
400-
401-
default:
402-
return fmt.Errorf("read master HD pubkey: %w", err)
403-
}
375+
func resolveMasterFingerprint(ctx context.Context, store db.Store,
376+
name string) (uint32, error) {
404377

405-
if len(masterHDPubKey) == 0 {
406-
// Defensive: persisted-but-empty is treated the
407-
// same as the ErrNoExist case above.
408-
return nil
409-
}
410-
411-
extKey, err := hdkeychain.NewKeyFromString(
412-
string(masterHDPubKey),
413-
)
414-
if err != nil {
415-
return fmt.Errorf("parse master HD pubkey: %w",
416-
err)
417-
}
378+
info, err := store.GetWallet(ctx, name)
379+
if err != nil {
380+
return 0, fmt.Errorf("get wallet: %w", err)
381+
}
418382

419-
mfp, err := masterKeyFingerprint(extKey)
420-
if err != nil {
421-
return fmt.Errorf("master fingerprint: %w", err)
422-
}
383+
// Shell / watch-only / pre-master-key wallets persist no master HD
384+
// public key, so the fingerprint stays zero.
385+
if len(info.MasterPubKey) == 0 {
386+
return 0, nil
387+
}
423388

424-
fingerprint = mfp
389+
extKey, err := hdkeychain.NewKeyFromString(string(info.MasterPubKey))
390+
if err != nil {
391+
return 0, fmt.Errorf("parse master HD pubkey: %w", err)
392+
}
425393

426-
return nil
427-
})
394+
mfp, err := masterKeyFingerprint(extKey)
428395
if err != nil {
429-
return 0, fmt.Errorf("read master fingerprint: %w", err)
396+
return 0, fmt.Errorf("master fingerprint: %w", err)
430397
}
431398

432-
return fingerprint, nil
399+
return mfp, nil
433400
}
434401

435402
// prepareWalletCreation validates the configuration and parameters, and derives

wallet/manager_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,12 @@ func TestManagerLoadSuccess(t *testing.T) {
398398
require.True(t, ok)
399399
require.Same(t, w, loadedW)
400400
require.Zero(t, w.ID())
401+
402+
// The master HD fingerprint is read through the Store during load. A
403+
// ModeGenSeed wallet persists a master HD public key, so the cached
404+
// fingerprint is non-zero and matches the value resolved at create time.
405+
require.NotZero(t, w.masterFingerprint)
406+
require.Equal(t, wCreated.masterFingerprint, w.masterFingerprint)
401407
}
402408

403409
// TestManagerLoad_ExistingWallet verifies that if Load is called for a wallet

0 commit comments

Comments
 (0)