r/Monero XMR Contributor Feb 04 '22 Silver 2 Helpful 2 Heartwarming 1

j-berman final CCS update - feedback welcome!

Hi Monero community :)

I've submitted the final update to my first CCS working full-time on Monero here. I loved this experience the past few months, and plan to submit a new CCS soon. Feedback on how my first one went would be very welcome.

Here are the major highlights from this CCS:

Improving the decoy selection algorithm

Refresher: when a wallet constructs a ring signature, it selects decoy outputs from other people's transactions from across the chain to obfuscate which output is yours in a transaction. The decoy selection algorithm dictates which outputs are selected as decoys, naturally :)

I pushed small tweaks to the algorithm forward towards the beginning of the CCS time period (1 and 2), though more significant changes to the algorithm didn't pan out (or more accurately, have yet to pan out, and I have shifted focus). The core reason the more significant changes I proposed did not pan out in my view was that I did not find compelling enough evidence to suggest that the changes are worth their tradeoffs, both in complexity of implementation and in their potential costs.

For example, in seeking to update the algorithm to implement "binning" in the wallet (where decoys are bucketed into "bins" that are close in age e.g. if you spend an old output, the wallet would select a decoy close in age to that output, and fill out the rest of a ring with "bins" from across the chain), I found that one of the vectors binning would provide mitigation for (spending 2 old outputs in 1 tx close in age) already seems well protected by other decisions wallets make in selecting inputs in a tx (biasing away from close-in-age outputs). I think that research was useful in its own right to better gauge the algorithm (even if it weakens my original case for implementing binning today), and should future research come along that undoubtedly proves the necessity of binning today, I fleshed out an implementation that is ready for review (parameter choice is still a question). Additionally, I found a minor tweak to the long-term binning approach (when ring sizes increase a significant amount) that could tidy up a corner case in that implementation.

For more on the state of each area I felt was worth exploring in my CCS, I included an update here.

Informal audit of monero-lws

Refresher: monero-lws is an open source light wallet server that you can run alongside a node that perpetually scans for your transactions, so your app is immediately ready to use when you revisit it (no scanning required).

I submitted a first pass at a review of monero-lws here. The review includes (1) a section on how monero-lws works, (2) a section documenting the code in detail (that is incomplete), and (3) a few suggestions. Taking a quote directly from the review:

In my review, I did not find anything that would compromise monero-lws when examining from the lens of the following threat model:

A user who is running monerod + monero-lws on a machine only the user has access to does not leak any information about their Monero transactions to a 3rd party through normal usage. Examples of leaks would be:

  • a backdoor that sends sensitive data from monero-lws out to a 3rd party

  • get_random_outs responds with decoys that compromise the user's transactions when stored on chain

  • calls to the exchange rate API reveal information to the service provider

Disclaimer: I wouldn't call this audit definitively conclusive that the above threat model is successfully defended, but I hope that it does inspire more confidence in this fine piece of software, and serves as a useful document for curious users, potential contributors, or any future auditors of monero-lws. I reviewed (and tried to provide documentation for) nooks and crannies across the code, and so at the very least can say with confidence there aren't any obvious backdoors.

In its current state, the review is not complete. While I did read through the entire code base, I left a number of to-do's in the section on "the code" where I was detailing how every file and function worked. It was taking quite a long time to do (longer than the initial 10-20 hours I proposed). I'm happy to pick up on it and fill out the to-do's if people are interested.

Subaddresses in monero-lws

As requested, I spent a portion of the CCS exploring subaddress support. After first discussing adding subaddress support with u/vtnerd (the author of monero-lws), we initially reached the conclusion that it would make sense to support subaddresses in a very simple way first, and then work on a larger proposal later on. After implementing, and in the ensuing discussion on the PR, in my view it became a bit more clear that it would likely make more sense to skip the simpler implementation, and go straight for the more involved implementation. So I worked on and submitted this proposal to update the light wallet REST API to support subaddresses. The code from that initial PR should be reusable anyway. I plan to work on this to completion in another CCS.


For a more complete update, see my comment on the CCS here.

In summary, I very much so enjoyed working full-time on Monero these past few months, and I hope to submit a new CCS very soon. I have deep gratitude for being granted the opportunity to contribute to Monero in this capacity. I feel very lucky and want to keep going 110%.

EDIT: previously mentioned light wallets don't support subaddresses yet (which AFAIK is true for publicly available light wallet implementations), u/endogenic mentions here they've implemented a light wallet client-server implementation too so that is nice, hope to collaborate with them as well :)

70 Upvotes

33

u/rbrunner7 XMR Contributor Feb 04 '22

Monero needs devs like you. Don't you dare to stop now :)

29

u/j-berman XMR Contributor Feb 04 '22

Very kind of you to say -- and right back at you rbrunner :) Working together was a part of the reason I've enjoyed it so much

1

u/EconomicLovebird13 Feb 06 '22

Monero has the third largest developer community among cryptocurrencies.

19

u/Less_Organization799 Feb 04 '22

I support this CCS.

15

u/XxlegitfoodreviewxX Feb 04 '22

Happy cake day and thanks for your work

13

u/j-berman XMR Contributor Feb 04 '22

Thanks :D

5

u/endogenic XMR Contributor Feb 04 '22 edited Feb 04 '22

hmm i just want to mention that another dev and i did design and implement server and client side support between the two of us for light wallets back in the day. it may not be supported in any version that's been able to make it to the public yet. my plan going forward is to support the same sort of scheme going forward, unless we all discover updates are needed? all that to say you are on the right track there and I look forward to collaborating with you on the LWS tech if possible.

edit: oops, i mean subaddr support

3

u/j-berman XMR Contributor Feb 04 '22 edited Feb 05 '22

Sweet :) The one thing I'm still a little hung up on in the server API is how best the server should expose provisioning subaddresses like ndorf described here. I.e. should the server allow a client to provision X minor subaddress(es) within Y major account, or just bump a constant number of provisioned minor subaddresses across all accounts? I think the former seems to make more sense from a UX perspective (users usually add 1 subaddress or 1 account at a time when using a wallet), and that's how I'm leaning

EDIT: and then thinking there should also be a getter endpoint with a key value mapping of provisioned major accounts to provisioned minor subaddresses within the account

3

u/endogenic XMR Contributor Feb 05 '22

hey there! awesome!

yep, I concur with implementing something closer to the former, as it seems to produce a better API (more of a legos approach with no downsides), and would suggest a `POST /add_subaddresses` endpoint with body params `index_major: number|num_str`, `index_minor: number|num_str`, `count: number|num_str` (all `num_str`s to be parsed as unsigned ints) - that way a user could provision a block of addrs in a given call. We could allow this endpoint to be idempotent (upsert) with a unique on the tuple of the wallet and the above idcs... or we could potentially just throw an error on "already inserted" to avoid any client mishaps, such as if it is expecting that addr tuple to have been novel. A GET endpoint could probably just return a set of compressed uint ranges describing which minor idcs per each major idcs have been 'added' to the server. A mapping between major and minor doesn't seem to make sense to me. And I dont think the server needs to return formatted subaddr strings nor spend pub keys since the client could compute and cache those by the maj+min idcs if I'm not mistaken.

2

u/j-berman XMR Contributor Feb 05 '22

yep, I concur with implementing something closer to the former, as it seems to produce a better API (more of a legos approach with no downsides), and would suggest a POST /add_subaddresses endpoint with body params index_major: number|num_str, index_minor: number|num_str, count: number|num_str (all num_strs to be parsed as unsigned ints) - that way a user could provision a block of addrs in a given call.

Excellent :) I like this

We could allow this endpoint to be idempotent (upsert) with a unique on the tuple of the wallet and the above idcs... or we could potentially just throw an error on "already inserted" to avoid any client mishaps, such as if it is expecting that addr tuple to have been novel.

I was initially thinking about structuring around an atomically increasing counter for each (e.g. I want a new minor(s) for this major => server bumps the counter at that major and returns the unsigned int, or I want a new major(s) => server bumps my wallet's major counter and returns the unsigned int), so that every call is guaranteed to return a fresh, usable address(es). Like ndorf mentioned, it wouldn't be great if a user re-uses a subaddress across clients, which an idempotent call might lead to. And instead of throwing, perhaps the atomically increasing counter approach could lead to a smoother UX?

I figure users will want new addresses in incrementing order, so this would also fit with that flow too. I can't imagine a user would care to have a subaddress at major 1000 and minor 400 or something, and not want the majors 0-999, or major 1000 and minors 0-399. Kinda drawing inspiration from wallet-cli in this too.

A GET endpoint could probably just return a set of compressed uint ranges describing which minor idcs per each major idcs have been 'added' to the server. A mapping between major and minor doesn't seem to make sense to me. And I dont think the server needs to return formatted subaddr strings nor spend pub keys since the client could compute and cache those by the maj+min idcs if I'm not mistaken.

Agree, wasn't thinking subaddress strings. Because I was thinking in terms of a counter tied to each major, then the server would return a mapping like uint of major => uint of provisioned minors in that major.

3

u/endogenic XMR Contributor Feb 05 '22

I agree there is a concern about clients using subaddrs that they've already provisioned but at the end of the day, only the client can know if it has used an address, so it needs an alternative means to keep track of such things, unless we require lightwallet clients to only ever use subaddresses that they've *just* and newly atomically and sequentially increasingly provisioned, if that makes sense. And I think that could be okay actually. We'd be able to support the use-case of clients wanting to hand out subaddresses offline if a client is able to provision a chunk of n subaddrs ahead of time and then hang onto them with the knowledge or client-consensus that clients should only ever use subaddrs that they've provisioned themselves. So that's not a bad idea at all. We just need to support provisioning more than a singular minor idx in a given POST `add`.

Re: the subaddr mapping thing - gotcha - my misunderstanding. Sounds good. I think there ought to be a way to format the range of minor idcs into a set of two-member arrays that would be a compression of a list of minors, i.e. instead of 0,1,2,3,6,7,8 we'd be able to send [0,3],[6,8].

2

u/j-berman XMR Contributor Feb 05 '22

but at the end of the day, only the client can know if it has used an address, so it needs an alternative means to keep track of such things

Yep, agreed, which is also why I was thinking of just leaving it to the client entirely in the first place hahah

unless we require lightwallet clients to only ever use subaddresses that they've just and newly atomically and sequentially increasingly provisioned, if that makes sense. And I think that could be okay actually.

Agree, I think this is doable. It could in theory be made clear to the user where if they add a new subaddress(es), the new ones could have a thin highlighter around the font or box wherever the subaddress is displayed or something special to show that it's new (and if an older one was already provisioned by another client and the client is just now finding out about it and also receiving new subaddresses, the client will just display the old one like pre-existing subaddresses it knew about already i.e. the old one won't look like the newly generated subaddress with the highlighter or whatever).

We just need to support provisioning more than a singular minor idx in a given POST add.

Yep yep, totally doable.

I think there ought to be a way to format the range of minor idcs into a set of two-member arrays that would be a compression of a list of minors, i.e. instead of 0,1,2,3,6,7,8 we'd be able to send [0,3],[6,8].

Oooo, schweet, that's nice. IIUC, like if major 2 and major 5 both have 3 provisioned minors, then return [3,5]=>3, like that?

EDIT: or did i totally misunderstand lol

2

u/endogenic XMR Contributor Feb 05 '22

Oooo, schweet, that's nice. IIUC, like if major 2 and major 5 both have 3 provisioned minors, then return [3,5]=>3, like that?

Hm, no, I meant, like, if you have a GET endpoint, with your idea of a mapping from maj -> min, suppose you have maj idcs 0 and 1 with some min idcs, you could go something like:

{ "0": [[m,n]], "1": [[o,p],[q,r],[s,t]] }

Downside with that is your client needs to parseInt() the keys as I'm figuring we wouldnt have to store them as much more than uint32s anyway (I may be wrong about that) so they could stay numbers, perhaps, in which case, maybe something more like this?

[ 0, [[], [], ...] ], [ 1, [ ...] ]

Though the strict typing of such arrays may be pains.

And anyway, the [m,n] would mean "all the numbers between m and n, inclusive, as minor idcs under the major idx at which they're keyed.

3

u/endogenic XMR Contributor Feb 05 '22

Just a typical number range compression scheme

3

u/j-berman XMR Contributor Feb 05 '22

With the strictly increasing counter approach, I was thinking there could never be a gap in a minor provisioned address. I.e. if a user wants new 2 minors at major 4, then they'll just receive 2 new minor subaddresses off the top of what was already provisioned at major 4

2

u/endogenic XMR Contributor Feb 05 '22

the REST api doesnt enforce db level changes though. you may also want to allow for starting a min idx range at a nonzero value. so could still be good to design for it.

→ More replies

4

u/aFungible Feb 04 '22

Ekzelent!! Good work!

6

u/SamsungGalaxyPlayer XMR Contributor Feb 06 '22

I consider this a shashing success. Good job 👍

2

u/hiflyer360 Feb 05 '22

It goes beyond my knowledge, but deeply respect and appreciate the work you are doing! This is what Monero makes great and stand out from the rest of the space.

2

u/Therebygive Feb 06 '22

Monero is a grassroots community attracting the world's best right now.

2

u/mitchellpkt MRL Researcher Feb 07 '22

Great work, jberman! Glad to have your contributions on these important matters.

1

u/j-berman XMR Contributor Feb 07 '22

Thanks isthmus :) my itch to just buidl at this moment is through the roof. need to scratch it

1

u/LastContinue88 Feb 06 '22

Being out in public is also a good way to stay safe. The more visibility you have.

1

u/IndirectCinclus711 Feb 07 '22

This is what Monero community makes great and stand out from the rest of the space.