Skip to content

Commit 4434765

Browse files
benmabenma
committed
Merge remote-tracking branch 'benma/scan'
2 parents 1e7f2e8 + 57c0982 commit 4434765

File tree

5 files changed

+82
-55
lines changed

5 files changed

+82
-55
lines changed

‎backend/accounts.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,13 @@ func (backend *Backend) CreateAndPersistAccountConfig(
406406
if hiddenAccount != nil {
407407
hiddenAccount.HiddenBecauseUnused = false
408408
hiddenAccount.Name = name
409+
// We only really show the account to the user now, so this is the moment to set the
410+
// watchonly flag on it if the user has the global watchonly setting enabled.
411+
if backend.config.AppConfig().Backend.Watchonly {
412+
t := true
413+
hiddenAccount.Watch = &t
414+
}
415+
409416
accountCode = hiddenAccount.Code
410417
return nil
411418
}
@@ -756,7 +763,10 @@ func (backend *Backend) persistBTCAccountConfig(
756763
}
757764

758765
var accountWatch *bool
759-
if backend.config.AppConfig().Backend.Watchonly {
766+
// If the account was added in the background as part of scanning, we don't mark it watchonly.
767+
// Otherwise the account would appear automatically once it received funds, even if it was not
768+
// visible before and the keystore is never connected again.
769+
if !hiddenBecauseUnused && backend.config.AppConfig().Backend.Watchonly {
760770
t := true
761771
accountWatch = &t
762772
}
@@ -858,7 +868,7 @@ func (backend *Backend) persistETHAccountConfig(
858868
}
859869

860870
var accountWatch *bool
861-
if backend.config.AppConfig().Backend.Watchonly {
871+
if !hiddenBecauseUnused && backend.config.AppConfig().Backend.Watchonly {
862872
t := true
863873
accountWatch = &t
864874
}
@@ -1055,7 +1065,10 @@ func (backend *Backend) updatePersistedAccounts(
10551065
return nil
10561066
}
10571067
for _, account := range accounts {
1058-
if account.Watch == nil {
1068+
// If the account was added in the background as part of scanning, we don't mark it
1069+
// watchonly. Otherwise the account would appear automatically once it received funds,
1070+
// even if it was not visible before and the keystore is never connected again.
1071+
if !account.HiddenBecauseUnused && account.Watch == nil {
10591072
t := true
10601073
account.Watch = &t
10611074
}
@@ -1166,7 +1179,7 @@ func (backend *Backend) maybeAddHiddenUnusedAccounts() {
11661179
WithField("rootFingerprint", hex.EncodeToString(rootFingerprint)).
11671180
WithField("coinCode", coinCode)
11681181

1169-
maxAccountNumber := uint16(0)
1182+
maxAccountNumber := -1
11701183
var maxAccount *config.Account
11711184
for _, accountConfig := range cfg.Accounts {
11721185
if coinCode != accountConfig.CoinCode {
@@ -1179,22 +1192,19 @@ func (backend *Backend) maybeAddHiddenUnusedAccounts() {
11791192
if err != nil {
11801193
continue
11811194
}
1182-
if maxAccount == nil || accountNumber > maxAccountNumber {
1183-
maxAccountNumber = accountNumber
1195+
if maxAccount == nil || int(accountNumber) > maxAccountNumber {
1196+
maxAccountNumber = int(accountNumber)
11841197
maxAccount = accountConfig
11851198
}
11861199
}
1187-
if maxAccount == nil {
1188-
return nil
1189-
}
11901200
// Account scan gap limit:
11911201
// - Previous account must be used for the next one to be scanned, but:
11921202
// - The first 5 accounts are always scanned as before we had accounts discovery, the
11931203
// BitBoxApp allowed manual creation of 5 accounts, so we need to always scan these.
1194-
if maxAccount.Used || maxAccountNumber < accountsHardLimit {
1204+
if maxAccount == nil || maxAccount.Used || maxAccountNumber < accountsHardLimit {
11951205
accountCode, err := backend.createAndPersistAccountConfig(
11961206
coinCode,
1197-
maxAccountNumber+1,
1207+
uint16(maxAccountNumber+1),
11981208
true,
11991209
"",
12001210
backend.keystore,
@@ -1284,6 +1294,14 @@ func (backend *Backend) checkAccountUsed(account accounts.Interface) {
12841294
}
12851295
acct.Used = true
12861296
acct.HiddenBecauseUnused = false
1297+
1298+
// We only really show the account to the user now, so this is the moment to set the
1299+
// watchonly flag on it if the user has the global watchonly setting enabled.
1300+
if backend.config.AppConfig().Backend.Watchonly {
1301+
t := true
1302+
acct.Watch = &t
1303+
}
1304+
12871305
return nil
12881306
})
12891307
if err != nil {

‎backend/accounts_test.go

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,25 @@ func mustXKey(key string) *hdkeychain.ExtendedKey {
6969
return xkey
7070
}
7171

72+
func checkShownAccountsLen(t *testing.T, b *Backend, expectedLoaded int, expectedPersisted int) {
73+
t.Helper()
74+
cntLoaded := 0
75+
for _, acct := range b.Accounts() {
76+
if !acct.Config().Config.HiddenBecauseUnused {
77+
cntLoaded++
78+
}
79+
}
80+
require.Equal(t, expectedLoaded, cntLoaded)
81+
82+
cntPersisted := 0
83+
for _, acct := range b.Config().AccountsConfig().Accounts {
84+
if !acct.HiddenBecauseUnused {
85+
cntPersisted++
86+
}
87+
}
88+
require.Equal(t, expectedPersisted, cntPersisted)
89+
}
90+
7291
// A keystore with a similar config to a BitBox02 - supporting unified and multiple accounts, no
7392
// legacy P2PKH.
7493
func makeBitbox02LikeKeystore() *keystoremock.KeystoreMock {
@@ -1061,8 +1080,7 @@ func TestAccountSupported(t *testing.T) {
10611080
// Registering a Bitcoin-only like keystore loads also the altcoins that were persisted
10621081
// previously, because they are marked watch-only, so they should be visible.
10631082
b.registerKeystore(bb02BtcOnly)
1064-
require.Len(t, b.Accounts(), 3)
1065-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1083+
checkShownAccountsLen(t, b, 3, 3)
10661084

10671085
// If watch-only is disabled, then these will not be loaded if not supported by the keystore.
10681086
require.NoError(t, b.SetWatchonly(false))
@@ -1071,8 +1089,7 @@ func TestAccountSupported(t *testing.T) {
10711089
// Registering a Bitcoin-only like keystore loads only the Bitcoin account, even though altcoins
10721090
// were persisted previously.
10731091
b.registerKeystore(bb02BtcOnly)
1074-
require.Len(t, b.Accounts(), 1)
1075-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1092+
checkShownAccountsLen(t, b, 1, 3)
10761093
}
10771094

10781095
func TestInactiveAccount(t *testing.T) {
@@ -1096,33 +1113,28 @@ func TestInactiveAccount(t *testing.T) {
10961113

10971114
// Deactive an account.
10981115
require.NoError(t, b.SetAccountActive("v0-55555555-btc-0", false))
1099-
require.Len(t, b.Accounts(), 3)
1100-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1116+
checkShownAccountsLen(t, b, 3, 3)
11011117
require.True(t, b.Config().AccountsConfig().Lookup("v0-55555555-btc-0").Inactive)
11021118
require.False(t, !lookup(b.Accounts(), "v0-55555555-btc-0").Config().Config.Inactive)
11031119

11041120
// Reactivate.
11051121
require.NoError(t, b.SetAccountActive("v0-55555555-btc-0", true))
1106-
require.Len(t, b.Accounts(), 3)
1107-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1122+
checkShownAccountsLen(t, b, 3, 3)
11081123
require.False(t, b.Config().AccountsConfig().Lookup("v0-55555555-btc-0").Inactive)
11091124
require.True(t, !lookup(b.Accounts(), "v0-55555555-btc-0").Config().Config.Inactive)
11101125

11111126
// Deactivating an ETH account with tokens also removes the tokens
11121127
require.NoError(t, b.SetTokenActive("v0-55555555-eth-0", "eth-erc20-usdt", true))
11131128
require.NoError(t, b.SetTokenActive("v0-55555555-eth-0", "eth-erc20-bat", true))
1114-
require.Len(t, b.Accounts(), 5)
1115-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1129+
checkShownAccountsLen(t, b, 5, 3)
11161130
require.NoError(t, b.SetAccountActive("v0-55555555-eth-0", false))
1117-
require.Len(t, b.Accounts(), 5)
1118-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1131+
checkShownAccountsLen(t, b, 5, 3)
11191132
require.False(t, !lookup(b.Accounts(), "v0-55555555-eth-0").Config().Config.Inactive)
11201133
require.False(t, !lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-usdt").Config().Config.Inactive)
11211134
require.False(t, !lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-bat").Config().Config.Inactive)
11221135
// Reactivating restores them again.
11231136
require.NoError(t, b.SetAccountActive("v0-55555555-eth-0", true))
1124-
require.Len(t, b.Accounts(), 5)
1125-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1137+
checkShownAccountsLen(t, b, 5, 3)
11261138
require.True(t, !lookup(b.Accounts(), "v0-55555555-eth-0").Config().Config.Inactive)
11271139
require.True(t, !lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-usdt").Config().Config.Inactive)
11281140
require.True(t, !lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-bat").Config().Config.Inactive)
@@ -1131,15 +1143,13 @@ func TestInactiveAccount(t *testing.T) {
11311143
require.NoError(t, b.SetAccountActive("v0-55555555-btc-0", false))
11321144
require.NoError(t, b.SetAccountActive("v0-55555555-ltc-0", false))
11331145
require.NoError(t, b.SetAccountActive("v0-55555555-eth-0", false))
1134-
require.Len(t, b.Accounts(), 5)
1135-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1146+
checkShownAccountsLen(t, b, 5, 3)
11361147

11371148
// Re-registering the keystore (i.e. replugging the device) ends in the same state: no
11381149
// additional accounts created.
11391150
b.DeregisterKeystore()
11401151
b.registerKeystore(bitbox02LikeKeystore)
1141-
require.Len(t, b.Accounts(), 5)
1142-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1152+
checkShownAccountsLen(t, b, 5, 3)
11431153
}
11441154

11451155
// Test that taproot subaccounts are added if a keytore gains taproot support (e.g. BitBox02 gained
@@ -1209,8 +1219,7 @@ func TestTaprootUpgrade(t *testing.T) {
12091219

12101220
// 1) Registering a new keystore persists a set of initial default accounts.
12111221
b.registerKeystore(bitbox02NoTaproot)
1212-
require.Len(t, b.Accounts(), 3)
1213-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1222+
checkShownAccountsLen(t, b, 3, 3)
12141223
btcAccount := lookup(b.Accounts(), "v0-55555555-btc-0")
12151224
require.NotNil(t, btcAccount)
12161225
ltcAccount := lookup(b.Accounts(), "v0-55555555-ltc-0")
@@ -1230,8 +1239,7 @@ func TestTaprootUpgrade(t *testing.T) {
12301239
// "Unplug", then insert an updated keystore with taproot support.
12311240
b.DeregisterKeystore()
12321241
b.registerKeystore(bitbox02Taproot)
1233-
require.Len(t, b.Accounts(), 3)
1234-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1242+
checkShownAccountsLen(t, b, 3, 3)
12351243
btcAccount = lookup(b.Accounts(), "v0-55555555-btc-0")
12361244
require.NotNil(t, btcAccount)
12371245
ltcAccount = lookup(b.Accounts(), "v0-55555555-ltc-0")
@@ -1270,15 +1278,18 @@ func TestMaybeAddHiddenUnusedAccounts(t *testing.T) {
12701278
b.registerKeystore(makeBitbox02LikeKeystore())
12711279

12721280
// Initial accounts added: Bitcoin, Litecoin, Ethereum.
1273-
require.Len(t, b.accounts, 3)
1274-
require.Len(t, b.config.AccountsConfig().Accounts, 3)
1281+
checkShownAccountsLen(t, b, 3, 3)
12751282

12761283
// Up to 6 hidden accounts for BTC/LTC are added to be scanned even if the accounts are all
1277-
// empty.
1278-
for i := 1; i <= 5; i++ {
1284+
// empty. Calling this function too many times does not add more than that.
1285+
for i := 1; i <= 10; i++ {
12791286
b.maybeAddHiddenUnusedAccounts()
1280-
require.Len(t, b.accounts, 3+2*i)
1281-
require.Len(t, b.config.AccountsConfig().Accounts, 3+2*i)
1287+
}
1288+
1289+
require.Len(t, b.accounts, 3+2*5)
1290+
require.Len(t, b.config.AccountsConfig().Accounts, 3+2*5)
1291+
1292+
for i := 1; i <= 5; i++ {
12821293
for _, addedAccountCode := range []string{
12831294
fmt.Sprintf("v0-55555555-btc-%d", i),
12841295
fmt.Sprintf("v0-55555555-ltc-%d", i),

‎backend/aopp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ func (backend *Backend) aoppKeystoreRegistered() {
155155
var accounts []account
156156
var filteredDueToScriptType bool
157157
for _, acct := range backend.accounts {
158-
if acct.Config().Config.Inactive {
158+
if acct.Config().Config.Inactive || acct.Config().Config.HiddenBecauseUnused {
159159
continue
160160
}
161161
if acct.Coin().Code() != backend.aopp.coinCode {

‎backend/backend.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,8 @@ func (backend *Backend) registerKeystore(keystore keystore.Keystore) {
653653
backend.aoppKeystoreRegistered()
654654

655655
backend.connectKeystore.onConnect(backend.keystore)
656+
657+
go backend.maybeAddHiddenUnusedAccounts()
656658
}
657659

658660
// DeregisterKeystore removes the registered keystore.

‎backend/backend_test.go

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -123,23 +123,20 @@ func TestRegisterKeystore(t *testing.T) {
123123
require.NoError(t, b.SetWatchonly(true))
124124

125125
b.DeregisterKeystore()
126-
require.Len(t, b.Accounts(), 3)
127-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
126+
checkShownAccountsLen(t, b, 3, 3)
128127
require.Len(t, b.Config().AccountsConfig().Keystores, 1)
129128

130129
// Registering the same keystore again loads the previously persisted accounts and does not
131130
// automatically persist more accounts.
132131
b.registerKeystore(ks1)
133-
require.Len(t, b.Accounts(), 3)
134-
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
132+
checkShownAccountsLen(t, b, 3, 3)
135133
require.Len(t, b.Config().AccountsConfig().Keystores, 1)
136134

137135
// Registering another keystore persists a set of initial default accounts and loads them.
138136
// They are added to the previous set of watchonly accounts
139137
b.DeregisterKeystore()
140138
b.registerKeystore(ks2)
141-
require.Len(t, b.Accounts(), 6)
142-
require.Len(t, b.Config().AccountsConfig().Accounts, 6)
139+
checkShownAccountsLen(t, b, 6, 6)
143140
require.NotNil(t, b.Config().AccountsConfig().Lookup("v0-66666666-btc-0"))
144141
require.NotNil(t, b.Config().AccountsConfig().Lookup("v0-66666666-ltc-0"))
145142
require.NotNil(t, b.Config().AccountsConfig().Lookup("v0-66666666-eth-0"))
@@ -161,7 +158,7 @@ func TestRegisterKeystore(t *testing.T) {
161158
return nil
162159
}))
163160
b.registerKeystore(ks1)
164-
require.Len(t, b.Accounts(), 5)
161+
checkShownAccountsLen(t, b, 5, 6)
165162
// v0-55555555-btc-0 loaded even though watch=false, as the keystore is connected.
166163
require.NotNil(t, b.accounts.lookup("v0-55555555-btc-0"))
167164
require.NotNil(t, b.accounts.lookup("v0-55555555-ltc-0"))
@@ -172,7 +169,7 @@ func TestRegisterKeystore(t *testing.T) {
172169
require.NotNil(t, b.accounts.lookup("v0-66666666-eth-0"))
173170

174171
b.DeregisterKeystore()
175-
require.Len(t, b.Accounts(), 4)
172+
checkShownAccountsLen(t, b, 4, 6)
176173
// v0-55555555-btc-0 not loaded (watch = false)
177174
require.Nil(t, b.accounts.lookup("v0-55555555-btc-0"))
178175
require.NotNil(t, b.accounts.lookup("v0-55555555-ltc-0"))
@@ -248,8 +245,7 @@ func TestAccounts(t *testing.T) {
248245
acctCode, err := b.CreateAndPersistAccountConfig(coinpkg.CodeBTC, "A second Bitcoin account", ks)
249246
require.NoError(t, err)
250247
require.Equal(t, accountsTypes.Code("v0-55555555-btc-1"), acctCode)
251-
require.Len(t, b.Accounts(), 4)
252-
require.Len(t, b.Config().AccountsConfig().Accounts, 4)
248+
checkShownAccountsLen(t, b, 4, 4)
253249

254250
// 3) Activate some ETH tokens
255251
require.NoError(t, b.SetTokenActive("v0-55555555-eth-0", "eth-erc20-usdt", true))
@@ -258,18 +254,18 @@ func TestAccounts(t *testing.T) {
258254
[]string{"eth-erc20-usdt", "eth-erc20-bat"},
259255
b.Config().AccountsConfig().Lookup("v0-55555555-eth-0").ActiveTokens,
260256
)
261-
require.Len(t, b.Accounts(), 6)
262-
require.Equal(t, accountsTypes.Code("v0-55555555-eth-0-eth-erc20-bat"), b.Accounts()[4].Config().Config.Code)
263-
require.Equal(t, accountsTypes.Code("v0-55555555-eth-0-eth-erc20-usdt"), b.Accounts()[5].Config().Config.Code)
257+
checkShownAccountsLen(t, b, 6, 4)
258+
require.NotNil(t, lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-bat"))
259+
require.NotNil(t, lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-usdt"))
264260

265261
// 4) Deactivate an ETH token
266262
require.NoError(t, b.SetTokenActive("v0-55555555-eth-0", "eth-erc20-usdt", false))
267263
require.Equal(t,
268264
[]string{"eth-erc20-bat"},
269265
b.Config().AccountsConfig().Lookup("v0-55555555-eth-0").ActiveTokens,
270266
)
271-
require.Len(t, b.Accounts(), 5)
272-
require.Equal(t, accountsTypes.Code("v0-55555555-eth-0-eth-erc20-bat"), b.Accounts()[4].Config().Config.Code)
267+
checkShownAccountsLen(t, b, 5, 4)
268+
require.NotNil(t, lookup(b.Accounts(), "v0-55555555-eth-0-eth-erc20-bat"))
273269

274270
// 5) Rename an account
275271
require.NoError(t, b.RenameAccount("v0-55555555-eth-0", "My ETH"))

0 commit comments

Comments
 (0)