search

monero-project / monero

Monero: the secure, private, untraceable cryptocurrency
https://getmonero.org
Other
8.98k stars 3.11k forks source link

Inconsistencies between wallet-cli and wallet-rpc #1695

Open amiuhle opened 7 years ago

amiuhle commented 7 years ago

I just looked into monero-wallet-rpc and discovered several inconsitencies to monero-wallet-cli.

For example, cli has show_transfers which can be used to get unconfirmed incoming transactions. In rpc, there's get_transfers which seems to do a different thing. There's no show_transfers in cli, and there's no get_transfers in rpc...
Also, documentation about rpc is outdated

I think it would be desirable to have 1:1 consistency between naming of methods and params in cli and rpc.

I'd create a couple of PRs to fix this if you agree. Would the current rpc calls have to be deprecated first, or is monero still alpha / beta enough to just do this in the next version?

vtnerd commented 7 years ago

As mentioned in IRC, it should be possible to get incoming mempool transfers. get_transfers with the pool option set to true should do it. Please test against that.

vtnerd commented 7 years ago

And my suggestion would be for not changing the name of existing RPC calls. Its tough to tell how many people this would impact.

vtnerd commented 7 years ago

Changing the names of the cli options might be more agreeable, but I don't if someone is attempting to strange things with piping commands into stdin or something. Some feedback from more in the community would help here.

amiuhle commented 7 years ago

I think the main question is: Do we want consistency between CLI and RPC?

Personally, I do. During development, it'll help a lot to just do the same thing manually and in code. Also, there would basically only be one API to document.

If the consensus is YES, then sooner is way better than later. I also think that RPC should be adjusted because I'm guessing way more people are using CLI.

Best way to do it IMO is to migrate all methods in RPC to the CLI method signatures, aliasing the deprecated ones and logging a big, fat, red message in the logs. Leave both new and deprecated messages in RPC and then remove the deprecated stuff in the January 18 HF.

Shouldn't be a big problem if it's done now. There will be 6 months of deprecation warnings, and almost everyone is probably following GitHub issues or r/Monero anyways, so they will know.

Like I said, better now than tomorrow. This won't get any easier.

moneromooo-monero commented 7 years ago

With this particular case, it's because simplewallet gets the transfer from RPC., then displays them. It doesn't attempt to exactly mimic the workings of the RPC (though in in this case it's pretty close). Some other wallet commands use more than one RPC too. Some use only part of what another RPC returns. While there might be things that could be named better, I don't think this is one of them.