Skip to content

Commit

Permalink
node: remove dependency on wallet backend packages (ethereum#23019)
Browse files Browse the repository at this point in the history 
* accounts: new AddBackends method in manager

* node,cmd/geth: mv accman backend init to cmd/geth

* node,cmd/geth: mv scrypt config downstreawm from node

* accounts: use static buffer size for accman sub chan

minor fix

* accounts,cmd/geth: update accman backends through its event loop

* accounts,node: add comments

* accounts: un-export newBackendEvent

* accounts: use chan instead of wg in newBlockEvent

* node: rename isKeyDirEphem

* accounts,cmd: AddBackends->AddBackend

* accounts: fix potential blocking when adding backend
  • Loading branch information
@gzliudan
gzliudan committed Jan 17, 2025
1 parent a4c4095 commit 034025a
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 97 deletions.
64 changes: 51 additions & 13 deletions accounts/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand All @@ -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)
Expand All @@ -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() {
Expand Down Expand Up @@ -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]
}

Expand Down
9 changes: 7 additions & 2 deletions cmd/XDC/accountcmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
56 changes: 56 additions & 0 deletions cmd/XDC/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
73 changes: 15 additions & 58 deletions node/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Loading

0 comments on commit 034025a

Please sign in to comment.