From 3b7f426e8c0f6258339ad09cbe69feb115482dca Mon Sep 17 00:00:00 2001 From: Rachit2323 Date: Sat, 2 May 2026 13:33:58 +0530 Subject: [PATCH 1/2] Replace todo!() panics with structured errors for unsupported ScAddress types in signer Three ScAddress variants (MuxedAccount, ClaimableBalance, LiquidityPool) were hitting todo!() macros which panic and crash the CLI process when encountered in Soroban authorization entries. Replace with proper Error variants so the CLI exits cleanly with a descriptive message instead of crashing. Also adds a unit test to prove the behavior is intentional. --- cmd/soroban-cli/src/signer/mod.rs | 77 +++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/cmd/soroban-cli/src/signer/mod.rs b/cmd/soroban-cli/src/signer/mod.rs index 4e1578b55..e39aa8e6d 100644 --- a/cmd/soroban-cli/src/signer/mod.rs +++ b/cmd/soroban-cli/src/signer/mod.rs @@ -27,6 +27,12 @@ pub enum Error { Ed25519(#[from] ed25519_dalek::SignatureError), #[error("Missing signing key for account {address}")] MissingSignerForAddress { address: String }, + #[error("muxed addresses are not supported for Soroban authorization signing")] + MuxedAddressNotSupported, + #[error("claimable balance addresses are not supported for Soroban authorization signing")] + ClaimableBalanceNotSupported, + #[error("liquidity pool addresses are not supported for Soroban authorization signing")] + LiquidityPoolNotSupported, #[error(transparent)] TryFromSlice(#[from] std::array::TryFromSliceError), #[error("User cancelled signing, perhaps need to add -y")] @@ -91,9 +97,9 @@ pub fn sign_soroban_authorizations( // See if we have a signer for this authorizationEntry // If not, then we Error let needle: &[u8; 32] = match address { - ScAddress::MuxedAccount(_) => todo!("muxed accounts are not supported"), - ScAddress::ClaimableBalance(_) => todo!("claimable balance not supported"), - ScAddress::LiquidityPool(_) => todo!("liquidity pool not supported"), + ScAddress::MuxedAccount(_) => return Err(Error::MuxedAddressNotSupported), + ScAddress::ClaimableBalance(_) => return Err(Error::ClaimableBalanceNotSupported), + ScAddress::LiquidityPool(_) => return Err(Error::LiquidityPoolNotSupported), ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(ref a)))) => a, ScAddress::Contract(stellar_xdr::curr::ContractId(Hash(c))) => { // This address is for a contract. This means we're using a custom @@ -379,3 +385,68 @@ impl SecureStoreEntry { Ok(sig) } } + +#[cfg(test)] +mod tests { + use super::*; + use xdr::{ + Hash, InvokeContractArgs, InvokeHostFunctionOp, Memo, MuxedEd25519Account, Operation, + OperationBody, Preconditions, ScAddress, ScSymbol, ScVal, SequenceNumber, + SorobanAddressCredentials, SorobanAuthorizationEntry, SorobanAuthorizedFunction, + SorobanAuthorizedInvocation, SorobanCredentials, Transaction, Uint256, VecM, + }; + + fn make_tx_with_muxed_auth() -> Transaction { + let auth_entry = SorobanAuthorizationEntry { + credentials: SorobanCredentials::Address(SorobanAddressCredentials { + address: ScAddress::MuxedAccount(MuxedEd25519Account { + id: 12345, + ed25519: Uint256([1u8; 32]), + }), + nonce: 0, + signature_expiration_ledger: 999, + signature: ScVal::Void, + }), + root_invocation: SorobanAuthorizedInvocation { + function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { + contract_address: ScAddress::Contract(xdr::ContractId(Hash([0u8; 32]))), + function_name: ScSymbol("transfer".try_into().unwrap()), + args: VecM::default(), + }), + sub_invocations: VecM::default(), + }, + }; + + Transaction { + source_account: xdr::MuxedAccount::Ed25519(Uint256([1u8; 32])), + fee: 100, + seq_num: SequenceNumber(1), + cond: Preconditions::None, + memo: Memo::None, + operations: vec![Operation { + source_account: None, + body: OperationBody::InvokeHostFunction(InvokeHostFunctionOp { + host_function: xdr::HostFunction::InvokeContract(InvokeContractArgs { + contract_address: ScAddress::Contract(xdr::ContractId(Hash([0u8; 32]))), + function_name: ScSymbol("transfer".try_into().unwrap()), + args: VecM::default(), + }), + auth: vec![auth_entry].try_into().unwrap(), + }), + }] + .try_into() + .unwrap(), + ext: xdr::TransactionExt::V0, + } + } + + #[test] + fn test_muxed_account_returns_error_not_panic() { + let tx = make_tx_with_muxed_auth(); + let result = sign_soroban_authorizations(&tx, &[], 999, "Test SDF Network ; September 2015"); + assert!( + matches!(result, Err(Error::MuxedAddressNotSupported)), + "expected MuxedAddressNotSupported error, got: {result:?}" + ); + } +} From 6f9524fa3833afd829bd126f4de755da8c8faa3b Mon Sep 17 00:00:00 2001 From: Rachit2323 Date: Sat, 2 May 2026 14:02:51 +0530 Subject: [PATCH 2/2] Add tests for ClaimableBalance and LiquidityPool error cases in signer --- cmd/soroban-cli/src/signer/mod.rs | 44 +++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/cmd/soroban-cli/src/signer/mod.rs b/cmd/soroban-cli/src/signer/mod.rs index e39aa8e6d..1c2165462 100644 --- a/cmd/soroban-cli/src/signer/mod.rs +++ b/cmd/soroban-cli/src/signer/mod.rs @@ -390,19 +390,17 @@ impl SecureStoreEntry { mod tests { use super::*; use xdr::{ - Hash, InvokeContractArgs, InvokeHostFunctionOp, Memo, MuxedEd25519Account, Operation, - OperationBody, Preconditions, ScAddress, ScSymbol, ScVal, SequenceNumber, - SorobanAddressCredentials, SorobanAuthorizationEntry, SorobanAuthorizedFunction, - SorobanAuthorizedInvocation, SorobanCredentials, Transaction, Uint256, VecM, + ClaimableBalanceId, Hash, InvokeContractArgs, InvokeHostFunctionOp, Memo, + MuxedEd25519Account, Operation, OperationBody, PoolId, Preconditions, ScAddress, ScSymbol, + ScVal, SequenceNumber, SorobanAddressCredentials, SorobanAuthorizationEntry, + SorobanAuthorizedFunction, SorobanAuthorizedInvocation, SorobanCredentials, Transaction, + Uint256, VecM, }; - fn make_tx_with_muxed_auth() -> Transaction { + fn make_tx_with_auth(address: ScAddress) -> Transaction { let auth_entry = SorobanAuthorizationEntry { credentials: SorobanCredentials::Address(SorobanAddressCredentials { - address: ScAddress::MuxedAccount(MuxedEd25519Account { - id: 12345, - ed25519: Uint256([1u8; 32]), - }), + address, nonce: 0, signature_expiration_ledger: 999, signature: ScVal::Void, @@ -416,7 +414,6 @@ mod tests { sub_invocations: VecM::default(), }, }; - Transaction { source_account: xdr::MuxedAccount::Ed25519(Uint256([1u8; 32])), fee: 100, @@ -442,11 +439,36 @@ mod tests { #[test] fn test_muxed_account_returns_error_not_panic() { - let tx = make_tx_with_muxed_auth(); + let tx = make_tx_with_auth(ScAddress::MuxedAccount(MuxedEd25519Account { + id: 12345, + ed25519: Uint256([1u8; 32]), + })); let result = sign_soroban_authorizations(&tx, &[], 999, "Test SDF Network ; September 2015"); assert!( matches!(result, Err(Error::MuxedAddressNotSupported)), "expected MuxedAddressNotSupported error, got: {result:?}" ); } + + #[test] + fn test_claimable_balance_returns_error_not_panic() { + let tx = make_tx_with_auth(ScAddress::ClaimableBalance( + ClaimableBalanceId::ClaimableBalanceIdTypeV0(Hash([0u8; 32])), + )); + let result = sign_soroban_authorizations(&tx, &[], 999, "Test SDF Network ; September 2015"); + assert!( + matches!(result, Err(Error::ClaimableBalanceNotSupported)), + "expected ClaimableBalanceNotSupported error, got: {result:?}" + ); + } + + #[test] + fn test_liquidity_pool_returns_error_not_panic() { + let tx = make_tx_with_auth(ScAddress::LiquidityPool(PoolId(Hash([0u8; 32])))); + let result = sign_soroban_authorizations(&tx, &[], 999, "Test SDF Network ; September 2015"); + assert!( + matches!(result, Err(Error::LiquidityPoolNotSupported)), + "expected LiquidityPoolNotSupported error, got: {result:?}" + ); + } }