diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 05e60e53..7fbab980 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -219,8 +219,10 @@ impl DaemonControl { pub fn get_new_address(&self) -> GetAddressResult { let mut db_conn = self.db.connection(); let index = db_conn.receive_index(); - // TODO: should we wrap around instead of failing? - db_conn.increment_receive_index(&self.secp); + let new_index = index + .increment() + .expect("Can't get into hardened territory"); + db_conn.set_receive_index(new_index, &self.secp); let address = self .config .main_descriptor @@ -362,12 +364,17 @@ impl DaemonControl { // an added output* (for the change). if nochange_feerate_vb > feerate_vb { // Get the change address to create a dummy change txo. + let change_index = db_conn.change_index(); let change_desc = self .config .main_descriptor .change_descriptor() - .derive(db_conn.change_index(), &self.secp); - db_conn.increment_change_index(&self.secp); + .derive(change_index, &self.secp); + // Don't forget to update our next change index! + let next_index = change_index + .increment() + .expect("Must not get into hardened territory"); + db_conn.set_change_index(next_index, &self.secp); let mut change_txo = bitcoin::TxOut { value: std::u64::MAX, script_pubkey: change_desc.script_pubkey(), diff --git a/src/database/mod.rs b/src/database/mod.rs index 263f3442..095fe40f 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -45,13 +45,25 @@ pub trait DatabaseConnection { /// Update our best chain seen. fn update_tip(&mut self, tip: &BlockChainTip); + /// Get the derivation index for the next receiving address fn receive_index(&mut self) -> bip32::ChildNumber; + /// Set the derivation index for the next receiving address + fn set_receive_index( + &mut self, + index: bip32::ChildNumber, + secp: &secp256k1::Secp256k1, + ); + + /// Get the derivation index for the next change address 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); + /// Set the derivation index for the next change address + fn set_change_index( + &mut self, + index: bip32::ChildNumber, + secp: &secp256k1::Secp256k1, + ); /// Get the timestamp at which to start rescaning from, if any. fn rescan_timestamp(&mut self) -> Option; @@ -131,16 +143,24 @@ impl DatabaseConnection for SqliteConn { self.db_wallet().deposit_derivation_index } + fn set_receive_index( + &mut self, + index: bip32::ChildNumber, + secp: &secp256k1::Secp256k1, + ) { + self.set_derivation_index(index, false, 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 set_change_index( + &mut self, + index: bip32::ChildNumber, + secp: &secp256k1::Secp256k1, + ) { + self.set_derivation_index(index, true, secp) } fn rescan_timestamp(&mut self) -> Option { diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index c942eef5..b11d8f3d 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, populate_address_mapping}, + utils::{create_fresh_db, db_exec, db_query, db_tx_query, LOOK_AHEAD_LIMIT}, }, Coin, }, @@ -24,8 +24,11 @@ use crate::{ use std::{cmp, convert::TryInto, fmt, io, path}; use miniscript::bitcoin::{ - self, consensus::encode, hashes::hex::ToHex, secp256k1, - util::psbt::PartiallySignedTransaction as Psbt, + self, + consensus::encode, + hashes::hex::ToHex, + secp256k1, + util::{bip32, psbt::PartiallySignedTransaction as Psbt}, }; const DB_VERSION: i64 = 0; @@ -210,61 +213,61 @@ impl SqliteConn { .expect("Database must be available") } - pub fn increment_deposit_index(&mut self, secp: &secp256k1::Secp256k1) { + /// Set the derivation index for receiving or change addresses. + /// + /// This will populate the address->deriv_index mapping with all the new entries between the + /// former and new gap limit indexes. + pub fn set_derivation_index( + &mut self, + index: bip32::ChildNumber, + change: bool, + 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 - .deposit_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( - "UPDATE wallets SET deposit_derivation_index = (?1)", - rusqlite::params![next_index], - )?; - if next_index > db_wallet.change_derivation_index.into() { - populate_address_mapping(db_tx, &db_wallet, next_index, network, secp)?; + // First of all set the derivation index + let index_u32: u32 = index.into(); + if change { + db_tx.execute( + "UPDATE wallets SET change_derivation_index = (?1)", + rusqlite::params![index_u32], + )?; + } else { + db_tx.execute( + "UPDATE wallets SET deposit_derivation_index = (?1)", + rusqlite::params![index_u32], + )?; } - Ok(()) - }) - .expect("Database must be available") - } + // Now if this new index is higher than the highest of our current derivation indexes, + // populate the addresses mapping for derivation indexes between our previous "gap + // limit index" and the new one. + let curr_highest_index = cmp::max( + db_wallet.deposit_derivation_index, + db_wallet.change_derivation_index, + ).into(); + if index_u32 > curr_highest_index { + let receive_desc = db_wallet.main_descriptor.receive_descriptor(); + let change_desc = db_wallet.main_descriptor.change_descriptor(); - pub fn increment_change_index(&mut self, secp: &secp256k1::Secp256k1) { - let network = self.db_tip().network; + for index in curr_highest_index + 1..=index_u32 { + let la_index = index + LOOK_AHEAD_LIMIT - 1; + let receive_addr = receive_desc.derive(la_index.into(), secp).address(network); + let change_addr = change_desc.derive(la_index.into(), secp).address(network); + db_tx.execute( + "INSERT INTO addresses (receive_address, change_address, derivation_index) VALUES (?1, ?2, ?3)", + rusqlite::params![receive_addr.to_string(), change_addr.to_string(), la_index], + )?; + } - 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( - "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(()) @@ -807,7 +810,7 @@ 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_deposit_index(&secp); + conn.set_derivation_index(1.into(), false, &secp); let db_addr = conn.db_address(&addr).unwrap(); assert_eq!(db_addr.derivation_index, 200.into()); @@ -829,11 +832,11 @@ mod tests { 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); + conn.set_derivation_index(1.into(), true, &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); + // But incrementing it once again it will be there for both change and receive. + conn.set_derivation_index(2.into(), true, &secp); let db_addr = conn.db_address(&addr).unwrap(); assert_eq!(db_addr.derivation_index, 201.into()); let addr = options @@ -843,6 +846,19 @@ mod tests { .address(options.bitcoind_network); let db_addr = conn.db_address(&addr).unwrap(); assert_eq!(db_addr.derivation_index, 201.into()); + + // Now setting it to a much higher will fill all the addresses within the gap + conn.set_derivation_index(52.into(), true, &secp); + for index in 2..52 { + let look_ahead_index = 200 + index; + let addr = options + .main_descriptor + .receive_descriptor() + .derive(look_ahead_index.into(), &secp) + .address(options.bitcoind_network); + let db_addr = conn.db_address(&addr).unwrap(); + assert_eq!(db_addr.derivation_index, look_ahead_index.into()); + } } fs::remove_dir_all(&tmp_dir).unwrap(); diff --git a/src/database/sqlite/utils.rs b/src/database/sqlite/utils.rs index c091d188..87b6555d 100644 --- a/src/database/sqlite/utils.rs +++ b/src/database/sqlite/utils.rs @@ -1,11 +1,8 @@ -use crate::database::sqlite::{ - schema::{DbWallet, SCHEMA}, - FreshDbOptions, SqliteDbError, DB_VERSION, -}; +use crate::database::sqlite::{schema::SCHEMA, FreshDbOptions, SqliteDbError, DB_VERSION}; use std::{convert::TryInto, fs, path, time}; -use miniscript::bitcoin::{self, secp256k1}; +use miniscript::bitcoin::secp256k1; pub const LOOK_AHEAD_LIMIT: u32 = 200; @@ -137,31 +134,3 @@ 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 7768cae9..51933dc1 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -145,18 +145,24 @@ impl DatabaseConnection for DummyDbConn { self.db.read().unwrap().deposit_index } + fn set_receive_index( + &mut self, + index: bip32::ChildNumber, + _: &secp256k1::Secp256k1, + ) { + self.db.write().unwrap().deposit_index = 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 set_change_index( + &mut self, + index: bip32::ChildNumber, + _: &secp256k1::Secp256k1, + ) { + self.db.write().unwrap().change_index = index; } fn coins(&mut self) -> HashMap {