taproot-assets icon indicating copy to clipboard operation
taproot-assets copied to clipboard

CLI: Migrate from `urfave/cli` to `spf13/cobra` to resolve misc. known issues

Open jharveyb opened this issue 2 years ago • 3 comments

The current library used to build the CLI has a few known issues:

Bad overflow handling (returns a 0 instead of an error) - https://github.com/lightningnetwork/lnd/pull/7350#discussion_r1087542359

Hard to prevent bool flags being misused, which causes assets to be minted with incorrect parameters, e.x. - https://github.com/lightninglabs/taro/issues/228

v2 of urfave/cli has the same issues as v1, which we currently use. https://cobra.dev/ is at least as well-supported / maintained, properly propogates numerical parsing errors, and has more features for validating input strings.

jharveyb avatar Feb 02 '23 19:02 jharveyb

Deliverables

  • [ ] CLI unit tests
  • [ ] Acceptance criteria

dstadulis avatar Nov 14 '23 19:11 dstadulis

Initial work here:

https://github.com/lightninglabs/taproot-assets/tree/cobra_cli

Most of the flags can be partially ported to the cobra format with refactoring tools. I tried to follow the structure used in chantools, where we wrap commands with per-parent-command variables that store parsed flags and other local state.

There is a small 'hack' with the usage template to match the existing behavior by hiding global options from subcommand help output.

The primary useful feature IMO is the cobra.Command.Args setting, which we can use to reject any arguments, or any arguments not paired with a flag. We can also mark flags as required easily instead of writing those checks manually.

We can also separate flag validation into cobra.Command.PreRunE from the actual RPC call in cobra.Command.RunE, which may make new commands easier to read/review.

The diff leaves existing urfave code in place since its WIP.

jharveyb avatar Nov 27 '23 23:11 jharveyb

Would also be good to remove this chain param from the profiles, which is a vestigial param from LND support for Litecoin:

https://github.com/lightninglabs/taproot-assets/blob/main/cmd/tapcli/profile.go#L30

jharveyb avatar Nov 28 '23 20:11 jharveyb