btcwallet: remove LookupAccount because of non-deterministic result

In theory, it should be only one custom account with a given name. However,
users could have created custom accounts with various key scopes. In that case,
'LookupAccount' has a non deterministic behaviour. To fix that, we browse
through all key scopes (deterministically) and return the first account we found.
This commit is contained in:
Torakushi 2023-03-06 18:19:18 +01:00
parent 2ef5ea9742
commit dd5ed71669
No known key found for this signature in database
GPG Key ID: 19C7D027EAB8B169
2 changed files with 39 additions and 15 deletions

View File

@ -82,11 +82,6 @@ var (
// requested for the default imported account within the wallet.
errNoImportedAddrGen = errors.New("addresses cannot be generated for " +
"the default imported account")
// errIncompatibleAccountAddr is an error returned when the type of a
// new address being requested is incompatible with the account.
errIncompatibleAccountAddr = errors.New("incompatible address type " +
"for account")
)
// BtcWallet is an implementation of the lnwallet.WalletController interface
@ -516,18 +511,14 @@ func (b *BtcWallet) keyScopeForAccountAddr(accountName string,
return addrKeyScope, defaultAccount, nil
}
// Otherwise, look up the account's key scope and check that it supports
// the requested address type.
keyScope, account, err := b.wallet.LookupAccount(accountName)
// Otherwise, look up the custom account and if it supports the given
// key scope.
accountNumber, err := b.wallet.AccountNumber(addrKeyScope, accountName)
if err != nil {
return waddrmgr.KeyScope{}, 0, err
}
if keyScope != addrKeyScope {
return waddrmgr.KeyScope{}, 0, errIncompatibleAccountAddr
}
return keyScope, account, nil
return addrKeyScope, accountNumber, nil
}
// NewAddress returns the next external or internal address for the wallet

View File

@ -97,7 +97,7 @@ func (b *BtcWallet) FundPsbt(packet *psbt.Packet, minConfs int32,
"custom change type for custom accounts")
}
scope, account, err := b.wallet.LookupAccount(accountName)
scope, account, err := b.lookupFirstCustomAccount(accountName)
if err != nil {
return 0, err
}
@ -512,7 +512,7 @@ func (b *BtcWallet) FinalizePsbt(packet *psbt.Packet, accountName string) error
// number to determine if the inputs belonging to this account should be
// signed.
default:
scope, account, err := b.wallet.LookupAccount(accountName)
scope, account, err := b.lookupFirstCustomAccount(accountName)
if err != nil {
return err
}
@ -522,3 +522,36 @@ func (b *BtcWallet) FinalizePsbt(packet *psbt.Packet, accountName string) error
return b.wallet.FinalizePsbt(keyScope, accountNum, packet)
}
// lookupFirstCustomAccount returns the first custom account found. In theory,
// there should be only one custom account for the given name. However, due to a
// lack of check, users could have created custom accounts with various key
// scopes. This behaviour has been fixed but, we still need to handle this
// specific case to avoid non-deterministic behaviour implied by LookupAccount.
func (b *BtcWallet) lookupFirstCustomAccount(
name string) (waddrmgr.KeyScope, uint32, error) {
var (
account *waddrmgr.AccountProperties
keyScope waddrmgr.KeyScope
)
for _, scope := range waddrmgr.DefaultKeyScopes {
var err error
account, err = b.wallet.AccountPropertiesByName(scope, name)
if waddrmgr.IsError(err, waddrmgr.ErrAccountNotFound) {
continue
}
if err != nil {
return keyScope, 0, err
}
keyScope = scope
break
}
if account == nil {
return waddrmgr.KeyScope{}, 0, newAccountNotFoundError(name)
}
return keyScope, account.AccountNumber, nil
}