fix: address PR review

Remove Blob trait. Add to TransactionHash trait to include hint. Move print to top level signer and fix test. Also remove the prompt for this PR since the sign command is already approval for signing.

FULL_HELP_DOCS.md

@@ -1331,14 +1331,12 @@ Calculate the hash of a transaction envelope from stdin
 
 Sign a transaction envolope appending the signature to the envelope
 
-**Usage:** `stellar tx sign [OPTIONS] --source-account <SOURCE_ACCOUNT>`
+**Usage:** `stellar tx sign [OPTIONS]`
 
 ###### **Options:**
 
 * `--sign-with-key <SIGN_WITH_KEY>` — Sign with a local key. Can be an identity (--sign-with-key alice), a secret key (--sign-with-key SC36…), or a seed phrase (--sign-with-key "kite urban…"). If using seed phrase, `--hd-path` defaults to the `0` path
 * `--hd-path <HD_PATH>` — If using a seed phrase to sign, sets which hierarchical deterministic path to use, e.g. `m/44'/148'/{hd_path}`. Example: `--hd-path 1`. Default: `0`
-* `--yes` — If one of `--sign-with-*` flags is provided, don't ask to confirm to sign a transaction
-* `--source-account <SOURCE_ACCOUNT>` — Account that signs the transaction. Alias `source`. Can be an identity (--source alice), a secret key (--source SC36…), or a seed phrase (--source "kite urban…")
 * `--rpc-url <RPC_URL>` — RPC server endpoint
 * `--network-passphrase <NETWORK_PASSPHRASE>` — Network passphrase to sign the transaction sent to the rpc server
 * `--network <NETWORK>` — Name of network to use from config

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

@@ -1,3 +1,4 @@
+use soroban_rpc::GetTransactionResponse;
 use soroban_sdk::xdr::{Limits, ReadXdr, TransactionEnvelope, WriteXdr};
 use soroban_test::{AssertExt, TestEnv};
 
@@ -71,14 +72,12 @@ async fn send() {
     );
 
     let rpc_result = send_manually(sandbox, &tx_env).await;
-
-    println!("Transaction sent: {rpc_result}");
+    assert_eq!(rpc_result.status, "SUCCESS");
 }
 
-async fn send_manually(sandbox: &TestEnv, tx_env: &TransactionEnvelope) -> String {
+async fn send_manually(sandbox: &TestEnv, tx_env: &TransactionEnvelope) -> GetTransactionResponse {
     let client = soroban_rpc::Client::new(&sandbox.rpc_url).unwrap();
-    let res = client.send_transaction_polling(tx_env).await.unwrap();
-    serde_json::to_string_pretty(&res).unwrap()
+    client.send_transaction_polling(tx_env).await.unwrap()
 }
 
 fn sign_manually(sandbox: &TestEnv, tx_env: &TransactionEnvelope) -> TransactionEnvelope {

cmd/soroban-cli/Cargo.toml

@@ -123,7 +123,6 @@ humantime = "2.1.0"
 phf = { version = "0.11.2", features = ["macros"] }
 semver = "1.0.0"
 glob = "0.3.1"
-crossterm = "0.28.1"
 
 # For hyper-tls
 [target.'cfg(unix)'.dependencies]

cmd/soroban-cli/src/commands/tx/hash.rs

@@ -1,6 +1,6 @@
 use hex;
 
-use crate::{commands::global, config::network, utils::transaction_hash};
+use crate::{commands::global, config::network, signer::types::transaction_hash};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {

cmd/soroban-cli/src/commands/tx/mod.rs

@@ -34,7 +34,7 @@ impl Cmd {
         match self {
             Cmd::Simulate(cmd) => cmd.run(global_args).await?,
             Cmd::Hash(cmd) => cmd.run(global_args)?,
-            Cmd::Sign(cmd) => cmd.run().await?,
+            Cmd::Sign(cmd) => cmd.run(global_args).await?,
         };
         Ok(())
     }

cmd/soroban-cli/src/commands/tx/sign.rs

@@ -1,4 +1,5 @@
 use crate::{
+    commands::global,
     config::{locator, network, sign_with},
     xdr::{self, Limits, TransactionEnvelope, WriteXdr},
 };
@@ -30,17 +31,21 @@ pub struct Cmd {
 
 impl Cmd {
     #[allow(clippy::unused_async)]
-    pub async fn run(&self) -> Result<(), Error> {
+    pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> {
         let txn_env = super::xdr::tx_envelope_from_stdin()?;
-        let envelope = self.sign_tx_env(txn_env).await?;
+        let envelope = self.sign_tx_env(txn_env, global_args.quiet).await?;
         println!("{}", envelope.to_xdr_base64(Limits::none())?.trim());
         Ok(())
     }
 
-    pub async fn sign_tx_env(&self, tx: TransactionEnvelope) -> Result<TransactionEnvelope, Error> {
+    pub async fn sign_tx_env(
+        &self,
+        tx: TransactionEnvelope,
+        quiet: bool,
+    ) -> Result<TransactionEnvelope, Error> {
         Ok(self
             .sign_with
-            .sign_txn_env(tx, &self.locator, &self.network.get(&self.locator)?)
+            .sign_txn_env(tx, &self.locator, &self.network.get(&self.locator)?, quiet)
             .await?)
     }
 }

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

@@ -3,11 +3,16 @@ use serde::{Deserialize, Serialize};
 use std::{io::Write, str::FromStr};
 use stellar_strkey::ed25519::{PrivateKey, PublicKey};
 
+use crate::print::Print;
+use crate::signer::types::transaction_hash;
+use crate::xdr::{self, DecoratedSignature};
 use crate::{
     signer::{self, LocalKey},
     utils,
 };
 
+use super::network::Network;
+
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
     #[error("invalid secret key")]
@@ -125,12 +130,21 @@ impl Secret {
         )?)
     }
 
-    pub fn signer(&self, index: Option<usize>, prompt: bool) -> Result<StellarSigner, Error> {
-        match self {
-            Secret::SecretKey { .. } | Secret::SeedPhrase { .. } => Ok(StellarSigner::Local(
-                LocalKey::new(self.key_pair(index)?, prompt),
-            )),
-        }
+    pub fn signer(
+        &self,
+        index: Option<usize>,
+        prompt: bool,
+        quiet: bool,
+    ) -> Result<StellarSigner, Error> {
+        let kind = match self {
+            Secret::SecretKey { .. } | Secret::SeedPhrase { .. } => {
+                SignerKind::Local(LocalKey::new(self.key_pair(index)?, prompt))
+            }
+        };
+        Ok(StellarSigner {
+            kind,
+            printer: Print::new(quiet),
+        })
     }
 
     pub fn key_pair(&self, index: Option<usize>) -> Result<ed25519_dalek::SigningKey, Error> {
@@ -153,15 +167,28 @@ impl Secret {
     }
 }
 
-pub enum StellarSigner {
+pub struct StellarSigner {
+    kind: SignerKind,
+    printer: Print,
+}
+
+pub enum SignerKind {
     Local(LocalKey),
 }
 
 #[async_trait::async_trait]
-impl signer::Blob for StellarSigner {
-    async fn sign_blob(&self, blob: &[u8]) -> Result<Vec<u8>, signer::types::Error> {
-        match self {
-            StellarSigner::Local(signer) => signer.sign_blob(blob).await,
+impl signer::Transaction for StellarSigner {
+    async fn sign_txn(
+        &self,
+        txn: &xdr::Transaction,
+        network: &Network,
+    ) -> Result<DecoratedSignature, signer::types::Error> {
+        let tx_hash = transaction_hash(txn, &network.network_passphrase)?;
+        let hex_hash = hex::encode(tx_hash);
+        self.printer
+            .infoln(format!("Signing transaction with hash: {hex_hash}"));
+        match &self.kind {
+            SignerKind::Local(key) => key.sign_txn(txn, network).await,
         }
     }
 }

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

@@ -6,7 +6,6 @@ use crate::{
     xdr::TransactionEnvelope,
 };
 use clap::arg;
-use stellar_strkey::ed25519::PublicKey;
 
 use super::{
     locator,
@@ -36,11 +35,7 @@ pub enum Error {
 #[group(skip)]
 pub struct Args {
     /// Sign with a local key. Can be an identity (--sign-with-key alice), a secret key (--sign-with-key SC36…), or a seed phrase (--sign-with-key "kite urban…"). If using seed phrase, `--hd-path` defaults to the `0` path.
-    #[arg(
-        long,
-        conflicts_with = "sign_with_lab",
-        env = "STELLAR_SIGN_WITH_KEY"
-    )]
+    #[arg(long, conflicts_with = "sign_with_lab", env = "STELLAR_SIGN_WITH_KEY")]
     pub sign_with_key: Option<String>,
     /// Sign with labratory
     #[arg(
@@ -54,22 +49,11 @@ pub struct Args {
     #[arg(long, conflicts_with = "sign_with_lab")]
     /// If using a seed phrase to sign, sets which hierarchical deterministic path to use, e.g. `m/44'/148'/{hd_path}`. Example: `--hd-path 1`. Default: `0`
     pub hd_path: Option<usize>,
-
-    /// If one of `--sign-with-*` flags is provided, don't ask to confirm to sign a transaction
-    #[arg(long)]
-    pub yes: bool,
-
-    /// Account that signs the transaction. Alias `source`. Can be an identity (--source alice), a secret key (--source SC36…), or a seed phrase (--source "kite urban…").
-    #[arg(long, visible_alias = "source", env = "STELLAR_ACCOUNT")]
-    pub source_account: String,
 }
 
 impl Args {
     pub fn secret(&self, locator: &locator::Args) -> Result<Secret, Error> {
-        let account = self
-            .sign_with_key
-            .as_deref()
-            .unwrap_or(&self.source_account);
+        let account = self.sign_with_key.as_deref().ok_or(Error::NoSignWithKey)?;
         Ok(locator.account(account)?)
     }
 
@@ -78,27 +62,19 @@ impl Args {
         tx: TransactionEnvelope,
         locator: &locator::Args,
         network: &Network,
+        quiet: bool,
     ) -> Result<TransactionEnvelope, Error> {
         let secret = self.secret(locator)?;
-        let signer = secret.signer(self.hd_path, !self.yes)?;
-        let source_account = self.source_account(locator)?;
-        self.sign_tx_env_with_signer(&signer, &source_account, tx, network)
-            .await
+        let signer = secret.signer(self.hd_path, false, quiet)?;
+        self.sign_tx_env_with_signer(&signer, tx, network).await
     }
 
     pub async fn sign_tx_env_with_signer(
         &self,
         signer: &(impl Transaction + std::marker::Sync),
-        source_account: &PublicKey,
         tx_env: TransactionEnvelope,
         network: &Network,
     ) -> Result<TransactionEnvelope, Error> {
-        Ok(sign_txn_env(signer, source_account, tx_env, &network).await?)
-    }
-
-    pub fn source_account(&self, locator: &locator::Args) -> Result<PublicKey, Error> {
-        Ok(locator
-            .account(&self.source_account)?
-            .public_key(self.hd_path)?)
+        Ok(sign_txn_env(signer, tx_env, network).await?)
     }
 }

cmd/soroban-cli/src/print.rs

@@ -3,8 +3,7 @@ use std::{env, fmt::Display};
 use soroban_env_host::xdr::{Error as XdrError, Transaction};
 
 use crate::{
-    config::network::Network,
-    utils::{explorer_url_for_transaction, transaction_hash},
+    config::network::Network, signer::types::transaction_hash, utils::explorer_url_for_transaction,
 };
 
 const TERMS: &[&str] = &["Apple_Terminal", "vscode"];

cmd/soroban-cli/src/signer.rs

@@ -11,7 +11,7 @@ use soroban_env_host::xdr::{
 };
 
 pub mod types;
-pub use types::{Blob, LocalKey, Transaction, TransactionHash};
+pub use types::{LocalKey, Transaction, TransactionHash};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {