Skip to content

Commit 240c34e

Browse files
committed
Merge remote-tracking branch 'benma/watchonly-toggle' into staging-watchonly
2 parents ba8f32f + bfcbfeb commit 240c34e

File tree

10 files changed

+279
-41
lines changed

10 files changed

+279
-41
lines changed

backend/accounts.go

Lines changed: 112 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,15 @@ import (
3636
"github.com/ethereum/go-ethereum/params"
3737
)
3838

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

@@ -478,6 +487,46 @@ func (backend *Backend) RenameAccount(accountCode accountsTypes.Code, name strin
478487
return nil
479488
}
480489

490+
// AccountSetWatch sets the account's persisted watch flag to `watch`. Set to `true` if the account
491+
// should be loaded even if its keystore is not connected.
492+
// If `watch` is set to `false`, the account is unloaded and the frontend notified.
493+
func (backend *Backend) AccountSetWatch(accountCode accountsTypes.Code, watch bool) error {
494+
err := backend.config.ModifyAccountsConfig(func(accountsConfig *config.AccountsConfig) error {
495+
acct := accountsConfig.Lookup(accountCode)
496+
if acct == nil {
497+
return errp.Newf("Could not find account %s", accountCode)
498+
}
499+
acct.Watch = &watch
500+
return nil
501+
})
502+
if err != nil {
503+
return err
504+
}
505+
defer backend.accountsAndKeystoreLock.Lock()()
506+
507+
// ETH tokens inherit the Watch-flag from the parent ETH account.
508+
// If the watch status of an ETH account was changed, we update it for its tokens as well.
509+
//
510+
// This ensures that removing a keystore after setting an ETH account including tokens to
511+
// watchonly results in the account *and* the tokens remaining loaded.
512+
if acct := backend.accounts.lookup(accountCode); acct != nil {
513+
if acct.Config().Config.CoinCode == coinpkg.CodeETH {
514+
for _, erc20TokenCode := range acct.Config().Config.ActiveTokens {
515+
erc20AccountCode := Erc20AccountCode(accountCode, erc20TokenCode)
516+
if tokenAcct := backend.accounts.lookup(erc20AccountCode); tokenAcct != nil {
517+
tokenAcct.Config().Config.Watch = &watch
518+
}
519+
}
520+
}
521+
}
522+
523+
if !watch {
524+
backend.initAccounts(false)
525+
backend.emitAccountsStatusChanged()
526+
}
527+
return nil
528+
}
529+
481530
// addAccount adds the given account to the backend.
482531
// The accountsAndKeystoreLock must be held when calling this function.
483532
func (backend *Backend) addAccount(account accounts.Interface) {
@@ -493,6 +542,10 @@ func (backend *Backend) addAccount(account accounts.Interface) {
493542

494543
// The accountsAndKeystoreLock must be held when calling this function.
495544
func (backend *Backend) createAndAddAccount(coin coinpkg.Coin, persistedConfig *config.Account) {
545+
if backend.accounts.lookup(persistedConfig.Code) != nil {
546+
// Do not create/load account if it is already loaded.
547+
return
548+
}
496549
var account accounts.Interface
497550
accountConfig := &accounts.AccountConfig{
498551
Config: persistedConfig,
@@ -549,9 +602,15 @@ func (backend *Backend) createAndAddAccount(coin coinpkg.Coin, persistedConfig *
549602
tokenName = fmt.Sprintf("%s %d", tokenName, accountNumber+1)
550603
}
551604

605+
var watchToken *bool
606+
if persistedConfig.Watch != nil {
607+
wCopy := *persistedConfig.Watch
608+
watchToken = &wCopy
609+
}
552610
erc20Config := &config.Account{
553611
Inactive: persistedConfig.Inactive,
554612
HiddenBecauseUnused: persistedConfig.HiddenBecauseUnused,
613+
Watch: watchToken,
555614
CoinCode: erc20CoinCode,
556615
Name: tokenName,
557616
Code: erc20AccountCode,
@@ -655,6 +714,7 @@ func (backend *Backend) persistBTCAccountConfig(
655714
if keystore.SupportsUnifiedAccounts() {
656715
return backend.persistAccount(config.Account{
657716
HiddenBecauseUnused: hiddenBecauseUnused,
717+
Watch: defaultWatch,
658718
CoinCode: coin.Code(),
659719
Name: name,
660720
Code: code,
@@ -674,6 +734,7 @@ func (backend *Backend) persistBTCAccountConfig(
674734

675735
err := backend.persistAccount(config.Account{
676736
HiddenBecauseUnused: hiddenBecauseUnused,
737+
Watch: defaultWatch,
677738
CoinCode: coin.Code(),
678739
Name: suffixedName,
679740
Code: splitAccountCode(code, cfg.ScriptType()),
@@ -730,6 +791,7 @@ func (backend *Backend) persistETHAccountConfig(
730791

731792
return backend.persistAccount(config.Account{
732793
HiddenBecauseUnused: hiddenBecauseUnused,
794+
Watch: defaultWatch,
733795
CoinCode: coin.Code(),
734796
Name: name,
735797
Code: code,
@@ -740,39 +802,56 @@ func (backend *Backend) persistETHAccountConfig(
740802

741803
// The accountsAndKeystoreLock must be held when calling this function.
742804
func (backend *Backend) initPersistedAccounts() {
743-
if backend.keystore == nil {
744-
return
745-
}
746-
// Only load accounts which belong to connected keystores.
747-
rootFingerprint, err := backend.keystore.RootFingerprint()
748-
if err != nil {
749-
backend.log.WithError(err).Error("Could not retrieve root fingerprint")
750-
return
751-
}
752-
keystoreConnected := func(account *config.Account) bool {
805+
// Only load accounts which belong to connected keystores or for which watchonly is enabled.
806+
keystoreConnectedOrWatch := func(account *config.Account) bool {
807+
if account.IsWatch() {
808+
return true
809+
}
810+
811+
if backend.keystore == nil {
812+
return false
813+
}
814+
rootFingerprint, err := backend.keystore.RootFingerprint()
815+
if err != nil {
816+
backend.log.WithError(err).Error("Could not retrieve root fingerprint")
817+
return false
818+
}
819+
753820
return account.SigningConfigurations.ContainsRootFingerprint(rootFingerprint)
754821
}
755822

756823
persistedAccounts := backend.config.AccountsConfig()
824+
825+
// In this loop, we add all accounts that match the filter, except for the ones whose signing
826+
// configuration is not supported by the connected keystore. The latter can happen for example
827+
// if a user connects a BitBox02 Multi edition first, which persists some altcoin accounts, and
828+
// then connects a BitBox02 BTC-only with the same seed. In that case, the unsupported accounts
829+
// will not be loaded, unless they have been marked as watch-only.
757830
outer:
758-
for _, account := range backend.filterAccounts(&persistedAccounts, keystoreConnected) {
831+
for _, account := range backend.filterAccounts(&persistedAccounts, keystoreConnectedOrWatch) {
759832
account := account
760833
coin, err := backend.Coin(account.CoinCode)
761834
if err != nil {
762835
backend.log.Errorf("skipping persisted account %s/%s, could not find coin",
763836
account.CoinCode, account.Code)
764837
continue
765838
}
766-
switch coin.(type) {
767-
case *btc.Coin:
768-
for _, cfg := range account.SigningConfigurations {
769-
if !backend.keystore.SupportsAccount(coin, cfg.ScriptType()) {
770-
continue outer
839+
840+
// Watch-only accounts are loaded regardless, and if later e.g. a BitBox02 BTC-only is
841+
// inserted with the same seed as a Multi, we will need to catch that mismatch when the
842+
// keystore will be used to e.g. display an Ethereum address etc.
843+
if backend.keystore != nil && !account.IsWatch() {
844+
switch coin.(type) {
845+
case *btc.Coin:
846+
for _, cfg := range account.SigningConfigurations {
847+
if !backend.keystore.SupportsAccount(coin, cfg.ScriptType()) {
848+
continue outer
849+
}
850+
}
851+
default:
852+
if !backend.keystore.SupportsAccount(coin, nil) {
853+
continue
771854
}
772-
}
773-
default:
774-
if !backend.keystore.SupportsAccount(coin, nil) {
775-
continue
776855
}
777856
}
778857

@@ -898,9 +977,10 @@ func (backend *Backend) updatePersistedAccounts(
898977
}
899978

900979
// The accountsAndKeystoreLock must be held when calling this function.
901-
func (backend *Backend) initAccounts() {
980+
// if force is true, all accounts are uninitialized first, even if they are watch-only.
981+
func (backend *Backend) initAccounts(force bool) {
902982
// Since initAccounts replaces all previous accounts, we need to properly close them first.
903-
backend.uninitAccounts()
983+
backend.uninitAccounts(force)
904984

905985
backend.initPersistedAccounts()
906986

@@ -920,19 +1000,26 @@ func (backend *Backend) ReinitializeAccounts() {
9201000
defer backend.accountsAndKeystoreLock.Lock()()
9211001

9221002
backend.log.Info("Reinitializing accounts")
923-
backend.initAccounts()
1003+
backend.initAccounts(true)
9241004
}
9251005

9261006
// The accountsAndKeystoreLock must be held when calling this function.
927-
func (backend *Backend) uninitAccounts() {
1007+
// if force is true, all accounts are uninitialized, even if they are watch-only.
1008+
func (backend *Backend) uninitAccounts(force bool) {
1009+
keep := []accounts.Interface{}
9281010
for _, account := range backend.accounts {
9291011
account := account
1012+
if !force && account.Config().Config.IsWatch() {
1013+
// Do not uninit/remove account that is being watched.
1014+
keep = append(keep, account)
1015+
continue
1016+
}
9301017
if backend.onAccountUninit != nil {
9311018
backend.onAccountUninit(account)
9321019
}
9331020
account.Close()
9341021
}
935-
backend.accounts = []accounts.Interface{}
1022+
backend.accounts = keep
9361023
}
9371024

9381025
// maybeAddHiddenUnusedAccounts adds a hidden account for scanning to facilitate accounts discovery.

backend/accounts_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
561561

562562
require.Equal(t,
563563
&config.Account{
564+
Watch: nil,
564565
CoinCode: "btc",
565566
Name: "bitcoin 2",
566567
Code: "v0-55555555-btc-1",
@@ -583,6 +584,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
583584
require.Equal(t, "v0-55555555-ltc-1", string(acctCode))
584585
require.Equal(t,
585586
&config.Account{
587+
Watch: nil,
586588
CoinCode: "ltc",
587589
Name: "litecoin 2",
588590
Code: "v0-55555555-ltc-1",
@@ -604,6 +606,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
604606
require.Equal(t, "v0-55555555-eth-1", string(acctCode))
605607
require.Equal(t,
606608
&config.Account{
609+
Watch: nil,
607610
CoinCode: "eth",
608611
Name: "ethereum 2",
609612
Code: "v0-55555555-eth-1",
@@ -624,6 +627,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
624627
require.Equal(t, "v0-55555555-btc-2", string(acctCode))
625628
require.Equal(t,
626629
&config.Account{
630+
Watch: nil,
627631
CoinCode: "btc",
628632
Name: "bitcoin 3",
629633
Code: "v0-55555555-btc-2",
@@ -646,6 +650,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
646650
require.Equal(t, "v0-55555555-ltc-2", string(acctCode))
647651
require.Equal(t,
648652
&config.Account{
653+
Watch: nil,
649654
CoinCode: "ltc",
650655
Name: "litecoin 2",
651656
Code: "v0-55555555-ltc-2",
@@ -667,6 +672,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
667672
require.Equal(t, "v0-55555555-eth-2", string(acctCode))
668673
require.Equal(t,
669674
&config.Account{
675+
Watch: nil,
670676
CoinCode: "eth",
671677
Name: "ethereum 2",
672678
Code: "v0-55555555-eth-2",
@@ -682,6 +688,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
682688
require.Equal(t,
683689
&config.Account{
684690
HiddenBecauseUnused: true,
691+
Watch: nil,
685692
CoinCode: "btc",
686693
Name: "Bitcoin 4",
687694
Code: "v0-55555555-btc-3",
@@ -704,6 +711,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
704711
require.Equal(t, "v0-55555555-btc-3", string(acctCode))
705712
require.Equal(t,
706713
&config.Account{
714+
Watch: nil,
707715
CoinCode: "btc",
708716
Name: "bitcoin 4 new name",
709717
Code: "v0-55555555-btc-3",
@@ -734,6 +742,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
734742
require.Equal(t, "v0-55555555-btc-0", string(acctCode))
735743
require.Equal(t,
736744
&config.Account{
745+
Watch: nil,
737746
CoinCode: "btc",
738747
Name: "bitcoin 1: bech32",
739748
Code: "v0-55555555-btc-0-p2wpkh",
@@ -745,6 +754,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
745754
)
746755
require.Equal(t,
747756
&config.Account{
757+
Watch: nil,
748758
CoinCode: "btc",
749759
Name: "bitcoin 1",
750760
Code: "v0-55555555-btc-0-p2wpkh-p2sh",
@@ -756,6 +766,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
756766
)
757767
require.Equal(t,
758768
&config.Account{
769+
Watch: nil,
759770
CoinCode: "btc",
760771
Name: "bitcoin 1: legacy",
761772
Code: "v0-55555555-btc-0-p2pkh",
@@ -777,6 +788,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
777788
require.Equal(t, "v0-55555555-ltc-0", string(acctCode))
778789
require.Equal(t,
779790
&config.Account{
791+
Watch: nil,
780792
CoinCode: "ltc",
781793
Name: "litecoin 1: bech32",
782794
Code: "v0-55555555-ltc-0-p2wpkh",
@@ -788,6 +800,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
788800
)
789801
require.Equal(t,
790802
&config.Account{
803+
Watch: nil,
791804
CoinCode: "ltc",
792805
Name: "litecoin 1",
793806
Code: "v0-55555555-ltc-0-p2wpkh-p2sh",
@@ -1037,8 +1050,31 @@ func TestAccountSupported(t *testing.T) {
10371050
b.registerKeystore(bb02Multi)
10381051
require.Len(t, b.Accounts(), 3)
10391052
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+
}))
10401061

10411062
b.DeregisterKeystore()
1063+
// Registering a Bitcoin-only like keystore loads also the altcoins that were persisted
1064+
// previously, because they are marked watch-only, so they should be visible.
1065+
b.registerKeystore(bb02BtcOnly)
1066+
require.Len(t, b.Accounts(), 3)
1067+
require.Len(t, b.Config().AccountsConfig().Accounts, 3)
1068+
1069+
b.DeregisterKeystore()
1070+
// 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+
}))
10421078
// Registering a Bitcoin-only like keystore loads only the Bitcoin account, even though altcoins
10431079
// were persisted previously.
10441080
b.registerKeystore(bb02BtcOnly)

backend/backend.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ func (backend *Backend) registerKeystore(keystore keystore.Keystore) {
556556
log.WithError(err).Error("Could not persist default accounts")
557557
}
558558

559-
backend.initAccounts()
559+
backend.initAccounts(false)
560560

561561
backend.aoppKeystoreRegistered()
562562
}
@@ -578,7 +578,7 @@ func (backend *Backend) DeregisterKeystore() {
578578
Action: action.Reload,
579579
})
580580

581-
backend.uninitAccounts()
581+
backend.uninitAccounts(false)
582582
// TODO: classify accounts by keystore, remove only the ones belonging to the deregistered
583583
// keystore. For now we just remove all, then re-add the rest.
584584
backend.initPersistedAccounts()
@@ -721,7 +721,7 @@ func (backend *Backend) Close() error {
721721

722722
backend.ratesUpdater.Stop()
723723

724-
backend.uninitAccounts()
724+
backend.uninitAccounts(true)
725725

726726
for _, coin := range backend.coins {
727727
if err := coin.Close(); err != nil {

0 commit comments

Comments
 (0)