fix: remove tx source from auth entry signing logic

Author: mootz12

cmd/crates/soroban-test/tests/it/integration/hello_world.rs

@@ -133,8 +133,8 @@ async fn invoke_contract() {
     invoke_auth(sandbox, id, &addr);
     invoke_auth_with_identity(sandbox, id, "test", &addr);
     invoke_auth_with_identity(sandbox, id, "testone", &addr_1);
+    invoke_auth_with_non_source_identity(sandbox, id, "test", "testone", &addr_1);
     invoke_auth_with_different_test_account_fail(sandbox, id, &addr_1).await;
-    // invoke_auth_with_different_test_account(sandbox, id);
     contract_data_read_failure(sandbox, id);
     invoke_with_seed(sandbox, id, &seed_phrase).await;
     invoke_with_sk(sandbox, id, &secret_key).await;
@@ -225,6 +225,30 @@ fn invoke_auth_with_identity(sandbox: &TestEnv, id: &str, key: &str, addr: &str)
         .success();
 }
 
+fn invoke_auth_with_non_source_identity(
+    sandbox: &TestEnv,
+    id: &str,
+    source: &str,
+    key: &str,
+    addr: &str,
+) {
+    sandbox
+        .new_assert_cmd("contract")
+        .arg("invoke")
+        .arg("--source")
+        .arg(source)
+        .arg("--id")
+        .arg(id)
+        .arg("--")
+        .arg("auth")
+        .arg("--addr")
+        .arg(key)
+        .arg("--world=world")
+        .assert()
+        .stdout(format!("\"{addr}\"\n"))
+        .success();
+}
+
 async fn invoke_auth_with_different_test_account_fail(sandbox: &TestEnv, id: &str, addr: &str) {
     let res = sandbox
         .invoke_with_test(&[

cmd/soroban-cli/src/assembled.rs

@@ -1,9 +1,9 @@
 use sha2::{Digest, Sha256};
 use stellar_xdr::curr::{
-    self as xdr, ExtensionPoint, Hash, InvokeHostFunctionOp, LedgerFootprint, Limits, Memo,
-    Operation, OperationBody, Preconditions, ReadXdr, RestoreFootprintOp,
-    SorobanAuthorizationEntry, SorobanAuthorizedFunction, SorobanResources, SorobanTransactionData,
-    Transaction, TransactionEnvelope, TransactionExt, TransactionSignaturePayload,
+    self as xdr, ExtensionPoint, Hash, LedgerFootprint, Limits, Memo, Operation, OperationBody,
+    Preconditions, ReadXdr, RestoreFootprintOp, SorobanAuthorizationEntry,
+    SorobanAuthorizedFunction, SorobanResources, SorobanTransactionData, Transaction,
+    TransactionEnvelope, TransactionExt, TransactionSignaturePayload,
     TransactionSignaturePayloadTaggedTransaction, TransactionV1Envelope, VecM, WriteXdr,
 };
 
@@ -156,11 +156,6 @@ impl Assembled {
         Ok(())
     }
 
-    #[must_use]
-    pub fn requires_auth(&self) -> bool {
-        requires_auth(&self.txn).is_some()
-    }
-
     #[must_use]
     pub fn is_view(&self) -> bool {
         let TransactionExt::V1(SorobanTransactionData {
@@ -275,21 +270,6 @@ fn assemble(
     Ok(tx)
 }
 
-fn requires_auth(txn: &Transaction) -> Option<xdr::Operation> {
-    let [op @ Operation {
-        body: OperationBody::InvokeHostFunction(InvokeHostFunctionOp { auth, .. }),
-        ..
-    }] = txn.operations.as_slice()
-    else {
-        return None;
-    };
-    matches!(
-        auth.first().map(|x| &x.root_invocation.function),
-        Some(&SorobanAuthorizedFunction::ContractFn(_))
-    )
-    .then(move || op.clone())
-}
-
 fn restore(parent: &Transaction, restore: &RestorePreamble) -> Result<Transaction, Error> {
     let transaction_data =
         SorobanTransactionData::from_xdr_base64(&restore.transaction_data, Limits::none())?;

cmd/soroban-cli/src/config/mod.rs

@@ -122,13 +122,11 @@ impl Args {
         signers: &[Signer],
     ) -> Result<Option<Transaction>, Error> {
         let network = self.get_network()?;
-        let source_signer = self.source_signer().await?;
         let client = network.rpc_client()?;
         let latest_ledger = client.get_latest_ledger().await?.sequence;
         let seq_num = latest_ledger + 60; // ~ 5 min
         Ok(signer::sign_soroban_authorizations(
             tx,
-            &source_signer,
             signers,
             seq_num,
             &network.network_passphrase,

cmd/soroban-cli/src/signer/mod.rs

@@ -1,9 +1,8 @@
 use crate::xdr::{
     self, AccountId, DecoratedSignature, Hash, HashIdPreimage, HashIdPreimageSorobanAuthorization,
-    InvokeHostFunctionOp, Limits, Operation, OperationBody, PublicKey, ScAddress, ScMap, ScSymbol,
-    ScVal, Signature, SignatureHint, SorobanAddressCredentials, SorobanAuthorizationEntry,
-    SorobanAuthorizedFunction, SorobanCredentials, Transaction, TransactionEnvelope,
-    TransactionV1Envelope, Uint256, VecM, WriteXdr,
+    Limits, Operation, OperationBody, PublicKey, ScAddress, ScMap, ScSymbol, ScVal, Signature,
+    SignatureHint, SorobanAddressCredentials, SorobanAuthorizationEntry, SorobanCredentials,
+    Transaction, TransactionEnvelope, TransactionV1Envelope, Uint256, VecM, WriteXdr,
 };
 use ed25519_dalek::{ed25519::signature::Signer as _, Signature as Ed25519Signature};
 use sha2::{Digest, Sha256};
@@ -46,39 +45,24 @@ pub enum Error {
     Decode(#[from] stellar_strkey::DecodeError),
 }
 
-fn requires_auth(txn: &Transaction) -> Option<xdr::Operation> {
-    let [op @ Operation {
-        body: OperationBody::InvokeHostFunction(InvokeHostFunctionOp { auth, .. }),
-        ..
-    }] = txn.operations.as_slice()
-    else {
-        return None;
-    };
-    matches!(
-        auth.first().map(|x| &x.root_invocation.function),
-        Some(&SorobanAuthorizedFunction::ContractFn(_))
-    )
-    .then(move || op.clone())
-}
-
-// Use the given source_key and signers, to sign all SorobanAuthorizationEntry's in the given
-// transaction. If unable to sign, return an error.
+/// Use the given source_key and signers, to sign all SorobanAuthorizationEntry's in the given
+/// transaction.
+///
+/// If no SorobanAuthorizationEntry's need signing (including if none exist), return Ok(None).
+///
+/// If a SorobanAuthorizationEntry needs signing, but a signature cannot be produced for it,
+/// return an Error
 pub fn sign_soroban_authorizations(
     raw: &Transaction,
-    source_signer: &Signer,
     signers: &[Signer],
     signature_expiration_ledger: u32,
     network_passphrase: &str,
 ) -> Result<Option<Transaction>, Error> {
-    let mut tx = raw.clone();
-    let Some(mut op) = requires_auth(&tx) else {
-        return Ok(None);
-    };
-
-    let Operation {
-        body: OperationBody::InvokeHostFunction(ref mut body),
+    // Check if we have exactly one operation and it's InvokeHostFunction
+    let [op @ Operation {
+        body: OperationBody::InvokeHostFunction(body),
         ..
-    } = op
+    }] = raw.operations.as_slice()
     else {
         return Ok(None);
     };
@@ -123,10 +107,6 @@ pub fn sign_soroban_authorizations(
             }
         }
 
-        if needle == &source_signer.get_public_key()?.0 {
-            signer = Some(source_signer);
-        }
-
         match signer {
             Some(signer) => {
                 let signed_entry = sign_soroban_authorization_entry(
@@ -148,9 +128,21 @@ pub fn sign_soroban_authorizations(
         }
     }
 
-    body.auth = signed_auths.try_into()?;
-    tx.operations = vec![op].try_into()?;
-    Ok(Some(tx))
+    // No signatures were made, return None to indicate no change to the transaction
+    if signed_auths.is_empty() {
+        return Ok(None);
+    }
+
+    // Build updated transaction with signed auth entries
+    let mut updated_op = op.clone();
+    if let OperationBody::InvokeHostFunction(ref mut updated_body) = updated_op.body {
+        let mut tx = raw.clone();
+        updated_body.auth = signed_auths.try_into()?;
+        tx.operations = vec![updated_op].try_into()?;
+        Ok(Some(tx))
+    } else {
+        Ok(None)
+    }
 }
 
 fn sign_soroban_authorization_entry(