node: remove dependency on wallet backend packages

accounts/manager.go

@@ -25,6 +25,10 @@ import (
  "github.com/ethereum/go-ethereum/event"
 )
 
+// managerSubBufferSize determines how many incoming wallet events
+// the manager will buffer in its channel.
+const managerSubBufferSize = 50
+
 // Config contains the settings of the global account manager.
 //
 // TODO(rjl493456442, karalabe, holiman): Get rid of this when account management
@@ -33,18 +37,27 @@ type Config struct {
  InsecureUnlockAllowed bool // Whether account unlocking in insecure environment is allowed
 }
 
+// newBackendEvent lets the manager know it should
+// track the given backend for wallet updates.
+type newBackendEvent struct {
+ backend Backend
+ processed chan struct{} // Informs event emitter that backend has been integrated
+}
+
 // Manager is an overarching account manager that can communicate with various
 // backends for signing transactions.
 type Manager struct {
- config *Config // Global account manager configurations
- backends map[reflect.Type][]Backend // Index of backends currently registered
- updaters []event.Subscription // Wallet update subscriptions for all backends
- updates chan WalletEvent // Subscription sink for backend wallet changes
- wallets []Wallet // Cache of all wallets from all registered backends
+ config *Config // Global account manager configurations
+ backends map[reflect.Type][]Backend // Index of backends currently registered
+ updaters []event.Subscription // Wallet update subscriptions for all backends
+ updates chan WalletEvent // Subscription sink for backend wallet changes
+ newBackends chan newBackendEvent // Incoming backends to be tracked by the manager
+ wallets []Wallet // Cache of all wallets from all registered backends
 
  feed event.Feed // Wallet feed notifying of arrivals/departures
 
  quit chan chan error
+ term chan struct{} // Channel is closed upon termination of the update loop
  lock sync.RWMutex
 }
 
@@ -57,20 +70,22 @@ func NewManager(config *Config, backends ...Backend) *Manager {
  wallets = merge(wallets, backend.Wallets()...)
  }
  // Subscribe to wallet notifications from all backends
- updates := make(chan WalletEvent, 4*len(backends))
+ updates := make(chan WalletEvent, managerSubBufferSize)
 
  subs := make([]event.Subscription, len(backends))
  for i, backend := range backends {
  subs[i] = backend.Subscribe(updates)
  }
  // Assemble the account manager and return
  am := &Manager{
- config: config,
- backends: make(map[reflect.Type][]Backend),
- updaters: subs,
- updates: updates,
- wallets: wallets,
- quit: make(chan chan error),
+ config: config,
+ backends: make(map[reflect.Type][]Backend),
+ updaters: subs,
+ updates: updates,
+ newBackends: make(chan newBackendEvent),
+ wallets: wallets,
+ quit: make(chan chan error),
+ term: make(chan struct{}),
  }
  for _, backend := range backends {
  kind := reflect.TypeOf(backend)
@@ -93,6 +108,14 @@ func (am *Manager) Config() *Config {
  return am.config
 }
 
+// AddBackend starts the tracking of an additional backend for wallet updates.
+// cmd/geth assumes once this func returns the backends have been already integrated.
+func (am *Manager) AddBackend(backend Backend) {
+ done := make(chan struct{})
+ am.newBackends <- newBackendEvent{backend, done}
+ <-done
+}
+
 // update is the wallet event loop listening for notifications from the backends
 // and updating the cache of wallets.
 func (am *Manager) update() {
@@ -122,17 +145,32 @@ func (am *Manager) update() {
 
  // Notify any listeners of the event
  am.feed.Send(event)
-
+ case event := <-am.newBackends:
+ am.lock.Lock()
+ // Update caches
+ backend := event.backend
+ am.wallets = merge(am.wallets, backend.Wallets()...)
+ am.updaters = append(am.updaters, backend.Subscribe(am.updates))
+ kind := reflect.TypeOf(backend)
+ am.backends[kind] = append(am.backends[kind], backend)
+ am.lock.Unlock()
+ close(event.processed)
  case errc := <-am.quit:
  // Manager terminating, return
  errc <- nil
+ // Signals event emitters the loop is not receiving values
+ // to prevent them from getting stuck.
+ close(am.term)
  return
  }
  }
 }
 
 // Backends retrieves the backend(s) with the given type from the account manager.
 func (am *Manager) Backends(kind reflect.Type) []Backend {
+ am.lock.RLock()
+ defer am.lock.RUnlock()
+
  return am.backends[kind]
 }
 

cmd/geth/accountcmd.go

@@ -268,11 +268,16 @@ func accountCreate(ctx *cli.Context) error {
  }
  }
  utils.SetNodeConfig(ctx, &cfg.Node)
- scryptN, scryptP, keydir, err := cfg.Node.AccountConfig()
-
+ keydir, err := cfg.Node.KeyDirConfig()
  if err != nil {
  utils.Fatalf("Failed to read configuration: %v", err)
  }
+ scryptN := keystore.StandardScryptN
+ scryptP := keystore.StandardScryptP
+ if cfg.Node.UseLightweightKDF {
+ scryptN = keystore.LightScryptN
+ scryptP = keystore.LightScryptP
+ }
 
  password := utils.GetPassPhraseWithList("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx))
 

cmd/geth/config.go

@@ -27,6 +27,10 @@ import (
 
  "gopkg.in/urfave/cli.v1"
 
+ "github.com/ethereum/go-ethereum/accounts/external"
+ "github.com/ethereum/go-ethereum/accounts/keystore"
+ "github.com/ethereum/go-ethereum/accounts/scwallet"
+ "github.com/ethereum/go-ethereum/accounts/usbwallet"
  "github.com/ethereum/go-ethereum/cmd/utils"
  "github.com/ethereum/go-ethereum/eth/catalyst"
  "github.com/ethereum/go-ethereum/eth/ethconfig"
@@ -135,6 +139,11 @@ func makeConfigNode(ctx *cli.Context) (*node.Node, gethConfig) {
  if err != nil {
  utils.Fatalf("Failed to create the protocol stack: %v", err)
  }
+ // Node doesn't by default populate account manager backends
+ if err := setAccountManagerBackends(stack); err != nil {
+ utils.Fatalf("Failed to set account manager backends: %v", err)
+ }
+
  utils.SetEthConfig(ctx, stack, &cfg.Eth)
  if ctx.GlobalIsSet(utils.EthStatsURLFlag.Name) {
  cfg.Ethstats.URL = ctx.GlobalString(utils.EthStatsURLFlag.Name)
@@ -245,3 +254,62 @@ func deprecated(field string) bool {
  return false
  }
 }
+
+func setAccountManagerBackends(stack *node.Node) error {
+ conf := stack.Config()
+ am := stack.AccountManager()
+ keydir := stack.KeyStoreDir()
+ scryptN := keystore.StandardScryptN
+ scryptP := keystore.StandardScryptP
+ if conf.UseLightweightKDF {
+ scryptN = keystore.LightScryptN
+ scryptP = keystore.LightScryptP
+ }
+
+ // Assemble the supported backends
+ if len(conf.ExternalSigner) > 0 {
+ log.Info("Using external signer", "url", conf.ExternalSigner)
+ if extapi, err := external.NewExternalBackend(conf.ExternalSigner); err == nil {
+ am.AddBackend(extapi)
+ return nil
+ } else {
+ return fmt.Errorf("error connecting to external signer: %v", err)
+ }
+ }
+
+ // For now, we're using EITHER external signer OR local signers.
+ // If/when we implement some form of lockfile for USB and keystore wallets,
+ // we can have both, but it's very confusing for the user to see the same
+ // accounts in both externally and locally, plus very racey.
+ am.AddBackend(keystore.NewKeyStore(keydir, scryptN, scryptP))
+ if conf.USB {
+ // Start a USB hub for Ledger hardware wallets
+ if ledgerhub, err := usbwallet.NewLedgerHub(); err != nil {
+ log.Warn(fmt.Sprintf("Failed to start Ledger hub, disabling: %v", err))
+ } else {
+ am.AddBackend(ledgerhub)
+ }
+ // Start a USB hub for Trezor hardware wallets (HID version)
+ if trezorhub, err := usbwallet.NewTrezorHubWithHID(); err != nil {
+ log.Warn(fmt.Sprintf("Failed to start HID Trezor hub, disabling: %v", err))
+ } else {
+ am.AddBackend(trezorhub)
+ }
+ // Start a USB hub for Trezor hardware wallets (WebUSB version)
+ if trezorhub, err := usbwallet.NewTrezorHubWithWebUSB(); err != nil {
+ log.Warn(fmt.Sprintf("Failed to start WebUSB Trezor hub, disabling: %v", err))
+ } else {
+ am.AddBackend(trezorhub)
+ }
+ }
+ if len(conf.SmartCardDaemonPath) > 0 {
+ // Start a smart card hub
+ if schub, err := scwallet.NewHub(conf.SmartCardDaemonPath, scwallet.Scheme, keydir); err != nil {
+ log.Warn(fmt.Sprintf("Failed to start smart card hub, disabling: %v", err))
+ } else {
+ am.AddBackend(schub)
+ }
+ }
+
+ return nil
+}

node/config.go

@@ -26,11 +26,6 @@ import (
  "strings"
  "sync"
 
- "github.com/ethereum/go-ethereum/accounts"
- "github.com/ethereum/go-ethereum/accounts/external"
- "github.com/ethereum/go-ethereum/accounts/keystore"
- "github.com/ethereum/go-ethereum/accounts/scwallet"
- "github.com/ethereum/go-ethereum/accounts/usbwallet"
  "github.com/ethereum/go-ethereum/common"
  "github.com/ethereum/go-ethereum/crypto"
  "github.com/ethereum/go-ethereum/log"
@@ -429,15 +424,8 @@ func (c *Config) parsePersistentNodes(w *bool, path string) []*enode.Node {
  return nodes
 }
 
-// AccountConfig determines the settings for scrypt and keydirectory
-func (c *Config) AccountConfig() (int, int, string, error) {
- scryptN := keystore.StandardScryptN
- scryptP := keystore.StandardScryptP
- if c.UseLightweightKDF {
- scryptN = keystore.LightScryptN
- scryptP = keystore.LightScryptP
- }
-
+// KeyDirConfig determines the settings for keydirectory
+func (c *Config) KeyDirConfig() (string, error) {
  var (
  keydir string
  err error
@@ -454,71 +442,31 @@ func (c *Config) AccountConfig() (int, int, string, error) {
  case c.KeyStoreDir != "":
  keydir, err = filepath.Abs(c.KeyStoreDir)
  }
- return scryptN, scryptP, keydir, err
+ return keydir, err
 }
 
-func makeAccountManager(conf *Config) (*accounts.Manager, string, error) {
- scryptN, scryptP, keydir, err := conf.AccountConfig()
- var ephemeral string
+// getKeyStoreDir retrieves the key directory and will create
+// and ephemeral one if necessary.
+func getKeyStoreDir(conf *Config) (string, bool, error) {
+ keydir, err := conf.KeyDirConfig()
+ if err != nil {
+ return "", false, err
+ }
+ isEphemeral := false
  if keydir == "" {
  // There is no datadir.
  keydir, err = ioutil.TempDir("", "go-ethereum-keystore")
- ephemeral = keydir
+ isEphemeral = true
  }
 
  if err != nil {
- return nil, "", err
+ return "", false, err
  }
  if err := os.MkdirAll(keydir, 0700); err != nil {
- return nil, "", err
- }
- // Assemble the account manager and supported backends
- var backends []accounts.Backend
- if len(conf.ExternalSigner) > 0 {
- log.Info("Using external signer", "url", conf.ExternalSigner)
- if extapi, err := external.NewExternalBackend(conf.ExternalSigner); err == nil {
- backends = append(backends, extapi)
- } else {
- return nil, "", fmt.Errorf("error connecting to external signer: %v", err)
- }
- }
- if len(backends) == 0 {
- // For now, we're using EITHER external signer OR local signers.
- // If/when we implement some form of lockfile for USB and keystore wallets,
- // we can have both, but it's very confusing for the user to see the same
- // accounts in both externally and locally, plus very racey.
- backends = append(backends, keystore.NewKeyStore(keydir, scryptN, scryptP))
- if conf.USB {
- // Start a USB hub for Ledger hardware wallets
- if ledgerhub, err := usbwallet.NewLedgerHub(); err != nil {
- log.Warn(fmt.Sprintf("Failed to start Ledger hub, disabling: %v", err))
- } else {
- backends = append(backends, ledgerhub)
- }
- // Start a USB hub for Trezor hardware wallets (HID version)
- if trezorhub, err := usbwallet.NewTrezorHubWithHID(); err != nil {
- log.Warn(fmt.Sprintf("Failed to start HID Trezor hub, disabling: %v", err))
- } else {
- backends = append(backends, trezorhub)
- }
- // Start a USB hub for Trezor hardware wallets (WebUSB version)
- if trezorhub, err := usbwallet.NewTrezorHubWithWebUSB(); err != nil {
- log.Warn(fmt.Sprintf("Failed to start WebUSB Trezor hub, disabling: %v", err))
- } else {
- backends = append(backends, trezorhub)
- }
- }
- if len(conf.SmartCardDaemonPath) > 0 {
- // Start a smart card hub
- if schub, err := scwallet.NewHub(conf.SmartCardDaemonPath, scwallet.Scheme, keydir); err != nil {
- log.Warn(fmt.Sprintf("Failed to start smart card hub, disabling: %v", err))
- } else {
- backends = append(backends, schub)
- }
- }
+ return "", false, err
  }
 
- return accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: conf.InsecureUnlockAllowed}, backends...), ephemeral, nil
+ return keydir, isEphemeral, nil
 }
 
 var warnLock sync.Mutex

node/node.go

@@ -42,7 +42,8 @@ type Node struct {
  config *Config
  accman *accounts.Manager
  log log.Logger
- ephemKeystore string // if non-empty, the key directory that will be removed by Stop
+ keyDir string // key store directory
+ keyDirTemp bool // If true, key directory will be removed by Stop
  dirLock fileutil.Releaser // prevents concurrent use of instance directory
  stop chan struct{} // Channel to wait for termination notifications
  server *p2p.Server // Currently running P2P networking layer
@@ -112,14 +113,15 @@ func New(conf *Config) (*Node, error) {
  if err := node.openDataDir(); err != nil {
  return nil, err
  }
- // Ensure that the AccountManager method works before the node has started. We rely on
- // this in cmd/geth.
- am, ephemeralKeystore, err := makeAccountManager(conf)
+ keyDir, isEphem, err := getKeyStoreDir(conf)
  if err != nil {
  return nil, err
  }
- node.accman = am
- node.ephemKeystore = ephemeralKeystore
+ node.keyDir = keyDir
+ node.keyDirTemp = isEphem
+ // Creates an empty AccountManager with no backends. Callers (e.g. cmd/geth)
+ // are required to add the backends later on.
+ node.accman = accounts.NewManager(&accounts.Config{InsecureUnlockAllowed: conf.InsecureUnlockAllowed})
 
  // Initialize the p2p server. This creates the node key and discovery databases.
  node.server.Config.PrivateKey = node.config.NodeKey()
@@ -233,8 +235,8 @@ func (n *Node) doClose(errs []error) error {
  if err := n.accman.Close(); err != nil {
  errs = append(errs, err)
  }
- if n.ephemKeystore != "" {
- if err := os.RemoveAll(n.ephemKeystore); err != nil {
+ if n.keyDirTemp {
+ if err := os.RemoveAll(n.keyDir); err != nil {
  errs = append(errs, err)
  }
  }
@@ -514,6 +516,11 @@ func (n *Node) InstanceDir() string {
  return n.config.instanceDir()
 }
 
+// KeyStoreDir retrieves the key directory
+func (n *Node) KeyStoreDir() string {
+ return n.keyDir
+}
+
 // AccountManager retrieves the account manager used by the protocol stack.
 func (n *Node) AccountManager() *accounts.Manager {
  return n.accman