diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index f2895ca..8dc279a 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -738,6 +738,43 @@ func (m *Manager) ForEachActiveAddress(ns walletdb.ReadBucket, fn func(addr btcu return nil } +// ForEachRelevantActiveAddress invokes the given closure on each active +// address relevant to the wallet. Ideally, only addresses within the default +// key scopes would be relevant, but due to a bug (now fixed) in which change +// addresses could be created outside of the default key scopes, we now need to +// check for those as well. +func (m *Manager) ForEachRelevantActiveAddress(ns walletdb.ReadBucket, + fn func(addr btcutil.Address) error) error { + + m.mtx.RLock() + defer m.mtx.RUnlock() + + for _, scopedMgr := range m.scopedManagers { + // If the manager is for a default key scope, we'll return all + // addresses, otherwise we'll only return internal addresses, as + // that's the branch used for change addresses. + isDefaultKeyScope := false + for _, defaultKeyScope := range DefaultKeyScopes { + if scopedMgr.Scope() == defaultKeyScope { + isDefaultKeyScope = true + break + } + } + + var err error + if isDefaultKeyScope { + err = scopedMgr.ForEachActiveAddress(ns, fn) + } else { + err = scopedMgr.ForEachInternalActiveAddress(ns, fn) + } + if err != nil { + return err + } + } + + return nil +} + // ForEachAccountAddress calls the given function with each address of // the given account stored in the manager, breaking early on error. func (m *Manager) ForEachAccountAddress(ns walletdb.ReadBucket, account uint32, @@ -756,29 +793,6 @@ func (m *Manager) ForEachAccountAddress(ns walletdb.ReadBucket, account uint32, return nil } -// ForEachDefaultScopeActiveAddress calls the given function with each active -// address stored in the manager within the default scopes, breaking early on -// error. -func (m *Manager) ForEachDefaultScopeActiveAddress(ns walletdb.ReadBucket, - fn func(addr btcutil.Address) error) error { - - m.mtx.RLock() - defer m.mtx.RUnlock() - - for _, keyScope := range DefaultKeyScopes { - scopedMgr, ok := m.scopedManagers[keyScope] - if !ok { - return fmt.Errorf("manager for default key scope with "+ - "purpose %v not found", keyScope.Purpose) - } - if err := scopedMgr.ForEachActiveAddress(ns, fn); err != nil { - return err - } - } - - return nil -} - // ChainParams returns the chain parameters for this address manager. func (m *Manager) ChainParams() *chaincfg.Params { // NOTE: No need for mutex here since the net field does not change diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index b9acda3..16bd83a 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -1760,3 +1760,30 @@ func (s *ScopedKeyManager) ForEachActiveAddress(ns walletdb.ReadBucket, return nil } + +// ForEachInternalActiveAddress invokes the given closure on each _internal_ +// active address belonging to the scoped key manager, breaking early on error. +func (s *ScopedKeyManager) ForEachInternalActiveAddress(ns walletdb.ReadBucket, + fn func(addr btcutil.Address) error) error { + + s.mtx.Lock() + defer s.mtx.Unlock() + + addrFn := func(rowInterface interface{}) error { + managedAddr, err := s.rowInterfaceToManaged(ns, rowInterface) + if err != nil { + return err + } + // Skip any non-internal branch addresses. + if !managedAddr.Internal() { + return nil + } + return fn(managedAddr.Address()) + } + + if err := forEachActiveAddress(ns, &s.scope, addrFn); err != nil { + return maybeConvertDbError(err) + } + + return nil +} diff --git a/wallet/createtx.go b/wallet/createtx.go index 9fa0973..6bc5aae 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -135,15 +135,21 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, inputSource := makeInputSource(eligible) changeSource := func() ([]byte, error) { - // Derive the change output script. As a hack to allow - // spending from the imported account, change addresses are - // created from account 0. + // Derive the change output script. We'll use the default key + // scope responsible for P2WPKH addresses to do so. As a hack to + // allow spending from the imported account, change addresses + // are created from account 0. var changeAddr btcutil.Address var err error + changeKeyScope := waddrmgr.KeyScopeBIP0084 if account == waddrmgr.ImportedAddrAccount { - changeAddr, err = w.newChangeAddress(addrmgrNs, 0) + changeAddr, err = w.newChangeAddress( + addrmgrNs, 0, changeKeyScope, + ) } else { - changeAddr, err = w.newChangeAddress(addrmgrNs, account) + changeAddr, err = w.newChangeAddress( + addrmgrNs, account, changeKeyScope, + ) } if err != nil { return nil, err diff --git a/wallet/wallet.go b/wallet/wallet.go index a3e092c..aaa9ff6 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -303,17 +303,15 @@ func (w *Wallet) SetChainSynced(synced bool) { w.chainClientSyncMtx.Unlock() } -// activeData returns the currently-active receiving addresses that exist within -// the wallet's default key scopes and all unspent outputs. This is primarily -// intended to provide the parameters for a rescan request. -func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, - []wtxmgr.Credit, error) { - +// activeData returns the currently-active receiving addresses and all unspent +// outputs. This is primarely intended to provide the parameters for a +// rescan request. +func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, []wtxmgr.Credit, error) { addrmgrNs := dbtx.ReadBucket(waddrmgrNamespaceKey) txmgrNs := dbtx.ReadBucket(wtxmgrNamespaceKey) var addrs []btcutil.Address - err := w.Manager.ForEachDefaultScopeActiveAddress( + err := w.Manager.ForEachRelevantActiveAddress( addrmgrNs, func(addr btcutil.Address) error { addrs = append(addrs, addr) return nil @@ -639,13 +637,15 @@ func (w *Wallet) recovery(chainClient chain.Interface, ) // In the event that this recovery is being resumed, we will need to - // repopulate all found addresses from the database. For basic recovery, - // we will only do so for the default scopes. - scopedMgrs, err := w.defaultScopeManagers() - if err != nil { - return err + // repopulate all found addresses from the database. Ideally, for basic + // recovery, we would only do so for the default scopes, but due to a + // bug in which the wallet would create change addresses outside of the + // default scopes, it's necessary to attempt all registered key scopes. + scopedMgrs := make(map[waddrmgr.KeyScope]*waddrmgr.ScopedKeyManager) + for _, scopedMgr := range w.Manager.ActiveScopedKeyManagers() { + scopedMgrs[scopedMgr.Scope()] = scopedMgr } - err = walletdb.View(w.db, func(tx walletdb.ReadTx) error { + err := walletdb.View(w.db, func(tx walletdb.ReadTx) error { txMgrNS := tx.ReadBucket(wtxmgrNamespaceKey) credits, err := w.TxStore.UnspentOutputs(txMgrNS) if err != nil { @@ -714,9 +714,9 @@ func (w *Wallet) recovery(chainClient chain.Interface, return err } } - return w.recoverDefaultScopes( + return w.recoverScopedAddresses( chainClient, tx, ns, recoveryBatch, - recoveryMgr.State(), + recoveryMgr.State(), scopedMgrs, ) }) if err != nil { @@ -739,48 +739,10 @@ func (w *Wallet) recovery(chainClient chain.Interface, return nil } -// defaultScopeManagers fetches the ScopedKeyManagers from the wallet using the -// default set of key scopes. -func (w *Wallet) defaultScopeManagers() ( - map[waddrmgr.KeyScope]*waddrmgr.ScopedKeyManager, error) { - - scopedMgrs := make(map[waddrmgr.KeyScope]*waddrmgr.ScopedKeyManager) - for _, scope := range waddrmgr.DefaultKeyScopes { - scopedMgr, err := w.Manager.FetchScopedKeyManager(scope) - if err != nil { - return nil, err - } - - scopedMgrs[scope] = scopedMgr - } - - return scopedMgrs, nil -} - -// recoverDefaultScopes attempts to recover any addresses belonging to any -// active scoped key managers known to the wallet. Recovery of each scope's -// default account will be done iteratively against the same batch of blocks. -// TODO(conner): parallelize/pipeline/cache intermediate network requests -func (w *Wallet) recoverDefaultScopes( - chainClient chain.Interface, - tx walletdb.ReadWriteTx, - ns walletdb.ReadWriteBucket, - batch []wtxmgr.BlockMeta, - recoveryState *RecoveryState) error { - - scopedMgrs, err := w.defaultScopeManagers() - if err != nil { - return err - } - - return w.recoverScopedAddresses( - chainClient, tx, ns, batch, recoveryState, scopedMgrs, - ) -} - -// recoverAccountAddresses scans a range of blocks in attempts to recover any +// recoverScopedAddresses scans a range of blocks in attempts to recover any // previously used addresses for a particular account derivation path. At a high // level, the algorithm works as follows: +// // 1) Ensure internal and external branch horizons are fully expanded. // 2) Filter the entire range of blocks, stopping if a non-zero number of // address are contained in a particular block. @@ -788,6 +750,8 @@ func (w *Wallet) recoverDefaultScopes( // 4) Record any outpoints found in the block that should be watched for spends // 5) Trim the range of blocks up to and including the one reporting the addrs. // 6) Repeat from (1) if there are still more blocks in the range. +// +// TODO(conner): parallelize/pipeline/cache intermediate network requests func (w *Wallet) recoverScopedAddresses( chainClient chain.Interface, tx walletdb.ReadWriteTx, @@ -2946,7 +2910,7 @@ func (w *Wallet) NewChangeAddress(account uint32, err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) var err error - addr, err = w.newChangeAddress(addrmgrNs, account) + addr, err = w.newChangeAddress(addrmgrNs, account, scope) return err }) if err != nil { @@ -2968,14 +2932,9 @@ func (w *Wallet) NewChangeAddress(account uint32, // method in order to detect when an on-chain transaction pays to the address // being created. func (w *Wallet) newChangeAddress(addrmgrNs walletdb.ReadWriteBucket, - account uint32) (btcutil.Address, error) { + account uint32, scope waddrmgr.KeyScope) (btcutil.Address, error) { - // As we're making a change address, we'll fetch the type of manager - // that is able to make p2wkh output as they're the most efficient. - scopes := w.Manager.ScopesForExternalAddrType( - waddrmgr.WitnessPubKey, - ) - manager, err := w.Manager.FetchScopedKeyManager(scopes[0]) + manager, err := w.Manager.FetchScopedKeyManager(scope) if err != nil { return nil, err }