mixed small stuff (#934)

* documentation and spelling
* unused values in tests; just .unwrap()
* unneeded macro_use (router)
* unneeded macro_use and use-imports
* unused value -> .unwrap
* unused variables
* clarify that " is required (see 107ea3c4dd)
* first steps to dig into #933 test_p2p issues
* less simulnet build warnings

chain/src/chain.rs

@@ -33,9 +33,12 @@ use types::*;
 use util::secp::pedersen::RangeProof;
 use util::LOGGER;
 
-const MAX_ORPHAN_AGE_SECS: u64 = 300;
+/// Orphan pool size is limited by MAX_ORPHAN_SIZE
 pub const MAX_ORPHAN_SIZE: usize = 200;
 
+/// When evicting, very old orphans are evicted first
+const MAX_ORPHAN_AGE_SECS: u64 = 300;
+
 #[derive(Debug, Clone)]
 struct Orphan {
 	block: Block,
@@ -654,6 +657,7 @@ impl Chain {
 		self.head.lock().unwrap().clone().total_difficulty
 	}
 
+	/// Orphans pool size
 	pub fn orphans_len(&self) -> usize {
 		self.orphans.len()
 	}
@@ -751,10 +755,11 @@ impl Chain {
 			.map_err(|e| Error::StoreErr(e, "retrieve block pmmr metadata".to_owned()))
 	}
 
+	/// Rebuilds height index. Reachable as endpoint POST /chain/height-index
 	pub fn rebuild_header_by_height(&self) -> Result<(), Error> {
 		let head = self.head_header()?;
 		self.store
 			.build_by_height_index(&head, true)
-			.map_err(|e| Error::StoreErr(e, "rebuilf header by height index".to_owned()))
+			.map_err(|e| Error::StoreErr(e, "rebuild header by height index".to_owned()))
 	}
 }

chain/src/lib.rs

@@ -30,7 +30,6 @@ extern crate serde_derive;
 extern crate slog;
 extern crate time;
 
-#[macro_use]
 extern crate grin_core as core;
 extern crate grin_keychain as keychain;
 extern crate grin_store;

chain/tests/data_file_integrity.rs

@@ -27,7 +27,6 @@ use std::sync::Arc;
 use chain::Chain;
 use chain::types::*;
 use core::core::{Block, BlockHeader, Transaction};
-use core::core::hash::Hashed;
 use core::core::target::Difficulty;
 use core::{consensus, genesis};
 use core::global;
@@ -101,7 +100,7 @@ fn data_files() {
 				global::sizeshift() as u32,
 			).unwrap();
 
-			let prev_bhash = b.header.previous;
+			// let prev_bhash = b.header.previous;
 			let bhash = b.hash();
 			chain
 				.process_block(b.clone(), chain::Options::MINE)
@@ -113,7 +112,7 @@ fn data_files() {
 			let cur_pmmr_md = chain
 				.get_block_pmmr_file_metadata(&head.last_block_h)
 				.expect("block pmmr file data doesn't exist");
-			let pref_pmmr_md = chain
+			chain
 				.get_block_pmmr_file_metadata(&head.prev_block_h)
 				.expect("previous block pmmr file data doesn't exist");
 

grin.toml

@@ -44,9 +44,9 @@ chain_type = "Testnet2"
 
 #The chain validation mode, defines how often (if at all) we
 #want to run a full chain validation. Can be:
-#EveryBlock - run full chain validation when processing each block (except during sync)
-#Disabled - disable full chain validation (just run regular block validation)
-#chain_validation_mode=Disabled
+#"EveryBlock" - run full chain validation when processing each block (except during sync)
+#"Disabled" - disable full chain validation (just run regular block validation)
+#chain_validation_mode = "Disabled"
 
 #run the node in "full archive" mode (default is fast-sync, pruned node)
 #archive_mode = false

grin/tests/api.rs

@@ -38,6 +38,7 @@ use util::{init_test_logger, LOGGER};
 
 #[test]
 fn simple_server_wallet() {
+	info!(LOGGER, "starting simple_server_wallet");
 	let test_name_dir = "test_servers";
 	core::global::set_mining_mode(core::global::ChainTypes::AutomatedTesting);
 	framework::clean_all_output(test_name_dir);
@@ -145,6 +146,7 @@ fn simple_server_wallet() {
 /// Creates 2 servers and test P2P API
 #[test]
 fn test_p2p() {
+	info!(LOGGER, "starting test_p2p");
 	global::set_mining_mode(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "test_servers";

grin/tests/dandelion.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#[macro_use]
 extern crate router;
 #[macro_use]
 extern crate slog;

grin/tests/framework/mod.rs

@@ -270,7 +270,7 @@ impl LocalServerContainer {
 		self.wallet_config.data_file_dir = self.working_dir.clone();
 
 		let _ = fs::create_dir_all(self.wallet_config.clone().data_file_dir);
-		wallet::WalletSeed::init_file(&self.wallet_config);
+		wallet::WalletSeed::init_file(&self.wallet_config).unwrap();
 
 		let wallet_seed = wallet::WalletSeed::from_file(&self.wallet_config)
 			.expect("Failed to read wallet seed file.");
@@ -285,7 +285,7 @@ impl LocalServerContainer {
 
 	pub fn get_wallet_seed(config: &WalletConfig) -> wallet::WalletSeed {
 		let _ = fs::create_dir_all(config.clone().data_file_dir);
-		wallet::WalletSeed::init_file(config);
+		wallet::WalletSeed::init_file(config).unwrap();
 		let wallet_seed =
 			wallet::WalletSeed::from_file(config).expect("Failed to read wallet seed file.");
 		wallet_seed

grin/tests/simulnet.rs

@@ -23,8 +23,6 @@ extern crate grin_wallet as wallet;
 
 mod framework;
 
-use std::fs;
-use std::sync::Arc;
 use std::thread;
 use std::time;
 use std::default::Default;
@@ -195,7 +193,7 @@ fn simulate_block_propagation() {
 
 	// start mining
 	servers[0].start_miner(miner_config());
-	let original_height = servers[0].head().height;
+	let _original_height = servers[0].head().height;
 
 	// monitor for a change of head on a different server and check whether
 	// chain height has changed
@@ -233,6 +231,7 @@ fn simulate_full_sync() {
 	s1.start_miner(miner_config());
 	thread::sleep(time::Duration::from_secs(8));
 
+	#[ignore(unused_mut)] // mut needed?
 	let mut conf = config(1001, "grin-sync", 1000);
 	let s2 = grin::Server::new(conf).unwrap();
 	while s2.head().height < 4 {
@@ -265,7 +264,7 @@ fn simulate_fast_sync() {
 	while s2.head().height != s2.header_head().height || s2.head().height < 20 {
 		thread::sleep(time::Duration::from_millis(1000));
 	}
-	let h2 = s2.chain.get_header_by_height(1).unwrap();
+	let _h2 = s2.chain.get_header_by_height(1).unwrap();
 
 	s1.stop();
 	s2.stop();
@@ -296,8 +295,8 @@ fn simulate_fast_sync_double() {
 		s2.stop();
 	}
 	// locks files don't seem to be cleaned properly until process exit
-	std::fs::remove_file("target/tmp/grin-double-fast2/grin-sync-1001/chain/LOCK");
-	std::fs::remove_file("target/tmp/grin-double-fast2/grin-sync-1001/peers/LOCK");
+	std::fs::remove_file("target/tmp/grin-double-fast2/grin-sync-1001/chain/LOCK").unwrap();
+	std::fs::remove_file("target/tmp/grin-double-fast2/grin-sync-1001/peers/LOCK").unwrap();
 	thread::sleep(time::Duration::from_secs(20));
 
 	let mut conf = config(3001, "grin-double-fast2", 3000);

grin/tests/wallet.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#[macro_use]
 extern crate router;
 #[macro_use]
 extern crate slog;
@@ -133,7 +132,7 @@ fn basic_wallet_transactions() {
 		LOGGER,
 		"Sending many small transactions to recipient wallet"
 	);
-	for i in 0..10 {
+	for _i in 0..10 {
 		LocalServerContainer::send_amount_to(
 			&coinbase_wallet_config,
 			"1.00",

keychain/src/keychain.rs

@@ -717,7 +717,7 @@ mod test {
 
 			let blind = blinding_factor.secret_key(&keychain.secp()).unwrap();
 
-			keychain.aggsig_create_context(&tx_id, blind);
+			keychain.aggsig_create_context(&tx_id, blind).unwrap();
 			keychain.aggsig_get_public_keys(&tx_id)
 		};
 
@@ -729,7 +729,7 @@ mod test {
 			// let blind = blind_sum.secret_key(&keychain.secp())?;
 			let blind = keychain.derived_key(&key_id).unwrap();
 
-			keychain.aggsig_create_context(&tx_id, blind);
+			keychain.aggsig_create_context(&tx_id, blind).unwrap();
 			let (pub_excess, pub_nonce) = keychain.aggsig_get_public_keys(&tx_id);
 			keychain.aggsig_add_output(&tx_id, &key_id);
 
@@ -891,7 +891,7 @@ mod test {
 
 			let blind = blinding_factor.secret_key(&keychain.secp()).unwrap();
 
-			keychain.aggsig_create_context(&tx_id, blind);
+			keychain.aggsig_create_context(&tx_id, blind).unwrap();
 			keychain.aggsig_get_public_keys(&tx_id)
 		};
 
@@ -902,7 +902,7 @@ mod test {
 
 			let blind = keychain.derived_key(&key_id).unwrap();
 
-			keychain.aggsig_create_context(&tx_id, blind);
+			keychain.aggsig_create_context(&tx_id, blind).unwrap();
 			let (pub_excess, pub_nonce) = keychain.aggsig_get_public_keys(&tx_id);
 			keychain.aggsig_add_output(&tx_id, &key_id);
 

p2p/src/serv.rs

@@ -92,7 +92,7 @@ impl Server {
 				Ok((stream, peer_addr)) => {
 					if !self.check_banned(&stream) {
 						if let Err(e) = self.handle_new_peer(stream) {
-							debug!(
+							warn!(
 								LOGGER,
 								"Error accepting peer {}: {:?}",
 								peer_addr.to_string(),
@@ -126,7 +126,7 @@ impl Server {
 
 		if let Some(p) = self.peers.get_connected_peer(addr) {
 			// if we're already connected to the addr, just return the peer
-			debug!(LOGGER, "connect_peer: already connected {}", addr);
+			trace!(LOGGER, "connect_peer: already connected {}", addr);
 			return Ok(p);
 		}
 

wallet/src/sender.rs

@@ -122,7 +122,7 @@ pub fn issue_send_tx(
 		WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
 			info!(LOGGER, "cleaning up unused change output from wallet");
 			wallet_data.delete_output(&change_key);
-		});
+		}).unwrap();
 		panic!(
 			"dest formatted as {} but send -d expected stdout or http://IP:port",
 			dest