From aebf6419b0fbabb137e43340b092ac3b83185b99 Mon Sep 17 00:00:00 2001 From: Jesse de Wit Date: Fri, 2 Aug 2024 12:19:20 +0200 Subject: [PATCH] waddrmgr: fix lock order in importScriptAddress --- waddrmgr/scoped_manager.go | 47 +++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index ed5575c776..1ce03d2ade 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -2200,7 +2200,12 @@ func (s *ScopedKeyManager) importScriptAddress(ns walletdb.ReadWriteBucket, error) { s.mtx.Lock() - defer s.mtx.Unlock() + unlockNeeded := true + defer func() { + if unlockNeeded { + s.mtx.Unlock() + } + }() // The manager must be unlocked to encrypt the imported script. if isSecretScript && s.rootManager.IsLocked() { @@ -2252,11 +2257,11 @@ func (s *ScopedKeyManager) importScriptAddress(ns walletdb.ReadWriteBucket, // The start block needs to be updated when the newly imported address // is before the current one. updateStartBlock := false - s.rootManager.mtx.Lock() + s.rootManager.mtx.RLock() if bs.Height < s.rootManager.syncState.startBlock.Height { updateStartBlock = true } - s.rootManager.mtx.Unlock() + s.rootManager.mtx.RUnlock() // Save the new imported address to the db and update start block (if // needed) in a single transaction. @@ -2278,21 +2283,6 @@ func (s *ScopedKeyManager) importScriptAddress(ns walletdb.ReadWriteBucket, return nil, maybeConvertDbError(err) } - if updateStartBlock { - err := putStartBlock(ns, bs) - if err != nil { - return nil, maybeConvertDbError(err) - } - } - - // Now that the database has been updated, update the start block in - // memory too if needed. - if updateStartBlock { - s.rootManager.mtx.Lock() - s.rootManager.syncState.startBlock = *bs - s.rootManager.mtx.Unlock() - } - // Create a new managed address based on the imported script. Also, // when not a watching-only address manager, make a copy of the script // since it will be cleared on lock and the script the caller passed @@ -2314,6 +2304,13 @@ func (s *ScopedKeyManager) importScriptAddress(ns walletdb.ReadWriteBucket, return nil, err } + if updateStartBlock { + err := putStartBlock(ns, bs) + if err != nil { + return nil, maybeConvertDbError(err) + } + } + // Even if the script is secret, we are currently unlocked, so we keep a // clear text copy of the script around to avoid decrypting it on each // access. @@ -2324,6 +2321,20 @@ func (s *ScopedKeyManager) importScriptAddress(ns walletdb.ReadWriteBucket, // Add the new managed address to the cache of recent addresses and // return it. s.addrs[addrKey(scriptIdent)] = managedAddr + + if updateStartBlock { + // Now that the database has been updated, update the start block in + // memory too if needed. Ensure the manager lock goes outside the scoped + // manager lock. + s.mtx.Unlock() + unlockNeeded = false + s.rootManager.mtx.Lock() + if bs.Height < s.rootManager.syncState.startBlock.Height { + s.rootManager.syncState.startBlock = *bs + } + s.rootManager.mtx.Unlock() + } + return managedAddr, nil }