Use trait 🪄🪄 to allow different result types in ibc_packet_receive

contracts/ibc-reflect-send/src/ibc.rs

@@ -1,7 +1,8 @@
 use cosmwasm_std::{
     entry_point, from_slice, to_binary, DepsMut, Env, IbcBasicResponse, IbcChannelCloseMsg,
     IbcChannelConnectMsg, IbcChannelOpenMsg, IbcMsg, IbcOrder, IbcPacketAckMsg,
-    IbcPacketReceiveMsg, IbcPacketTimeoutMsg, IbcReceiveResponse, Never, StdError, StdResult,
+    IbcPacketReceiveMsg, IbcPacketTimeoutMsg, IbcReceiveResponse, IbcResult, Never, StdError,
+    StdResult,
 };
 
 use crate::ibc_msg::{
@@ -95,10 +96,8 @@ pub fn ibc_packet_receive(
     _deps: DepsMut,
     _env: Env,
     _packet: IbcPacketReceiveMsg,
-) -> Result<IbcReceiveResponse, Never> {
-    Ok(IbcReceiveResponse::new()
-        .set_ack(b"{}")
-        .add_attribute("action", "ibc_packet_ack"))
+) -> IbcResult<IbcReceiveResponse, Never> {
+    IbcResult::Abort
 }
 
 #[entry_point]

packages/std/src/exports.rs

@@ -24,7 +24,9 @@ use crate::memory::{alloc, consume_region, release_buffer, Region};
 #[cfg(feature = "abort")]
 use crate::panic::install_panic_handler;
 use crate::query::CustomQuery;
-use crate::results::{ContractResult, QueryResponse, Reply, Response};
+use crate::results::{
+    ContractResult, IbcContractResult, IbcResult, QueryResponse, Reply, Response,
+};
 use crate::serde::{from_slice, to_vec};
 use crate::types::Env;
 use crate::{CustomMsg, Deps, DepsMut, MessageInfo};
@@ -86,6 +88,21 @@ macro_rules! r#try_into_contract_result {
     };
 }
 
+// TODO: replace with https://doc.rust-lang.org/std/ops/trait.Try.html once stabilized
+macro_rules! r#try_into_ibc_contract_result {
+    ($expr:expr) => {
+        match $expr {
+            Ok(val) => val,
+            Err(err) => {
+                return IbcContractResult::Err(err.to_string());
+            }
+        }
+    };
+    ($expr:expr,) => {
+        $crate::try_into_ibc_contract_result!($expr)
+    };
+}
+
 /// This should be wrapped in an external "C" export, containing a contract-specific function as an argument.
 ///
 /// - `Q`: custom query type (see QueryRequest)
@@ -312,24 +329,41 @@ where
     release_buffer(v) as u32
 }
 
+/// An internal result type that allows using Result and IbcResult as the return type of
+/// `ibc_packet_receive`.
+///
+/// The parameter `S` is the success value type.
+pub trait ResultPlusPlus<S>: Into<IbcContractResult<S>> {}
+
+impl<C: CustomMsg, E: ToString> ResultPlusPlus<IbcReceiveResponse<C>>
+    for Result<IbcReceiveResponse<C>, E>
+{
+}
+
+impl<C: CustomMsg, E: ToString> ResultPlusPlus<IbcReceiveResponse<C>>
+    for IbcResult<IbcReceiveResponse<C>, E>
+{
+}
+
 /// do_ibc_packet_receive is designed for use with #[entry_point] to make a "C" extern
 ///
 /// contract_fn is called when this chain receives an IBC Packet on a channel belonging
 /// to this contract
 ///
 /// - `Q`: custom query type (see QueryRequest)
 /// - `C`: custom response message type (see CosmosMsg)
+/// - `R`: Result type (typically `std::result::Result`)
 /// - `E`: error type for responses
 #[cfg(feature = "stargate")]
-pub fn do_ibc_packet_receive<Q, C, E>(
-    contract_fn: &dyn Fn(DepsMut<Q>, Env, IbcPacketReceiveMsg) -> Result<IbcReceiveResponse<C>, E>,
+pub fn do_ibc_packet_receive<Q, C, R>(
+    contract_fn: &dyn Fn(DepsMut<Q>, Env, IbcPacketReceiveMsg) -> R,
     env_ptr: u32,
     msg_ptr: u32,
 ) -> u32
 where
     Q: CustomQuery,
     C: CustomMsg,
-    E: ToString,
+    R: ResultPlusPlus<IbcReceiveResponse<C>>,
 {
     #[cfg(feature = "abort")]
     install_panic_handler();
@@ -584,21 +618,21 @@ where
 }
 
 #[cfg(feature = "stargate")]
-fn _do_ibc_packet_receive<Q, C, E>(
-    contract_fn: &dyn Fn(DepsMut<Q>, Env, IbcPacketReceiveMsg) -> Result<IbcReceiveResponse<C>, E>,
+fn _do_ibc_packet_receive<Q, C, R>(
+    contract_fn: &dyn Fn(DepsMut<Q>, Env, IbcPacketReceiveMsg) -> R,
     env_ptr: *mut Region,
     msg_ptr: *mut Region,
-) -> ContractResult<IbcReceiveResponse<C>>
+) -> IbcContractResult<IbcReceiveResponse<C>>
 where
     Q: CustomQuery,
     C: CustomMsg,
-    E: ToString,
+    R: ResultPlusPlus<IbcReceiveResponse<C>>,
 {
     let env: Vec<u8> = unsafe { consume_region(env_ptr) };
     let msg: Vec<u8> = unsafe { consume_region(msg_ptr) };
 
-    let env: Env = try_into_contract_result!(from_slice(&env));
-    let msg: IbcPacketReceiveMsg = try_into_contract_result!(from_slice(&msg));
+    let env: Env = try_into_ibc_contract_result!(from_slice(&env));
+    let msg: IbcPacketReceiveMsg = try_into_ibc_contract_result!(from_slice(&msg));
 
     let mut deps = make_dependencies();
     contract_fn(deps.as_mut(), env, msg).into()

packages/std/src/lib.rs

@@ -72,8 +72,8 @@ pub use crate::results::SubMsgExecutionResponse;
 pub use crate::results::WeightedVoteOption;
 pub use crate::results::{
     attr, wasm_execute, wasm_instantiate, Attribute, BankMsg, ContractResult, CosmosMsg, CustomMsg,
-    Empty, Event, QueryResponse, Reply, ReplyOn, Response, SubMsg, SubMsgResponse, SubMsgResult,
-    SystemResult, WasmMsg,
+    Empty, Event, IbcContractResult, IbcResult, QueryResponse, Reply, ReplyOn, Response, SubMsg,
+    SubMsgResponse, SubMsgResult, SystemResult, WasmMsg,
 };
 #[cfg(feature = "staking")]
 pub use crate::results::{DistributionMsg, StakingMsg};

packages/std/src/results/ibc_contract_result.rs

@@ -0,0 +1,182 @@
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};
+
+use crate::IbcResult;
+
+/// This is the final result type that is created and serialized in a contract for
+/// every init/execute/migrate call. The VM then deserializes this type to distinguish
+/// between successful and failed executions.
+///
+/// We use a custom type here instead of Rust's Result because we want to be able to
+/// define the serialization, which is a public interface. Every language that compiles
+/// to Wasm and runs in the ComsWasm VM needs to create the same JSON representation.
+///
+/// # Examples
+///
+/// Success:
+///
+/// ```
+/// # use cosmwasm_std::{to_vec, IbcContractResult, Response};
+/// let response: Response = Response::default();
+/// let result: IbcContractResult<Response> = IbcContractResult::Ok(response);
+/// assert_eq!(to_vec(&result).unwrap(), br#"{"ok":{"messages":[],"attributes":[],"events":[],"data":null}}"#);
+/// ```
+///
+/// Failure:
+///
+/// ```
+/// # use cosmwasm_std::{to_vec, IbcContractResult, Response};
+/// let error_msg = String::from("Something went wrong");
+/// let result: IbcContractResult<Response> = IbcContractResult::Err(error_msg);
+/// assert_eq!(to_vec(&result).unwrap(), br#"{"error":"Something went wrong"}"#);
+/// ```
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)]
+#[serde(rename_all = "snake_case")]
+pub enum IbcContractResult<S> {
+    Ok(S),
+    /// An error type that every custom error created by contract developers can be converted to.
+    /// This could potientially have more structure, but String is the easiest.
+    #[serde(rename = "error")]
+    Err(String),
+    Abort,
+}
+
+// Implementations here mimic the Result API and should be implemented via a conversion to Result
+// to ensure API consistency
+impl<S> IbcContractResult<S> {
+    pub fn unwrap(self) -> S {
+        match self {
+            IbcContractResult::Ok(value) => value,
+            IbcContractResult::Err(_) => panic!("error"),
+            IbcContractResult::Abort => panic!("abort"),
+        }
+    }
+
+    pub fn is_ok(&self) -> bool {
+        matches!(self, IbcContractResult::Ok(_))
+    }
+
+    pub fn is_err(&self) -> bool {
+        matches!(self, IbcContractResult::Err(_))
+    }
+}
+
+// impl<S: fmt::Debug> IbcContractResult<S> {
+//     pub fn unwrap_err(self) -> String {
+//         self.into_result().unwrap_err()
+//         match self {
+//             IbcContractResult::Ok(_) => value,
+//             IbcContractResult::Err(_) => panic!("error"),
+//             IbcContractResult::Abort => panic!("abort"),
+//         }
+//     }
+// }
+
+impl<S, E: ToString> From<Result<S, E>> for IbcContractResult<S> {
+    fn from(original: Result<S, E>) -> IbcContractResult<S> {
+        match original {
+            Ok(value) => IbcContractResult::Ok(value),
+            Err(err) => IbcContractResult::Err(err.to_string()),
+        }
+    }
+}
+
+impl<S, E: ToString> From<IbcResult<S, E>> for IbcContractResult<S> {
+    fn from(original: IbcResult<S, E>) -> IbcContractResult<S> {
+        match original {
+            IbcResult::Ok(value) => IbcContractResult::Ok(value),
+            IbcResult::Err(err) => IbcContractResult::Err(err.to_string()),
+            IbcResult::Abort => IbcContractResult::Abort,
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{from_slice, to_vec, Response, StdError, StdResult};
+
+    #[test]
+    fn contract_result_serialization_works() {
+        let result = IbcContractResult::Ok(12);
+        assert_eq!(&to_vec(&result).unwrap(), b"{\"ok\":12}");
+
+        let result = IbcContractResult::Ok("foo");
+        assert_eq!(&to_vec(&result).unwrap(), b"{\"ok\":\"foo\"}");
+
+        let result: IbcContractResult<Response> = IbcContractResult::Ok(Response::default());
+        assert_eq!(
+            to_vec(&result).unwrap(),
+            br#"{"ok":{"messages":[],"attributes":[],"events":[],"data":null}}"#
+        );
+
+        let result: IbcContractResult<Response> = IbcContractResult::Err("broken".to_string());
+        assert_eq!(&to_vec(&result).unwrap(), b"{\"error\":\"broken\"}");
+    }
+
+    #[test]
+    fn contract_result_deserialization_works() {
+        let result: IbcContractResult<u64> = from_slice(br#"{"ok":12}"#).unwrap();
+        assert_eq!(result, IbcContractResult::Ok(12));
+
+        let result: IbcContractResult<String> = from_slice(br#"{"ok":"foo"}"#).unwrap();
+        assert_eq!(result, IbcContractResult::Ok("foo".to_string()));
+
+        let result: IbcContractResult<Response> =
+            from_slice(br#"{"ok":{"messages":[],"attributes":[],"events":[],"data":null}}"#)
+                .unwrap();
+        assert_eq!(result, IbcContractResult::Ok(Response::default()));
+
+        let result: IbcContractResult<Response> = from_slice(br#"{"error":"broken"}"#).unwrap();
+        assert_eq!(result, IbcContractResult::Err("broken".to_string()));
+
+        // ignores whitespace
+        let result: IbcContractResult<u64> = from_slice(b" {\n\t  \"ok\": 5898\n}  ").unwrap();
+        assert_eq!(result, IbcContractResult::Ok(5898));
+
+        // fails for additional attributes
+        let parse: StdResult<IbcContractResult<u64>> =
+            from_slice(br#"{"unrelated":321,"ok":4554}"#);
+        match parse.unwrap_err() {
+            StdError::ParseErr { .. } => {}
+            err => panic!("Unexpected error: {:?}", err),
+        }
+        let parse: StdResult<IbcContractResult<u64>> =
+            from_slice(br#"{"ok":4554,"unrelated":321}"#);
+        match parse.unwrap_err() {
+            StdError::ParseErr { .. } => {}
+            err => panic!("Unexpected error: {:?}", err),
+        }
+        let parse: StdResult<IbcContractResult<u64>> =
+            from_slice(br#"{"ok":4554,"error":"What's up now?"}"#);
+        match parse.unwrap_err() {
+            StdError::ParseErr { .. } => {}
+            err => panic!("Unexpected error: {:?}", err),
+        }
+    }
+
+    #[test]
+    fn can_convert_from_core_result() {
+        let original: Result<Response, StdError> = Ok(Response::default());
+        let converted: IbcContractResult<Response> = original.into();
+        assert_eq!(converted, IbcContractResult::Ok(Response::default()));
+
+        let original: Result<Response, StdError> = Err(StdError::generic_err("broken"));
+        let converted: IbcContractResult<Response> = original.into();
+        assert_eq!(
+            converted,
+            IbcContractResult::Err("Generic error: broken".to_string())
+        );
+    }
+
+    // #[test]
+    // fn can_convert_to_core_result() {
+    //     let original = IbcContractResult::Ok(Response::default());
+    //     let converted: Result<Response, String> = original.into();
+    //     assert_eq!(converted, Ok(Response::default()));
+    //
+    //     let original = IbcContractResult::Err("went wrong".to_string());
+    //     let converted: Result<Response, String> = original.into();
+    //     assert_eq!(converted, Err("went wrong".to_string()));
+    // }
+}

packages/std/src/results/ibc_result.rs

@@ -0,0 +1,15 @@
+/// This is like Result but we add an Abort case
+pub enum IbcResult<S, E> {
+    Ok(S),
+    Err(E),
+    Abort,
+}
+
+impl<S, E> From<Result<S, E>> for IbcResult<S, E> {
+    fn from(original: Result<S, E>) -> IbcResult<S, E> {
+        match original {
+            Ok(value) => IbcResult::Ok(value),
+            Err(err) => IbcResult::Err(err),
+        }
+    }
+}

packages/std/src/results/mod.rs

@@ -4,6 +4,8 @@ mod contract_result;
 mod cosmos_msg;
 mod empty;
 mod events;
+mod ibc_contract_result;
+mod ibc_result;
 mod query;
 mod response;
 mod submessages;
@@ -19,6 +21,8 @@ pub use cosmos_msg::{DistributionMsg, StakingMsg};
 pub use cosmos_msg::{GovMsg, VoteOption};
 pub use empty::Empty;
 pub use events::{attr, Attribute, Event};
+pub use ibc_contract_result::IbcContractResult;
+pub use ibc_result::IbcResult;
 pub use query::QueryResponse;
 pub use response::Response;
 #[allow(deprecated)]