fix!: require a specific signer for input spend + key manager backend

applications/tari_app_utilities/src/transaction_executor.rs

@@ -5,6 +5,7 @@ use std::sync::Arc;
 
 use log::*;
 use tari_engine::{
+    executables::Executable,
     fees::{FeeModule, FeeTable},
     runtime::{AuthParams, RuntimeModule},
     state_store::{memory::ReadOnlyMemoryStateStore, StateStoreError},
@@ -120,10 +121,7 @@ where TTemplateProvider: TemplateProvider<Template = LoadedTemplate>
         // Include signature public key badges for all transaction signers in the initial auth scope
         // NOTE: we assume all signatures have already been validated.
         let initial_ownership_proofs = transaction
-            .signatures()
-            .iter()
-            .map(|p| p.public_key())
-            .chain(Some(transaction.seal_signature().public_key()).filter(|_| transaction.is_seal_signer_authorized()))
+            .signers_iter()
             .map(|pk| NonFungibleAddress::from_public_key(*pk))
             .collect();
         let auth_params = AuthParams {

applications/tari_walletd/src/handlers/accounts.rs

@@ -28,7 +28,7 @@ use tari_ootle_wallet_sdk::{
         stealth_transfer::StealthTransferParams,
         substate::ValidatorScanResult,
     },
-    models::{KeyBranch, KeyId, NewAccountData},
+    models::{KeyBranch, NewAccountData},
 };
 use tari_ootle_wallet_sdk_services::events::TransactionSubmittedEvent;
 use tari_template_builtin::ACCOUNT_TEMPLATE_ADDRESS;
@@ -534,11 +534,14 @@ pub async fn handle_claim_burn(
         public_nonce: reciprocal_claim_public_key_expanded,
     };
 
+    let public_signer_key = sdk.key_manager_api().next_public_key(KeyBranch::Nonce)?;
+
     let pay_fee_and_mint_output = sdk.stealth_crypto_api().generate_transfer_statement(
         array::from_ref(&input),
         0,
         array::from_ref(&output_statement),
         max_fee,
+        public_signer_key.public_key.to_byte_type(),
     )?;
     // We'll create an output with the same encrypted data that was used on L1 burn. Note that this is not strictly
     // necessary. The engine will create the output with whatever you give it, so we could reencrypt.
@@ -556,12 +559,9 @@ pub async fn handle_claim_burn(
         .add_input(XTR)
         .build();
 
-    // The signer does not authorize this transaction, as the claim burn instruction is authorized by the proofs. So we
-    // can sign with any key.
-    let nonce = sdk.key_manager_api().next_public_key(KeyBranch::Nonce)?;
     let transaction = sdk
         .local_signer_api()
-        .sign(KeyBranch::Nonce, nonce.key_id, transaction)?;
+        .sign(KeyBranch::Nonce, public_signer_key.key_id, transaction)?;
 
     let tx_id = context.transaction_service().submit_transaction(transaction).await?;
 
@@ -959,7 +959,7 @@ pub async fn handle_stealth_transfer(
     let network = sdk.sdk_config().network;
     let notifier = context.notifier().clone();
     let owner_account = get_account(&req.owner_account, &sdk.accounts_api())?;
-    let Some(owner_key_id) = owner_account.owner_key_id() else {
+    if owner_account.owner_key_id().is_none() {
         return Err(invalid_params(
             "owner_account",
             Some("cannot transfer from an account without an owner key"),
@@ -987,22 +987,11 @@ pub async fn handle_stealth_transfer(
     task::spawn(async move {
         let transfer = sdk.stealth_transfer_api().transfer(owner_account, params).await?;
 
-        let must_sign_with_account_key =
-            transfer.fee_inputs.revealed.is_positive() || transfer.transfer_inputs.revealed.is_positive();
-
-        let transaction = transfer.transaction.authorized_sealed_signer().build(vec![]);
-
-        let (key_branch, key_id) = if must_sign_with_account_key {
-            (KeyBranch::Account, owner_key_id)
-        } else {
-            // Since we don't require account auth, use a throwaway nonce to sign the transaction
-            (
-                KeyBranch::Nonce,
-                KeyId::derived(sdk.key_manager_api().next_derived_key_index(KeyBranch::Nonce)?),
-            )
-        };
+        let transaction = transfer.transaction.authorized_sealed_signer().build();
 
-        let transaction = sdk.local_signer_api().sign(key_branch, key_id, transaction)?;
+        let transaction =
+            sdk.local_signer_api()
+                .sign(transfer.signing_key_branch, transfer.signing_key_id, transaction)?;
 
         // TODO: if submitting fails we need to unlock the inputs again
         if req.dry_run {

applications/tari_walletd/src/handlers/nfts.rs

@@ -303,7 +303,7 @@ pub async fn handle_transfer(
         .with_inputs(inputs.into_iter().map(|input| input.into_unversioned()))
         // Seal signer is the fee payer account
         .with_authorized_seal_signer()
-        .then(|builder| {
+        .map(|builder| {
             sdk.local_signer_api().sign_with_context(
                 KeyBranch::Account,
                 account_owner_key_id,

applications/tari_walletd/src/handlers/transaction.rs

@@ -327,7 +327,7 @@ pub async fn handle_submit_manifest(
             }
         })
         .with_instructions(instructions.instructions)
-        .then(|builder| {
+        .map(|builder| {
             if signing_key_id == account_owner_key_id {
                 Ok(builder)
             } else {
@@ -350,7 +350,7 @@ pub async fn handle_submit_manifest(
     let transaction = transaction
         .with_inputs(inputs)
         .authorized_sealed_signer()
-        .build(signatures);
+        .build_with_signatures(signatures);
 
     let transaction = sdk
         .local_signer_api()

applications/tari_walletd/src/handlers/validator.rs

@@ -179,26 +179,23 @@ pub async fn handle_claim_validator_fees(
         .with_inputs(inputs.into_iter().map(|input| input.into_unversioned()))
         .with_inputs(fee_pool_addresses.map(SubstateRequirement::unversioned))
         .add_input(XTR)
-        .then(|builder| {
+        .map(|builder| {
             if let Some(index) = req.claim_key_index {
                 if claim_public_key == *account.address.account_public_key() {
-                    builder
+                    Ok(builder)
                 } else {
                     // If the claim key is different from the account secret, we need to sign with both
-                    sdk.local_signer_api()
-                        .sign_with_context(
-                            KeyBranch::Account,
-                            KeyId::derived(index),
-                            account.address.account_public_key(),
-                            builder.with_authorized_seal_signer(),
-                        )
-                        // We happen to know that signing with a derived key is infallible
-                        .expect("Signing with should work")
+                    sdk.local_signer_api().sign_with_context(
+                        KeyBranch::Account,
+                        KeyId::derived(index),
+                        account.address.account_public_key(),
+                        builder.with_authorized_seal_signer(),
+                    )
                 }
             } else {
-                builder
+                Ok(builder)
             }
-        })
+        })?
         .build();
 
     let transaction = sdk

crates/engine/src/executables/mod.rs

@@ -18,6 +18,7 @@ pub trait Executable {
     fn all_inputs_iter(&self) -> impl Iterator<Item = SubstateRequirementRef<'_>> + '_;
 
     fn main_signer(&self) -> Option<RistrettoPublicKeyBytes>;
+    fn signers_iter(&self) -> impl Iterator<Item = &RistrettoPublicKeyBytes>;
 
     fn into_instructions(self) -> Instructions;
 }

crates/engine/src/executables/transaction.rs

@@ -20,11 +20,14 @@ impl Executable for Transaction {
         // TODO: If the seal signer is authorized we use this as the signer public key, if not we use the first
         // signature as the "default" owner. This is due to limitations of the current transaction model.
         // We could remove the idea of a default owner (OwnedBySigner) entirely.
-        Some(self.seal_signature())
+        self.signers_iter().next().copied()
+    }
+
+    fn signers_iter(&self) -> impl Iterator<Item = &RistrettoPublicKeyBytes> {
+        Some(self.seal_signature().public_key())
             .filter(|_| self.is_seal_signer_authorized())
-            .map(|s| s.public_key())
-            .or(self.signatures().first().map(|s| s.public_key()))
-            .copied()
+            .into_iter()
+            .chain(self.signatures().iter().map(|s| s.public_key()))
     }
 
     fn into_instructions(self) -> Instructions {

crates/engine/src/runtime/error.rs

@@ -45,6 +45,7 @@ use tari_template_lib::{
         ResourceAddress,
         VaultId,
     },
+    prelude::RistrettoPublicKeyBytes,
     types::{Amount, TemplateAddress},
 };
 use tari_transaction::args::{WorkspaceId, WorkspaceOffsetId};
@@ -173,6 +174,8 @@ pub enum RuntimeError {
     AccessDeniedAuthHook { action_ident: ActionIdent, details: String },
     #[error("Access Denied: You must be the owner to perform this action: {action}")]
     AccessDeniedOwnerRequired { action: ActionIdent },
+    #[error("Access Denied: Stealth transfer requires a signer with public key {required_signer}")]
+    AccessDeniedStealthTransferSigner { required_signer: RistrettoPublicKeyBytes },
     #[error("Invalid method address rule for {template_name}: {details}")]
     InvalidMethodAccessRule { template_name: String, details: String },
     #[error("Runtime module error: {0}")]

crates/engine/src/runtime/working_state.rs

@@ -14,7 +14,7 @@ use tari_crypto::ristretto::RistrettoPublicKey;
 use tari_engine_types::{
     bucket::Bucket,
     component::ComponentHeader,
-    crypto::verify_utxo_spend_permission,
+    crypto::messages,
     events::Event,
     fees::FeeReceipt,
     id_provider::{IdProvider, ObjectIds},
@@ -27,6 +27,7 @@ use tari_engine_types::{
     resource::Resource,
     resource_container::{ResourceContainer, ResourceError},
     stealth,
+    stealth::ValidatedStealthTransfer,
     substate::{Substate, SubstateDiff, SubstateId, SubstateValue},
     transaction_receipt::TransactionReceipt,
     vault::Vault,
@@ -49,7 +50,6 @@ use tari_template_lib::{
         NonFungibleAddress,
         ProofId,
         ResourceAddress,
-        StealthInputsStatement,
         StealthTransferStatement,
         UtxoAddress,
         VaultId,
@@ -267,15 +267,32 @@ impl WorkingState {
         Ok(resource)
     }
 
-    pub fn spend_stealth_utxos(
+    pub fn validate_and_spend_stealth_utxos(
         &mut self,
         resource_address: ResourceAddress,
-        stmt: &StealthInputsStatement,
-    ) -> Result<(), RuntimeError> {
-        for input in &stmt.inputs {
+        stmt: &StealthTransferStatement,
+        view_key: Option<&RistrettoPublicKey>,
+    ) -> Result<ValidatedStealthTransfer, RuntimeError> {
+        let required_signer = &stmt.inputs_statement.required_signer;
+
+        // Check that the required_signed is in scope
+        let proofs = self.base_call_scope().auth_scope().virtual_proofs();
+        if proofs
+            .iter()
+            .filter(|p| *p.resource_address() == PUBLIC_IDENTITY_RESOURCE_ADDRESS)
+            .all(|p| p.id().as_u256().map(|b| b.as_slice()) != Some(required_signer.as_bytes()))
+        {
+            return Err(RuntimeError::AccessDeniedStealthTransferSigner {
+                required_signer: *required_signer,
+            });
+        }
+
+        let metadata_hash = messages::stealth_statement_metadata64(&stmt.outputs_statement);
+        for input in &stmt.inputs_statement.inputs {
             let address = UtxoAddress::new(resource_address, input.commitment.into());
             let lock_id = self.store.try_lock(&address.clone().into(), LockFlag::Write)?;
             let utxo = self.store.down_utxo(lock_id)?;
+            self.store.try_unlock(lock_id)?;
             if utxo.is_frozen() {
                 return Err(ResourceError::InvalidSpend {
                     details: format!("Utxo {} is frozen", address),
@@ -287,9 +304,11 @@ impl WorkingState {
                 details: format!("Utxo {} is burnt", address),
             })?;
 
-            verify_utxo_spend_permission(output, input)?;
+            stealth::validate_ownership_proof(output, input, required_signer, &metadata_hash)?;
         }
-        Ok(())
+
+        let valid_transfer = stealth::validate_transfer_balance(stmt, view_key)?;
+        Ok(valid_transfer)
     }
 
     pub fn get_non_fungible(&self, locked: &LockedSubstate) -> Result<&NonFungibleContainer, RuntimeError> {
@@ -1526,8 +1545,6 @@ impl WorkingState {
             },
         }
 
-        self.spend_stealth_utxos(resource_address, &statement.inputs_statement)?;
-
         let resource = self.get_resource(&resource_lock)?;
         let view_key = resource
             .view_key()
@@ -1541,7 +1558,7 @@ impl WorkingState {
                 }
             })?;
 
-        let valid_transfer = stealth::validate_transfer(&statement, view_key.as_ref())?;
+        let valid_transfer = self.validate_and_spend_stealth_utxos(resource_address, &statement, view_key.as_ref())?;
 
         for output in valid_transfer.outputs {
             let address = UtxoAddress::new(resource_address, output.output.commitment.to_byte_type().into());

crates/engine/tests/signature.rs

@@ -76,13 +76,19 @@ fn claim_with_valid_signature() {
         .as_vault_id()
         .unwrap();
 
-    let transfer = stealth::generate_transfer_data(&[], 1_000_000_000_000u64, Some(1_000_000_000_000), 0);
+    let transfer = stealth::generate_transfer_data(
+        &[],
+        1_000_000_000_000u64,
+        Some(1_000_000_000_000),
+        0,
+        test.to_public_key_bytes(),
+    );
     let signature = sign_it(&s1);
     let result = test.execute_expect_success(
         Transaction::builder()
             .call_method(faucet, "claim_funds", args![p1, signature, transfer.statement])
             .build_and_seal(test.secret_key()),
-        vec![],
+        vec![test.owner_proof()],
     );
 
     let diff = result.finalize.any_accept().unwrap();
@@ -104,16 +110,16 @@ fn multi_claim() {
     let p2 = PublicKey::from(p2.to_byte_type());
     let (mut test, faucet) = setup(vec![p1, p2]);
 
-    let transfer1 = stealth::generate_transfer_data(&[], 1000, Some(1000), 0);
-    let transfer2 = stealth::generate_transfer_data(&[], 1000, Some(1000), 0);
+    let transfer1 = stealth::generate_transfer_data(&[], 1000, Some(1000), 0, test.to_public_key_bytes());
+    let transfer2 = stealth::generate_transfer_data(&[], 1000, Some(1000), 0, test.to_public_key_bytes());
     let sig1 = sign_it(&s1);
     let sig2 = sign_it(&s2);
     test.execute_expect_success(
         Transaction::builder()
             .call_method(faucet, "claim_funds", args![p1, sig1, transfer1.statement])
             .call_method(faucet, "claim_funds", args![p2, sig2, transfer2.statement])
             .build_and_seal(test.secret_key()),
-        vec![],
+        vec![test.owner_proof()],
     );
 }
 
@@ -123,13 +129,13 @@ fn bad_signature() {
     let p1 = PublicKey::from(p1.to_byte_type());
     let (mut test, faucet) = setup(vec![p1]);
 
-    let transfer = stealth::generate_transfer_data(&[], 1000, Some(1000), 0);
+    let transfer = stealth::generate_transfer_data(&[], 1000, Some(1000), 0, test.to_public_key_bytes());
     let sig1 = sign_it_with(&s1, b"A different message");
     let reason = test.execute_expect_failure(
         Transaction::builder()
             .call_method(faucet, "claim_funds", args![p1, sig1, transfer.statement])
             .build_and_seal(test.secret_key()),
-        vec![],
+        vec![test.owner_proof()],
     );
 
     assert_reject_reason(reason.clone(), "Your signature is invalid, so no funds for you");