rpc help list commands

[rsbondi]

No description provided.

[pinheadmz]

Tested and works nicely. Definitely much cleaner execution than #550 but the drawback is that the commands are just listed, not explained. For reference, the REST API help returns explanations:

$ bcoin-cli help
Unrecognized command.
Commands:
  $ info: Get server info.
  $ broadcast [tx-hex]: Broadcast transaction.
  $ mempool: Get mempool snapshot.
  $ tx [hash/address]: View transactions.
  $ coin [hash+index/address]: View coins.
  $ block [hash/height]: View block.
  $ reset [height/hash]: Reset chain to desired block.
  $ rpc [command] [args]: Execute RPC command. (`bcoin-cli rpc help` for more)

Also worth noting that Bitcoin Core generates the "help list" algorithmically, each method has its own definition property and they are all aggregated for help. Maybe we could do something like that. See https://github.com/bitcoin/bitcoin/search?q=RPCHelpMan

And finally, as long as this is open for review again we also need a related fix for bclient, see bcoin-org/bcurl#7

[braydonf]

@pinheadmz I agree it would be good to have a single line description with the command, as in #550 though it would be good to have them generated, as in this PR. Each command could have short description, category, and full help text that could be used for generated output. For example bcoin-cli rpc help would give the condensed list with descriptions, and then bcoin-cli rpc help <command> would give the more verbose details for each command.

[pinheadmz]

@rsbondi see Braydon's suggestion here... is that something you are comfortable adding to this PR? If not I could look into it and either submit a PR to your branch or just take over the branch from here?

I feel like it might involve adding properties to to this.calls, which might require a PR to bweb but I'm not sure

[rsbondi]

I agree that it appears that bweb looks like the place, it is now just a key/value for this.calls, where the value is the function, maybe refactor all the this.add calls to another member that would also store the help info like this.addCall('stop', this.stop, helpSummary, helpDetail); that wraps this.add and then add the help to a new this.helpInfo which would hold {summary: helpSummary, detail: helpDetail}. I think if you add properties to this.calls this could break backward compatibility if someone is using this lib and expecting a function and not an object containing the function and help info.

I could do this locally, but it really seems like it should be at the base class, not sure how you coordinate the 2 repos?

[pinheadmz]

Yeah sometimes we need to make PRs in multiple repos that are linked :-)

Another approach to this, which may be kilnda silly, but each RPC function could parse the help value beyond its truthiness,
e.g.:

  async addNode(args, help) {
    if(help === 2)
      return 'Add a new peer by IP address to the peers list and attempt to connect';

    if (help || args.length !== 2)
      throw new RPCError(errs.MISC_ERROR, 'addnode "node" "add|remove|onetry"');
    ...
  }

Then in help(), iterate through this.calls and print this.calls[i]([], 2)

[braydonf]

It may not need to be defined at the same time as the api call, but would map with the name of the command. For example:

this.addHelp(<commandname>, <category>, <summary>, <text>);

[rsbondi]

looking at core

RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples);

compare doc to code they are generating the separate sections: arguments, results and examples, and use the arguments to generate the signature as well. Not sure if you want to go that far, but I could write a script that could generate the javascript code for all the core functions from a core node, and then manually add any bcoin extras, and generate help from that, but not sure how in sync the bcoin/bitcoin commands are.

core help with no args

== Blockchain ==
getbestblockhash
getblock "blockhash" ( verbosity ) 
getblockchaininfo
getblockcount
...

which is similar to what I have done without the categories or args but a deviation from bcoin-cli, but maybe this is ok, just the command and arguments, and you can drill down in help if needed.

[braydonf]

Having a list of commands is an improvement over what is currently without info. How many details are included initially may be something to look at to see what becomes too much info.

Right now I reference: https://github.com/bcoin-org/bcoin/blob/master/lib/node/rpc.js#L149 to see what is available, and will dig into the implemented method for details. And then https://github.com/bcoin-org/bcoin/blob/master/lib/wallet/rpc.js#L120 for the wallet side.

[rsbondi]

I made a proof of concept branch as a potential path. this separates the help data to a separate file, helpdata.js which was auto generated from helpgen.js talking to a bitcoind node, call me lazy but less typing, so this would need manual intervention for bcoin specific help.

Using a separate data file for help would eliminate needing to set via an addHelp type function and remove the clutter such a function would introduce.

This will mimic the bitcoind node, except I did not separate out arguments and examples, just combined in detail so less granular.

Note: this is a proof of concept to open the discussion, not linter of file structure friendly and I only applied the detail to getnetworkinfo for illustration.

[braydonf]

Also, a lot of the result and arguments are documented at, for example getrawtransaction: https://bcoin.io/api-docs/index.html#getrawtransaction

[braydonf]

@rsbondi Thanks for the info.

I'm wondering if a simple description and list of arguments would be enough, and then external documentation for the rest of the response as in the API Docs.

[rsbondi]

I think the gist looks good, you could generate the signature from the arguments for when help is called with no arguments, list all as signature and use description when args are passed to help, or even just list all command signatures and short description and maybe no need to pass args to help, but may still be convenient to not have to look at the whole list if you know the command but not the args. Also maybe tack the url link at the end of the list, for more detail visit https://bcoin.io/api-docs/index.html.

My personal opinion is that the signature is usually more valuable than a description, so at the minimum list that so in other words, command [args] over command "description".

[rsbondi]

here is example output from clightning, I think this format is good, you get call, args and description in a simple readable format.

waitinvoice label
    Wait for an incoming payment matching the invoice with {label}, or if the invoice expires

decodepay bolt11 [description]
    Decode {bolt11}, using {description} if necessary

[braydonf]

With the arguments defined, the help and argument check can be implemented for all methods, with something such as:

if (help || args.length < cmd.required.length || args.length > cmd.args.length) {
  const required = cmd.required.join(' ');
  const optional = cmd.optional.join(' ');
  throw new RPCError(errs.MISC_ERROR, `${cmd.name} ${required} (${optional})`);
}

And then it wouldn't need to be implemented within each method that is sometimes prone to mistakes.

[braydonf]

How about we bring in this PR so that there is at the least some basic info on which commands to select, and the follow up with other PRs and improvements.

We can also have #550 be for fixes to the RPC interface, and separate out updates to the help command for additional follow-up.

Lot's of good ideas here.

[braydonf]

If you want to address the nit, and squash the commits it should be good to go, I think.

[braydonf]

This will also close #538

[pinheadmz]

Thanks again @rsbondi !!!!!