Skip to content

Commit b4f2c9d

Browse files
committed
watchonly: add global watchonly setting
With option to exempt individual accounts. - When global watchonly is enabled, all currently loaded accounts and accounts loaded in the future will be set to watchonly. Accounts that had been persisted beforehand are not automatically set to watch-only. - When global watchonly is disabled, all persisted accounts' watchonly status is reset. This way, when one enables watchonly again, accounts that are already persisted don't immediately appear - one has to first load them with a keystore.
1 parent 6b8ec3c commit b4f2c9d

File tree

12 files changed

+258
-78
lines changed

12 files changed

+258
-78
lines changed

backend/accounts.go

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,6 @@ import (
3737
"github.com/ethereum/go-ethereum/params"
3838
)
3939

40-
// Set the `watch` setting on new accounts to this default value.
41-
// For now we keep it unset, and have users opt-in to watching specific accounts.
42-
//
43-
// We set it to `nil` not `false` so that we reserve the possibility to default all accounts to
44-
// watch-only for accounts where the user hasn't made an active decision (e.g. turn on watch-only
45-
// for all accounts of a keystore if the user did not activate/deactivate watch-only manually on any
46-
// of them).
47-
var defaultWatch *bool = nil
48-
4940
// hardenedKeystart is the BIP44 offset to make a keypath element hardened.
5041
const hardenedKeystart uint32 = hdkeychain.HardenedKeyStart
5142

@@ -488,16 +479,28 @@ func (backend *Backend) RenameAccount(accountCode accountsTypes.Code, name strin
488479
return nil
489480
}
490481

482+
// copyBool makes a copy, so that multiple values do not share the same reference. This avoids
483+
// potential future bugs if someone modified a flag like `*account.Watch = X`,
484+
// accidentally changing the value for many accounts that share the same reference.
485+
func copyBool(b *bool) *bool {
486+
if b == nil {
487+
return nil
488+
}
489+
cpy := *b
490+
return &cpy
491+
}
492+
491493
// AccountSetWatch sets the account's persisted watch flag to `watch`. Set to `true` if the account
492494
// should be loaded even if its keystore is not connected.
493495
// If `watch` is set to `false`, the account is unloaded and the frontend notified.
494-
func (backend *Backend) AccountSetWatch(accountCode accountsTypes.Code, watch bool) error {
496+
func (backend *Backend) AccountSetWatch(filter func(*config.Account) bool, watch *bool) error {
495497
err := backend.config.ModifyAccountsConfig(func(accountsConfig *config.AccountsConfig) error {
496-
acct := accountsConfig.Lookup(accountCode)
497-
if acct == nil {
498-
return errp.Newf("Could not find account %s", accountCode)
498+
for _, acct := range accountsConfig.Accounts {
499+
if !filter(acct) {
500+
continue
501+
}
502+
acct.Watch = copyBool(watch)
499503
}
500-
acct.Watch = &watch
501504
return nil
502505
})
503506
if err != nil {
@@ -510,18 +513,18 @@ func (backend *Backend) AccountSetWatch(accountCode accountsTypes.Code, watch bo
510513
//
511514
// This ensures that removing a keystore after setting an ETH account including tokens to
512515
// watchonly results in the account *and* the tokens remaining loaded.
513-
if acct := backend.accounts.lookup(accountCode); acct != nil {
516+
for _, acct := range backend.accounts {
514517
if acct.Config().Config.CoinCode == coinpkg.CodeETH {
515518
for _, erc20TokenCode := range acct.Config().Config.ActiveTokens {
516-
erc20AccountCode := Erc20AccountCode(accountCode, erc20TokenCode)
519+
erc20AccountCode := Erc20AccountCode(acct.Config().Config.Code, erc20TokenCode)
517520
if tokenAcct := backend.accounts.lookup(erc20AccountCode); tokenAcct != nil {
518-
tokenAcct.Config().Config.Watch = &watch
521+
tokenAcct.Config().Config.Watch = copyBool(watch)
519522
}
520523
}
521524
}
522525
}
523526

524-
if !watch {
527+
if watch == nil || !*watch {
525528
backend.initAccounts(false)
526529
backend.emitAccountsStatusChanged()
527530
}
@@ -753,6 +756,12 @@ func (backend *Backend) persistBTCAccountConfig(
753756
return err
754757
}
755758

759+
var accountWatch *bool
760+
if backend.config.AppConfig().Backend.Watchonly {
761+
t := true
762+
accountWatch = &t
763+
}
764+
756765
var signingConfigurations signing.Configurations
757766
for _, cfg := range supportedConfigs {
758767
extendedPublicKey, err := keystore.ExtendedPublicKey(coin, cfg.keypath)
@@ -774,7 +783,7 @@ func (backend *Backend) persistBTCAccountConfig(
774783
if keystore.SupportsUnifiedAccounts() {
775784
return backend.persistAccount(config.Account{
776785
HiddenBecauseUnused: hiddenBecauseUnused,
777-
Watch: defaultWatch,
786+
Watch: accountWatch,
778787
CoinCode: coin.Code(),
779788
Name: name,
780789
Code: code,
@@ -794,7 +803,7 @@ func (backend *Backend) persistBTCAccountConfig(
794803

795804
err := backend.persistAccount(config.Account{
796805
HiddenBecauseUnused: hiddenBecauseUnused,
797-
Watch: defaultWatch,
806+
Watch: copyBool(accountWatch),
798807
CoinCode: coin.Code(),
799808
Name: suffixedName,
800809
Code: splitAccountCode(code, cfg.ScriptType()),
@@ -849,9 +858,15 @@ func (backend *Backend) persistETHAccountConfig(
849858
),
850859
}
851860

861+
var accountWatch *bool
862+
if backend.config.AppConfig().Backend.Watchonly {
863+
t := true
864+
accountWatch = &t
865+
}
866+
852867
return backend.persistAccount(config.Account{
853868
HiddenBecauseUnused: hiddenBecauseUnused,
854-
Watch: defaultWatch,
869+
Watch: accountWatch,
855870
CoinCode: coin.Code(),
856871
Name: name,
857872
Code: code,
@@ -864,7 +879,7 @@ func (backend *Backend) persistETHAccountConfig(
864879
func (backend *Backend) initPersistedAccounts() {
865880
// Only load accounts which belong to connected keystores or for which watchonly is enabled.
866881
keystoreConnectedOrWatch := func(account *config.Account) bool {
867-
if account.IsWatch() {
882+
if account.IsWatch(backend.config.AppConfig().Backend.Watchonly) {
868883
return true
869884
}
870885

@@ -900,7 +915,7 @@ outer:
900915
// Watch-only accounts are loaded regardless, and if later e.g. a BitBox02 BTC-only is
901916
// inserted with the same seed as a Multi, we will need to catch that mismatch when the
902917
// keystore will be used to e.g. display an Ethereum address etc.
903-
if backend.keystore != nil && !account.IsWatch() {
918+
if backend.keystore != nil && !account.IsWatch(backend.config.AppConfig().Backend.Watchonly) {
904919
switch coin.(type) {
905920
case *btc.Coin:
906921
for _, cfg := range account.SigningConfigurations {
@@ -1033,6 +1048,26 @@ func (backend *Backend) maybeAddP2TR(keystore keystore.Keystore, accounts []*con
10331048
// were created (persisted) before the introduction of taproot support.
10341049
func (backend *Backend) updatePersistedAccounts(
10351050
keystore keystore.Keystore, accounts []*config.Account) error {
1051+
1052+
// setWatch, if the global Watchonly flag is enabled, sets the `Watch`
1053+
// flag to `true`, turning this account into a watch-only account.
1054+
setWatch := func() error {
1055+
if !backend.config.AppConfig().Backend.Watchonly {
1056+
return nil
1057+
}
1058+
for _, account := range accounts {
1059+
if account.Watch == nil {
1060+
t := true
1061+
account.Watch = &t
1062+
}
1063+
}
1064+
return nil
1065+
}
1066+
1067+
if err := setWatch(); err != nil {
1068+
return err
1069+
}
1070+
10361071
return backend.maybeAddP2TR(keystore, accounts)
10371072
}
10381073

backend/accounts_test.go

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,18 +1046,12 @@ func TestAccountSupported(t *testing.T) {
10461046
b := newBackend(t, testnetDisabled, regtestDisabled)
10471047
defer b.Close()
10481048

1049+
require.NoError(t, b.SetWatchonly(true))
1050+
10491051
// Registering a new keystore persists a set of initial default accounts.
10501052
b.registerKeystore(bb02Multi)
10511053
require.Len(t, b.Accounts(), 3)
10521054
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1053-
// Mark all as watch-only.
1054-
require.NoError(t, b.config.ModifyAccountsConfig(func(cfg *config.AccountsConfig) error {
1055-
for _, acct := range cfg.Accounts {
1056-
f := true
1057-
acct.Watch = &f
1058-
}
1059-
return nil
1060-
}))
10611055

10621056
b.DeregisterKeystore()
10631057
// Registering a Bitcoin-only like keystore loads also the altcoins that were persisted
@@ -1066,15 +1060,10 @@ func TestAccountSupported(t *testing.T) {
10661060
require.Len(t, b.Accounts(), 3)
10671061
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
10681062

1069-
b.DeregisterKeystore()
10701063
// If watch-only is disabled, then these will not be loaded if not supported by the keystore.
1071-
require.NoError(t, b.config.ModifyAccountsConfig(func(cfg *config.AccountsConfig) error {
1072-
for _, acct := range cfg.Accounts {
1073-
f := false
1074-
acct.Watch = &f
1075-
}
1076-
return nil
1077-
}))
1064+
require.NoError(t, b.SetWatchonly(false))
1065+
b.DeregisterKeystore()
1066+
10781067
// Registering a Bitcoin-only like keystore loads only the Bitcoin account, even though altcoins
10791068
// were persisted previously.
10801069
b.registerKeystore(bb02BtcOnly)

backend/backend.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,3 +789,39 @@ func (backend *Backend) GetAccountFromCode(code string) (accounts.Interface, err
789789
func (backend *Backend) CancelConnectKeystore() {
790790
backend.connectKeystore.cancel(errUserAbort)
791791
}
792+
793+
// SetWatchonly sets the app config's watchonly flag to `watchonly`.
794+
// When enabling watchonly, all currently loaded accounts are turned into watchonly accounts.
795+
// When disabling watchonly, all persisted accounts's watchonly status is reset.
796+
func (backend *Backend) SetWatchonly(watchonly bool) error {
797+
err := backend.config.ModifyAppConfig(func(config *config.AppConfig) error {
798+
config.Backend.Watchonly = watchonly
799+
return nil
800+
})
801+
if err != nil {
802+
return err
803+
}
804+
805+
if !watchonly {
806+
// When disabling watchonly, we reset the Watch flag for each account, so that when the user
807+
// enables watchonly again, it does not show all accounts again - they first need to be
808+
// loaded via their keystore.
809+
return backend.AccountSetWatch(
810+
func(*config.Account) bool {
811+
// Apply to each account.
812+
return true
813+
},
814+
nil,
815+
)
816+
}
817+
// When enabling watchonly, we turn the currently loaded accounts into watch-only accounts.
818+
t := true
819+
return backend.AccountSetWatch(
820+
func(account *config.Account) bool {
821+
// Apply to each currently loaded account.
822+
defer backend.accountsAndKeystoreLock.RLock()()
823+
return backend.accounts.lookup(account.Code) != nil
824+
},
825+
&t,
826+
)
827+
}

backend/backend_test.go

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,8 @@ func TestRegisterKeystore(t *testing.T) {
120120
// Deregistering the keystore leaves the loaded accounts (watchonly), and leaves the persisted
121121
// accounts and keystores.
122122
// Mark accounts as watch-only.
123-
require.NoError(t, b.config.ModifyAccountsConfig(func(cfg *config.AccountsConfig) error {
124-
for _, acct := range cfg.Accounts {
125-
f := true
126-
acct.Watch = &f
127-
}
128-
return nil
129-
}))
123+
require.NoError(t, b.SetWatchonly(true))
124+
130125
b.DeregisterKeystore()
131126
require.Len(t, b.Accounts(), 3)
132127
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
@@ -156,14 +151,10 @@ func TestRegisterKeystore(t *testing.T) {
156151
require.Equal(t, rootFingerprint2, []byte(b.Config().AccountsConfig().Keystores[1].RootFingerprint))
157152

158153
b.DeregisterKeystore()
159-
// Enable watch-only for all but two accounts, one of each keystore. Now, all watch-only
160-
// accounts plus the non-watch only accounts of the connected keystore will be loaded.
161-
require.NoError(t, b.config.ModifyAccountsConfig(func(cfg *config.AccountsConfig) error {
162-
for _, acct := range cfg.Accounts {
163-
t := true
164-
acct.Watch = &t
165-
}
166154

155+
// Disable watch-only for two accounts, one of each keystore. Now, all accounts plus the
156+
// non-watch only accounts of the connected keystore will be loaded.
157+
require.NoError(t, b.config.ModifyAccountsConfig(func(cfg *config.AccountsConfig) error {
167158
f := false
168159
cfg.Lookup("v0-55555555-btc-0").Watch = &f
169160
cfg.Lookup("v0-66666666-ltc-0").Watch = &f

backend/config/accounts.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ type Account struct {
4242
// If false, the account is only displayed if the keystore is connected. If true, it is loaded
4343
// and displayed when the app launches.
4444
//
45-
// If nil, it is considered false.
45+
// If nil, it is considered false. The reason for this is that we don't want to suddenly show
46+
// all persisted accounts when the Watchonly setting is enabled - only accounts that are loaded
47+
// when Watchonly is enabled should do this.
4648
Watch *bool `json:"watch"`
4749
CoinCode coin.Code `json:"coinCode"`
4850
Name string `json:"name"`
@@ -73,9 +75,10 @@ func (acct *Account) SetTokenActive(tokenCode string, active bool) error {
7375
return nil
7476
}
7577

76-
// IsWatchOnly returns true if the `Watch` setting is set to true.
77-
func (acct *Account) IsWatch() bool {
78-
return acct.Watch != nil && *acct.Watch
78+
// IsWatch returns true if the `Watch` setting is set to true and `defaultWatchonly` is true. For
79+
// `defaultWatchonly`, you should provide the global watchonly setting from the backend config.
80+
func (acct *Account) IsWatch(defaultWatchonly bool) bool {
81+
return defaultWatchonly && acct.Watch != nil && *acct.Watch
7982
}
8083

8184
// Keystore holds information related to keystores such as the BitBox02.

backend/config/config.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ type Backend struct {
100100

101101
// BtcUnit is the unit used to represent Bitcoin amounts. See `coin.BtcUnit` for details.
102102
BtcUnit coin.BtcUnit `json:"btcUnit"`
103+
104+
// Watchonly determines if accounts should be loaded even if their keystore is not conneced.
105+
// If false, accounts are only loaded if their keystore is connected.
106+
// If true, they are loaded and displayed when the app launches.
107+
// Individual accounts can be exempt from being loaded even if this flag is true by setting the account's
108+
// `Watch` flag to false.
109+
Watchonly bool `json:"watchonly"`
103110
}
104111

105112
// DeprecatedCoinActive returns the Active setting for a coin by code. This call is should not be
@@ -325,6 +332,16 @@ func (config *Config) SetAppConfig(appConfig AppConfig) error {
325332
return config.save(config.appConfigFilename, config.appConfig)
326333
}
327334

335+
// ModifyAppConfig calls f with the current config, allowing f to make any changes, and
336+
// persists the result if f returns nil error. It propagates the f's error as is.
337+
func (config *Config) ModifyAppConfig(f func(*AppConfig) error) error {
338+
defer config.appConfigLock.Lock()()
339+
if err := f(&config.appConfig); err != nil {
340+
return err
341+
}
342+
return config.save(config.appConfigFilename, config.appConfig)
343+
}
344+
328345
// AccountsConfig returns the accounts config.
329346
func (config *Config) AccountsConfig() AccountsConfig {
330347
defer config.accountsConfigLock.RLock()()

0 commit comments

Comments
 (0)