From 5eedab5a3ea60692f74fe376874fdeadabdc2170 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Tue, 7 May 2024 14:53:57 -0400 Subject: [PATCH] fix: use sign_blob and share sign_txn_hash default method --- cmd/crates/stellar-ledger/src/lib.rs | 144 +++++++++++---------------- cmd/soroban-cli/src/signer.rs | 57 ++++++----- 2 files changed, 91 insertions(+), 110 deletions(-) diff --git a/cmd/crates/stellar-ledger/src/lib.rs b/cmd/crates/stellar-ledger/src/lib.rs index e5bde33dd..665cfcad4 100644 --- a/cmd/crates/stellar-ledger/src/lib.rs +++ b/cmd/crates/stellar-ledger/src/lib.rs @@ -412,20 +412,15 @@ fn bip_path_from_index(index: u32) -> Result { fn hd_path_to_bytes(hd_path: &slip10::BIP32Path) -> Result, LedgerError> { let hd_path_indices = 0..hd_path.depth(); - let mut result = Vec::with_capacity(hd_path.depth() as usize); - - for index in hd_path_indices { - let value = hd_path.index(index); - if let Some(v) = value { - let value_bytes = v.to_be_bytes(); - result.push(value_bytes); - } else { - return Err(LedgerError::Bip32PathError( - "Error getting index of hd path".to_string(), - )); - } - } - + let result = hd_path_indices + .into_iter() + .map(|index| { + Ok(hd_path + .index(index) + .ok_or_else(|| LedgerError::Bip32PathError(format!("{hd_path}")))? + .to_be_bytes()) + }) + .collect::, LedgerError>>()?; Ok(result.into_iter().flatten().collect()) } @@ -434,7 +429,6 @@ pub fn get_transport() -> Result { let hidapi = HidApi::new().map_err(LedgerError::HidApiError)?; TransportNativeHID::new(&hidapi).map_err(LedgerError::LedgerHidError) } - #[cfg(test)] mod test { use std::str::FromStr; @@ -473,25 +467,16 @@ mod test { }); let transport = EmulatorHttpTransport::new(&server.host(), server.port()); - let ledger_options = Some(LedgerOptions { + let ledger_options = LedgerOptions { exchange: transport, hd_path: slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(), - }); + }; - let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, ledger_options); - match ledger.get_public_key(0).await { - Ok(public_key) => { - let public_key_string = public_key.to_string(); - // This is determined by the seed phrase used to start up the emulator - let expected_public_key = - "GDUTHCF37UX32EMANXIL2WOOVEDZ47GHBTT3DYKU6EKM37SOIZXM2FN7"; - assert_eq!(public_key_string, expected_public_key); - } - Err(e) => { - println!("{e}"); - assert!(false); - } - } + let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, Some(ledger_options)); + let public_key = ledger.get_public_key(0).await.unwrap(); + let public_key_string = public_key.to_string(); + let expected_public_key = "GDUTHCF37UX32EMANXIL2WOOVEDZ47GHBTT3DYKU6EKM37SOIZXM2FN7"; + assert_eq!(public_key_string, expected_public_key); mock_server.assert(); } @@ -511,22 +496,15 @@ mod test { }); let transport = EmulatorHttpTransport::new(&server.host(), server.port()); - let ledger_options = Some(LedgerOptions { + let ledger_options = LedgerOptions { exchange: transport, hd_path: slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(), - }); - - let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, ledger_options); - match ledger.get_app_configuration().await { - Ok(config) => { - assert_eq!(config, vec![0, 5, 0, 3]); - } - Err(e) => { - println!("{e}"); - assert!(false); - } }; + let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, Some(ledger_options)); + let config = ledger.get_app_configuration().await.unwrap(); + assert_eq!(config, vec![0, 5, 0, 3]); + mock_server.assert(); } @@ -556,15 +534,15 @@ mod test { }); let transport = EmulatorHttpTransport::new(&server.host(), server.port()); - let ledger_options = Some(LedgerOptions { + let ledger_options = LedgerOptions { exchange: transport, hd_path: slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(), - }); - let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, ledger_options); + }; + let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, Some(ledger_options)); let path = slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(); - let fake_source_acct = [0 as u8; 32]; - let fake_dest_acct = [0 as u8; 32]; + let fake_source_acct = [0; 32]; + let fake_dest_acct = [0; 32]; let tx = Transaction { source_account: MuxedAccount::Ed25519(Uint256(fake_source_acct)), fee: 100, @@ -584,23 +562,18 @@ mod test { .unwrap(), }; - let result = ledger.sign_transaction(path, tx).await; - match result { - Ok(response) => { - assert_eq!( hex::encode(response), "5c2f8eb41e11ab922800071990a25cf9713cc6e7c43e50e0780ddc4c0c6da50c784609ef14c528a12f520d8ea9343b49083f59c51e3f28af8c62b3edeaade60e"); - } - Err(e) => { - println!("{e}"); - assert!(false); - } - }; + let response = ledger.sign_transaction(path, tx).await.unwrap(); + assert_eq!( + hex::encode(response), + "5c2f8eb41e11ab922800071990a25cf9713cc6e7c43e50e0780ddc4c0c6da50c784609ef14c528a12f520d8ea9343b49083f59c51e3f28af8c62b3edeaade60e" + ); + mock_request_1.assert(); mock_request_2.assert(); } #[tokio::test] async fn test_sign_tx_hash_when_hash_signing_is_not_enabled() { - //when hash signing isn't enabled on the device we expect an error let server = MockServer::start(); let mock_server = server.mock(|when, then| { when.method(POST) @@ -614,21 +587,23 @@ mod test { }); let transport = EmulatorHttpTransport::new(&server.host(), server.port()); - let ledger_options = Some(LedgerOptions { + let ledger_options = LedgerOptions { exchange: transport, hd_path: slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(), - }); - let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, ledger_options); + }; + let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, Some(ledger_options)); let path = slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(); let test_hash = b"3389e9f0f1a65f19736cacf544c2e825313e8447f569233bb8db39aa607c8889"; - let result = ledger.sign_transaction_hash(path, test_hash).await; - if let Err(LedgerError::APDUExchangeError(msg)) = result { + let err = ledger + .sign_transaction_hash(path, test_hash) + .await + .unwrap_err(); + if let LedgerError::APDUExchangeError(msg) = err { assert_eq!(msg, "Ledger APDU retcode: 0x6C66"); - // this error code is SW_TX_HASH_SIGNING_MODE_NOT_ENABLED https://github.com/LedgerHQ/app-stellar/blob/develop/docs/COMMANDS.md } else { - panic!("Unexpected result: {:?}", result); + panic!("Unexpected error: {err:?}"); } mock_server.assert(); @@ -649,34 +624,29 @@ mod test { }); let transport = EmulatorHttpTransport::new(&server.host(), server.port()); - let ledger_options: Option> = Some(LedgerOptions { + let ledger_options = LedgerOptions { exchange: transport, hd_path: slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(), - }); - let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, ledger_options); + }; + let ledger = LedgerSigner::new(TEST_NETWORK_PASSPHRASE, Some(ledger_options)); let path = slip10::BIP32Path::from_str("m/44'/148'/0'").unwrap(); let mut test_hash = vec![0u8; 32]; - match hex::decode_to_slice( + hex::decode_to_slice( "3389e9f0f1a65f19736cacf544c2e825313e8447f569233bb8db39aa607c8889", &mut test_hash as &mut [u8], - ) { - Ok(()) => {} - Err(e) => { - panic!("Unexpected result: {e}"); - } - } - - let result = ledger.sign_transaction_hash(path, &test_hash).await; - - match result { - Ok(response) => { - assert_eq!( hex::encode(response), "6970b9c9d3a6f4de7fb93e8d3920ec704fc4fece411873c40570015bbb1a60a197622bc3bf5644bb38ae73e1b96e4d487d716d142d46c7e944f008dece92df07"); - } - Err(e) => { - panic!("Unexpected result: {e}"); - } - } + ) + .unwrap(); + + let response = ledger + .sign_transaction_hash(path, &test_hash) + .await + .unwrap(); + + assert_eq!( + hex::encode(response), + "6970b9c9d3a6f4de7fb93e8d3920ec704fc4fece411873c40570015bbb1a60a197622bc3bf5644bb38ae73e1b96e4d487d716d142d46c7e944f008dece92df07" + ); mock_server.assert(); } diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index a6657b632..e0750d9a7 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -10,6 +10,25 @@ use soroban_env_host::xdr::{ TransactionV1Envelope, Uint256, WriteXdr, }; use stellar_ledger::NativeSigner; +use stellar_strkey::Strkey; + +trait ToBytes{ + fn to_bytes(&self) -> Vec; +} + +impl ToBytes for Strkey { + fn to_bytes(&self) -> Vec { + match self { + Strkey::PublicKeyEd25519(i) => i.0, + Strkey::PrivateKeyEd25519(i) => i.0, + Strkey::PreAuthTx(i) => i.0, + Strkey::HashX(i) => i.0, + Strkey::MuxedAccountEd25519(_) => todo!("Muxed account"), + Strkey::SignedPayloadEd25519(_) => todo!("Signed Payload"), + Strkey::Contract(i) => i.0, + }.to_vec() + } +} #[derive(thiserror::Error, Debug)] pub enum Error { @@ -50,11 +69,22 @@ pub trait Stellar { /// Sign a transaction hash with the given source account /// # Errors /// Returns an error if the source account is not found - fn sign_txn_hash( + async fn sign_txn_hash( &self, txn: [u8; 32], source_account: &stellar_strkey::Strkey, - ) -> impl std::future::Future> + Send; + ) -> Result{ + let tx_signature = self.sign_blob(&txn, source_account).await?; + Ok(DecoratedSignature { + // TODO: remove this unwrap. It's safe because we know the length of the array + hint: SignatureHint( + source_account.to_bytes()[28..] + .try_into() + .unwrap(), + ), + signature: Signature(tx_signature.try_into()?), + }) + } async fn sign_blob( &self, @@ -257,24 +287,6 @@ impl Stellar for InMemory { Ok(sig.to_bytes().to_vec()) } - async fn sign_txn_hash( - &self, - txn: [u8; 32], - source_account: &stellar_strkey::Strkey, - ) -> Result { - let source_account = self.get_key(source_account)?; - let tx_signature = source_account.sign(&txn); - Ok(DecoratedSignature { - // TODO: remove this unwrap. It's safe because we know the length of the array - hint: SignatureHint( - source_account.verifying_key().to_bytes()[28..] - .try_into() - .unwrap(), - ), - signature: Signature(tx_signature.to_bytes().try_into()?), - }) - } - fn network_hash(&self) -> xdr::Hash { xdr::Hash(Sha256::digest(self.network_passphrase.as_bytes()).into()) } @@ -299,9 +311,10 @@ impl Stellar for Box { async fn sign_blob( &self, data: &[u8], - _source_account: &stellar_strkey::Strkey, + _: &stellar_strkey::Strkey, ) -> Result, Error> { let index = self.as_ref().as_ref().hd_path.clone(); + eprintln!("You should see the following on your ledger:\n{}", hex::encode(data)); Ok(self.as_ref().as_ref().sign_blob(index, data).await?) } @@ -310,8 +323,6 @@ impl Stellar for Box { txn: [u8; 32], source_account: &stellar_strkey::Strkey, ) -> Result { - let hash = hex::encode(&txn); - eprintln!("You should see the following on your ledger:\n{hash}"); let index = self.as_ref().as_ref().hd_path.clone(); let res = self .as_ref()