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

Wishlist: Static or zcash-cli only usage help. #2973

Open
nathan-at-least opened this issue Feb 21, 2018 · 4 comments
Open

Wishlist: Static or zcash-cli only usage help. #2973

nathan-at-least opened this issue Feb 21, 2018 · 4 comments
Labels
A-documentation Area: Documentation A-rpc-interface Area: RPC interface D-bitcoin-divergence Design issue: Divergence from Bitcoin (upstream code and/or architecture). E-good-first-issue Effort: Suitable for someone new to the codebase. usability use case user interface

Comments

@nathan-at-least
Copy link
Contributor

Motivation:

I find it quite annoying that I can't use zcash-cli help without a running daemon. I want to be able to read all cli help from a "static source" without connecting to a zcash network or starting to verify blocks.

Proposed Specification:

Modify zcash-cli and zcashd as follows:

  1. zcash-cli is modified so that every response from a daemon reports the release version of zcashd and if that version is different than the version fo zcash-cli in any request then zcash-cli emits an error to stderr saying something like "ERROR: This zcash-cli has version FOO but the daemon reports version BAR. Please use zcash-cli version BAR to connect to that daemon." Then zcash-cli exits with an error message. This is introducing a policy change from bitcoin{d,-cli} and it will break any deployment that's doing this weird cross-version client/server thing. If any such deployments even exist, we can start a conversation with those users about zcashd complexity vs deployment complexity.
  2. zcash-cli help [COMMAND] fulfills the request locally. (Note: This is a notable design change from bitcoin{d,-cli} because this is the first case of a "rich client" behavior instead of a "thin client".

Meanwhile, the RPC calls for help remain functional with no interface changes.

Implementation:

In terms of implementation, both the zcashd RPC response code and the zcash-cli implementation should use exactly the same code that serves text that comes from static sources. These static sources should be easy for humans to read directly, so for example we can link to the help text for any revision within github and share those links.

@nathan-at-least nathan-at-least added A-documentation Area: Documentation use case usability A-rpc-interface Area: RPC interface D-bitcoin-divergence Design issue: Divergence from Bitcoin (upstream code and/or architecture). E-good-first-issue Effort: Suitable for someone new to the codebase. user interface labels Feb 21, 2018
@str4d
Copy link
Contributor

str4d commented Mar 6, 2018

This would require significant refactoring, because in the design inherited from upstream, help text is implemented as an initial conditional inside every RPC method's implementation. We would need to extract every instance into its own function (which could be called from the original location), and then figure out a way to connect them all to the RPC client mechanism correctly (i.e. they would need to be associated with their corresponding RPC method names, meaning that relationship is defined in two separate places).

This would be a very noisy change (depending on how we implement it, and how git renders the resulting diff, it will either affect every single help text section or every single RPC method function declaration; more likely both), and would add significant cost to pulling in most upstream PRs that touch the RPC interface (because we would need to modify new upstream RPCs to follow our format, and any refactor or slight change upstream that affects the help text would create merge conflicts).

I'm not saying it's a straight-up bad idea, but we need to be aware of what we would be getting ourselves into.

@str4d
Copy link
Contributor

str4d commented Mar 6, 2018

Oh, and the help text is not always static. By convention, we modify the help text for experimental features to specify that it won't work, if the feature is disabled. This is only possible because the help text is live (the enabling happens on zcashd, not zcash-cli).

@daira
Copy link
Contributor

daira commented Mar 8, 2018

This would require significant refactoring, because in the design inherited from upstream, help text is implemented as an initial conditional inside every RPC method's implementation.

This just doesn't seem very hard to change. It's a boilerplate change to each RPC implementation that would be easy to review. We can avoid changing indentation so that the diff is small. For example:

-UniValue getblockcount(const UniValue& params, bool fHelp)
+UniValue getblockcount_help()
 {
-    if (fHelp || params.size() != 0)
-        throw runtime_error(
+    return (
             "getblockcount\n"
             "\nReturns the number of blocks in the best valid block chain.\n"
             "\nResult:\n"
@@ -181,7 +180,9 @@ UniValue getblockcount(const UniValue& params, bool fHelp)
             + HelpExampleCli("getblockcount", "")
             + HelpExampleRpc("getblockcount", "")
         );
+}
 
+UniValue getblockcount(const UniValue& params) {
     LOCK(cs_main);
     return chainActive.Height();
 }

The point about the help text not being static is well-taken, but there are several possible solutions:

  • we could just describe what config options enable the command in static text (I think this is actually better in some ways);
  • zcash-cli could read zcash.conf to see which options are enabled (would require some refactoring but not much).

@daira
Copy link
Contributor

daira commented Nov 20, 2018

See also #3661 (comment) , which could be complementary to this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Documentation A-rpc-interface Area: RPC interface D-bitcoin-divergence Design issue: Divergence from Bitcoin (upstream code and/or architecture). E-good-first-issue Effort: Suitable for someone new to the codebase. usability use case user interface
Projects
None yet
Development

No branches or pull requests

3 participants