Skip to content

Commit 576a81d

Browse files
committed
fix: return a structured error instead of panicking on non-authorizer auth entry addresses
An auth entry whose credential address is a muxed account, claimable balance, or liquidity pool crashed the CLI with a raw `todo!()` panic and a backtrace. These ScAddress variants are values, not valid authorizers, so signing now fails with a diagnosable error naming the address kind and its strkey. Also refreshes the resolve_secret comment that pointed at the removed `todo!`. Part of #2534
1 parent 1e3bc83 commit 576a81d

2 files changed

Lines changed: 87 additions & 5 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ impl UnresolvedMuxedAccount {
8585
// A literal public key has no secret on its own, but a stored
8686
// identity may hold the matching key. Scan identities by public key
8787
// so `G...` signs like its alias would; fall back to `CannotSign`
88-
// when nothing matches. Muxed accounts (`M...`) aren't signable
89-
// end-to-end yet (see the `todo!` in `sign_soroban_authorizations`),
88+
// when nothing matches. Muxed accounts (`M...`) carry no secret of
89+
// their own and are not valid authorizers (see
90+
// `Error::UnsupportedAuthAddress` in `sign_soroban_authorizations`),
9091
// so they keep returning `CannotSign`.
9192
UnresolvedMuxedAccount::Resolved(muxed_account) => {
9293
let xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) = muxed_account else {

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

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ pub mod secure_store;
2929
pub enum Error {
3030
#[error("Contract addresses are not supported to sign auth entries {address}")]
3131
ContractAddressAreNotSupported { address: String },
32+
#[error("{kind} addresses cannot authorize Soroban invocations: {address}")]
33+
UnsupportedAuthAddress { kind: &'static str, address: String },
3234
#[error(transparent)]
3335
Ed25519(#[from] ed25519_dalek::SignatureError),
3436
#[error("Missing signing key for account {address}")]
@@ -130,9 +132,28 @@ pub async fn sign_soroban_authorizations(
130132
// See if we have a signer for this authorizationEntry
131133
// If not, then we Error
132134
let auth_address_bytes: &[u8; 32] = match address {
133-
ScAddress::MuxedAccount(_) => todo!("muxed accounts are not supported"),
134-
ScAddress::ClaimableBalance(_) => todo!("claimable balance not supported"),
135-
ScAddress::LiquidityPool(_) => todo!("liquidity pool not supported"),
135+
// Only account (G…) and contract addresses can authorize an
136+
// invocation; the remaining ScAddress variants are values, not
137+
// authorizers, so reject them with a structured error instead of
138+
// panicking (#2534).
139+
ScAddress::MuxedAccount(_) => {
140+
return Err(Error::UnsupportedAuthAddress {
141+
kind: "Muxed (M…)",
142+
address: address.to_string(),
143+
})
144+
}
145+
ScAddress::ClaimableBalance(_) => {
146+
return Err(Error::UnsupportedAuthAddress {
147+
kind: "Claimable balance",
148+
address: address.to_string(),
149+
})
150+
}
151+
ScAddress::LiquidityPool(_) => {
152+
return Err(Error::UnsupportedAuthAddress {
153+
kind: "Liquidity pool",
154+
address: address.to_string(),
155+
})
156+
}
136157
ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(ref a)))) => a,
137158
ScAddress::Contract(stellar_xdr::ContractId(Hash(c))) => {
138159
// This address is for a contract. This means we're using a custom
@@ -644,6 +665,66 @@ mod tests {
644665
);
645666
}
646667

668+
/// Regression for #2534: an auth entry whose credential address is not a
669+
/// valid authorizer (muxed account, claimable balance, liquidity pool)
670+
/// must yield a structured error, not the `todo!()` panic it used to hit.
671+
async fn assert_unsupported_auth_address(address: ScAddress, expected_kind: &str) {
672+
let signer = local_signer([1u8; 32]);
673+
let source = MuxedAccount::Ed25519(Uint256([9u8; 32]));
674+
let contract = [42u8; 32];
675+
676+
let entry = address_auth(address, invocation(contract, "hello"));
677+
let host_fn = HostFunction::InvokeContract(invoke_args(contract, "hello"));
678+
let tx = build_tx(source, host_fn, vec![entry]);
679+
680+
let res = sign_soroban_authorizations(
681+
&tx,
682+
&[signer],
683+
EXPIRATION_LEDGER,
684+
NETWORK,
685+
false,
686+
&Print::new(true),
687+
)
688+
.await;
689+
690+
match res {
691+
Err(Error::UnsupportedAuthAddress { kind, .. }) => assert_eq!(kind, expected_kind),
692+
other => panic!("expected UnsupportedAuthAddress error, got: {other:?}"),
693+
}
694+
}
695+
696+
#[tokio::test]
697+
async fn test_muxed_account_auth_address_errors_instead_of_panicking() {
698+
assert_unsupported_auth_address(
699+
ScAddress::MuxedAccount(xdr::MuxedEd25519Account {
700+
id: 1,
701+
ed25519: Uint256([7u8; 32]),
702+
}),
703+
"Muxed (M…)",
704+
)
705+
.await;
706+
}
707+
708+
#[tokio::test]
709+
async fn test_claimable_balance_auth_address_errors_instead_of_panicking() {
710+
assert_unsupported_auth_address(
711+
ScAddress::ClaimableBalance(xdr::ClaimableBalanceId::ClaimableBalanceIdTypeV0(Hash(
712+
[8u8; 32],
713+
))),
714+
"Claimable balance",
715+
)
716+
.await;
717+
}
718+
719+
#[tokio::test]
720+
async fn test_liquidity_pool_auth_address_errors_instead_of_panicking() {
721+
assert_unsupported_auth_address(
722+
ScAddress::LiquidityPool(xdr::PoolId(Hash([6u8; 32]))),
723+
"Liquidity pool",
724+
)
725+
.await;
726+
}
727+
647728
#[tokio::test]
648729
async fn test_non_strict_auth_signs_when_allowed() {
649730
let signer = local_signer([1u8; 32]);

0 commit comments

Comments
 (0)