Store and use last scanned PMMR height in check_repair (#246)

* store last scanned PMMR check index

* rustfmt

* fix issue where account names will be overwritten on check_repair

* rustfmt

* attempts to include check_repair scan as part of normal update

* rustfmt

* fix error on restore due to incorrect parent key id being set

* addition of calls to heigt_range_to_pmmr_indices traits and implementations

* rustfmt

* get_chain_height -> get_chain_tip

* rustfmt

* retrieve height+hash from node, modify check_repair to use block heights

* rustfmt

* fixes from live testing

* rustfmt

* test cleanup and change dependencies back to grin master

* rustfmt

api/src/owner.rs

@@ -1232,7 +1232,13 @@ where
 		let w = w_lock.lc_provider()?.wallet_inst()?;
 		// Test keychain mask, to keep API consistent
 		let _ = w.keychain(keychain_mask)?;
-		owner::node_height(&mut **w, keychain_mask)
+		let mut res = owner::node_height(&mut **w, keychain_mask)?;
+		if self.doctest_mode {
+			// return a consistent hash for doctest
+			res.header_hash =
+				"d4b3d3c40695afd8c7760f8fc423565f7d41310b7a4e1c4a4a7950a66f16240d".to_owned();
+		}
+		Ok(res)
 	}
 
 	// LIFECYCLE FUNCTIONS

api/src/owner_rpc.rs

@@ -1239,6 +1239,7 @@ pub trait OwnerRpc: Sync + Send {
 		"jsonrpc": "2.0",
 		"result": {
 			"Ok": {
+				"header_hash": "d4b3d3c40695afd8c7760f8fc423565f7d41310b7a4e1c4a4a7950a66f16240d",
 				"height": "5",
 				"updated_from_node": true
 			}

api/src/owner_rpc_s.rs

@@ -1304,6 +1304,7 @@ pub trait OwnerRpcS {
 		"jsonrpc": "2.0",
 		"result": {
 			"Ok": {
+				"header_hash": "d4b3d3c40695afd8c7760f8fc423565f7d41310b7a4e1c4a4a7950a66f16240d",
 				"height": "5",
 				"updated_from_node": true
 			}

controller/tests/check.rs

@@ -25,7 +25,7 @@ use self::core::global;
 use grin_wallet_libwallet as libwallet;
 use impls::test_framework::{self, LocalWalletClient};
 use impls::{PathToSlate, SlatePutter as _};
-use libwallet::InitTxArgs;
+use libwallet::{InitTxArgs, NodeClient};
 use std::thread;
 use std::time::Duration;
 use util::ZeroingString;
@@ -90,10 +90,10 @@ fn check_repair_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
 
 	// add some accounts
 	wallet::controller::owner_single_use(wallet1.clone(), mask1, |api, m| {
-		api.create_account_path(m, "account_1")?;
+		api.create_account_path(m, "named_account_1")?;
 		api.create_account_path(m, "account_2")?;
 		api.create_account_path(m, "account_3")?;
-		api.set_active_account(m, "account_1")?;
+		api.set_active_account(m, "named_account_1")?;
 		Ok(())
 	})?;
 
@@ -165,6 +165,11 @@ fn check_repair_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
 		let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(m, true, 1)?;
 		assert!(wallet1_refreshed);
 		assert_eq!(wallet1_info.total, bh * reward);
+		// And check account names haven't been splatted
+		let accounts = api.accounts(m)?;
+		assert_eq!(accounts.len(), 4);
+		assert!(api.set_active_account(m, "account_1").is_err());
+		assert!(api.set_active_account(m, "named_account_1").is_ok());
 		Ok(())
 	})?;
 
@@ -190,7 +195,8 @@ fn check_repair_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
 
 	// check we're all locked
 	wallet::controller::owner_single_use(wallet1.clone(), mask1, |api, m| {
-		let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?;
+		let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(wallet1_refreshed);
 		assert!(wallet1_info.amount_currently_spendable == 0);
 		Ok(())
 	})?;
@@ -438,13 +444,15 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 	bh += cm as u64;
 
 	// confirm balances
+	// since info is now performing a partial check_repair, these should confirm
+	// as containing all outputs
 	let info = wallet_info!(wallet1.clone(), mask1)?;
-	assert_eq!(info.amount_currently_spendable, base_amount * 6);
-	assert_eq!(info.total, base_amount * 6);
+	assert_eq!(info.amount_currently_spendable, base_amount * 21);
+	assert_eq!(info.total, base_amount * 21);
 
 	let info = wallet_info!(wallet2.clone(), mask2)?;
-	assert_eq!(info.amount_currently_spendable, base_amount * 15);
-	assert_eq!(info.total, base_amount * 15);
+	assert_eq!(info.amount_currently_spendable, base_amount * 21);
+	assert_eq!(info.total, base_amount * 21);
 
 	// Now there should be outputs on the chain using the same
 	// seed + BIP32 path.
@@ -480,6 +488,8 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 
 	// 3) If I recover from seed and start using the wallet without restoring,
 	// check_repair should restore the older outputs
+	// update, again, since check_repair is run automatically, balances on both
+	// wallets should turn out the same
 	send_to_dest!(
 		miner.clone(),
 		miner_mask,
@@ -509,8 +519,8 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 	wallet::controller::owner_single_use(wallet4.clone(), mask4, |api, m| {
 		let info = wallet_info!(wallet4.clone(), m)?;
 		let outputs = api.retrieve_outputs(m, true, false, None)?.1;
-		assert_eq!(outputs.len(), 3);
-		assert_eq!(info.amount_currently_spendable, base_amount * 24);
+		assert_eq!(outputs.len(), 9);
+		assert_eq!(info.amount_currently_spendable, base_amount * 45);
 		Ok(())
 	})?;
 
@@ -564,8 +574,8 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 	wallet::controller::owner_single_use(wallet6.clone(), mask6, |api, m| {
 		let info = wallet_info!(wallet6.clone(), m)?;
 		let outputs = api.retrieve_outputs(m, true, false, None)?.1;
-		assert_eq!(outputs.len(), 3);
-		assert_eq!(info.amount_currently_spendable, base_amount * 33);
+		assert_eq!(outputs.len(), 12);
+		assert_eq!(info.amount_currently_spendable, base_amount * 78);
 		Ok(())
 	})?;
 
@@ -650,8 +660,8 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 		api.set_active_account(m, "default")?;
 		let info = wallet_info!(wallet7.clone(), m)?;
 		let outputs = api.retrieve_outputs(m, true, false, None)?.1;
-		assert_eq!(outputs.len(), 3);
-		assert_eq!(info.amount_currently_spendable, base_amount * 42);
+		assert_eq!(outputs.len(), 15);
+		assert_eq!(info.amount_currently_spendable, base_amount * 120);
 		Ok(())
 	})?;
 
@@ -706,8 +716,8 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 	wallet::controller::owner_single_use(wallet9.clone(), mask9, |api, m| {
 		let info = wallet_info!(wallet9.clone(), m)?;
 		let outputs = api.retrieve_outputs(m, true, false, None)?.1;
-		assert_eq!(outputs.len(), 3);
-		assert_eq!(info.amount_currently_spendable, base_amount * 15);
+		assert_eq!(outputs.len(), 6);
+		assert_eq!(info.amount_currently_spendable, base_amount * 21);
 		api.check_repair(m, true)?;
 		let info = wallet_info!(wallet9.clone(), m)?;
 		let outputs = api.retrieve_outputs(m, true, false, None)?.1;
@@ -745,9 +755,94 @@ fn two_wallets_one_seed_impl(test_dir: &'static str) -> Result<(), libwallet::Er
 	thread::sleep(Duration::from_millis(200));
 	Ok(())
 }
+
+// Testing output scanning functionality, easier here as the testing framework
+// is all here
+fn output_scanning_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
+	let mut wallet_proxy = create_wallet_proxy(test_dir);
+	let chain = wallet_proxy.chain.clone();
+	// Create a new wallet test client, and set its queues to communicate with the
+	// proxy
+	create_wallet_and_add!(
+		client1,
+		wallet1,
+		mask1_i,
+		test_dir,
+		"wallet1",
+		None,
+		&mut wallet_proxy,
+		false
+	);
+	let mask1 = (&mask1_i).as_ref();
+	thread::spawn(move || {
+		if let Err(e) = wallet_proxy.run() {
+			error!("Wallet Proxy error: {}", e);
+		}
+	});
+
+	// Do some mining
+	let bh = 20u64;
+	let _ =
+		test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, bh as usize, false);
+
+	// Now some chain scanning
+	{
+		// Entire range should be correct
+		let ranges = client1.height_range_to_pmmr_indices(1, None)?;
+		assert_eq!(ranges, (1, 38));
+		let outputs = client1.get_outputs_by_pmmr_index(ranges.0, Some(ranges.1), 1000)?;
+		assert_eq!(outputs.2.len(), 20);
+
+		// Basic range should be correct
+		let ranges = client1.height_range_to_pmmr_indices(1, Some(14))?;
+		assert_eq!(ranges, (1, 25));
+		let outputs = client1.get_outputs_by_pmmr_index(ranges.0, Some(ranges.1), 1000)?;
+		println!(
+			"Last Index: {}, Max: {}, Outputs.len: {}",
+			outputs.0,
+			outputs.1,
+			outputs.2.len()
+		);
+		assert_eq!(outputs.2.len(), 14);
+
+		// mid range
+		let ranges = client1.height_range_to_pmmr_indices(5, Some(14))?;
+		assert_eq!(ranges, (8, 25));
+		let outputs = client1.get_outputs_by_pmmr_index(ranges.0, Some(ranges.1), 1000)?;
+		println!(
+			"Last Index: {}, Max: {}, Outputs.len: {}",
+			outputs.0,
+			outputs.1,
+			outputs.2.len()
+		);
+		for o in outputs.2.clone() {
+			println!("height: {}, mmr_index: {}", o.3, o.4);
+		}
+		assert_eq!(outputs.2.len(), 10);
+
+		// end
+		let ranges = client1.height_range_to_pmmr_indices(5, None)?;
+		assert_eq!(ranges, (8, 38));
+		let outputs = client1.get_outputs_by_pmmr_index(ranges.0, Some(ranges.1), 1000)?;
+		println!(
+			"Last Index: {}, Max: {}, Outputs.len: {}",
+			outputs.0,
+			outputs.1,
+			outputs.2.len()
+		);
+		for o in outputs.2.clone() {
+			println!("height: {}, mmr_index: {}", o.3, o.4);
+		}
+		assert_eq!(outputs.2.len(), 16);
+	}
+
+	Ok(())
+}
+
 #[test]
 fn check_repair() {
 	let test_dir = "test_output/check_repair";
+	setup(test_dir);
 	if let Err(e) = check_repair_impl(test_dir) {
 		panic!("Libwallet Error: {} - {}", e, e.backtrace().unwrap());
 	}
@@ -763,3 +858,13 @@ fn two_wallets_one_seed() {
 	}
 	clean_output_dir(test_dir);
 }
+
+#[test]
+fn output_scanning() {
+	let test_dir = "test_output/output_scanning";
+	setup(test_dir);
+	if let Err(e) = output_scanning_impl(test_dir) {
+		panic!("Libwallet Error: {} - {}", e, e.backtrace().unwrap());
+	}
+	clean_output_dir(test_dir);
+}

impls/src/backends/lmdb.rs

@@ -33,8 +33,8 @@ use crate::core::core::Transaction;
 use crate::core::ser;
 use crate::libwallet::{check_repair, restore};
 use crate::libwallet::{
-	AcctPathMapping, Context, Error, ErrorKind, NodeClient, OutputData, TxLogEntry, WalletBackend,
-	WalletOutputBatch,
+	AcctPathMapping, Context, Error, ErrorKind, NodeClient, OutputData, ScannedBlockInfo,
+	TxLogEntry, WalletBackend, WalletOutputBatch,
 };
 use crate::util::secp::constants::SECRET_KEY_SIZE;
 use crate::util::secp::key::SecretKey;
@@ -53,6 +53,8 @@ const PRIVATE_TX_CONTEXT_PREFIX: u8 = 'p' as u8;
 const TX_LOG_ENTRY_PREFIX: u8 = 't' as u8;
 const TX_LOG_ID_PREFIX: u8 = 'i' as u8;
 const ACCOUNT_PATH_MAPPING_PREFIX: u8 = 'a' as u8;
+const LAST_SCANNED_BLOCK: u8 = 'l' as u8;
+const LAST_SCANNED_KEY: &str = "LAST_SCANNED_KEY";
 
 /// test to see if database files exist in the current directory. If so,
 /// use a DB backend for all operations
@@ -395,6 +397,18 @@ where
 		}))
 	}
 
+	fn current_child_index<'a>(&mut self, parent_key_id: &Identifier) -> Result<u32, Error> {
+		let index = {
+			let batch = self.db.batch()?;
+			let deriv_key = to_key(DERIV_PREFIX, &mut parent_key_id.to_bytes().to_vec());
+			match batch.get_ser(&deriv_key)? {
+				Some(idx) => idx,
+				None => 0,
+			}
+		};
+		Ok(index)
+	}
+
 	fn next_child<'a>(&mut self, keychain_mask: Option<&SecretKey>) -> Result<Identifier, Error> {
 		let parent_key_id = self.parent_key_id.clone();
 		let mut deriv_idx = {
@@ -428,18 +442,51 @@ where
 		Ok(last_confirmed_height)
 	}
 
-	fn restore(&mut self, keychain_mask: Option<&SecretKey>) -> Result<(), Error> {
-		restore(self, keychain_mask).context(ErrorKind::Restore)?;
-		Ok(())
+	fn last_scanned_block<'a>(&mut self) -> Result<ScannedBlockInfo, Error> {
+		let batch = self.db.batch()?;
+		let scanned_block_key = to_key(
+			LAST_SCANNED_BLOCK,
+			&mut LAST_SCANNED_KEY.as_bytes().to_vec(),
+		);
+		let last_scanned_block = match batch.get_ser(&scanned_block_key)? {
+			Some(b) => b,
+			None => ScannedBlockInfo {
+				height: 0,
+				hash: "".to_owned(),
+				start_pmmr_index: 0,
+				last_pmmr_index: 0,
+			},
+		};
+		Ok(last_scanned_block)
+	}
+
+	fn restore(
+		&mut self,
+		keychain_mask: Option<&SecretKey>,
+		to_height: u64,
+	) -> Result<Option<ScannedBlockInfo>, Error> {
+		let res = restore(self, keychain_mask, to_height).context(ErrorKind::Restore)?;
+		Ok(res)
 	}
 
 	fn check_repair(
 		&mut self,
 		keychain_mask: Option<&SecretKey>,
 		delete_unconfirmed: bool,
-	) -> Result<(), Error> {
-		check_repair(self, keychain_mask, delete_unconfirmed).context(ErrorKind::Restore)?;
-		Ok(())
+		start_height: u64,
+		end_height: u64,
+		status_fn: fn(&str),
+	) -> Result<ScannedBlockInfo, Error> {
+		let res = check_repair(
+			self,
+			keychain_mask,
+			delete_unconfirmed,
+			start_height,
+			end_height,
+			status_fn,
+		)
+		.context(ErrorKind::Restore)?;
+		Ok(res)
 	}
 }
 
@@ -558,6 +605,19 @@ where
 		Ok(())
 	}
 
+	fn save_last_scanned_block(&mut self, block_info: ScannedBlockInfo) -> Result<(), Error> {
+		let pmmr_index_key = to_key(
+			LAST_SCANNED_BLOCK,
+			&mut LAST_SCANNED_KEY.as_bytes().to_vec(),
+		);
+		self.db
+			.borrow()
+			.as_ref()
+			.unwrap()
+			.put_ser(&pmmr_index_key, &block_info)?;
+		Ok(())
+	}
+
 	fn save_child_index(&mut self, parent_id: &Identifier, child_n: u32) -> Result<(), Error> {
 		let deriv_key = to_key(DERIV_PREFIX, &mut parent_id.to_bytes().to_vec());
 		self.db