tx: improve the serialization api

crates/apps_lib/src/client/tx.rs

@@ -1069,7 +1069,8 @@ pub async fn sign_tx<N: Namada>(
 where
     <N::Client as namada_sdk::io::Client>::Error: std::fmt::Display,
 {
-    let tx = if let Ok(transaction) = Tx::deserialize(tx_data.as_ref()) {
+    let tx = if let Ok(transaction) = Tx::try_from_json_bytes(tx_data.as_ref())
+    {
         transaction
     } else {
         edisplay_line!(namada.io(), "Couldn't decode the transaction.");

crates/apps_lib/src/client/utils.rs

@@ -1050,7 +1050,8 @@ pub async fn sign_offline(
         safe_exit(1)
     };
 
-    let tx = if let Ok(transaction) = Tx::deserialize(tx_data.as_ref()) {
+    let tx = if let Ok(transaction) = Tx::try_from_json_bytes(tx_data.as_ref())
+    {
         transaction
     } else {
         eprintln!("Couldn't decode the transaction.");

crates/node/src/dry_run_tx.rs

@@ -29,7 +29,7 @@ where
     H: 'static + StorageHasher + Sync,
     CA: 'static + WasmCacheAccess + Sync,
 {
-    let tx = Tx::try_from(&request.data[..]).into_storage_result()?;
+    let tx = Tx::try_from_bytes(&request.data[..]).into_storage_result()?;
     tx.validate_tx().into_storage_result()?;
 
     let gas_scale = parameters::get_gas_scale(&state)?;

crates/node/src/shell/finalize_block.rs

@@ -605,16 +605,17 @@ where
         let mut successful_wrappers = vec![];
 
         for (tx_index, processed_tx) in processed_txs.iter().enumerate() {
-            let tx = if let Ok(tx) = Tx::try_from(processed_tx.tx.as_ref()) {
-                tx
-            } else {
-                tracing::error!(
-                    "FinalizeBlock received a tx that could not be \
-                     deserialized to a Tx type. This is likely a protocol \
-                     transaction."
-                );
-                continue;
-            };
+            let tx =
+                if let Ok(tx) = Tx::try_from_bytes(processed_tx.tx.as_ref()) {
+                    tx
+                } else {
+                    tracing::error!(
+                        "FinalizeBlock received a tx that could not be \
+                         deserialized to a Tx type. This is likely a protocol \
+                         transaction."
+                    );
+                    continue;
+                };
 
             let result_code = ResultCode::from_u32(processed_tx.result.code)
                 .expect("Result code conversion should not fail");

crates/node/src/shell/mod.rs

@@ -1057,7 +1057,7 @@ where
         }
 
         // Tx format check
-        let tx = match Tx::try_from(tx_bytes).map_err(Error::TxDecoding) {
+        let tx = match Tx::try_from_bytes(tx_bytes).map_err(Error::TxDecoding) {
             Ok(t) => t,
             Err(msg) => {
                 response.code = ResultCode::InvalidTx.into();
@@ -2108,7 +2108,7 @@ mod shell_tests {
             )
             .await
             .unwrap();
-            let tx = Tx::try_from(&serialized_tx[..]).unwrap();
+            let tx = Tx::try_from_bytes(&serialized_tx[..]).unwrap();
 
             match ethereum_tx_data_variants::ValSetUpdateVext::try_from(&tx) {
                 Ok(signed_valset_upd) => break signed_valset_upd,
@@ -2158,7 +2158,7 @@ mod shell_tests {
         // attempt to receive vote extension tx aggregating
         // all expired events
         let serialized_tx = broadcaster_rx.blocking_recv().unwrap();
-        let tx = Tx::try_from(&serialized_tx[..]).unwrap();
+        let tx = Tx::try_from_bytes(&serialized_tx[..]).unwrap();
 
         // check data inside tx
         let vote_extension =

crates/node/src/shell/prepare_proposal.rs

@@ -289,7 +289,7 @@ where
     H: StorageHasher + Sync + 'static,
     CA: 'static + WasmCacheAccess + Sync,
 {
-    let tx = Tx::try_from(tx_bytes).map_err(|_| ())?;
+    let tx = Tx::try_from_bytes(tx_bytes).map_err(|_| ())?;
     let wrapper = tx.header.wrapper().ok_or(())?;
 
     // If tx doesn't have an expiration it is valid. If time cannot be
@@ -754,7 +754,7 @@ mod test_prepare_proposal {
         assert_eq!(rsp.txs.len(), 1);
 
         let tx_bytes = rsp.txs.remove(0);
-        let got = Tx::try_from(&tx_bytes[..]).unwrap();
+        let got = Tx::try_from_bytes(&tx_bytes[..]).unwrap();
         let eth_tx_data = (&got).try_into().expect("Test failed");
         let rsp_ext = match eth_tx_data {
             EthereumTxData::EthEventsVext(ext) => ext,
@@ -1371,7 +1371,7 @@ mod test_prepare_proposal {
             };
             let proposed_txs =
                 shell.prepare_proposal(req).txs.into_iter().map(|tx_bytes| {
-                    Tx::try_from(tx_bytes.as_ref()).expect("Test failed")
+                    Tx::try_from_bytes(tx_bytes.as_ref()).expect("Test failed")
                 });
             // since no events with valid nonces are contained in the vote
             // extension, we drop it from the proposal
@@ -1419,7 +1419,7 @@ mod test_prepare_proposal {
             };
             let proposed_txs =
                 shell.prepare_proposal(req).txs.into_iter().map(|tx_bytes| {
-                    Tx::try_from(tx_bytes.as_ref()).expect("Test failed")
+                    Tx::try_from_bytes(tx_bytes.as_ref()).expect("Test failed")
                 });
             // find the event with the good nonce
             let mut ext = 'ext: {

crates/node/src/shell/process_proposal.rs

@@ -238,7 +238,7 @@ where
             };
         }
 
-        let maybe_tx = Tx::try_from(tx_bytes).map_or_else(
+        let maybe_tx = Tx::try_from_bytes(tx_bytes).map_or_else(
             |err| {
                 tracing::debug!(
                     ?err,

crates/node/src/shell/vote_extensions.rs

@@ -124,7 +124,7 @@ where
     ) -> DrainFilter<'shell, TxBytes, impl FnMut(&mut TxBytes) -> bool + 'shell>
     {
         drain_filter_polyfill::VecExt::drain_filter(txs, move |tx_bytes| {
-            let tx = match Tx::try_from(tx_bytes.as_ref()) {
+            let tx = match Tx::try_from_bytes(tx_bytes.as_ref()) {
                 Ok(tx) => tx,
                 Err(err) => {
                     tracing::warn!(

crates/sdk/src/masp/utilities.rs

@@ -110,8 +110,9 @@ impl<C: Client + Send + Sync> MaspClient for LedgerMaspClient<C> {
                 masp_refs,
             } in txs_results
             {
-                let tx = Tx::try_from(block[tx_index.0 as usize].as_ref())
-                    .map_err(|e| Error::Other(e.to_string()))?;
+                let tx =
+                    Tx::try_from_bytes(block[tx_index.0 as usize].as_ref())
+                        .map_err(|e| Error::Other(e.to_string()))?;
                 let extracted_masp_txs = extract_masp_tx(&tx, &masp_refs)
                     .map_err(|e| Error::Other(e.to_string()))?;
 

crates/sdk/src/tx.rs

@@ -200,7 +200,7 @@ pub fn dump_tx<IO: Io>(io: &IO, args: &args::Tx, tx: Tx) {
             ));
             let out = File::create(&tx_path)
                 .expect("Should be able to create a file to dump tx");
-            serde_json::to_writer_pretty(out, &tx)
+            tx.to_writer_json(out)
                 .expect("Should be able to write to file.");
             display_line!(
                 io,
@@ -3734,7 +3734,7 @@ pub async fn build_custom(
     let fee_amount = validate_fee(context, tx_args).await?;
 
     let mut tx = if let Some(serialized_tx) = serialized_tx {
-        Tx::deserialize(serialized_tx.as_ref()).map_err(|_| {
+        Tx::try_from_json_bytes(serialized_tx.as_ref()).map_err(|_| {
             Error::Other(
                 "Invalid tx deserialization. Please make sure you are passing \
                  a file in .tx format, typically produced from using the \

crates/tests/src/integration/ledger_tests.rs

@@ -27,7 +27,7 @@ use namada_sdk::collections::HashMap;
 use namada_sdk::migrations;
 use namada_sdk::queries::RPC;
 use namada_sdk::token::{self, DenominatedAmount};
-use namada_sdk::tx::{TX_TRANSFER_WASM, VP_USER_WASM};
+use namada_sdk::tx::{self, Tx, TX_TRANSFER_WASM, VP_USER_WASM};
 use namada_test_utils::TestWasms;
 use test_log::test;
 
@@ -1901,9 +1901,9 @@ fn enforce_fee_payment() -> Result<()> {
 
     let mut txs = vec![];
     for bytes in txs_bytes {
-        let mut tx = namada_sdk::tx::Tx::deserialize(&bytes).unwrap();
+        let mut tx = Tx::try_from_json_bytes(&bytes).unwrap();
         tx.add_wrapper(
-            namada_sdk::tx::data::wrapper::Fee {
+            tx::data::wrapper::Fee {
                 amount_per_gas_unit: DenominatedAmount::native(
                     token::Amount::native_whole(1),
                 ),

crates/tests/src/integration/masp.rs

@@ -16,7 +16,8 @@ use namada_sdk::state::{StorageRead, StorageWrite};
 use namada_sdk::time::DateTimeUtc;
 use namada_sdk::token::storage_key::masp_token_map_key;
 use namada_sdk::token::{self, DenominatedAmount};
-use namada_sdk::DEFAULT_GAS_LIMIT;
+use namada_sdk::tx::Tx;
+use namada_sdk::{tx, DEFAULT_GAS_LIMIT};
 use test_log::test;
 
 use super::setup;
@@ -1503,9 +1504,9 @@ fn multiple_unfetched_txs_same_block() -> Result<()> {
         .clone();
     let mut txs = vec![];
     for bytes in txs_bytes {
-        let mut tx = namada_sdk::tx::Tx::deserialize(&bytes).unwrap();
+        let mut tx = Tx::try_from_json_bytes(&bytes).unwrap();
         tx.add_wrapper(
-            namada_sdk::tx::data::wrapper::Fee {
+            tx::data::wrapper::Fee {
                 amount_per_gas_unit: DenominatedAmount::native(1.into()),
                 token: native_token.clone(),
             },
@@ -1640,7 +1641,7 @@ fn expired_masp_tx() -> Result<()> {
         .in_mem()
         .native_token
         .clone();
-    let mut tx = namada_sdk::tx::Tx::deserialize(&tx_bytes).unwrap();
+    let mut tx = Tx::try_from_json_bytes(&tx_bytes).unwrap();
     // Remove the expiration field to avoid a failure because of it, we only
     // want to check the expiration in the masp vp
     tx.header.expiration = None;

crates/tx/src/types.rs

@@ -1,9 +1,9 @@
 use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::hash::Hash;
+use std::io;
 use std::ops::{Bound, RangeBounds};
 
-use data_encoding::HEXUPPER;
 use masp_primitives::transaction::Transaction;
 use namada_account::AccountPublicKeysMap;
 use namada_core::address::Address;
@@ -34,8 +34,6 @@ use crate::{
 pub enum DecodeError {
     #[error("Invalid signature index bytes: {0}")]
     InvalidEncoding(std::io::Error),
-    #[error("Invalid signature index JSON string")]
-    InvalidJsonString,
     #[error("Invalid signature index: {0}")]
     InvalidHex(data_encoding::DecodeError),
     #[error("Error decoding a transaction from bytes: {0}")]
@@ -73,6 +71,7 @@ pub enum TxError {
     BorshSchema,
     Serialize,
     Deserialize,
+    PartialEq,
 )]
 pub struct Tx {
     /// Type indicating how to process transaction
@@ -86,12 +85,7 @@ impl TryFrom<&[u8]> for Tx {
     type Error = DecodeError;
 
     fn try_from(tx_bytes: &[u8]) -> Result<Self, DecodeError> {
-        use prost::Message;
-
-        let tx = proto::Tx::decode(tx_bytes)
-            .map_err(DecodeError::TxDecodingError)?;
-        BorshDeserialize::try_from_slice(&tx.data)
-            .map_err(DecodeError::InvalidEncoding)
+        Tx::try_from_bytes(tx_bytes)
     }
 }
 
@@ -125,19 +119,21 @@ impl Tx {
         }
     }
 
-    /// Serialize tx to hex string
-    pub fn serialize(&self) -> String {
-        let tx_bytes = self.serialize_to_vec();
-        HEXUPPER.encode(&tx_bytes)
+    /// Serialize tx to pretty JSON into an I/O stream
+    ///
+    /// For protobuf encoding, see `to_bytes/try_to_bytes`.
+    pub fn to_writer_json<W>(&self, writer: W) -> serde_json::Result<()>
+    where
+        W: io::Write,
+    {
+        serde_json::to_writer_pretty(writer, self)
     }
 
-    /// Deserialize tx from json
-    pub fn deserialize(data: &[u8]) -> Result<Self, DecodeError> {
-        if let Ok(tx) = serde_json::from_slice::<Tx>(data) {
-            Ok(tx)
-        } else {
-            Err(DecodeError::InvalidJsonString)
-        }
+    /// Deserialize tx from JSON string bytes
+    ///
+    /// For protobuf decoding, see `try_from_bytes`.
+    pub fn try_from_json_bytes(data: &[u8]) -> serde_json::Result<Self> {
+        serde_json::from_slice::<Tx>(data)
     }
 
     /// Add new default commitments to the transaction. Returns false if the
@@ -341,7 +337,9 @@ impl Tx {
         }
     }
 
-    /// Convert this transaction into protobufs bytes
+    /// Convert this transaction into protobufs bytes.
+    ///
+    /// For JSON encoding see `to_writer_json`.
     pub fn to_bytes(&self) -> Vec<u8> {
         use prost::Message;
 
@@ -355,6 +353,8 @@ impl Tx {
     }
 
     /// Convert this transaction into protobufs bytes
+    ///
+    /// For JSON encoding see `to_writer_json`.
     pub fn try_to_bytes(&self) -> std::io::Result<Vec<u8>> {
         use prost::Message;
 
@@ -368,6 +368,18 @@ impl Tx {
         Ok(bytes)
     }
 
+    /// Try to deserialize a tx from protobuf bytes
+    ///
+    /// For JSON decoding see `try_from_json_bytes`.
+    pub fn try_from_bytes(tx_bytes: &[u8]) -> Result<Self, DecodeError> {
+        use prost::Message;
+
+        let tx = proto::Tx::decode(tx_bytes)
+            .map_err(DecodeError::TxDecodingError)?;
+        BorshDeserialize::try_from_slice(&tx.data)
+            .map_err(DecodeError::InvalidEncoding)
+    }
+
     /// Verify that the section with the given hash has been signed by the given
     /// public key
     pub fn verify_signatures<F>(
@@ -949,13 +961,34 @@ mod test {
 
         for serialized_tx in serialized_txs {
             let raw_bytes = HEXLOWER.decode(serialized_tx.as_bytes()).unwrap();
-            let tx = Tx::try_from(raw_bytes.as_ref()).unwrap();
+            let tx = Tx::try_from_bytes(raw_bytes.as_ref()).unwrap();
 
             assert_eq!(tx.try_to_bytes().unwrap(), raw_bytes);
             assert_eq!(tx.to_bytes(), raw_bytes);
         }
     }
 
+    #[test]
+    fn test_tx_protobuf_serialization() {
+        let tx = Tx::default();
+
+        let buffer = tx.to_bytes();
+
+        let deserialized = Tx::try_from_bytes(&buffer).unwrap();
+        assert_eq!(tx, deserialized);
+    }
+
+    #[test]
+    fn test_tx_json_serialization() {
+        let tx = Tx::default();
+
+        let mut buffer = vec![];
+        tx.to_writer_json(&mut buffer).unwrap();
+
+        let deserialized = Tx::try_from_json_bytes(&buffer).unwrap();
+        assert_eq!(tx, deserialized);
+    }
+
     #[test]
     fn test_wrapper_tx_signing() {
         let sk1 = key::testing::keypair_1();