raedahgroup / godcr-old Goto Github PK

This first part is fine.

$ ./dcrcli kelp
dcrcli: 'kelp' is not a supported command.

The below usage information does not need to be printed on the above failure.

Usage:
  dcrcli [OPTIONS] <command> [<args...>]

Available commands:

     balance | show your balance
        send | send a transaction
 send-custom | send a transaction, manually selecting inputs from unspent outputs
     receive | show your address to receive funds

Should only be a single display of the help menu text when using the -h flag.

The readme should contain more (detailed) information on the dependency of dcrcli on actively running dcrd and dcrwallet binaries

It should also show (or provide links to information on) how to download, install and run dcrd and dcrwallet

It's worth pointing out how to setup dcrd and dcrwallet to use testnet and how to configure the rpc options. Would make it easier for devs joining the project to get started more quickly.

Currently, dcrcli receive command works with an optional argument to specify the account number for which to generate a receive address.

We should instead accept account name. dcrwallet rpc has a method to retrieve account number using account name. And users would/may find it easier to remember account names rather than account numbers.

Update readme to show that dcrlibwallet is another method of connecting to decred wallets in godcr. Readme should also reflect that this is the default method of connecting to a wallet but godcr can be configured to use dcrwallet rpc instead by setting the right value in config or command-line options (see godcr -h).

Also mention the desktop interface mode for running godcr. The readme should indicate that is is very much still a work in progress.

The readme currently mentions using godcr --init to create a config file. The PR bearing this functionality has not been merged yet and may not be merged very quickly. The readme should rather instruct users to make a copy of the sample-dcrcli.conf file at the default config file location. The default config file location is different for each OS, and can be gotten by running godcr -h.

Also, the sample-dcrcli.conf needs to be renamed to godcr.conf. The content of the file should also be cross-checked with the Config struct in app/config/config.go to ensure there are no deprecated options in the file.

Add code to properly terminate the opened connection to dcrwallet daemon over rpc after running a cli command or after the http server is shutdown

The connection is currently auto-terminated but a proper cleanup and shutdown is in order

$ ./dcrcli --http
Running in http mode
Cannot start http server. Server address not set

expected

$ ./dcrcli --http
Running http server at 127.0.0.1:1234
CTRL-C to exit.

Could potentially provide an interface for creating multiple wallets within dcrcli, but don't want to get ahead of ourselves. That could be a backlog wishlist. The wallets database should probably/preferably be created in dcrcli app data folder/wallets. For linux, that would default to ~./dcrcli/wallets unless the user specifies an alternate app data directory

Currently, dcrcli receive command requires account number argument and returns error message if no account number argument is supplied.

Seeing as users should not really be expected to memorize the account numbers for all the accounts in their wallet, it might help to give users a prompt to select the account they want to receive funds for. This functionality is already implemented in the web server package and similar functionality exists for the dcrcli send command as described in #10

The dcrcli receive command should retain its ability to accept an account number argument though and should only prompt user for selection if no argument is provided or an invalid argument was provided

1. send cmd asks for source account, should list off account names, show and use 'default' as the default account.
2. send cmd Needs to return a readable error if input is not valid.
3. send cmd, If error is shown, it should after that again show the input prompt.
4. send cmd, I get 'error: Invalid addressDestination address' even when inputting a valid address. Does it support testnet? Could specify which net is being used in the input address question.

Blockchain sync feature is introduced in PR #88. The feature should be optimized to display a progress bar during sync operations rather than logging messages to terminal. Same should be done for the web interface.

At the time this issue was created, work is ongoing on dcrandroid to add similar feature to the decred wallet android app. Perhaps some of that code could be reusable. At the very least, the approach taken by dcrandroid would provide some insight for implementing this feature in godcr

Update:
dcrandroid now has sync progress display: planetdecred/dcrandroid#286

[x@rose dcrcli]$ ./dcrcli send
Destination Address:
invalid address

Destination Address:
invalid address

Correct msg would be 'You did not specify an address.' and then just exit.

Otherwise, "That is not a valid address. Try again."
Do not have unneeded blank lines in the output when re-asking the question.

Connection info, on the webui, persistent across all screens, could be in the top area where there is just the DCRCLI title currently

• Total Balance
• Synced with 8 Peers
• Latest Block
• Network type
• Database path

For the cli could just be a 'status' command that returns similar info...obviously balance not needed there since we already have a balance cmd.

Once #106 is completed, we'll have a clearer idea of what options and flags ideally belong to the config file. A godcr config command that provides an interactive way for a user to create a config file.

The command should prompt the user for values for all the options/flags that should be in config, providing default values for each option/flag. If there is no default value for a particular option/flag, and the user does not provide a value for that option/flag, the option should be included in the config file without a value, but commented out.

#64 has some context that can bring this to life

Both cli and web interface should allow users provide multiple destination addresses, specifying the amount to send to each address. On cli, the flow can be as follows:

...
Destination Address:
Amount (DCR):
Destination Address 2 (or blank to continue):
Amount (DCR):
[repeat until done]
...

webui gets a transaction details page

cli can get a showtransaction cmd

UPDATE:
Cli command implemented in #83

Unspent output selection
Do not show the list by default. Instead, ask the user if to list the outputs for manual selection or to allow the program auto pick the first best sized input(s) to use. The following prompt should do the trick:

Would you like to (a)utomatically or (m)anually select inputs? (A/m)

Unspent output display
When listing unspent outputs, show date, confirms and Received with Address instead of showing the utxo id.
This could look like:

[1] 4.1998656 DCR from TsfEKMhofscqRTbr1dunLQgzRb7K8hSiy8U    02/01/2019 5:01 PM    10 confirmation(s)

Currently, it looks like:

[1]: f54b60c7ac171f42d10871c37808b82e7c22f7ab30ac6647a080be8bbb748123:0 (4.1998656 DCR)

Separate commands into category, such that when displaying godcr -h, commands such as send-custom would not appear under the Available commands section but under a new section ### Advanced commands or Experimental commands

Also, the functionality described in #53 should be fully achieved

Add a ./dcrcli help command. It will be a more verbose version of the simple help that is returned when just running ./dcrcli by itself. Also have ./dcrcli help cmd return just the extended help for that one command.

The dcrcli settings parameters are all centered in the dcrcli.conf file. It would be nice to have the option to pass flags with the parameters via terminal.

Example:

$ ./dcrcli accounts --rpcuser xxx --rpcpass **** --cert /path/to/cert.pen

Typical scenario to illustrate the above issue:
Account has balance of ~75 DCR. The list of unspent outputs on the account as received from dcrwallet rpc is:

The last output though is marked as spent in dcr explorer: https://testnet.dcrdata.org/tx/bf466064da7ea21574ac98ca3e1d3d5671d16748a23863b41ac57efd6c833351

Implication is, this account cannot send out more than ~3 DCR. Trying to send anything above that amount or by manually selecting that last output results in the following error:
error publishing transaction: rpc error: code = Unknown desc = wallet.PublishTransaction(5cc928b418eea7eb7c1cd124eb2c7ff8186cf5b094b92015d818c6cef5fc902c):: dcrd.jsonrpc.sendrawtransaction: -1: Rejected transaction 5cc928b418eea7eb7c1cd124eb2c7ff8186cf5b094b92015d818c6cef5fc902c: orphan transaction 5cc928b418eea7eb7c1cd124eb2c7ff8186cf5b094b92015d818c6cef5fc902c references outputs of unknown or fully-spent transaction bf466064da7ea21574ac98ca3e1d3d5671d16748a23863b41ac57efd6c833351

Currently, there' a t/--testnet flag for instructing dcrcli to use testnet and not mainnet. This is primarily used by the godcr program for connecting to a wallet. dcrlibwallet uses that information to decide which wallet database to load, testnet wallet database is stored in godcr's appdata dir in a "testnet" subdirectory (for linux, that is ~/.godcr/testnet) while mainnet is stored in "mainnet" subdirectory (~/.godcr/mainnet for linux).

The flag should be removed, and godcr should autodetect wallets on a user's PC and the network type for each wallet, save this information in a wallets.conf file and use the information saved when establishing a connection to a wallet.

A list of potential directories to find wallet databases should be maintained on the program. Currently,

• dcrwallet stores wallet database files in dcrwallet's appdata dir, under subdirectories "testnet3" for testnet and "mainnet" for mainnet network types.
• dcrlibwallet stores wallet database files in godcr's appdata dir, under subdirectories "testnet" for testnet and "mainnet" for mainnet network types.
• decredition uses $HOME/.config/decrediton/wallets

Desired behaviour on cli mode

• Add godcr detect command to perform a scan for wallet databases that godcr can connect to. A wallets.conf file should be created in the godcr appdata dir to store information about discovered wallets and their network type. Regardless of the wallet connection mode set by the user, godcr discover should detect and save information for all "detectable" wallets.

• The command should list all wallets detected and prompt user to select a default wallet to connect to. A --wallet config option should be added to godcr.conf to store the "id" of the default wallet. If only one wallet is detected, that should be made default without prior confirmation from user.

• Every run of godcr should check if the wallets.conf file exists in the appdata dir. If no wallets are configured, it should ask if user wants to detect existing wallets. If godcr wallet config already exists, then no detect is needed (unless explicitly called with the detect command).

• If godcr wallets.conf exists but contains no wallet information, user should be asked if to create a new wallet. The wallet creation process should ask the user what network type he wants to create a wallet for. After wallet is created, information about the newly created wallet should be added to the wallets.conf file.

• If godcr wallets.conf exists and has data for just one wallet, godcr should automatically load that wallet. If multiple wallets were detected, the default wallet as specified by the --wallet option should be loaded. The user should be able to override the default wallet option saved in godcr.conf by passing --wallet="id" in command-line options.

• Similar functionalities should be added to the web and desktop interfaces (preferably in separate PRs). The testnet flag and wallet connection current behaviour can be retained for those interfaces until this new feature is implemented for all interfaces. After which, the testnet config flag should be discontinued for dcrlibwallet connections.

dcrcli send asks for amount in DCR but amount debited is not in DCR. This issue has been fixed for the web server package in PR #30, specifically in this comment, just to replicate in the cli package

Either of the following messages (or some other similar message) is displayed if in the process of executing a command, dcrcli tries to connect to dcrwallet via rpc and an error occurs during the connection:

Message 1:
rpc error: code = Unavailable desc = all SubConns are in TransientFailure, latest connection error: connection error: desc = "transport: Error while dialing dial tcp: missing address"

Message 2:
rpc error: code = Unavailable desc = all SubConns are in TransientFailure, latest connection error: connection error: desc = "transport: Error while dialing dial tcp [::1]:19111: connect: connection refused"

Possible reason for

• Message 1: value for dcrwallet rpc server address is not set. Either there is no dcrcli config file with this info or the config file does not contain the rpc server address value. The value is also not present in the command-line options. #50 and #64 already take care of cases where dcrcli config file does not exist. This issue should thus assume that the file exists and instruct the user to set the dcrwallet rpc server address in the config file or pass it in command-line options
• Message 2: dcrwallet rpc server address is set, but does not point to an actively running dcrwallet daemon. See this comment for more info.

UPDATE:
Work done in #80 will successfully prevent the first error case. #50/#64 are no longer particularly geared towards fixing error case 1.

Enhancements to simplify the current behaviour of dcrcli.

Display available commands if the program is run without a command argument.

Currently the user is shown the following message if the dcrcli program is run without any command argument.

$ ./dcrcli
usage: dcrcli [OPTIONS] <command> [<args...>]

available dcrcli commands:

For available options, see 'dcrcli -h'

Instead of asking the user to type dcrcli -h to get a list of commands, the available commands should be displayed immediately as follows.

$ ./dcrcli
usage: dcrcli [OPTIONS] <command> [<args...>]

available dcrcli commands:
     Command | Description 
             |   
     receive | Generate address to receive funds 
        send | Send DCR to address. Multi-step 
     balance | Check balance of an account 
 nextaccount | Generate next account for wallet 
          -l | List all supported commands 

Replace the --mode flag with --http

The default mode for the program is CLI. To run in HTTP mode, the --http flag can be used. This is simpler and more concise. Take the godoc program.

$ ./dcrcli --mode=http balance becomes $ ./dcrcli --http balance
$ ./dcrcli --mode=cli balance becomes $ ./dcrcli balance

Display available commands in alphabetical order

Running $ ./dcrcli -l (to list available commands) displays the commands in random order. This is because the commands are displayed by ranging over a map: Go maps are not ordered.
The commands should be listed in the same order each time. Below are outputs from three runs of the program.

$ ./dcrcli 
usage: dcrcli [OPTIONS] <command> [<args...>]

available dcrcli commands:
     Command | Description 
             |   
     receive | Generate address to receive funds 
        send | Send DCR to address. Multi-step 
     balance | Check balance of an account 
 nextaccount | Generate next account for wallet 
          -l | List all supported commands

$ ./dcrcli 
usage: dcrcli [OPTIONS] <command> [<args...>]

available dcrcli commands:
     Command | Description 
             |   
          -l | List all supported commands 
     receive | Generate address to receive funds 
        send | Send DCR to address. Multi-step 
     balance | Check balance of an account 
 nextaccount | Generate next account for wallet

$ ./dcrcli 
usage: dcrcli [OPTIONS] <command> [<args...>]

available dcrcli commands:
     Command | Description 
             |   
     balance | Check balance of an account 
 nextaccount | Generate next account for wallet 
          -l | List all supported commands 
     receive | Generate address to receive funds 
        send | Send DCR to address. Multi-step

On PR 83, the block hash returned by dcrwallet is the same as the transaction hash. This has been confirmed as a bug on the dcrwallet project, and an issue has been submitted. This bug shall be documented for now, and this issue shall await a fix from the dcrwallet project.

This (https://github.com/decred/slog) should be used for logging throughout the dcrcli app to achieve the objective described in this comment:

I notice we're simply logging errors to the console. I suggest we log them to a file as well, and let the users know about the file. That way, the history of errors does not get lost, and we can get easily introduce telemetry if we want to. The main benefit is being able to go look at detailed error output (including stack traces if necessary) in the log file, and knowing where to start looking for problems. If a user reports an error, we can request the log file for richer context.

Passwords entered on terminal are currently visible when running dcrcli on Windows. This was introduced as a temporary pattern in #79. It's something we should correct some time, probably when there aren't more important things to do.

Open client connection to dcrwallet gRPC server when program starts and close when program ends/exit.

gPRC invocations should use context.Background() instead of context.WithTimeout()

I got the following error when trying to run dcrcli with a no-longer-supported option in my config file:

Extremely long. Took several tries (switching between branches) to determine what the error was. The actual error was hidden in the long message.

I'm thinking we should instead show just the error message like so:

and then add "instructions"/"tip" on how to view a list of the supported options, or something along that line.

The current approach will only get longer as options are added to config

When running ./dcrcli with no commands, currently the simple dcrcli usage cmds are shown. But these commands are not yet available if the user does not have dcrcli.conf set with the correct rpc login information. So this should be detected, and a msg displayed to let the user know that they need to specify this information. The simple usage commands would not be displayed, and instead instructions would be displayed to set edit the config file or to specify the appropriate rpc command line options (-h).

Additionally, we could autoconfigure by looking for the default dcrwallet config and auto parsing the needed settings, asking the user if they would like to use these values.

When dcrwallet creates a wallet on first run, the wallet has 2 accounts

• default (account number 0)
• imported (account number 2147483647)

Attempting to generate receive address for the imported account (for example via cli: dcrcli receive 2147483647) returns the following error:

wallet.NewExternalAddress: item does not exist: account 2147483647

Show count of tickets. Sum of the below numbers.

getstakeinfo

"ownmempooltix": 2,
"immature": 6,
"live": 138,

Show a list of all tickets with their current status.
Valid status are Mempool, Immature, Live, Voted, Revoked.

The command line menu (godcr -h) should be super clean and not cluttered by things that really should primarily be used from the config file. This would require separating config file options from command-line options. Command-line options can also be set in and parsed from the config file but config file options should have no business in command line (for now).

Later on, it may always be possible to abstract all the config commands to be accessible from the command line, but focus for now is keeping the command line menu super clean and uncluttered.

After closing issue #6 (comment) and enabling balance without parameters, remove "accounts" and "overview" commands as will be superfluous.

The balance command by default should have simple output, which is, just returning the Total. For the more detailed view, an argument would be passed.

This style would be duplicated across other UI's as well, with the default balance showing only the Total, and a detailed view being available by user selection.

When the program is run, the application name used in help messages, errors, and other such usages is read from the name of the binary that started the process.

$ ./godcr -C dcrcli.conf -v --http
godcr version: 0.0.1
$ cp godcr fake-godcr
$ ./fake-godcr -C dcrcli.conf -v --http
fake-godcr version: 0.0.1

I suggest hard-coding the program name instead of using the name of the binary.

Make 'dcrcli balance' command work with no parameters. Should list all available accounts and use minconfs 0.

Current output is

$ ./dcrcli
Usage:
  dcrcli [OPTIONS] <command> [<args...>]

Available commands:

     balance | show your balance
        send | send a transaction
 send-custom | send a transaction, manually selecting inputs from unspent outputs
     receive | show your address to receive funds

This can be simplified to.

$ ./dcrcli
available cmds:  balance, send, receive
experimental:  sendcustom

$ ./dcrcli receive
Error running command command 'receive' requires at least 1 param. 0 found'

Would be more helpful if we displayed the usage parameters.

This is one of 2 efforts to eliminate the need for a network type flag/option in godcr. The other effort is described in #89. Fixing this issue and #89 should totally render the testnet config flag unnecessary and the flag should be removed.

There are 2 current usages of the testnet flag in dcrwallet rpc:

• For "constructing" a dcrwallet rpc server address if user does not provide one. See here.
• For defining the network parameters for the connected wallet. See here. The defined network parameter is used by different core wallet functions to differentiate addresses and keys for one network from those intended for use on another network.

For the first use case, if the user does not provide a rpc server address, the immediate solution is to display an error message to the user, asking the user to set the dcrwallet rpc server address in config file. See #152

For the second use case, the network type can be determined once godcr is successfully connected to dcrwallet via gRPC. See this. Once the network type is determined, the network params can be defined.

Allow user to define number of change outputs to use for a transaction. If user choses to use more than 1 change output, ask if user wants to use automatically generated random amounts for each change output, or to manually specify amount for each change output. If user choses to use 1 change output, all the change amount from transaction should be assigned to the single change output.

Addresses should be generated for each change output and displayed to the user (with the assigned change amount) for final confirmation before broadcasting transaction. Total amount assigned to all change addresses should equal total change amount from transaction.

An idea of how this should look on cli:

....
How many change outputs would you like to use? (default: 1): 2
Use random amounts for the change outputs? (Y/n): y
Enter your spending passphrase: 
You are about to spend the input
9.423 DCR from TsgUAq54uZ6GW1VQwgjtgG7KB68pJwy73dJ
and send
2.123 DCR to TsgQwgjtgG7KB68pJwyUAq54uZ6GW1V73dJ
1.123 DCR to TsgJwyUAq54uZ6GWQwgjtgG7KB68p1V83dJ
3.123 DCR to TsgJwyWQwgjtgG7KB68pUAq54uZ6G1V83dJ (change)
3.123 DCR to TsgJwyuZ6G1V8WQwgjtgG7KB68pUAq543dJ (change)
Do you want to broadcast it? (y/n): y
Sent. Txid b5f844895a4e5b4e496929be0356e8435a8c30976d6bd59a29539fddd37fc9d7

If godcr is run with usedcrwallet=true but a dcrwallet rpc connection cannot be established, it might be because dcrwallet is not running.

The following confirmation should be requested from the user, after which godcr should start dcrd and dcrwallet:

There is no dcrwallet process started, would you like godcr to run the process? [Y/n]

It's important to note that

• A dcrwallet rpc connection failure doesn't always mean a dcrwallet process is not running
• It could be because the rpc server address was not set by the user
• #152 takes care of that scenario. But before a fix for #152 is merged, the absence of rpc server address in config should not be taken to mean that dcrwallet is not started
• Instead, it's necessary to first confirm that dcrwallet is not running by checking for it in the list of processes running in the user's PC before asking the user if to start dcrwallet
• If dcrwallet is found to be running, the user should instead be asked to enter a valid server address. See the concluding part of planetdecred/godcr#152 (comment)

Currently balances are being shown in their full atomic amount. 99912727581 would become 999.12727581

receive cmd is currently returning a panic. does not function

[x@rose dcrcli]$ ./dcrcli
No command specified
Usage:
  dcrcli [OPTIONS] <command> <args...>

Specify -h to show available options
Specify -l to list available commands

When ran with no parameters, what is currently being output with -l should be shown here.

Make a working default value for rpccert setting.