Skip to content

Commit

Permalink
backend: convert global watchonly setting to a keystore setting
Browse files Browse the repository at this point in the history 
Before, if the global watchonly setting was active, all currently
loaded accounts become watchonly, as well as all future accounts, even
from different keystores (ther BitBox02s, same BitBox02 with different
passphrases, etc.).

This could be confusing. We instead make the global watchonly setting
a keystore setting instead.
  • Loading branch information
@benma
benma committed Dec 5, 2023
1 parent 6817b4a commit 0de1abc
Show file tree
Hide file tree
Showing 12 changed files with 177 additions and 200 deletions.
83 changes: 56 additions & 27 deletions backend/accounts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func sortAccounts(accounts []accounts.Interface) {

// filterAccounts fetches all persisted accounts that pass the provided filter. Testnet/regtest
// accounts are not loaded in mainnet and vice versa.
func (backend *Backend) filterAccounts(accountsConfig *config.AccountsConfig, filter func(*config.Account) bool) []*config.Account {
func (backend *Backend) filterAccounts(accountsConfig *config.AccountsConfig, filter func(*config.AccountsConfig, *config.Account) bool) []*config.Account {
var accounts []*config.Account
for _, account := range accountsConfig.Accounts {
if !backend.arguments.Regtest() {
Expand All @@ -151,7 +151,7 @@ func (backend *Backend) filterAccounts(accountsConfig *config.AccountsConfig, fi
continue
}

if !filter(account) {
if !filter(accountsConfig, account) {
continue
}
accounts = append(accounts, account)
Expand Down Expand Up @@ -406,9 +406,15 @@ func (backend *Backend) CreateAndPersistAccountConfig(
if hiddenAccount != nil {
hiddenAccount.HiddenBecauseUnused = false
hiddenAccount.Name = name

rootFingerprint, err := keystore.RootFingerprint()
if err != nil {
return err
}

// We only really show the account to the user now, so this is the moment to set the
// watchonly flag on it if the user has the global watchonly setting enabled.
if backend.config.AppConfig().Backend.Watchonly {
// watchonly flag on it if the user has the keystore's watchonly setting enabled.
if accountsConfig.IsKeystoreWatchonly(rootFingerprint) {
t := true
hiddenAccount.Watch = &t
}
Expand Down Expand Up @@ -500,7 +506,7 @@ func copyBool(b *bool) *bool {
// AccountSetWatch sets the account's persisted watch flag to `watch`. Set to `true` if the account
// should be loaded even if its keystore is not connected.
// If `watch` is set to `false`, the account is unloaded and the frontend notified.
// If `watch` is set to `true`, the account is loaded (if the global watchonly flag is enabled) and the frontend notified.
// If `watch` is set to `true`, the account is loaded (if its keystore's watchonly flag is enabled) and the frontend notified.
func (backend *Backend) AccountSetWatch(filter func(*config.Account) bool, watch *bool) error {
err := backend.config.ModifyAccountsConfig(func(accountsConfig *config.AccountsConfig) error {
for _, acct := range accountsConfig.Accounts {
Expand Down Expand Up @@ -766,7 +772,7 @@ func (backend *Backend) persistBTCAccountConfig(
// If the account was added in the background as part of scanning, we don't mark it watchonly.
// Otherwise the account would appear automatically once it received funds, even if it was not
// visible before and the keystore is never connected again.
if !hiddenBecauseUnused && backend.config.AppConfig().Backend.Watchonly {
if !hiddenBecauseUnused && accountsConfig.IsKeystoreWatchonly(rootFingerprint) {
t := true
accountWatch = &t
}
Expand Down Expand Up @@ -868,7 +874,7 @@ func (backend *Backend) persistETHAccountConfig(
}

var accountWatch *bool
if !hiddenBecauseUnused && backend.config.AppConfig().Backend.Watchonly {
if !hiddenBecauseUnused && accountsConfig.IsKeystoreWatchonly(rootFingerprint) {
t := true
accountWatch = &t
}
Expand All @@ -887,8 +893,11 @@ func (backend *Backend) persistETHAccountConfig(
// The accountsAndKeystoreLock must be held when calling this function.
func (backend *Backend) initPersistedAccounts() {
// Only load accounts which belong to connected keystores or for which watchonly is enabled.
keystoreConnectedOrWatch := func(account *config.Account) bool {
if account.IsWatch(backend.config.AppConfig().Backend.Watchonly) {
keystoreConnectedOrWatch := func(accountsConfig *config.AccountsConfig, account *config.Account) bool {
isWatch, err := accountsConfig.IsAccountWatchonly(account)
if err != nil {
backend.log.WithError(err).Error("Can't determine watch status of account")
} else if isWatch {
return true
}

Expand Down Expand Up @@ -924,17 +933,24 @@ outer:
// Watch-only accounts are loaded regardless, and if later e.g. a BitBox02 BTC-only is
// inserted with the same seed as a Multi, we will need to catch that mismatch when the
// keystore will be used to e.g. display an Ethereum address etc.
if backend.keystore != nil && !account.IsWatch(backend.config.AppConfig().Backend.Watchonly) {
switch coin.(type) {
case *btc.Coin:
for _, cfg := range account.SigningConfigurations {
if !backend.keystore.SupportsAccount(coin, cfg.ScriptType()) {
continue outer
if backend.keystore != nil {
isWatch, err := persistedAccounts.IsAccountWatchonly(account)
if err != nil {
backend.log.WithError(err).Error("Could not retrieve root fingerprint")
continue
}
if !isWatch {
switch coin.(type) {
case *btc.Coin:
for _, cfg := range account.SigningConfigurations {
if !backend.keystore.SupportsAccount(coin, cfg.ScriptType()) {
continue outer
}
}
default:
if !backend.keystore.SupportsAccount(coin, nil) {
continue
}
}
default:
if !backend.keystore.SupportsAccount(coin, nil) {
continue
}
}
}
Expand Down Expand Up @@ -1056,15 +1072,19 @@ func (backend *Backend) maybeAddP2TR(keystore keystore.Keystore, accounts []*con
// perform migrations, updates etc. We use it to add taproot subaccounts to Bitcoin accounts that
// were created (persisted) before the introduction of taproot support.
func (backend *Backend) updatePersistedAccounts(
keystore keystore.Keystore, accounts []*config.Account) error {
keystore keystore.Keystore, accountsConfig *config.AccountsConfig) error {

// setWatch, if the global Watchonly flag is enabled, sets the `Watch`
// setWatch, if the keystore Watchonly flag is enabled, sets the `Watch`
// flag to `true`, turning this account into a watch-only account.
setWatch := func() error {
if !backend.config.AppConfig().Backend.Watchonly {
rootFingerprint, err := keystore.RootFingerprint()
if err != nil {
return err
}
if !accountsConfig.IsKeystoreWatchonly(rootFingerprint) {
return nil
}
for _, account := range accounts {
for _, account := range accountsConfig.Accounts {
// If the account was added in the background as part of scanning, we don't mark it
// watchonly. Otherwise the account would appear automatically once it received funds,
// even if it was not visible before and the keystore is never connected again.
Expand All @@ -1080,7 +1100,7 @@ func (backend *Backend) updatePersistedAccounts(
return err
}

return backend.maybeAddP2TR(keystore, accounts)
return backend.maybeAddP2TR(keystore, accountsConfig.Accounts)
}

// The accountsAndKeystoreLock must be held when calling this function.
Expand Down Expand Up @@ -1127,7 +1147,12 @@ func (backend *Backend) uninitAccounts(force bool) {
}
}

if !force && (belongsToKeystore || account.Config().Config.IsWatch(backend.config.AppConfig().Backend.Watchonly)) {
isWatchonly, err := backend.config.AccountsConfig().IsAccountWatchonly(account.Config().Config)
if err != nil {
backend.log.WithError(err).Error("could not retrieve keystore fingerprint")
isWatchonly = false
}
if !force && (belongsToKeystore || isWatchonly) {
// Do not uninit/remove account that is being watched.
keep = append(keep, account)
continue
Expand Down Expand Up @@ -1296,8 +1321,12 @@ func (backend *Backend) checkAccountUsed(account accounts.Interface) {
acct.HiddenBecauseUnused = false

// We only really show the account to the user now, so this is the moment to set the
// watchonly flag on it if the user has the global watchonly setting enabled.
if backend.config.AppConfig().Backend.Watchonly {
// watchonly flag on it if the user has the accont's keystore's watchonly setting enabled.
rootFingerprint, err := acct.SigningConfigurations.RootFingerprint()
if err != nil {
return err
}
if accountsConfig.IsKeystoreWatchonly(rootFingerprint) {
t := true
acct.Watch = &t
}
Expand Down
99 changes: 63 additions & 36 deletions backend/accounts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1031,14 +1031,21 @@ func TestAccountSupported(t *testing.T) {
bb02Multi := makeBitBox02Multi()
bb02BtcOnly := makeBitBox02BTCOnly()

rootFingerprint, err := bb02Multi.RootFingerprint()
require.NoError(t, err)

rootFingerprint2, err := bb02BtcOnly.RootFingerprint()
require.NoError(t, err)

require.Equal(t, rootFingerprint, rootFingerprint2)

b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()

require.NoError(t, b.SetWatchonly(true))

// Registering a new keystore persists a set of initial default accounts.
b.registerKeystore(bb02Multi)
checkShownAccountsLen(t, b, 3, 3)
require.NoError(t, b.SetWatchonly(rootFingerprint, true))

b.DeregisterKeystore()
// Registering a Bitcoin-only like keystore loads also the altcoins that were persisted
Expand All @@ -1047,7 +1054,7 @@ func TestAccountSupported(t *testing.T) {
checkShownAccountsLen(t, b, 3, 3)

// If watch-only is disabled, then these will not be loaded if not supported by the keystore.
require.NoError(t, b.SetWatchonly(false))
require.NoError(t, b.SetWatchonly(rootFingerprint, false))
b.DeregisterKeystore()

// Registering a Bitcoin-only like keystore loads only the Bitcoin account, even though altcoins
Expand Down Expand Up @@ -1302,26 +1309,20 @@ func TestWatchonly(t *testing.T) {
checkShownAccountsLen(t, b, 0, 3)
})

// Watchonly enabled before keystore is registered - all loaded accounts thereafter become
// watched.
t.Run("", func(t *testing.T) {
b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()
require.NoError(t, b.SetWatchonly(true))
b.registerKeystore(makeBitBox02Multi())
checkShownAccountsLen(t, b, 3, 3)
b.DeregisterKeystore()
// Accounts remain loaded.
checkShownAccountsLen(t, b, 3, 3)
})

// Watchonly enabled while keystore is registered - all already loaded accounts become watched.
t.Run("", func(t *testing.T) {
b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()
b.registerKeystore(makeBitBox02Multi())

ks := makeBitBox02Multi()

rootFingerprint, err := ks.RootFingerprint()
require.NoError(t, err)

b.registerKeystore(ks)
checkShownAccountsLen(t, b, 3, 3)
require.NoError(t, b.SetWatchonly(true))
require.NoError(t, b.SetWatchonly(rootFingerprint, true))

b.DeregisterKeystore()
// Accounts remain loaded.
checkShownAccountsLen(t, b, 3, 3)
Expand All @@ -1331,8 +1332,15 @@ func TestWatchonly(t *testing.T) {
t.Run("", func(t *testing.T) {
b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()
require.NoError(t, b.SetWatchonly(true))
b.registerKeystore(makeBitBox02Multi())

ks := makeBitBox02Multi()

rootFingerprint, err := ks.RootFingerprint()
require.NoError(t, err)

b.registerKeystore(ks)
require.NoError(t, b.SetWatchonly(rootFingerprint, true))

checkShownAccountsLen(t, b, 3, 3)
exclude := accountsTypes.Code("v0-55555555-btc-0")
require.NotNil(t, lookup(b.Accounts(), exclude))
Expand All @@ -1351,59 +1359,78 @@ func TestWatchonly(t *testing.T) {
require.NotNil(t, lookup(b.Accounts(), exclude))
})

// Watchonly is disabled while some watched accounts are shown with no keystore connected. All
// accounts should disappear. When re-enabling watchonly, they do not reappear - connecting the
// keystore again is necessary.
// Watchonly of a keystore is disabled while some watched accounts are shown with no keystore
// connected. All accounts of the keystore should disappear. When re-enabling watchonly, they
// do not reappear - connecting the keystore again is necessary.
t.Run("", func(t *testing.T) {
b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()
require.NoError(t, b.SetWatchonly(true))
b.registerKeystore(makeBitBox02Multi())

ks := makeBitBox02Multi()

rootFingerprint, err := ks.RootFingerprint()
require.NoError(t, err)

b.registerKeystore(ks)
require.NoError(t, b.SetWatchonly(rootFingerprint, true))

b.DeregisterKeystore()
// Accounts remain loaded.
checkShownAccountsLen(t, b, 3, 3)

// Disable watchonly, all accounts disappear.
require.NoError(t, b.SetWatchonly(false))
require.NoError(t, b.SetWatchonly(rootFingerprint, false))
checkShownAccountsLen(t, b, 0, 3)

// Re-enable watchonly - accounts do not show up yet.
require.NoError(t, b.SetWatchonly(true))
require.NoError(t, b.SetWatchonly(rootFingerprint, true))
checkShownAccountsLen(t, b, 0, 3)

// Reconnecting the keystore brings back the watched accounts.
b.registerKeystore(makeBitBox02Multi())
b.registerKeystore(ks)
checkShownAccountsLen(t, b, 3, 3)
b.DeregisterKeystore()
checkShownAccountsLen(t, b, 3, 3)
})

// Disable global watchonly while keystore is connected does not make the accounts disappear
// yet. They only disappear once the keytore is disconnected.
// Disable keystore's watchonly setting while keystore is connected does not make the accounts
// disappear yet. They only disappear once the keytore is disconnected.
t.Run("", func(t *testing.T) {
b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()
require.NoError(t, b.SetWatchonly(true))
b.registerKeystore(makeBitBox02Multi())

ks := makeBitBox02Multi()

rootFingerprint, err := ks.RootFingerprint()
require.NoError(t, err)

b.registerKeystore(ks)
checkShownAccountsLen(t, b, 3, 3)
require.NoError(t, b.SetWatchonly(rootFingerprint, true))

// Disable watchonly, all accounts remain as the keystore is still connected.
require.NoError(t, b.SetWatchonly(false))
require.NoError(t, b.SetWatchonly(rootFingerprint, false))
checkShownAccountsLen(t, b, 3, 3)

// Accounts disappear when the keystore is disconnected.
b.DeregisterKeystore()
checkShownAccountsLen(t, b, 0, 3)
})

// Disable watchonly for a specific account while keystore is connected does not make the
// account disappear yet. It only disappears once the keytore is disconnected.
// Disable watchonly for a specific account while its keystore is connected does not make the
// account disappear yet. It only disappears once the keystore is disconnected.
t.Run("", func(t *testing.T) {
b := newBackend(t, testnetDisabled, regtestDisabled)
defer b.Close()
require.NoError(t, b.SetWatchonly(true))

ks := makeBitBox02Multi()

rootFingerprint, err := ks.RootFingerprint()
require.NoError(t, err)

b.registerKeystore(makeBitBox02Multi())
checkShownAccountsLen(t, b, 3, 3)
require.NoError(t, b.SetWatchonly(rootFingerprint, true))

exclude := accountsTypes.Code("v0-55555555-btc-0")
require.NotNil(t, lookup(b.Accounts(), exclude))
Expand Down
20 changes: 12 additions & 8 deletions backend/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ func (backend *Backend) registerKeystore(keystore keystore.Keystore) {
Action: action.Reload,
})

belongsToKeystore := func(account *config.Account) bool {
belongsToKeystore := func(_ *config.AccountsConfig, account *config.Account) bool {
return account.SigningConfigurations.ContainsRootFingerprint(fingerprint)
}

Expand All @@ -640,7 +640,7 @@ func (backend *Backend) registerKeystore(keystore keystore.Keystore) {
// needed on the persisted accounts.
accounts := backend.filterAccounts(accountsConfig, belongsToKeystore)
if len(accounts) != 0 {
return backend.updatePersistedAccounts(keystore, accounts)
return backend.updatePersistedAccounts(keystore, accountsConfig)
}
return backend.persistDefaultAccountConfigs(keystore, accountsConfig)
})
Expand Down Expand Up @@ -886,12 +886,16 @@ func (backend *Backend) CancelConnectKeystore() {
backend.connectKeystore.cancel(errUserAbort)
}

// SetWatchonly sets the app config's watchonly flag to `watchonly`.
// When enabling watchonly, all currently loaded accounts are turned into watchonly accounts.
// When disabling watchonly, all persisted accounts's watchonly status is reset.
func (backend *Backend) SetWatchonly(watchonly bool) error {
err := backend.config.ModifyAppConfig(func(config *config.AppConfig) error {
config.Backend.Watchonly = watchonly
// SetWatchonly sets the keystore's watchonly flag to `watchonly`.
// When enabling watchonly, all currently loaded accounts of that keystore are turned into watchonly accounts.
// When disabling watchonly, all the watchonly status of all of the keystore's persisted accounts is reset.
func (backend *Backend) SetWatchonly(rootFingerprint []byte, watchonly bool) error {
err := backend.config.ModifyAccountsConfig(func(config *config.AccountsConfig) error {
ks, err := config.LookupKeystore(rootFingerprint)
if err != nil {
return err
}
ks.Watchonly = watchonly
return nil
})
if err != nil {
Expand Down
Loading

0 comments on commit 0de1abc

Please sign in to comment.