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

zcash_client_sqlite: Implement migrations related to autoshielding #594

Closed
str4d opened this issue Jul 28, 2022 · 5 comments
Closed

zcash_client_sqlite: Implement migrations related to autoshielding #594

str4d opened this issue Jul 28, 2022 · 5 comments

Comments

@str4d
Copy link
Contributor

str4d commented Jul 28, 2022

The autoshielding PR (#341) made changes to several tables, for which migrations were implemented manually in the Android and iOS SDKs. This is insufficient because zcash_client_sqlite needs to be a standalone crate, and thus any changes that can't be done simply as new tables (which is all we'd done in the past) need to be handled with migrations.

Once we have migration support (#369), we need to implement the migration from zcash_client_sqlite 0.3.0 to either the current database format, or to whatever we decide to implement in #489. This issue is for ensuring we do one of the two.

@str4d
Copy link
Contributor Author

str4d commented Jul 28, 2022

I've collected the database schemas as they existed at various relevant points in history:

zcash_client_sqlite 0.3.0

Wallet DB
CREATE TABLE IF NOT EXISTS accounts (
    account INTEGER PRIMARY KEY,
    extfvk TEXT NOT NULL,
    address TEXT NOT NULL
)
CREATE TABLE IF NOT EXISTS blocks (
    height INTEGER PRIMARY KEY,
    hash BLOB NOT NULL,
    time INTEGER NOT NULL,
    sapling_tree BLOB NOT NULL
)
CREATE TABLE IF NOT EXISTS transactions (
    id_tx INTEGER PRIMARY KEY,
    txid BLOB NOT NULL UNIQUE,
    created TEXT,
    block INTEGER,
    tx_index INTEGER,
    expiry_height INTEGER,
    raw BLOB,
    FOREIGN KEY (block) REFERENCES blocks(height)
)
CREATE TABLE IF NOT EXISTS received_notes (
    id_note INTEGER PRIMARY KEY,
    tx INTEGER NOT NULL,
    output_index INTEGER NOT NULL,
    account INTEGER NOT NULL,
    diversifier BLOB NOT NULL,
    value INTEGER NOT NULL,
    rcm BLOB NOT NULL,
    nf BLOB NOT NULL UNIQUE,
    is_change INTEGER NOT NULL,
    memo BLOB,
    spent INTEGER,
    FOREIGN KEY (tx) REFERENCES transactions(id_tx),
    FOREIGN KEY (account) REFERENCES accounts(account),
    FOREIGN KEY (spent) REFERENCES transactions(id_tx),
    CONSTRAINT tx_output UNIQUE (tx, output_index)
)
CREATE TABLE IF NOT EXISTS sapling_witnesses (
    id_witness INTEGER PRIMARY KEY,
    note INTEGER NOT NULL,
    block INTEGER NOT NULL,
    witness BLOB NOT NULL,
    FOREIGN KEY (note) REFERENCES received_notes(id_note),
    FOREIGN KEY (block) REFERENCES blocks(height),
    CONSTRAINT witness_height UNIQUE (note, block)
)
CREATE TABLE IF NOT EXISTS sent_notes (
    id_note INTEGER PRIMARY KEY,
    tx INTEGER NOT NULL,
    output_index INTEGER NOT NULL,
    from_account INTEGER NOT NULL,
    address TEXT NOT NULL,
    value INTEGER NOT NULL,
    memo BLOB,
    FOREIGN KEY (tx) REFERENCES transactions(id_tx),
    FOREIGN KEY (from_account) REFERENCES accounts(account),
    CONSTRAINT tx_output UNIQUE (tx, output_index)
)
Cache DB
CREATE TABLE IF NOT EXISTS compactblocks (
    height INTEGER PRIMARY KEY,
    data BLOB NOT NULL
)

37fc286 (current main at time of writing)

Wallet DB
CREATE TABLE IF NOT EXISTS accounts (
    account INTEGER PRIMARY KEY,
    ufvk TEXT,
    address TEXT,
    transparent_address TEXT
)
CREATE TABLE IF NOT EXISTS blocks (
    height INTEGER PRIMARY KEY,
    hash BLOB NOT NULL,
    time INTEGER NOT NULL,
    sapling_tree BLOB NOT NULL
)
CREATE TABLE IF NOT EXISTS transactions (
    id_tx INTEGER PRIMARY KEY,
    txid BLOB NOT NULL UNIQUE,
    created TEXT,
    block INTEGER,
    tx_index INTEGER,
    expiry_height INTEGER,
    raw BLOB,
    FOREIGN KEY (block) REFERENCES blocks(height)
)
CREATE TABLE IF NOT EXISTS received_notes (
    id_note INTEGER PRIMARY KEY,
    tx INTEGER NOT NULL,
    output_index INTEGER NOT NULL,
    account INTEGER NOT NULL,
    diversifier BLOB NOT NULL,
    value INTEGER NOT NULL,
    rcm BLOB NOT NULL,
    nf BLOB NOT NULL UNIQUE,
    is_change INTEGER NOT NULL,
    memo BLOB,
    spent INTEGER,
    FOREIGN KEY (tx) REFERENCES transactions(id_tx),
    FOREIGN KEY (account) REFERENCES accounts(account),
    FOREIGN KEY (spent) REFERENCES transactions(id_tx),
    CONSTRAINT tx_output UNIQUE (tx, output_index)
)
CREATE TABLE IF NOT EXISTS sapling_witnesses (
    id_witness INTEGER PRIMARY KEY,
    note INTEGER NOT NULL,
    block INTEGER NOT NULL,
    witness BLOB NOT NULL,
    FOREIGN KEY (note) REFERENCES received_notes(id_note),
    FOREIGN KEY (block) REFERENCES blocks(height),
    CONSTRAINT witness_height UNIQUE (note, block)
)
CREATE TABLE IF NOT EXISTS sent_notes (
    id_note INTEGER PRIMARY KEY,
    tx INTEGER NOT NULL,
    output_pool INTEGER NOT NULL,
    output_index INTEGER NOT NULL,
    from_account INTEGER NOT NULL,
    address TEXT NOT NULL,
    value INTEGER NOT NULL,
    memo BLOB,
    FOREIGN KEY (tx) REFERENCES transactions(id_tx),
    FOREIGN KEY (from_account) REFERENCES accounts(account),
    CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index)
)
CREATE TABLE IF NOT EXISTS utxos (
    id_utxo INTEGER PRIMARY KEY,
    address TEXT NOT NULL, 
    prevout_txid BLOB NOT NULL, 
    prevout_idx INTEGER NOT NULL, 
    script BLOB NOT NULL, 
    value_zat INTEGER NOT NULL, 
    height INTEGER NOT NULL,
    spent_in_tx INTEGER,
    FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx),
    CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx)
)
Cache DB
CREATE TABLE IF NOT EXISTS compactblocks (
    height INTEGER PRIMARY KEY,
    data BLOB NOT NULL
)

0a1ed9b (the revision currently used by the Android and iOS SDKs)

Wallet DB
CREATE TABLE IF NOT EXISTS accounts (
    account INTEGER PRIMARY KEY,
    extfvk TEXT NOT NULL,
    address TEXT NOT NULL,
    transparent_address TEXT NOT NULL
)
CREATE TABLE IF NOT EXISTS blocks (
    height INTEGER PRIMARY KEY,
    hash BLOB NOT NULL,
    time INTEGER NOT NULL,
    sapling_tree BLOB NOT NULL
)
CREATE TABLE IF NOT EXISTS transactions (
    id_tx INTEGER PRIMARY KEY,
    txid BLOB NOT NULL UNIQUE,
    created TEXT,
    block INTEGER,
    tx_index INTEGER,
    expiry_height INTEGER,
    raw BLOB,
    FOREIGN KEY (block) REFERENCES blocks(height)
)
CREATE TABLE IF NOT EXISTS received_notes (
    id_note INTEGER PRIMARY KEY,
    tx INTEGER NOT NULL,
    output_index INTEGER NOT NULL,
    account INTEGER NOT NULL,
    diversifier BLOB NOT NULL,
    value INTEGER NOT NULL,
    rcm BLOB NOT NULL,
    nf BLOB NOT NULL UNIQUE,
    is_change INTEGER NOT NULL,
    memo BLOB,
    spent INTEGER,
    FOREIGN KEY (tx) REFERENCES transactions(id_tx),
    FOREIGN KEY (account) REFERENCES accounts(account),
    FOREIGN KEY (spent) REFERENCES transactions(id_tx),
    CONSTRAINT tx_output UNIQUE (tx, output_index)
)
CREATE TABLE IF NOT EXISTS sapling_witnesses (
    id_witness INTEGER PRIMARY KEY,
    note INTEGER NOT NULL,
    block INTEGER NOT NULL,
    witness BLOB NOT NULL,
    FOREIGN KEY (note) REFERENCES received_notes(id_note),
    FOREIGN KEY (block) REFERENCES blocks(height),
    CONSTRAINT witness_height UNIQUE (note, block)
)
CREATE TABLE IF NOT EXISTS sent_notes (
    id_note INTEGER PRIMARY KEY,
    tx INTEGER NOT NULL,
    output_index INTEGER NOT NULL,
    from_account INTEGER NOT NULL,
    address TEXT NOT NULL,
    value INTEGER NOT NULL,
    memo BLOB,
    FOREIGN KEY (tx) REFERENCES transactions(id_tx),
    FOREIGN KEY (from_account) REFERENCES accounts(account),
    CONSTRAINT tx_output UNIQUE (tx, output_index)
)
CREATE TABLE IF NOT EXISTS utxos (
    id_utxo INTEGER PRIMARY KEY,
    address TEXT NOT NULL, 
    prevout_txid BLOB NOT NULL, 
    prevout_idx INTEGER NOT NULL, 
    script BLOB NOT NULL, 
    value_zat INTEGER NOT NULL, 
    height INTEGER NOT NULL,
    spent_in_tx INTEGER,
    FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx),
    CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx)
)
Cache DB
CREATE TABLE IF NOT EXISTS compactblocks (
    height INTEGER PRIMARY KEY,
    data BLOB NOT NULL
)

@str4d
Copy link
Contributor Author

str4d commented Jul 28, 2022

There are no differences in the cache DB schema across the three versions. Here are the differences in the wallet schema along the migration pathways that are relevant:

--- wallet-schema-0.3.0.sql
+++ wallet-schema-autoshielding.sql
@@ -1,7 +1,8 @@
 CREATE TABLE IF NOT EXISTS accounts (
     account INTEGER PRIMARY KEY,
     extfvk TEXT NOT NULL,
-    address TEXT NOT NULL
+    address TEXT NOT NULL,
+    transparent_address TEXT NOT NULL
 )
 CREATE TABLE IF NOT EXISTS blocks (
     height INTEGER PRIMARY KEY,
@@ -56,4 +57,16 @@
     FOREIGN KEY (tx) REFERENCES transactions(id_tx),
     FOREIGN KEY (from_account) REFERENCES accounts(account),
     CONSTRAINT tx_output UNIQUE (tx, output_index)
+)
+CREATE TABLE IF NOT EXISTS utxos (
+    id_utxo INTEGER PRIMARY KEY,
+    address TEXT NOT NULL, 
+    prevout_txid BLOB NOT NULL, 
+    prevout_idx INTEGER NOT NULL, 
+    script BLOB NOT NULL, 
+    value_zat INTEGER NOT NULL, 
+    height INTEGER NOT NULL,
+    spent_in_tx INTEGER,
+    FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx),
+    CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx)
 )
--- wallet-schema-autoshielding.sql
+++ wallet-schema-main.sql
@@ -1,8 +1,8 @@
 CREATE TABLE IF NOT EXISTS accounts (
     account INTEGER PRIMARY KEY,
-    extfvk TEXT NOT NULL,
-    address TEXT NOT NULL,
-    transparent_address TEXT NOT NULL
+    ufvk TEXT,
+    address TEXT,
+    transparent_address TEXT
 )
 CREATE TABLE IF NOT EXISTS blocks (
     height INTEGER PRIMARY KEY,
@@ -49,6 +49,7 @@
 CREATE TABLE IF NOT EXISTS sent_notes (
     id_note INTEGER PRIMARY KEY,
     tx INTEGER NOT NULL,
+    output_pool INTEGER NOT NULL,
     output_index INTEGER NOT NULL,
     from_account INTEGER NOT NULL,
     address TEXT NOT NULL,
@@ -56,7 +57,7 @@
     memo BLOB,
     FOREIGN KEY (tx) REFERENCES transactions(id_tx),
     FOREIGN KEY (from_account) REFERENCES accounts(account),
-    CONSTRAINT tx_output UNIQUE (tx, output_index)
+    CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index)
 )
 CREATE TABLE IF NOT EXISTS utxos (
     id_utxo INTEGER PRIMARY KEY,
--- wallet-schema-0.3.0.sql
+++ wallet-schema-main.sql
@@ -1,7 +1,8 @@
 CREATE TABLE IF NOT EXISTS accounts (
     account INTEGER PRIMARY KEY,
-    extfvk TEXT NOT NULL,
-    address TEXT NOT NULL
+    ufvk TEXT,
+    address TEXT,
+    transparent_address TEXT
 )
 CREATE TABLE IF NOT EXISTS blocks (
     height INTEGER PRIMARY KEY,
@@ -48,6 +49,7 @@
 CREATE TABLE IF NOT EXISTS sent_notes (
     id_note INTEGER PRIMARY KEY,
     tx INTEGER NOT NULL,
+    output_pool INTEGER NOT NULL,
     output_index INTEGER NOT NULL,
     from_account INTEGER NOT NULL,
     address TEXT NOT NULL,
@@ -55,5 +57,17 @@
     memo BLOB,
     FOREIGN KEY (tx) REFERENCES transactions(id_tx),
     FOREIGN KEY (from_account) REFERENCES accounts(account),
-    CONSTRAINT tx_output UNIQUE (tx, output_index)
+    CONSTRAINT tx_output UNIQUE (tx, output_pool, output_index)
+)
+CREATE TABLE IF NOT EXISTS utxos (
+    id_utxo INTEGER PRIMARY KEY,
+    address TEXT NOT NULL, 
+    prevout_txid BLOB NOT NULL, 
+    prevout_idx INTEGER NOT NULL, 
+    script BLOB NOT NULL, 
+    value_zat INTEGER NOT NULL, 
+    height INTEGER NOT NULL,
+    spent_in_tx INTEGER,
+    FOREIGN KEY (spent_in_tx) REFERENCES transactions(id_tx),
+    CONSTRAINT tx_outpoint UNIQUE (prevout_txid, prevout_idx)
 )

Looking at these diffs in isolation, I think it would make sense to implement the migrations as 0.3.0 -> autoshielding -> main, even though the autoshielding schema was never present in the main branch. Users of 0.3.0 would have their wallet DBs migrated through the autoshielding schema, as the changes in this direction are purely cumulative (modulo removing the NOT NULL constraint on the new transparent_address column).

However, when we take #489 and #503 into account, we might decide on a different migration strategy. We should decide precisely what the database schemas will look like in those issues and then update this issue.

@nuttycom
Copy link
Contributor

nuttycom commented Aug 2, 2022

So here's the migration strategy I think we need to use:

  • The initial migration takes us to the 0.3.0 wallet database state
  • The first migration adds the utxos table if it does not exist, but does not add the transparent_address column to the accounts table. This way, if this migration is run against a database from the autoshielding-poc branch, it will not crash due to an alter table statement that's trying to add a column that already exists.
  • The second migration takes the seed as context; it renames the old accounts table, then re-creates without either of the address columns; then, it queries the old accounts table (ignoring whatever transparent address was there) and simply verifies that the UFVK for the account correctly generates the Sapling address, then inserts it.

If we need for some reason to preserve the Sapling and transparent address columns, we should just regenerate them from the UFVK, with the caveat that we always generate the 0-indexed transparent address.

The core idea is that we should never migrate into the autoshielding-poc database state; instead, we just need to ensure that we correctly migrate from that database state.

@str4d
Copy link
Contributor Author

str4d commented Aug 2, 2022

If we need for some reason to preserve the Sapling and transparent address columns, we should just regenerate them from the UFVK, with the caveat that we always generate the 0-indexed transparent address.

I believe the current behaviour was already to generate the 0-indexed transparent address, but we should check this. We might also want to take #489 into account for the second migration if it makes things easier (but we can also just add a third migration which also takes the seed if needed).

@str4d
Copy link
Contributor Author

str4d commented Aug 2, 2022

The core idea is that we should never migrate into the autoshielding-poc database state; instead, we just need to ensure that we correctly migrate from that database state.

Agreed. The autoshielding-poc database state only existed inside the Android and iOS SDKs, and those (currently) use their own database migration logic to reach that point; we just need to be able to continue from where they currently are.

We might also need to be able to replicate subsequent migrations in the Android and iOS migration logic (at least in the short term) if it's too much work to replace their usage (I know Android's Room can be somewhat prescriptive). I'm hopeful that we could just call through from inside those migrations directly into the same Rust code being written for here.

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

2 participants