Database format upgrade for supporting various viewing key types

[arya2]

Zebra-scan currently stores keys and results in a single RocksDB column family where the key is an encoded Sapling extended full viewing key (efvk), a block height, and the transaction index in that order.

This should be okay for now because the byte length of an encoded Sapling efvk is constant.

Before adding support for different types of viewing keys in zebra-scan, we need to ensure that the disk format for different key types in that database column will remain constant so that our functions for retrieving or deleting results continue to work as expected.

We could do this by:

• Padding shorter keys with empty bytes so that the disk format stays constant
• Hashing keys and using hashes to select entries in the database
• Using different db columns for different kinds of keys
• Adding a new db column to map viewing-key -> id
  • Using account ids as the key and adding a new column for getting account ids by the full encoded key (this may also help with feat(scan): Associate scanning keys with AccountIDs #8229)
  • Associating registered viewing keys with UUIDs

Update: The likelihood of this happening is negligible, and we can guarantee that it doesn't happen by adding a byte after the viewing key for specifying the type of viewing key. We may want to add a note on correctness to the IntoDisk impl for scanning keys to explain why it's okay.

[upbqdn]

Could you describe or link to a description of the underlying issue?

I only see that a shorter key can become a prefix (or a suffix, depending on which one matters) of a longer, unrelated key, but I still feel like I don't see the full picture.

[arya2]

Yep, I'm less worried about this happening now, I think it would only matter when:

• a shorter key is a suffix of longer key, and
• the first X bytes in the longer key are 0 where X is the length difference between the keys

The likelihood of that happening should be negligible, so I think we can close this discussion or convert it to a tech-debt issue, but the IntoDisk impl for SaplingScanningKey here is of a variable length, so if those conditions are met, it could replace/return/delete the results of either key when it's deleting, retrieving, or inserting Sapling results.

I may have thought the TransactionLocation was also of variable length, so that if:

• A shorter key were a prefix of a longer key, and
• There were some result(s) with a TransactionLocation matching some of the remainder of the longer key
  then those results would have fallen in the retrieve/delete range
• The length difference of the keys were less than 5 after trimming leading zeros
  This would also have been unlikely.

Padding the shorter key wouldn't help, I think we could instead add a byte for specifying the viewing key type between the encoded key and the transaction location to guarantee that it can't happen.

So, it shouldn't be a problem in its current state, we should be careful to avoid conflicts if we make changes to it since it's the only variable-length key in Zebra (zebra-state column families are here, all of the IntoDisk impls for their keys return a fixed-size array of bytes except the orchard/sapling subtree keys, which in practice are also always 32 bytes - though we may want to update the type there too).

[arya2]

The likelihood of this happening is negligible, and we can guarantee that it can't happen by adding a byte after the viewing key for specifying the type of viewing key.