From 917072e1212e679bc8cfe22c7d7badd69d2a3a05 Mon Sep 17 00:00:00 2001 From: idky137 Date: Tue, 18 Jun 2024 16:13:46 +0100 Subject: [PATCH] removed unwraps in PR production code --- zingo-rpc/src/blockcache/block.rs | 2 +- zingo-rpc/src/blockcache/utils.rs | 16 +- zingo-rpc/src/jsonrpc/primitives.rs | 26 +++- zingo-rpc/src/rpc/service.rs | 223 +++++++++------------------- 4 files changed, 104 insertions(+), 163 deletions(-) diff --git a/zingo-rpc/src/blockcache/block.rs b/zingo-rpc/src/blockcache/block.rs index 8bdf8f6..a00fadc 100644 --- a/zingo-rpc/src/blockcache/block.rs +++ b/zingo-rpc/src/blockcache/block.rs @@ -417,7 +417,7 @@ pub async fn get_block_from_node( } Ok(GetBlockResponse::Raw(block_hex)) => Ok(FullBlock::parse_to_compact( block_hex.as_ref(), - Some(display_txids_to_server(tx)), + Some(display_txids_to_server(tx)?), trees.sapling.size as u32, trees.orchard.size as u32, )?), diff --git a/zingo-rpc/src/blockcache/utils.rs b/zingo-rpc/src/blockcache/utils.rs index de620d1..d8169a3 100644 --- a/zingo-rpc/src/blockcache/utils.rs +++ b/zingo-rpc/src/blockcache/utils.rs @@ -17,6 +17,12 @@ pub enum ParseError { /// Errors from the JsonRPC client. #[error("JsonRPC Connector Error: {0}")] JsonRpcError(#[from] JsonRpcConnectorError), + /// UTF-8 conversion error. + #[error("UTF-8 Error: {0}")] + Utf8Error(#[from] std::str::Utf8Error), + /// Hexadecimal parsing error. + #[error("Hex Parse Error: {0}")] + ParseIntError(#[from] std::num::ParseIntError), } /// Used for decoding zcash blocks from a bytestring. @@ -121,18 +127,18 @@ pub fn read_zcash_script_i64(cursor: &mut Cursor<&[u8]>) -> Result) -> Vec> { +pub fn display_txids_to_server(txids: Vec) -> Result>, ParseError> { txids .iter() .map(|txid| { txid.as_bytes() .chunks(2) .map(|chunk| { - let hex_pair = std::str::from_utf8(chunk).unwrap(); - u8::from_str_radix(hex_pair, 16).unwrap() + let hex_pair = std::str::from_utf8(chunk).map_err(ParseError::from)?; + u8::from_str_radix(hex_pair, 16).map_err(ParseError::from) }) .rev() - .collect() + .collect::, _>>() }) - .collect() + .collect::>, _>>() } diff --git a/zingo-rpc/src/jsonrpc/primitives.rs b/zingo-rpc/src/jsonrpc/primitives.rs index ee11669..10a6371 100644 --- a/zingo-rpc/src/jsonrpc/primitives.rs +++ b/zingo-rpc/src/jsonrpc/primitives.rs @@ -445,21 +445,33 @@ impl<'de> Deserialize<'de> for GetTransactionResponse { { let v = serde_json::Value::deserialize(deserializer)?; if v.get("height").is_some() && v.get("confirmations").is_some() { + let hex = serde_json::from_value(v["hex"].clone()).map_err(serde::de::Error::custom)?; + let height = v["height"] + .as_i64() + .ok_or_else(|| serde::de::Error::custom("Missing or invalid height"))? + as i32; + let confirmations = v["confirmations"] + .as_u64() + .ok_or_else(|| serde::de::Error::custom("Missing or invalid confirmations"))? + as u32; let obj = GetTransactionResponse::Object { - hex: serde_json::from_value(v["hex"].clone()).unwrap(), - height: v["height"].as_i64().unwrap() as i32, - confirmations: v["confirmations"].as_u64().unwrap() as u32, + hex, + height, + confirmations, }; Ok(obj) } else if v.get("hex").is_some() && v.get("txid").is_some() { + let hex = serde_json::from_value(v["hex"].clone()).map_err(serde::de::Error::custom)?; let obj = GetTransactionResponse::Object { - hex: serde_json::from_value(v["hex"].clone()).unwrap(), - height: -1 as i32, - confirmations: 0 as u32, + hex, + height: -1, + confirmations: 0, }; Ok(obj) } else { - let raw = GetTransactionResponse::Raw(serde_json::from_value(v.clone()).unwrap()); + let raw = GetTransactionResponse::Raw( + serde_json::from_value(v.clone()).map_err(serde::de::Error::custom)?, + ); Ok(raw) } } diff --git a/zingo-rpc/src/rpc/service.rs b/zingo-rpc/src/rpc/service.rs index 36799e6..6a502ee 100644 --- a/zingo-rpc/src/rpc/service.rs +++ b/zingo-rpc/src/rpc/service.rs @@ -13,7 +13,6 @@ use zebra_chain::block::Height; use crate::{ blockcache::{block::get_block_from_node, mempool::Mempool}, - // define_grpc_passthrough, jsonrpc::{ connector::JsonRpcConnector, primitives::{GetTransactionResponse, ProxyConsensusBranchIdHex}, @@ -124,12 +123,6 @@ impl CompactTxStreamer for ProxyClient { Ok(tonic::Response::new(block_id)) }) } - // define_grpc_passthrough!( - // fn get_latest_block( - // &self, - // request: tonic::Request, - // ) -> BlockId - // ); /// Return the compact block corresponding to the given block identifier. /// @@ -160,12 +153,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_block not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_block( - // &self, - // request: tonic::Request, - // ) -> CompactBlock - // ); /// Same as GetBlock except actions contain only nullifiers. /// @@ -194,16 +181,9 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_block_nullifiers not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_block_nullifiers( - // &self, - // request: tonic::Request, - // ) -> CompactBlock - // ); /// Server streaming response type for the GetBlockRange method. #[doc = "Server streaming response type for the GetBlockRange method."] - // type GetBlockRangeStream = tonic::Streaming; type GetBlockRangeStream = std::pin::Pin>; /// Return a list of consecutive compact blocks. @@ -271,12 +251,6 @@ impl CompactTxStreamer for ProxyClient { Ok(tonic::Response::new(stream_boxed)) }) } - // define_grpc_passthrough!( - // fn get_block_range( - // &self, - // request: tonic::Request, - // ) -> Self::GetBlockRangeStream - // ); /// Server streaming response type for the GetBlockRangeNullifiers method. #[doc = " Server streaming response type for the GetBlockRangeNullifiers method."] @@ -309,12 +283,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_block_range_nullifiers not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_block_range_nullifiers( - // &self, - // request: tonic::request, - // ) -> self::getblockrangenullifiersstream - // ); /// Return the requested full (not compact) transaction (as from zcashd). fn get_transaction<'life0, 'async_trait>( @@ -356,12 +324,16 @@ impl CompactTxStreamer for ProxyClient { } else { return Err(tonic::Status::not_found("Transaction not received")); }; + let height: u64 = height.try_into().map_err(|_e| { + tonic::Status::internal( + "Invalid response from server - Height conversion failed", + ) + })?; - // TODO: Remove unwrap on height and handle error. Ok(tonic::Response::new( zcash_client_backend::proto::service::RawTransaction { data: hex.bytes, - height: height.try_into().unwrap(), + height, }, )) } else { @@ -371,12 +343,6 @@ impl CompactTxStreamer for ProxyClient { } }) } - // define_grpc_passthrough!( - // fn get_transaction( - // &self, - // request: tonic::Request, - // ) -> RawTransaction - // ); /// Submit the given transaction to the Zcash network. fn send_transaction<'life0, 'async_trait>( @@ -418,16 +384,9 @@ impl CompactTxStreamer for ProxyClient { )) }) } - // define_grpc_passthrough!( - // fn send_transaction( - // &self, - // request: tonic::Request, - // ) -> SendResponse - // ); /// Server streaming response type for the GetTaddressTxids method. #[doc = "Server streaming response type for the GetTaddressTxids method."] - // type GetTaddressTxidsStream = tonic::Streaming; type GetTaddressTxidsStream = std::pin::Pin>; /// This name is misleading, returns the full transactions that have either inputs or outputs connected to the given transparent address. @@ -525,12 +484,6 @@ impl CompactTxStreamer for ProxyClient { Ok(tonic::Response::new(stream_boxed)) }) } - // define_grpc_passthrough!( - // fn get_taddress_txids( - // &self, - // request: tonic::Request, - // ) -> Self::GetTaddressTxidsStream - // ); /// This RPC has not been implemented as it is not currently used by zingolib. /// If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy). @@ -557,12 +510,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_taddress_balance not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_taddress_balance( - // &self, - // request: tonic::Request, - // ) -> Balance - // ); /// This RPC has not been implemented as it is not currently used by zingolib. /// If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy). @@ -627,23 +574,17 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_mempool_tx not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_mempool_tx( - // &self, - // request: tonic::Request, - // ) -> Self::GetMempoolTxStream - // ); /// Server streaming response type for the GetMempoolStream method. #[doc = "Server streaming response type for the GetMempoolStream method."] - // type GetMempoolStreamStream = tonic::Streaming; type GetMempoolStreamStream = std::pin::Pin>; /// Return a stream of current Mempool transactions. This will keep the output stream open while /// there are mempool transactions. It will close the returned stream when a new block is mined. /// - /// TODO: This implementation is slow. Zingo-Proxy's blockcache state engine should keep its own intyernal mempool state. - /// - This RPC should query Zingo-Proxy's internal mempool state rather than creating its owm mempool and directly querying zebrad. + /// TODO: This implementation is slow. Zingo-Proxy's blockcache state engine should keep its own internal mempool state. + /// - This RPC should query Zingo-Proxy's internal mempool state rather than creating its own mempool and directly querying zebrad. + /// TODO: Add 30s timeout. fn get_mempool_stream<'life0, 'async_trait>( &'life0 self, _request: tonic::Request, @@ -675,53 +616,77 @@ impl CompactTxStreamer for ProxyClient { let (tx, rx) = tokio::sync::mpsc::channel(32); tokio::spawn(async move { let mempool = Mempool::new(); - mempool.update(&zebrad_uri).await.unwrap(); + if let Err(e) = mempool.update(&zebrad_uri).await { + tx.send(Err(tonic::Status::internal(e.to_string()))) + .await + .ok(); + return; + } let mut mined = false; let mut txid_index: usize = 0; while !mined { - let mempool_txids = mempool.get_mempool_txids().await.unwrap(); - for txid in &mempool_txids[txid_index..] { - let transaction = zebrad_client - .get_raw_transaction(txid.clone(), Some(1)) - .await; - match transaction { - Ok(GetTransactionResponse::Object { hex, height, .. }) => { - txid_index += 1; - if tx - .send(Ok(RawTransaction { - data: hex.bytes, - height: height as u64, - })) - .await - .is_err() - { + match mempool.get_mempool_txids().await { + Ok(mempool_txids) => { + for txid in &mempool_txids[txid_index..] { + match zebrad_client + .get_raw_transaction(txid.clone(), Some(1)) + .await { + Ok(GetTransactionResponse::Object { hex, height, .. }) => { + txid_index += 1; + if tx + .send(Ok(RawTransaction { + data: hex.bytes, + height: height as u64, + })) + .await + .is_err() + { + break; + } + } + Ok(GetTransactionResponse::Raw(_)) => { + if tx + .send(Err(tonic::Status::internal( + "Received raw transaction type, this should not be impossible.", + ))) + .await + .is_err() + { + break; + } + } + Err(e) => { + if tx + .send(Err(tonic::Status::internal(e.to_string()))) + .await + .is_err() + { break; + } + } } } - Ok(GetTransactionResponse::Raw(_)) => { - if tx - .send(Err(tonic::Status::internal( - "Received raw transaction type, this should not be impossible.", - ))) + } + Err(e) => { + if tx + .send(Err(tonic::Status::internal(e.to_string()))) .await .is_err() { break; } - } - Err(e) => { - if tx - .send(Err(tonic::Status::internal(e.to_string()))) - .await - .is_err() - { - break; - } - } } } tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; - mined = mempool.update(&zebrad_uri).await.unwrap(); + mined = match mempool.update(&zebrad_uri).await { + Ok(mined) => mined, + Err(e) => { + tx.send(Err(tonic::Status::internal(e.to_string()))) + .await + .ok(); + break; + } + }; } }); let output_stream = RawTransactionStream::new(rx); @@ -729,12 +694,6 @@ impl CompactTxStreamer for ProxyClient { Ok(tonic::Response::new(stream_boxed)) }) } - // define_grpc_passthrough!( - // fn get_mempool_stream( - // &self, - // request: tonic::Request, - // ) -> Self::GetMempoolStreamStream - // ); /// GetTreeState returns the note commitment tree state corresponding to the given block. /// See section 3.7 of the Zcash protocol specification. It returns several other useful @@ -796,12 +755,6 @@ impl CompactTxStreamer for ProxyClient { )) }) } - // define_grpc_passthrough!( - // fn get_tree_state( - // &self, - // request: tonic::Request, - // ) -> zcash_client_backend::proto::service::TreeState - // ); /// This RPC has not been implemented as it is not currently used by zingolib. /// If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy). @@ -828,12 +781,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_latest_tree_state not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_latest_tree_state( - // &self, - // request: tonic::Request, - // ) -> TreeState - // ); /// Server streaming response type for the GetSubtreeRoots method. #[doc = " Server streaming response type for the GetSubtreeRoots method."] @@ -867,12 +814,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_subtree_roots not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_subtree_roots( - // &self, - // request: tonic::Request, - // ) -> Self::GetSubtreeRootsStream - // ); /// This RPC has not been implemented as it is not currently used by zingolib. /// If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy). @@ -901,12 +842,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_address_utxos not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_address_utxos( - // &self, - // request: tonic::Request, - // ) -> GetAddressUtxosReplyList - // ); /// Server streaming response type for the GetAddressUtxosStream method. #[doc = "Server streaming response type for the GetAddressUtxosStream method."] @@ -937,12 +872,6 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("get_address_utxos_stream not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn get_address_utxos_stream( - // &self, - // request: tonic::Request, - // ) -> tonic::Streaming - // ); /// Return information about this lightwalletd instance and the blockchain fn get_lightd_info<'life0, 'async_trait>( @@ -985,7 +914,13 @@ impl CompactTxStreamer for ProxyClient { let sapling_id_str = "76b809bb"; let sapling_id = ProxyConsensusBranchIdHex( - zebra_chain::parameters::ConsensusBranchId::from_hex(sapling_id_str).unwrap(), + zebra_chain::parameters::ConsensusBranchId::from_hex(sapling_id_str).map_err( + |_e| { + tonic::Status::internal( + "Internal Error - Consesnsus Branch ID hex conversion failed", + ) + }, + )?, ); let sapling_height = blockchain_info .upgrades @@ -1014,12 +949,6 @@ impl CompactTxStreamer for ProxyClient { Ok(tonic::Response::new(lightd_info)) }) } - // define_grpc_passthrough!( - // fn get_lightd_info( - // &self, - // request: tonic::Request, - // ) -> LightdInfo - // ); // /// Testing-only, requires lightwalletd --ping-very-insecure (do not enable in production) [from zebrad] /// This RPC has not been implemented as it is not currently used by zingolib. @@ -1047,10 +976,4 @@ impl CompactTxStreamer for ProxyClient { Err(tonic::Status::unimplemented("ping not yet implemented. If you require this RPC please open an issue or PR at the Zingo-Proxy github (https://github.com/zingolabs/zingo-proxy).")) }) } - // define_grpc_passthrough!( - // fn ping( - // &self, - // request: tonic::Request, - // ) -> PingResponse - // ); }