[WIP] add functions to wallet API

[SNeedlewoods]

Do not merge, this is still in development and just PRd for easier discussion.

This is an attempt to make the wallet API "feature complete" and it's part of this proposal.

The approach was to search for every wallet2 function in simplewallet and wallet_rpc_server, if it is used in either of those, but is not implemented in the API I considered it missing and it was added in this PR. I believe wallet2_api.h now contains all missing functions, but some are not implemented yet. I'm waiting on feedback on those, if you want to have a look, things I need help with are marked as QUESTION, you can also comment on things marked TODO (those may become QUESTIONs if I'm unable(/unsure how) to solve them) or anything else.

Also gathered this collection of (mostly minor) issues found during API work.

[rbrunner7]

As we can see from the table above, there are quite some things that wallet2 provides that are missing from current Wallet API. The additions will therefore be quite substantial.

I think we should not just rush headlong into adding those many things. After all, if we are successful, this API will be very important, will live for a long time, and will be used by many people. If the new resulting interface is not well designed, it will haunt us for years to come.

Problem is that there are too many ways how you could design it, it's not "naturally clear" what types and methods there have to be. My feeling is that you could do almost everything in a hundred different ways.

To contribute something useful in these quite difficult circumstances I try to come up with some general rules to follow when defining the API that will hopefully guide us towards a good solution.

[rbrunner7]

Make it complete

This rule is pretty self-evident and clear, but I repeat it here nevertheless in the sake of completeness.

Goal is to provide a viable successor to the "API" that the public things in wallet2.h provide and eventually be able to completely remove that header file from the codebase. Each and every user of it, e.g. the CLI wallet and the wallet RPC server, will have to switch and use the Wallet API exclusivelly.

Because of this, we have to make sure that the Wallet API is feature complete. We must not lose any wallet functionality that is needed and in actual use.

[rbrunner7]

Make it small

We should try, while respecting the first rule of completeness of course, to make the API small. The smaller it is, the faster will new devs be able to read into it, the easier it will be to find the right methods to use, the less work to document it, etc.

Where do we have potential to reduce the number of methods? One way I see is this:

Do not include methods from wallet2.h that would be trivial to implement by clients using other methods in the interface themselves with only a few lines of code. I would speak of essential methods, that being methods that do provide something that is not available through any other method and that clients can't build themselves because of this. So, include only such essential methods.

I have mentioned an example of such method in this earlier issue about wallet related methods: There is a variant of wallet2::get_payments that filters for payment ID. This method is not "essential" in the sense that I propose here because clients can easily use the general get_payments method and search themselves.

Of course use common sense: It may be essential that already the code of the Wallet API filters or sorts something because otherwise too much data must get transferred, or doing things in the client would be much, much slower.

[rbrunner7]

Be careful about terminology

A good API uses a consistent terminology when naming things, and uses terms that are "expressive", hard to misunderstand and hard to confuse.

IMHO quite a number of things in wallet2 have sub-optimal names, maybe because not enough importance was given to terminology when originally naming things. Prime example for me is the mixup of the terms transaction, payment and transfer. I detailed some examples in this already mentioned issue.

This motivated me to propose to ban the term transfer outright exactly because it's so easy to misunderstand and easy to confuse. I detailed my proposal here.

Of course many things in the Wallet API already have the names they have, and we must let them stand and are not completely free how to name the new things we add. Still I see quite a degree of freedom left.

[rbrunner7]

Limit the influence of existing clients

We have 2 main clients of wallet2.h in the existing codebase: The CLI wallet and the wallet RPC server. According to the plan we will have to migrate them to the Wallet API.

Of course, striclty only seen from the point of view of that migration, the more similar the Wallet API is to the "old" wallet2.h, the less work it will take. The question arises: It this important, and is this something that should guide us when designing the new API?

I propose "No" as answer. I think having a good API is much more important than trying to optimize towards migration work on the CLI wallet and the wallet RPC server being small. That migration happens once and is then over for all times, whereas the Wallet API will go indefinitely into the future and (hopefully) shape tons of Monero related software that together trump any migration work.

In detail that means we don't try to give each method of wallet2.h a more or less one-to-one counterpart in the parts we add to the Wallet API. We mostly design the API independently, following the other rules that I propose here, and then migrate.

[rbrunner7]

Keep the style consistent

I write this down more for the sake of completeness, like the very first rule Make it complete:

There is a clash in naming style between the Wallet API and most of the rest of the Monero codebase: The first uses casing to form names like TransactionInfo and getUnlockedBalance while the rest uses "snake casing", like so: device_derivation_path_option, unconfirmed_transfer_details.

For the things we add we basically have two choices: We keep the naming style consistent with the things that are already there in the Wallet API, or we use the snake casing that is much more widely used in the codebase. Both choices are well possible, and both have advantages as well as disadvantages. (Renaming existing things as a third option and a way to achieve overal naming style consistency is probably not on the table.)

We dicussed this in a Seraphis wallet workgroup meeting, and it seemed that more people voted for keeping the style consistent. I myself was also in that camp: To continue with the "non-standard" naming style is IMHO the lesser evil than mixing styles within a single API.

[rbrunner7]

Now my opinion about one the more important design questions:

wallet2.h has a number of separate methods to get various types of payments / transactions, e.g. get_payments (incoming confirmed payments), get_unconfirmed_payments (incoming unconfirmed payments), get_payments_out (outgoing confirmed transactions), and get_unconfirmed_payments_out (outgoing unconfirmed transactions).

As I explained here, neither the Wallet API, nor the RPC interface, nor Woodser's wallet API have such separate calls. They basically work with a single call that get payments / transactions of all types, using a struct that is "encompassing" i.e. is able to store all details of all these types. I hope I got that right, the Wallet API call for this seems to be history.

According to the rules Make it small and Limit the influence of existing clients I propose to continue with this one call and not create direct equivalents of these methods in the Wallet API.

Probably the type TransactionInfo needs some extensions to be able to give back every last bit of info that all those wallet2 methods provide, but I see this as a quite natural extension of the interface.

[rbrunner7]

II think this "interface draft" will soon reach a state where we should consider to try to get feedback about the draft in the form it reached from as many wallet app writers and maintainers as we can reach. Not sure how to best do that, however.