Bug fixes in wallet and related API

Fixes a few loose ends in the full cycle of sending coins,
accepting them, pushing that transaction to the pool and having it
mined. More specifically:

* The API output endpoint needs to be a UTXO endpoint, as the
server can't make any guarantee about having a spent output.
* Bubbling up HTTP not found errors.
* Wallet output status checker now handles spent outputs.
* Transaction pool validates the transaction before accepting it.
* Fixed the operation API routes.
* Fixed too greedy wallet coin selection loop.

api/Cargo.toml

@@ -8,6 +8,7 @@ workspace = ".."
 grin_core = { path = "../core" }
 grin_chain = { path = "../chain" }
 grin_pool = { path = "../pool" }
+grin_store = { path = "../store" }
 grin_util = { path = "../util" }
 secp256k1zkp = { path = "../secp256k1zkp" }
 

api/src/client.rs

@@ -16,7 +16,7 @@
 
 use hyper;
 use hyper::client::Response;
-use hyper::status::StatusClass;
+use hyper::status::{StatusClass, StatusCode};
 use serde::{Serialize, Deserialize};
 use serde_json;
 
@@ -59,7 +59,13 @@ fn check_error(res: hyper::Result<Response>) -> Result<Response, Error> {
 	match response.status.class() {
 		StatusClass::Success => Ok(response),
 		StatusClass::ServerError => Err(Error::Internal(format!("Server error."))),
-		StatusClass::ClientError => Err(Error::Argument(format!("Argument error"))),
+		StatusClass::ClientError => {
+			if response.status == StatusCode::NotFound {
+				Err(Error::NotFound)
+			} else {
+				Err(Error::Argument(format!("Argument error")))
+			}
+		}
 		_ => Err(Error::Internal(format!("Unrecognized error."))),
 	}
 }

api/src/endpoints.rs

@@ -21,7 +21,7 @@
 //   }
 // }
 
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, Mutex, RwLock};
 use std::thread;
 
 use core::core::{Transaction, Output};
@@ -61,6 +61,7 @@ impl ApiEndpoint for ChainApi {
 pub struct OutputApi {
 	/// data store access
 	chain_store: Arc<chain::ChainStore>,
+	chain_head: Arc<Mutex<chain::Tip>>,
 }
 
 impl ApiEndpoint for OutputApi {
@@ -76,10 +77,34 @@ impl ApiEndpoint for OutputApi {
 	fn get(&self, id: String) -> ApiResult<Output> {
 		debug!("GET output {}", id);
 		let c = util::from_hex(id.clone()).map_err(|e| Error::Argument(format!("Not a valid commitment: {}", id)))?;
-		let out = self.chain_store
-			.get_output_by_commit(&Commitment::from_vec(c))
-			.map_err(|e| Error::Internal(e.to_string()));
-		out
+		let commitment = Commitment::from_vec(c);
+
+		// TODO use an actual UTXO tree
+		// in the meantime doing it the *very* expensive way:
+		//   1. check the output exists
+		//   2. run the chain back from the head to check it hasn't been spent
+		if let Ok(out) = self.chain_store.get_output_by_commit(&commitment) {
+			let mut block_h: Hash;
+			{
+				let chain_head = self.chain_head.clone();
+				let head = chain_head.lock().unwrap();
+				block_h = head.last_block_h;
+			}
+			loop {
+				let b = self.chain_store.get_block(&block_h)?;
+				for input in b.inputs {
+					if input.commitment() == commitment {
+						return Err(Error::NotFound);
+					}
+				}
+				if b.header.height == 1 {
+					return Ok(out);
+				} else {
+					block_h = b.header.previous;
+				}
+			}
+		}
+		Err(Error::NotFound)
 	}
 }
 
@@ -130,6 +155,9 @@ impl<T> ApiEndpoint for PoolApi<T>
 			debug_name: "push-api".to_string(),
 			identifier: "?.?.?.?".to_string(),
 		};
+		debug!("Pushing transaction with {} inputs and {} outputs to pool.",
+		       tx.inputs.len(),
+		       tx.outputs.len());
 		self.tx_pool
 			.write()
 			.unwrap()
@@ -149,6 +177,7 @@ struct TxWrapper {
 /// instance and runs the corresponding HTTP server.
 pub fn start_rest_apis<T>(addr: String,
                           chain_store: Arc<chain::ChainStore>,
+                          chain_head: Arc<Mutex<chain::Tip>>,
                           tx_pool: Arc<RwLock<pool::TransactionPool<T>>>)
 	where T: pool::BlockChain + Clone + Send + Sync + 'static
 {
@@ -157,8 +186,11 @@ pub fn start_rest_apis<T>(addr: String,
 		let mut apis = ApiServer::new("/v1".to_string());
 		apis.register_endpoint("/chain".to_string(),
 		                       ChainApi { chain_store: chain_store.clone() });
-		apis.register_endpoint("/chain/output".to_string(),
-		                       OutputApi { chain_store: chain_store.clone() });
+		apis.register_endpoint("/chain/utxo".to_string(),
+		                       OutputApi {
+			                       chain_store: chain_store.clone(),
+			                       chain_head: chain_head.clone(),
+		                       });
 		apis.register_endpoint("/pool".to_string(), PoolApi { tx_pool: tx_pool });
 
 		apis.start(&addr[..]).unwrap_or_else(|e| {

api/src/lib.rs

@@ -15,6 +15,7 @@
 extern crate grin_core as core;
 extern crate grin_chain as chain;
 extern crate grin_pool as pool;
+extern crate grin_store as store;
 extern crate grin_util as util;
 extern crate secp256k1zkp as secp;
 

api/src/rest.rs

@@ -34,11 +34,14 @@ use serde::{Serialize, Deserialize};
 use serde::de::DeserializeOwned;
 use serde_json;
 
+use store;
+
 /// Errors that can be returned by an ApiEndpoint implementation.
 #[derive(Debug)]
 pub enum Error {
 	Internal(String),
 	Argument(String),
+	NotFound,
 }
 
 impl Display for Error {
@@ -46,6 +49,7 @@ impl Display for Error {
 		match *self {
 			Error::Argument(ref s) => write!(f, "Bad arguments: {}", s),
 			Error::Internal(ref s) => write!(f, "Internal error: {}", s),
+			Error::NotFound => write!(f, "Not found."),
 		}
 	}
 }
@@ -55,6 +59,7 @@ impl error::Error for Error {
 		match *self {
 			Error::Argument(_) => "Bad arguments.",
 			Error::Internal(_) => "Internal error.",
+			Error::NotFound => "Not found.",
 		}
 	}
 }
@@ -64,6 +69,16 @@ impl From<Error> for IronError {
 		match e {
 			Error::Argument(_) => IronError::new(e, status::Status::BadRequest),
 			Error::Internal(_) => IronError::new(e, status::Status::InternalServerError),
+			Error::NotFound => IronError::new(e, status::Status::NotFound),
+		}
+	}
+}
+
+impl From<store::Error> for Error {
+	fn from(e: store::Error) -> Error {
+		match e {
+			store::Error::NotFoundErr => Error::NotFound,
+			_ => Error::Internal(e.to_string()),
 		}
 	}
 }
@@ -250,7 +265,7 @@ impl ApiServer {
 					operation: op_s.clone(),
 					endpoint: endpoint.clone(),
 				};
-				let full_path = format!("{}", root.clone());
+				let full_path = format!("{}/{}", root.clone(), op_s.clone());
 				self.router.route(op.to_method(), full_path.clone(), wrapper, route_name);
 				info!("route: POST {}", full_path);
 			} else {

chain/src/pipe.rs

@@ -105,9 +105,11 @@ pub fn process_block(b: &Block,
 		head: head,
 	};
 
-	info!("Starting validation pipeline for block {} at {}.",
+	info!("Starting validation pipeline for block {} at {} with {} inputs and {} outputs.",
 	      b.hash(),
-	      b.header.height);
+	      b.header.height,
+	      b.inputs.len(),
+	      b.outputs.len());
 	try!(check_known(b.hash(), &mut ctx));
 
 	if !ctx.opts.intersects(SYNC) {

grin/src/miner.rs

@@ -145,6 +145,13 @@ impl Miner {
 		let txs = txs_box.iter().map(|tx| tx.as_ref()).collect();
 		let (output, kernel) = coinbase;
 		let mut b = core::Block::with_reward(head, txs, output, kernel).unwrap();
+		debug!("Built new block with {} inputs and {} outputs",
+		       b.inputs.len(),
+		       b.outputs.len());
+
+		// making sure we're not spending time mining a useless block
+		let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit);
+		b.validate(&secp).expect("Built an invalid block!");
 
 		let mut rng = rand::OsRng::new().unwrap();
 		b.header.nonce = rng.gen();
@@ -161,7 +168,7 @@ impl Miner {
 			let skey = secp::key::SecretKey::new(&secp_inst, &mut rng);
 			core::Block::reward_output(skey, &secp_inst).unwrap()
 		} else {
-			let url = format!("{}/v1/receive_coinbase",
+			let url = format!("{}/v1/receive/coinbase",
 			                  self.config.wallet_receiver_url.as_str());
 			let res: CbData = api::client::post(url.as_str(),
 			                                    &CbAmount { amount: consensus::REWARD })

grin/src/server.rs

@@ -117,6 +117,7 @@ impl Server {
 
 		api::start_rest_apis(config.api_http_addr.clone(),
 		                     chain_store.clone(),
+		                     shared_head.clone(),
 		                     tx_pool.clone());
 
 		warn!("Grin server started.");

pool/src/pool.rs

@@ -23,6 +23,7 @@ use core::core::hash;
 // Temporary blockchain dummy impls
 use blockchain::{DummyChain, DummyChainImpl, DummyUtxoSet};
 
+use secp;
 use secp::pedersen::Commitment;
 
 use std::sync::{Arc, RwLock, Weak};
@@ -109,8 +110,12 @@ impl<T> TransactionPool<T> where T: BlockChain {
     /// if necessary, and performing any connection-related validity checks.
     /// Happens under an exclusive mutable reference gated by the write portion
     /// of a RWLock.
-    ///
     pub fn add_to_memory_pool(&mut self, source: TxSource, tx: transaction::Transaction) -> Result<(), PoolError> {
+        // Making sure the transaction is valid before anything else.
+        let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit);
+        tx.validate(&secp).map_err(|_| PoolError::Invalid)?;
+        
+
         // The first check invovles ensuring that an identical transaction is 
         // not already in the pool's transaction set.
         // A non-authoritative similar check should be performed under the 
@@ -132,7 +137,6 @@ impl<T> TransactionPool<T> where T: BlockChain {
         let mut orphan_refs: Vec<graph::Edge> = Vec::new();
         let mut blockchain_refs: Vec<graph::Edge> = Vec::new();
 
-
         for input in &tx.inputs {
             let base = graph::Edge::new(None, Some(tx_hash),
                 input.commitment());

src/bin/grin.rs

@@ -209,8 +209,7 @@ fn wallet_command(wallet_args: &ArgMatches) {
 			} else {
 				info!("Starting the Grin wallet receiving daemon...");
 				let mut apis = api::ApiServer::new("/v1".to_string());
-				apis.register_endpoint("/receive_coinbase".to_string(),
-				                       wallet::WalletReceiver { key: key });
+				apis.register_endpoint("/receive".to_string(), wallet::WalletReceiver { key: key });
 				apis.start("127.0.0.1:13416").unwrap_or_else(|e| {
 					error!("Failed to start Grin wallet receiver: {}.", e);
 				});

wallet/src/checker.rs

@@ -36,10 +36,17 @@ pub fn refresh_outputs(config: &WalletConfig, ext_key: &ExtendedKey) {
 			let commitment = secp.commit(out.value, key.key).unwrap();
 
 			// TODO check the pool for unconfirmed
+
 			let out_res = get_output_by_commitment(config, commitment);
 			if out_res.is_ok() {
 				out.status = OutputStatus::Unspent;
 				changed += 1;
+			} else if out.status == OutputStatus::Unspent {
+				// a UTXO we can't find anymore has been spent
+				if let Err(api::Error::NotFound) = out_res {
+					out.status = OutputStatus::Spent;
+					changed += 1;
+				}
 			}
 		}
 	}
@@ -53,7 +60,7 @@ pub fn refresh_outputs(config: &WalletConfig, ext_key: &ExtendedKey) {
 fn get_output_by_commitment(config: &WalletConfig,
                             commit: pedersen::Commitment)
                             -> Result<Output, api::Error> {
-	let url = format!("{}/v1/chain/output/{}",
+	let url = format!("{}/v1/chain/utxo/{}",
 	                  config.api_http_addr,
 	                  util::to_hex(commit.as_ref().to_vec()));
 	api::client::get::<Output>(url.as_str())

wallet/src/receiver.rs

@@ -75,7 +75,7 @@ pub fn receive_json_tx(ext_key: &ExtendedKey, partial_tx_str: &str) -> Result<()
 	let tx_hex = util::to_hex(ser::ser_vec(&final_tx).unwrap());
 
 	let config = WalletConfig::default();
-	let url = format!("{}/v1/receive_coinbase", config.api_http_addr.as_str());
+	let url = format!("{}/v1/pool/push", config.api_http_addr.as_str());
 	api::client::post(url.as_str(), &TxWrapper { tx_hex: tx_hex })?;
 	Ok(())
 }
@@ -107,7 +107,7 @@ impl ApiEndpoint for WalletReceiver {
 	type OP_OUT = CbData;
 
 	fn operations(&self) -> Vec<Operation> {
-		vec![Operation::Custom("receive_coinbase".to_string())]
+		vec![Operation::Custom("coinbase".to_string())]
 	}
 
 	fn operation(&self, op: String, input: CbAmount) -> ApiResult<CbData> {
@@ -116,7 +116,7 @@ impl ApiEndpoint for WalletReceiver {
 			return Err(api::Error::Argument(format!("Zero amount not allowed.")));
 		}
 		match op.as_str() {
-			"receive_coinbase" => {
+			"coinbase" => {
 				let (out, kern) =
 					receive_coinbase(&self.key, input.amount).map_err(|e| {
 							api::Error::Internal(format!("Error building coinbase: {:?}", e))
@@ -157,8 +157,8 @@ fn receive_coinbase(ext_key: &ExtendedKey, amount: u64) -> Result<(Output, TxKer
 	});
 	wallet_data.write()?;
 
-	info!("Using child {} for a new coinbase output.",
-	      coinbase_key.n_child);
+	debug!("Using child {} for a new coinbase output.",
+	       coinbase_key.n_child);
 
 	Block::reward_output(coinbase_key.key, &secp).map_err(&From::from)
 }
@@ -181,6 +181,10 @@ fn receive_transaction(ext_key: &ExtendedKey,
 	                                            build::with_excess(blinding),
 	                                            build::output(amount, out_key.key)])?;
 
+	// make sure the resulting transaction is valid (could have been lied to
+	// on excess)
+	tx_final.validate(&secp)?;
+
 	// track the new output and return the finalized transaction to broadcast
 	wallet_data.append_output(OutputData {
 		fingerprint: out_key.fingerprint,
@@ -190,7 +194,8 @@ fn receive_transaction(ext_key: &ExtendedKey,
 	});
 	wallet_data.write()?;
 
-	info!("Using child {} for a new coinbase output.", out_key.n_child);
+	debug!("Using child {} for a new transaction output.",
+	       out_key.n_child);
 
 	Ok(tx_final)
 }

wallet/src/types.rs

@@ -178,6 +178,9 @@ impl WalletData {
 			if out.status == OutputStatus::Unspent && out.fingerprint == fingerprint {
 				to_spend.push(out.clone());
 				input_total += out.value;
+				if input_total >= amount {
+					break;
+				}
 			}
 		}
 		(to_spend, (input_total as i64) - (amount as i64))