Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend cardano Wallet CLI with selected commands #96

Closed
10 tasks done
KtorZ opened this issue Mar 20, 2019 · 4 comments
Closed
10 tasks done

Extend cardano Wallet CLI with selected commands #96

KtorZ opened this issue Mar 20, 2019 · 4 comments
Assignees

Comments

@KtorZ
Copy link
Member

KtorZ commented Mar 20, 2019

Context

We'll provide a CLI to interact with the wallet layer from the terminal. The CLI acts as a proxy to the wallet backend server (and therefore, requires the wallet server to be up-and-running) such that every endpoint from the API has an equivalent command in the CLI (which delegates the logic to the API via an HTTP call).

e.g. POST /api/wallets has a corresponding command cardano-wallet wallets create --param1 --param2 --param3

Decision

This first iteration only concerns the following endpoints for which, at least mock API handlers should exists.

Acceptance Criteria

  1. Commands for endpoints listed above must be available in the CLI
  2. There should be an extra command to start the underlying web server (e.g. server start)
  3. We may rename the cardano-wallet-server to simply cardano-wallet for the CLI has a more general purpose than just being a web-server.
  4. Each command should output data in a JSON format, matching the one from the API
  5. Sensitive information (like passphrase or mnemonics) must be prompted and not passed as options or arguments

Development Plan

  • Define an interface specification using the docopt format.
  • Use the docopt Haskell library to parse thedocopt-based specification.
  • Use servant-client to define a client interface based on the wallet Servant API.
  • For each type in the API: extract out a smart constructor from its FromJSON instance, and reuse this smart constructor when parsing a user-supplied value within the CLI code.
  • Reuse record types (such as WalletPostData) defined within the API layer within the CLI layer.
  • Share any user-facing error messages between the API and the CLI code.
  • Pretty-print the JSON output of calling any API functions.
  • Add functions that ask for passphrases: don't accept passphrases on the command line.
  • Implement command for transaction endpoint. Because of docopt limitations, we may only allow one ellipse parameter and parse transaction's recipients as <coin value>@<address>, for instance:
    $ cardano-wallet create transaction --target 42@AeZ4...TrW --target 14@Zzcy54...D3Re
    
  • Write down some manual test procedure

PR

QA

https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface

@akegalj
Copy link
Contributor

akegalj commented May 2, 2019

Add options for loading passphrases from files.

I don't believe this is necessary. My experience with CLI tools is that sometimes there is an option to read passphrase from stdin - and then user can redirect file to stdin or similar.

But adding option to store passphrase to the file might not be the wise thing - as it might lead to errors from the user:

  • user might forget that there is a file with his passphrase
  • user might forget to change file permissions of passphrase stored file

We can protect user from the later but there are other ways user can mess up so I would rather not add this option or if we really have to have this feature then reading from stdin should be enough

@piotr-iohk
Copy link
Contributor

I have added some integration tests for CLI #264 (however don't know if it's gonna work, in particular still not sure how to test cardano-wallet wallet create since it is a responsive cli)
Also planning actually to create some manual regression test suite and make it part of the Release Checklist. (especially for anything that cannot be covered by integration tests)

Anyway still it would be good perhaps to improve on the code coverage here -> https://coveralls.io/builds/23377405/source?filename=lib/cli/src/Cardano/CLI.hs

@KtorZ
Copy link
Member Author

KtorZ commented May 17, 2019

@piotr-iohk I just realized that we won't get any code-coverage with the CLISpec integration tests. Because stack only collects code-coverage for code that is executed through the test-suites (and here, it's executed through a separate executable).

There seems to be a way to instrument stack to generate coverage reports with extra inputs from other executables. I'll have a look and see whether I can get this to work with shc/coveralls as well!

@piotr-iohk
Copy link
Contributor

Added more integration tests. Still plan to add more, but we can close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants