Repository: CosmWasm/cosmwasm
Pull request #910 (2021-04-29 18:46:02)

Remove unneeded callback
In #885  `ibc-reflect` stopped using `reflect` "execute callback" on `instantiate` and uses submessages. But I kept the logic and it got an unhandled `execute` callback.

Remove this flow completely.

 


Patch:
diff --git a/contracts/ibc-reflect/src/contract.rs b/contracts/ibc-reflect/src/contract.rs
index 4644218b1..88965263e 100644
--- a/contracts/ibc-reflect/src/contract.rs
+++ b/contracts/ibc-reflect/src/contract.rs
@@ -1,14 +1,13 @@
 use cosmwasm_std::{
-    attr, entry_point, from_slice, to_binary, wasm_execute, wasm_instantiate, BankMsg, Binary,
-    ContractResult, CosmosMsg, Deps, DepsMut, Empty, Env, Event, IbcAcknowledgement,
-    IbcBasicResponse, IbcChannel, IbcOrder, IbcPacket, IbcReceiveResponse, MessageInfo, Order,
-    QueryResponse, Reply, ReplyOn, Response, StdError, StdResult, SubMsg, SubcallResponse,
+    attr, entry_point, from_slice, to_binary, wasm_execute, BankMsg, Binary, ContractResult,
+    CosmosMsg, Deps, DepsMut, Empty, Env, Event, IbcAcknowledgement, IbcBasicResponse, IbcChannel,
+    IbcOrder, IbcPacket, IbcReceiveResponse, MessageInfo, Order, QueryResponse, Reply, ReplyOn,
+    Response, StdError, StdResult, SubMsg, SubcallResponse, WasmMsg,
 };
 
 use crate::msg::{
     AccountInfo, AccountResponse, AcknowledgementMsg, BalancesResponse, DispatchResponse,
-    InstantiateMsg, ListAccountsResponse, PacketMsg, QueryMsg, ReflectExecuteMsg,
-    ReflectInstantiateMsg, WhoAmIResponse,
+    InstantiateMsg, ListAccountsResponse, PacketMsg, QueryMsg, ReflectExecuteMsg, WhoAmIResponse,
 };
 use crate::state::{accounts, accounts_read, config, pending_channel, Config};
 
@@ -161,12 +160,13 @@ pub fn ibc_channel_connect(
     let cfg = config(deps.storage).load()?;
     let chan_id = channel.endpoint.channel_id;
 
-    let label = format!("ibc-reflect-{}", &chan_id);
-    let payload = ReflectInstantiateMsg {
-        callback_id: Some(chan_id.clone()),
+    let msg = WasmMsg::Instantiate {
+        admin: None,
+        code_id: cfg.reflect_code_id,
+        msg: b"{}".into(),
+        send: vec![],
+        label: format!("ibc-reflect-{}", &chan_id),
     };
-    let msg = wasm_instantiate(cfg.reflect_code_id, &payload, vec![], label)?;
-
     let sub_msg = SubMsg {
         id: INIT_CALLBACK_ID,
         msg: msg.into(),
@@ -476,7 +476,7 @@ mod tests {
         if let CosmosMsg::Wasm(WasmMsg::Instantiate {
             admin,
             code_id,
-            msg,
+            msg: _,
             send,
             label,
         }) = &res.submessages[0].msg
@@ -485,9 +485,6 @@ mod tests {
             assert_eq!(*code_id, REFLECT_ID);
             assert_eq!(send.len(), 0);
             assert!(label.contains(channel_id));
-            // parse the message - should callback with proper channel_id
-            let rmsg: ReflectInstantiateMsg = from_slice(&msg).unwrap();
-            assert_eq!(rmsg.callback_id, Some(channel_id.to_string()));
         } else {
             panic!("invalid return message: {:?}", res.messages[0]);
         }
diff --git a/contracts/ibc-reflect/src/msg.rs b/contracts/ibc-reflect/src/msg.rs
index 9b434f4e2..0a43def07 100644
--- a/contracts/ibc-reflect/src/msg.rs
+++ b/contracts/ibc-reflect/src/msg.rs
@@ -35,12 +35,6 @@ pub struct AccountInfo {
     pub channel_id: String,
 }
 
-/// This is the message we send to the reflect contract to initialize it
-#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
-pub struct ReflectInstantiateMsg {
-    pub callback_id: Option<String>,
-}
-
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 #[serde(rename_all = "snake_case")]
 pub enum ReflectExecuteMsg {
diff --git a/contracts/reflect/schema/instantiate_msg.json b/contracts/reflect/schema/instantiate_msg.json
index 196554c88..44588cf22 100644
--- a/contracts/reflect/schema/instantiate_msg.json
+++ b/contracts/reflect/schema/instantiate_msg.json
@@ -1,14 +1,5 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
   "title": "InstantiateMsg",
-  "type": "object",
-  "properties": {
-    "callback_id": {
-      "description": "if set, returns CallbackMsg::InstantiateCallback{} to the caller with this contract's address and this id",
-      "type": [
-        "string",
-        "null"
-      ]
-    }
-  }
+  "type": "object"
 }
diff --git a/contracts/reflect/src/contract.rs b/contracts/reflect/src/contract.rs
index dcd31f538..c40bbc1d3 100644
--- a/contracts/reflect/src/contract.rs
+++ b/contracts/reflect/src/contract.rs
@@ -1,41 +1,25 @@
 use cosmwasm_std::{
     attr, entry_point, to_binary, to_vec, Binary, ContractResult, CosmosMsg, Deps, DepsMut, Env,
     MessageInfo, QueryRequest, QueryResponse, Reply, Response, StdError, StdResult, SubMsg,
-    SystemResult, WasmMsg,
+    SystemResult,
 };
 
 use crate::errors::ReflectError;
 use crate::msg::{
-    CallbackMsg, CapitalizedResponse, ChainResponse, CustomMsg, ExecuteMsg, InstantiateMsg,
-    OwnerResponse, QueryMsg, RawResponse, SpecialQuery, SpecialResponse,
+    CapitalizedResponse, ChainResponse, CustomMsg, ExecuteMsg, InstantiateMsg, OwnerResponse,
+    QueryMsg, RawResponse, SpecialQuery, SpecialResponse,
 };
 use crate::state::{config, config_read, replies, replies_read, State};
 
 pub fn instantiate(
     deps: DepsMut,
-    env: Env,
+    _env: Env,
     info: MessageInfo,
-    msg: InstantiateMsg,
+    _msg: InstantiateMsg,
 ) -> StdResult<Response<CustomMsg>> {
-    let state = State {
-        owner: info.sender.clone(),
-    };
+    let state = State { owner: info.sender };
     config(deps.storage).save(&state)?;
-
-    let mut resp = Response::new();
-    if let Some(id) = msg.callback_id {
-        let data = CallbackMsg::InitCallback {
-            id,
-            contract_addr: env.contract.address.into(),
-        };
-        let msg = WasmMsg::Execute {
-            contract_addr: info.sender.into(),
-            msg: to_binary(&data)?,
-            send: vec![],
-        };
-        resp.add_message(msg);
-    }
-    Ok(resp)
+    Ok(Response::default())
 }
 
 pub fn execute(
@@ -202,7 +186,7 @@ mod tests {
     fn proper_instantialization() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(1000, "earth"));
 
         // we can just call .unwrap() to assert this was a success
@@ -214,50 +198,11 @@ mod tests {
         assert_eq!("creator", value.owner.as_str());
     }
 
-    #[test]
-    fn instantiate_with_callback() {
-        let mut deps = mock_dependencies_with_custom_querier(&[]);
-        let caller = String::from("calling-contract");
-
-        let msg = InstantiateMsg {
-            callback_id: Some("foobar".to_string()),
-        };
-        let info = mock_info(&caller, &coins(1000, "earth"));
-
-        // we can just call .unwrap() to assert this was a success
-        let res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
-        assert_eq!(1, res.messages.len());
-        let msg = &res.messages[0];
-        match msg {
-            CosmosMsg::Wasm(WasmMsg::Execute {
-                contract_addr,
-                msg,
-                send,
-            }) => {
-                assert_eq!(contract_addr.as_str(), &caller);
-                let parsed: CallbackMsg = from_binary(&msg).unwrap();
-                assert_eq!(
-                    parsed,
-                    CallbackMsg::InitCallback {
-                        id: "foobar".to_string(),
-                        contract_addr: MOCK_CONTRACT_ADDR.into(),
-                    }
-                );
-                assert_eq!(0, send.len());
-            }
-            _ => panic!("expect wasm execute message"),
-        }
-
-        // it worked, let's query the state
-        let value = query_owner(deps.as_ref()).unwrap();
-        assert_eq!(value.owner, caller);
-    }
-
     #[test]
     fn reflect() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -279,7 +224,7 @@ mod tests {
     fn reflect_requires_owner() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -303,7 +248,7 @@ mod tests {
     fn reflect_reject_empty_msgs() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -319,7 +264,7 @@ mod tests {
     fn reflect_multiple_messages() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -351,7 +296,7 @@ mod tests {
     fn change_owner_works() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -370,7 +315,7 @@ mod tests {
     fn change_owner_requires_current_owner_as_sender() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let creator = String::from("creator");
         let info = mock_info(&creator, &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
@@ -395,7 +340,7 @@ mod tests {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
         let creator = String::from("creator");
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info(&creator, &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -454,7 +399,7 @@ mod tests {
     fn reflect_subcall() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
@@ -486,7 +431,7 @@ mod tests {
     fn reply_and_query() {
         let mut deps = mock_dependencies_with_custom_querier(&[]);
 
-        let msg = InstantiateMsg { callback_id: None };
+        let msg = InstantiateMsg {};
         let info = mock_info("creator", &coins(2, "token"));
         let _res = instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
diff --git a/contracts/reflect/src/msg.rs b/contracts/reflect/src/msg.rs
index 14f4b364c..f3c488822 100644
--- a/contracts/reflect/src/msg.rs
+++ b/contracts/reflect/src/msg.rs
@@ -4,24 +4,7 @@ use serde::{Deserialize, Serialize};
 use cosmwasm_std::{Binary, CosmosMsg, CustomQuery, QueryRequest, SubMsg};
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
-pub struct InstantiateMsg {
-    /// if set, returns CallbackMsg::InstantiateCallback{} to the caller with this contract's address
-    /// and this id
-    pub callback_id: Option<String>,
-}
-
-/// This is what we return upon init if callback is set
-#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
-#[serde(rename_all = "snake_case")]
-pub enum CallbackMsg {
-    /// This type must match [ExecuteMsg::InitCallback from ibc-reflect](https://github.com/CosmWasm/cosmwasm/blob/9fd06ea/contracts/ibc-reflect/src/msg.rs#L17-L22).
-    InitCallback {
-        /// Callback ID provided in the InstantiateMsg
-        id: String,
-        /// contract_addr is the address of this contract
-        contract_addr: String,
-    },
-}
+pub struct InstantiateMsg {}
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 #[serde(rename_all = "snake_case")]


Test patch:
diff --git a/contracts/ibc-reflect/tests/integration.rs b/contracts/ibc-reflect/tests/integration.rs
index d85ccd06c..f01b1eaf9 100644
--- a/contracts/ibc-reflect/tests/integration.rs
+++ b/contracts/ibc-reflect/tests/integration.rs
@@ -31,7 +31,7 @@ use cosmwasm_vm::{from_slice, Instance};
 use ibc_reflect::contract::{IBC_VERSION, RECEIVE_DISPATCH_ID};
 use ibc_reflect::msg::{
     AccountInfo, AccountResponse, AcknowledgementMsg, DispatchResponse, InstantiateMsg,
-    ListAccountsResponse, PacketMsg, QueryMsg, ReflectExecuteMsg, ReflectInstantiateMsg,
+    ListAccountsResponse, PacketMsg, QueryMsg, ReflectExecuteMsg,
 };
 
 // This line will test the output of cargo wasm
@@ -144,7 +144,7 @@ fn proper_handshake_flow() {
     if let CosmosMsg::Wasm(WasmMsg::Instantiate {
         admin,
         code_id,
-        msg,
+        msg: _,
         send,
         label,
     }) = &res.submessages[0].msg
@@ -153,9 +153,6 @@ fn proper_handshake_flow() {
         assert_eq!(*code_id, REFLECT_ID);
         assert_eq!(send.len(), 0);
         assert!(label.contains(channel_id));
-        // parse the message - should callback with proper channel_id
-        let rmsg: ReflectInstantiateMsg = from_slice(&msg).unwrap();
-        assert_eq!(rmsg.callback_id, Some(channel_id.to_string()));
     } else {
         panic!("invalid return message: {:?}", res.messages[0]);
     }
diff --git a/contracts/reflect/tests/integration.rs b/contracts/reflect/tests/integration.rs
index 775ae7f03..63cc8eb85 100644
--- a/contracts/reflect/tests/integration.rs
+++ b/contracts/reflect/tests/integration.rs
@@ -60,7 +60,7 @@ pub fn mock_dependencies_with_custom_querier(
 fn proper_initialization() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(1000, "earth"));
 
     // we can just call .unwrap() to assert this was a success
@@ -77,7 +77,7 @@ fn proper_initialization() {
 fn reflect() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(2, "token"));
     let _res: Response<CustomMsg> = instantiate(&mut deps, mock_env(), info, msg).unwrap();
 
@@ -110,7 +110,7 @@ fn reflect() {
 fn reflect_requires_owner() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(2, "token"));
     let _res: Response<CustomMsg> = instantiate(&mut deps, mock_env(), info, msg).unwrap();
 
@@ -132,7 +132,7 @@ fn reflect_requires_owner() {
 fn transfer() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(2, "token"));
     let _res: Response<CustomMsg> = instantiate(&mut deps, mock_env(), info, msg).unwrap();
 
@@ -152,7 +152,7 @@ fn transfer() {
 fn transfer_requires_owner() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(2, "token"));
     let _res: Response<CustomMsg> = instantiate(&mut deps, mock_env(), info, msg).unwrap();
 
@@ -190,7 +190,7 @@ fn dispatch_custom_query() {
 fn reflect_subcall() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(2, "token"));
     let _res: Response = instantiate(&mut deps, mock_env(), info, msg).unwrap();
 
@@ -222,7 +222,7 @@ fn reflect_subcall() {
 fn reply_and_query() {
     let mut deps = mock_instance(WASM, &[]);
 
-    let msg = InstantiateMsg { callback_id: None };
+    let msg = InstantiateMsg {};
     let info = mock_info("creator", &coins(2, "token"));
     let _res: Response = instantiate(&mut deps, mock_env(), info, msg).unwrap();