fedimint / fedimint

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

Unify `gateway-cli` and `fedimint-cli` commands #1561

Closed dpc closed 2 months ago

dpc commented 1 year ago

These two should behave generally very similarly.

dpc commented 1 year ago

@okjodom thoughts?

okjodom commented 1 year ago

While I'd like to have one cli to operate across the board, I'm not sure we can cleanly merge the experience of these two clis Maybe:

I think of gateway-cli commands as a superset of fedimint-cli commands

dpc commented 1 year ago

Position of argument is not the main concern

Positional arguments are PITA to work with in practice. I will postulate to ban them. :D

gateway cli operates over multiple federations,

I'm using gateway-cli over multiple federations just fine as well. I don't really see a fundamental reason to have that difference. I'd say we should change fedimint-cli to work like gateway-cli as well and track federations it was connected to.

connect-info vs info - not equal

OK, these are not the same, but gateway could have ability to show connect-info as well. And if fedimint-cli tracks federations, it could have info as well.

okjodom commented 1 year ago

Position of argument is not the main concern

Positional arguments are PITA to work with in practice. I will postulate to ban them. :D

yes! ha.

gateway cli operates over multiple federations,

I'm using gateway-cli over multiple federations just fine as well. I don't really see a fundamental reason to have that difference. I'd say we should change fedimint-cli to work like gateway-cli as well and track federations it was connected to.

okay gateway-cli might be adding one more dimension. it should be able to operate over any gatewayd, and assume multiple federations over each gatewayd controlled. See, gatewayd api has the notion of multiple federations. I'm not sure fedimintd api has the same behavior

dpc commented 1 year ago

See, gatewayd api has the notion of multiple federations. I'm not sure fedimintd api has the same behavior

gateway-cli --gateway-rpc http://127.0.0.1:8176 info

should display roughly the same thing as:

fedimint-cli --data-dir ./somedir info

So, yes - the gateway-cli has another dimention, as it can connect to multiple gateways out there, each connected to its own set of federations.

But it will largely disappear:

export FM_GATEWAY_RPC=http://127.0.0.1:8176
export FM_GATEWAY_PASSWORD=foobar
gateway-cli info

export FM_DATA_DIR=./somedir
client-cli info

Same invocation, corresponding output.

okjodom commented 1 year ago

Nice. Ack. and looks feasible.

How would we tie in this suggestion by @elsirion . I initially took 'Unify' in the issue title to mean merging the two clis, but our discussion so far has been about matching and aligning the experience of commands. The referenced suggestion takes a step further and proposes merging the two clis into one experience

dpc commented 1 year ago

This is orthogonal. The two could be one binary, or could be separate binary and share code (one can nest existing #[derive(Parser)]struct Options to extend it AFAIR, or could be a multi-call binary.

In this issue, I just want them to largely feel the same (uniformly).

okjodom commented 1 year ago

Makes sense. As a point of note, current command namings in gateway-cli correspond most to gatewayd API's that the cli call into.

cdmoss commented 1 year ago

Hey, I'm new here :) I wouldn't mind trying to tackle this, but I may need to ask a silly few questions along the way. Just so I'm clear, the goal here is to make gateway-cli and fedimint-cli have a uniform set of commands?

dpc commented 1 year ago

the goal here is to make gateway-cli and fedimint-cli have a uniform set of commands?

Yes. So they behave and feel much more similar.

PaarthAgarwal commented 1 year ago

I'd like to work on this issue. How to get started with it?

justinmoon commented 1 year ago

I'd like to work on this issue. How to get started with it?

For starters, rename the gateway-cli connect-fed command to gateway-cli join-federation

okjodom commented 1 year ago

cc @m1sterc001guy

jp1ac4 commented 11 months ago

For starters, rename the gateway-cli connect-fed command to gateway-cli join-federation

Hi, I could make this change if it's still required?

If any other participants in this issue have already made a start, please let me know.

justinmoon commented 11 months ago

I think it's best to hold off on this one for now. fedimint-cli and the gateway are both heavily under construction right now so probably best to wait until the dust clears a bit more.

justinmoon commented 5 months ago

Threw this on 0.2.1 ... it would be nice to either merge the two CLIs, or decide we won't do it and close this issue. I'd be in favor of merging them. Removing one component from the system just makes our lives and the lives of federation operators a little simpler.

justinmoon commented 5 months ago

dev call: at very least the 2 binaries should have similar command names etc

richarddushime commented 2 months ago

Greetings to everyone

I'm new here and got interest in the fedimint project and currently looking into this issue as my starting point, and want to ensure I'm well-prepared before proceeding. Is there any new information or updates I should be aware of? Additionally, are there any specific instructions or details you think would be helpful for me to know before I proceed with my attempt?

elsirion commented 2 months ago

I think we have 2 options:

Both seem a lot of work for not all that much gain. I think we should come to consensus if we want to do this first. @dpc @bradleystachurski @m1sterc001guy

bradleystachurski commented 2 months ago

In general, I think it's okay to break CLI compatibility if there's a material benefit, e.g. updating restore in 0.3.0. (https://github.com/fedimint/fedimint/pull/3918).

Both seem a lot of work for not all that much gain

I agree, however I don't have as much experience working with gateway-cli as others, so I'm open to the idea of unifying if this is a significant pain for users. From what I gather from this thread, it's a minor inconvenience.

m1sterc001guy commented 2 months ago

I don't think maintaining CLI backwards compatibility is the biggest deal, the problem is that they're our best tests for maintaining consensus/api/db backwards compatibility, which is very important. I agree with Brad and Eric, I don't see too much benefit in this given the amount of refactoring it would take to do correctly.

justinmoon commented 2 months ago

Dev call: "juice is not worth the squeeze". Let's just close this now. Maybe in the future we re-visit.