Support subaddresses in light wallet API

[j-berman]

UPDATE Apr 11: collaborated with @paulshapiro to add endpoints to create or retrieve subaddresses (/provision_subaddrs, /upsert_subaddrs, /get_subaddrs). Also in line with @ndorf 's comment to support this here.

Overview

Hoping to reach consensus on how lightwallet servers should support subaddresses @vtnerd @ndorf @paulshapiro @moneroexamples. The changes proposed here are not so major, however a server that supports subaddresses as described would not be backwards compatible with a client that does not. I figure we can keep this proposal as a draft until there is a well-tested server-side and client-side implementation in place everyone is happy with.

Key highlights:

• all endpoints only accept a standard address as the address param in requests (not subaddresses)
• /get_address_info, /get_address_txs, and /get_unspent_outs return wallet-level data, rather than just the info for the standard address
  • these endpoints return address metadata on outputs (maj_i, min_i) that enables the client to generate key images without needing to re-determine which subaddress received outputs
  • note this is backwards incompatible because a client that does not support subaddresses would not be able to generate key images for outputs received to subaddresses. It would be simple to maintain backwards compatibility if desired by requiring an additional flag in the request to each of these endpoints.
• added 3 new endpoints:
  • /provision_subaddrs
    • every successful call is guaranteed to provision fresh subaddresses that have not already been provisioned by the server
  • /upsert_subaddrs
    • enables one-off idempotent provisioning of 1 or more subaddresses starting at particular major and/or minor subaddress indexes
  • /get_subaddrs
• the server admin can use whatever default lookahead they want (wallet2 defaults to 50 major * 200 minor e.g.)
• the client would manage subaddress creation on its end without touching the server (client adds "accounts" and "addresses within accounts" locally, gives them out to counter-parties when requesting payment, then the server will pick up on the received outputs via a default lookahead when scanning for received outs) (UPDATE Apr 11: client can provision subaddresses via the server)

Existing OpenMonero/monero-lws implementations

Currently OpenMonero supports subaddresses in place of the address param in all requests. I have a PR to monero-lws that implements this same thing. However, I don't think this will end up being a great way to work with subaddresses in a lightwallet server in production for 3 main reasons:

(1) This comment from @trasherdk:

Also, I don't see the use-case, where you would need a hosted wallet, for single dedicated sub-addresses, but maybe that's just me. Wasn't sub-addressed supposed to be single use anyway?

It does not seem users would care much for subaddress-level data, and thus it just does not seem sensible for the API and backend to build around subaddress-based "accounts" over the /login endpoint.

(2) it's not entirely clear how the client could smoothly retrieve a wallet's (or account's) entire balance. The default lookahead in wallet2 is 50*200 subaddresses, meaning I would guess the expected default for the wallet would be to make 10,000 requests to the server for its balance, which is unrealistic. Then if there are more subaddresses such that the lookahead should be bumped, the client would make more round trips to the server. This does not seem workable.

(3) it requires a quasi-hacky solution to prevent someone malicious from being able to tell that an honest user's subaddress is stored on a server, even if the malicious party doesn't know the honest user's view key. See the monero-lws PR for more discussion on this if you have a lot of time and are into that sorta thing.

The goal of this proposal is to support subaddresses in a clean way that avoids the above pitfalls. Thus, I specifically propose not to allow subaddresses in place of the address param, and instead propose that API implementers follow this proposal to have a good time.

Future extensions

Data by subaddress

If we find that clients actually do want to retrieve granular subaddress data over the endpoints, rather than have the server accept a subaddress in the address param of requests, requests can include a major_index (and minor_index if needed) as additional params to the standard address + view key. This way the server doesn't need to perform any hacky and potentially unsafe authentication on subaddresses.

JAMTIS

The idea that the endpoints should support a wallet-level abstraction rather than an address-level abstraction fits neatly with what is brewing with JAMTIS: the current research on the next gen address scheme for Monero. In JAMTIS, a lightwallet server need not even see user addresses at all, and could only return the wallet's plausible received outputs to the client. See this comment explaining the brewing design further.

Customizable lookahead

(UPDATE Apr 11: customizable lookahead not needed anymore. Client can request server create arbitrary counts of new subaddresses)

If there is demand from lots of individual light wallet users to pre-load tons of subaddresses beyond the default lookahead, the server can also support a user-specified lookahead over the /login endpoint for example. This seems like more of a nice-to-have than a necessity imo. I think we can get away without it until the transition to the next gen protocol.

[ndorf]

I have started work on adding this support in MyMonero as well, and it is pretty much in line with your proposal here already. A couple of things:

1. I don't think it's worth having address in each tx, the client can easily compute it from the major_index and minor_index

2. Unfortunately, I think the server is needed to provision subaddresses. Otherwise, a user with two clients (e.g. desktop and mobile) might end up inadvertently generating and using the same subaddress on both of them, which could be a disaster.

In order to support "generating" new subaddresses while offline, each client can pre-request a number of them from the server. The provisioning endpoint would have a parameter indicating how many new subaddresses should be returned at once.

Thoughts?

[ndorf]

BTW, I don't think there can be any question that the API must be wallet-level. We do not want an API request for each subaddress, users can reasonably expect to have hundreds or thousands of them.

Querying an individual subaddress could be supported in addition to that, if there is in fact a use case for that?

[j-berman]

I don't think it's worth having address in each tx, the client can easily compute it from the major_index and minor_index

Agree wasn't sure on this, will remove :)

Unfortunately, I think the server is needed to provision subaddresses. Otherwise, a user with two clients (e.g. desktop and mobile) might end up inadvertently generating and using the same subaddress on both of them, which could be a disaster.

Interesting consideration -- a cross-device wallet (any wallet, not just light) needs to point to a server to protect against this. I agree this is worth protecting here. On the solution:

In order to support "generating" new subaddresses while offline, each client can pre-request a number of them from the server. The provisioning endpoint would have a parameter indicating how many new subaddresses should be returned at once.

I like this! At one point I had something along these lines too. I imagine clients would want a getter as well to see their already provisioned subaddresses. Also, I would think if you want to provision additional minor subaddresses, you would do so by account (i.e. let's say I want to provision an additional 10 minor subaddresses, I wouldn't want to do so across all 50 majors, just for the individual account I'm working with). Thus, there would be a different number of minor subaddresses by major, and so both getter and setter endpoints would be built around that. Thoughts on that? Or think it's fine to just assume constant # of provisioned minors in all majors? I would think this is more relevant for MyMonero, considering provisioning constant # of minor subaddresses across all majors would increase the lookahead impact when scanning, which has a more significant memory cost when you have tons and tons of accounts.

Querying an individual subaddress could be supported in addition to that, if there is in fact a use case for that?

Agreed, this is what I was thinking with "Data by subaddress". Doesn't seem to be a necessary thing at this time unless people really want it. The best use case I can see is for users who have tons and tons of transactions across different accounts (major subaddresses), those users could have a better UX navigating the wallet by account. (Edit: but with this approach, you'd restructure wallet flow for all users to first retrieve provisioned subaddresses, then display accounts probably without balances, and allow clicking into each one)

[j-berman]

Added endpoints to create or retrieve subaddresses, and removed address from each tx as per @ndorf 's suggestion.

Collaborated with @paulshapiro to add 2 endpoints to create subaddresses (/provision_subaddrs, /upsert_subaddrs), and 1 to retrieve (/get_subaddrs).

• /provision_subaddrs every successful call the server is guaranteed to provision fresh subaddresses that have not already been provisioned by the server.
• /upsert_subaddrs enables one-off idempotent provisioning of 1 or more subaddresses starting at particular major and/or minor subaddress indexes.
• /get_subaddrs is self-explanatory.

I stuck with the convention to use descriptive endpoint prefixes (e.g. "get_") and kept them as POSTs. I think it's fairly self-explanatory as is, although not technically best practice. Perhaps a PUT /subaddrs instead of /upsert_subaddrs may have been simpler for example as suggested by @paulshapiro :)

[CryptoGrampy]

Thoughts on something like (didn't include all potential query params )?, (also didn't scroll up and read all the way 😄 ):

defaults to account 0 unless specified in query params
POST /subaddress?count=10&account=2 => always creates/returns 10 newly provisioned subaddress, 
POST /subaddress => creates/returns 1 newly provisioned subaddress defaults to account 0
GET  /subaddress/{address} => info on specific subaddress...
GET /subaddress => info on all subaddresses
GET /subaddress?index=4&account=2 => info on subaddresses by index/account

[j-berman]

@CryptoGrampy first thing, the reason I didn't follow RESTful convention (e.g. GET /subaddress instead of the proposed /get_subaddrs) is because the other endpoints like /get_address_info don't either FWIW. I tried to keep this proposal consistent with the convention already in the spec

Going through your suggestions:

POST /subaddress?count=10&account=2 => always creates/returns 10 newly provisioned subaddress, 
POST /subaddress => creates/returns 1 newly provisioned subaddress defaults to account 0

These 2 are covered by /provision_subaddrs (check the latest commit to see how to structure a query to get what you want)

The additional proposed /upsert_subaddrs endpoint covers the case when you want to create a one-off at specific major and minor indexes

---

GET  /subaddress/{address} => info on specific subaddress...
GET /subaddress => info on all subaddresses
GET /subaddress?index=4&account=2 => info on subaddresses by index/account

So there is /get_subaddrs which allows you to see all subaddresses provisioned for the wallet

And then I proposed updating the existing /get_address_info and /get_address_txs endpoints to return wallet-level data (which includes all data for all subaddresses contained within the wallet)

I didn't include anything that enables you to retrieve data by subaddress, but would imagine those 3 getters could be updated to support maj_i and min_i as params if the use case is desired. Can you expand on why you'd want to data by subaddress? Seems like a nice-to-have at least to start

[CryptoGrampy]

Thoughts after having not seen this in a while :) :

1. I think the endpoints in the PR are great and make sense.
2. Would it make sense to add an API version as part of this PR to make it easier for servers/wallets and for future updates? e.g. /v2/get_address_info
3. Can we use a consistent naming convention with regard to abbreviations? Looking at addrs/address/subaddrs specifically.

[paulshapiro]

Hey all, I figured I would mention for the record that I was pinged by jberman about a year ago and we collaborated for a few weeks on the subaddr API design. We ended up making some conclusions. I went and fully implemented everything, and even refined things slightly. My plan was to PR the enhancement to the spec asap but I've been dealing with some blockers so I hadn't gotten around to it yet. I'd be happy to post my work on that shortly.

One of the things I've discovered over my past year of work is that we will certainly need to start thinking in versions of LWS - not specifically for the stuff described in this thread but for supporting other things properly. There is a flaw I discovered in the existing API that probably doesn't affect any existing use cases but as we start to bring LWS more in line with mainline, we will start to see those things. I will publish on them shortly. The main thing to know is, we don't need URL based versioning for these things, as I have enhanced the LWS core client code to be able to handle a newer versus older LWS. I believe this is a good plan going forward - no API versioning, where possible, so that new clients can still talk to old servers within the limits possible - and currently, it is just fine. I do not see a reason currently to version /get_address_info specifically - servers and clients are able to simply add and detect fields respectively for the changes we're discussing here.

In terms of normalizing endpoint and field names, I will PR some of those suggestions I have. I will get more specific in the PRs but basically, yes, no reason to say "addresses" (addrs), in fact, no reason to say "subaddress" (subaddrs),... no reason to say "major" (maj), no reason to say "idx" (i), etc. This all saves on wire payload size provided no compression is occurring. But we still want clients to be as widely compatible as possible so I do not see a reason to go back and change /get_address_txs to /get_addr_txs for now. Maybe one day if we actually need a new endpoint. But those endpoints likely won't ever get made since /get_address_txs is pretty complete aside from one or two enhancements I will publish on soon. Realistically, we can actually fully deprecate /get_address_info - LWS clients do not really need it afaict.

There's lots of stuff I have notes on that I'm waiting to make a tiny bit more progress on before I publish. I would appreciate support in collaborating on integrating these things as I've done a ton of work on the LWS over the past year or two after my years building the core client code and incidentally, a realtime version of the API, etc., I think I have some solid suggestions to share.

[vtnerd]

Why is this uint32-string? Why not just uint32? Was this supposed to be a uint64-string (seems unlikely)?

[paulshapiro]

uint32 is fine here. There's no reason to make it a string since it cannot be a uint64.

[vtnerd]

What happens if it already exists? The client can't tell if it's a new insert or an existing one. I suppose this is the difference between the other API call? Or should a HTTP error be returned?

[paulshapiro]

Upsert as in update unless exists in which case insert. Server should return its known set of subaddrs so client can know, for various reasons, which I will describe soon in some updates.

Over the past 1-2 years, I have done a massive amount of work on the LWS and discovered that the subaddr support in the LWS API as currently specced and as it even currently behaves is badly insufficient, so I will share a list of things I've discovered with some spec proposals shortly. I'm finishing it up now after collecting everything.

[paulshapiro]

insert unless exists*

[vtnerd]

@j-berman is this a single subaddrs or an array? I'm assuming an array because the other instances (new_subaddrs and all_subaddrs) have to be arrays by design.

[paulshapiro]

@vtnerd it needs to be what i call a complex range of idxs. that is, a dictionary with maj idx str keys pointing to arrays of arrays of min idxs.

[vtnerd]

That sounds nearly identical to this design. A major index, pointing to an array of index ranges.

[paulshapiro]

well, it was your question, not anything major I'm claiming to point out yet, but btw I did intend to type array of arrays. you need support for discontiguous min idx ranges.

[vtnerd]

This API does support discontiguous min ranges. [10, [0,2],[4,7]] means 0,1,2,4,5,6,7 minor indexes with major index 10.

[paulshapiro]

but that is a discontiguous min range.

[CryptoGrampy]

Had a thought today:

Today, (I think) LWS can be used for something like a public donation monitor or fundraiser (or more). You could make your address and view key public (or even a list of public addresses/view keys for multiple fundraisers) , and it can be thrown on a static site that can ping a LWS API (front end call direct from browser)) and get live updates for anyone wanting to see the fundraising progress.

There's not really any way that a public person visiting this website can do something nefarious even though they have your public address and view key. They can't do anything with these credentials to trigger the LWS server to change its data- even restore height can't be changed from the regular public facing API today.

If there is an endpoint to provision subaddresses, I think this is a change to that paradigm in that anyone with access to the public and view key credentials will be able to make a nefarious change to the LWS server database. Not sure if this is a good or bad thing, but I think it's probably bad.

[paulshapiro]

hi grampy,
well noted!
I have an alt auth scheme that solves this which I will publish in a sec. wrapping up a couple loose ends 😓 nearly done
Paul

[vtnerd]

If there is an endpoint to provision subaddresses, I think this is a change to that paradigm in that anyone with access to the public and view key credentials will be able to make a nefarious change to the LWS server database. Not sure if this is a good or bad thing, but I think it's probably bad.

The monero-LWS server will have a runtime configurable limit for subaddresses per account. A value of 0 means subaddress support is disabled.

[vtnerd]

@j-berman bumping this review once again, as there are two changes I think necessary:

• The uint32-string should be uint32 in subaddrs dictionary.
• The transaction object should have a receipient field added to it. You can infer this via the spent_outputs which will all mirror the value, but if there are no spent outputs the subaddress cannot be determined.

[j-berman]

The transaction object should have a receipient field added to it. You can infer this via the spent_outputs which will all mirror the value, but if there are no spent outputs the subaddress cannot be determined.

Thinking on it, it sounds like you want the light wallet client to be able to separate funds by subaddress in the UI via get_address_txs alone? In that case I think the transaction object should then also have an array of received output objects (and no need to return the fields that are already on the transaction object). An array seems necessary in case a user receives to multiple subaddresses in a single transaction (and the output object also captures the amount).

Worth noting get_address_info doesn't support separating funds by subaddress alone.

Also worth noting get_unspent_outs does have the recipient field on each output, so technically clients could separate funds by subaddress with an additional call to get_unspent_outs.

[vtnerd]

Thinking on it, it sounds like you want the light wallet client to be able to separate funds by subaddress in the UI via get_address_txs alone? In that case I think the transaction object should then also have an array of received output objects (and no need to return the fields that are already on the transaction object). An array seems necessary in case a user receives to multiple subaddresses in a single transaction (and the output object also captures the amount).

Without additional changes to this PR, if the UI wants to display transactions by subaddresses (like the CLI wallet), it will have to use both get_unspent_outs and get_address_txs. As an example, the payment_id is not available over get_unspent_outs, but is available via get_address_txs.

Perhaps an optional filter by major index added to the get_address_txs request, then an array of objects in a transaction response with the specific major+minor indexes? That should cover the standard UI use-case.

Worth noting get_address_info doesn't support separating funds by subaddress alone.

Yes, with subaddresses this becomes a total balance over all accounts calculation only. I'm wondering whether this should be updated too, because it makes the endpoint less useful to the typical UI setup.

[j-berman]

Yep makes sense.

Perhaps an optional filter by major index added to the get_address_txs request, then an array of objects in a transaction response with the specific major+minor indexes?

SGTM with one nit.. the optional filter seems extra, no? Seems it's fine to just default return the array of objects in each transaction by default

I'm wondering whether this should be updated too, because it makes the endpoint less useful to the typical UI setup.

Open to suggestions :)

[vtnerd]

I think this is good now. get_address_txs-> [transaction] -> [spend] provides the information needed to display a specific subaddress or all of them at once. I somehow overlooked that transaction had spend objects.