From 4bf6b4a11c1cb930a2ecb18b2b8c86b1c5f6d61a Mon Sep 17 00:00:00 2001 From: Marko Bencun Date: Mon, 18 May 2026 22:01:44 +0000 Subject: [PATCH 1/2] keystore: remove multiple account capability --- backend/accounts.go | 4 --- backend/accounts_test.go | 29 ++++----------------- backend/aopp_test.go | 3 --- backend/backend_test.go | 3 --- backend/devices/bitbox02/keystore.go | 5 ---- backend/keystore/keystore.go | 4 --- backend/keystore/mocks/keystore.go | 37 --------------------------- backend/keystore/software/software.go | 5 ---- 8 files changed, 5 insertions(+), 85 deletions(-) diff --git a/backend/accounts.go b/backend/accounts.go index c972e80498..80ddecf8c8 100644 --- a/backend/accounts.go +++ b/backend/accounts.go @@ -727,10 +727,6 @@ func nextAccountNumber(coinCode coinpkg.Code, keystore keystore.Keystore, accoun nextAccountNumber = accountNumber + 1 } } - if !keystore.SupportsMultipleAccounts() && nextAccountNumber >= 1 { - return 0, errp.WithStack(errAccountLimitReached) - } - if int(nextAccountNumber) >= accountsHardLimit(coinCode) { return 0, errp.WithStack(errAccountLimitReached) } diff --git a/backend/accounts_test.go b/backend/accounts_test.go index 5978db3bc8..1b0e98b468 100644 --- a/backend/accounts_test.go +++ b/backend/accounts_test.go @@ -337,7 +337,7 @@ func TestObserveKeystoreNameChanged(t *testing.T) { func TestNextAccountNumber(t *testing.T) { fingerprintEmpty := []byte{0x77, 0x77, 0x77, 0x77} - ks := func(fingerprint []byte, supportsMultipleAccounts bool) *keystoremock.KeystoreMock { + ks := func(fingerprint []byte) *keystoremock.KeystoreMock { return &keystoremock.KeystoreMock{ SupportsCoinFunc: func(coin coinpkg.Coin) bool { return true @@ -345,9 +345,6 @@ func TestNextAccountNumber(t *testing.T) { RootFingerprintFunc: func() ([]byte, error) { return fingerprint, nil }, - SupportsMultipleAccountsFunc: func() bool { - return supportsMultipleAccounts - }, } } @@ -405,26 +402,19 @@ func TestNextAccountNumber(t *testing.T) { }, } - num, err := nextAccountNumber(coinpkg.CodeTBTC, ks(fingerprintEmpty, true), accountsConfig) - require.NoError(t, err) - require.Equal(t, uint16(0), num) - - num, err = nextAccountNumber(coinpkg.CodeTBTC, ks(fingerprintEmpty, false), accountsConfig) + num, err := nextAccountNumber(coinpkg.CodeTBTC, ks(fingerprintEmpty), accountsConfig) require.NoError(t, err) require.Equal(t, uint16(0), num) - num, err = nextAccountNumber(coinpkg.CodeBTC, ks(rootFingerprint1, true), accountsConfig) + num, err = nextAccountNumber(coinpkg.CodeBTC, ks(rootFingerprint1), accountsConfig) require.NoError(t, err) require.Equal(t, uint16(1), num) - _, err = nextAccountNumber(coinpkg.CodeBTC, ks(rootFingerprint1, false), accountsConfig) - require.Equal(t, errAccountLimitReached, errp.Cause(err)) - - num, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint1, true), accountsConfig) + num, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint1), accountsConfig) require.NoError(t, err) require.Equal(t, uint16(4), num) - _, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint2, true), accountsConfig) + _, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint2), accountsConfig) require.Equal(t, errAccountLimitReached, errp.Cause(err)) } @@ -695,9 +685,6 @@ func TestCreateAndPersistAccountConfig(t *testing.T) { SupportsAccountFunc: func(coin coinpkg.Coin, meta interface{}) bool { return true }, - SupportsMultipleAccountsFunc: func() bool { - return true - }, BTCXPubsFunc: func(coin coinpkg.Coin, keypaths []signing.AbsoluteKeypath, ) ([]*hdkeychain.ExtendedKey, error) { return nil, expectedErr @@ -1028,9 +1015,6 @@ func TestTaprootUpgrade(t *testing.T) { return true } }, - SupportsMultipleAccountsFunc: func() bool { - return true - }, ExtendedPublicKeyFunc: keystoreHelper.ExtendedPublicKey, BTCXPubsFunc: keystoreHelper.BTCXPubs, } @@ -1060,9 +1044,6 @@ func TestTaprootUpgrade(t *testing.T) { return true } }, - SupportsMultipleAccountsFunc: func() bool { - return true - }, ExtendedPublicKeyFunc: keystoreHelper.ExtendedPublicKey, BTCXPubsFunc: keystoreHelper.BTCXPubs, } diff --git a/backend/aopp_test.go b/backend/aopp_test.go index 04f9d86c8e..2d94af373c 100644 --- a/backend/aopp_test.go +++ b/backend/aopp_test.go @@ -68,9 +68,6 @@ func makeKeystore( return true } }, - SupportsMultipleAccountsFunc: func() bool { - return true - }, CanSignMessageFunc: func(coinpkg.Code) bool { return true }, diff --git a/backend/backend_test.go b/backend/backend_test.go index 983af5227e..1487b29226 100644 --- a/backend/backend_test.go +++ b/backend/backend_test.go @@ -90,9 +90,6 @@ func makeBitBox02Multi() *keystoremock.KeystoreMock { return true } }, - SupportsMultipleAccountsFunc: func() bool { - return true - }, ExtendedPublicKeyFunc: ksHelper.ExtendedPublicKey, BTCXPubsFunc: ksHelper.BTCXPubs, } diff --git a/backend/devices/bitbox02/keystore.go b/backend/devices/bitbox02/keystore.go index 61108c5a84..4766bb3f16 100644 --- a/backend/devices/bitbox02/keystore.go +++ b/backend/devices/bitbox02/keystore.go @@ -111,11 +111,6 @@ func (keystore *keystore) SupportsAccount(coin coinpkg.Coin, meta interface{}) b } } -// SupportsMultipleAccounts implements keystore.Keystore. -func (keystore *keystore) SupportsMultipleAccounts() bool { - return true -} - // CanVerifyAddress implements keystore.Keystore. func (keystore *keystore) CanVerifyAddress(coin coinpkg.Coin) (bool, bool, error) { const optional = false diff --git a/backend/keystore/keystore.go b/backend/keystore/keystore.go index 9b31dc4a5d..858fcb84e3 100644 --- a/backend/keystore/keystore.go +++ b/backend/keystore/keystore.go @@ -82,10 +82,6 @@ type Keystore interface { // meta is a coin-specific metadata related to the account type. SupportsAccount(coinInstance coin.Coin, meta interface{}) bool - // SupportsMultipleAccounts returns true if the keystore can handle more than one account per - // coin. - SupportsMultipleAccounts() bool - // CanVerifyAddress returns whether the keystore supports to output an address securely. // This is typically done through a screen on the device or through a paired mobile phone. // optional is true if the user can skip verification, and false if they should be forced to diff --git a/backend/keystore/mocks/keystore.go b/backend/keystore/mocks/keystore.go index edbd51f67d..7bba4c9efa 100644 --- a/backend/keystore/mocks/keystore.go +++ b/backend/keystore/mocks/keystore.go @@ -75,9 +75,6 @@ var _ keystore.Keystore = &KeystoreMock{} // SupportsEIP1559Func: func() bool { // panic("mock out the SupportsEIP1559 method") // }, -// SupportsMultipleAccountsFunc: func() bool { -// panic("mock out the SupportsMultipleAccounts method") -// }, // SupportsPaymentRequestsFunc: func() error { // panic("mock out the SupportsPaymentRequests method") // }, @@ -154,9 +151,6 @@ type KeystoreMock struct { // SupportsEIP1559Func mocks the SupportsEIP1559 method. SupportsEIP1559Func func() bool - // SupportsMultipleAccountsFunc mocks the SupportsMultipleAccounts method. - SupportsMultipleAccountsFunc func() bool - // SupportsPaymentRequestsFunc mocks the SupportsPaymentRequests method. SupportsPaymentRequestsFunc func() error @@ -276,9 +270,6 @@ type KeystoreMock struct { // SupportsEIP1559 holds details about calls to the SupportsEIP1559 method. SupportsEIP1559 []struct { } - // SupportsMultipleAccounts holds details about calls to the SupportsMultipleAccounts method. - SupportsMultipleAccounts []struct { - } // SupportsPaymentRequests holds details about calls to the SupportsPaymentRequests method. SupportsPaymentRequests []struct { } @@ -329,7 +320,6 @@ type KeystoreMock struct { lockSupportsAccount sync.RWMutex lockSupportsCoin sync.RWMutex lockSupportsEIP1559 sync.RWMutex - lockSupportsMultipleAccounts sync.RWMutex lockSupportsPaymentRequests sync.RWMutex lockSupportsSwapPaymentRequests sync.RWMutex lockType sync.RWMutex @@ -905,33 +895,6 @@ func (mock *KeystoreMock) SupportsEIP1559Calls() []struct { return calls } -// SupportsMultipleAccounts calls SupportsMultipleAccountsFunc. -func (mock *KeystoreMock) SupportsMultipleAccounts() bool { - if mock.SupportsMultipleAccountsFunc == nil { - panic("KeystoreMock.SupportsMultipleAccountsFunc: method is nil but Keystore.SupportsMultipleAccounts was just called") - } - callInfo := struct { - }{} - mock.lockSupportsMultipleAccounts.Lock() - mock.calls.SupportsMultipleAccounts = append(mock.calls.SupportsMultipleAccounts, callInfo) - mock.lockSupportsMultipleAccounts.Unlock() - return mock.SupportsMultipleAccountsFunc() -} - -// SupportsMultipleAccountsCalls gets all the calls that were made to SupportsMultipleAccounts. -// Check the length with: -// -// len(mockedKeystore.SupportsMultipleAccountsCalls()) -func (mock *KeystoreMock) SupportsMultipleAccountsCalls() []struct { -} { - var calls []struct { - } - mock.lockSupportsMultipleAccounts.RLock() - calls = mock.calls.SupportsMultipleAccounts - mock.lockSupportsMultipleAccounts.RUnlock() - return calls -} - // SupportsPaymentRequests calls SupportsPaymentRequestsFunc. func (mock *KeystoreMock) SupportsPaymentRequests() error { if mock.SupportsPaymentRequestsFunc == nil { diff --git a/backend/keystore/software/software.go b/backend/keystore/software/software.go index ce067e2b37..12405e70fd 100644 --- a/backend/keystore/software/software.go +++ b/backend/keystore/software/software.go @@ -157,11 +157,6 @@ func (keystore *Keystore) SupportsAccount(coin coinpkg.Coin, meta interface{}) b } } -// SupportsMultipleAccounts implements keystore.Keystore. -func (keystore *Keystore) SupportsMultipleAccounts() bool { - return true -} - // Identifier implements keystore.Keystore. func (keystore *Keystore) Identifier() (string, error) { return keystore.identifier, nil From 345a7ed63748391969a22af2ad8f7879a67bb22b Mon Sep 17 00:00:00 2001 From: Marko Bencun Date: Mon, 18 May 2026 21:48:26 +0000 Subject: [PATCH 2/2] backend: extract account planning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findHiddenAccount`, `nextAccountNumber`, and hidden discovery all scanned config for “accounts of coin X and fingerprint Y” with account numbers. This is now owned by account planning helpers account_planning.go. --- backend/account_planning.go | 131 ++++++++++++++++++ backend/account_planning_test.go | 227 +++++++++++++++++++++++++++++++ backend/accounts.go | 134 ++++-------------- backend/accounts_test.go | 83 ----------- 4 files changed, 382 insertions(+), 193 deletions(-) create mode 100644 backend/account_planning.go create mode 100644 backend/account_planning_test.go diff --git a/backend/account_planning.go b/backend/account_planning.go new file mode 100644 index 0000000000..8defe8b987 --- /dev/null +++ b/backend/account_planning.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Apache-2.0 + +package backend + +import ( + coinpkg "github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/coin" + "github.com/BitBoxSwiss/bitbox-wallet-app/backend/config" + "github.com/BitBoxSwiss/bitbox-wallet-app/backend/keystore" + "github.com/BitBoxSwiss/bitbox-wallet-app/util/errp" +) + +type accountCandidate struct { + account *config.Account + number uint16 +} + +// accountCandidates returns persisted accounts for the coin, root fingerprint, and valid account +// numbers. Accounts with malformed signing configurations are ignored. +func accountCandidates( + accountsConfig *config.AccountsConfig, + rootFingerprint []byte, + coinCode coinpkg.Code, +) []accountCandidate { + var candidates []accountCandidate + for _, account := range accountsConfig.Accounts { + if coinCode != account.CoinCode { + continue + } + if !account.SigningConfigurations.ContainsRootFingerprint(rootFingerprint) { + continue + } + accountNumber, err := account.SigningConfigurations.AccountNumber() + if err != nil { + continue + } + candidates = append(candidates, accountCandidate{ + account: account, + number: accountNumber, + }) + } + return candidates +} + +// findHiddenAccount finds the hidden unused account with the lowest account number. +func findHiddenAccount( + coinCode coinpkg.Code, + keystore keystore.Keystore, + accountsConfig *config.AccountsConfig, +) (*config.Account, error) { + rootFingerprint, err := keystore.RootFingerprint() + if err != nil { + return nil, err + } + return lowestHiddenAccount(accountCandidates(accountsConfig, rootFingerprint, coinCode)), nil +} + +func lowestHiddenAccount(candidates []accountCandidate) *config.Account { + var result *config.Account + var resultNumber uint16 + for _, candidate := range candidates { + if !candidate.account.HiddenBecauseUnused { + continue + } + if result == nil || candidate.number < resultNumber { + result = candidate.account + resultNumber = candidate.number + } + } + return result +} + +// nextAccountNumber checks if an account for the given coin can be added, and if so, returns the +// account number of the new account. +func nextAccountNumber( + coinCode coinpkg.Code, + keystore keystore.Keystore, + accountsConfig *config.AccountsConfig, +) (uint16, error) { + rootFingerprint, err := keystore.RootFingerprint() + if err != nil { + return 0, err + } + candidates := accountCandidates(accountsConfig, rootFingerprint, coinCode) + return nextManualAccountNumber(coinCode, candidates) +} + +func nextManualAccountNumber( + coinCode coinpkg.Code, + candidates []accountCandidate, +) (uint16, error) { + nextAccountNumber := nextAccountNumberAfter(candidates) + if int(nextAccountNumber) >= accountsHardLimit(coinCode) { + return 0, errp.WithStack(errAccountLimitReached) + } + return nextAccountNumber, nil +} + +func nextAccountNumberAfter(candidates []accountCandidate) uint16 { + nextAccountNumber := uint16(0) + for _, candidate := range candidates { + if candidate.number+1 > nextAccountNumber { + nextAccountNumber = candidate.number + 1 + } + } + return nextAccountNumber +} + +func nextDiscoveryAccountNumber( + coinCode coinpkg.Code, + candidates []accountCandidate, +) (uint16, bool) { + maxAccountNumber := -1 + var maxAccount *config.Account + for _, candidate := range candidates { + if maxAccount == nil || int(candidate.number) > maxAccountNumber { + maxAccountNumber = int(candidate.number) + maxAccount = candidate.account + } + } + + // Account scan gap limit: + // - Previous account must be used for the next one to be scanned, but: + // - The first accounts up to the hard limit are always scanned as before we had accounts + // discovery, the BitBoxApp allowed manual creation of that many accounts, so we need to scan + // these. + nextAccountNumber := maxAccountNumber + 1 + if maxAccount == nil || maxAccount.Used || nextAccountNumber < accountsHardLimit(coinCode) { + return uint16(nextAccountNumber), true + } + return 0, false +} diff --git a/backend/account_planning_test.go b/backend/account_planning_test.go new file mode 100644 index 0000000000..ec2320e643 --- /dev/null +++ b/backend/account_planning_test.go @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: Apache-2.0 + +package backend + +import ( + "testing" + + accountsTypes "github.com/BitBoxSwiss/bitbox-wallet-app/backend/accounts/types" + coinpkg "github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/coin" + "github.com/BitBoxSwiss/bitbox-wallet-app/backend/config" + keystoremock "github.com/BitBoxSwiss/bitbox-wallet-app/backend/keystore/mocks" + "github.com/BitBoxSwiss/bitbox-wallet-app/backend/signing" + "github.com/BitBoxSwiss/bitbox-wallet-app/util/errp" + "github.com/BitBoxSwiss/bitbox-wallet-app/util/test" + "github.com/btcsuite/btcd/btcutil/hdkeychain" + "github.com/btcsuite/btcd/chaincfg" + "github.com/stretchr/testify/require" +) + +func planningAccount( + code string, + coinCode coinpkg.Code, + rootFingerprint []byte, + accountNumber uint32, +) *config.Account { + return &config.Account{ + Code: accountsTypes.Code(code), + CoinCode: coinCode, + SigningConfigurations: signing.Configurations{ + signing.NewBitcoinConfiguration( + signing.ScriptTypeP2WPKH, + rootFingerprint, + signing.NewAbsoluteKeypathFromUint32( + 84+hardenedKeystart, + hardenedKeystart, + accountNumber+hardenedKeystart, + ), + test.TstMustXKey("xpub6Cxa67Bfe1Aw5VvLM1Ppua9x28CXH1zUYoAuBzFRjR6hWnA6aUcny84KYkeVcZWnWXxKSkxCEyMA8xic54ydBPWm5oziXpsXq6nX8FELMQn"), + ), + }, + } +} + +func planningAccountWithInvalidAccountNumber( + code string, + coinCode coinpkg.Code, + rootFingerprint []byte, +) *config.Account { + return &config.Account{ + Code: accountsTypes.Code(code), + CoinCode: coinCode, + SigningConfigurations: signing.Configurations{ + signing.NewBitcoinConfiguration( + signing.ScriptTypeP2WPKH, + rootFingerprint, + mustKeypath("m/84'/0'/0/1"), + test.TstMustXKey("xpub6Cxa67Bfe1Aw5VvLM1Ppua9x28CXH1zUYoAuBzFRjR6hWnA6aUcny84KYkeVcZWnWXxKSkxCEyMA8xic54ydBPWm5oziXpsXq6nX8FELMQn"), + ), + }, + } +} + +func TestNextAccountNumber(t *testing.T) { + fingerprintEmpty := []byte{0x77, 0x77, 0x77, 0x77} + ks := func(fingerprint []byte) *keystoremock.KeystoreMock { + return &keystoremock.KeystoreMock{ + SupportsCoinFunc: func(coin coinpkg.Coin) bool { + return true + }, + RootFingerprintFunc: func() ([]byte, error) { + return fingerprint, nil + }, + } + } + + xprv, err := hdkeychain.NewMaster(make([]byte, hdkeychain.RecommendedSeedLen), &chaincfg.TestNet3Params) + require.NoError(t, err) + xpub, err := xprv.Neuter() + require.NoError(t, err) + + accountsConfig := &config.AccountsConfig{ + Accounts: []*config.Account{ + { + CoinCode: coinpkg.CodeBTC, + SigningConfigurations: signing.Configurations{ + signing.NewBitcoinConfiguration( + signing.ScriptTypeP2WPKH, + rootFingerprint1, + mustKeypath("m/84'/0'/0'"), + xpub, + ), + }, + }, + { + CoinCode: coinpkg.CodeBTC, + SigningConfigurations: signing.Configurations{ + signing.NewBitcoinConfiguration( + signing.ScriptTypeP2WPKHP2SH, + rootFingerprint1, + mustKeypath("m/49'/0'/0'"), + xpub, + ), + }, + }, + { + CoinCode: coinpkg.CodeTBTC, + SigningConfigurations: signing.Configurations{ + signing.NewBitcoinConfiguration( + signing.ScriptTypeP2WPKH, + rootFingerprint1, + mustKeypath("m/84'/0'/3'"), + xpub, + ), + }, + }, + { + CoinCode: coinpkg.CodeTBTC, + SigningConfigurations: signing.Configurations{ + signing.NewBitcoinConfiguration( + signing.ScriptTypeP2WPKH, + rootFingerprint2, + mustKeypath("m/84'/0'/5'"), + xpub, + ), + }, + }, + }, + } + + num, err := nextAccountNumber(coinpkg.CodeTBTC, ks(fingerprintEmpty), accountsConfig) + require.NoError(t, err) + require.Equal(t, uint16(0), num) + + num, err = nextAccountNumber(coinpkg.CodeBTC, ks(rootFingerprint1), accountsConfig) + require.NoError(t, err) + require.Equal(t, uint16(1), num) + + num, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint1), accountsConfig) + require.NoError(t, err) + require.Equal(t, uint16(4), num) + + _, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint2), accountsConfig) + require.Equal(t, errAccountLimitReached, errp.Cause(err)) +} + +func TestAccountCandidates(t *testing.T) { + accountsConfig := &config.AccountsConfig{ + Accounts: []*config.Account{ + planningAccount("btc-0", coinpkg.CodeBTC, rootFingerprint1, 0), + planningAccount("btc-other-fingerprint", coinpkg.CodeBTC, rootFingerprint2, 1), + planningAccount("ltc-2", coinpkg.CodeLTC, rootFingerprint1, 2), + planningAccountWithInvalidAccountNumber("btc-invalid-account-number", coinpkg.CodeBTC, rootFingerprint1), + {Code: "btc-no-config", CoinCode: coinpkg.CodeBTC}, + planningAccount("btc-3", coinpkg.CodeBTC, rootFingerprint1, 3), + }, + } + + candidates := accountCandidates(accountsConfig, rootFingerprint1, coinpkg.CodeBTC) + require.Equal(t, []accountCandidate{ + {account: accountsConfig.Accounts[0], number: 0}, + {account: accountsConfig.Accounts[5], number: 3}, + }, candidates) +} + +func TestLowestHiddenAccount(t *testing.T) { + account0 := &config.Account{Code: "account-0"} + account1 := &config.Account{Code: "account-1", HiddenBecauseUnused: true} + account3 := &config.Account{Code: "account-3", HiddenBecauseUnused: true} + + require.Equal(t, account1, lowestHiddenAccount([]accountCandidate{ + {account: account3, number: 3}, + {account: account0, number: 0}, + {account: account1, number: 1}, + })) + require.Nil(t, lowestHiddenAccount([]accountCandidate{ + {account: account0, number: 0}, + })) +} + +func TestNextManualAccountNumber(t *testing.T) { + account0 := &config.Account{Code: "account-0"} + account3 := &config.Account{Code: "account-3"} + account5 := &config.Account{Code: "account-5"} + + accountNumber, err := nextManualAccountNumber(coinpkg.CodeBTC, []accountCandidate{ + {account: account0, number: 0}, + {account: account3, number: 3}, + }) + require.NoError(t, err) + require.Equal(t, uint16(4), accountNumber) + + accountNumber, err = nextManualAccountNumber(coinpkg.CodeBTC, nil) + require.NoError(t, err) + require.Equal(t, uint16(0), accountNumber) + + _, err = nextManualAccountNumber(coinpkg.CodeBTC, []accountCandidate{ + {account: account5, number: 5}, + }) + require.Equal(t, errAccountLimitReached, errp.Cause(err)) +} + +func TestNextDiscoveryAccountNumber(t *testing.T) { + account4 := &config.Account{Code: "account-4"} + account5 := &config.Account{Code: "account-5"} + usedAccount5 := &config.Account{Code: "used-account-5", Used: true} + + accountNumber, ok := nextDiscoveryAccountNumber(coinpkg.CodeBTC, nil) + require.True(t, ok) + require.Equal(t, uint16(0), accountNumber) + + accountNumber, ok = nextDiscoveryAccountNumber(coinpkg.CodeBTC, []accountCandidate{ + {account: account4, number: 4}, + }) + require.True(t, ok) + require.Equal(t, uint16(5), accountNumber) + + _, ok = nextDiscoveryAccountNumber(coinpkg.CodeBTC, []accountCandidate{ + {account: account5, number: 5}, + }) + require.False(t, ok) + + accountNumber, ok = nextDiscoveryAccountNumber(coinpkg.CodeBTC, []accountCandidate{ + {account: usedAccount5, number: 5}, + }) + require.True(t, ok) + require.Equal(t, uint16(6), accountNumber) +} diff --git a/backend/accounts.go b/backend/accounts.go index 80ddecf8c8..cbdc991363 100644 --- a/backend/accounts.go +++ b/backend/accounts.go @@ -6,7 +6,6 @@ import ( "bytes" "encoding/hex" "fmt" - "math" "math/big" "sort" "strings" @@ -666,73 +665,6 @@ func (backend *Backend) createAndPersistAccountConfig( } } -// findHiddenAccount finds the first (lowest account number) account which is hidden because it is -// unused. Returns nil if no such account exists. -func findHiddenAccount( - coinCode coinpkg.Code, - keystore keystore.Keystore, - accountsConfig *config.AccountsConfig) (*config.Account, error) { - rootFingerprint, err := keystore.RootFingerprint() - if err != nil { - return nil, err - } - smallestHiddenAccountNumber := uint16(math.MaxUint16) - var result *config.Account - - for _, account := range accountsConfig.Accounts { - if coinCode != account.CoinCode { - continue - } - if !account.SigningConfigurations.ContainsRootFingerprint(rootFingerprint) { - continue - } - if len(account.SigningConfigurations) == 0 { - continue - } - accountNumber, err := account.SigningConfigurations[0].AccountNumber() - if err != nil { - continue - } - if account.HiddenBecauseUnused && accountNumber < smallestHiddenAccountNumber { - smallestHiddenAccountNumber = accountNumber - result = account - } - } - return result, nil -} - -// nextAccountNumber checks if an account for the given coin can be added, and if so, returns the -// account number of the new account. -func nextAccountNumber(coinCode coinpkg.Code, keystore keystore.Keystore, accountsConfig *config.AccountsConfig) (uint16, error) { - rootFingerprint, err := keystore.RootFingerprint() - if err != nil { - return 0, err - } - nextAccountNumber := uint16(0) - for _, account := range accountsConfig.Accounts { - if coinCode != account.CoinCode { - continue - } - if !account.SigningConfigurations.ContainsRootFingerprint(rootFingerprint) { - continue - } - if len(account.SigningConfigurations) == 0 { - continue - } - accountNumber, err := account.SigningConfigurations[0].AccountNumber() - if err != nil { - continue - } - if accountNumber+1 > nextAccountNumber { - nextAccountNumber = accountNumber + 1 - } - } - if int(nextAccountNumber) >= accountsHardLimit(coinCode) { - return 0, errp.WithStack(errAccountLimitReached) - } - return nextAccountNumber, nil -} - // CanAddAccount returns true if it is possible to add an account for the given coin and keystore, // along with a suggested name for the account. func (backend *Backend) CanAddAccount(coinCode coinpkg.Code, keystore keystore.Keystore) (string, bool) { @@ -1574,50 +1506,32 @@ func (backend *Backend) maybeAddHiddenUnusedAccounts() { WithField("rootFingerprint", hex.EncodeToString(rootFingerprint)). WithField("coinCode", coinCode) - maxAccountNumber := -1 - 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 || int(accountNumber) > maxAccountNumber { - maxAccountNumber = int(accountNumber) - maxAccount = accountConfig - } + nextAccountNumber, ok := nextDiscoveryAccountNumber( + coinCode, + accountCandidates(cfg, rootFingerprint, coinCode), + ) + if !ok { + 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 - nextAccountNumber := maxAccountNumber + 1 - if maxAccount == nil || maxAccount.Used || nextAccountNumber < accountsHardLimit(coinCode) { - accountCode, err := backend.createAndPersistAccountConfig( - coinCode, - uint16(nextAccountNumber), - true, - "", - backend.keystore, - nil, - cfg, - ) - if err != nil { - log.WithError(err).Error("adding hidden account failed") - return nil - } - log. - WithField("accountCode", accountCode). - WithField("accountNumber", nextAccountNumber). - Info("automatically created hidden account") - return &accountCode + + accountCode, err := backend.createAndPersistAccountConfig( + coinCode, + nextAccountNumber, + true, + "", + backend.keystore, + nil, + cfg, + ) + if err != nil { + log.WithError(err).Error("adding hidden account failed") + return nil } - return nil + log. + WithField("accountCode", accountCode). + WithField("accountNumber", nextAccountNumber). + Info("automatically created hidden account") + return &accountCode } // Enable accounts discovery for these coins. diff --git a/backend/accounts_test.go b/backend/accounts_test.go index 1b0e98b468..9830c9e2f4 100644 --- a/backend/accounts_test.go +++ b/backend/accounts_test.go @@ -335,89 +335,6 @@ func TestObserveKeystoreNameChanged(t *testing.T) { }) } -func TestNextAccountNumber(t *testing.T) { - fingerprintEmpty := []byte{0x77, 0x77, 0x77, 0x77} - ks := func(fingerprint []byte) *keystoremock.KeystoreMock { - return &keystoremock.KeystoreMock{ - SupportsCoinFunc: func(coin coinpkg.Coin) bool { - return true - }, - RootFingerprintFunc: func() ([]byte, error) { - return fingerprint, nil - }, - } - } - - xprv, err := hdkeychain.NewMaster(make([]byte, hdkeychain.RecommendedSeedLen), &chaincfg.TestNet3Params) - require.NoError(t, err) - xpub, err := xprv.Neuter() - require.NoError(t, err) - - accountsConfig := &config.AccountsConfig{ - Accounts: []*config.Account{ - { - CoinCode: coinpkg.CodeBTC, - SigningConfigurations: signing.Configurations{ - signing.NewBitcoinConfiguration( - signing.ScriptTypeP2WPKH, - rootFingerprint1, - mustKeypath("m/84'/0'/0'"), - xpub, - ), - }, - }, - { - CoinCode: coinpkg.CodeBTC, - SigningConfigurations: signing.Configurations{ - signing.NewBitcoinConfiguration( - signing.ScriptTypeP2WPKHP2SH, - rootFingerprint1, - mustKeypath("m/49'/0'/0'"), - xpub, - ), - }, - }, - { - CoinCode: coinpkg.CodeTBTC, - SigningConfigurations: signing.Configurations{ - signing.NewBitcoinConfiguration( - signing.ScriptTypeP2WPKH, - rootFingerprint1, - mustKeypath("m/84'/0'/3'"), - xpub, - ), - }, - }, - { - CoinCode: coinpkg.CodeTBTC, - SigningConfigurations: signing.Configurations{ - signing.NewBitcoinConfiguration( - signing.ScriptTypeP2WPKH, - rootFingerprint2, - mustKeypath("m/84'/0'/5'"), - xpub, - ), - }, - }, - }, - } - - num, err := nextAccountNumber(coinpkg.CodeTBTC, ks(fingerprintEmpty), accountsConfig) - require.NoError(t, err) - require.Equal(t, uint16(0), num) - - num, err = nextAccountNumber(coinpkg.CodeBTC, ks(rootFingerprint1), accountsConfig) - require.NoError(t, err) - require.Equal(t, uint16(1), num) - - num, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint1), accountsConfig) - require.NoError(t, err) - require.Equal(t, uint16(4), num) - - _, err = nextAccountNumber(coinpkg.CodeTBTC, ks(rootFingerprint2), accountsConfig) - require.Equal(t, errAccountLimitReached, errp.Cause(err)) -} - func TestSupportedCoins(t *testing.T) { t.Run("all coins supported, mainnet", func(t *testing.T) { b := newBackend(t, testnetDisabled, regtestDisabled)