refactor(tx-builder): allow building unsigned transactions

applications/tari_dan_wallet_cli/src/command/transaction.rs

@@ -263,6 +263,7 @@ pub async fn handle_submit(args: SubmitArgs, client: &mut WalletDaemonClient) ->
     }];
 
     let request = TransactionSubmitRequest {
+        transaction: None,
         signing_key_index: None,
         fee_instructions,
         instructions,
@@ -293,6 +294,7 @@ async fn handle_submit_manifest(
     }
 
     let request = TransactionSubmitRequest {
+        transaction: None,
         signing_key_index: None,
         fee_instructions: instructions
             .fee_instructions

applications/tari_dan_wallet_daemon/src/handlers/transaction.rs

@@ -1,6 +1,6 @@
 //   Copyright 2023 The Tari Project
 //   SPDX-License-Identifier: BSD-3-Clause
-use std::{collections::HashSet, convert::TryFrom, time::Duration};
+use std::{collections::HashSet, time::Duration};
 
 use anyhow::anyhow;
 use futures::{future, future::Either};
@@ -43,43 +43,49 @@ pub async fn handle_submit_instruction(
     token: Option<String>,
     req: CallInstructionRequest,
 ) -> Result<TransactionSubmitResponse, anyhow::Error> {
-    let mut instructions = req.instructions;
+    let mut builder = Transaction::builder().with_instructions(req.instructions);
+
     if let Some(dump_account) = req.dump_outputs_into {
-        instructions.push(Instruction::PutLastInstructionOutputOnWorkspace {
-            key: b"bucket".to_vec(),
-        });
         let AccountGetResponse {
             account: dump_account, ..
         } = accounts::handle_get(context, token.clone(), AccountGetRequest {
             name_or_address: dump_account,
         })
         .await?;
-        instructions.push(Instruction::CallMethod {
-            component_address: dump_account.address.as_component_address().unwrap(),
-            method: "deposit".to_string(),
-            args: args![Variable("bucket")],
-        });
+
+        builder = builder.put_last_instruction_output_on_workspace("bucket").call_method(
+            dump_account.address.as_component_address().unwrap(),
+            "deposit",
+            args![Variable("bucket")],
+        );
     }
     let AccountGetResponse {
         account: fee_account, ..
     } = accounts::handle_get(context, token.clone(), AccountGetRequest {
         name_or_address: req.fee_account,
     })
     .await?;
+
+    let transaction = builder
+        .fee_transaction_pay_from_component(
+            fee_account.address.as_component_address().unwrap(),
+            req.max_fee.try_into()?,
+        )
+        .with_min_epoch(req.min_epoch.map(Epoch))
+        .with_max_epoch(req.max_epoch.map(Epoch))
+        .build_unsigned_transaction();
+
     let request = TransactionSubmitRequest {
+        transaction: Some(transaction),
         signing_key_index: Some(fee_account.key_index),
-        fee_instructions: vec![Instruction::CallMethod {
-            component_address: fee_account.address.as_component_address().unwrap(),
-            method: "pay_fee".to_string(),
-            args: args![Amount::try_from(req.max_fee)?],
-        }],
-        instructions,
+        fee_instructions: vec![],
+        instructions: vec![],
         inputs: req.inputs,
         override_inputs: req.override_inputs.unwrap_or_default(),
         is_dry_run: req.is_dry_run,
         proof_ids: vec![],
-        min_epoch: req.min_epoch.map(Epoch),
-        max_epoch: req.max_epoch.map(Epoch),
+        min_epoch: None,
+        max_epoch: None,
     };
     handle_submit(context, token, request).await
 }
@@ -115,13 +121,20 @@ pub async fn handle_submit(
         [req.inputs, loaded_dependent_substates].concat()
     };
 
-    let transaction = Transaction::builder()
-        .with_instructions(req.instructions)
-        .with_fee_instructions(req.fee_instructions)
-        .with_min_epoch(req.min_epoch)
-        .with_max_epoch(req.max_epoch)
-        .sign(&key.key)
-        .build();
+    let transaction = if let Some(transaction) = req.transaction {
+        Transaction::builder()
+            .with_unsigned_transaction(transaction)
+            .sign(&key.key)
+            .build()
+    } else {
+        Transaction::builder()
+            .with_instructions(req.instructions)
+            .with_fee_instructions(req.fee_instructions)
+            .with_min_epoch(req.min_epoch)
+            .with_max_epoch(req.max_epoch)
+            .sign(&key.key)
+            .build()
+    };
 
     for proof_id in req.proof_ids {
         // update the proofs table with the corresponding transaction hash

applications/tari_validator_node/src/p2p/services/mempool/validators/before/signature.rs

@@ -2,7 +2,7 @@
 //    SPDX-License-Identifier: BSD-3-Clause
 
 use async_trait::async_trait;
-use tari_transaction::{Transaction, TransactionSignatureFields};
+use tari_transaction::Transaction;
 
 use crate::p2p::services::mempool::{MempoolError, Validator};
 
@@ -14,16 +14,7 @@ impl Validator<Transaction> for TransactionSignatureValidator {
     type Error = MempoolError;
 
     async fn validate(&self, transaction: &Transaction) -> Result<(), MempoolError> {
-        let signature_fields = TransactionSignatureFields {
-            fee_instructions: transaction.fee_instructions().to_vec(),
-            instructions: transaction.instructions().to_vec(),
-            inputs: transaction.inputs().to_vec(),
-            input_refs: transaction.input_refs().to_vec(),
-            min_epoch: transaction.min_epoch(),
-            max_epoch: transaction.max_epoch(),
-        };
-
-        if !transaction.signature().verify(signature_fields) {
+        if !transaction.signature().verify(&transaction.into()) {
             return Err(MempoolError::InvalidSignature);
         }
 

clients/wallet_daemon_client/src/types.rs

@@ -45,7 +45,7 @@ use tari_template_lib::{
     models::{Amount, ConfidentialOutputProof, NonFungibleId, ResourceAddress},
     prelude::{ComponentAddress, ConfidentialWithdrawProof, ResourceType},
 };
-use tari_transaction::{SubstateRequirement, Transaction, TransactionId};
+use tari_transaction::{SubstateRequirement, Transaction, TransactionId, UnsignedTransaction};
 #[cfg(feature = "ts")]
 use ts_rs::TS;
 
@@ -87,22 +87,25 @@ pub struct CallInstructionRequest {
     pub max_epoch: Option<u64>,
 }
 
-#[derive(Debug, Clone, Deserialize, Serialize)]
+#[derive(Debug, Clone, Default, Deserialize, Serialize)]
 #[cfg_attr(
     feature = "ts",
     derive(TS),
     ts(export, export_to = "../../bindings/src/types/wallet-daemon-client/")
 )]
 pub struct TransactionSubmitRequest {
+    // TODO: make this mandatory once we remove the rest of the deprecated fields
+    pub transaction: Option<UnsignedTransaction>,
     #[cfg_attr(feature = "ts", ts(type = "number | null"))]
     pub signing_key_index: Option<u64>,
-    pub fee_instructions: Vec<Instruction>,
-    pub instructions: Vec<Instruction>,
     pub inputs: Vec<SubstateRequirement>,
     pub override_inputs: bool,
     pub is_dry_run: bool,
     #[cfg_attr(feature = "ts", ts(type = "Array<number>"))]
     pub proof_ids: Vec<ConfidentialProofId>,
+    // TODO: remove the following fields
+    pub fee_instructions: Vec<Instruction>,
+    pub instructions: Vec<Instruction>,
     pub min_epoch: Option<Epoch>,
     pub max_epoch: Option<Epoch>,
 }

dan_layer/transaction/src/builder.rs

@@ -17,31 +17,26 @@ use tari_template_lib::{
     models::{Amount, ComponentAddress, ConfidentialWithdrawProof, ResourceAddress},
 };
 
-use crate::{signature::TransactionSignatureFields, Transaction, TransactionSignature};
+use crate::{unsigned_transaction::UnsignedTransaction, Transaction, TransactionSignature};
 
 #[derive(Debug, Clone, Default)]
 pub struct TransactionBuilder {
-    instructions: Vec<Instruction>,
-    fee_instructions: Vec<Instruction>,
+    unsigned_transaction: UnsignedTransaction,
     signature: Option<TransactionSignature>,
-    inputs: Vec<SubstateAddress>,
-    input_refs: Vec<SubstateAddress>,
-    outputs: Vec<SubstateAddress>,
-    min_epoch: Option<Epoch>,
-    max_epoch: Option<Epoch>,
 }
 
 impl TransactionBuilder {
     pub fn new() -> Self {
         Self {
-            instructions: Vec::new(),
-            fee_instructions: Vec::new(),
+            unsigned_transaction: UnsignedTransaction::default(),
+            signature: None,
+        }
+    }
+
+    pub fn with_unsigned_transaction(self, unsigned_transaction: UnsignedTransaction) -> Self {
+        Self {
+            unsigned_transaction,
             signature: None,
-            inputs: Vec::new(),
-            input_refs: Vec::new(),
-            outputs: Vec::new(),
-            min_epoch: None,
-            max_epoch: None,
         }
     }
 
@@ -112,36 +107,36 @@ impl TransactionBuilder {
     }
 
     pub fn with_fee_instructions(mut self, instructions: Vec<Instruction>) -> Self {
-        self.fee_instructions = instructions;
+        self.unsigned_transaction.fee_instructions = instructions;
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
     pub fn with_fee_instructions_builder<F: FnOnce(TransactionBuilder) -> TransactionBuilder>(mut self, f: F) -> Self {
         let builder = f(TransactionBuilder::new());
-        self.fee_instructions = builder.instructions;
+        self.unsigned_transaction.fee_instructions = builder.unsigned_transaction.instructions;
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
     pub fn add_fee_instruction(mut self, instruction: Instruction) -> Self {
-        self.fee_instructions.push(instruction);
+        self.unsigned_transaction.fee_instructions.push(instruction);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
     pub fn add_instruction(mut self, instruction: Instruction) -> Self {
-        self.instructions.push(instruction);
+        self.unsigned_transaction.instructions.push(instruction);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
     pub fn with_instructions(mut self, instructions: Vec<Instruction>) -> Self {
-        self.instructions.extend(instructions);
+        self.unsigned_transaction.instructions.extend(instructions);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
@@ -153,22 +148,13 @@ impl TransactionBuilder {
     }
 
     pub fn sign(mut self, secret_key: &PrivateKey) -> Self {
-        let signature_fields = TransactionSignatureFields {
-            fee_instructions: self.fee_instructions.clone(),
-            instructions: self.instructions.clone(),
-            inputs: self.inputs.clone(),
-            input_refs: self.input_refs.clone(),
-            min_epoch: self.min_epoch,
-            max_epoch: self.max_epoch,
-        };
-
-        self.signature = Some(TransactionSignature::sign(secret_key, signature_fields));
+        self.signature = Some(TransactionSignature::sign(secret_key, &self.unsigned_transaction));
         self
     }
 
     /// Add an input to be consumed
     pub fn add_input(mut self, input_object: SubstateAddress) -> Self {
-        self.inputs.push(input_object);
+        self.unsigned_transaction.inputs.push(input_object);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
@@ -183,15 +169,15 @@ impl TransactionBuilder {
     }
 
     pub fn with_inputs<I: IntoIterator<Item = SubstateAddress>>(mut self, inputs: I) -> Self {
-        self.inputs.extend(inputs);
+        self.unsigned_transaction.inputs.extend(inputs);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
     /// Add an input to be used without mutation
     pub fn add_input_ref(mut self, input_object: SubstateAddress) -> Self {
-        self.input_refs.push(input_object);
+        self.unsigned_transaction.input_refs.push(input_object);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
@@ -206,63 +192,50 @@ impl TransactionBuilder {
     }
 
     pub fn with_input_refs<I: IntoIterator<Item = SubstateAddress>>(mut self, inputs: I) -> Self {
-        self.input_refs.extend(inputs);
+        self.unsigned_transaction.input_refs.extend(inputs);
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
-    pub fn add_output(mut self, output_object: SubstateAddress) -> Self {
-        self.outputs.push(output_object);
-        self
-    }
-
-    pub fn with_substate_outputs<I: IntoIterator<Item = (B, u32)>, B: Borrow<SubstateId>>(self, outputs: I) -> Self {
-        self.with_outputs(
-            outputs
-                .into_iter()
-                .map(|(a, v)| SubstateAddress::from_address(a.borrow(), v)),
-        )
-    }
-
-    pub fn with_outputs<I: IntoIterator<Item = SubstateAddress>>(mut self, outputs: I) -> Self {
-        self.outputs.extend(outputs);
-        self
-    }
-
-    pub fn add_output_ref(mut self, output_object: SubstateAddress) -> Self {
-        self.outputs.push(output_object);
-        self
-    }
-
     pub fn with_min_epoch(mut self, min_epoch: Option<Epoch>) -> Self {
-        self.min_epoch = min_epoch;
+        self.unsigned_transaction.min_epoch = min_epoch;
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
     pub fn with_max_epoch(mut self, max_epoch: Option<Epoch>) -> Self {
-        self.max_epoch = max_epoch;
+        self.unsigned_transaction.max_epoch = max_epoch;
         // Reset the signature as it is no longer valid
         self.signature = None;
         self
     }
 
-    pub fn build_as_instructions(mut self) -> Vec<Instruction> {
-        self.instructions.drain(..).collect()
+    pub fn build_unsigned_transaction(self) -> UnsignedTransaction {
+        self.unsigned_transaction
     }
 
-    pub fn build(mut self) -> Transaction {
+    pub fn build(self) -> Transaction {
+        let UnsignedTransaction {
+            fee_instructions,
+            instructions,
+            inputs,
+            input_refs,
+            filled_inputs,
+            min_epoch,
+            max_epoch,
+        } = self.unsigned_transaction;
+
         Transaction::new(
-            self.fee_instructions.drain(..).collect(),
-            self.instructions.drain(..).collect(),
-            self.signature.take().expect("not signed"),
-            self.inputs,
-            self.input_refs,
-            vec![],
-            self.min_epoch,
-            self.max_epoch,
+            fee_instructions,
+            instructions,
+            self.signature.expect("not signed"),
+            inputs,
+            input_refs,
+            filled_inputs,
+            min_epoch,
+            max_epoch,
         )
     }
 }