Skip to content

Commit e919fcb

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 fb0fd18 commit e919fcb

5 files changed

Lines changed: 83 additions & 64 deletions

File tree

bwtest/mock/addr_store.go

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

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

waddrmgr/interface.go

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

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

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/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)