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.

Sign up for GitHub

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

Add WalletRead::get_seed_account #1259

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Add WalletRead::get_seed_account #1259

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33e943d
zcash_client_backend: Add `WalletRead::get_seed_account`
nuttycom Mar 9, 2024
328e854
zcash_client_sqlite: Ensure uniqueness of (hd_seed_fingerprinte, hd_a…
nuttycom Mar 12, 2024
79f5bb4
zcash_client_sqlite: Add index verification to `verify_schema`
nuttycom Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions zcash_client_sqlite/src/wallet/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,54 @@ mod tests {
expected_idx += 1;
}

let expected_indices = vec![
r#"CREATE UNIQUE INDEX accounts_ufvk ON "accounts" (ufvk)"#,
r#"CREATE UNIQUE INDEX accounts_uivk ON "accounts" (uivk)"#,
r#"CREATE UNIQUE INDEX hd_account ON "accounts" (hd_seed_fingerprint, hd_account_index)"#,
r#"CREATE INDEX "addresses_accounts" ON "addresses" (
Comment on lines +439 to +443
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this; I was unaware that uniqueness constraints were starting to be embedded in the indices (which prior to now I only thought were present for performance). I rely heavily on changes to init.rs to track changes to the database structure and determine what constraints are applied.

"account_id" ASC
)"#,
r#"CREATE INDEX nf_map_locator_idx ON nullifier_map(block_height, tx_index)"#,
r#"CREATE INDEX orchard_received_notes_account ON orchard_received_notes (
account_id ASC
)"#,
r#"CREATE INDEX orchard_received_notes_spent ON orchard_received_notes (
spent ASC
)"#,
r#"CREATE INDEX orchard_received_notes_tx ON orchard_received_notes (
tx ASC
)"#,
r#"CREATE INDEX "sapling_received_notes_account" ON "sapling_received_notes" (
"account_id" ASC
)"#,
r#"CREATE INDEX "sapling_received_notes_spent" ON "sapling_received_notes" (
"spent" ASC
)"#,
r#"CREATE INDEX "sapling_received_notes_tx" ON "sapling_received_notes" (
"tx" ASC
)"#,
r#"CREATE INDEX sent_notes_from_account ON "sent_notes" (from_account_id)"#,
r#"CREATE INDEX sent_notes_to_account ON "sent_notes" (to_account_id)"#,
r#"CREATE INDEX sent_notes_tx ON "sent_notes" (tx)"#,
r#"CREATE INDEX utxos_received_by_account ON "utxos" (received_by_account_id)"#,
r#"CREATE INDEX utxos_spent_in_tx ON "utxos" (spent_in_tx)"#,
];
let mut indices_query = st
.wallet()
.conn
.prepare("SELECT sql FROM sqlite_master WHERE type = 'index' AND sql != '' ORDER BY tbl_name, name")
.unwrap();
let mut rows = indices_query.query([]).unwrap();
let mut expected_idx = 0;
while let Some(row) = rows.next().unwrap() {
let sql: String = row.get(0).unwrap();
assert_eq!(
re.replace_all(&sql, " "),
re.replace_all(expected_indices[expected_idx], " ")
);
expected_idx += 1;
}

let expected_views = vec![
// v_orchard_shard_scan_ranges
format!(
Expand Down
4 changes: 2 additions & 2 deletions zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
)
);
CREATE UNIQUE INDEX hd_account ON accounts_new (hd_seed_fingerprint, hd_account_index);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the full subtleties of UNIQUE INDEX vs CONSTRAIN UNIQUE and why you'd choose one over the other. AFAICT:

  • They both produce an index.
  • UNIQUE INDEX can be removed and added without altering the table (and I guess CREATE UNIQUE INDEX fails if the table contains non-unique data?)
  • UNIQUE INDEX can include expressions
  • CONSTRAIN UNIQUE has greater support in ON CONFLICT logic (whereas UNIQUE INDEX only supports INSERT ... ON CONFLICT I think).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was looking at this today. I think that the ON CONFLICT capabilities are actually the same, from what I was reading, at least for recent releases? Given that the separate indices can be dropped and added independently, I think I have a slight preference for them, but I don't think it makes much difference one way or the other.

CREATE UNIQUE INDEX accounts_uivk ON accounts_new ("uivk");
CREATE UNIQUE INDEX accounts_ufvk ON accounts_new ("ufvk");
CREATE UNIQUE INDEX accounts_uivk ON accounts_new (uivk);
CREATE UNIQUE INDEX accounts_ufvk ON accounts_new (ufvk);
"#),
)?;

Expand Down
Loading