Allow supplying final hop memos in XCS contracts

CHANGELOG.md

@@ -48,6 +48,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### State Compatible
 
+* [#8895](https://github.com/osmosis-labs/osmosis/pull/8895) (xcs) feat: support XCS final hop memos
+
 ## v28.0.1
 
 ### State Breaking

app/apptesting/test_suite.go

@@ -103,12 +103,17 @@ func init() {
 // Setup sets up basic environment for suite (App, Ctx, and test accounts)
 // preserves the caching enabled/disabled state.
 func (s *KeeperTestHelper) Setup() {
+	s.T().Log("Setting up KeeperTestHelper")
 	dir, err := os.MkdirTemp("", "osmosisd-test-home")
 	if err != nil {
 		panic(fmt.Sprintf("failed creating temporary directory: %v", err))
 	}
 	s.T().Cleanup(func() { os.RemoveAll(dir); s.withCaching = false })
-	s.App = app.SetupWithCustomHome(false, dir)
+	if app.IsDebugLogEnabled() {
+		s.App = app.SetupWithCustomHome(false, dir, s.T())
+	} else {
+		s.App = app.SetupWithCustomHome(false, dir)
+	}
 	s.setupGeneral()
 
 	// Manually set validator signing info, otherwise we panic

app/test_helpers.go

@@ -3,6 +3,7 @@ package app
 import (
 	"encoding/json"
 	"os"
+	"testing"
 	"time"
 
 	"cosmossdk.io/log"
@@ -17,6 +18,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
+	cosmoserver "github.com/cosmos/cosmos-sdk/server"
+	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	"github.com/cosmos/cosmos-sdk/testutil/mock"
 	sims "github.com/cosmos/cosmos-sdk/testutil/sims"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -117,13 +120,55 @@ func GenesisStateWithValSet(app *OsmosisApp) GenesisState {
 var defaultGenesisStatebytes = []byte{}
 
 // SetupWithCustomHome initializes a new OsmosisApp with a custom home directory
-func SetupWithCustomHome(isCheckTx bool, dir string) *OsmosisApp {
-	return SetupWithCustomHomeAndChainId(isCheckTx, dir, "osmosis-1")
+func SetupWithCustomHome(isCheckTx bool, dir string, t ...*testing.T) *OsmosisApp {
+	return SetupWithCustomHomeAndChainId(isCheckTx, dir, "osmosis-1", t...)
 }
 
-func SetupWithCustomHomeAndChainId(isCheckTx bool, dir, chainId string) *OsmosisApp {
+// DebugAppOptions is a stub implementing AppOptions
+type DebugAppOptions struct{}
+
+// Get implements AppOptions
+func (ao DebugAppOptions) Get(o string) interface{} {
+	if o == cosmoserver.FlagTrace {
+		return true
+	}
+	return nil
+}
+
+func IsDebugLogEnabled() bool {
+	return os.Getenv("OSMO_KEEPER_DEBUG") != ""
+}
+
+func SetupWithCustomHomeAndChainId(isCheckTx bool, dir, chainId string, t ...*testing.T) *OsmosisApp {
 	db := cosmosdb.NewMemDB()
-	app := NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, dir, 0, sims.EmptyAppOptions{}, EmptyWasmOpts, baseapp.SetChainID(chainId))
+	var (
+		l       log.Logger
+		appOpts servertypes.AppOptions
+	)
+	if IsDebugLogEnabled() {
+		appOpts = DebugAppOptions{}
+	} else {
+		appOpts = sims.EmptyAppOptions{}
+	}
+	if len(t) > 0 {
+		testEnv := t[0]
+		testEnv.Log("Using test environment logger")
+		l = log.NewTestLogger(testEnv)
+	} else {
+		l = log.NewNopLogger()
+	}
+	app := NewOsmosisApp(
+		l,
+		db,
+		nil,
+		true,
+		map[int64]bool{},
+		dir,
+		0,
+		appOpts,
+		EmptyWasmOpts,
+		baseapp.SetChainID(chainId),
+	)
 	if !isCheckTx {
 		if len(defaultGenesisStatebytes) == 0 {
 			var err error

cosmwasm/contracts/crosschain-registry/src/contract.rs

@@ -82,6 +82,7 @@ pub fn execute(
             receiver,
             into_chain,
             with_memo,
+            final_memo,
         } => {
             let registries = Registry::new(deps.as_ref(), env.contract.address.to_string())?;
             let coin = cw_utils::one_coin(&info)?;
@@ -92,6 +93,7 @@ pub fn execute(
                 env.contract.address.to_string(),
                 env.block.time,
                 with_memo,
+                final_memo,
                 None,
                 false,
             )?;

cosmwasm/contracts/crosschain-registry/src/execute.rs

@@ -131,6 +131,7 @@ pub fn propose_pfm(
         own_addr.to_string(),
         env.block.time,
         format!(r#"{{"ibc_callback":"{own_addr}"}}"#),
+        String::new(), // no last transfer memo
         Some(Callback {
             contract: own_addr,
             msg: format!(r#"{{"validate_pfm": {{"chain": "{chain}"}} }}"#).try_into()?,

cosmwasm/contracts/crosschain-registry/src/msg.rs

@@ -53,6 +53,8 @@ pub enum ExecuteMsg {
         into_chain: Option<String>,
         #[serde(default = "String::new")]
         with_memo: String,
+        #[serde(default = "String::new")]
+        final_memo: String,
     },
 }
 

cosmwasm/contracts/crosschain-swaps/src/checks.rs

@@ -39,7 +39,9 @@ fn validate_explicit_receiver(deps: Deps, receiver: &str) -> Result<(String, Add
     }
 
     let Ok(_) = bech32::decode(&address) else {
-        return Err(ContractError::InvalidReceiver { receiver: receiver.to_string() })
+        return Err(ContractError::InvalidReceiver {
+            receiver: receiver.to_string(),
+        });
     };
 
     let registry = get_registry(deps)?;
@@ -54,7 +56,9 @@ fn validate_explicit_receiver(deps: Deps, receiver: &str) -> Result<(String, Add
 /// transfers from failing after forwarding
 fn validate_bech32_receiver(deps: Deps, receiver: &str) -> Result<(String, Addr), ContractError> {
     let Ok((prefix, _, _)) = bech32::decode(receiver) else {
-        return Err(ContractError::InvalidReceiver { receiver: receiver.to_string() })
+        return Err(ContractError::InvalidReceiver {
+            receiver: receiver.to_string(),
+        });
     };
 
     let registry = get_registry(deps)?;
@@ -65,14 +69,18 @@ fn validate_bech32_receiver(deps: Deps, receiver: &str) -> Result<(String, Addr)
 
 fn validate_chain_receiver(deps: Deps, receiver: &str) -> Result<(String, Addr), ContractError> {
     let Some((chain, addr)) = receiver.split('/').collect_tuple() else {
-        return Err(ContractError::InvalidReceiver { receiver: receiver.to_string() })
+        return Err(ContractError::InvalidReceiver {
+            receiver: receiver.to_string(),
+        });
     };
 
     // TODO: validate that the prefix of the receiver matches the chain
     let _registry = get_registry(deps)?;
 
     let Ok(_) = bech32::decode(addr) else {
-        return Err(ContractError::InvalidReceiver { receiver: receiver.to_string() })
+        return Err(ContractError::InvalidReceiver {
+            receiver: receiver.to_string(),
+        });
     };
 
     Ok((chain.to_string(), Addr::unchecked(addr)))

cosmwasm/contracts/crosschain-swaps/src/contract.rs

@@ -59,6 +59,7 @@ pub fn execute(
             receiver,
             slippage,
             next_memo,
+            final_memo,
             on_failed_delivery,
             route,
         } => execute::unwrap_or_swap_and_forward(
@@ -67,6 +68,7 @@ pub fn execute(
             slippage,
             &receiver,
             next_memo,
+            final_memo,
             on_failed_delivery,
             route,
         ),

cosmwasm/contracts/crosschain-swaps/src/execute.rs

@@ -34,12 +34,14 @@ fn ibc_message_event<T: Debug>(context: &str, message: T) -> cosmwasm_std::Event
 /// valid channel), it will just proceed to swap and forward. If it's not, then
 /// it will send an IBC message to unwrap it first and provide a callback to
 /// ensure the right swap_and_forward gets called after the unwrap succeeds
+#[allow(clippy::too_many_arguments)]
 pub fn unwrap_or_swap_and_forward(
     ctx: (DepsMut, Env, MessageInfo),
     output_denom: String,
     slippage: swaprouter::Slippage,
     receiver: &str,
     next_memo: Option<SerializableJson>,
+    final_memo: Option<SerializableJson>,
     failed_delivery_action: FailedDeliveryAction,
     route: Option<Vec<SwapAmountInRoute>>,
 ) -> Result<Response, ContractError> {
@@ -73,17 +75,19 @@ pub fn unwrap_or_swap_and_forward(
             env.contract.address.to_string(),
             env.block.time,
             build_memo(None, env.contract.address.as_str())?,
+            String::new(),
             Some(Callback {
                 contract: env.contract.address.clone(),
                 msg: serde_cw_value::to_value(&ExecuteMsg::OsmosisSwap {
                     output_denom,
                     receiver: receiver.to_string(),
                     slippage,
                     next_memo,
+                    final_memo,
                     on_failed_delivery: failed_delivery_action.clone(),
                     route,
                 })?
-                .into(),
+                .try_into()?,
             }),
             false,
         )?;
@@ -125,6 +129,7 @@ pub fn unwrap_or_swap_and_forward(
         slippage,
         receiver,
         next_memo,
+        final_memo,
         failed_delivery_action,
         route,
     )
@@ -142,6 +147,7 @@ pub fn swap_and_forward(
     slippage: swaprouter::Slippage,
     receiver: &str,
     next_memo: Option<SerializableJson>,
+    final_memo: Option<SerializableJson>,
     failed_delivery_action: FailedDeliveryAction,
     route: Option<Vec<SwapAmountInRoute>>,
 ) -> Result<Response, ContractError> {
@@ -152,16 +158,11 @@ pub fn swap_and_forward(
 
     // Check that the received is valid and retrieve its channel
     let (valid_chain, valid_receiver) = validate_receiver(deps.as_ref(), receiver)?;
-    // If there is a memo, check that it is valid (i.e. a valud json object that
+
+    // If there are memos, check that they are valid (i.e. a valid json object that
     // doesn't contain the key that we will insert later)
-    let memo = if let Some(memo) = &next_memo {
-        // Ensure the json is an object ({...}) and that it does not contain the CALLBACK_KEY
-        deps.api.debug(&format!("checking memo: {memo:?}"));
-        ensure_key_missing(memo.as_value(), CALLBACK_KEY)?;
-        serde_json_wasm::to_string(&memo)?
-    } else {
-        String::new()
-    };
+    let s_next_memo = validate_user_provided_memo(&deps, next_memo.as_ref())?;
+    let s_final_memo = validate_user_provided_memo(&deps, final_memo.as_ref())?;
 
     // Validate that the swapped token can be unwrapped. If it can't, abort
     // early to avoid swapping unnecessarily
@@ -172,7 +173,8 @@ pub fn swap_and_forward(
         Some(&valid_chain),
         env.contract.address.to_string(),
         env.block.time,
-        memo,
+        s_next_memo,
+        s_final_memo,
         None,
         false,
     )?;
@@ -208,6 +210,7 @@ pub fn swap_and_forward(
                 chain: valid_chain,
                 receiver: valid_receiver,
                 next_memo,
+                final_memo,
                 on_failed_delivery: failed_delivery_action,
             },
         },
@@ -256,7 +259,13 @@ pub fn handle_swap_reply(
     // If the memo is provided we want to include it in the IBC message. If not,
     // we default to an empty object. The resulting memo will always include the
     // callback so this contract can track the IBC send
-    let memo = build_memo(swap_msg_state.forward_to.next_memo, contract_addr.as_str())?;
+    let next_memo = build_memo(swap_msg_state.forward_to.next_memo, contract_addr.as_str())?;
+    let final_memo = swap_msg_state
+        .forward_to
+        .final_memo
+        .map(|final_memo| serde_json_wasm::to_string(&final_memo))
+        .transpose()?
+        .unwrap_or_default();
 
     let registry = get_registry(deps.as_ref())?;
     let ibc_transfer = registry.unwrap_coin_into(
@@ -268,7 +277,8 @@ pub fn handle_swap_reply(
         Some(&swap_msg_state.forward_to.chain),
         env.contract.address.to_string(),
         env.block.time,
-        memo,
+        next_memo,
+        final_memo,
         None,
         false,
     )?;
@@ -314,7 +324,9 @@ pub fn handle_forward_reply(
     deps.api.debug(&format!("handle_forward_reply"));
     // Parse the result from the underlying chain call (IBC send)
     let SubMsgResult::Ok(SubMsgResponse { data: Some(b), .. }) = msg.result else {
-        return Err(ContractError::FailedIBCTransfer { msg: format!("failed reply: {:?}", msg.result) })
+        return Err(ContractError::FailedIBCTransfer {
+            msg: format!("failed reply: {:?}", msg.result),
+        });
     };
 
     // The response contains the packet sequence. This is needed to be able to
@@ -442,6 +454,20 @@ pub fn set_swap_contract(
     Ok(Response::new().add_attribute("action", "set_swaps_contract"))
 }
 
+fn validate_user_provided_memo(
+    deps: &DepsMut,
+    user_memo: Option<&SerializableJson>,
+) -> Result<String, ContractError> {
+    Ok(if let Some(memo) = user_memo {
+        // Ensure the json is an object ({...}) and that it does not contain the CALLBACK_KEY
+        deps.api.debug(&format!("checking memo: {memo:?}"));
+        ensure_key_missing(memo.as_value(), CALLBACK_KEY)?;
+        serde_json_wasm::to_string(&memo)?
+    } else {
+        String::new()
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info};

cosmwasm/contracts/crosschain-swaps/src/ibc_lifecycle.rs

@@ -21,7 +21,7 @@ fn create_recovery(
         // RECOVERY_STATES map.
         recovery.status = recovery_reason;
         let Some(mut recoveries) = recoveries else {
-            return Ok::<_, ContractError>(vec![recovery])
+            return Ok::<_, ContractError>(vec![recovery]);
         };
         recoveries.push(recovery);
         Ok(recoveries)
@@ -65,7 +65,7 @@ pub fn receive_ack(
     let sent_packet = INFLIGHT_PACKETS.may_load(deps.storage, (&source_channel, sequence))?;
     let Some(inflight_packet) = sent_packet else {
         // If there isn't, continue
-        return Ok(response.add_attribute("msg", "received unexpected ack"))
+        return Ok(response.add_attribute("msg", "received unexpected ack"));
     };
     // Remove the in-flight packet
     INFLIGHT_PACKETS.remove(deps.storage, (&source_channel, sequence));
@@ -102,7 +102,7 @@ pub fn receive_timeout(
     let sent_packet = INFLIGHT_PACKETS.may_load(deps.storage, (&source_channel, sequence))?;
     let Some(inflight_packet) = sent_packet else {
         // If there isn't, continue
-        return Ok(response.add_attribute("msg", "received unexpected timeout"))
+        return Ok(response.add_attribute("msg", "received unexpected timeout"));
     };
     // Remove the in-flight packet
     INFLIGHT_PACKETS.remove(deps.storage, (&source_channel, sequence));

cosmwasm/contracts/crosschain-swaps/src/msg.rs

@@ -54,6 +54,10 @@ pub enum ExecuteMsg {
         /// packet to include a memo, this is the field where they can specify
         /// it. If provided, the memo is expected to be a valid JSON object
         next_memo: Option<SerializableJson>,
+        /// IBC packets can contain an optional memo. If a sender wants the last
+        /// hop's packet to include a memo, this is the field where they can specify
+        /// it. If provided, the memo is expected to be a valid JSON object
+        final_memo: Option<SerializableJson>,
         /// If for any reason the swap were to fail, users can specify a
         /// "recovery address" that can clain the funds on osmosis after a
         /// confirmed failure.

cosmwasm/contracts/crosschain-swaps/src/state.rs

@@ -18,6 +18,7 @@ pub struct ForwardTo {
     pub chain: String,
     pub receiver: Addr,
     pub next_memo: Option<SerializableJson>,
+    pub final_memo: Option<SerializableJson>,
     pub on_failed_delivery: FailedDeliveryAction,
 }