Refactor SentTransactionHash to be a stricter type (#3706)

* Stub `sendrawtransaction` RPC method

Register the RPC method, and stub an implementation that currently just
panics. The method has a single `String` parameter with the hexadecimal
string of the raw transaction's bytes and returns a
`SentTransactionHash` wrapper type that's just a hexadecimal `String` of
the sent transaction's hash.

* Add mempool service instance to `RpcImpl`

Use a type parameter to represent the mempool service using the
interface defined by `zebra-node-services`.

* Update test vector to use a mock mempool service

Update the test to be compatible with the changes to `RpcImpl`. The mock
mempool service is expected to not be used during the test.

* Use a `tower::Buffer` for the mempool service

Make it simpler to send requests to the service in a concurrent manner.

* Return a `Future` from `send_raw_transaction`

Make the call asynchronous.

* Implement `sendrawtransaction` RPC

Deserialize the transaction and send it to be queued for verification
and subsequent inclusion in the mempool.

* Test if mempool receives sent raw transaction

Use a mock service as the mempool service and check that it receives a
sent raw transaction.

* Test using non-hexadecimal string parameter

The method should return an error.

* Test with bytes that fail deserialization

Check that the method returns an invalid parameters error if the input
can't be deserialized as a `Transaction`.

* Test if mempool errors are forwarded to caller

Mempool service errors should be sent back to the remote caller as
server errors.

* Test transactions rejected by the mempool service

Transactions that are rejected by the mempool service should result in
a server error being sent to the caller.

* Improve error message

Add the word "structurally" to make it clear that the issue is in the
transaction's deserialization.

Co-authored-by: Deirdre Connolly <[email protected]>

* Add note regarding missing `allowhighfees` param.

The parameter isn't supported yet because `lightwalletd` doesn't use it.

* Update the documentation to be consistent

Follow the convention adopted by the `get_info` RPC method.

* Implement `ToHex` and `FromHex` for `Hash`

Make it easier to generate hexadecimal strings from `transaction::Hash`
instances.

* Use `ToHex` in `Debug` and `Display`

Reduce repeated code.

* Refactor to add `bytes_in_display_order` method

Use it to remove repeated code and improve clarity a bit.

* Use `hex::serialize` to serialize transaction hash

Make the type stricter in its contents, while still serializing the
transaction has as a hexadecimal string.

* Simplify serialization attribute

Deserialization should also use `hex::deserialize`, so using the shorter
attribute makes things easier to read and more future proof.

* Update zebra-chain/src/transaction/hash.rs

* Remove unnecessary lifetime

The anonymous lifetime is automatically inferred by the compiler.

Co-authored-by: Deirdre Connolly <[email protected]>

zebra-chain/src/transaction/hash.rs

@@ -34,6 +34,7 @@ use std::{
 #[cfg(any(test, feature = "proptest-impl"))]
 use proptest_derive::Arbitrary;
 
+use hex::{FromHex, ToHex};
 use serde::{Deserialize, Serialize};
 
 use crate::serialization::{
@@ -100,20 +101,58 @@ impl From<&Hash> for [u8; 32] {
     }
 }
 
-impl fmt::Display for Hash {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+impl Hash {
+    /// Return the hash bytes in big-endian byte-order suitable for printing out byte by byte.
+    ///
+    /// Zebra displays transaction and block hashes in big-endian byte-order,
+    /// following the u256 convention set by Bitcoin and zcashd.
+    fn bytes_in_display_order(&self) -> [u8; 32] {
         let mut reversed_bytes = self.0;
         reversed_bytes.reverse();
-        f.write_str(&hex::encode(&reversed_bytes))
+        reversed_bytes
+    }
+}
+
+impl ToHex for &Hash {
+    fn encode_hex<T: FromIterator<char>>(&self) -> T {
+        self.bytes_in_display_order().encode_hex()
+    }
+
+    fn encode_hex_upper<T: FromIterator<char>>(&self) -> T {
+        self.bytes_in_display_order().encode_hex_upper()
+    }
+}
+
+impl ToHex for Hash {
+    fn encode_hex<T: FromIterator<char>>(&self) -> T {
+        (&self).encode_hex()
+    }
+
+    fn encode_hex_upper<T: FromIterator<char>>(&self) -> T {
+        (&self).encode_hex_upper()
+    }
+}
+
+impl FromHex for Hash {
+    type Error = <[u8; 32] as FromHex>::Error;
+
+    fn from_hex<T: AsRef<[u8]>>(hex: T) -> Result<Self, Self::Error> {
+        let hash = <[u8; 32]>::from_hex(hex)?;
+
+        Ok(hash.into())
+    }
+}
+
+impl fmt::Display for Hash {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write_str(&self.encode_hex::<String>())
     }
 }
 
 impl fmt::Debug for Hash {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let mut reversed_bytes = self.0;
-        reversed_bytes.reverse();
         f.debug_tuple("transaction::Hash")
-            .field(&hex::encode(reversed_bytes))
+            .field(&self.encode_hex::<String>())
             .finish()
     }
 }

zebra-rpc/Cargo.toml

@@ -28,8 +28,8 @@ tower = "0.4.12"
 tracing = "0.1.31"
 tracing-futures = "0.2.5"
 
+hex = { version = "0.4.3", features = ["serde"] }
 serde = { version = "1.0.136", features = ["serde_derive"] }
-hex = "0.4.3"
 
 [dev-dependencies]
 proptest = "0.10.1"

zebra-rpc/src/methods.rs

@@ -12,7 +12,10 @@ use jsonrpc_core::{self, BoxFuture, Error, ErrorCode, Result};
 use jsonrpc_derive::rpc;
 use tower::{buffer::Buffer, ServiceExt};
 
-use zebra_chain::{serialization::ZcashDeserialize, transaction::Transaction};
+use zebra_chain::{
+    serialization::ZcashDeserialize,
+    transaction::{self, Transaction},
+};
 use zebra_network::constants::USER_AGENT;
 use zebra_node_services::{mempool, BoxError};
 
@@ -156,7 +159,7 @@ where
             );
 
             match &queue_results[0] {
-                Ok(()) => Ok(SentTransactionHash(transaction_hash.to_string())),
+                Ok(()) => Ok(SentTransactionHash(transaction_hash)),
                 Err(error) => Err(Error {
                     code: ErrorCode::ServerError(0),
                     message: error.to_string(),
@@ -186,4 +189,4 @@ pub struct GetBlockChainInfo {
 /// Response to a `sendrawtransaction` RPC request.
 ///
 /// A JSON string with the transaction hash in hexadecimal.
-pub struct SentTransactionHash(String);
+pub struct SentTransactionHash(#[serde(with = "hex")] transaction::Hash);

zebra-rpc/src/methods/tests/prop.rs

@@ -21,7 +21,7 @@ proptest! {
         runtime.block_on(async move {
             let mut mempool = MockService::build().for_prop_tests();
             let rpc = RpcImpl::new("RPC test".to_owned(), Buffer::new(mempool.clone(), 1));
-            let hash = SentTransactionHash(transaction.hash().to_string());
+            let hash = SentTransactionHash(transaction.hash());
 
             let transaction_bytes = transaction
                 .zcash_serialize_to_vec()

zebrad/src/commands/start.rs

@@ -189,7 +189,7 @@ impl StartCmd {
             chain_tip_change,
         );
 
-        let mempool_queue_checker_task_handle = mempool::QueueChecker::spawn(mempool);
+        let mempool_queue_checker_task_handle = mempool::QueueChecker::spawn(mempool.clone());
 
         let tx_gossip_task_handle = tokio::spawn(
             mempool::gossip_mempool_transaction_id(mempool_transaction_receiver, peer_set)