console, cli, api fixes #1840
console, cli, api fixes #1840
Conversation
Updated: Wed Oct 21 15:04:14 UTC 2015 |
|
@zelig, review done. |
| return passwordRepl | ||
| } else { | ||
| return input | ||
| func postprocess(input string) (string, bool) { |
There was a problem hiding this comment.
This function doesn't do any real post processing and the returned string is not actually necessary. Consider changing it to something like func mustLogInHistory(input string) bool.
There was a problem hiding this comment.
how about forgetting this automation entirely, pass interactivity as a parameter to modules and simply enforce interactive use on the console(s).
There was a problem hiding this comment.
I think @ethers (#1697) asked for this feature not to hide passwords but to prevent dangerous commands not to end up in the history from which they can accidentally be executed again when the users browses through history. That means that besides password interaction also transaction confirmation from the console should be enabled since transactions will have irreversible consequences.
|
@bas-vk I agree with all points, thanks for the thorough review |
Current coverage is
|
| @@ -399,6 +400,16 @@ func run(ctx *cli.Context) { | |||
|
|
|||
| ethereum, err := eth.New(cfg) | |||
| if err != nil { | |||
| var ok bool | |||
| for _, no := range datadirInUseErrNos { | |||
| if uint(err.(syscall.Errno)) == no { | |||
There was a problem hiding this comment.
This still doesn't work.
ethdb.NewLDBDatabase will return a syscall.Errno error for which you can check. In backend.go these errors are converted to an *errors.errorString instance.
chainDb, err := newdb(filepath.Join(config.DataDir, "chaindata"))
if err != nil {
return nil, fmt.Errorf("blockchain db err: %v", err)
}
The check in main.go for an errno wil then obviously fail.
Second problem is that the check is only added to the run subcommand. Users who will start the console (or any other subcommand) will still get the old error message when another instance already claimed the database datadir.
|
What did you end up doing for #1697? |
| @@ -281,6 +286,18 @@ func New(config *Config) (*Ethereum, error) { | |||
| // Open the chain database and perform any upgrades needed | |||
| chainDb, err := newdb(filepath.Join(config.DataDir, "chaindata")) | |||
| if err != nil { | |||
| var ok bool | |||
| errno := uint(err.(syscall.Errno)) | |||
| fmt.Printf("encountered error %d: %v\n", errno, err) | |||
There was a problem hiding this comment.
This is probably a debug line and can be removed?
|
Apart from the minor issue I'm ok 👍 |
* lines with leading space are ommitted from history * exit processed even with whitespace around * all whitespace lines (not only empty ones) are ignored add 7 missing commands to admin api autocomplete registrar: methods now return proper error if reg addresses are not set. fixes #1457 rpc/console: fix personal.newAccount() regression. Now all comms accept interactive password registrar: add registrar tests for errors crypto: catch AES decryption error on presale wallet import + fix error msg format. fixes #1580 CLI: improve error message when starting a second instance of geth. fixes #1564 cli/accounts: unlock multiple accounts. fixes #1785 * make unlocking multiple accounts work with inline <() fd * passwdfile now correctly read only once * improve logs * fix CLI help text for unlocking fix regression with docRoot / admin API * docRoot/jspath passed to rpc/api ParseApis, which passes onto adminApi * docRoot field for JS console in order to pass when RPC is (re)started * improve flag desc for jspath common/docserver: catch http errors from response fix rpc/api tests common/natspec: fix end to end test (skipped because takes 8s) registrar: fix major regression: * deploy registrars on frontier * register HashsReg and UrlHint in GlobalRegistrar. * set all 3 contract addresses in code * zero out addresses first in tests
….12.2 Big merge from v1.10.16 to v1.12.2
This PR addresses several minor bugs, regressions, inconsistencies relating to geth UI including console, cli, rpc api