Forgotten testnet1 cherries (#475)

* Very quick peer banning endpoint, helps with #406
* Ping heights (#407)
* add height to ping/ping
* reformat output
* fix p2p test
* Fix orphan handling, not related to current head. Fixes #412
* Check before borrow, fixes #267
* Not finding an output commit in pos is an AlreadySpent
* Fix race condition, sending before conn is ready
* Explicit error for unknown pos of a forked block
* Remove config outdated tests. Fix #333
* Check ref and try before borrow, fix #400
* We do not want to sync with old peers anyway
* Hide cargo compiler warning for unused NoopAdapter and unused test code. Add TODOs

api/src/handlers.rs

@@ -257,6 +257,43 @@ impl Handler for PeersConnectedHandler {
 	}
 }
 
+/// Get details about a given peer and peer operations
+/// TODO GET /v1/peers/10.12.12.13
+/// POST /v1/peers/10.12.12.13/ban
+/// TODO POST /v1/peers/10.12.12.13/unban
+pub struct PeerHandler {
+	pub p2p_server: Arc<p2p::Server>,
+}
+
+impl Handler for PeerHandler {
+	fn handle(&self, req: &mut Request) -> IronResult<Response> {
+		let url = req.url.clone();
+		let mut path_elems = url.path();
+		if *path_elems.last().unwrap() == "" {
+			path_elems.pop();
+		}
+		match *path_elems.last().unwrap() {
+      "ban" => {
+        path_elems.pop();
+        if let Ok(addr) = path_elems.last().unwrap().parse() {
+          self.p2p_server.ban_peer(&addr);
+          Ok(Response::with((status::Ok, "")))
+        } else {
+          Ok(Response::with((status::BadRequest, "")))
+        }
+      }
+      "unban" => {
+        // TODO
+        Ok(Response::with((status::BadRequest, "")))
+      }
+      _ => {
+        // TODO peer details
+        Ok(Response::with((status::BadRequest, "")))
+      }
+    }
+	}
+}
+
 // Chain handler. Get the head details.
 // GET /v1/chain
 pub struct ChainHandler {
@@ -452,6 +489,9 @@ pub fn start_rest_apis<T>(
 		let peers_connected_handler = PeersConnectedHandler {
 			p2p_server: p2p_server.clone(),
 		};
+		let peer_handler = PeerHandler {
+			p2p_server: p2p_server.clone(),
+		};
 
 		let route_list = vec!(
 			"get /".to_string(),
@@ -478,6 +518,7 @@ pub fn start_rest_apis<T>(
 			pool_push: post "/pool/push" => pool_push_handler,
 			peers_all: get "/peers/all" => peers_all_handler,
 			peers_connected: get "/peers/connected" => peers_connected_handler,
+			peer: post "/peers/*" => peer_handler,
 		);
 
 		let mut apis = ApiServer::new("/v1".to_string());

chain/src/pipe.rs

@@ -156,9 +156,6 @@ fn check_known(bh: Hash, ctx: &mut BlockContext) -> Result<(), Error> {
 /// arranged by order of cost to have as little DoS surface as possible.
 /// TODO require only the block header (with length information)
 fn validate_header(header: &BlockHeader, ctx: &mut BlockContext) -> Result<(), Error> {
-	if header.height > ctx.head.height + 1 {
-		return Err(Error::Orphan);
-	}
 
 	// check version, enforces scheduled hard fork
 	if !consensus::valid_header_version(header.height, header.version) {
@@ -188,16 +185,20 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext) -> Result<(), E
 	}
 
 	// first I/O cost, better as late as possible
-	let prev = try!(ctx.store.get_block_header(&header.previous,).map_err(|e| {
+	let prev = match ctx.store.get_block_header(&header.previous) {
-		Error::StoreErr(e, format!("previous block header {}", header.previous))
+		Ok(prev) => Ok(prev),
-	},));
+		Err(grin_store::Error::NotFoundErr) => Err(Error::Orphan),
+		Err(e) =>{
+			Err(Error::StoreErr(e, format!("previous header {}", header.previous)))
+		}
+	}?;
 
 	if header.height != prev.height + 1 {
 		return Err(Error::InvalidBlockHeight);
 	}
 	if header.timestamp <= prev.timestamp && !global::is_automated_testing_mode() {
 		// prevent time warp attacks and some timestamp manipulations by forcing strict
-  // time progression (but not in CI mode)
+		// time progression (but not in CI mode)
 		return Err(Error::InvalidBlockTime);
 	}
 

chain/src/sumtree.rs

@@ -253,9 +253,7 @@ impl<'a> Extension<'a> {
 					Err(s) => return Err(Error::SumTreeErr(s)),
 				}
 			} else {
-				return Err(Error::SumTreeErr(
+				return Err(Error::AlreadySpent);
-					format!("Missing index for {:?}", input.commitment()),
-				));
 			}
 		}
 
@@ -346,12 +344,18 @@ impl<'a> Extension<'a> {
 		);
 
 		let out_pos_rew = match block.outputs.last() {
-			Some(output) => self.commit_index.get_output_pos(&output.commitment())?,
+			Some(output) => self.commit_index.get_output_pos(&output.commitment())
+				.map_err(|e| {
+					Error::StoreErr(e, format!("missing output pos for known block"))
+				})?,
 			None => 0,
 		};
 
 		let kern_pos_rew = match block.kernels.last() {
-			Some(kernel) => self.commit_index.get_kernel_pos(&kernel.excess)?,
+			Some(kernel) => self.commit_index.get_kernel_pos(&kernel.excess)
+				.map_err(|e| {
+					Error::StoreErr(e, format!("missing kernel pos for known block"))
+				})?,
 			None => 0,
 		};
 

config/src/config.rs

@@ -186,37 +186,3 @@ impl GlobalConfig {
 			.enable_mining;
 	}
 }
-
-#[test]
-fn test_read_config() {
-	let toml_str = r#"
-        #Section is optional, if not here or enable_server is false, will only run wallet
-        [server]
-        enable_server = true
-        api_http_addr = "127.0.0.1"
-        db_root = "."
-        seeding_type = "None"
-        test_mode = false
-        #7 = FULL_NODE, not sure how to serialise this properly to use constants
-        capabilities = [7]
-
-        [server.p2p_config]
-        host = "127.0.0.1"
-        port = 13414
-
-        #Mining section is optional, if it's not here it will default to not mining
-        [mining]
-        enable_mining = true
-        wallet_listener_url = "http://127.0.0.1:13415"
-        burn_reward = false
-        #testing value, optional
-        #slow_down_in_millis = 30
-
-    "#;
-
-	let mut decoded: GlobalConfig = toml::from_str(toml_str).unwrap();
-	decoded.server.as_mut().unwrap().mining_config = decoded.mining;
-	println!("Decoded.server: {:?}", decoded.server);
-	println!("Decoded wallet: {:?}", decoded.wallet);
-	panic!("panic");
-}

core/src/core/block.rs

@@ -618,6 +618,8 @@ mod test {
 	}
 
 	// Too slow for now #[test]
+	// TODO: make this fast enough or add similar but faster test?
+	#[allow(dead_code)]
 	fn too_large_block() {
 		let keychain = Keychain::from_random_seed().unwrap();
 		let max_out = MAX_BLOCK_WEIGHT / BLOCK_OUTPUT_WEIGHT;

grin/src/adapters.rs

@@ -43,6 +43,10 @@ impl NetAdapter for NetToChainAdapter {
 		self.chain.total_difficulty()
 	}
 
+	fn total_height(&self) -> u64 {
+		self.chain.head().unwrap().height
+	}
+
 	fn transaction_received(&self, tx: core::Transaction) {
 		let source = pool::TxSource {
 			debug_name: "p2p".to_string(),
@@ -247,16 +251,19 @@ impl NetAdapter for NetToChainAdapter {
 		}
 	}
 
-	fn peer_difficulty(&self, addr: SocketAddr, diff: Difficulty) {
+	fn peer_difficulty(&self, addr: SocketAddr, diff: Difficulty, height: u64) {
 		debug!(
 			LOGGER,
-			"peer total_diff (ping/pong): {}, {} vs us {}",
+			"peer total_diff @ height (ping/pong): {}: {} @ {} \
+				 vs us: {} @ {}",
 			addr,
 			diff,
+			height,
 			self.total_difficulty(),
+			self.total_height()
 		);
 
-		if diff.into_num() > 0 {
+		if self.p2p_server.is_initialized() {
 			if let Some(peer) = self.p2p_server.borrow().get_peer(&addr) {
 				let mut peer = peer.write().unwrap();
 				peer.info.total_difficulty = diff;

p2p/src/msg.rs

@@ -479,11 +479,14 @@ pub struct Ping {
 	/// total difficulty accumulated by the sender, used to check whether sync
 	/// may be needed
 	pub total_difficulty: Difficulty,
+	/// total height
+	pub height: u64,
 }
 
 impl Writeable for Ping {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
 		self.total_difficulty.write(writer).unwrap();
+		self.height.write(writer).unwrap();
 		Ok(())
 	}
 }
@@ -495,7 +498,11 @@ impl Readable for Ping {
 			Ok(diff) => diff,
 			Err(_) => Difficulty::zero(),
 		};
-		Ok(Ping { total_difficulty })
+		let height = match reader.read_u64(){
+			Ok(h) => h,
+			Err(_) => 0,
+		};
+Ok(Ping { total_difficulty, height })
 	}
 }
 
@@ -503,11 +510,14 @@ pub struct Pong {
 	/// total difficulty accumulated by the sender, used to check whether sync
 	/// may be needed
 	pub total_difficulty: Difficulty,
+	/// height accumulated by sender
+	pub height: u64
 }
 
 impl Writeable for Pong {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
 		self.total_difficulty.write(writer).unwrap();
+		self.height.write(writer).unwrap();
 		Ok(())
 	}
 }
@@ -519,6 +529,10 @@ impl Readable for Pong {
 			Ok(diff) => diff,
 			Err(_) => Difficulty::zero(),
 		};
-		Ok(Pong { total_difficulty })
+		let height = match reader.read_u64() {
+			Ok(h) => h,
+			Err(_) => 0,
+		};
+		Ok(Pong { total_difficulty, height })
 	}
 }

p2p/src/peer.rs

@@ -140,14 +140,15 @@ impl Peer {
 		self.proto.transmitted_bytes()
 	}
 
-	pub fn send_ping(&self, total_difficulty: Difficulty) -> Result<(), Error> {
+	pub fn send_ping(&self, total_difficulty: Difficulty, height: u64) -> Result<(), Error> {
-		self.proto.send_ping(total_difficulty)
+		self.proto.send_ping(total_difficulty, height)
 	}
 
 	/// Sends the provided block to the remote peer. The request may be dropped
 	/// if the remote peer is known to already have the block.
 	pub fn send_block(&self, b: &core::Block) -> Result<(), Error> {
 		if !self.tracking_adapter.has(b.hash()) {
+			debug!(LOGGER, "Send block {} to {}", b.hash(), self.info.addr);
 			self.proto.send_block(b)
 		} else {
 			Ok(())
@@ -225,6 +226,10 @@ impl NetAdapter for TrackingAdapter {
 		self.adapter.total_difficulty()
 	}
 
+	fn total_height(&self) -> u64 {
+		self.adapter.total_height()
+	}
+
 	fn transaction_received(&self, tx: core::Transaction) {
 		self.push(tx.hash());
 		self.adapter.transaction_received(tx)
@@ -259,7 +264,7 @@ impl NetAdapter for TrackingAdapter {
 		self.adapter.peer_connected(pi)
 	}
 
-	fn peer_difficulty(&self, addr: SocketAddr, diff: Difficulty) {
+	fn peer_difficulty(&self, addr: SocketAddr, diff: Difficulty, height:u64) {
-		self.adapter.peer_difficulty(addr, diff)
+		self.adapter.peer_difficulty(addr, diff, height)
 	}
 }

p2p/src/protocol.rs

@@ -67,11 +67,11 @@ impl Protocol for ProtocolV1 {
 
 	/// Sends a ping message to the remote peer. Will panic if handle has never
 	/// been called on this protocol.
-	fn send_ping(&self, total_difficulty: Difficulty) -> Result<(), Error> {
+	fn send_ping(&self, total_difficulty: Difficulty, height: u64) -> Result<(), Error> {
 		self.send_request(
 			Type::Ping,
 			Type::Pong,
-			&Ping { total_difficulty },
+			&Ping { total_difficulty, height },
 			None,
 		)
 	}
@@ -128,7 +128,11 @@ impl ProtocolV1 {
 		body: &W,
 		expect_resp: Option<Hash>,
 	) -> Result<(), Error> {
-		self.conn.borrow().send_request(t, rt, body, expect_resp)
+		if self.conn.is_initialized() {
+			self.conn.borrow().send_request(t, rt, body, expect_resp)
+		} else {
+			Ok(())
+		}
 	}
 }
 
@@ -142,8 +146,8 @@ fn handle_payload(
 	match header.msg_type {
 		Type::Ping => {
 			let ping = ser::deserialize::<Ping>(&mut &buf[..])?;
-			adapter.peer_difficulty(addr, ping.total_difficulty);
+			adapter.peer_difficulty(addr, ping.total_difficulty, ping.height);
-			let pong = Pong { total_difficulty: adapter.total_difficulty() };
+			let pong = Pong { total_difficulty: adapter.total_difficulty(), height: adapter.total_height() };
 			let mut body_data = vec![];
 			try!(ser::serialize(&mut body_data, &pong));
 			let mut data = vec![];
@@ -157,7 +161,7 @@ fn handle_payload(
 		}
 		Type::Pong => {
 			let pong = ser::deserialize::<Pong>(&mut &buf[..])?;
-			adapter.peer_difficulty(addr, pong.total_difficulty);
+			adapter.peer_difficulty(addr, pong.total_difficulty, pong.height);
 			Ok(None)
 		},
 		Type::Transaction => {

p2p/src/server.rs

@@ -44,6 +44,9 @@ impl NetAdapter for DummyAdapter {
 	fn total_difficulty(&self) -> Difficulty {
 		Difficulty::one()
 	}
+	fn total_height(&self) -> u64 {
+		0
+	}
 	fn transaction_received(&self, _: core::Transaction) {}
 	fn block_received(&self, _: core::Block, _: SocketAddr) {}
 	fn headers_received(&self, _: Vec<core::BlockHeader>, _:SocketAddr) {}
@@ -58,7 +61,7 @@ impl NetAdapter for DummyAdapter {
 	}
 	fn peer_addrs_received(&self, _: Vec<SocketAddr>) {}
 	fn peer_connected(&self, _: &PeerInfo) {}
-	fn peer_difficulty(&self, _: SocketAddr, _: Difficulty) {}
+	fn peer_difficulty(&self, _: SocketAddr, _: Difficulty, _:u64) {}
 }
 
 /// P2P server implementation, handling bootstrapping to find and connect to
@@ -189,7 +192,8 @@ impl Server {
 			.interval(Duration::new(20, 0))
 			.fold((), move |_, _| {
 				let total_diff = adapter.total_difficulty();
-				check_peers(peers_inner.clone(), total_diff);
+				let total_height = adapter.total_height();
+				check_peers(peers_inner.clone(), total_diff, total_height);
 				Ok(())
 			});
 
@@ -507,12 +511,13 @@ where
 fn check_peers(
 	peers: Arc<RwLock<HashMap<SocketAddr, Arc<RwLock<Peer>>>>>,
 	total_difficulty: Difficulty,
+	height: u64
 ) {
 	let peers_map = peers.read().unwrap();
 	for p in peers_map.values() {
 		let p = p.read().unwrap();
 		if p.is_connected() {
-			let _ = p.send_ping(total_difficulty.clone());
+			let _ = p.send_ping(total_difficulty.clone(), height);
 		}
 	}
 }

p2p/src/types.rs

@@ -135,7 +135,7 @@ pub trait Protocol {
 		-> Box<Future<Item = (), Error = Error>>;
 
 	/// Sends a ping message to the remote peer.
-	fn send_ping(&self, total_difficulty: Difficulty) -> Result<(), Error>;
+	fn send_ping(&self, total_difficulty: Difficulty, height: u64) -> Result<(), Error>;
 
 	/// Relays a block to the remote peer.
 	fn send_block(&self, b: &core::Block) -> Result<(), Error>;
@@ -163,9 +163,12 @@ pub trait Protocol {
 /// forwarding or querying of blocks and transactions from the network among
 /// other things.
 pub trait NetAdapter: Sync + Send {
-	/// Current height of our chain.
+	/// Current total difficulty on our chain
 	fn total_difficulty(&self) -> Difficulty;
 
+	/// Current total height
+	fn total_height(&self) -> u64;
+
 	/// A valid transaction has been received from one of our peers
 	fn transaction_received(&self, tx: core::Transaction);
 
@@ -196,5 +199,5 @@ pub trait NetAdapter: Sync + Send {
 	fn peer_connected(&self, &PeerInfo);
 
 	/// Heard total_difficulty from a connected peer (via ping/pong).
-	fn peer_difficulty(&self, SocketAddr, Difficulty);
+	fn peer_difficulty(&self, SocketAddr, Difficulty, u64);
 }

p2p/tests/peer_handshake.rs

@@ -74,7 +74,7 @@ fn peer_handshake() {
 						rhandle.spawn(peer.run(socket).map_err(|e| {
 							panic!("Client run failed: {:?}", e);
 						}));
-						peer.send_ping(Difficulty::one()).unwrap();
+						peer.send_ping(Difficulty::one(), 0).unwrap();
 						timeout_send.from_err().map(|_| peer)
 					})
 					.and_then(|peer| {

pool/src/types.rs

@@ -178,6 +178,8 @@ pub trait PoolAdapter: Send + Sync {
 }
 
 /// Dummy adapter used as a placeholder for real implementations
+// TODO: do we need this dummy, if it's never used?
+#[allow(dead_code)]
 pub struct NoopAdapter {}
 impl PoolAdapter for NoopAdapter {
 	fn tx_accepted(&self, _: &transaction::Transaction) {}

src/bin/grin.rs

@@ -340,9 +340,6 @@ fn server_command(server_args: &ArgMatches, global_config: GlobalConfig) {
 	match server_args.subcommand() {
 		("run", _) => {
 			grin::Server::start(server_config).unwrap();
-			loop {
-				thread::sleep(Duration::from_secs(60));
-			}
 		}
 		("start", _) => {
 			let daemonize = Daemonize::new()

util/src/lib.rs

@@ -87,8 +87,10 @@ impl<T> OneTime<T> {
 
 	/// Whether the OneTime has been initialized
 	pub fn is_initialized(&self) -> bool {
-		let inner = self.inner.borrow();
+		match self.inner.try_borrow() {
-		inner.is_some()
+			Ok(inner) => inner.is_some(),
+			Err(_) => false,
+		}
 	}
 
 	/// Borrows the OneTime, should only be called after initialization.