Skip to content

Commit

Permalink
waddrmgr: fix lock order in importScriptAddress
Browse files Browse the repository at this point in the history
  • Loading branch information
JssDWt committed Oct 7, 2024
1 parent ffb143c commit aebf641
Showing 1 changed file with 29 additions and 18 deletions.
47 changes: 29 additions & 18 deletions waddrmgr/scoped_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
}

Expand Down

0 comments on commit aebf641

Please sign in to comment.