diff --git a/accounts/manager.go b/accounts/manager.go index e2ad6ddc8c..a1f3afed2f 100644 --- a/accounts/manager.go +++ b/accounts/manager.go @@ -25,6 +25,10 @@ import ( "github.com/XinFinOrg/XDPoSChain/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,7 +70,7 @@ 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 { @@ -65,12 +78,14 @@ func NewManager(config *Config, backends ...Backend) *Manager { } // 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,10 +145,22 @@ 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 } } @@ -133,6 +168,9 @@ func (am *Manager) update() { // 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] } diff --git a/cmd/XDC/accountcmd.go b/cmd/XDC/accountcmd.go index ae89a698ea..252a109b3d 100644 --- a/cmd/XDC/accountcmd.go +++ b/cmd/XDC/accountcmd.go @@ -302,11 +302,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 := getPassPhrase("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx)) diff --git a/cmd/XDC/config.go b/cmd/XDC/config.go index ef0dbbb00d..78c63fdc6a 100644 --- a/cmd/XDC/config.go +++ b/cmd/XDC/config.go @@ -28,6 +28,9 @@ import ( "unicode" "github.com/XinFinOrg/XDPoSChain/XDCx" + "github.com/XinFinOrg/XDPoSChain/accounts/keystore" + "github.com/XinFinOrg/XDPoSChain/accounts/scwallet" + "github.com/XinFinOrg/XDPoSChain/accounts/usbwallet" "github.com/XinFinOrg/XDPoSChain/cmd/utils" "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/eth/ethconfig" @@ -207,6 +210,11 @@ func makeConfigNode(ctx *cli.Context) (*node.Node, XDCConfig) { 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.IsSet(utils.EthStatsURLFlag.Name) { cfg.Ethstats.URL = ctx.String(utils.EthStatsURLFlag.Name) @@ -335,3 +343,51 @@ func applyMetricConfig(ctx *cli.Context, cfg *XDCConfig) { } } } + +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 + } + + // 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 +} diff --git a/node/config.go b/node/config.go index f6a261ee4b..186d0bc91f 100644 --- a/node/config.go +++ b/node/config.go @@ -24,10 +24,6 @@ import ( "runtime" "strings" - "github.com/XinFinOrg/XDPoSChain/accounts" - "github.com/XinFinOrg/XDPoSChain/accounts/keystore" - "github.com/XinFinOrg/XDPoSChain/accounts/scwallet" - "github.com/XinFinOrg/XDPoSChain/accounts/usbwallet" "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/crypto" "github.com/XinFinOrg/XDPoSChain/log" @@ -389,15 +385,8 @@ func (c *Config) parsePersistentNodes(path string) []*discover.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 @@ -414,61 +403,29 @@ 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 = os.MkdirTemp("", "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(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.NoUSB { - // 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 } diff --git a/node/node.go b/node/node.go index 32201efa3b..d5d1ca433f 100644 --- a/node/node.go +++ b/node/node.go @@ -41,9 +41,12 @@ import ( // Node is a container on which services can be registered. type Node struct { - eventmux *event.TypeMux // Event multiplexer used between the services of a stack - config *Config - accman *accounts.Manager + eventmux *event.TypeMux // Event multiplexer used between the services of a stack + config *Config + accman *accounts.Manager + log log.Logger + keyDir string // key store directory + keyDirTemp bool // If true, key directory will be removed by Stop ephemeralKeystore string // if non-empty, the key directory that will be removed by Stop instanceDirLock flock.Releaser // prevents concurrent use of instance directory @@ -75,8 +78,6 @@ type Node struct { stop chan struct{} // Channel to wait for termination notifications lock sync.RWMutex - - log log.Logger } const ( @@ -96,6 +97,10 @@ func New(conf *Config) (*Node, error) { } conf.DataDir = absdatadir } + if conf.Logger == nil { + conf.Logger = log.New() + } + // Ensure that the instance name doesn't cause weird conflicts with // other files in the data directory. if strings.ContainsAny(conf.Name, `/\`) { @@ -107,28 +112,28 @@ func New(conf *Config) (*Node, error) { if strings.HasSuffix(conf.Name, ".ipc") { return nil, errors.New(`Config.Name cannot end in ".ipc"`) } - // Ensure that the AccountManager method works before the node has started. - // We rely on this in cmd/geth. - am, ephemeralKeystore, err := makeAccountManager(conf) + + node := &Node{ + config: conf, + eventmux: new(event.TypeMux), + log: conf.Logger, + serviceFuncs: []ServiceConstructor{}, + ipcEndpoint: conf.IPCEndpoint(), + httpEndpoint: conf.HTTPEndpoint(), + wsEndpoint: conf.WSEndpoint(), + } + + keyDir, isEphem, err := getKeyStoreDir(conf) if err != nil { return nil, err } - if conf.Logger == nil { - conf.Logger = log.New() - } - // Note: any interaction with Config that would create/touch files - // in the data directory or instance directory is delayed until Start. - return &Node{ - accman: am, - ephemeralKeystore: ephemeralKeystore, - config: conf, - serviceFuncs: []ServiceConstructor{}, - ipcEndpoint: conf.IPCEndpoint(), - httpEndpoint: conf.HTTPEndpoint(), - wsEndpoint: conf.WSEndpoint(), - eventmux: new(event.TypeMux), - log: conf.Logger, - }, nil + 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}) + + return node, nil } // Close stops the Node and releases resources acquired in @@ -143,6 +148,12 @@ func (n *Node) Close() error { if err := n.accman.Close(); err != nil { errs = append(errs, err) } + if n.keyDirTemp { + if err := os.RemoveAll(n.keyDir); err != nil { + errs = append(errs, err) + } + } + // Report any errors that might have occurred switch len(errs) { case 0: @@ -569,6 +580,11 @@ func (n *Node) RPCHandler() (*rpc.Server, error) { return n.inprocHandler, nil } +// Config returns the configuration of node. +func (n *Node) Config() *Config { + return n.config +} + // Server retrieves the currently running P2P network layer. This method is meant // only to inspect fields of the currently running server, life cycle management // should be left to this Node entity. @@ -608,6 +624,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