meta
meta copied to clipboard
Support subaddresses in light wallet API
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.
- these endpoints return address metadata on outputs (
- 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.~
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:
-
I don't think it's worth having
address
in each tx, the client can easily compute it from themajor_index
andminor_index
-
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?
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?
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)
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 :)
Thoughts on something like (didn't include all potential query params )?, (also didn't scroll up and read all the way :smile: ):
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
@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
Thoughts after having not seen this in a while :) :
- I think the endpoints in the PR are great and make sense.
- 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
- Can we use a consistent naming convention with regard to abbreviations? Looking at addrs/address/subaddrs specifically.
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.
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.
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
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.
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
.
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.
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 :)
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.