From f8c481b5272a8515503e029f2ff6e02cd2b7cf72 Mon Sep 17 00:00:00 2001 From: Syzygys <54611697+Syzygys@users.noreply.github.com> Date: Thu, 2 Jul 2026 22:30:24 +0800 Subject: [PATCH] fix(swap): return errors instead of panicking on reply parse failure parse_market_order_response() used two unwrap()s after already constructing the correct ContractError variants (SubMsgFailure, ReplyParseFailure) - change its return type from StdResult to Result<_, ContractError> so those errors can actually be returned with '?' instead of discarded via panic. Also handle an empty msg_responses list explicitly instead of unwrapping Option::None. fix(admin): validate route continuity in set_route verify_route_exists() only checked that the first market contains the source denom and the last market contains the target denom. A multi-hop route with a broken middle hop (market[i] and market[i+1] sharing no denom) would pass set_route and only fail later at swap execution time with a generic exchange-module rejection. Add an explicit continuity check between each consecutive pair of markets so a bad route is rejected immediately, with a specific error message, at configuration time. --- contracts/swap/src/admin.rs | 20 ++++++++++++++++++++ contracts/swap/src/swap.rs | 22 +++++++++++++--------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/contracts/swap/src/admin.rs b/contracts/swap/src/admin.rs index 462644a..1df99d0 100644 --- a/contracts/swap/src/admin.rs +++ b/contracts/swap/src/admin.rs @@ -149,6 +149,26 @@ fn verify_route_exists(deps: Deps, route: &SwapRoute) -> } ); + // A multi-hop route is only tradeable if each market in the chain actually shares a denom + // with the next one. The checks above only look at the very first and very last market, so a + // route with a broken middle hop (e.g. [A/B, C/D] where B != C and B != D) would previously + // pass `set_route` and only fail later, at swap time, when the exchange module rejects the + // order for a market/denom mismatch. Failing fast here gives the admin an immediate, specific + // error instead of a route that looks valid but can never execute. + for pair in denoms.windows(2) { + let (current, next) = (&pair[0], &pair[1]); + let shares_denom = current.quote_denom == next.quote_denom + || current.quote_denom == next.base_denom + || current.base_denom == next.quote_denom + || current.base_denom == next.base_denom; + ensure!( + shares_denom, + CustomError { + val: "Route is not continuous: two consecutive markets do not share a denom".to_string() + } + ); + } + Ok(()) } diff --git a/contracts/swap/src/swap.rs b/contracts/swap/src/swap.rs index 60e40c1..c2a3b76 100644 --- a/contracts/swap/src/swap.rs +++ b/contracts/swap/src/swap.rs @@ -257,16 +257,20 @@ pub fn handle_atomic_order_reply(deps: DepsMut, env: Env, Ok(response) } -pub fn parse_market_order_response(msg: Reply) -> StdResult { - let binding = msg.result.into_result().map_err(ContractError::SubMsgFailure).unwrap(); - - let first_message = binding.msg_responses.first(); - let order_response = MsgCreateSpotMarketOrderResponse::decode(first_message.unwrap().value.as_slice()) - .map_err(|err| ContractError::ReplyParseFailure { - id: msg.id, +pub fn parse_market_order_response(msg: Reply) -> Result { + let reply_id = msg.id; + let binding = msg.result.into_result().map_err(ContractError::SubMsgFailure)?; + + let first_message = binding.msg_responses.first().ok_or_else(|| ContractError::ReplyParseFailure { + id: reply_id, + err: "sub-message result contained no msg_responses".to_string(), + })?; + + let order_response = + MsgCreateSpotMarketOrderResponse::decode(first_message.value.as_slice()).map_err(|err| ContractError::ReplyParseFailure { + id: reply_id, err: err.to_string(), - }) - .unwrap(); + })?; Ok(order_response) }