From 1ae91cc5a12056b6de16c4488e9912aad40868ca Mon Sep 17 00:00:00 2001 From: Marko Bencun Date: Tue, 25 Apr 2023 03:44:16 +0200 Subject: [PATCH] backend: automatically scan and discover accounts We implement BIP-44 accounts scanning, meaning: accounts per coin are scanned until we find an unused account. See the docstring of `maybeAddHiddenUnusedAccounts()` for details, especially how and why we deviate from BIP-44. --- backend/accounts.go | 147 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 142 insertions(+), 5 deletions(-) diff --git a/backend/accounts.go b/backend/accounts.go index 396ac61bed..be12b743d3 100644 --- a/backend/accounts.go +++ b/backend/accounts.go @@ -15,6 +15,7 @@ package backend import ( + "encoding/hex" "fmt" "math" "sort" @@ -207,6 +208,7 @@ func defaultAccountName(coin coinpkg.Coin, accountNumber uint16) string { func (backend *Backend) createAndPersistAccountConfig( coinCode coinpkg.Code, accountNumber uint16, + hiddenBecauseUnused bool, name string, keystore keystore.Keystore, activeTokens []string, @@ -242,6 +244,7 @@ func (backend *Backend) createAndPersistAccountConfig( } return accountCode, backend.persistBTCAccountConfig(keystore, coin, accountCode, + hiddenBecauseUnused, name, []scriptTypeWithKeypath{ {signing.ScriptTypeP2WPKH, signing.NewAbsoluteKeypathFromUint32(84+hardenedKeystart, bip44Coin, accountNumberHardened)}, @@ -258,6 +261,7 @@ func (backend *Backend) createAndPersistAccountConfig( } return accountCode, backend.persistBTCAccountConfig(keystore, coin, accountCode, + hiddenBecauseUnused, name, []scriptTypeWithKeypath{ {signing.ScriptTypeP2WPKH, signing.NewAbsoluteKeypathFromUint32(84+hardenedKeystart, bip44Coin, accountNumberHardened)}, @@ -271,7 +275,7 @@ func (backend *Backend) createAndPersistAccountConfig( bip44Coin = "60'" } return accountCode, backend.persistETHAccountConfig( - keystore, coin, accountCode, + keystore, coin, accountCode, hiddenBecauseUnused, // TODO: Use []uint32 instead of a string keypath fmt.Sprintf("m/44'/%s/0'/0/%d", bip44Coin, accountNumber), name, @@ -407,7 +411,7 @@ func (backend *Backend) CreateAndPersistAccountConfig( return err } accountCode, err = backend.createAndPersistAccountConfig( - coinCode, nextAccountNumber, name, keystore, nil, accountsConfig) + coinCode, nextAccountNumber, false, name, keystore, nil, accountsConfig) return err }) if err != nil { @@ -602,6 +606,7 @@ func (backend *Backend) persistBTCAccountConfig( keystore keystore.Keystore, coin coinpkg.Coin, code accountsTypes.Code, + hiddenBecauseUnused bool, name string, configs []scriptTypeWithKeypath, accountsConfig *config.AccountsConfig, @@ -644,6 +649,7 @@ func (backend *Backend) persistBTCAccountConfig( if keystore.SupportsUnifiedAccounts() { return backend.persistAccount(config.Account{ + HiddenBecauseUnused: hiddenBecauseUnused, CoinCode: coin.Code(), Name: name, Code: code, @@ -662,6 +668,7 @@ func (backend *Backend) persistBTCAccountConfig( } err := backend.persistAccount(config.Account{ + HiddenBecauseUnused: hiddenBecauseUnused, CoinCode: coin.Code(), Name: suffixedName, Code: splitAccountCode(code, cfg.ScriptType()), @@ -678,6 +685,7 @@ func (backend *Backend) persistETHAccountConfig( keystore keystore.Keystore, coin coinpkg.Coin, code accountsTypes.Code, + hiddenBecauseUnused bool, keypath string, name string, activeTokens []string, @@ -716,6 +724,7 @@ func (backend *Backend) persistETHAccountConfig( } return backend.persistAccount(config.Account{ + HiddenBecauseUnused: hiddenBecauseUnused, CoinCode: coin.Code(), Name: name, Code: code, @@ -778,14 +787,16 @@ func (backend *Backend) persistDefaultAccountConfigs(keystore keystore.Keystore, if backend.arguments.Testing() { if backend.arguments.Regtest() { if backend.config.AppConfig().Backend.DeprecatedCoinActive(coinpkg.CodeRBTC) { - if _, err := backend.createAndPersistAccountConfig(coinpkg.CodeRBTC, 0, "", keystore, nil, accountsConfig); err != nil { + if _, err := backend.createAndPersistAccountConfig( + coinpkg.CodeRBTC, 0, false, "", keystore, nil, accountsConfig); err != nil { return err } } } else { for _, coinCode := range []coinpkg.Code{coinpkg.CodeTBTC, coinpkg.CodeTLTC, coinpkg.CodeGOETH} { if backend.config.AppConfig().Backend.DeprecatedCoinActive(coinCode) { - if _, err := backend.createAndPersistAccountConfig(coinCode, 0, "", keystore, nil, accountsConfig); err != nil { + if _, err := backend.createAndPersistAccountConfig( + coinCode, 0, false, "", keystore, nil, accountsConfig); err != nil { return err } @@ -808,7 +819,8 @@ func (backend *Backend) persistDefaultAccountConfigs(keystore keystore.Keystore, } } - if _, err := backend.createAndPersistAccountConfig(coinCode, 0, "", keystore, activeTokens, accountsConfig); err != nil { + if _, err := backend.createAndPersistAccountConfig( + coinCode, 0, false, "", keystore, activeTokens, accountsConfig); err != nil { return err } } @@ -918,6 +930,130 @@ func (backend *Backend) uninitAccounts() { backend.accounts = []accounts.Interface{} } +// maybeAddHiddenUnusedAccounts adds a hidden account for scanning to facilitate accounts discovery. +// A hidden account is added per coin if: +// - the highest account is used (so another one needs to be scanned) OR +// - there are less than 5 accounts: we need to always scan the first 5 accounts because we used +// to allow adding up to 5 accounts before we added the accounts discovery feature in v4.38. +// +// For now this only happens for btc/ltc, not for ETH. +// Supporting ETH needs more care as we currently use Etherscan with a rate limit as the ETH backend. +// +// See https://github.com/bitcoin/bips/blob/3db736243cd01389a4dfd98738204df1856dc5b9/bip-0044.mediawiki#user-content-Account_discovery. +// +// We deviate from BIP-44 significantly in two ways: +// +// - we always scan the first 5 accounts, as historically we allowed +// users to add that many accounts even if all of them were empty. We +// need to scan these as such gaps probably exist in the wild. +// - the accounts scan in BIP-44 is per script type (per purpose field in +// the BIP-44 keypath). Since we support unified accounts, we consider +// them together. This means that someone could have many accounts that +// all have coins on e.g. a P2WPKH address and none on a P2TR address, +// and still be able to receive to P2TR in the highest account. Such a P2TR +// account would not be discovered by other BIP44-compatible software. +func (backend *Backend) maybeAddHiddenUnusedAccounts() { + defer backend.accountsAndKeystoreLock.Lock()() + if backend.keystore == nil { + return + } + // Only load accounts which belong to connected keystores. + rootFingerprint, err := backend.keystore.RootFingerprint() + if err != nil { + backend.log.WithError(err).Error("Could not retrieve root fingerprint") + return + } + + do := func(cfg *config.AccountsConfig, coinCode coinpkg.Code) *accountsTypes.Code { + log := backend.log. + WithField("rootFingerprint", hex.EncodeToString(rootFingerprint)). + WithField("coinCode", coinCode) + + maxAccountNumber := uint16(0) + var maxAccount *config.Account + for _, accountConfig := range cfg.Accounts { + if coinCode != accountConfig.CoinCode { + continue + } + if !accountConfig.SigningConfigurations.ContainsRootFingerprint(rootFingerprint) { + continue + } + accountNumber, err := accountConfig.SigningConfigurations[0].AccountNumber() + if err != nil { + continue + } + if maxAccount == nil || accountNumber > maxAccountNumber { + maxAccountNumber = accountNumber + maxAccount = accountConfig + } + } + if maxAccount == nil { + return nil + } + // Account scan gap limit: + // - Previous account must be used for the next one to be scanned, but: + // - The first 5 accounts are always scanned as before we had accounts discovery, the + // BitBoxApp allowed manual creation of 5 accounts, so we need to always scan these. + if maxAccount.Used || maxAccountNumber < accountsHardLimit { + accountCode, err := backend.createAndPersistAccountConfig( + coinCode, + maxAccountNumber+1, + true, + "", + backend.keystore, + nil, + cfg, + ) + if err != nil { + log.WithError(err).Error("adding hidden account failed") + return nil + } + log. + WithField("accountCode", accountCode). + WithField("accountNumber", maxAccountNumber+1). + Info("automatically created hidden account") + return &accountCode + } + return nil + } + + // Enable accounts discovery for these coins. + coinCodes := []coinpkg.Code{ + coinpkg.CodeBTC, + coinpkg.CodeTBTC, + coinpkg.CodeLTC, + coinpkg.CodeTLTC, + } + for _, coinCode := range coinCodes { + var newAccountCode *accountsTypes.Code + err = backend.config.ModifyAccountsConfig(func(cfg *config.AccountsConfig) error { + newAccountCode = do(cfg, coinCode) + return nil + }) + if err != nil { + backend.log. + WithField("coinCode", coinCode). + WithError(err). + Error("maybeAddHiddenUnusedAccounts failed") + continue + } + if newAccountCode != nil { + coin, err := backend.Coin(coinCode) + if err != nil { + backend.log.Errorf("could not find coin %s", coinCode) + continue + } + accountConfig := backend.config.AccountsConfig().Lookup(*newAccountCode) + if accountConfig == nil { + backend.log.Errorf("could not find newly persisted account %s", *newAccountCode) + continue + } + backend.createAndAddAccount(coin, accountConfig) + backend.emitAccountsStatusChanged() + } + } +} + func (backend *Backend) checkAccountUsed(account accounts.Interface) { log := backend.log.WithField("accountCode", account.Config().Config.Code) if err := account.Initialize(); err != nil { @@ -947,4 +1083,5 @@ func (backend *Backend) checkAccountUsed(account accounts.Interface) { return } backend.emitAccountsStatusChanged() + backend.maybeAddHiddenUnusedAccounts() }