From d9f905a19a5c6076683bbe7714ecb86fbafe0555 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 24 Oct 2022 12:01:06 +0200 Subject: [PATCH] db: track the next unused derivation index for change, too --- src/commands/mod.rs | 4 +- src/database/mod.rs | 22 +++++++--- src/database/sqlite/mod.rs | 82 +++++++++++++++++++++++++---------- src/database/sqlite/schema.rs | 7 ++- src/database/sqlite/utils.rs | 41 +++++++++++++++--- src/testutils.rs | 25 ++++++++--- 6 files changed, 139 insertions(+), 42 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 92d5e19d..6f0e9a9a 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -200,9 +200,9 @@ impl DaemonControl { /// whether it was actually used. pub fn get_new_address(&self) -> GetAddressResult { let mut db_conn = self.db.connection(); - let index = db_conn.derivation_index(); + let index = db_conn.receive_index(); // TODO: should we wrap around instead of failing? - db_conn.increment_derivation_index(&self.secp); + db_conn.increment_receive_index(&self.secp); let address = self .derived_desc(index) .address(self.config.bitcoin_config.network); diff --git a/src/database/mod.rs b/src/database/mod.rs index 6097248e..20795bb1 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -45,9 +45,13 @@ pub trait DatabaseConnection { /// Update our best chain seen. fn update_tip(&mut self, tip: &BlockChainTip); - fn derivation_index(&mut self) -> bip32::ChildNumber; + fn receive_index(&mut self) -> bip32::ChildNumber; - fn increment_derivation_index(&mut self, secp: &secp256k1::Secp256k1); + fn change_index(&mut self) -> bip32::ChildNumber; + + fn increment_receive_index(&mut self, secp: &secp256k1::Secp256k1); + + fn increment_change_index(&mut self, secp: &secp256k1::Secp256k1); /// Get the derivation index for this address, as well as whether this address is change. fn derivation_index_by_address( @@ -114,12 +118,20 @@ impl DatabaseConnection for SqliteConn { self.update_tip(tip) } - fn derivation_index(&mut self) -> bip32::ChildNumber { + fn receive_index(&mut self) -> bip32::ChildNumber { self.db_wallet().deposit_derivation_index } - fn increment_derivation_index(&mut self, secp: &secp256k1::Secp256k1) { - self.increment_derivation_index(secp) + fn change_index(&mut self) -> bip32::ChildNumber { + self.db_wallet().change_derivation_index + } + + fn increment_receive_index(&mut self, secp: &secp256k1::Secp256k1) { + self.increment_deposit_index(secp) + } + + fn increment_change_index(&mut self, secp: &secp256k1::Secp256k1) { + self.increment_change_index(secp) } fn coins(&mut self) -> HashMap { diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index d6b14d30..dc15827b 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -14,7 +14,7 @@ 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, populate_address_mapping}, }, Coin, }, @@ -210,10 +210,7 @@ impl SqliteConn { .expect("Database must be available") } - pub fn increment_derivation_index( - &mut self, - secp: &secp256k1::Secp256k1, - ) { + pub fn increment_deposit_index(&mut self, secp: &secp256k1::Secp256k1) { let network = self.db_tip().network; db_exec(&mut self.conn, |db_tx| { @@ -235,24 +232,41 @@ impl SqliteConn { rusqlite::params![next_index], )?; - // Update the address to derivation index mapping. - // TODO: have this as a helper in descriptors.rs - let next_la_index = next_index + LOOK_AHEAD_LIMIT - 1; - let next_receive_address = db_wallet - .main_descriptor - .receive_descriptor() - .derive(next_la_index.into(), secp) - .address(network); - let next_change_address = db_wallet - .main_descriptor - .change_descriptor() - .derive(next_la_index.into(), secp) - .address(network); + if next_index > db_wallet.change_derivation_index.into() { + populate_address_mapping(db_tx, &db_wallet, next_index, network, secp)?; + } + + Ok(()) + }) + .expect("Database must be available") + } + + pub fn increment_change_index(&mut self, secp: &secp256k1::Secp256k1) { + let network = self.db_tip().network; + + db_exec(&mut self.conn, |db_tx| { + let db_wallet: DbWallet = + db_tx_query(db_tx, "SELECT * FROM wallets", rusqlite::params![], |row| { + row.try_into() + }) + .expect("Db must not fail") + .pop() + .expect("There is always a row in the wallet table"); + let next_index: u32 = db_wallet + .change_derivation_index + .increment() + .expect("Must not get in hardened territory") + .into(); + // NOTE: should be updated if we ever have multi-wallet support db_tx.execute( - "INSERT INTO addresses (receive_address, change_address, derivation_index) VALUES (?1, ?2, ?3)", - rusqlite::params![next_receive_address.to_string(), next_change_address.to_string(), next_la_index], + "UPDATE wallets SET change_derivation_index = (?1)", + rusqlite::params![next_index], )?; + if next_index > db_wallet.deposit_derivation_index.into() { + populate_address_mapping(db_tx, &db_wallet, next_index, network, secp)?; + } + Ok(()) }) .expect("Database must be available") @@ -756,11 +770,11 @@ mod tests { assert!(conn.db_address(&addr).is_none()); // But if we increment the deposit derivation index, the 200th one will be there. - conn.increment_derivation_index(&secp); + conn.increment_deposit_index(&secp); let db_addr = conn.db_address(&addr).unwrap(); assert_eq!(db_addr.derivation_index, 200.into()); - // Same for the change descriptor. + // It will also be there for the change descriptor. let addr = options .main_descriptor .change_descriptor() @@ -768,6 +782,30 @@ mod tests { .address(options.bitcoind_network); let db_addr = conn.db_address(&addr).unwrap(); assert_eq!(db_addr.derivation_index, 200.into()); + + // But not for the 201th. + let addr = options + .main_descriptor + .change_descriptor() + .derive(201.into(), &secp) + .address(options.bitcoind_network); + assert!(conn.db_address(&addr).is_none()); + + // If we increment the *change* derivation index to 1, it will still not be there. + conn.increment_change_index(&secp); + assert!(conn.db_address(&addr).is_none()); + + // But doing it once again it will be there for both change and receive. + conn.increment_change_index(&secp); + let db_addr = conn.db_address(&addr).unwrap(); + assert_eq!(db_addr.derivation_index, 201.into()); + let addr = options + .main_descriptor + .receive_descriptor() + .derive(201.into(), &secp) + .address(options.bitcoind_network); + let db_addr = conn.db_address(&addr).unwrap(); + assert_eq!(db_addr.derivation_index, 201.into()); } fs::remove_dir_all(&tmp_dir).unwrap(); diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 1f729d41..40398df7 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -27,7 +27,8 @@ CREATE TABLE wallets ( id INTEGER PRIMARY KEY NOT NULL, timestamp INTEGER NOT NULL, main_descriptor TEXT NOT NULL, - deposit_derivation_index INTEGER NOT NULL + deposit_derivation_index INTEGER NOT NULL, + change_derivation_index INTEGER NOT NULL ); /* Our (U)TxOs. @@ -107,6 +108,7 @@ pub struct DbWallet { pub timestamp: u32, pub main_descriptor: MultipathDescriptor, pub deposit_derivation_index: bip32::ChildNumber, + pub change_derivation_index: bip32::ChildNumber, } impl TryFrom<&rusqlite::Row<'_>> for DbWallet { @@ -122,12 +124,15 @@ impl TryFrom<&rusqlite::Row<'_>> for DbWallet { let der_idx: u32 = row.get(3)?; let deposit_derivation_index = bip32::ChildNumber::from(der_idx); + let der_idx: u32 = row.get(4)?; + let change_derivation_index = bip32::ChildNumber::from(der_idx); Ok(DbWallet { id, timestamp, main_descriptor, deposit_derivation_index, + change_derivation_index, }) } } diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index 82d09d84..c091d188 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -1,8 +1,11 @@ -use crate::database::sqlite::{schema::SCHEMA, FreshDbOptions, SqliteDbError, DB_VERSION}; +use crate::database::sqlite::{ + schema::{DbWallet, SCHEMA}, + FreshDbOptions, SqliteDbError, DB_VERSION, +}; use std::{convert::TryInto, fs, path, time}; -use miniscript::bitcoin::secp256k1; +use miniscript::bitcoin::{self, secp256k1}; pub const LOOK_AHEAD_LIMIT: u32 = 200; @@ -123,9 +126,9 @@ pub fn create_fresh_db( rusqlite::params![options.bitcoind_network.to_string()], )?; tx.execute( - "INSERT INTO wallets (timestamp, main_descriptor, deposit_derivation_index) \ - VALUES (?1, ?2, ?3)", - rusqlite::params![timestamp, options.main_descriptor.to_string(), 0,], + "INSERT INTO wallets (timestamp, main_descriptor, deposit_derivation_index, change_derivation_index) \ + VALUES (?1, ?2, ?3, ?4)", + rusqlite::params![timestamp, options.main_descriptor.to_string(), 0, 0], )?; tx.execute_batch(&query)?; @@ -134,3 +137,31 @@ pub fn create_fresh_db( Ok(()) } + +/// Insert the deposit and change addresses for this index in the address->index mapping table +pub fn populate_address_mapping( + db_tx: &rusqlite::Transaction, + db_wallet: &DbWallet, + next_index: u32, + network: bitcoin::Network, + secp: &secp256k1::Secp256k1, +) -> rusqlite::Result<()> { + // Update the address to derivation index mapping. + let next_la_index = next_index + LOOK_AHEAD_LIMIT - 1; + let next_receive_address = db_wallet + .main_descriptor + .receive_descriptor() + .derive(next_la_index.into(), secp) + .address(network); + let next_change_address = db_wallet + .main_descriptor + .change_descriptor() + .derive(next_la_index.into(), secp) + .address(network); + db_tx.execute( + "INSERT INTO addresses (receive_address, change_address, derivation_index) VALUES (?1, ?2, ?3)", + rusqlite::params![next_receive_address.to_string(), next_change_address.to_string(), next_la_index], + )?; + + Ok(()) +} diff --git a/src/testutils.rs b/src/testutils.rs index ea13d1d2..eff9da06 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -77,7 +77,8 @@ impl BitcoinInterface for DummyBitcoind { } pub struct DummyDb { - curr_index: bip32::ChildNumber, + deposit_index: bip32::ChildNumber, + change_index: bip32::ChildNumber, curr_tip: Option, coins: HashMap, spend_txs: HashMap, @@ -86,7 +87,8 @@ pub struct DummyDb { impl DummyDb { pub fn new() -> DummyDb { DummyDb { - curr_index: 0.into(), + deposit_index: 0.into(), + change_index: 0.into(), curr_tip: None, coins: HashMap::new(), spend_txs: HashMap::new(), @@ -123,13 +125,22 @@ impl DatabaseConnection for DummyDbConn { self.db.write().unwrap().curr_tip = Some(*tip); } - fn derivation_index(&mut self) -> bip32::ChildNumber { - self.db.read().unwrap().curr_index + fn receive_index(&mut self) -> bip32::ChildNumber { + self.db.read().unwrap().deposit_index } - fn increment_derivation_index(&mut self, _: &secp256k1::Secp256k1) { - let next_index = self.db.write().unwrap().curr_index.increment().unwrap(); - self.db.write().unwrap().curr_index = next_index; + fn change_index(&mut self) -> bip32::ChildNumber { + self.db.read().unwrap().deposit_index + } + + fn increment_receive_index(&mut self, _: &secp256k1::Secp256k1) { + let next_index = self.db.write().unwrap().deposit_index.increment().unwrap(); + self.db.write().unwrap().deposit_index = next_index; + } + + fn increment_change_index(&mut self, _: &secp256k1::Secp256k1) { + let next_index = self.db.write().unwrap().change_index.increment().unwrap(); + self.db.write().unwrap().change_index = next_index; } fn coins(&mut self) -> HashMap {