taproot-assets
taproot-assets copied to clipboard
CLI: Migrate from `urfave/cli` to `spf13/cobra` to resolve misc. known issues
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.
Deliverables
- [ ] CLI unit tests
- [ ] Acceptance criteria
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.
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