Improve docs and runtime checks of osmosis swaps

crates/apps_lib/src/cli.rs

@@ -5206,13 +5206,16 @@ pub mod args {
                     "The IBC denomination (on Osmosis) of the desired asset."
                 )))
                 .arg(TARGET.def().help(wrap!(
-                    "The address that shall receive the swapped tokens."
+                    "The Namada address that shall receive the swapped tokens."
                 )))
                 .arg(SLIPPAGE.def().help(wrap!(
-                    "The slippage percentage as an integer between 0 and 100."
+                    "The slippage percentage as an integer between 0 and 100. \
+                     Represents the maximum acceptable deviation from the \
+                     expected price during a trade."
                 )))
                 .arg(WINDOW_SECONDS.def().help(wrap!(
-                    "A mysterious thing that should be set to 10s."
+                    "The time period (in seconds) over which the average \
+                     price is calculated."
                 )))
                 .arg(LOCAL_RECOVERY_ADDR.def().help(wrap!(
                     "An address on Osmosis from which to recover funds in \

crates/apps_lib/src/cli/client.rs

@@ -123,7 +123,7 @@ impl CliApi {
 
                         let args = args.to_sdk(&mut ctx)?;
                         let namada = ctx.to_sdk(client, io);
-                        let args = args.assemble(&namada).await;
+                        let args = args.into_ibc_transfer(&namada).await?;
 
                         tx::submit_ibc_transfer(&namada, args).await?;
                     }

crates/sdk/Cargo.toml

@@ -91,6 +91,7 @@ namada_wallet = {path = "../wallet" }
 
 arbitrary = { workspace = true, optional = true }
 async-trait.workspace = true
+bech32.workspace = true
 bimap.workspace = true
 borsh.workspace = true
 borsh-ext.workspace = true

crates/sdk/src/args.rs

@@ -26,6 +26,7 @@ use namada_tx::Memo;
 use serde::{Deserialize, Serialize, Serializer};
 use zeroize::Zeroizing;
 
+use crate::error::Error;
 use crate::eth_bridge::bridge_pool;
 use crate::ibc::core::host::types::identifiers::{ChannelId, PortId};
 use crate::ibc::{NamadaMemo, NamadaMemoData};
@@ -498,23 +499,27 @@ impl FromStr for OsmosisPoolHop {
 pub struct TxOsmosisSwap<C: NamadaTypes = SdkTypes> {
     /// The IBC transfer data
     pub transfer: TxIbcTransfer<C>,
-    /// The token we wish to receive
+    /// The token we wish to receive (on Osmosis)
     pub output_denom: String,
-    /// Recipient address
+    /// Address of the recipient on Namada
     pub recipient: C::Address,
-    /// Slippage percent
+    /// The maximum percentage difference allowed between the estimated and
+    /// actual trade price
     pub slippage_percent: u64,
-    /// TODO! Figure out what this is
+    /// The time period (in seconds) over which the average price is calculated
     pub window_seconds: u64,
-    /// A recovery address (on Osmosis) in case of failure
+    /// Recovery address (on Osmosis) in case of failure
     pub local_recovery_addr: String,
     /// The route to take through Osmosis pools
     pub route: Option<Vec<OsmosisPoolHop>>,
 }
 
 impl TxOsmosisSwap<SdkTypes> {
     /// Create an IBC transfer from the input arguments
-    pub async fn assemble(self, ctx: &impl Namada) -> TxIbcTransfer<SdkTypes> {
+    pub async fn into_ibc_transfer(
+        self,
+        ctx: &impl Namada,
+    ) -> crate::error::Result<TxIbcTransfer<SdkTypes>> {
         #[derive(Serialize)]
         struct Memo {
             wasm: Wasm,
@@ -533,14 +538,13 @@ impl TxOsmosisSwap<SdkTypes> {
 
         #[derive(Serialize)]
         struct OsmosisSwap {
+            receiver: String,
             output_denom: String,
             slippage: Slippage,
-            receiver: String,
-            #[serde(skip_serializing_if = "Option::is_none")]
-            next_memo: Option<String>,
             on_failed_delivery: LocalRecoveryAddr,
+            route: Vec<OsmosisPoolHop>,
             #[serde(skip_serializing_if = "Option::is_none")]
-            route: Option<Vec<OsmosisPoolHop>>,
+            next_memo: Option<String>,
         }
 
         #[derive(Serialize)]
@@ -582,13 +586,28 @@ impl TxOsmosisSwap<SdkTypes> {
             route,
         } = self;
 
+        // validate `local_recovery_addr`
+        'valid_osmo_addr: {
+            if let Ok((hrp, _, _)) = bech32::decode(&local_recovery_addr) {
+                if hrp == "osmo" {
+                    break 'valid_osmo_addr;
+                }
+            }
+            return Err(Error::Other(format!(
+                "Invalid Osmosis address {local_recovery_addr:?}"
+            )));
+        }
+
         let next_memo = transfer.ibc_memo.take().map(|memo| {
             serde_json::to_string(&NamadaMemo {
                 namada: NamadaMemoData::Memo(memo),
             })
             .unwrap()
         });
-        let route = if route.is_none() {
+
+        let route = if let Some(route) = route {
+            route
+        } else {
             query_osmosis_pool_routes(
                 ctx,
                 &transfer.token,
@@ -597,11 +616,15 @@ impl TxOsmosisSwap<SdkTypes> {
                 &output_denom,
                 OSMOSIS_SQS_SERVER,
             )
-            .await
-            .unwrap()
+            .await?
             .pop()
-        } else {
-            route
+            .ok_or_else(|| {
+                Error::Other(format!(
+                    "No route found to swap {:?} of Namada's {} with Osmosis' \
+                     {}",
+                    transfer.amount, transfer.token, output_denom
+                ))
+            })?
         };
 
         let memo = Memo {
@@ -628,7 +651,7 @@ impl TxOsmosisSwap<SdkTypes> {
         };
 
         transfer.ibc_memo = Some(serde_json::to_string(&memo).unwrap());
-        transfer
+        Ok(transfer)
     }
 }