cmd/utils: don't enumerate USB unless --usb is set (ethereum#22130)

USB enumeration still occured. Make sure it will only occur if --usb is set. This also deprecates the 'NoUSB' config file option in favor of a new option 'USB'.
holiman · Jan 13, 2021 · c7a6be1 · c7a6be1
1 parent 93a89b2
commit c7a6be1
Show file tree

Hide file tree

Showing 12 changed files with 21 additions and 23 deletions.
diff --git a/cmd/geth/accountcmd_test.go b/cmd/geth/accountcmd_test.go
@@ -43,13 +43,13 @@ func tmpDatadirWithKeystore(t *testing.T) string {
 }
 
 func TestAccountListEmpty(t *testing.T) {
-	geth := runGeth(t, "--nousb", "account", "list")
+	geth := runGeth(t, "account", "list")
 	geth.ExpectExit()
 }
 
 func TestAccountList(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
-	geth := runGeth(t, "--nousb", "account", "list", "--datadir", datadir)
+	geth := runGeth(t, "account", "list", "--datadir", datadir)
 	defer geth.ExpectExit()
 	if runtime.GOOS == "windows" {
 		geth.Expect(`
@@ -139,7 +139,7 @@ Fatal: Passwords do not match
 
 func TestAccountUpdate(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
-	geth := runGeth(t, "--nousb", "account", "update",
+	geth := runGeth(t, "account", "update",
 		"--datadir", datadir, "--lightkdf",
 		"f466859ead1932d743d622cb74fc058882e8648a")
 	defer geth.ExpectExit()
@@ -154,7 +154,7 @@ Repeat password: {{.InputLine "foobar2"}}
 }
 
 func TestWalletImport(t *testing.T) {
-	geth := runGeth(t, "--nousb", "wallet", "import", "--lightkdf", "testdata/guswallet.json")
+	geth := runGeth(t, "wallet", "import", "--lightkdf", "testdata/guswallet.json")
 	defer geth.ExpectExit()
 	geth.Expect(`
 !! Unsupported terminal, password will be echoed.
@@ -169,7 +169,7 @@ Address: {d4584b5f6229b7be90727b0fc8c6b91bb427821f}
 }
 
 func TestWalletImportBadPassword(t *testing.T) {
-	geth := runGeth(t, "--nousb", "wallet", "import", "--lightkdf", "testdata/guswallet.json")
+	geth := runGeth(t, "wallet", "import", "--lightkdf", "testdata/guswallet.json")
 	defer geth.ExpectExit()
 	geth.Expect(`
 !! Unsupported terminal, password will be echoed.

diff --git a/cmd/geth/consolecmd_test.go b/cmd/geth/consolecmd_test.go
@@ -42,7 +42,7 @@ func runMinimalGeth(t *testing.T, args ...string) *testgeth {
 	// --ropsten to make the 'writing genesis to disk' faster (no accounts)
 	// --networkid=1337 to avoid cache bump
 	// --syncmode=full to avoid allocating fast sync bloom
-	allArgs := []string{"--ropsten", "--nousb", "--networkid", "1337", "--syncmode=full", "--port", "0",
+	allArgs := []string{"--ropsten", "--networkid", "1337", "--syncmode=full", "--port", "0",
 		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--cache", "64"}
 	return runGeth(t, append(allArgs, args...)...)
 }

diff --git a/cmd/geth/dao_test.go b/cmd/geth/dao_test.go
@@ -115,10 +115,10 @@ func testDAOForkBlockNewChain(t *testing.T, test int, genesis string, expectBloc
 		if err := ioutil.WriteFile(json, []byte(genesis), 0600); err != nil {
 			t.Fatalf("test %d: failed to write genesis file: %v", test, err)
 		}
-		runGeth(t, "--datadir", datadir, "--nousb", "--networkid", "1337", "init", json).WaitExit()
+		runGeth(t, "--datadir", datadir, "--networkid", "1337", "init", json).WaitExit()
 	} else {
 		// Force chain initialization
-		args := []string{"--port", "0", "--nousb", "--networkid", "1337", "--maxpeers", "0", "--nodiscover", "--nat", "none", "--ipcdisable", "--datadir", datadir}
+		args := []string{"--port", "0", "--networkid", "1337", "--maxpeers", "0", "--nodiscover", "--nat", "none", "--ipcdisable", "--datadir", datadir}
 		runGeth(t, append(args, []string{"--exec", "2+2", "console"}...)...).WaitExit()
 	}
 	// Retrieve the DAO config flag from the database

diff --git a/cmd/geth/genesis_test.go b/cmd/geth/genesis_test.go
@@ -81,10 +81,10 @@ func TestCustomGenesis(t *testing.T) {
 		if err := ioutil.WriteFile(json, []byte(tt.genesis), 0600); err != nil {
 			t.Fatalf("test %d: failed to write genesis file: %v", i, err)
 		}
-		runGeth(t, "--nousb", "--datadir", datadir, "init", json).WaitExit()
+		runGeth(t, "--datadir", datadir, "init", json).WaitExit()
 
 		// Query the custom genesis block
-		geth := runGeth(t, "--nousb", "--networkid", "1337", "--syncmode=full",
+		geth := runGeth(t, "--networkid", "1337", "--syncmode=full",
 			"--datadir", datadir, "--maxpeers", "0", "--port", "0",
 			"--nodiscover", "--nat", "none", "--ipcdisable",
 			"--exec", tt.query, "console")

diff --git a/cmd/geth/les_test.go b/cmd/geth/les_test.go
@@ -130,7 +130,7 @@ var nextIPC = uint32(0)
 
 func startGethWithIpc(t *testing.T, name string, args ...string) *gethrpc {
 	ipcName := fmt.Sprintf("geth-%d.ipc", atomic.AddUint32(&nextIPC, 1))
-	args = append([]string{"--networkid=42", "--port=0", "--nousb", "--ipcpath", ipcName}, args...)
+	args = append([]string{"--networkid=42", "--port=0", "--ipcpath", ipcName}, args...)
 	t.Logf("Starting %v with rpc: %v", name, args)
 
 	g := &gethrpc{
@@ -148,7 +148,7 @@ func startGethWithIpc(t *testing.T, name string, args ...string) *gethrpc {
 }
 
 func initGeth(t *testing.T) string {
-	args := []string{"--nousb", "--networkid=42", "init", "./testdata/clique.json"}
+	args := []string{"--networkid=42", "init", "./testdata/clique.json"}
 	t.Logf("Initializing geth: %v ", args)
 	g := runGeth(t, args...)
 	datadir := g.Datadir
@@ -159,7 +159,7 @@ func initGeth(t *testing.T) string {
 func startLightServer(t *testing.T) *gethrpc {
 	datadir := initGeth(t)
 	t.Logf("Importing keys to geth")
-	runGeth(t, "--nousb", "--datadir", datadir, "--password", "./testdata/password.txt", "account", "import", "./testdata/key.prv", "--lightkdf").WaitExit()
+	runGeth(t, "--datadir", datadir, "--password", "./testdata/password.txt", "account", "import", "./testdata/key.prv", "--lightkdf").WaitExit()
 	account := "0x02f0d131f1f97aef08aec6e3291b957d9efe7105"
 	server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1", "--verbosity=4")
 	return server

diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go
@@ -1233,12 +1233,12 @@ func SetNodeConfig(ctx *cli.Context, cfg *node.Config) {
 	if ctx.GlobalIsSet(LightKDFFlag.Name) {
 		cfg.UseLightweightKDF = ctx.GlobalBool(LightKDFFlag.Name)
 	}
-	if ctx.GlobalIsSet(USBFlag.Name) {
-		cfg.NoUSB = !ctx.GlobalBool(USBFlag.Name)
-	}
-	if ctx.GlobalIsSet(NoUSBFlag.Name) {
+	if ctx.GlobalIsSet(NoUSBFlag.Name) || cfg.NoUSB {
 		log.Warn("Option nousb is deprecated and USB is deactivated by default. Use --usb to enable")
 	}
+	if ctx.GlobalIsSet(USBFlag.Name) {
+		cfg.USB = ctx.GlobalBool(USBFlag.Name)
+	}
 	if ctx.GlobalIsSet(InsecureUnlockAllowedFlag.Name) {
 		cfg.InsecureUnlockAllowed = ctx.GlobalBool(InsecureUnlockAllowedFlag.Name)
 	}

diff --git a/miner/stress_clique.go b/miner/stress_clique.go
@@ -178,7 +178,6 @@ func makeSealer(genesis *core.Genesis) (*node.Node, *eth.Ethereum, error) {
 			NoDiscovery: true,
 			MaxPeers:    25,
 		},
-		NoUSB: true,
 	}
 	// Start the node and configure a full Ethereum node on it
 	stack, err := node.New(config)

diff --git a/miner/stress_ethash.go b/miner/stress_ethash.go
@@ -155,7 +155,6 @@ func makeMiner(genesis *core.Genesis) (*node.Node, *eth.Ethereum, error) {
 			NoDiscovery: true,
 			MaxPeers:    25,
 		},
-		NoUSB:             true,
 		UseLightweightKDF: true,
 	}
 	// Create the node and configure a full Ethereum node on it

diff --git a/node/api_test.go b/node/api_test.go
@@ -248,7 +248,6 @@ func TestStartRPC(t *testing.T) {
 			// Apply some sane defaults.
 			config := test.cfg
 			// config.Logger = testlog.Logger(t, log.LvlDebug)
-			config.NoUSB = true
 			config.P2P.NoDiscovery = true
 
 			// Create Node.

diff --git a/node/config.go b/node/config.go
@@ -95,6 +95,9 @@ type Config struct {
 	// NoUSB disables hardware wallet monitoring and connectivity.
 	NoUSB bool `toml:",omitempty"`
 
+	// USB enables hardware wallet monitoring and connectivity.
+	USB bool `toml:",omitempty"`
+
 	// SmartCardDaemonPath is the path to the smartcard daemon's socket
 	SmartCardDaemonPath string `toml:",omitempty"`
 
@@ -476,7 +479,7 @@ func makeAccountManager(conf *Config) (*accounts.Manager, string, error) {
 		// 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 {
+		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))

diff --git a/p2p/simulations/adapters/exec.go b/p2p/simulations/adapters/exec.go
@@ -115,7 +115,6 @@ func (e *ExecAdapter) NewNode(config *NodeConfig) (Node, error) {
 	conf.Stack.P2P.EnableMsgEvents = config.EnableMsgEvents
 	conf.Stack.P2P.NoDiscovery = true
 	conf.Stack.P2P.NAT = nil
-	conf.Stack.NoUSB = true
 
 	// Listen on a localhost port, which we set when we
 	// initialise NodeConfig (usually a random port)

diff --git a/p2p/simulations/adapters/inproc.go b/p2p/simulations/adapters/inproc.go
@@ -100,7 +100,6 @@ func (s *SimAdapter) NewNode(config *NodeConfig) (Node, error) {
 			EnableMsgEvents: config.EnableMsgEvents,
 		},
 		ExternalSigner: config.ExternalSigner,
-		NoUSB:          true,
 		Logger:         log.New("node.id", id.String()),
 	})
 	if err != nil {