Skip to content

Commit

Permalink
waddrmgr: use atomic.Bool instead of mutex for IsLocked()
Browse files Browse the repository at this point in the history
  • Loading branch information
aakselrod committed Nov 25, 2024
1 parent 3f4b45a commit df51fa6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 29 deletions.
38 changes: 14 additions & 24 deletions waddrmgr/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ type Manager struct {
syncState syncState
watchingOnly atomic.Bool
birthday time.Time
locked bool
locked atomic.Bool
closed bool
chainParams *chaincfg.Params

Expand Down Expand Up @@ -459,7 +459,7 @@ func (m *Manager) lock() {
// which uses a separate derived key from the database even when it is
// locked.

m.locked = true
m.locked.Store(true)
}

// Close cleanly shuts down the manager. It makes a best try effort to remove
Expand All @@ -479,7 +479,7 @@ func (m *Manager) Close() {
}

// Attempt to clear private key material from memory.
if !m.WatchOnly() && !m.locked {
if !m.WatchOnly() && !m.IsLocked() {
m.lock()
}

Expand Down Expand Up @@ -510,7 +510,7 @@ func (m *Manager) NewScopedKeyManager(ns walletdb.ReadWriteBucket,
if !m.WatchOnly() {
// If the manager is locked, then we can't create a new scoped
// manager.
if m.locked {
if m.IsLocked() {
return nil, managerError(ErrLocked, errLocked, nil)
}

Expand Down Expand Up @@ -969,7 +969,7 @@ func (m *Manager) ChangePassphrase(ns walletdb.ReadWriteBucket, oldPassphrase,
// If unlocked, create the new passphrase hash with the new
// passphrase and salt.
var hashedPassphrase [sha512.Size]byte
if m.locked {
if m.IsLocked() {
newMasterKey.Zero()
} else {
saltedPassphrase := append(passphraseSalt[:],
Expand Down Expand Up @@ -1062,7 +1062,7 @@ func (m *Manager) ConvertToWatchingOnly(ns walletdb.ReadWriteBucket) error {

// Lock the manager to remove all clear text private key material from
// memory if needed.
if !m.locked {
if !m.IsLocked() {
m.lock()
}

Expand Down Expand Up @@ -1117,19 +1117,7 @@ func (m *Manager) ConvertToWatchingOnly(ns walletdb.ReadWriteBucket) error {
// unlocked, the decryption key needed to decrypt private keys used for signing
// is in memory.
func (m *Manager) IsLocked() bool {
m.mtx.RLock()
defer m.mtx.RUnlock()

return m.isLocked()
}

// isLocked is an internal method returning whether or not the address manager
// is locked via an unprotected read.
//
// NOTE: The caller *MUST* acquire the Manager's mutex before invocation to
// avoid data races.
func (m *Manager) isLocked() bool {
return m.locked
return m.locked.Load()
}

// Lock performs a best try effort to remove and zero all secret keys associated
Expand All @@ -1147,7 +1135,7 @@ func (m *Manager) Lock() error {
defer m.mtx.Unlock()

// Error on attempt to lock an already locked manager.
if m.locked {
if m.IsLocked() {
return managerError(ErrLocked, errLocked, nil)
}

Expand All @@ -1174,7 +1162,7 @@ func (m *Manager) Unlock(ns walletdb.ReadBucket, passphrase []byte) error {

// Avoid actually unlocking if the manager is already unlocked
// and the passphrases match.
if !m.locked {
if !m.IsLocked() {
saltedPassphrase := append(m.privPassphraseSalt[:],
passphrase...)
hashedPassphrase := sha512.Sum512(saltedPassphrase)
Expand Down Expand Up @@ -1273,7 +1261,7 @@ func (m *Manager) Unlock(ns walletdb.ReadBucket, passphrase []byte) error {
}
}

m.locked = false
m.locked.Store(false)
saltedPassphrase := append(m.privPassphraseSalt[:], passphrase...)
m.hashedPrivPassphrase = sha512.Sum512(saltedPassphrase)
zero.Bytes(saltedPassphrase)
Expand Down Expand Up @@ -1320,7 +1308,7 @@ func (m *Manager) LookupAccount(ns walletdb.ReadBucket, name string) (KeyScope,
func (m *Manager) selectCryptoKey(keyType CryptoKeyType) (EncryptorDecryptor, error) {
if keyType == CKTPrivate || keyType == CKTScript {
// The manager must be unlocked to work with the private keys.
if m.locked || m.WatchOnly() {
if m.IsLocked() || m.WatchOnly() {
return nil, managerError(ErrLocked, errLocked, nil)
}
}
Expand Down Expand Up @@ -1389,7 +1377,6 @@ func newManager(chainParams *chaincfg.Params, masterKeyPub *snacl.SecretKey,
m := &Manager{
chainParams: chainParams,
syncState: *syncInfo,
locked: true,
birthday: birthday,
masterKeyPub: masterKeyPub,
masterKeyPriv: masterKeyPriv,
Expand All @@ -1403,7 +1390,10 @@ func newManager(chainParams *chaincfg.Params, masterKeyPub *snacl.SecretKey,
externalAddrSchemas: make(map[AddressType][]KeyScope),
internalAddrSchemas: make(map[AddressType][]KeyScope),
}

// Store the atomic.Bool values for the manager.
m.watchingOnly.Store(watchingOnly)
m.locked.Store(true)

for _, sMgr := range m.scopedManagers {
externalType := sMgr.AddrSchema().ExternalAddrType
Expand Down
8 changes: 3 additions & 5 deletions waddrmgr/scoped_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ func (s *ScopedKeyManager) loadAccountInfo(ns walletdb.ReadBucket,
// The wallet will only contain private keys for default accounts if the
// wallet's not set up as watch-only and it's been unlocked.
watchOnly := s.rootManager.WatchOnly()
hasPrivateKey := !s.rootManager.isLocked() && !watchOnly
hasPrivateKey := !s.rootManager.IsLocked() && !watchOnly

// Create the new account info with the known information. The rest of
// the fields are filled out below.
Expand Down Expand Up @@ -775,9 +775,7 @@ func (s *ScopedKeyManager) deriveKeyFromPath(ns walletdb.ReadBucket,
func (s *ScopedKeyManager) chainAddressRowToManaged(ns walletdb.ReadBucket,
row *dbChainAddressRow) (ManagedAddress, error) {

// Since the manger's mutex is assumed to held when invoking this
// function, we use the internal isLocked to avoid a deadlock.
private := !s.rootManager.isLocked() && !s.rootManager.WatchOnly()
private := !s.rootManager.IsLocked() && !s.rootManager.WatchOnly()

addressKey, acctKey, masterKeyFingerprint, err := s.deriveKeyFromPath(
ns, row.account, row.branch, row.index, private,
Expand Down Expand Up @@ -1203,7 +1201,7 @@ func (s *ScopedKeyManager) nextAddresses(ns walletdb.ReadWriteBucket,
// Add the new managed address to the list of addresses
// that need their private keys derived when the
// address manager is next unlocked.
if s.rootManager.isLocked() && !watchOnly {
if s.rootManager.IsLocked() && !watchOnly {
s.deriveOnUnlock = append(s.deriveOnUnlock, info)
}
}
Expand Down

0 comments on commit df51fa6

Please sign in to comment.