diff --git a/cmd/crates/soroban-test/tests/it/emulator.rs b/cmd/crates/soroban-test/tests/it/emulator.rs index dd0878dbc..33e63f032 100644 --- a/cmd/crates/soroban-test/tests/it/emulator.rs +++ b/cmd/crates/soroban-test/tests/it/emulator.rs @@ -5,15 +5,21 @@ use std::sync::Arc; use stellar_ledger::emulator_test_support::*; -use soroban_cli::{ - tx::builder::TxExt, - xdr::{self, Limits, OperationBody, ReadXdr, TransactionEnvelope, WriteXdr}, +use soroban_cli::xdr::{ + self, Limits, ReadXdr, TransactionEnvelope, TransactionV1Envelope, VecM, WriteXdr, }; use test_case::test_case; const HELLO_WORLD: &Wasm = &Wasm::Custom("test-wasms", "test_hello_world"); +// Sign a classic Payment envelope with a Ledger identity end-to-end. After the +// blind-signing fix the CLI sends the full `TransactionSignaturePayload` to the +// device via APDU `INS=0x04` (SIGN_TX), so the user approves the parsed +// operation — not a hex hash. The Speculos approval flow used here +// (`approve_tx_signature`) and the transaction shape mirror stellar-ledger's +// `test_sign_tx`, whose Speculos click counts are calibrated for this exact +// Payment + memo layout. #[test_case("nanos", 0; "when the device is NanoS")] #[test_case("nanox", 1; "when the device is NanoX")] #[test_case("nanosp", 2; "when the device is NanoS Plus")] @@ -27,23 +33,41 @@ async fn test_signer(ledger_device_model: &str, hd_path: u32) { let ledger = ledger(host_port).await; let key = ledger.get_public_key(&hd_path.into()).await.unwrap(); - let verifying_key = ed25519_dalek::VerifyingKey::from_bytes(&key.0).unwrap(); - let body: OperationBody = - (&soroban_cli::commands::tx::new::bump_sequence::Args { bump_to: 100 }).into(); - let operation = xdr::Operation { - body, - source_account: None, - }; + + let destination = stellar_strkey::ed25519::PublicKey::from_string( + "GCKUD4BHIYSAYHU7HBB5FDSW6CSYH3GSOUBPWD2KE7KNBERP4BSKEJDV", + ) + .unwrap(); let source_account = xdr::MuxedAccount::Ed25519(key.0.into()); - let tx_env: TransactionEnvelope = - xdr::Transaction::new_tx(source_account, 100, 100, operation).into(); - let tx_env = tx_env.to_xdr_base64(Limits::none()).unwrap(); + let tx = xdr::Transaction { + source_account: source_account.clone(), + fee: 100, + seq_num: xdr::SequenceNumber(1), + cond: xdr::Preconditions::None, + memo: xdr::Memo::Text("Stellar".try_into().unwrap()), + ext: xdr::TransactionExt::V0, + operations: [xdr::Operation { + source_account: Some(source_account), + body: xdr::OperationBody::Payment(xdr::PaymentOp { + destination: xdr::MuxedAccount::Ed25519(destination.0.into()), + asset: xdr::Asset::Native, + amount: 100, + }), + }] + .try_into() + .unwrap(), + }; + let tx_env: TransactionEnvelope = TransactionEnvelope::Tx(TransactionV1Envelope { + tx, + signatures: VecM::default(), + }); + let tx_env_b64 = tx_env.to_xdr_base64(Limits::none()).unwrap(); let hash: xdr::Hash = sandbox .new_assert_cmd("tx") .arg("hash") - .write_stdin(tx_env.as_bytes()) + .write_stdin(tx_env_b64.as_bytes()) .assert() .success() .stdout_as_str() @@ -52,6 +76,7 @@ async fn test_signer(ledger_device_model: &str, hd_path: u32) { let sign = tokio::task::spawn_blocking({ let sandbox = Arc::clone(&sandbox); + let tx_env_b64 = tx_env_b64.clone(); move || { sandbox @@ -60,7 +85,7 @@ async fn test_signer(ledger_device_model: &str, hd_path: u32) { .arg("--sign-with-ledger") .arg("--hd-path") .arg(hd_path.to_string()) - .write_stdin(tx_env.as_bytes()) + .write_stdin(tx_env_b64.as_bytes()) .env("SPECULOS_PORT", host_port.to_string()) .env("RUST_LOGS", "trace") .assert() @@ -69,7 +94,7 @@ async fn test_signer(ledger_device_model: &str, hd_path: u32) { } }); - let approve = tokio::task::spawn(approve_tx_hash_signature( + let approve = tokio::task::spawn(approve_tx_signature( ui_host_port, ledger_device_model.to_string(), )); diff --git a/cmd/crates/stellar-ledger/src/emulator_test_support/util.rs b/cmd/crates/stellar-ledger/src/emulator_test_support/util.rs index 053e7e3f3..ecfe770a8 100644 --- a/cmd/crates/stellar-ledger/src/emulator_test_support/util.rs +++ b/cmd/crates/stellar-ledger/src/emulator_test_support/util.rs @@ -192,20 +192,29 @@ pub async fn get_emulator_events_with_retries( } } -pub async fn approve_tx_hash_signature(ui_host_port: u16, device_model: String) { +pub async fn approve_tx_hash_signature(ui_host_port: u16, _device_model: String) { wait_for_review_transaction_text(ui_host_port).await; - let number_of_right_clicks = if device_model == "nanos" { 10 } else { 6 }; - for _ in 0..number_of_right_clicks { - click(ui_host_port, "button/right").await; - } + advance_to_approve_and_confirm(ui_host_port).await; +} - click(ui_host_port, "button/both").await; +pub async fn approve_tx_signature(ui_host_port: u16, _device_model: String) { + wait_for_review_transaction_text(ui_host_port).await; + advance_to_approve_and_confirm(ui_host_port).await; } -pub async fn approve_tx_signature(ui_host_port: u16, device_model: String) { - let number_of_right_clicks = if device_model == "nanos" { 17 } else { 11 }; - for _ in 0..number_of_right_clicks { +// Right-click through the device review screens until the on-screen text +// shows "Approve", then click both buttons to confirm. Replaces hard-coded +// click counts that needed recalibration for every change in transaction +// shape, device model, or app version. +async fn advance_to_approve_and_confirm(ui_host_port: u16) { + const MAX_CLICKS: usize = 50; + for _ in 0..MAX_CLICKS { + let events = get_emulator_events(ui_host_port).await; + if events.iter().any(|event| event.text == "Approve") { + click(ui_host_port, "button/both").await; + return; + } click(ui_host_port, "button/right").await; } - click(ui_host_port, "button/both").await; + panic!("Approve screen not reached after {MAX_CLICKS} right-clicks"); } diff --git a/cmd/crates/stellar-ledger/src/lib.rs b/cmd/crates/stellar-ledger/src/lib.rs index 5f9004a9c..4e74db4d6 100644 --- a/cmd/crates/stellar-ledger/src/lib.rs +++ b/cmd/crates/stellar-ledger/src/lib.rs @@ -12,7 +12,7 @@ pub use ledger_transport_hid::TransportNativeHID; use std::vec; use stellar_strkey::DecodeError; use stellar_xdr::curr::{ - self as xdr, Hash, Limits, Transaction, TransactionSignaturePayload, + self as xdr, FeeBumpTransaction, Hash, Limits, Transaction, TransactionSignaturePayload, TransactionSignaturePayloadTaggedTransaction, WriteXdr, }; @@ -149,14 +149,43 @@ where /// Sign a Stellar transaction with the account on the Ledger device /// # Errors /// Returns an error if there is an issue with connecting with the device or signing the given tx on the device - #[allow(clippy::missing_panics_doc)] pub async fn sign_transaction( &self, hd_path: impl Into, transaction: Transaction, network_id: Hash, ) -> Result, Error> { - let tagged_transaction = TransactionSignaturePayloadTaggedTransaction::Tx(transaction); + self.sign_tagged_transaction( + hd_path, + TransactionSignaturePayloadTaggedTransaction::Tx(transaction), + network_id, + ) + .await + } + + /// Sign a Stellar fee-bump transaction with the account on the Ledger device. + /// # Errors + /// Returns an error if there is an issue with connecting with the device or signing the given tx on the device + pub async fn sign_fee_bump_transaction( + &self, + hd_path: impl Into, + transaction: FeeBumpTransaction, + network_id: Hash, + ) -> Result, Error> { + self.sign_tagged_transaction( + hd_path, + TransactionSignaturePayloadTaggedTransaction::TxFeeBump(transaction), + network_id, + ) + .await + } + + async fn sign_tagged_transaction( + &self, + hd_path: impl Into, + tagged_transaction: TransactionSignaturePayloadTaggedTransaction, + network_id: Hash, + ) -> Result, Error> { let signature_payload = TransactionSignaturePayload { network_id, tagged_transaction, @@ -346,7 +375,10 @@ mod test { use crate::{test_network_hash, Error, LedgerSigner}; use stellar_xdr::curr::{ - Memo, MuxedAccount, PaymentOp, Preconditions, SequenceNumber, TransactionExt, + FeeBumpTransaction, FeeBumpTransactionExt, FeeBumpTransactionInnerTx, Limits, Memo, + MuxedAccount, PaymentOp, Preconditions, SequenceNumber, TransactionExt, + TransactionSignaturePayload, TransactionSignaturePayloadTaggedTransaction, + TransactionV1Envelope, VecM, WriteXdr, }; fn ledger(server: &MockServer) -> LedgerSigner { @@ -457,6 +489,105 @@ mod test { mock_request_2.assert(); } + #[tokio::test] + async fn test_sign_fee_bump_tx() { + // Wraps the Payment from `test_sign_tx` in a FeeBumpTransaction and + // signs the outer envelope. Exercises the new `sign_fee_bump_transaction` + // path, which differs from `sign_transaction` only in the + // TaggedTransaction discriminator (`TxFeeBump` vs `Tx`); the chunking + // and APDU framing are shared via `sign_tagged_transaction`. + + let fake_acct = [0; 32]; + let inner_tx = Transaction { + source_account: MuxedAccount::Ed25519(Uint256(fake_acct)), + fee: 100, + seq_num: SequenceNumber(1), + cond: Preconditions::None, + memo: Memo::Text("Stellar".as_bytes().try_into().unwrap()), + ext: TransactionExt::V0, + operations: [Operation { + source_account: Some(MuxedAccount::Ed25519(Uint256(fake_acct))), + body: OperationBody::Payment(PaymentOp { + destination: MuxedAccount::Ed25519(Uint256(fake_acct)), + asset: xdr::Asset::Native, + amount: 100, + }), + }] + .try_into() + .unwrap(), + }; + + let fee_source = [1u8; 32]; + let fee_bump_tx = FeeBumpTransaction { + fee_source: MuxedAccount::Ed25519(Uint256(fee_source)), + fee: 200, + inner_tx: FeeBumpTransactionInnerTx::Tx(TransactionV1Envelope { + tx: inner_tx, + signatures: VecM::default(), + }), + ext: FeeBumpTransactionExt::V0, + }; + + // Build the expected APDU chunks the same way `sign_tagged_transaction` + // does, so the mock can match exact request bodies. + let payload = TransactionSignaturePayload { + network_id: test_network_hash(), + tagged_transaction: TransactionSignaturePayloadTaggedTransaction::TxFeeBump( + fee_bump_tx.clone(), + ), + }; + let payload_bytes = payload.to_xdr(Limits::none()).unwrap(); + let mut data = vec![super::HD_PATH_ELEMENTS_COUNT]; + // HD path for index 0: m/44'/148'/0' (hardened). + data.extend_from_slice(&[0x80, 0, 0, 0x2c, 0x80, 0, 0, 0x94, 0x80, 0, 0, 0]); + data.extend(&payload_bytes); + let chunks: Vec> = data + .chunks(super::CHUNK_SIZE as usize) + .map(<[u8]>::to_vec) + .collect(); + assert_eq!( + chunks.len(), + 2, + "fee-bump payload should split into two SIGN_TX chunks" + ); + let apdu1 = format!("e0040080{:02x}{}", chunks[0].len(), hex::encode(&chunks[0])); + let apdu2 = format!("e0048000{:02x}{}", chunks[1].len(), hex::encode(&chunks[1])); + + let expected_sig = "5c2f8eb41e11ab922800071990a25cf9713cc6e7c43e50e0780ddc4c0c6da50c784609ef14c528a12f520d8ea9343b49083f59c51e3f28af8c62b3edeaade60e"; + + let server = MockServer::start(); + let mock_request_1 = server.mock(|when, then| { + when.method(POST) + .path("/") + .header("accept", "application/json") + .header("content-type", "application/json") + .json_body(json!({ "apduHex": apdu1 })); + then.status(200) + .header("content-type", "application/json") + .json_body(json!({ "data": "9000" })); + }); + let mock_request_2 = server.mock(|when, then| { + when.method(POST) + .path("/") + .header("accept", "application/json") + .header("content-type", "application/json") + .json_body(json!({ "apduHex": apdu2 })); + then.status(200) + .header("content-type", "application/json") + .json_body(json!({ "data": format!("{expected_sig}9000") })); + }); + + let ledger = ledger(&server); + let response = ledger + .sign_fee_bump_transaction(0, fee_bump_tx, test_network_hash()) + .await + .unwrap(); + assert_eq!(hex::encode(response), expected_sig); + + mock_request_1.assert(); + mock_request_2.assert(); + } + #[tokio::test] async fn test_sign_tx_hash_when_hash_signing_is_not_enabled() { let server = MockServer::start(); diff --git a/cmd/soroban-cli/src/signer/ledger.rs b/cmd/soroban-cli/src/signer/ledger.rs index 957716968..fe09276d5 100644 --- a/cmd/soroban-cli/src/signer/ledger.rs +++ b/cmd/soroban-cli/src/signer/ledger.rs @@ -1,7 +1,38 @@ use crate::xdr; +#[cfg(feature = "additional-libs")] +use crate::xdr::{OperationBody, Transaction, TransactionEnvelope}; pub use ledger_impl::*; +// Operations the Ledger Stellar app cannot pretty-print. When any of these +// appears in the envelope, the device falls into hash-signing mode (requires +// `Hash Signing` enabled in app settings); sending `SIGN_TX` (0x04) for them +// ends up at the same UX as `SIGN_TX_HASH` (0x08) but with extra device-side +// parsing churn, so the CLI sends the hash directly. +#[cfg(feature = "additional-libs")] +pub(super) fn is_soroban_tx(tx: &Transaction) -> bool { + tx.operations.iter().any(|op| { + matches!( + op.body, + OperationBody::InvokeHostFunction(_) + | OperationBody::ExtendFootprintTtl(_) + | OperationBody::RestoreFootprint(_), + ) + }) +} + +#[cfg(feature = "additional-libs")] +pub(super) fn is_soroban_tx_env(tx_env: &TransactionEnvelope) -> bool { + match tx_env { + TransactionEnvelope::Tx(v1) => is_soroban_tx(&v1.tx), + TransactionEnvelope::TxFeeBump(fb) => { + let xdr::FeeBumpTransactionInnerTx::Tx(inner) = &fb.tx.inner_tx; + is_soroban_tx(&inner.tx) + } + TransactionEnvelope::TxV0(_) => false, + } +} + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Ledger Device keys are not allowed: additional-libs feature must be enabled")] @@ -16,15 +47,26 @@ pub enum Error { #[error(transparent)] Xdr(#[from] xdr::Error), + + #[error("Transaction envelope type not supported for Ledger signing")] + UnsupportedTransactionEnvelopeType, } #[cfg(feature = "additional-libs")] mod ledger_impl { - use super::Error; - use crate::xdr::{DecoratedSignature, Hash, Signature, SignatureHint, Transaction}; + use super::{is_soroban_tx_env, Error}; + use crate::{ + print::Print, + utils::transaction_env_hash, + xdr::{ + DecoratedSignature, Hash, Signature, SignatureHint, TransactionEnvelope, + TransactionV1Envelope, + }, + }; use ed25519_dalek::Signature as Ed25519Signature; use sha2::{Digest, Sha256}; use stellar_ledger::{Blob as _, Exchange, LedgerSigner}; + use stellar_xdr::curr::FeeBumpTransactionEnvelope; #[cfg(not(feature = "emulator-tests"))] pub type LedgerType = Ledger; @@ -41,22 +83,67 @@ mod ledger_impl { } impl LedgerEntry { - pub async fn sign_tx_hash(&self, tx_hash: [u8; 32]) -> Result { + // Sign a transaction envelope on the Ledger device. + // + // Classic envelopes are clear-signed (APDU SIGN_TX, 0x04): the full + // `TransactionSignaturePayload` is sent so the device parses and + // displays each operation for verification. + // + // Soroban envelopes (envelopes containing `InvokeHostFunction`, + // `ExtendFootprintTtl`, or `RestoreFootprint`) are blind-signed (APDU + // SIGN_TX_HASH, 0x08): the Ledger Stellar app cannot pretty-print + // those operations, so the device shows the transaction hash and + // requires `Hash Signing` enabled in app settings. + pub async fn sign_tx_env( + &self, + tx_env: &TransactionEnvelope, + network_passphrase: &str, + print: &Print, + ) -> Result { let live = new(self.hd_path).await?; let key = match self.public_key { Some(pk) => pk, None => live.public_key().await?, }; let hint = SignatureHint(key.0[28..].try_into()?); - let signature = Signature( + + let signature_bytes = if is_soroban_tx_env(tx_env) { + let tx_hash = transaction_env_hash(tx_env, network_passphrase)?; + print.infoln(format!( + "Approve the transaction {} on your Ledger device…", + hex::encode(tx_hash), + )); live.signer .sign_transaction_hash(live.index, &tx_hash) .await? - .try_into()?, - ); - Ok(DecoratedSignature { hint, signature }) + } else { + print.infoln("Approve the transaction on your Ledger device…"); + let network_id = Hash(Sha256::digest(network_passphrase).into()); + match tx_env { + TransactionEnvelope::Tx(TransactionV1Envelope { tx, .. }) => { + live.signer + .sign_transaction(live.index, tx.clone(), network_id) + .await? + } + TransactionEnvelope::TxFeeBump(FeeBumpTransactionEnvelope { tx, .. }) => { + live.signer + .sign_fee_bump_transaction(live.index, tx.clone(), network_id) + .await? + } + TransactionEnvelope::TxV0(_) => { + return Err(Error::UnsupportedTransactionEnvelopeType); + } + } + }; + + Ok(DecoratedSignature { + hint, + signature: Signature(signature_bytes.try_into()?), + }) } + // Blind-sign a 32-byte payload. Used for Soroban authorization-entry + // preimage digests, which have no on-device pretty-print. pub async fn sign_payload(&self, payload: [u8; 32]) -> Result { let live = new(self.hd_path).await?; let bytes = live @@ -102,22 +189,6 @@ mod ledger_impl { } impl Ledger { - pub async fn sign_transaction( - &self, - tx: Transaction, - network_passphrase: &str, - ) -> Result { - let network_id = Hash(Sha256::digest(network_passphrase).into()); - let signature = self - .signer - .sign_transaction(self.index, tx, network_id) - .await?; - let key = self.public_key().await?; - let hint = SignatureHint(key.0[28..].try_into()?); - let signature = Signature(signature.try_into()?); - Ok(DecoratedSignature { hint, signature }) - } - pub async fn public_key(&self) -> Result { Ok(self.signer.get_public_key(&self.index.into()).await?) } @@ -127,7 +198,10 @@ mod ledger_impl { #[cfg(not(feature = "additional-libs"))] mod ledger_impl { use super::Error; - use crate::xdr::{DecoratedSignature, Transaction}; + use crate::{ + print::Print, + xdr::{DecoratedSignature, TransactionEnvelope}, + }; use ed25519_dalek::Signature as Ed25519Signature; use std::marker::PhantomData; @@ -145,7 +219,12 @@ mod ledger_impl { impl LedgerEntry { #[allow(clippy::unused_async)] - pub async fn sign_tx_hash(&self, _tx_hash: [u8; 32]) -> Result { + pub async fn sign_tx_env( + &self, + _tx_env: &TransactionEnvelope, + _network_passphrase: &str, + _print: &Print, + ) -> Result { Err(Error::FeatureNotEnabled) } @@ -161,15 +240,6 @@ mod ledger_impl { } impl Ledger { - #[allow(clippy::unused_async)] - pub async fn sign_transaction( - &self, - _tx: Transaction, - _network_passphrase: &str, - ) -> Result { - Err(Error::FeatureNotEnabled) - } - #[allow(clippy::unused_async)] pub async fn public_key(&self) -> Result { Err(Error::FeatureNotEnabled) diff --git a/cmd/soroban-cli/src/signer/mod.rs b/cmd/soroban-cli/src/signer/mod.rs index 87b93bab2..3f4a06e52 100644 --- a/cmd/soroban-cli/src/signer/mod.rs +++ b/cmd/soroban-cli/src/signer/mod.rs @@ -1,6 +1,5 @@ use crate::{ signer::ledger::LedgerEntry, - utils::fee_bump_transaction_hash, xdr::{ self, AccountId, DecoratedSignature, FeeBumpTransactionEnvelope, Hash, HashIdPreimage, HashIdPreimageSorobanAuthorization, Limits, Operation, OperationBody, PublicKey, ScAddress, @@ -12,7 +11,7 @@ use crate::{ use ed25519_dalek::{ed25519::signature::Signer as _, Signature as Ed25519Signature}; use sha2::{Digest, Sha256}; -use crate::{config::network::Network, print::Print, utils::transaction_hash}; +use crate::{config::network::Network, print::Print, utils::transaction_env_hash}; pub mod ledger; @@ -240,12 +239,12 @@ impl Signer { tx_env: &TransactionEnvelope, network: &Network, ) -> Result { - match &tx_env { + if matches!(tx_env, TransactionEnvelope::TxV0(_)) { + return Err(Error::UnsupportedTransactionEnvelopeType); + } + let decorated_signature = self.produce_signature(tx_env, network).await?; + match tx_env { TransactionEnvelope::Tx(TransactionV1Envelope { tx, signatures }) => { - let tx_hash = transaction_hash(tx, &network.network_passphrase)?; - self.print - .infoln(format!("Signing transaction: {}", hex::encode(tx_hash))); - let decorated_signature = self.sign_tx_hash(tx_hash, tx_env, network).await?; let mut sigs = signatures.clone().into_vec(); sigs.push(decorated_signature); Ok(TransactionEnvelope::Tx(TransactionV1Envelope { @@ -254,12 +253,6 @@ impl Signer { })) } TransactionEnvelope::TxFeeBump(FeeBumpTransactionEnvelope { tx, signatures }) => { - let tx_hash = fee_bump_transaction_hash(tx, &network.network_passphrase)?; - self.print.infoln(format!( - "Signing fee bump transaction: {}", - hex::encode(tx_hash), - )); - let decorated_signature = self.sign_tx_hash(tx_hash, tx_env, network).await?; let mut sigs = signatures.clone().into_vec(); sigs.push(decorated_signature); Ok(TransactionEnvelope::TxFeeBump(FeeBumpTransactionEnvelope { @@ -293,21 +286,39 @@ impl Signer { } } - async fn sign_tx_hash( + async fn produce_signature( &self, - tx_hash: [u8; 32], tx_env: &TransactionEnvelope, network: &Network, ) -> Result { match &self.kind { - SignerKind::Local(key) => key.sign_tx_hash(tx_hash), + SignerKind::Ledger(ledger) => ledger + .sign_tx_env(tx_env, &network.network_passphrase, &self.print) + .await + .map_err(Error::from), SignerKind::Lab => Lab::sign_tx_env(tx_env, network, &self.print), - SignerKind::Ledger(ledger) => ledger.sign_tx_hash(tx_hash).await.map_err(Error::from), - SignerKind::SecureStore(entry) => entry.sign_tx_hash(tx_hash), + SignerKind::Local(key) => { + let tx_hash = transaction_env_hash(tx_env, &network.network_passphrase)?; + self.print.infoln(format_signing_message(tx_env, tx_hash)); + key.sign_tx_hash(tx_hash) + } + SignerKind::SecureStore(entry) => { + let tx_hash = transaction_env_hash(tx_env, &network.network_passphrase)?; + self.print.infoln(format_signing_message(tx_env, tx_hash)); + entry.sign_tx_hash(tx_hash) + } } } } +fn format_signing_message(tx_env: &TransactionEnvelope, tx_hash: [u8; 32]) -> String { + let label = match tx_env { + TransactionEnvelope::TxFeeBump(_) => "fee bump transaction", + _ => "transaction", + }; + format!("Signing {label}: {}", hex::encode(tx_hash)) +} + pub struct LocalKey { pub key: ed25519_dalek::SigningKey, }