From 2a7651879e2bed4ed0dbdcf3925150577e52ecc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 23 Sep 2024 11:16:40 +0100 Subject: [PATCH] tx: improve the serialization api --- crates/apps_lib/src/client/tx.rs | 3 +- crates/apps_lib/src/client/utils.rs | 3 +- crates/node/src/dry_run_tx.rs | 2 +- crates/node/src/shell/finalize_block.rs | 21 +++--- crates/node/src/shell/mod.rs | 6 +- crates/node/src/shell/prepare_proposal.rs | 8 +- crates/node/src/shell/process_proposal.rs | 2 +- crates/node/src/shell/vote_extensions.rs | 2 +- crates/sdk/src/masp/utilities.rs | 5 +- crates/sdk/src/tx.rs | 4 +- crates/tests/src/integration/ledger_tests.rs | 6 +- crates/tests/src/integration/masp.rs | 9 ++- crates/tx/src/types.rs | 77 ++++++++++++++------ 13 files changed, 93 insertions(+), 55 deletions(-) diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 08344641ef..2384f21b2b 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -1069,7 +1069,8 @@ pub async fn sign_tx( where ::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."); diff --git a/crates/apps_lib/src/client/utils.rs b/crates/apps_lib/src/client/utils.rs index 8e8c7c6fc5..1bb53164b6 100644 --- a/crates/apps_lib/src/client/utils.rs +++ b/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."); diff --git a/crates/node/src/dry_run_tx.rs b/crates/node/src/dry_run_tx.rs index d5efc49921..1957045f0b 100644 --- a/crates/node/src/dry_run_tx.rs +++ b/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)?; diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 48165a8548..b71c18c730 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/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"); diff --git a/crates/node/src/shell/mod.rs b/crates/node/src/shell/mod.rs index ea27241312..6115fcb789 100644 --- a/crates/node/src/shell/mod.rs +++ b/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 = diff --git a/crates/node/src/shell/prepare_proposal.rs b/crates/node/src/shell/prepare_proposal.rs index f5271c1115..f791ffb606 100644 --- a/crates/node/src/shell/prepare_proposal.rs +++ b/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: { diff --git a/crates/node/src/shell/process_proposal.rs b/crates/node/src/shell/process_proposal.rs index 1485b06f15..53ca249f6f 100644 --- a/crates/node/src/shell/process_proposal.rs +++ b/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, diff --git a/crates/node/src/shell/vote_extensions.rs b/crates/node/src/shell/vote_extensions.rs index ed0925f6b8..6b5958debe 100644 --- a/crates/node/src/shell/vote_extensions.rs +++ b/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!( diff --git a/crates/sdk/src/masp/utilities.rs b/crates/sdk/src/masp/utilities.rs index ebe0c83b0d..95c9565005 100644 --- a/crates/sdk/src/masp/utilities.rs +++ b/crates/sdk/src/masp/utilities.rs @@ -110,8 +110,9 @@ impl MaspClient for LedgerMaspClient { 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()))?; diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 9e74fd3cc5..3e03cac17e 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -200,7 +200,7 @@ pub fn dump_tx(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 \ diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 7d6322d448..dac6f9a52b 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/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), ), diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 07690d1e0e..b9e5442cfc 100644 --- a/crates/tests/src/integration/masp.rs +++ b/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; diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 3c79ad8d62..4eb5ba7434 100644 --- a/crates/tx/src/types.rs +++ b/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 { - 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(&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 { - if let Ok(tx) = serde_json::from_slice::(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 { + serde_json::from_slice::(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 { 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> { 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 { + 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( @@ -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();