fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
536 stars 210 forks source link

Expose helpful info about available modules from CLI #3119

Closed shaurya947 closed 1 month ago

shaurya947 commented 8 months ago

Once #3118 gets merged we'll have the ability to call arbitrary commands that are specific to custom, non-default modules. As such it would be helpful to have a command that exposes all the available modules (both instance IDs + kinds) so it's easy to figure out which modules can be interacted with in a session.

Furthermore, it might also be worth investigating ways to make the module subcommand smarter such that it can use all the available modules to return more helpful error messages or even have something akin to an autocomplete etc.

elsirion commented 8 months ago

I think if modules used clap internally for parsing they could possibly return an error message showing the typical help/command list if no arguments are supplied and the CLI could be slightly more intelligent about displaying these (not as normal JSON output but as text on stderr as clap does afaik). I'd like to avoid being too clap-specific in our client module interface.

15IITian commented 3 months ago

I understood some part that we require a new command that list all the modules available ( default and non-default ) . If yes, please guide how to start with it.

elsirion commented 3 months ago

I think a fedimint module list command would be good, then I'd use fedimint module <module> <command> <args…> for custom module commands. What do you think @dpc, I know you wanted shorter commands, but I think if we keep the shortcuts while allowing arbitrary commands to be called this way that would be ok.

There should also be two optional, mutually exclusive flags --federation and --client that display the theoretically supported list of modules of the client and federation respectively, while the normal command only shown the modules supported by both.

dpc commented 3 months ago

I think a fedimint module list command would be good, then I'd use fedimint module <module> <command> <args…> for custom module commands. What do you think @dpc, I know you wanted shorter commands, but I think if we keep the shortcuts while allowing arbitrary commands to be called this way that would be ok.

Generally sounds good to me, short is good, but not most important.

There should also be two optional, mutually exclusive flags --federation and --client that display the theoretically supported list of modules of the client and federation respectively, while the normal command only shown the modules supported by both.

I'm no sure if it's necessary. Client knows about modules it doesn't support, so could still list them with some info.

> fedimint-cli module list
{
   "0": {
     "kind": "bazinga",
     "status": "unsupported",
    },
   "1": {
      "kind": "mint"
      "status": "recovery",
      "progress": {
         "complete": 41,
         "total": 666
       }
       /// possibly some other most important fields
    },
    "2": {
       "kind": "wallet",
       "status": "active",
       // possibly more
    }
}
elsirion commented 3 months ago

I'm no sure if it's necessary. Client knows about modules it doesn't support, so could still list them with some info.

Good point! I was thinking about making the CLI output more human readable by default i.e. just a newline deliminated list of strings) in the future, but we can make this a future problem. We use JSON everywhere anyway for now.

15IITian commented 3 months ago

After deep dive into the code , I have deduced some conclusions , Please do tell what I am missing out to solve this issue ->

elsirion commented 3 months ago

Since we are talking about the client here the server module registry isn't relevant. There is the client config containing all modules supported by the federation and the client module registry that contains the modules supported by the client. When you look at the client module interface you should also find a way to query backup recovery progress.

15IITian commented 3 months ago

I'm no sure if it's necessary. Client knows about modules it doesn't support, so could still list them with some info.

so means , we have to show all the modules supported by client with this command fedimint-cli module list , right?

elsirion commented 3 months ago

@dpc's suggestion was to show the union of federation-supported and client-supported modules and just annotate this fact such that a user could filter the output for the modules that are supported by both client and federation (e.g. using jq).

15IITian commented 3 months ago

Thanks for clarification.

15IITian commented 3 months ago

in json -> what fields other than module kind and moduleInstance ID do I have to include?

elsirion commented 3 months ago

I guess at least a status field would be nice for now, it could be

enum ModuleStatus {
    Active,
    NotSupportedByFederation,
    NotSupportedByClient,
}

for now and later extended to

enum ModuleStatus {
    Active,
    Recovering,
    NotSupportedByFederation,
    NotSupportedByClient,
}

to indicate if it can actually be used. But the whole recovery is a bit in flux right now and that might be a bit too complex for you rn, so let's just do the first three states.

dpc commented 3 months ago

NotSupportedByFederation

How can a module be not supported by Federation? If Federation returned it (is part of client config) they have to support it, no?

15IITian commented 3 months ago

Yeah , I think it must be Supported_by_federation and similar for client one

dpc commented 3 months ago

If it's active or recovering it must have been supported by the client. So AFAICT there are 3 possible state currently: unsupported (by the client), recovering, active.

15IITian commented 3 months ago

If modules from client module registry -> client modules -> active = true clientconfig -> federation modules -> not getting how to find whether , it is client_supported or not ??

15IITian commented 3 months ago

enum ModuleStatus - must be struct right?

dpc commented 3 months ago

No idea what you just asked about. :D

elsirion commented 3 months ago

@dpc Why not list modules supported by the client in principle but not the federation?

dpc commented 3 months ago

@dpc Why not list modules supported by the client in principle but not the federation?

Oh. In my mind there's a difference between module kinds and module instances, and mixing them didn't even cross my mind. "It's either a ps or ls :D". Built-in module kinds would fit better into version output or some other call about "listing supported features, versions (consensus, api), etc.". It kind of doesn't have anything to do with the currently joined Federation.

elsirion commented 3 months ago

Good point, I have the opposite problem, I always forget that instances exist and not only kinds :see_no_evil: