feat!: provide a compact form of TransactionInput (#3460)

Description
---

This PR updates the TransactionInput struct to support containing the duplicated Output data of the output it is spending OR just the hash of the output. This is done in the original TransactionInput as opposed to creating a new struct to allow for easier reuse of all the code that already supports the TransactionInput.

Methods are provided to:
- Create a new compact or full TransactionInput
- Return a compact form of the current TransactionInput
- Provide the output data that is referenced in the TransactionInput

This PR also updates the RPC definitions and all the conversion methods to support both forms of the TransactionInput

Motivation and Context
---

The reason for this change is to reduce the amount of duplicate data stored in the Blockchain db and sent across the wire during Block sync. Currently the input is stored with all the duplicated data from the output it is spending along side the spent output in the database which is a duplication of data. Now the TransactionInput is converted into its compact form with just a reference to the spent output before being stored in the database and before being sent across the wire. When the Input is read from the database or received then the associated output data is retrieved from the local datastore.

TODO: apply the compact form of the TransactionInput to transactions submitted to the base node mempool. This will further reduce data sent from wallets to the base nodes.

How Has This Been Tested?
---
cargo test

applications/tari_app_grpc/proto/types.proto

@@ -169,6 +169,8 @@ message TransactionInput {
     ComSignature script_signature = 7;
     // The offset public key, K_O
     bytes sender_offset_public_key = 8;
+    // The hash of the output this input is spending
+    bytes output_hash = 9;
 }
 
 // Output for a transaction, defining the new ownership of coins that are being transferred. The commitment is a

applications/tari_app_grpc/src/conversions/aggregate_body.rs

@@ -27,14 +27,16 @@ use tari_utilities::convert::try_convert_all;
 
 use crate::tari_rpc as grpc;
 
-impl From<AggregateBody> for grpc::AggregateBody {
-    fn from(source: AggregateBody) -> Self {
-        Self {
+impl TryFrom<AggregateBody> for grpc::AggregateBody {
+    type Error = String;
+
+    fn try_from(source: AggregateBody) -> Result<Self, Self::Error> {
+        Ok(Self {
             inputs: source
                 .inputs()
                 .iter()
-                .map(|input| grpc::TransactionInput::from(input.clone()))
-                .collect(),
+                .map(|input| grpc::TransactionInput::try_from(input.clone()))
+                .collect::<Result<Vec<grpc::TransactionInput>, _>>()?,
             outputs: source
                 .outputs()
                 .iter()
@@ -45,7 +47,7 @@ impl From<AggregateBody> for grpc::AggregateBody {
                 .iter()
                 .map(|kernel| grpc::TransactionKernel::from(kernel.clone()))
                 .collect(),
-        }
+        })
     }
 }
 

applications/tari_app_grpc/src/conversions/block.rs

@@ -26,12 +26,14 @@ use tari_core::blocks::Block;
 
 use crate::tari_rpc as grpc;
 
-impl From<tari_core::blocks::Block> for grpc::Block {
-    fn from(block: Block) -> Self {
-        Self {
-            body: Some(block.body.into()),
+impl TryFrom<tari_core::blocks::Block> for grpc::Block {
+    type Error = String;
+
+    fn try_from(block: Block) -> Result<Self, Self::Error> {
+        Ok(Self {
+            body: Some(block.body.try_into()?),
             header: Some(block.header.into()),
-        }
+        })
     }
 }
 

applications/tari_app_grpc/src/conversions/historical_block.rs

@@ -20,7 +20,7 @@
 // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 
 use tari_core::{blocks::HistoricalBlock, chain_storage::ChainStorageError};
 
@@ -32,7 +32,11 @@ impl TryFrom<HistoricalBlock> for grpc::HistoricalBlock {
     fn try_from(hb: HistoricalBlock) -> Result<Self, Self::Error> {
         Ok(Self {
             confirmations: hb.confirmations,
-            block: Some(hb.try_into_block()?.into()),
+            block: Some(
+                hb.try_into_block()?
+                    .try_into()
+                    .map_err(ChainStorageError::ConversionError)?,
+            ),
         })
     }
 }

applications/tari_app_grpc/src/conversions/new_block_template.rs

@@ -31,8 +31,10 @@ use tari_utilities::ByteArray;
 
 use crate::tari_rpc as grpc;
 
-impl From<NewBlockTemplate> for grpc::NewBlockTemplate {
-    fn from(block: NewBlockTemplate) -> Self {
+impl TryFrom<NewBlockTemplate> for grpc::NewBlockTemplate {
+    type Error = String;
+
+    fn try_from(block: NewBlockTemplate) -> Result<Self, Self::Error> {
         let header = grpc::NewBlockHeaderTemplate {
             version: block.header.version as u32,
             height: block.header.height,
@@ -44,14 +46,14 @@ impl From<NewBlockTemplate> for grpc::NewBlockTemplate {
                 pow_data: block.header.pow.pow_data,
             }),
         };
-        Self {
+        Ok(Self {
             body: Some(grpc::AggregateBody {
                 inputs: block
                     .body
                     .inputs()
                     .iter()
-                    .map(|input| grpc::TransactionInput::from(input.clone()))
-                    .collect(),
+                    .map(|input| grpc::TransactionInput::try_from(input.clone()))
+                    .collect::<Result<Vec<grpc::TransactionInput>, _>>()?,
                 outputs: block
                     .body
                     .outputs()
@@ -66,7 +68,7 @@ impl From<NewBlockTemplate> for grpc::NewBlockTemplate {
                     .collect(),
             }),
             header: Some(header),
-        }
+        })
     }
 }
 impl TryFrom<grpc::NewBlockTemplate> for NewBlockTemplate {

applications/tari_app_grpc/src/conversions/transaction.rs

@@ -29,13 +29,15 @@ use tari_utilities::ByteArray;
 
 use crate::tari_rpc as grpc;
 
-impl From<Transaction> for grpc::Transaction {
-    fn from(source: Transaction) -> Self {
-        Self {
+impl TryFrom<Transaction> for grpc::Transaction {
+    type Error = String;
+
+    fn try_from(source: Transaction) -> Result<Self, Self::Error> {
+        Ok(Self {
             offset: Vec::from(source.offset.as_bytes()),
-            body: Some(source.body.into()),
+            body: Some(source.body.try_into()?),
             script_offset: Vec::from(source.script_offset.as_bytes()),
-        }
+        })
     }
 }
 

applications/tari_app_grpc/src/conversions/transaction_input.rs

@@ -26,7 +26,7 @@ use tari_common_types::types::{Commitment, PublicKey};
 use tari_core::transactions::transaction::TransactionInput;
 use tari_crypto::{
     script::{ExecutionStack, TariScript},
-    tari_utilities::{ByteArray, Hashable},
+    tari_utilities::ByteArray,
 };
 
 use crate::tari_rpc as grpc;
@@ -35,51 +35,95 @@ impl TryFrom<grpc::TransactionInput> for TransactionInput {
     type Error = String;
 
     fn try_from(input: grpc::TransactionInput) -> Result<Self, Self::Error> {
-        let features = input
-            .features
-            .map(TryInto::try_into)
-            .ok_or_else(|| "transaction output features not provided".to_string())??;
-
-        let commitment = Commitment::from_bytes(&input.commitment)
-            .map_err(|err| format!("Could not convert input commitment:{}", err))?;
-
         let script_signature = input
             .script_signature
             .ok_or_else(|| "script_signature not provided".to_string())?
             .try_into()
             .map_err(|_| "script_signature could not be converted".to_string())?;
 
-        let sender_offset_public_key =
-            PublicKey::from_bytes(input.sender_offset_public_key.as_bytes()).map_err(|err| format!("{:?}", err))?;
-        let script = TariScript::from_bytes(input.script.as_slice()).map_err(|err| format!("{:?}", err))?;
-        let input_data = ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?;
+        // Check if the received Transaction input is in compact form or not
+        if !input.commitment.is_empty() {
+            let commitment = Commitment::from_bytes(&input.commitment).map_err(|e| e.to_string())?;
+            let features = input
+                .features
+                .map(TryInto::try_into)
+                .ok_or_else(|| "transaction output features not provided".to_string())??;
 
-        Ok(Self {
-            features,
-            commitment,
-            script,
-            input_data,
-            script_signature,
-            sender_offset_public_key,
-        })
+            let sender_offset_public_key =
+                PublicKey::from_bytes(input.sender_offset_public_key.as_bytes()).map_err(|err| format!("{:?}", err))?;
+
+            Ok(TransactionInput::new_with_output_data(
+                features,
+                commitment,
+                TariScript::from_bytes(input.script.as_slice()).map_err(|err| format!("{:?}", err))?,
+                ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?,
+                script_signature,
+                sender_offset_public_key,
+            ))
+        } else {
+            if input.output_hash.is_empty() {
+                return Err("Compact Transaction Input does not contain `output_hash`".to_string());
+            }
+            Ok(TransactionInput::new_with_output_hash(
+                input.output_hash,
+                ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?,
+                script_signature,
+            ))
+        }
     }
 }
 
-impl From<TransactionInput> for grpc::TransactionInput {
-    fn from(input: TransactionInput) -> Self {
-        let hash = input.hash();
-        Self {
-            features: Some(input.features.into()),
-            commitment: Vec::from(input.commitment.as_bytes()),
-            hash,
-            script: input.script.as_bytes(),
-            input_data: input.input_data.as_bytes(),
-            script_signature: Some(grpc::ComSignature {
-                public_nonce_commitment: Vec::from(input.script_signature.public_nonce().as_bytes()),
-                signature_u: Vec::from(input.script_signature.u().as_bytes()),
-                signature_v: Vec::from(input.script_signature.v().as_bytes()),
-            }),
-            sender_offset_public_key: input.sender_offset_public_key.as_bytes().to_vec(),
+impl TryFrom<TransactionInput> for grpc::TransactionInput {
+    type Error = String;
+
+    fn try_from(input: TransactionInput) -> Result<Self, Self::Error> {
+        let script_signature = Some(grpc::ComSignature {
+            public_nonce_commitment: Vec::from(input.script_signature.public_nonce().as_bytes()),
+            signature_u: Vec::from(input.script_signature.u().as_bytes()),
+            signature_v: Vec::from(input.script_signature.v().as_bytes()),
+        });
+        if input.is_compact() {
+            let output_hash = input.output_hash();
+            Ok(Self {
+                features: None,
+                commitment: Vec::new(),
+                hash: Vec::new(),
+                script: Vec::new(),
+                input_data: Vec::new(),
+                script_signature,
+                sender_offset_public_key: Vec::new(),
+                output_hash,
+            })
+        } else {
+            let features = input
+                .features()
+                .map_err(|_| "Non-compact Transaction input should contain features".to_string())?;
+
+            Ok(Self {
+                features: Some(features.clone().into()),
+                commitment: input
+                    .commitment()
+                    .map_err(|_| "Non-compact Transaction input should contain commitment".to_string())?
+                    .clone()
+                    .as_bytes()
+                    .to_vec(),
+                hash: input
+                    .canonical_hash()
+                    .map_err(|_| "Non-compact Transaction input should be able to be hashed".to_string())?,
+
+                script: input
+                    .script()
+                    .map_err(|_| "Non-compact Transaction input should contain script".to_string())?
+                    .as_bytes(),
+                input_data: input.input_data.as_bytes(),
+                script_signature,
+                sender_offset_public_key: input
+                    .sender_offset_public_key()
+                    .map_err(|_| "Non-compact Transaction input should contain sender_offset_public_key".to_string())?
+                    .as_bytes()
+                    .to_vec(),
+                output_hash: Vec::new(),
+            })
         }
     }
 }

applications/tari_base_node/src/grpc/base_node_grpc_server.rs

@@ -234,9 +234,26 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
                 Ok(data) => data,
             };
             for transaction in transactions.unconfirmed_pool {
+                let transaction = match tari_rpc::Transaction::try_from(transaction) {
+                    Ok(t) => t,
+                    Err(e) => {
+                        warn!(
+                            target: LOG_TARGET,
+                            "Error sending converting transaction for GRPC:  {}", e
+                        );
+                        match tx.send(Err(Status::internal("Error converting transaction"))).await {
+                            Ok(_) => (),
+                            Err(send_err) => {
+                                warn!(target: LOG_TARGET, "Error sending error to GRPC client: {}", send_err)
+                            },
+                        }
+                        return;
+                    },
+                };
+
                 match tx
                     .send(Ok(tari_rpc::GetMempoolTransactionsResponse {
-                        transaction: Some(transaction.into()),
+                        transaction: Some(transaction),
                     }))
                     .await
                 {
@@ -638,7 +655,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
                 total_fees: new_template.total_fees.into(),
                 algo: Some(tari_rpc::PowAlgo { pow_algo: pow }),
             }),
-            new_block_template: Some(new_template.into()),
+            new_block_template: Some(new_template.try_into().map_err(Status::internal)?),
 
             initial_sync_achieved: (*status_watch.borrow()).bootstrapped,
         };
@@ -677,7 +694,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
         // construct response
         let block_hash = new_block.hash();
         let mining_hash = new_block.header.merged_mining_hash();
-        let block: Option<tari_rpc::Block> = Some(new_block.into());
+        let block: Option<tari_rpc::Block> = Some(new_block.try_into().map_err(Status::internal)?);
 
         let response = tari_rpc::GetNewBlockResult {
             block_hash,

applications/tari_console_wallet/src/automation/commands.rs

@@ -53,6 +53,7 @@ use tari_crypto::{
 };
 use tari_utilities::hex::Hex;
 use tari_wallet::{
+    error::WalletError,
     output_manager_service::handle::OutputManagerHandle,
     transaction_service::handle::{TransactionEvent, TransactionServiceHandle},
     WalletSqlite,
@@ -941,7 +942,10 @@ fn write_utxos_to_csv_file(utxos: Vec<UnblindedOutput>, file_path: String) -> Re
             i + 1,
             utxo.value.0,
             utxo.spending_key.to_hex(),
-            utxo.as_transaction_input(&factory)?.commitment.to_hex(),
+            utxo.as_transaction_input(&factory)?
+                .commitment()
+                .map_err(|e| CommandError::WalletError(WalletError::TransactionError(e)))?
+                .to_hex(),
             utxo.features.flags,
             utxo.features.maturity,
             utxo.script.to_hex(),

applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs

@@ -190,7 +190,7 @@ impl wallet_server::Wallet for WalletGrpcServer {
 
         match response {
             Ok(resp) => Ok(Response::new(GetCoinbaseResponse {
-                transaction: Some(resp.into()),
+                transaction: Some(resp.try_into().map_err(Status::internal)?),
             })),
             Err(err) => Err(Status::unknown(err.to_string())),
         }

applications/tari_console_wallet/src/ui/state/app_state.rs

@@ -969,8 +969,10 @@ pub struct CompletedTransactionInfo {
 fn first_unique_id(tx: &CompletedTransaction) -> String {
     let body = tx.transaction.body();
     for input in body.inputs() {
-        if let Some(ref unique_id) = input.features.unique_id {
-            return unique_id.to_hex();
+        if let Ok(features) = input.features() {
+            if let Some(ref unique_id) = features.unique_id {
+                return unique_id.to_hex();
+            }
         }
     }
     for output in body.outputs() {

applications/tari_merge_mining_proxy/src/common/merge_mining.rs

@@ -20,7 +20,7 @@
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 
 use tari_app_grpc::tari_rpc as grpc;
 use tari_core::{
@@ -42,5 +42,5 @@ pub fn add_coinbase(
         .map_err(MmProxyError::MissingDataError)?;
     block_template.body.add_output(output);
     block_template.body.add_kernel(kernel);
-    Ok(block_template.into())
+    block_template.try_into().map_err(MmProxyError::ConversionError)
 }

applications/tari_merge_mining_proxy/src/error.rs

@@ -80,6 +80,8 @@ pub enum MmProxyError {
     InvalidHeaderValue(#[from] InvalidHeaderValue),
     #[error("Block was lost due to a failed precondition, and should be retried")]
     FailedPreconditionBlockLostRetry,
+    #[error("Could not convert data:{0}")]
+    ConversionError(String),
     #[error("No reachable servers in configuration")]
     ServersUnavailable,
 }