From 29ae0a4a5e5e17ae0ca4951ed3b4e7043803c683 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 29 Mar 2023 14:57:41 +0200 Subject: [PATCH 1/4] db: add a new 'updated_at' column to Spend transactions Since this is our first modification to the database schema since the first release of the software this also introduces migration logic for existing databases. --- src/database/sqlite/mod.rs | 26 +++++++---------- src/database/sqlite/schema.rs | 13 +++++++-- src/database/sqlite/utils.rs | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 60eca4e0..3c074625 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -14,7 +14,10 @@ use crate::{ database::{ sqlite::{ schema::{DbAddress, DbCoin, DbSpendTransaction, DbTip, DbWallet}, - utils::{create_fresh_db, db_exec, db_query, db_tx_query, LOOK_AHEAD_LIMIT}, + utils::{ + create_fresh_db, db_exec, db_query, db_tx_query, db_version, maybe_apply_migration, + LOOK_AHEAD_LIMIT, + }, }, Coin, CoinType, }, @@ -31,7 +34,7 @@ use miniscript::bitcoin::{ util::{bip32, psbt::PartiallySignedTransaction as Psbt}, }; -const DB_VERSION: i64 = 0; +const DB_VERSION: i64 = 1; #[derive(Debug)] pub enum SqliteDbError { @@ -108,6 +111,9 @@ impl SqliteDb { return Err(SqliteDbError::FileNotFound(db_path)); } + log::info!("Checking if the database needs upgrading."); + maybe_apply_migration(&db_path)?; + Ok(SqliteDb { db_path }) } @@ -126,8 +132,7 @@ impl SqliteDb { ) -> Result<(), SqliteDbError> { let mut conn = self.connection()?; - // Check if there database isn't from the future. - // NOTE: we'll do migration there eventually. Until then be strict on the check. + // At this point any migration must have been applied. let db_version = conn.db_version(); if db_version != DB_VERSION { return Err(SqliteDbError::UnsupportedVersion(db_version)); @@ -160,18 +165,7 @@ pub struct SqliteConn { impl SqliteConn { pub fn db_version(&mut self) -> i64 { - db_query( - &mut self.conn, - "SELECT version FROM version", - rusqlite::params![], - |row| { - let version: i64 = row.get(0)?; - Ok(version) - }, - ) - .expect("db must not fail") - .pop() - .expect("There is always a row in the version table") + db_version(&mut self.conn).expect("db must not fail") } /// Get the network tip. diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 9a5d6354..597d8114 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -74,7 +74,8 @@ CREATE TABLE addresses ( CREATE TABLE spend_transactions ( id INTEGER PRIMARY KEY NOT NULL, psbt BLOB UNIQUE NOT NULL, - txid BLOB UNIQUE NOT NULL + txid BLOB UNIQUE NOT NULL, + updated_at INTEGER ); "; @@ -253,6 +254,7 @@ pub struct DbSpendTransaction { pub id: i64, pub psbt: Psbt, pub txid: bitcoin::Txid, + pub updated_at: Option, } impl TryFrom<&rusqlite::Row<'_>> for DbSpendTransaction { @@ -268,6 +270,13 @@ impl TryFrom<&rusqlite::Row<'_>> for DbSpendTransaction { let txid: bitcoin::Txid = encode::deserialize(&txid).expect("We only store valid txids"); assert_eq!(txid, psbt.unsigned_tx.txid()); - Ok(DbSpendTransaction { id, psbt, txid }) + let updated_at = row.get(3)?; + + Ok(DbSpendTransaction { + id, + psbt, + txid, + updated_at, + }) } } diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index 87b6555d..b808812f 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -134,3 +134,58 @@ pub fn create_fresh_db( Ok(()) } + +pub fn db_version(conn: &mut rusqlite::Connection) -> Result { + Ok(db_query( + conn, + "SELECT version FROM version", + rusqlite::params![], + |row| { + let version: i64 = row.get(0)?; + Ok(version) + }, + )? + .pop() + .expect("There is always a row in the version table")) +} + +// In Liana 0.4 we upgraded the schema to hold a timestamp for transaction drafts. Existing +// transaction drafts are not set any timestamp on purpose. +fn migrate_v0_to_v1(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError> { + db_exec(conn, |tx| { + tx.execute( + "ALTER TABLE spend_transactions ADD COLUMN updated_at", + rusqlite::params![], + )?; + tx.execute( + "UPDATE version SET version = 1", + rusqlite::params![], + )?; + Ok(()) + })?; + + Ok(()) +} + +/// Check the database version and if necessary apply the migrations to upgrade it to the current +/// one. +pub fn maybe_apply_migration(db_path: &path::Path) -> Result<(), SqliteDbError> { + let mut conn = rusqlite::Connection::open(db_path)?; + + // Iteratively apply the database migrations necessary. + loop { + let version = db_version(&mut conn)?; + match version { + DB_VERSION => { + log::info!("Database is up to date."); + return Ok(()); + } + 0 => { + log::warn!("Upgrading database from version 0 to version 1."); + migrate_v0_to_v1(&mut conn)?; + log::warn!("Migration from database version 0 to version 1 successful."); + } + _ => return Err(SqliteDbError::UnsupportedVersion(version)), + } + } +} From 6b666e75c0fc306dc58574a48346af0bc958a153 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 29 Mar 2023 15:28:59 +0200 Subject: [PATCH 2/4] db: unit test the migration from v0 to v1 --- src/database/sqlite/mod.rs | 172 +++++++++++++++++++++++++++++++++-- src/database/sqlite/utils.rs | 12 +-- src/lib.rs | 8 +- 3 files changed, 174 insertions(+), 18 deletions(-) diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 3c074625..ed81983d 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -13,7 +13,7 @@ use crate::{ bitcoin::BlockChainTip, database::{ sqlite::{ - schema::{DbAddress, DbCoin, DbSpendTransaction, DbTip, DbWallet}, + schema::{DbAddress, DbCoin, DbSpendTransaction, DbTip, DbWallet, SCHEMA}, utils::{ create_fresh_db, db_exec, db_query, db_tx_query, db_version, maybe_apply_migration, LOOK_AHEAD_LIMIT, @@ -85,8 +85,24 @@ impl From for SqliteDbError { #[derive(Debug, Clone)] pub struct FreshDbOptions { - pub bitcoind_network: bitcoin::Network, - pub main_descriptor: LianaDescriptor, + pub(self) bitcoind_network: bitcoin::Network, + pub(self) main_descriptor: LianaDescriptor, + pub(self) schema: &'static str, + pub(self) version: i64, +} + +impl FreshDbOptions { + pub fn new( + bitcoind_network: bitcoin::Network, + main_descriptor: LianaDescriptor, + ) -> FreshDbOptions { + FreshDbOptions { + bitcoind_network, + main_descriptor, + schema: SCHEMA, + version: DB_VERSION, + } + } } #[derive(Debug, Clone)] @@ -589,13 +605,86 @@ mod tests { use bitcoin::{hashes::Hash, util::bip32}; + // The database schema used by the first versions of Liana (database version 0). Used to test + // migrations starting from the first version. + const V0_SCHEMA: &str = "\ +CREATE TABLE version ( + version INTEGER NOT NULL +); + +/* About the Bitcoin network. */ +CREATE TABLE tip ( + network TEXT NOT NULL, + blockheight INTEGER, + blockhash BLOB +); + +/* This stores metadata about our wallet. We only support single wallet for + * now (and the foreseeable future). + * + * The 'timestamp' field is the creation date of the wallet. We guarantee to have seen all + * information related to our descriptor(s) that occured after this date. + * The optional 'rescan_timestamp' field is a the timestamp we need to rescan the chain + * for events related to our descriptor(s) from. + */ +CREATE TABLE wallets ( + id INTEGER PRIMARY KEY NOT NULL, + timestamp INTEGER NOT NULL, + main_descriptor TEXT NOT NULL, + deposit_derivation_index INTEGER NOT NULL, + change_derivation_index INTEGER NOT NULL, + rescan_timestamp INTEGER +); + +/* Our (U)TxOs. + * + * The 'spend_block_height' and 'spend_block.time' are only present if the spending + * transaction for this coin exists and was confirmed. + */ +CREATE TABLE coins ( + id INTEGER PRIMARY KEY NOT NULL, + wallet_id INTEGER NOT NULL, + blockheight INTEGER, + blocktime INTEGER, + txid BLOB NOT NULL, + vout INTEGER NOT NULL, + amount_sat INTEGER NOT NULL, + derivation_index INTEGER NOT NULL, + is_change BOOLEAN NOT NULL CHECK (is_change IN (0,1)), + spend_txid BLOB, + spend_block_height INTEGER, + spend_block_time INTEGER, + UNIQUE (txid, vout), + FOREIGN KEY (wallet_id) REFERENCES wallets (id) + ON UPDATE RESTRICT + ON DELETE RESTRICT +); + +/* A mapping from descriptor address to derivation index. Necessary until + * we can get the derivation index from the parent descriptor from bitcoind. + */ +CREATE TABLE addresses ( + receive_address TEXT NOT NULL UNIQUE, + change_address TEXT NOT NULL UNIQUE, + derivation_index INTEGER NOT NULL UNIQUE +); + +/* Transactions we created that spend some of our coins. */ +CREATE TABLE spend_transactions ( + id INTEGER PRIMARY KEY NOT NULL, + psbt BLOB UNIQUE NOT NULL, + txid BLOB UNIQUE NOT NULL +); +"; + + fn psbt_from_str(psbt_str: &str) -> Psbt { + bitcoin::consensus::deserialize(&base64::decode(psbt_str).unwrap()).unwrap() + } + fn dummy_options() -> FreshDbOptions { let desc_str = "wsh(andor(pk([aabbccdd]tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/<0;1>/*),older(10000),pk([aabbccdd]tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/<0;1>/*)))#dw4ulnrs"; let main_descriptor = LianaDescriptor::from_str(desc_str).unwrap(); - FreshDbOptions { - bitcoind_network: bitcoin::Network::Bitcoin, - main_descriptor, - } + FreshDbOptions::new(bitcoin::Network::Bitcoin, main_descriptor) } fn dummy_db() -> ( @@ -1315,4 +1404,73 @@ mod tests { fs::remove_dir_all(tmp_dir).unwrap(); } + + #[test] + fn v0_to_v1_migration() { + let secp = secp256k1::Secp256k1::verification_only(); + + // Create a database with version 0, using the old schema. + let tmp_dir = tmp_dir(); + eprintln!("{}", tmp_dir.as_path().to_string_lossy()); + fs::create_dir_all(&tmp_dir).unwrap(); + let db_path: path::PathBuf = [tmp_dir.as_path(), path::Path::new("lianad_v0.sqlite3")] + .iter() + .collect(); + let mut options = dummy_options(); + options.schema = V0_SCHEMA; + options.version = 0; + create_fresh_db(&db_path, options, &secp).unwrap(); + + // Two PSBTs we'll insert in the DB before and after the migration. Note they are random + // PSBTs taken from the descriptor unit tests, it doesn't matter. + let first_psbt = psbt_from_str("cHNidP8BAIkCAAAAAWi3OFgkj1CqCDT3Swm8kbxZS9lxz4L3i4W2v9KGC7nqAQAAAAD9////AkANAwAAAAAAIgAg27lNc1rog+dOq80ohRuds4Hgg/RcpxVun2XwgpuLSrFYMwwAAAAAACIAIDyWveqaElWmFGkTbFojg1zXWHODtiipSNjfgi2DqBy9AAAAAAABAOoCAAAAAAEBsRWl70USoAFFozxc86pC7Dovttdg4kvja//3WMEJskEBAAAAAP7///8CWKmCIk4GAAAWABRKBWYWkCNS46jgF0r69Ehdnq+7T0BCDwAAAAAAIgAgTt5fs+CiB+FRzNC8lHcgWLH205sNjz1pT59ghXlG5tQCRzBEAiBXK9MF8z3bX/VnY2aefgBBmiAHPL4tyDbUOe7+KpYA4AIgL5kU0DFG8szKd+szRzz/OTUWJ0tZqij41h2eU9rSe1IBIQNBB1hy+jKsg1TihMT0dXw7etpu9TkO3NuvhBDFJlBj1cP2AQABAStAQg8AAAAAACIAIE7eX7PgogfhUczQvJR3IFix9tObDY89aU+fYIV5RubUIgICSKJsNs0zFJN58yd2aYQ+C3vhMbi0x7k0FV3wBhR4THlIMEUCIQCPWWWOhs2lThxOq/G8X2fYBRvM9MXSm7qPH+dRVYQZEwIgfut2vx3RvwZWcgEj4ohQJD5lNJlwOkA4PAiN1fjx6dABIgID3mvj1zerZKohOVhKCiskYk+3qrCum6PIwDhQ16ePACpHMEQCICZNR+0/1hPkrDQwPFmg5VjUHkh6aK9cXUu3kPbM8hirAiAyE/5NUXKfmFKij30isuyysJbq8HrURjivd+S9vdRGKQEBBZNSIQJIomw2zTMUk3nzJ3ZphD4Le+ExuLTHuTQVXfAGFHhMeSEC9OfCXl+sJOrxUFLBuMV4ZUlJYjuzNGZSld5ioY14y8FSrnNkUSED3mvj1zerZKohOVhKCiskYk+3qrCum6PIwDhQ16ePACohA+ECH+HlR+8Sf3pumaXH3IwSsoqSLCH7H1THiBP93z3ZUq9SsmgiBgJIomw2zTMUk3nzJ3ZphD4Le+ExuLTHuTQVXfAGFHhMeRxjat8/MAAAgAEAAIAAAACAAgAAgAAAAAABAAAAIgYC9OfCXl+sJOrxUFLBuMV4ZUlJYjuzNGZSld5ioY14y8Ec/9Y8jTAAAIABAACAAAAAgAIAAIAAAAAAAQAAACIGA95r49c3q2SqITlYSgorJGJPt6qwrpujyMA4UNenjwAqHGNq3z8wAACAAQAAgAEAAIACAACAAAAAAAEAAAAiBgPhAh/h5UfvEn96bpmlx9yMErKKkiwh+x9Ux4gT/d892Rz/1jyNMAAAgAEAAIABAACAAgAAgAAAAAABAAAAACICAlBQ7gGocg7eF3sXrCio+zusAC9+xfoyIV95AeR69DWvHGNq3z8wAACAAQAAgAEAAIACAACAAAAAAAMAAAAiAgMvVy984eg8Kgvj058PBHetFayWbRGb7L0DMnS9KHSJzBxjat8/MAAAgAEAAIAAAACAAgAAgAAAAAADAAAAIgIDSRIG1dn6njdjsDXenHa2lUvQHWGPLKBVrSzbQOhiIxgc/9Y8jTAAAIABAACAAAAAgAIAAIAAAAAAAwAAACICA0/epE59sVEj7Et0I4R9qJQNuX23RNvDZKCRL7eUps9FHP/WPI0wAACAAQAAgAEAAIACAACAAAAAAAMAAAAAIgICgldCOK6iHscv//2NipgaMABLV5TICU/zlP7HlQmlg08cY2rfPzAAAIABAACAAQAAgAIAAIABAAAAAQAAACICApb0p9rfpJshB3J186PGWrvzQdixcwQZWmebOUMdkquZHP/WPI0wAACAAQAAgAAAAIACAACAAQAAAAEAAAAiAgLY5q+unoDxC/HI5BaNiPq12ei1REZIcUAN304JfKXUwxz/1jyNMAAAgAEAAIABAACAAgAAgAEAAAABAAAAIgIDg6cUVCJB79cMcofiURHojxFARWyS4YEhJNRixuOZZRgcY2rfPzAAAIABAACAAAAAgAIAAIABAAAAAQAAAAA="); + let second_psbt = psbt_from_str("cHNidP8BAP0fAQIAAAAGAGo6V8K5MtKcQ8vRFedf5oJiOREiH4JJcEniyRv2800BAAAAAP3///9e3dVLjWKPAGwDeuUOmKFzOYEP5Ipu4LWdOPA+lITrRgAAAAAA/f///7cl9oeu9ssBXKnkWMCUnlgZPXhb+qQO2+OPeLEsbdGkAQAAAAD9////idkxRErbs34vsHUZ7QCYaiVaAFDV9gxNvvtwQLozwHsAAAAAAP3///9EakyJhd2PjwYh1I7zT2cmcTFI5g1nBd3srLeL7wKEewIAAAAA/f///7BcaP77nMaA2NjT/hyI6zueB/2jU/jK4oxmSqMaFkAzAQAAAAD9////AUAfAAAAAAAAFgAUqo7zdMr638p2kC3bXPYcYLv9nYUAAAAAAAEA/X4BAgAAAAABApEoe5xCmSi8hNTtIFwsy46aj3hlcLrtFrug39v5wy+EAQAAAGpHMEQCIDeI8JTWCTyX6opCCJBhWc4FytH8g6fxDaH+Wa/QqUoMAiAgbITpz8TBhwxhv/W4xEXzehZpOjOTjKnPw36GIy6SHAEhA6QnYCHUbU045FVh6ZwRwYTVineqRrB9tbqagxjaaBKh/v///+v1seDE9gGsZiWwewQs3TKuh0KSBIHiEtG8ABbz2DpAAQAAAAD+////Aqhaex4AAAAAFgAUkcVOEjVMct0jyCzhZN6zBT+lvTQvIAAAAAAAACIAIKKDUd/GWjAnwU99llS9TAK2dK80/nSRNLjmrhj0odUEAAJHMEQCICSn+boh4ItAa3/b4gRUpdfblKdcWtMLKZrgSEFFrC+zAiBtXCx/Dq0NutLSu1qmzFF1lpwSCB3w3MAxp5W90z7b/QEhA51S2ERUi0bg+l+bnJMJeAfDknaetMTagfQR9+AOrVKlxdMkAAEBKy8gAAAAAAAAIgAgooNR38ZaMCfBT32WVL1MArZ0rzT+dJE0uOauGPSh1QQiAgN+zbSfdr8oJBtlKomnQTHynF2b/UhovAwf0eS8awRSqUgwRQIhAJhm6xQvxt2LY+eNZqjhsgMOAxD0OPYty6nf9WaQZtgkAiBf/AXkeyq6ALknO9TZwY6ZRa0evY+DQ3j3XaqiBiAMfgEBBUEhA37NtJ92vygkG2UqiadBMfKcXZv9SGi8DB/R5LxrBFKprHNkdqkUxttmGj2sqzzaxSaacJTnJPDCbY6IrVqyaCIGAv9qeBDEB+5kvM/sZ8jQ7QApfZcDrqtq5OAe2gQ1V+pmDIpk8qkAAAAA0AAAACIGA37NtJ92vygkG2UqiadBMfKcXZv9SGi8DB/R5LxrBFKpDPWswv0AAAAA0AAAAAABAOoCAAAAAAEB0OPoVJs9ihvnAwjO16k/wGJuEus1IEE1Yo2KBjC2NSEAAAAAAP7///8C6AMAAAAAAAAiACBfeUS9jQv6O1a96Aw/mPV6gHxHl3mfj+f0frfAs2sMpP1QGgAAAAAAFgAUDS4UAIpdm1RlFYmg0OoCxW0yBT4CRzBEAiAPvbNlnhiUxLNshxN83AuK/lGWwlpXOvmcqoxsMLzIKwIgWwATJuYPf9buLe9z5SnXVnPVL0q6UZaWE5mjCvEl1RUBIQI54LFZmq9Lw0pxKpEGeqI74NnIfQmLMDcv5ySplUS1/wDMJAABASvoAwAAAAAAACIAIF95RL2NC/o7Vr3oDD+Y9XqAfEeXeZ+P5/R+t8CzawykIgICYn4eZbb6KGoxB1PEv/XPiujZFDhfoi/rJPtfHPVML2lHMEQCIDOHEqKdBozXIPLVgtBj3eWC1MeIxcKYDADe4zw0DbcMAiAq4+dbkTNCAjyCxJi0TKz5DWrPulxrqOdjMRHWngXHsQEBBUEhAmJ+HmW2+ihqMQdTxL/1z4ro2RQ4X6Iv6yT7Xxz1TC9prHNkdqkUzc/gCLoe6rQw63CGXhIR3YRz1qCIrVqyaCIGAmJ+HmW2+ihqMQdTxL/1z4ro2RQ4X6Iv6yT7Xxz1TC9pDPWswv0AAAAAqgAAACIGA8JCTIzdSoTJhiKN1pn+NnlkyuKOndiTgH2NIX+yNsYqDIpk8qkAAAAAqgAAAAABAOoCAAAAAAEBRGpMiYXdj48GIdSO809nJnExSOYNZwXd7Ky3i+8ChHsAAAAAAP7///8COMMQAAAAAAAWABQ5rnyuG5T8iuhqfaGAmpzlybo3t+gDAAAAAAAAIgAg7Kz3CX1RBjIvbK9LBYztmi7F1XIxQpX6mtCUkflvvl8CRzBEAiBaYx4sOHckEZwDnSrbb1ivc6seX4Puasm1PBGnBWgSTQIgCeUiXvd90ajI3F4/BHifLUI4fVIgVQFCqLTbbeXQD5oBIQOmGm+gTRx1slzF+wn8NhZoR1xfSYgoKX6bpRSVRjLcEXrOJAABASvoAwAAAAAAACIAIOys9wl9UQYyL2yvSwWM7ZouxdVyMUKV+prQlJH5b75fIgID0X2UJhC5+2jgJqUrihxZxDZHK7jgPFlrUYzoSHQTmP9HMEQCIEM4K8lVACvE2oSMZHDJiOeD81qsYgAvgpRgcSYgKc3AAiAQjdDr2COBea69W+2iVbnODuH3QwacgShW3dS4yeggJAEBBUEhA9F9lCYQufto4CalK4ocWcQ2Ryu44DxZa1GM6Eh0E5j/rHNkdqkU0DTexcgOQQ+BFjgS031OTxcWiH2IrVqyaCIGA9F9lCYQufto4CalK4ocWcQ2Ryu44DxZa1GM6Eh0E5j/DPWswv0AAAAAvwAAACIGA/xg4Uvem3JHVPpyTLP5JWiUH/yk3Y/uUI6JkZasCmHhDIpk8qkAAAAAvwAAAAABAOoCAAAAAAEBmG+mPq0O6QSWEMctsMjvv5LzWHGoT8wsA9Oa05kxIxsBAAAAAP7///8C6AMAAAAAAAAiACDUvIILFr0OxybADV3fB7ms7+ufnFZgicHR0nbI+LFCw1UoGwAAAAAAFgAUC+1ZjCC1lmMcvJ/4JkevqoZF4igCRzBEAiA3d8o96CNgNWHUkaINWHTvAUinjUINvXq0KBeWcsSWuwIgKfzRNWFR2LDbnB/fMBsBY/ylVXcSYwLs8YC+kmko1zIBIQOpEfsLv0htuertA1sgzCwGvHB0vE4zFO69wWEoHClKmAfMJAABASvoAwAAAAAAACIAINS8ggsWvQ7HJsANXd8Huazv65+cVmCJwdHSdsj4sULDIgID96jZc0sCi0IIXf2CpfE7tY+9LRmMsOdSTTHelFxfCwJHMEQCIHlaiMMznx8Cag8Y3X2gXi9Qtg0ZuyHEC6DsOzipSGOKAiAV2eC+S3Mbq6ig5QtRvTBsq5M3hCBdEJQlOrLVhWWt6AEBBUEhA/eo2XNLAotCCF39gqXxO7WPvS0ZjLDnUk0x3pRcXwsCrHNkdqkUyJ+Cbx7vYVY665yjJnMNODyYrAuIrVqyaCIGAt8UyDXk+mW3Y6IZNIBuDJHkdOaZi/UEShkN5L3GiHR5DIpk8qkAAAAAuAAAACIGA/eo2XNLAotCCF39gqXxO7WPvS0ZjLDnUk0x3pRcXwsCDPWswv0AAAAAuAAAAAABAP0JAQIAAAAAAQG7Zoy4I3J9x+OybAlIhxVKcYRuPFrkDFJfxMiC3kIqIAEAAAAA/v///wO5xxAAAAAAABYAFHgBzs9wJNVk6YwR81IMKmckTmC56AMAAAAAAAAWABTQ/LmJix5JoHBOr8LcgEChXHdLROgDAAAAAAAAIgAg7Kz3CX1RBjIvbK9LBYztmi7F1XIxQpX6mtCUkflvvl8CRzBEAiA+sIKnWVE3SmngjUgJdu1K2teW6eqeolfGe0d11b+irAIgL20zSabXaFRNM8dqVlcFsfNJ0exukzvxEOKl/OcF8VsBIQJrUspHq45AMSwbm24//2a9JM8XHFWbOKpyV+gNCtW71nrOJAABASvoAwAAAAAAACIAIOys9wl9UQYyL2yvSwWM7ZouxdVyMUKV+prQlJH5b75fIgID0X2UJhC5+2jgJqUrihxZxDZHK7jgPFlrUYzoSHQTmP9IMEUCIQCmDhJ9fyhlQwPruoOUemDuldtRu3ZkiTM3DA0OhkguSQIgYerNaYdP43DcqI5tnnL3n4jEeMHFCs+TBkOd6hDnqAkBAQVBIQPRfZQmELn7aOAmpSuKHFnENkcruOA8WWtRjOhIdBOY/6xzZHapFNA03sXIDkEPgRY4EtN9Tk8XFoh9iK1asmgiBgPRfZQmELn7aOAmpSuKHFnENkcruOA8WWtRjOhIdBOY/wz1rML9AAAAAL8AAAAiBgP8YOFL3ptyR1T6ckyz+SVolB/8pN2P7lCOiZGWrAph4QyKZPKpAAAAAL8AAAAAAQDqAgAAAAABAT6/vc6qBRzhQyjVtkC25NS2BvGyl2XjjEsw3e8vAesjAAAAAAD+////AgPBAO4HAAAAFgAUEwiWd/qI1ergMUw0F1+qLys5G/foAwAAAAAAACIAIOOPEiwmp2ZXR7ciyrveITXw0tn6zbQUA1Eikd9QlHRhAkcwRAIgJMZdO5A5u2UIMrAOgrR4NcxfNgZI6OfY7GKlZP0O8yUCIDFujbBRnamLEbf0887qidnXo6UgQA9IwTx6Zomd4RvJASEDoNmR2/XcqSyCWrE1tjGJ1oLWlKt4zsFekK9oyB4Hl0HF0yQAAQEr6AMAAAAAAAAiACDjjxIsJqdmV0e3Isq73iE18NLZ+s20FANRIpHfUJR0YSICAo3uyJxKHR9Z8fwvU7cywQCnZyPvtMl3nv54wPW1GSGqSDBFAiEAlLY98zqEL/xTUvm9ZKy5kBa4UWfr4Ryu6BmSZjseXPQCIGy7efKbZLQSDq8RhgNNjl1384gWFTN7nPwWV//SGriyAQEFQSECje7InEodH1nx/C9TtzLBAKdnI++0yXee/njA9bUZIaqsc2R2qRQhPRlaLsh/M/K/9fvbjxF/M20cNoitWrJoIgYCF7Rj5jFhe5L6VDzP5m2BeaG0mA9e7+6fMeWkWxLwpbAMimTyqQAAAADNAAAAIgYCje7InEodH1nx/C9TtzLBAKdnI++0yXee/njA9bUZIaoM9azC/QAAAADNAAAAAAA="); + + // The helper that was used to store Spend transaction in previous versions of the software + // when there was no associated timestamp. + fn store_spend_old(conn: &mut rusqlite::Connection, psbt: &Psbt) { + let txid = psbt.unsigned_tx.txid().to_vec(); + let psbt = encode::serialize(psbt); + + db_exec(conn, |db_tx| { + db_tx.execute( + "INSERT into spend_transactions (psbt, txid) VALUES (?1, ?2) \ + ON CONFLICT DO UPDATE SET psbt=excluded.psbt", + rusqlite::params![psbt, txid], + )?; + Ok(()) + }) + .expect("Db must not fail"); + } + + // Store a PSBT before the migration. + { + let mut conn = rusqlite::Connection::open(&db_path).unwrap(); + store_spend_old(&mut conn, &first_psbt); + } + + // Migrate the DB. We should be able to insert another PSBT, to query both, and the first + // PSBT must have no associated timestamp. + maybe_apply_migration(&db_path).unwrap(); + maybe_apply_migration(&db_path).unwrap(); // Migrating twice will be a no-op. + let db = SqliteDb::new(db_path, None, &secp).unwrap(); + { + let mut conn = db.connection().unwrap(); + conn.store_spend(&second_psbt); + let db_spends = conn.list_spend(); + let first_spend = db_spends + .iter() + .find(|db_spend| db_spend.psbt == first_psbt) + .unwrap(); + assert!(first_spend.updated_at.is_none()); + // TODO: update once we update store_spend() to take a timestamp. + let second_spend = db_spends + .iter() + .find(|db_spend| db_spend.psbt == second_psbt) + .unwrap(); + assert!(second_spend.updated_at.is_none()); + } + + fs::remove_dir_all(tmp_dir).unwrap(); + } } diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index b808812f..db94a114 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -1,4 +1,4 @@ -use crate::database::sqlite::{schema::SCHEMA, FreshDbOptions, SqliteDbError, DB_VERSION}; +use crate::database::sqlite::{FreshDbOptions, SqliteDbError, DB_VERSION}; use std::{convert::TryInto, fs, path, time}; @@ -79,6 +79,7 @@ pub fn create_db_file(db_path: &path::Path) -> Result<(), std::io::Error> { }; } +/// Create a fresh Liana database with the given schema. pub fn create_fresh_db( db_path: &path::Path, options: FreshDbOptions, @@ -113,10 +114,10 @@ pub fn create_fresh_db( let mut conn = rusqlite::Connection::open(db_path)?; db_exec(&mut conn, |tx| { - tx.execute_batch(SCHEMA)?; + tx.execute_batch(options.schema)?; tx.execute( "INSERT INTO version (version) VALUES (?1)", - rusqlite::params![DB_VERSION], + rusqlite::params![options.version], )?; tx.execute( "INSERT INTO tip (network, blockheight, blockhash) VALUES (?1, NULL, NULL)", @@ -157,10 +158,7 @@ fn migrate_v0_to_v1(conn: &mut rusqlite::Connection) -> Result<(), SqliteDbError "ALTER TABLE spend_transactions ADD COLUMN updated_at", rusqlite::params![], )?; - tx.execute( - "UPDATE version SET version = 1", - rusqlite::params![], - )?; + tx.execute("UPDATE version SET version = 1", rusqlite::params![])?; Ok(()) })?; diff --git a/src/lib.rs b/src/lib.rs index 401d2606..411a338e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -184,10 +184,10 @@ fn setup_sqlite( .iter() .collect(); let options = if fresh_data_dir { - Some(FreshDbOptions { - bitcoind_network: config.bitcoin_config.network, - main_descriptor: config.main_descriptor.clone(), - }) + Some(FreshDbOptions::new( + config.bitcoin_config.network, + config.main_descriptor.clone(), + )) } else { None }; From 104c6e1a093238045cad0c149f61f424ac0a9dc7 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 29 Mar 2023 16:28:25 +0200 Subject: [PATCH 3/4] commands: add an 'updated_at' field to listspendtxs entries --- doc/API.md | 1 + src/commands/mod.rs | 3 ++- src/database/mod.rs | 8 ++++---- src/database/sqlite/mod.rs | 11 +++++------ src/database/sqlite/utils.rs | 18 +++++++++--------- src/testutils.rs | 14 ++++++++++---- tests/test_rpc.py | 3 +++ 7 files changed, 34 insertions(+), 24 deletions(-) diff --git a/doc/API.md b/doc/API.md index bcb93b58..bc8f037e 100644 --- a/doc/API.md +++ b/doc/API.md @@ -174,6 +174,7 @@ This command does not take any parameter for now. | Field | Type | Description | | -------------- | ----------------- | ----------------------------------------------------------------------- | | `psbt` | string | Base64-encoded PSBT of the Spend transaction. | +| `updated_at` | int or null | UNIX timestamp of the last time this PSBT was updated. | ### `delspendtx` diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 1f54f900..73b3f11d 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -563,7 +563,7 @@ impl DaemonControl { let spend_txs = db_conn .list_spend() .into_iter() - .map(|psbt| ListSpendEntry { psbt }) + .map(|(psbt, updated_at)| ListSpendEntry { psbt, updated_at }) .collect(); ListSpendResult { spend_txs } } @@ -840,6 +840,7 @@ pub struct CreateSpendResult { pub struct ListSpendEntry { #[serde(serialize_with = "ser_base64", deserialize_with = "deser_base64")] pub psbt: Psbt, + pub updated_at: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/database/mod.rs b/src/database/mod.rs index 857041c1..699cd5d8 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -112,8 +112,8 @@ pub trait DatabaseConnection { /// Insert a new Spend transaction or replace an existing one. fn store_spend(&mut self, psbt: &Psbt); - /// List all existing Spend transactions. - fn list_spend(&mut self) -> Vec; + /// List all existing Spend transactions, along with an optional last update timestamp. + fn list_spend(&mut self) -> Vec<(Psbt, Option)>; /// Delete a Spend transaction from database. fn delete_spend(&mut self, txid: &bitcoin::Txid); @@ -241,10 +241,10 @@ impl DatabaseConnection for SqliteConn { self.store_spend(psbt) } - fn list_spend(&mut self) -> Vec { + fn list_spend(&mut self) -> Vec<(Psbt, Option)> { self.list_spend() .into_iter() - .map(|db_spend| db_spend.psbt) + .map(|db_spend| (db_spend.psbt, db_spend.updated_at)) .collect() } diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index ed81983d..c59a79d8 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -15,8 +15,8 @@ use crate::{ sqlite::{ schema::{DbAddress, DbCoin, DbSpendTransaction, DbTip, DbWallet, SCHEMA}, utils::{ - create_fresh_db, db_exec, db_query, db_tx_query, db_version, maybe_apply_migration, - LOOK_AHEAD_LIMIT, + create_fresh_db, curr_timestamp, db_exec, db_query, db_tx_query, db_version, + maybe_apply_migration, LOOK_AHEAD_LIMIT, }, }, Coin, CoinType, @@ -500,9 +500,9 @@ impl SqliteConn { db_exec(&mut self.conn, |db_tx| { db_tx.execute( - "INSERT into spend_transactions (psbt, txid) VALUES (?1, ?2) \ + "INSERT into spend_transactions (psbt, txid, updated_at) VALUES (?1, ?2, ?3) \ ON CONFLICT DO UPDATE SET psbt=excluded.psbt", - rusqlite::params![psbt, txid], + rusqlite::params![psbt, txid, curr_timestamp()], )?; Ok(()) }) @@ -1463,12 +1463,11 @@ CREATE TABLE spend_transactions ( .find(|db_spend| db_spend.psbt == first_psbt) .unwrap(); assert!(first_spend.updated_at.is_none()); - // TODO: update once we update store_spend() to take a timestamp. let second_spend = db_spends .iter() .find(|db_spend| db_spend.psbt == second_psbt) .unwrap(); - assert!(second_spend.updated_at.is_none()); + assert!(second_spend.updated_at.is_some()); } fs::remove_dir_all(tmp_dir).unwrap(); diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index db94a114..073fbe7f 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -50,11 +50,14 @@ where .collect::>>() } -// Sqlite supports up to i64, thus rusqlite prevents us from inserting u64's. -// We use this to panic rather than inserting a truncated integer into the database (as we'd have -// done by using `n as u32`). -fn timestamp_to_u32(n: u64) -> u32 { - n.try_into() +/// The current time as the number of seconds since the UNIX epoch, truncated to u32 since SQLite +/// only supports i64 integers. +pub fn curr_timestamp() -> u32 { + time::SystemTime::now() + .duration_since(time::UNIX_EPOCH) + .expect("System clock went backward the epoch?") + .as_secs() + .try_into() .expect("Is this the year 2106 yet? Misconfigured system clock.") } @@ -87,10 +90,7 @@ pub fn create_fresh_db( ) -> Result<(), SqliteDbError> { create_db_file(db_path)?; - let timestamp = time::SystemTime::now() - .duration_since(time::UNIX_EPOCH) - .map(|dur| timestamp_to_u32(dur.as_secs())) - .expect("System clock went backward the epoch?"); + let timestamp = curr_timestamp(); // Fill the initial addresses. On a fresh database, the deposit_derivation_index is // necessarily 0. diff --git a/src/testutils.rs b/src/testutils.rs index d91f222d..ee5b0e3b 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -120,7 +120,7 @@ struct DummyDbState { change_index: bip32::ChildNumber, curr_tip: Option, coins: HashMap, - spend_txs: HashMap, + spend_txs: HashMap)>, } pub struct DummyDatabase { @@ -293,14 +293,20 @@ impl DatabaseConnection for DummyDatabase { .write() .unwrap() .spend_txs - .insert(txid, psbt.clone()); + .insert(txid, (psbt.clone(), None)); } fn spend_tx(&mut self, txid: &bitcoin::Txid) -> Option { - self.db.read().unwrap().spend_txs.get(txid).cloned() + self.db + .read() + .unwrap() + .spend_txs + .get(txid) + .cloned() + .map(|x| x.0) } - fn list_spend(&mut self) -> Vec { + fn list_spend(&mut self) -> Vec<(Psbt, Option)> { self.db .read() .unwrap() diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 55c76858..c8f5fb63 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -159,6 +159,7 @@ def test_list_spend(lianad, bitcoind): assert "psbt" in res_b # Store them both in DB. + time_before_update = int(time.time()) assert len(lianad.rpc.listspendtxs()["spend_txs"]) == 0 lianad.rpc.updatespend(res["psbt"]) lianad.rpc.updatespend(res_b["psbt"]) @@ -168,7 +169,9 @@ def test_list_spend(lianad, bitcoind): list_res = lianad.rpc.listspendtxs()["spend_txs"] assert len(list_res) == 2 first_psbt = next(entry for entry in list_res if entry["psbt"] == res["psbt"]) + assert time_before_update <= first_psbt["updated_at"] <= int(time.time()) second_psbt = next(entry for entry in list_res if entry["psbt"] == res_b["psbt"]) + assert time_before_update <= second_psbt["updated_at"] <= int(time.time()) # If we delete the first one, we'll get only the second one. first_psbt = PSBT.from_base64(res["psbt"]) From f262ca2d1ca864e37704ab2a35d9426f85a31c6c Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 29 Mar 2023 16:32:48 +0200 Subject: [PATCH 4/4] tests: reduce the number of workers for the executor We don't use the executor much anyways. --- tests/test_framework/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_framework/utils.py b/tests/test_framework/utils.py index 75862282..d57c329c 100644 --- a/tests/test_framework/utils.py +++ b/tests/test_framework/utils.py @@ -12,7 +12,7 @@ from io import BytesIO from .serializations import CTransaction, PSBT TIMEOUT = int(os.getenv("TIMEOUT", 20)) -EXECUTOR_WORKERS = int(os.getenv("EXECUTOR_WORKERS", 20)) +EXECUTOR_WORKERS = int(os.getenv("EXECUTOR_WORKERS", 5)) VERBOSE = os.getenv("VERBOSE", "0") == "1" LOG_LEVEL = os.getenv("LOG_LEVEL", "debug") assert LOG_LEVEL in ["trace", "debug", "info", "warn", "error"]