accounts/usbwallet: make wallet responsive while Ledger is busy

accounts/usbwallet/ledger_wallet.go

@@ -90,26 +90,47 @@ type ledgerWallet struct {
 	accounts []accounts.Account                         // List of derive accounts pinned on the Ledger
 	paths    map[common.Address]accounts.DerivationPath // Known derivation paths for signing operations
 
-	selfDeriveNextPath accounts.DerivationPath   // Next derivation path for account auto-discovery
+	deriveNextPath accounts.DerivationPath   // Next derivation path for account auto-discovery
-	selfDeriveNextAddr common.Address            // Next derived account address for auto-discovery
+	deriveNextAddr common.Address            // Next derived account address for auto-discovery
-	selfDerivePrevZero common.Address            // Last zero-address where auto-discovery stopped
+	deriveChain    ethereum.ChainStateReader // Blockchain state reader to discover used account with
-	selfDeriveChain    ethereum.ChainStateReader // Blockchain state reader to discover used account with
+	deriveReq      chan chan struct{}        // Channel to request a self-derivation on
-	selfDeriveTime     time.Time                 // Timestamp of the last self-derivation to avoid thrashing
+	deriveQuit     chan chan error           // Channel to terminate the self-deriver with
 
-	quit chan chan error
+	healthQuit chan chan error
-	lock sync.RWMutex
+
+	// Locking a hardware wallet is a bit special. Since hardware devices are lower
+	// performing, any communication with them might take a non negligible amount of
+	// time. Worse still, waiting for user confirmation can take arbitrarily long,
+	// but exclusive communication must be upheld during. Locking the entire wallet
+	// in the mean time however would stall any parts of the system that don't want
+	// to communicate, just read some state (e.g. list the accounts).
+	//
+	// As such, a hardware wallet needs two locks to function correctly. A state
+	// lock can be used to protect the wallet's software-side internal state, which
+	// must not be held exlusively during hardware communication. A communication
+	// lock can be used to achieve exclusive access to the device itself, this one
+	// however should allow "skipping" waiting for operations that might want to
+	// use the device, but can live without too (e.g. account self-derivation).
+	//
+	// Since we have two locks, it's important to know how to properly use them:
+	//   - Communication requires the `device` to not change, so obtaining the
+	//     commsLock should be done after having a stateLock.
+	//   - Communication must not disable read access to the wallet state, so it
+	//     must only ever hold a *read* lock to stateLock.
+	commsLock chan struct{} // Mutex (buf=1) for the USB comms without keeping the state locked
+	stateLock sync.RWMutex  // Protects read and write access to the wallet struct fields
 }
 
 // URL implements accounts.Wallet, returning the URL of the Ledger device.
 func (w *ledgerWallet) URL() accounts.URL {
-	return *w.url
+	return *w.url // Immutable, no need for a lock
 }
 
 // Status implements accounts.Wallet, always whether the Ledger is opened, closed
 // or whether the Ethereum app was not started on it.
 func (w *ledgerWallet) Status() string {
-	w.lock.RLock()
+	w.stateLock.RLock() // No device communication, state lock is enough
-	defer w.lock.RUnlock()
+	defer w.stateLock.RUnlock()
 
 	if w.failure != nil {
 		return fmt.Sprintf("Failed: %v", w.failure)
@@ -124,25 +145,29 @@ func (w *ledgerWallet) Status() string {
 }
 
 // offline returns whether the wallet and the Ethereum app is offline or not.
+//
+// The method assumes that the state lock is held!
 func (w *ledgerWallet) offline() bool {
 	return w.version == [3]byte{0, 0, 0}
 }
 
 // failed returns if the USB device wrapped by the wallet failed for some reason.
 // This is used by the device scanner to report failed wallets as departed.
+//
+// The method assumes that the state lock is *not* held!
 func (w *ledgerWallet) failed() bool {
-	w.lock.RLock()
+	w.stateLock.RLock() // No device communication, state lock is enough
-	defer w.lock.RUnlock()
+	defer w.stateLock.RUnlock()
 
 	return w.failure != nil
 }
 
 // Open implements accounts.Wallet, attempting to open a USB connection to the
-// Ledger hardware wallet. The Ledger does not require a user passphrase so that
+// Ledger hardware wallet. The Ledger does not require a user passphrase, so that
-// is silently discarded.
+// parameter is silently discarded.
 func (w *ledgerWallet) Open(passphrase string) error {
-	w.lock.Lock()
+	w.stateLock.Lock() // State lock is enough since there's no connection yet at this point
-	defer w.lock.Unlock()
+	defer w.stateLock.Unlock()
 
 	// If the wallet was already opened, don't try to open again
 	if w.device != nil {
@@ -199,19 +224,26 @@ func (w *ledgerWallet) Open(passphrase string) error {
 	}
 	// Wallet seems to be successfully opened, guess if the Ethereum app is running
 	w.device, w.input, w.output = device, input, output
+	w.commsLock = make(chan struct{}, 1)
+	w.commsLock <- struct{}{} // Enable lock
 
 	w.paths = make(map[common.Address]accounts.DerivationPath)
-	w.quit = make(chan chan error)
+
+	w.deriveReq = make(chan chan struct{})
+	w.deriveQuit = make(chan chan error)
+	w.healthQuit = make(chan chan error)
+
 	defer func() {
 		go w.heartbeat()
+		go w.selfDerive()
 	}()
 
-	if _, err := w.deriveAddress(accounts.DefaultBaseDerivationPath); err != nil {
+	if _, err = w.ledgerDerive(accounts.DefaultBaseDerivationPath); err != nil {
 		// Ethereum app is not running, nothing more to do, return
 		return nil
 	}
 	// Try to resolve the Ethereum app's version, will fail prior to v1.0.2
-	if w.resolveVersion() != nil {
+	if w.version, err = w.ledgerVersion(); err != nil {
 		w.version = [3]byte{1, 0, 0} // Assume worst case, can't verify if v1.0.0 or v1.0.1
 	}
 	return nil
@@ -222,57 +254,94 @@ func (w *ledgerWallet) Open(passphrase string) error {
 //  - libusb on Windows doesn't support hotplug, so we can't detect USB unplugs
 //  - communication timeout on the Ledger requires a device power cycle to fix
 func (w *ledgerWallet) heartbeat() {
+	glog.V(logger.Debug).Infof("%s health-check started", w.url.String())
+	defer glog.V(logger.Debug).Infof("%s health-check stopped", w.url.String())
+
 	// Execute heartbeat checks until termination or error
 	var (
 		errc chan error
-		fail error
+		err  error
 	)
-	for errc == nil && fail == nil {
+	for errc == nil && err == nil {
 		// Wait until termination is requested or the heartbeat cycle arrives
 		select {
-		case errc = <-w.quit:
+		case errc = <-w.healthQuit:
 			// Termination requested
 			continue
 		case <-time.After(ledgerHeartbeatCycle):
 			// Heartbeat time
 		}
 		// Execute a tiny data exchange to see responsiveness
-		w.lock.Lock()
+		w.stateLock.RLock()
-		if err := w.resolveVersion(); err == usb.ERROR_IO || err == usb.ERROR_NO_DEVICE {
+		if w.device == nil {
+			// Terminated while waiting for the lock
+			w.stateLock.RUnlock()
+			continue
+		}
+		<-w.commsLock // Don't lock state while resolving version
+		_, err = w.ledgerVersion()
+		w.commsLock <- struct{}{}
+		w.stateLock.RUnlock()
+
+		if err == usb.ERROR_IO || err == usb.ERROR_NO_DEVICE {
+			w.stateLock.Lock() // Lock state to tear the wallet down
 			w.failure = err
 			w.close()
-
+			w.stateLock.Unlock()
-			fail = err
 		}
-		w.lock.Unlock()
+		// Ignore uninteresting errors
+		err = nil
 	}
 	// In case of error, wait for termination
-	if fail != nil {
+	if err != nil {
-		errc = <-w.quit
+		glog.V(logger.Debug).Infof("%s health-check failed: %v", w.url.String(), err)
+		errc = <-w.healthQuit
 	}
-	errc <- fail
+	errc <- err
 }
 
 // Close implements accounts.Wallet, closing the USB connection to the Ledger.
 func (w *ledgerWallet) Close() error {
+	// Ensure the wallet was opened
+	w.stateLock.RLock()
+	hQuit, dQuit := w.healthQuit, w.deriveQuit
+	w.stateLock.RUnlock()
+
 	// Terminate the health checks
+	var herr error
+	if hQuit != nil {
 		errc := make(chan error)
-	w.quit <- errc
+		hQuit <- errc
-	herr := <-errc // Save for later, we *must* close the USB
+		herr = <-errc // Save for later, we *must* close the USB
-
+	}
+	// Terminate the self-derivations
+	var derr error
+	if dQuit != nil {
+		errc := make(chan error)
+		dQuit <- errc
+		derr = <-errc // Save for later, we *must* close the USB
+	}
 	// Terminate the device connection
-	w.lock.Lock()
+	w.stateLock.Lock()
-	defer w.lock.Unlock()
+	defer w.stateLock.Unlock()
+
+	w.healthQuit = nil
+	w.deriveQuit = nil
+	w.deriveReq = nil
 
-	w.quit = nil
 	if err := w.close(); err != nil {
 		return err
 	}
-	return herr // If all went well, return any health-check errors
+	if herr != nil {
+		return herr
+	}
+	return derr
 }
 
 // close is the internal wallet closer that terminates the USB connection and
-// resets all the fields to their defaults. It assumes the lock is held.
+// resets all the fields to their defaults.
+//
+// Note, close assumes the state lock is held!
 func (w *ledgerWallet) close() error {
 	// Allow duplicate closes, especially for health-check failures
 	if w.device == nil {
@@ -282,97 +351,169 @@ func (w *ledgerWallet) close() error {
 	err := w.device.Close()
 
 	w.device, w.input, w.output = nil, nil, nil
-	w.version, w.paths = [3]byte{}, nil
+	w.version, w.accounts, w.paths = [3]byte{}, nil, nil
 
 	return err
 }
 
 // Accounts implements accounts.Wallet, returning the list of accounts pinned to
-// the Ledger hardware wallet. If self derivation was enabled, the account list
+// the Ledger hardware wallet. If self-derivation was enabled, the account list
 // is periodically expanded based on current chain state.
 func (w *ledgerWallet) Accounts() []accounts.Account {
-	w.lock.Lock()
+	// Attempt self-derivation if it's running
-	defer w.lock.Unlock()
+	reqc := make(chan struct{}, 1)
-
+	select {
-	// If the wallet is offline, there are no accounts to return
+	case w.deriveReq <- reqc:
-	if w.offline() {
+		// Self-derivation request accepted, wait for it
-		return nil
+		<-reqc
+	default:
+		// Self-derivation offline, throttled or busy, skip
 	}
-	// If no self derivation is done (or throttled), return the current accounts
+	// Return whatever account list we ended up with
-	if w.selfDeriveChain == nil || time.Since(w.selfDeriveTime) < ledgerSelfDeriveThrottling {
+	w.stateLock.RLock()
+	defer w.stateLock.RUnlock()
+
 	cpy := make([]accounts.Account, len(w.accounts))
 	copy(cpy, w.accounts)
 	return cpy
 }
-	// Self derivation requested, try to expand our account list
+
-	ctx := context.Background()
+// selfDerive is an account derivation loop that upon request attempts to find
+// new non-zero accounts.
+func (w *ledgerWallet) selfDerive() {
+	glog.V(logger.Debug).Infof("%s self-derivation started", w.url.String())
+	defer glog.V(logger.Debug).Infof("%s self-derivation stopped", w.url.String())
+
+	// Execute self-derivations until termination or error
+	var (
+		reqc chan struct{}
+		errc chan error
+		err  error
+	)
+	for errc == nil && err == nil {
+		// Wait until either derivation or termination is requested
+		select {
+		case errc = <-w.deriveQuit:
+			// Termination requested
+			continue
+		case reqc = <-w.deriveReq:
+			// Account discovery requested
+		}
+		// Derivation needs a chain and device access, skip if either unavailable
+		w.stateLock.RLock()
+		if w.device == nil || w.deriveChain == nil || w.offline() {
+			w.stateLock.RUnlock()
+			reqc <- struct{}{}
+			continue
+		}
+		select {
+		case <-w.commsLock:
+		default:
+			w.stateLock.RUnlock()
+			reqc <- struct{}{}
+			continue
+		}
+		// Device lock obtained, derive the next batch of accounts
+		var (
+			accs  []accounts.Account
+			paths []accounts.DerivationPath
+
+			nextAddr = w.deriveNextAddr
+			nextPath = w.deriveNextPath
+
+			context = context.Background()
+		)
 		for empty := false; !empty; {
 			// Retrieve the next derived Ethereum account
-		var err error
+			if nextAddr == (common.Address{}) {
-		if w.selfDeriveNextAddr == (common.Address{}) {
+				if nextAddr, err = w.ledgerDerive(nextPath); err != nil {
-			w.selfDeriveNextAddr, err = w.deriveAddress(w.selfDeriveNextPath)
+					glog.V(logger.Warn).Infof("%s self-derivation failed: %v", w.url.String(), err)
-			if err != nil {
-				// Derivation failed, disable auto discovery
-				glog.V(logger.Warn).Infof("self-derivation failed: %v", err)
-				w.selfDeriveChain = nil
 					break
 				}
 			}
 			// Check the account's status against the current chain state
-		balance, err := w.selfDeriveChain.BalanceAt(ctx, w.selfDeriveNextAddr, nil)
+			var (
+				balance *big.Int
+				nonce   uint64
+			)
+			balance, err = w.deriveChain.BalanceAt(context, nextAddr, nil)
 			if err != nil {
-			glog.V(logger.Warn).Infof("self-derivation balance retrieval failed: %v", err)
+				glog.V(logger.Warn).Infof("%s self-derivation balance retrieval failed: %v", w.url.String(), err)
-			w.selfDeriveChain = nil
 				break
 			}
-		nonce, err := w.selfDeriveChain.NonceAt(ctx, w.selfDeriveNextAddr, nil)
+			nonce, err = w.deriveChain.NonceAt(context, nextAddr, nil)
 			if err != nil {
-			glog.V(logger.Warn).Infof("self-derivation nonce retrieval failed: %v", err)
+				glog.V(logger.Warn).Infof("%s self-derivation nonce retrieval failed: %v", w.url.String(), err)
-			w.selfDeriveChain = nil
 				break
 			}
 			// If the next account is empty, stop self-derivation, but add it nonetheless
 			if balance.BitLen() == 0 && nonce == 0 {
-			w.selfDerivePrevZero = w.selfDeriveNextAddr
 				empty = true
 			}
-		// We've just self-derived a new non-zero account, start tracking it
+			// We've just self-derived a new account, start tracking it locally
-		path := make(accounts.DerivationPath, len(w.selfDeriveNextPath))
+			path := make(accounts.DerivationPath, len(nextPath))
-		copy(path[:], w.selfDeriveNextPath[:])
+			copy(path[:], nextPath[:])
+			paths = append(paths, path)
 
 			account := accounts.Account{
-			Address: w.selfDeriveNextAddr,
+				Address: nextAddr,
 				URL:     accounts.URL{Scheme: w.url.Scheme, Path: fmt.Sprintf("%s/%s", w.url.Path, path)},
 			}
-		_, known := w.paths[w.selfDeriveNextAddr]
+			accs = append(accs, account)
-		if !known || (!empty && w.selfDeriveNextAddr == w.selfDerivePrevZero) {
+
-			// Either fully new account, or previous zero. Report discovery either way
+			// Display a log message to the user for new (or previously empty accounts)
-			glog.V(logger.Info).Infof("%s discovered %s (balance %d, nonce %d) at %s", w.url.String(), w.selfDeriveNextAddr.Hex(), balance, nonce, path)
+			if _, known := w.paths[nextAddr]; !known || (!empty && nextAddr == w.deriveNextAddr) {
-		}
+				glog.V(logger.Info).Infof("%s discovered %s (balance %d, nonce %d) at %s", w.url.String(), nextAddr.Hex(), balance, nonce, path)
-		if !known {
-			w.accounts = append(w.accounts, account)
-			w.paths[w.selfDeriveNextAddr] = path
 			}
 			// Fetch the next potential account
 			if !empty {
-			w.selfDeriveNextAddr = common.Address{}
+				nextAddr = common.Address{}
-			w.selfDeriveNextPath[len(w.selfDeriveNextPath)-1]++
+				nextPath[len(nextPath)-1]++
 			}
 		}
-	w.selfDeriveTime = time.Now()
+		// Self derivation complete, release device lock
+		w.commsLock <- struct{}{}
+		w.stateLock.RUnlock()
 
-	// Return whatever account list we ended up with
+		// Insert any accounts successfully derived
-	cpy := make([]accounts.Account, len(w.accounts))
+		w.stateLock.Lock()
-	copy(cpy, w.accounts)
+		for i := 0; i < len(accs); i++ {
-	return cpy
+			if _, ok := w.paths[accs[i].Address]; !ok {
+				w.accounts = append(w.accounts, accs[i])
+				w.paths[accs[i].Address] = paths[i]
+			}
+		}
+		// Shift the self-derivation forward
+		// TODO(karalabe): don't overwrite changes from wallet.SelfDerive
+		w.deriveNextAddr = nextAddr
+		w.deriveNextPath = nextPath
+		w.stateLock.Unlock()
+
+		// Notify the user of termination and loop after a bit of time (to avoid trashing)
+		reqc <- struct{}{}
+		if err == nil {
+			select {
+			case errc = <-w.deriveQuit:
+				// Termination requested, abort
+			case <-time.After(ledgerSelfDeriveThrottling):
+				// Waited enough, willing to self-derive again
+			}
+		}
+	}
+	// In case of error, wait for termination
+	if err != nil {
+		glog.V(logger.Debug).Infof("%s self-derivation failed: %s", w.url.String(), err)
+		errc = <-w.deriveQuit
+	}
+	errc <- err
 }
 
 // Contains implements accounts.Wallet, returning whether a particular account is
 // or is not pinned into this Ledger instance. Although we could attempt to resolve
 // unpinned accounts, that would be an non-negligible hardware operation.
 func (w *ledgerWallet) Contains(account accounts.Account) bool {
-	w.lock.RLock()
+	w.stateLock.RLock()
-	defer w.lock.RUnlock()
+	defer w.stateLock.RUnlock()
 
 	_, exists := w.paths[account.Address]
 	return exists
@@ -382,15 +523,20 @@ func (w *ledgerWallet) Contains(account accounts.Account) bool {
 // derivation path. If pin is set to true, the account will be added to the list
 // of tracked accounts.
 func (w *ledgerWallet) Derive(path accounts.DerivationPath, pin bool) (accounts.Account, error) {
-	w.lock.Lock()
+	// Try to derive the actual account and update its URL if successful
-	defer w.lock.Unlock()
+	w.stateLock.RLock() // Avoid device disappearing during derivation
 
-	// If the wallet is closed, or the Ethereum app doesn't run, abort
 	if w.device == nil || w.offline() {
+		w.stateLock.RUnlock()
 		return accounts.Account{}, accounts.ErrWalletClosed
 	}
-	// Try to derive the actual account and update it's URL if succeeful
+	<-w.commsLock // Avoid concurrent hardware access
-	address, err := w.deriveAddress(path)
+	address, err := w.ledgerDerive(path)
+	w.commsLock <- struct{}{}
+
+	w.stateLock.RUnlock()
+
+	// If an error occurred or no pinning was requested, return
 	if err != nil {
 		return accounts.Account{}, err
 	}
@@ -398,13 +544,17 @@ func (w *ledgerWallet) Derive(path accounts.DerivationPath, pin bool) (accounts.
 		Address: address,
 		URL:     accounts.URL{Scheme: w.url.Scheme, Path: fmt.Sprintf("%s/%s", w.url.Path, path)},
 	}
-	// If pinning was requested, track the account
+	if !pin {
-	if pin {
+		return account, nil
+	}
+	// Pinning needs to modify the state
+	w.stateLock.Lock()
+	defer w.stateLock.Unlock()
+
 	if _, ok := w.paths[address]; !ok {
 		w.accounts = append(w.accounts, account)
 		w.paths[address] = path
 	}
-	}
 	return account, nil
 }
 
@@ -413,14 +563,14 @@ func (w *ledgerWallet) Derive(path accounts.DerivationPath, pin bool) (accounts.
 // explicitly pin to the wallet manually. To avoid chain head monitoring, self
 // derivation only runs during account listing (and even then throttled).
 func (w *ledgerWallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {
-	w.lock.Lock()
+	w.stateLock.Lock()
-	defer w.lock.Unlock()
+	defer w.stateLock.Unlock()
 
-	w.selfDeriveNextPath = make(accounts.DerivationPath, len(base))
+	w.deriveNextPath = make(accounts.DerivationPath, len(base))
-	copy(w.selfDeriveNextPath[:], base[:])
+	copy(w.deriveNextPath[:], base[:])
 
-	w.selfDeriveNextAddr = common.Address{}
+	w.deriveNextAddr = common.Address{}
-	w.selfDeriveChain = chain
+	w.deriveChain = chain
 }
 
 // SignHash implements accounts.Wallet, however signing arbitrary data is not
@@ -437,9 +587,13 @@ func (w *ledgerWallet) SignHash(acc accounts.Account, hash []byte) ([]byte, erro
 // too old to sign EIP-155 transactions, but such is requested nonetheless, an error
 // will be returned opposed to silently signing in Homestead mode.
 func (w *ledgerWallet) SignTx(account accounts.Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
-	w.lock.Lock()
+	w.stateLock.RLock() // Comms have own mutex, this is for the state fields
-	defer w.lock.Unlock()
+	defer w.stateLock.RUnlock()
 
+	// If the wallet is closed, or the Ethereum app doesn't run, abort
+	if w.device == nil || w.offline() {
+		return nil, accounts.ErrWalletClosed
+	}
 	// Make sure the requested account is contained within
 	path, ok := w.paths[account.Address]
 	if !ok {
@@ -447,10 +601,13 @@ func (w *ledgerWallet) SignTx(account accounts.Account, tx *types.Transaction, c
 	}
 	// Ensure the wallet is capable of signing the given transaction
 	if chainID != nil && w.version[0] <= 1 && w.version[1] <= 0 && w.version[2] <= 2 {
-		return nil, fmt.Errorf("Ledger v%d.%d.%d doesn't support signing this transaction, please update to v1.0.3 at least",
+		return nil, fmt.Errorf("Ledger v%d.%d.%d doesn't support signing this transaction, please update to v1.0.3 at least", w.version[0], w.version[1], w.version[2])
-			w.version[0], w.version[1], w.version[2])
 	}
-	return w.sign(path, account.Address, tx, chainID)
+	// All infos gathered and metadata checks out, request signing
+	<-w.commsLock
+	defer func() { w.commsLock <- struct{}{} }()
+
+	return w.ledgerSign(path, account.Address, tx, chainID)
 }
 
 // SignHashWithPassphrase implements accounts.Wallet, however signing arbitrary
@@ -467,8 +624,8 @@ func (w *ledgerWallet) SignTxWithPassphrase(account accounts.Account, passphrase
 	return w.SignTx(account, tx, chainID)
 }
 
-// resolveVersion retrieves the current version of the Ethereum wallet app running
+// ledgerVersion retrieves the current version of the Ethereum wallet app running
-// on the Ledger wallet and caches it for future reference.
+// on the Ledger wallet.
 //
 // The version retrieval protocol is defined as follows:
 //
@@ -484,21 +641,22 @@ func (w *ledgerWallet) SignTxWithPassphrase(account accounts.Account, passphrase
 //   Application major version                          | 1 byte
 //   Application minor version                          | 1 byte
 //   Application patch version                          | 1 byte
-func (wallet *ledgerWallet) resolveVersion() error {
+func (w *ledgerWallet) ledgerVersion() ([3]byte, error) {
 	// Send the request and wait for the response
-	reply, err := wallet.exchange(ledgerOpGetConfiguration, 0, 0, nil)
+	reply, err := w.ledgerExchange(ledgerOpGetConfiguration, 0, 0, nil)
 	if err != nil {
-		return err
+		return [3]byte{}, err
 	}
 	if len(reply) != 4 {
-		return errors.New("reply not of correct size")
+		return [3]byte{}, errors.New("reply not of correct size")
 	}
 	// Cache the version for future reference
-	copy(wallet.version[:], reply[1:])
+	var version [3]byte
-	return nil
+	copy(version[:], reply[1:])
+	return version, nil
 }
 
-// deriveAddress retrieves the currently active Ethereum address from a Ledger
+// ledgerDerive retrieves the currently active Ethereum address from a Ledger
 // wallet at the specified derivation path.
 //
 // The address derivation protocol is defined as follows:
@@ -529,7 +687,7 @@ func (wallet *ledgerWallet) resolveVersion() error {
 //   Ethereum address length | 1 byte
 //   Ethereum address        | 40 bytes hex ascii
 //   Chain code if requested | 32 bytes
-func (w *ledgerWallet) deriveAddress(derivationPath []uint32) (common.Address, error) {
+func (w *ledgerWallet) ledgerDerive(derivationPath []uint32) (common.Address, error) {
 	// Flatten the derivation path into the Ledger request
 	path := make([]byte, 1+4*len(derivationPath))
 	path[0] = byte(len(derivationPath))
@@ -537,7 +695,7 @@ func (w *ledgerWallet) deriveAddress(derivationPath []uint32) (common.Address, e
 		binary.BigEndian.PutUint32(path[1+4*i:], component)
 	}
 	// Send the request and wait for the response
-	reply, err := w.exchange(ledgerOpRetrieveAddress, ledgerP1DirectlyFetchAddress, ledgerP2DiscardAddressChainCode, path)
+	reply, err := w.ledgerExchange(ledgerOpRetrieveAddress, ledgerP1DirectlyFetchAddress, ledgerP2DiscardAddressChainCode, path)
 	if err != nil {
 		return common.Address{}, err
 	}
@@ -559,8 +717,8 @@ func (w *ledgerWallet) deriveAddress(derivationPath []uint32) (common.Address, e
 	return address, nil
 }
 
-// sign sends the transaction to the Ledger wallet, and waits for the user to
+// ledgerSign sends the transaction to the Ledger wallet, and waits for the user
-// confirm or deny the transaction.
+// to confirm or deny the transaction.
 //
 // The transaction signing protocol is defined as follows:
 //
@@ -593,7 +751,7 @@ func (w *ledgerWallet) deriveAddress(derivationPath []uint32) (common.Address, e
 //   signature V | 1 byte
 //   signature R | 32 bytes
 //   signature S | 32 bytes
-func (w *ledgerWallet) sign(derivationPath []uint32, address common.Address, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
+func (w *ledgerWallet) ledgerSign(derivationPath []uint32, address common.Address, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
 	// We need to modify the timeouts to account for user feedback
 	defer func(old time.Duration) { w.device.ReadTimeout = old }(w.device.ReadTimeout)
 	w.device.ReadTimeout = time.Minute
@@ -632,7 +790,7 @@ func (w *ledgerWallet) sign(derivationPath []uint32, address common.Address, tx
 			chunk = len(payload)
 		}
 		// Send the chunk over, ensuring it's processed correctly
-		reply, err = w.exchange(ledgerOpSignTransaction, op, 0, payload[:chunk])
+		reply, err = w.ledgerExchange(ledgerOpSignTransaction, op, 0, payload[:chunk])
 		if err != nil {
 			return nil, err
 		}
@@ -669,8 +827,8 @@ func (w *ledgerWallet) sign(derivationPath []uint32, address common.Address, tx
 	return signed, nil
 }
 
-// exchange performs a data exchange with the Ledger wallet, sending it a message
+// ledgerExchange performs a data exchange with the Ledger wallet, sending it a
-// and retrieving the response.
+// message and retrieving the response.
 //
 // The common transport header is defined as follows:
 //
@@ -702,7 +860,7 @@ func (w *ledgerWallet) sign(derivationPath []uint32, address common.Address, tx
 //  APDU P2                  | 1 byte
 //  APDU length              | 1 byte
 //  Optional APDU data       | arbitrary
-func (w *ledgerWallet) exchange(opcode ledgerOpcode, p1 ledgerParam1, p2 ledgerParam2, data []byte) ([]byte, error) {
+func (w *ledgerWallet) ledgerExchange(opcode ledgerOpcode, p1 ledgerParam1, p2 ledgerParam2, data []byte) ([]byte, error) {
 	// Construct the message payload, possibly split into multiple chunks
 	var chunks [][]byte
 	for left := data; len(left) > 0 || len(chunks) == 0; {
@@ -731,7 +889,7 @@ func (w *ledgerWallet) exchange(opcode ledgerOpcode, p1 ledgerParam1, p2 ledgerP
 		msg := append(header, chunk...)
 
 		// Send over to the device
-		if glog.V(logger.Core) {
+		if glog.V(logger.Detail) {
 			glog.Infof("-> %03d.%03d: %x", w.device.Bus, w.device.Address, msg)
 		}
 		if _, err := w.input.Write(msg); err != nil {
@@ -746,7 +904,7 @@ func (w *ledgerWallet) exchange(opcode ledgerOpcode, p1 ledgerParam1, p2 ledgerP
 		if _, err := io.ReadFull(w.output, chunk); err != nil {
 			return nil, err
 		}
-		if glog.V(logger.Core) {
+		if glog.V(logger.Detail) {
 			glog.Infof("<- %03d.%03d: %x", w.device.Bus, w.device.Address, chunk)
 		}
 		// Make sure the transport header matches

cmd/geth/main.go

@@ -269,6 +269,13 @@ func startNode(ctx *cli.Context, stack *node.Node) {
 		}
 		stateReader := ethclient.NewClient(rpcClient)
 
+		// Open and self derive any wallets already attached
+		for _, wallet := range stack.AccountManager().Wallets() {
+			if err := wallet.Open(""); err != nil {
+				glog.V(logger.Warn).Infof("Failed to open wallet %s: %v", wallet.URL(), err)
+			}
+			wallet.SelfDerive(accounts.DefaultBaseDerivationPath, stateReader)
+		}
 		// Listen for wallet event till termination
 		for event := range events {
 			if event.Arrive {
@@ -280,6 +287,7 @@ func startNode(ctx *cli.Context, stack *node.Node) {
 				event.Wallet.SelfDerive(accounts.DefaultBaseDerivationPath, stateReader)
 			} else {
 				glog.V(logger.Info).Infof("Old wallet dropped:  %s", event.Wallet.URL())
+				event.Wallet.Close()
 			}
 		}
 	}()