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

Support older sqlite versions. #1511

Merged
merged 6 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 17 additions & 4 deletions zcash_client_sqlite/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ and this library adheres to Rust's notion of

## [Unreleased]

## [0.11.2] - 2024-08-21

### Changed
- The `v_tx_outputs` view was modified slightly to support older versions of
`sqlite`. Queries to the exposed `v_tx_outputs` and `v_transactions` views
are supported for SQLite versions back to `3.19.x`.
- `zcash_client_sqlite::wallet::init::WalletMigrationError` has an additional
variant, `DatabaseNotSupported`. The `init_wallet_db` function now checks
that the sqlite version in use is compatible with the features required by
the wallet and returns this error if not. SQLite version `3.35` or higher
is required for use with `zcash_client_sqlite`.


## [0.11.1] - 2024-08-21

### Fixed
Expand All @@ -18,7 +31,7 @@ and this library adheres to Rust's notion of
`zcash_client_sqlite` now provides capabilities for the management of ephemeral
transparent addresses in support of the creation of ZIP 320 transaction pairs.

In addition, `zcash_client_sqlite` now provides improved tracking of transparent
In addition, `zcash_client_sqlite` now provides improved tracking of transparent
wallet history in support of the API changes in `zcash_client_backend 0.13`,
and the `v_transactions` view has been modified to provide additional metadata
about the relationship of each transaction to the wallet, in particular whether
Expand Down Expand Up @@ -65,11 +78,11 @@ or not the transaction represents a wallet-internal shielding operation.
## [0.10.1] - 2024-03-25

### Fixed
- The `sent_notes` table's `received_note` constraint was excessively restrictive
after zcash/librustzcash#1306. Any databases that have migrations from
- The `sent_notes` table's `received_note` constraint was excessively restrictive
after zcash/librustzcash#1306. Any databases that have migrations from
zcash_client_sqlite 0.10.0 applied should be wiped and restored from seed.
In order to ensure that the incorrect migration is not used, the migration
id for the `full_account_ids` migration has been changed from
id for the `full_account_ids` migration has been changed from
`0x1b104345_f27e_42da_a9e3_1de22694da43` to `0x6d02ec76_8720_4cc6_b646_c4e2ce69221c`

## [0.10.0] - 2024-03-25
Expand Down
4 changes: 2 additions & 2 deletions zcash_client_sqlite/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "zcash_client_sqlite"
description = "An SQLite-based Zcash light client"
version = "0.11.1"
version = "0.11.2"
authors = [
"Jack Grigg <jack@z.cash>",
"Kris Nuttycombe <kris@electriccoin.co>"
Expand Down Expand Up @@ -71,6 +71,7 @@ schemer.workspace = true
schemer-rusqlite.workspace = true
time.workspace = true
uuid.workspace = true
regex = "1.4"

# Dependencies used internally:
# (Breaking upgrades to these are usually backwards-compatible, but check MSRVs.)
Expand All @@ -87,7 +88,6 @@ orchard = { workspace = true, features = ["test-dependencies"] }
proptest.workspace = true
rand_chacha.workspace = true
rand_core.workspace = true
regex = "1.4"
tempfile = "3.5.0"
zcash_keys = { workspace = true, features = ["test-dependencies"] }
zcash_note_encryption.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion zcash_client_sqlite/src/wallet/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ SELECT transactions.txid AS txid,
NULL AS to_account_id,
sent_notes.to_address AS to_address,
sent_notes.value AS value,
FALSE AS is_change,
0 AS is_change,
sent_notes.memo AS memo
FROM sent_notes
JOIN transactions
Expand Down
51 changes: 51 additions & 0 deletions zcash_client_sqlite/src/wallet/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::fmt;
use std::rc::Rc;

use regex::Regex;
use schemer::{Migrator, MigratorError};
use schemer_rusqlite::RusqliteAdapter;
use secrecy::SecretVec;
Expand All @@ -20,8 +21,15 @@ use crate::{error::SqliteClientError, WalletDb};

mod migrations;

const SQLITE_MAJOR_VERSION: u32 = 3;
const MIN_SQLITE_MINOR_VERSION: u32 = 35;

#[derive(Debug)]
pub enum WalletMigrationError {
/// A feature required by the wallet database is not supported by the version of
/// SQLite that the migration is running against.
DatabaseNotSupported(String),

/// The seed is required for the migration.
SeedRequired,

Expand Down Expand Up @@ -100,6 +108,13 @@ impl From<SqliteClientError> for WalletMigrationError {
impl fmt::Display for WalletMigrationError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match &self {
WalletMigrationError::DatabaseNotSupported(version) => {
write!(
f,
"The installed SQLite version {} does not support operations required by the wallet.",
version
)
}
WalletMigrationError::SeedRequired => {
write!(
f,
Expand Down Expand Up @@ -305,6 +320,8 @@ fn init_wallet_db_internal<P: consensus::Parameters + 'static>(
) -> Result<(), MigratorError<WalletMigrationError>> {
let seed = seed.map(Rc::new);

verify_sqlite_version_compatibility(&wdb.conn).map_err(MigratorError::Adapter)?;
nuttycom marked this conversation as resolved.
Show resolved Hide resolved

// Turn off foreign key enforcement, to ensure that table replacement does not break foreign
// key references in table definitions.
//
Expand Down Expand Up @@ -357,6 +374,40 @@ fn init_wallet_db_internal<P: consensus::Parameters + 'static>(
Ok(())
}

/// Verify that the sqlite version in use supports the features required by this library.
/// Note that the version of sqlite available to the database backend may be different
/// from what is used to query the views that are part of the public API.
fn verify_sqlite_version_compatibility(
conn: &rusqlite::Connection,
) -> Result<(), WalletMigrationError> {
let sqlite_version =
conn.query_row("SELECT sqlite_version()", [], |row| row.get::<_, String>(0))?;

let version_re = Regex::new(r"^(?<major>[0-9]+)\.(?<minor>[0-9]+).*$").unwrap();
let captures =
version_re
.captures(&sqlite_version)
.ok_or(WalletMigrationError::DatabaseNotSupported(
"Unknown".to_owned(),
))?;
let parse_version_part = |part: &str| {
captures[part].parse::<u32>().map_err(|_| {
WalletMigrationError::CorruptedData(format!(
"Cannot decode SQLite {} version component {}",
part, &captures[part]
))
nuttycom marked this conversation as resolved.
Show resolved Hide resolved
})
};
let major = parse_version_part("major")?;
let minor = parse_version_part("minor")?;

if major != SQLITE_MAJOR_VERSION || minor < MIN_SQLITE_MINOR_VERSION {
Err(WalletMigrationError::DatabaseNotSupported(sqlite_version))
} else {
Ok(())
}
}

#[cfg(test)]
#[allow(deprecated)]
mod tests {
Expand Down
4 changes: 4 additions & 0 deletions zcash_client_sqlite/src/wallet/init/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod sapling_memo_consistency;
mod sent_notes_to_internal;
mod shardtree_support;
mod spend_key_available;
mod support_legacy_sqlite;
mod tx_retrieval_queue;
mod ufvk_support;
mod utxos_table;
Expand Down Expand Up @@ -74,6 +75,8 @@ pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
// \ \ ephemeral_addresses / /
// \ \ | / /
// ------------------------------ tx_retrieval_queue ----------------------------
// |
// support_legacy_sqlite
vec![
Box::new(initial_setup::Migration {}),
Box::new(utxos_table::Migration {}),
Expand Down Expand Up @@ -131,6 +134,7 @@ pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
Box::new(tx_retrieval_queue::Migration {
params: params.clone(),
}),
Box::new(support_legacy_sqlite::Migration),
]
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
//! Modifies definitions to avoid keywords that may not be available in older SQLite versions.
use std::collections::HashSet;

use rusqlite;
use schemer;
use schemer_rusqlite::RusqliteMigration;
use uuid::Uuid;

use crate::wallet::init::{migrations::tx_retrieval_queue, WalletMigrationError};

pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0x156d8c8f_2173_4b59_89b6_75697d5a2103);

const DEPENDENCIES: &[Uuid] = &[tx_retrieval_queue::MIGRATION_ID];

pub(super) struct Migration;

impl schemer::Migration for Migration {
fn id(&self) -> Uuid {
MIGRATION_ID
}

fn dependencies(&self) -> HashSet<Uuid> {
DEPENDENCIES.iter().copied().collect()
}

fn description(&self) -> &'static str {
"Modifies definitions to avoid keywords that may not be available in older SQLite versions."
}
}

impl RusqliteMigration for Migration {
type Error = WalletMigrationError;

fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> {
transaction.execute_batch(
r#"
DROP VIEW v_tx_outputs;
CREATE VIEW v_tx_outputs AS
-- select all outputs received by the wallet
SELECT transactions.txid AS txid,
ro.pool AS output_pool,
ro.output_index AS output_index,
sent_notes.from_account_id AS from_account_id,
ro.account_id AS to_account_id,
NULL AS to_address,
ro.value AS value,
ro.is_change AS is_change,
ro.memo AS memo
FROM v_received_outputs ro
JOIN transactions
ON transactions.id_tx = ro.transaction_id
-- join to the sent_notes table to obtain `from_account_id`
LEFT JOIN sent_notes ON sent_notes.id = ro.sent_note_id
UNION
-- select all outputs sent from the wallet to external recipients
SELECT transactions.txid AS txid,
sent_notes.output_pool AS output_pool,
sent_notes.output_index AS output_index,
sent_notes.from_account_id AS from_account_id,
NULL AS to_account_id,
sent_notes.to_address AS to_address,
sent_notes.value AS value,
0 AS is_change,
sent_notes.memo AS memo
FROM sent_notes
JOIN transactions
ON transactions.id_tx = sent_notes.tx
LEFT JOIN v_received_outputs ro ON ro.sent_note_id = sent_notes.id
-- exclude any sent notes for which a row exists in the v_received_outputs view
WHERE ro.account_id IS NULL
"#,
)?;

Ok(())
}

fn down(&self, _: &rusqlite::Transaction) -> Result<(), WalletMigrationError> {
Err(WalletMigrationError::CannotRevert(MIGRATION_ID))
}
}

#[cfg(test)]
mod tests {
use crate::wallet::init::migrations::tests::test_migrate;

#[test]
fn migrate() {
test_migrate(&[super::MIGRATION_ID]);
}
}
Loading