Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement UA-specific migrations for zcash_client_sqlite #489

Closed
str4d opened this issue Jan 31, 2022 · 8 comments
Closed

Implement UA-specific migrations for zcash_client_sqlite #489

str4d opened this issue Jan 31, 2022 · 8 comments

Comments

@str4d
Copy link
Contributor

str4d commented Jan 31, 2022

The zcash_client_sqlite data DB needs to be updated to account for UAs. Specifically:

  • We should have an addresses table for storing the derived diversified UAs within the known account(s).
  • The accounts table should have its address-related columns removed (migrating the Sapling and transparent content into the addresses table), and a transparent_fvk column added.
    • The UFVK can then be derived from the component columns, and e.g. if a wallet doesn't want transparent support, they'd represent this by leaving the transparent_fvk column set to NULL.
    • The alternative would be to just have a ufvk column that stores the encoded UFVK, and parse it from there, but storing the individual components is probably more introspectable.
    • This migration requires the caller to provide access to the UFVKs.
@str4d
Copy link
Contributor Author

str4d commented Jan 31, 2022

@daira
Copy link
Contributor

daira commented Feb 2, 2022

Related to #369, which is the more general issue for non-UA-specific database migration.

@pacu
Copy link
Contributor

pacu commented Feb 2, 2022

I've seen other wallets like Yoroi (cardano) that keep track of the addresses given out and if they have received funds or not.
This way they don't roll you a new address automatically if the latest one hasn't received funds.

@str4d
Copy link
Contributor Author

str4d commented Feb 2, 2022

For Bitcoin, that's usually not a choice: wallets following BIP 44 semantics require users to receive funds in (at least some of) generated addresses before subsequent new ones are generated, otherwise they would exceed the gap limit and become unable to detect funds. We will have this issue for our transparent components, but less so: we can always detect any shielded funds received at any index within an account (due to how diversified addresses work), so we can use that to skip forward over any unused transparent receivers.

@pacu
Copy link
Contributor

pacu commented Feb 2, 2022

Great, we ought to weed out what information of all this is useful to be available for end users so that we can have te proper APIs in place.

@str4d
Copy link
Contributor Author

str4d commented Jun 6, 2022

Wrt storing the UFVK as one column per receiver vs just storing the UFVK directly in the table, both of these will require migrations:

  • One column per receiver: we need to migrate the extfvk TEXT column to a dfvk BLOB column (can be done without the seed), add BLOB columns for the transparent FVK and Orchard FVK, and then migrate the existing accounts to UFVKs (via the seed).
    • We can't serialize a dfvk as an extfvk as the former omits data that the latter contained.
    • We don't have a string encoding for Sapling diversifiable FVKs. We could just store them encoded as a Sapling-only UFVK, but that's rather wasteful.
  • Storing UFVK directly: we need to replace the extfvk TEXT column with a ufvk TEXT column, and then migrate the existing Sapling extended FVKs to UFVKs (via the seed).
    • We can split the migration into two steps if we want the initial table migration without access to the seed; we'd just migrate to a Sapling-only-UFVK first, and then separately add the transparent and Orchard receivers.
    • Upside: UFVKs are directly inspectable in the SQL table.
    • Upside: we can in theory import UFVKs with arbitrary (unknown) receivers, as long as they also contain receivers we know how to deal with. We'd still want these to be derived from a (seed, account) pair (or track the ones that are not tied to the seed).
    • Downside: adding a new pool later (e.g. Orchard) means rewriting the existing rows of the ufvk column, instead of adding an extra column and deriving the UFVK on the fly.

@str4d
Copy link
Contributor Author

str4d commented Jun 6, 2022

Decided we'll go with a single ufvk column.

str4d added a commit that referenced this issue Jun 14, 2022
This is a breaking change to the database format. We don't have support
for migrations yet, so existing wallets won't work after this commit
until #489 is done.
adityapk00 pushed a commit to adityapk00/librustzcash that referenced this issue Aug 8, 2022
This is a breaking change to the database format. We don't have support
for migrations yet, so existing wallets won't work after this commit
until zcash#489 is done.
@str4d str4d added this to the UAs in Mobile SDKs milestone Aug 30, 2022
@str4d
Copy link
Contributor Author

str4d commented Aug 31, 2022

#600 implemented a migration that replaced the extfvk column of the accounts table with ufvk, and also ensured that the address column consistently contained the default Unified Address for the account. The remaining work is to move this address column into a separate addresses table, so that we can remember the diversified addresses that have been generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants