Merge pull request #19438 from karalabe/ledger-new-derivation-path

 accounts: switch Ledger derivation path to canonical one

8 files changed, 167 insertions, 137 deletions

diff --git a/accounts/accounts.go b/accounts/accounts.go
index a52aa425c..afeb412fe 100644
--- a/accounts/accounts.go
+++ b/accounts/accounts.go
@@ -92,9 +92,13 @@ type Wallet interface {
 	// opposed to decending into a child path to allow discovering accounts starting
 	// from non zero components.
 	//
+	// Some hardware wallets switched derivation paths through their evolution, so
+	// this method supports providing multiple bases to discover old user accounts
+	// too. Only the last base will be used to derive the next empty account.
+	//
 	// You can disable automatic account discovery by calling SelfDerive with a nil
 	// chain state reader.
-	SelfDerive(base DerivationPath, chain ethereum.ChainStateReader)
+	SelfDerive(bases []DerivationPath, chain ethereum.ChainStateReader)
 
 	// SignData requests the wallet to sign the hash of the given data
 	// It looks up the account specified either solely via its address contained within,
diff --git a/accounts/external/backend.go b/accounts/external/backend.go
index 21a313b66..23037f52d 100644
--- a/accounts/external/backend.go
+++ b/accounts/external/backend.go
@@ -143,7 +143,7 @@ func (api *ExternalSigner) Derive(path accounts.DerivationPath, pin bool) (accou
 	return accounts.Account{}, fmt.Errorf("operation not supported on external signers")
 }
 
-func (api *ExternalSigner) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {
+func (api *ExternalSigner) SelfDerive(bases []accounts.DerivationPath, chain ethereum.ChainStateReader) {
 	log.Error("operation SelfDerive not supported on external signers")
 }
 
diff --git a/accounts/hd.go b/accounts/hd.go
index 24aa777ae..75c476110 100644
--- a/accounts/hd.go
+++ b/accounts/hd.go
@@ -35,10 +35,10 @@ var DefaultRootDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60,
 // at m/44'/60'/0'/0/1, etc.
 var DefaultBaseDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0, 0}
 
-// DefaultLedgerBaseDerivationPath is the base path from which custom derivation endpoints
-// are incremented. As such, the first account will be at m/44'/60'/0'/0, the second
-// at m/44'/60'/0'/1, etc.
-var DefaultLedgerBaseDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0}
+// LegacyLedgerBaseDerivationPath is the legacy base path from which custom derivation
+// endpoints are incremented. As such, the first account will be at m/44'/60'/0'/0, the
+// second at m/44'/60'/0'/1, etc.
+var LegacyLedgerBaseDerivationPath = DerivationPath{0x80000000 + 44, 0x80000000 + 60, 0x80000000 + 0, 0}
 
 // DerivationPath represents the computer friendly version of a hierarchical
 // deterministic wallet account derivaion path.
diff --git a/accounts/keystore/wallet.go b/accounts/keystore/wallet.go
index 1b36b6dff..498067d49 100644
--- a/accounts/keystore/wallet.go
+++ b/accounts/keystore/wallet.go
@@ -77,7 +77,8 @@ func (w *keystoreWallet) Derive(path accounts.DerivationPath, pin bool) (account
 
 // SelfDerive implements accounts.Wallet, but is a noop for plain wallets since
 // there is no notion of hierarchical account derivation for plain keystore accounts.
-func (w *keystoreWallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {}
+func (w *keystoreWallet) SelfDerive(bases []accounts.DerivationPath, chain ethereum.ChainStateReader) {
+}
 
 // signHash attempts to sign the given hash with
 // the given account. If the wallet does not wrap this particular account, an
diff --git a/accounts/scwallet/wallet.go b/accounts/scwallet/wallet.go
index 4c9a422a3..4c0824eb9 100644
--- a/accounts/scwallet/wallet.go
+++ b/accounts/scwallet/wallet.go
@@ -119,11 +119,11 @@ type Wallet struct {
 	session *Session   // The secure communication session with the card
 	log     log.Logger // Contextual logger to tag the base with its id
 
-	deriveNextPath accounts.DerivationPath   // Next derivation path for account auto-discovery
-	deriveNextAddr common.Address            // Next derived account address for auto-discovery
-	deriveChain    ethereum.ChainStateReader // Blockchain state reader to discover used account with
-	deriveReq      chan chan struct{}        // Channel to request a self-derivation on
-	deriveQuit     chan chan error           // Channel to terminate the self-deriver with
+	deriveNextPaths []accounts.DerivationPath // Next derivation paths for account auto-discovery (multiple bases supported)
+	deriveNextAddrs []common.Address          // Next derived account addresses for auto-discovery (multiple bases supported)
+	deriveChain     ethereum.ChainStateReader // Blockchain state reader to discover used account with
+	deriveReq       chan chan struct{}        // Channel to request a self-derivation on
+	deriveQuit      chan chan error           // Channel to terminate the self-deriver with
 }
 
 // NewWallet constructs and returns a new Wallet instance.
@@ -390,7 +390,7 @@ func (w *Wallet) Open(passphrase string) error {
 	w.deriveReq = make(chan chan struct{})
 	w.deriveQuit = make(chan chan error)
 
-	go w.selfDerive(0)
+	go w.selfDerive()
 
 	// Notify anyone listening for wallet events that a new device is accessible
 	go w.Hub.updateFeed.Send(accounts.WalletEvent{Wallet: w, Kind: accounts.WalletOpened})
@@ -426,9 +426,8 @@ func (w *Wallet) Close() error {
 }
 
 // selfDerive is an account derivation loop that upon request attempts to find
-// new non-zero accounts. maxEmpty specifies the number of empty accounts that
-// should be derived once an initial empty account has been found.
-func (w *Wallet) selfDerive(maxEmpty int) {
+// new non-zero accounts.
+func (w *Wallet) selfDerive() {
 	w.log.Debug("Smart card wallet self-derivation started")
 	defer w.log.Debug("Smart card wallet self-derivation stopped")
 
@@ -461,56 +460,59 @@ func (w *Wallet) selfDerive(maxEmpty int) {
 			paths   []accounts.DerivationPath
 			nextAcc accounts.Account
 
-			nextAddr = w.deriveNextAddr
-			nextPath = w.deriveNextPath
+			nextPaths = append([]accounts.DerivationPath{}, w.deriveNextPaths...)
+			nextAddrs = append([]common.Address{}, w.deriveNextAddrs...)
 
 			context = context.Background()
 		)
-		for empty, emptyCount := false, maxEmpty+1; !empty || emptyCount > 0; {
-			// Retrieve the next derived Ethereum account
-			if nextAddr == (common.Address{}) {
-				if nextAcc, err = w.session.derive(nextPath); err != nil {
-					w.log.Warn("Smartcard wallet account derivation failed", "err", err)
+		for i := 0; i < len(nextAddrs); i++ {
+			for empty := false; !empty; {
+				// Retrieve the next derived Ethereum account
+				if nextAddrs[i] == (common.Address{}) {
+					if nextAcc, err = w.session.derive(nextPaths[i]); err != nil {
+						w.log.Warn("Smartcard wallet account derivation failed", "err", err)
+						break
+					}
+					nextAddrs[i] = nextAcc.Address
+				}
+				// Check the account's status against the current chain state
+				var (
+					balance *big.Int
+					nonce   uint64
+				)
+				balance, err = w.deriveChain.BalanceAt(context, nextAddrs[i], nil)
+				if err != nil {
+					w.log.Warn("Smartcard wallet balance retrieval failed", "err", err)
 					break
 				}
-				nextAddr = nextAcc.Address
-			}
-			// Check the account's status against the current chain state
-			var (
-				balance *big.Int
-				nonce   uint64
-			)
-			balance, err = w.deriveChain.BalanceAt(context, nextAddr, nil)
-			if err != nil {
-				w.log.Warn("Smartcard wallet balance retrieval failed", "err", err)
-				break
-			}
-			nonce, err = w.deriveChain.NonceAt(context, nextAddr, nil)
-			if err != nil {
-				w.log.Warn("Smartcard wallet nonce retrieval failed", "err", err)
-				break
-			}
-			// If the next account is empty and no more empty accounts are
-			// allowed, stop self-derivation. Add the current one nonetheless.
-			if balance.Sign() == 0 && nonce == 0 {
-				empty = true
-				emptyCount--
-			}
-			// We've just self-derived a new account, start tracking it locally
-			path := make(accounts.DerivationPath, len(nextPath))
-			copy(path[:], nextPath[:])
-			paths = append(paths, path)
-
-			// Display a log message to the user for new (or previously empty accounts)
-			if _, known := pairing.Accounts[nextAddr]; !known || !empty || nextAddr != w.deriveNextAddr {
-				w.log.Info("Smartcard wallet discovered new account", "address", nextAddr, "path", path, "balance", balance, "nonce", nonce)
-			}
-			pairing.Accounts[nextAddr] = path
+				nonce, err = w.deriveChain.NonceAt(context, nextAddrs[i], nil)
+				if err != nil {
+					w.log.Warn("Smartcard wallet nonce retrieval failed", "err", err)
+					break
+				}
+				// If the next account is empty, stop self-derivation, but add for the last base path
+				if balance.Sign() == 0 && nonce == 0 {
+					empty = true
+					if i < len(nextAddrs)-1 {
+						break
+					}
+				}
+				// We've just self-derived a new account, start tracking it locally
+				path := make(accounts.DerivationPath, len(nextPaths[i]))
+				copy(path[:], nextPaths[i][:])
+				paths = append(paths, path)
+
+				// Display a log message to the user for new (or previously empty accounts)
+				if _, known := pairing.Accounts[nextAddrs[i]]; !known || !empty || nextAddrs[i] != w.deriveNextAddrs[i] {
+					w.log.Info("Smartcard wallet discovered new account", "address", nextAddrs[i], "path", path, "balance", balance, "nonce", nonce)
+				}
+				pairing.Accounts[nextAddrs[i]] = path
 
-			// Fetch the next potential account
-			if !empty || emptyCount > 0 {
-				nextAddr = common.Address{}
-				nextPath[len(nextPath)-1]++
+				// Fetch the next potential account
+				if !empty {
+					nextAddrs[i] = common.Address{}
+					nextPaths[i][len(nextPaths[i])-1]++
+				}
 			}
 		}
 		// If there are new accounts, write them out
@@ -518,8 +520,8 @@ func (w *Wallet) selfDerive(maxEmpty int) {
 			err = w.Hub.setPairing(w, pairing)
 		}
 		// Shift the self-derivation forward
-		w.deriveNextAddr = nextAddr
-		w.deriveNextPath = nextPath
+		w.deriveNextAddrs = nextAddrs
+		w.deriveNextPaths = nextPaths
 
 		// Self derivation complete, release device lock
 		w.lock.Unlock()
@@ -592,7 +594,7 @@ func (w *Wallet) Contains(account accounts.Account) bool {
 
 // Initialize installs a keypair generated from the provided key into the wallet.
 func (w *Wallet) Initialize(seed []byte) error {
-	go w.selfDerive(0)
+	go w.selfDerive()
 	// DO NOT lock at this stage, as the initialize
 	// function relies on Status()
 	return w.session.initialize(seed)
@@ -629,16 +631,22 @@ func (w *Wallet) Derive(path accounts.DerivationPath, pin bool) (accounts.Accoun
 // opposed to decending into a child path to allow discovering accounts starting
 // from non zero components.
 //
+// Some hardware wallets switched derivation paths through their evolution, so
+// this method supports providing multiple bases to discover old user accounts
+// too. Only the last base will be used to derive the next empty account.
+//
 // You can disable automatic account discovery by calling SelfDerive with a nil
 // chain state reader.
-func (w *Wallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {
+func (w *Wallet) SelfDerive(bases []accounts.DerivationPath, chain ethereum.ChainStateReader) {
 	w.lock.Lock()
 	defer w.lock.Unlock()
 
-	w.deriveNextPath = make(accounts.DerivationPath, len(base))
-	copy(w.deriveNextPath[:], base[:])
-
-	w.deriveNextAddr = common.Address{}
+	w.deriveNextPaths = make([]accounts.DerivationPath, len(bases))
+	for i, base := range bases {
+		w.deriveNextPaths[i] = make(accounts.DerivationPath, len(base))
+		copy(w.deriveNextPaths[i][:], base[:])
+	}
+	w.deriveNextAddrs = make([]common.Address, len(bases))
 	w.deriveChain = chain
 }
 
diff --git a/accounts/usbwallet/wallet.go b/accounts/usbwallet/wallet.go
index 2ddfa30a6..7ff9c5372 100644
--- a/accounts/usbwallet/wallet.go
+++ b/accounts/usbwallet/wallet.go
@@ -83,11 +83,11 @@ type wallet struct {
 	accounts []accounts.Account                         // List of derive accounts pinned on the hardware wallet
 	paths    map[common.Address]accounts.DerivationPath // Known derivation paths for signing operations
 
-	deriveNextPath accounts.DerivationPath   // Next derivation path for account auto-discovery
-	deriveNextAddr common.Address            // Next derived account address for auto-discovery
-	deriveChain    ethereum.ChainStateReader // Blockchain state reader to discover used account with
-	deriveReq      chan chan struct{}        // Channel to request a self-derivation on
-	deriveQuit     chan chan error           // Channel to terminate the self-deriver with
+	deriveNextPaths []accounts.DerivationPath // Next derivation paths for account auto-discovery (multiple bases supported)
+	deriveNextAddrs []common.Address          // Next derived account addresses for auto-discovery (multiple bases supported)
+	deriveChain     ethereum.ChainStateReader // Blockchain state reader to discover used account with
+	deriveReq       chan chan struct{}        // Channel to request a self-derivation on
+	deriveQuit      chan chan error           // Channel to terminate the self-deriver with
 
 	healthQuit chan chan error
 
@@ -339,57 +339,62 @@ func (w *wallet) selfDerive() {
 			accs  []accounts.Account
 			paths []accounts.DerivationPath
 
-			nextAddr = w.deriveNextAddr
-			nextPath = w.deriveNextPath
+			nextPaths = append([]accounts.DerivationPath{}, w.deriveNextPaths...)
+			nextAddrs = append([]common.Address{}, w.deriveNextAddrs...)
 
 			context = context.Background()
 		)
-		for empty := false; !empty; {
-			// Retrieve the next derived Ethereum account
-			if nextAddr == (common.Address{}) {
-				if nextAddr, err = w.driver.Derive(nextPath); err != nil {
-					w.log.Warn("USB wallet account derivation failed", "err", err)
+		for i := 0; i < len(nextAddrs); i++ {
+			for empty := false; !empty; {
+				// Retrieve the next derived Ethereum account
+				if nextAddrs[i] == (common.Address{}) {
+					if nextAddrs[i], err = w.driver.Derive(nextPaths[i]); err != nil {
+						w.log.Warn("USB wallet account derivation failed", "err", err)
+						break
+					}
+				}
+				// Check the account's status against the current chain state
+				var (
+					balance *big.Int
+					nonce   uint64
+				)
+				balance, err = w.deriveChain.BalanceAt(context, nextAddrs[i], nil)
+				if err != nil {
+					w.log.Warn("USB wallet balance retrieval failed", "err", err)
 					break
 				}
-			}
-			// Check the account's status against the current chain state
-			var (
-				balance *big.Int
-				nonce   uint64
-			)
-			balance, err = w.deriveChain.BalanceAt(context, nextAddr, nil)
-			if err != nil {
-				w.log.Warn("USB wallet balance retrieval failed", "err", err)
-				break
-			}
-			nonce, err = w.deriveChain.NonceAt(context, nextAddr, nil)
-			if err != nil {
-				w.log.Warn("USB wallet nonce retrieval failed", "err", err)
-				break
-			}
-			// If the next account is empty, stop self-derivation, but add it nonetheless
-			if balance.Sign() == 0 && nonce == 0 {
-				empty = true
-			}
-			// We've just self-derived a new account, start tracking it locally
-			path := make(accounts.DerivationPath, len(nextPath))
-			copy(path[:], nextPath[:])
-			paths = append(paths, path)
-
-			account := accounts.Account{
-				Address: nextAddr,
-				URL:     accounts.URL{Scheme: w.url.Scheme, Path: fmt.Sprintf("%s/%s", w.url.Path, path)},
-			}
-			accs = append(accs, account)
+				nonce, err = w.deriveChain.NonceAt(context, nextAddrs[i], nil)
+				if err != nil {
+					w.log.Warn("USB wallet nonce retrieval failed", "err", err)
+					break
+				}
+				// If the next account is empty, stop self-derivation, but add for the last base path
+				if balance.Sign() == 0 && nonce == 0 {
+					empty = true
+					if i < len(nextAddrs)-1 {
+						break
+					}
+				}
+				// We've just self-derived a new account, start tracking it locally
+				path := make(accounts.DerivationPath, len(nextPaths[i]))
+				copy(path[:], nextPaths[i][:])
+				paths = append(paths, path)
+
+				account := accounts.Account{
+					Address: nextAddrs[i],
+					URL:     accounts.URL{Scheme: w.url.Scheme, Path: fmt.Sprintf("%s/%s", w.url.Path, path)},
+				}
+				accs = append(accs, account)
 
-			// Display a log message to the user for new (or previously empty accounts)
-			if _, known := w.paths[nextAddr]; !known || (!empty && nextAddr == w.deriveNextAddr) {
-				w.log.Info("USB wallet discovered new account", "address", nextAddr, "path", path, "balance", balance, "nonce", nonce)
-			}
-			// Fetch the next potential account
-			if !empty {
-				nextAddr = common.Address{}
-				nextPath[len(nextPath)-1]++
+				// Display a log message to the user for new (or previously empty accounts)
+				if _, known := w.paths[nextAddrs[i]]; !known || (!empty && nextAddrs[i] == w.deriveNextAddrs[i]) {
+					w.log.Info("USB wallet discovered new account", "address", nextAddrs[i], "path", path, "balance", balance, "nonce", nonce)
+				}
+				// Fetch the next potential account
+				if !empty {
+					nextAddrs[i] = common.Address{}
+					nextPaths[i][len(nextPaths[i])-1]++
+				}
 			}
 		}
 		// Self derivation complete, release device lock
@@ -406,8 +411,8 @@ func (w *wallet) selfDerive() {
 		}
 		// Shift the self-derivation forward
 		// TODO(karalabe): don't overwrite changes from wallet.SelfDerive
-		w.deriveNextAddr = nextAddr
-		w.deriveNextPath = nextPath
+		w.deriveNextAddrs = nextAddrs
+		w.deriveNextPaths = nextPaths
 		w.stateLock.Unlock()
 
 		// Notify the user of termination and loop after a bit of time (to avoid trashing)
@@ -479,18 +484,30 @@ func (w *wallet) Derive(path accounts.DerivationPath, pin bool) (accounts.Accoun
 	return account, nil
 }
 
-// SelfDerive implements accounts.Wallet, trying to discover accounts that the
-// user used previously (based on the chain state), but ones that he/she did not
-// explicitly pin to the wallet manually. To avoid chain head monitoring, self
-// derivation only runs during account listing (and even then throttled).
-func (w *wallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {
+// SelfDerive sets a base account derivation path from which the wallet attempts
+// to discover non zero accounts and automatically add them to list of tracked
+// accounts.
+//
+// Note, self derivaton will increment the last component of the specified path
+// opposed to decending into a child path to allow discovering accounts starting
+// from non zero components.
+//
+// Some hardware wallets switched derivation paths through their evolution, so
+// this method supports providing multiple bases to discover old user accounts
+// too. Only the last base will be used to derive the next empty account.
+//
+// You can disable automatic account discovery by calling SelfDerive with a nil
+// chain state reader.
+func (w *wallet) SelfDerive(bases []accounts.DerivationPath, chain ethereum.ChainStateReader) {
 	w.stateLock.Lock()
 	defer w.stateLock.Unlock()
 
-	w.deriveNextPath = make(accounts.DerivationPath, len(base))
-	copy(w.deriveNextPath[:], base[:])
-
-	w.deriveNextAddr = common.Address{}
+	w.deriveNextPaths = make([]accounts.DerivationPath, len(bases))
+	for i, base := range bases {
+		w.deriveNextPaths[i] = make(accounts.DerivationPath, len(base))
+		copy(w.deriveNextPaths[i][:], base[:])
+	}
+	w.deriveNextAddrs = make([]common.Address, len(bases))
 	w.deriveChain = chain
 }
 
diff --git a/cmd/geth/main.go b/cmd/geth/main.go
index c439e36fc..0e89481fe 100644
--- a/cmd/geth/main.go
+++ b/cmd/geth/main.go
@@ -342,11 +342,13 @@ func startNode(ctx *cli.Context, stack *node.Node) {
 				status, _ := event.Wallet.Status()
 				log.Info("New wallet appeared", "url", event.Wallet.URL(), "status", status)
 
-				derivationPath := accounts.DefaultBaseDerivationPath
+				var derivationPaths []accounts.DerivationPath
 				if event.Wallet.URL().Scheme == "ledger" {
-					derivationPath = accounts.DefaultLedgerBaseDerivationPath
+					derivationPaths = append(derivationPaths, accounts.LegacyLedgerBaseDerivationPath)
 				}
-				event.Wallet.SelfDerive(derivationPath, stateReader)
+				derivationPaths = append(derivationPaths, accounts.DefaultBaseDerivationPath)
+
+				event.Wallet.SelfDerive(derivationPaths, stateReader)
 
 			case accounts.WalletDropped:
 				log.Info("Old wallet dropped", "url", event.Wallet.URL())
diff --git a/signer/core/api.go b/signer/core/api.go
index 0ed037fc2..671fbf79b 100644
--- a/signer/core/api.go
+++ b/signer/core/api.go
@@ -319,12 +319,10 @@ func (api *SignerAPI) startUSBListener() {
 				status, _ := event.Wallet.Status()
 				log.Info("New wallet appeared", "url", event.Wallet.URL(), "status", status)
 
-				derivationPath := accounts.DefaultBaseDerivationPath
-				if event.Wallet.URL().Scheme == "ledger" {
-					derivationPath = accounts.DefaultLedgerBaseDerivationPath
-				}
-				var nextPath = derivationPath
 				// Derive first N accounts, hardcoded for now
+				var nextPath = make(accounts.DerivationPath, len(accounts.DefaultBaseDerivationPath))
+				copy(nextPath[:], accounts.DefaultBaseDerivationPath[:])
+
 				for i := 0; i < numberOfAccountsToDerive; i++ {
 					acc, err := event.Wallet.Derive(nextPath, true)
 					if err != nil {