diff --git a/waddrmgr/internal_test.go b/waddrmgr/internal_test.go index 7997a41..6279ecb 100644 --- a/waddrmgr/internal_test.go +++ b/waddrmgr/internal_test.go @@ -21,20 +21,6 @@ import ( // for change when the tests are run. var TstLatestMgrVersion = &latestMgrVersion -// Replace the Manager.newSecretKey function with the given one and calls -// the callback function. Afterwards the original newSecretKey -// function will be restored. -func TstRunWithReplacedNewSecretKey(callback func()) { - orig := newSecretKey - defer func() { - newSecretKey = orig - }() - newSecretKey = func(passphrase *[]byte, config *ScryptOptions) (*snacl.SecretKey, error) { - return nil, snacl.ErrDecryptFailed - } - callback() -} - // TstCheckPublicPassphrase returns true if the provided public passphrase is // correct for the manager. func (m *Manager) TstCheckPublicPassphrase(pubPassphrase []byte) bool { diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index 3be1387..a2f8962 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -174,15 +174,46 @@ type unlockDeriveInfo struct { index uint32 } +// SecretKeyGenerator is the function signature of a method that can generate +// secret keys for the address manager. +type SecretKeyGenerator func( + passphrase *[]byte, config *ScryptOptions) (*snacl.SecretKey, error) + // defaultNewSecretKey returns a new secret key. See newSecretKey. -func defaultNewSecretKey(passphrase *[]byte, config *ScryptOptions) (*snacl.SecretKey, error) { +func defaultNewSecretKey(passphrase *[]byte, + config *ScryptOptions) (*snacl.SecretKey, error) { return snacl.NewSecretKey(passphrase, config.N, config.R, config.P) } -// newSecretKey is used as a way to replace the new secret key generation -// function used so tests can provide a version that fails for testing error -// paths. -var newSecretKey = defaultNewSecretKey +var ( + // secretKeyGen is the inner method that is executed when calling + // newSecretKey. + secretKeyGen = defaultNewSecretKey + + // secretKeyGenMtx protects access to secretKeyGen, so that it can be + // replaced in testing. + secretKeyGenMtx sync.RWMutex +) + +// SetSecretKeyGen replaces the existing secret key generator, and returns the +// previous generator. +func SetSecretKeyGen(keyGen SecretKeyGenerator) SecretKeyGenerator { + secretKeyGenMtx.Lock() + oldKeyGen := secretKeyGen + secretKeyGen = keyGen + secretKeyGenMtx.Unlock() + + return oldKeyGen +} + +// newSecretKey generates a new secret key using the active secretKeyGen. +func newSecretKey(passphrase *[]byte, + config *ScryptOptions) (*snacl.SecretKey, error) { + + secretKeyGenMtx.RLock() + defer secretKeyGenMtx.RUnlock() + return secretKeyGen(passphrase, config) +} // EncryptorDecryptor provides an abstraction on top of snacl.CryptoKey so that // our tests can use dependency injection to force the behaviour they need. diff --git a/waddrmgr/manager_test.go b/waddrmgr/manager_test.go index c355355..67edc7b 100644 --- a/waddrmgr/manager_test.go +++ b/waddrmgr/manager_test.go @@ -16,6 +16,7 @@ import ( "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcutil" + "github.com/btcsuite/btcwallet/snacl" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/walletdb" "github.com/davecgh/go-spew/spew" @@ -33,6 +34,13 @@ func newHash(hexStr string) *chainhash.Hash { return hash } +// failingSecretKeyGen is a waddrmgr.SecretKeyGenerator that always returns +// snacl.ErrDecryptFailed. +func failingSecretKeyGen(passphrase *[]byte, + config *waddrmgr.ScryptOptions) (*snacl.SecretKey, error) { + return nil, snacl.ErrDecryptFailed +} + // testContext is used to store context information about a running test which // is passed into helper functions. The useSpends field indicates whether or // not the spend data should be empty or figure it out based on the specific @@ -1099,14 +1107,12 @@ func testChangePassphrase(tc *testContext) bool { // that intentionally errors. testName := "ChangePassphrase (public) with invalid new secret key" - var err error - waddrmgr.TstRunWithReplacedNewSecretKey(func() { - err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { - ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) - return tc.rootManager.ChangePassphrase( - ns, pubPassphrase, pubPassphrase2, false, fastScrypt, - ) - }) + oldKeyGen := waddrmgr.SetSecretKeyGen(failingSecretKeyGen) + err := walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { + ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) + return tc.rootManager.ChangePassphrase( + ns, pubPassphrase, pubPassphrase2, false, fastScrypt, + ) }) if !checkManagerError(tc.t, testName, err, waddrmgr.ErrCrypto) { return false @@ -1114,6 +1120,7 @@ func testChangePassphrase(tc *testContext) bool { // Attempt to change public passphrase with invalid old passphrase. testName = "ChangePassphrase (public) with invalid old passphrase" + waddrmgr.SetSecretKeyGen(oldKeyGen) err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) return tc.rootManager.ChangePassphrase( diff --git a/wallet/wallet.go b/wallet/wallet.go index a1684da..d828b1e 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -1434,33 +1434,62 @@ func (w *Wallet) CalculateAccountBalances(account uint32, confirms int32) (Balan // been used (there is at least one transaction spending to it in the // blockchain or btcd mempool), the next chained address is returned. func (w *Wallet) CurrentAddress(account uint32, scope waddrmgr.KeyScope) (btcutil.Address, error) { + chainClient, err := w.requireChainClient() + if err != nil { + return nil, err + } + manager, err := w.Manager.FetchScopedKeyManager(scope) if err != nil { return nil, err } - var addr btcutil.Address + var ( + addr btcutil.Address + props *waddrmgr.AccountProperties + ) err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) maddr, err := manager.LastExternalAddress(addrmgrNs, account) if err != nil { - // If no address exists yet, create the first external address + // If no address exists yet, create the first external + // address. if waddrmgr.IsError(err, waddrmgr.ErrAddressNotFound) { - addr, err = w.newAddress(addrmgrNs, account, scope) + addr, props, err = w.newAddress( + addrmgrNs, account, scope, + ) } return err } - // Get next chained address if the last one has already been used. + // Get next chained address if the last one has already been + // used. if maddr.Used(addrmgrNs) { - addr, err = w.newAddress(addrmgrNs, account, scope) + addr, props, err = w.newAddress( + addrmgrNs, account, scope, + ) return err } addr = maddr.Address() return nil }) - return addr, err + if err != nil { + return nil, err + } + + // If the props have been initially, then we had to create a new address + // to satisfy the query. Notify the rpc server about the new address. + if props != nil { + err = chainClient.NotifyReceived([]btcutil.Address{addr}) + if err != nil { + return nil, err + } + + w.NtfnServer.notifyAccountProperties(props) + } + + return addr, nil } // PubKeyForAddress looks up the associated public key for a P2PKH address. @@ -2763,69 +2792,94 @@ func (w *Wallet) SortedActivePaymentAddresses() ([]string, error) { // NewAddress returns the next external chained address for a wallet. func (w *Wallet) NewAddress(account uint32, - scope waddrmgr.KeyScope) (addr btcutil.Address, err error) { + scope waddrmgr.KeyScope) (btcutil.Address, error) { + chainClient, err := w.requireChainClient() + if err != nil { + return nil, err + } + + var ( + addr btcutil.Address + props *waddrmgr.AccountProperties + ) err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) var err error - addr, err = w.newAddress(addrmgrNs, account, scope) + addr, props, err = w.newAddress(addrmgrNs, account, scope) return err }) - return + if err != nil { + return nil, err + } + + // Notify the rpc server about the newly created address. + err = chainClient.NotifyReceived([]btcutil.Address{addr}) + if err != nil { + return nil, err + } + + w.NtfnServer.notifyAccountProperties(props) + + return addr, nil } func (w *Wallet) newAddress(addrmgrNs walletdb.ReadWriteBucket, account uint32, - scope waddrmgr.KeyScope) (btcutil.Address, error) { + scope waddrmgr.KeyScope) (btcutil.Address, *waddrmgr.AccountProperties, error) { manager, err := w.Manager.FetchScopedKeyManager(scope) if err != nil { - return nil, err + return nil, nil, err } // Get next address from wallet. addrs, err := manager.NextExternalAddresses(addrmgrNs, account, 1) if err != nil { - return nil, err - } - - // Request updates from btcd for new transactions sent to this address. - utilAddrs := make([]btcutil.Address, len(addrs)) - for i, addr := range addrs { - utilAddrs[i] = addr.Address() - } - w.chainClientLock.Lock() - chainClient := w.chainClient - w.chainClientLock.Unlock() - if chainClient != nil { - err := chainClient.NotifyReceived(utilAddrs) - if err != nil { - return nil, err - } + return nil, nil, err } props, err := manager.AccountProperties(addrmgrNs, account) if err != nil { log.Errorf("Cannot fetch account properties for notification "+ "after deriving next external address: %v", err) - } else { - w.NtfnServer.notifyAccountProperties(props) + return nil, nil, err } - return utilAddrs[0], nil + return addrs[0].Address(), props, nil } // NewChangeAddress returns a new change address for a wallet. -func (w *Wallet) NewChangeAddress(account uint32, scope waddrmgr.KeyScope) (addr btcutil.Address, err error) { +func (w *Wallet) NewChangeAddress(account uint32, + scope waddrmgr.KeyScope) (btcutil.Address, error) { + + chainClient, err := w.requireChainClient() + if err != nil { + return nil, err + } + + var addr btcutil.Address err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) var err error addr, err = w.newChangeAddress(addrmgrNs, account) return err }) - return + if err != nil { + return nil, err + } + + // Notify the rpc server about the newly created address. + err = chainClient.NotifyReceived([]btcutil.Address{addr}) + if err != nil { + return nil, err + } + + return addr, nil } -func (w *Wallet) newChangeAddress(addrmgrNs walletdb.ReadWriteBucket, account uint32) (btcutil.Address, error) { +func (w *Wallet) newChangeAddress(addrmgrNs walletdb.ReadWriteBucket, + account uint32) (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( @@ -2842,21 +2896,7 @@ func (w *Wallet) newChangeAddress(addrmgrNs walletdb.ReadWriteBucket, account ui return nil, err } - // Request updates from btcd for new transactions sent to this address. - utilAddrs := make([]btcutil.Address, len(addrs)) - for i, addr := range addrs { - utilAddrs[i] = addr.Address() - } - - chainClient, err := w.requireChainClient() - if err == nil { - err = chainClient.NotifyReceived(utilAddrs) - if err != nil { - return nil, err - } - } - - return utilAddrs[0], nil + return addrs[0].Address(), nil } // confirmed checks whether a transaction at height txHeight has met minconf