lightwalletd icon indicating copy to clipboard operation
lightwalletd copied to clipboard

Add `out_ciphertext` to CompactOutput as optional field

Open adityapk00 opened this issue 3 years ago • 6 comments

What is your feature request? The CompactOutput returned by LightwalletD today doesn't have enough information to do a trial decryption of a sapling CompactOutput with an OutgoingViewKey. Specifically, it is missing the out_ciphertext. Please see: https://github.com/zcash/librustzcash/issues/381

Note that out_ciphertext is 80 bytes, so this is some overhead.

It would be nice if Light clients could optionally request this field during GetBlockRange and GetBlock.

How would this feature help you? This will allow Light wallets that are scanning backwards (i.e., sync that sees spends before receives) to detect outgoing transactions reliably, significantly improving sync speed.

adityapk00 avatar May 05 '21 16:05 adityapk00

cc: @LarryRuane @gmale @braddmiller

adityapk00 avatar May 05 '21 16:05 adityapk00

Right now, whenever the wallet finds a compact transaction that relates to it, it requests the related full transaction. One of the reasons is to get the memo but another major reason was to find outbound transaction information. Otherwise, on restore a wallet only shows inbound transactions. It would also miss transactions sent with that same seed from another device.

I believe that if we added this out_ciphertext it would reduce some of the need to fetch full transactions at the expense of 80 bytes per output for all transactions in every block. At 15 tx per block and 2 outputs per tx that's rougly 2.8MB per day and 83MB per month.

To me that's reasonable. Especially if it enables significantly faster scanning.

gmale avatar May 05 '21 17:05 gmale

One slight complication with this is that the compact block cache (used to be in-memory, now on-disk) will have to be rebuilt. That is, lightwalletd will have to resync from Sapling activation height, from zcashd. Last I checked, this took about 40 minutes on mainnet. Not a big deal, but just something to keep in mind. Also, we'll have to make sure that we detect that this needs to be done (on first startup after upgrade), without (obviously) doing this after just the first time.

LarryRuane avatar May 06 '21 18:05 LarryRuane

My only remarks from a security point of view is that it adds complexity to lightwalletd, and if it's optional, as @str4d points out in the other ticket, network adversaries can distinguish between requests that ask for the output ciphertexts and those that don't. As far as I can tell that won't actually change anything in the privacy threat model (i.e. it doesn't enable any attacks that I can see), so no objections from me if we're fine with the additional bandwidth usage and the added code complexity in lightwalletd (which is probably nontrivial if it's optional).

defuse avatar May 13 '21 02:05 defuse

It's also worth noting that in NU5 we're adding the ability to authenticate CompactBlock data, and this new field won't be included in that. An adversary can't provide an invalid or malicious outCiphertext that they want the client to treat as valid, because the note commitment check of the resulting decrypted note still fail. But they can provide an invalid outCiphertext in place of what would have been a valid one, to undetectably (at the time, detectable in forensics while the block cache exists or if the full transaction is later fetched) prevent the client from decrypting an output.

str4d avatar May 13 '21 05:05 str4d

Update: This is now lower priority for Zecwallet, since we'll use nullifiers to detect outgoing transactions even for BlazeSync, so that part of the usecase no longer applies.

However, it would be a nice-to-have, in case we wanted to add a feature where users could turn off fetching "full transactions" to improve privacy. In that case, this would allow decoding the outgoing tx amount and address just from the CompactBlock.

adityapk00 avatar May 18 '21 20:05 adityapk00