Add query for remote address (#33)

* dinished passing subMsgResponse as is to callback

* remove unneeded annotation

* finish handling callback

* fmt

* Make remote address queryable and return in callback.

* Update simtest callback types

* Rebase done, fixing test.

* Fix the remaining tests.

* Simplify error interface.

wasmd does not return errors beyond the error code, so we can't return
meaningful errors for message execution until that is changed. this
refactors to reflect that as this was discovered during integration
testing.

* Add test for query failure.

* Remove outdated unit test

---------

Co-authored-by: Art3mix <r3mix.il@gmail.com>

contracts/note/src/contract.rs

@@ -4,6 +4,7 @@ use cosmwasm_std::{
     to_binary, Binary, Deps, DepsMut, Env, IbcMsg, IbcTimeout, MessageInfo, Response, StdResult,
 };
 use cw2::set_contract_version;
+use polytone::callback::CallbackRequestType;
 use polytone::{callback, ibc};
 
 use crate::error::ContractError;
@@ -42,20 +43,36 @@ pub fn execute(
     info: MessageInfo,
     msg: ExecuteMsg,
 ) -> Result<Response, ContractError> {
-    let (msg, callback, timeout_seconds) = match msg {
+    let (msg, callback, timeout_seconds, request_type) = match msg {
         ExecuteMsg::Execute {
             msgs,
             callback,
             timeout_seconds,
-        } => (ibc::Msg::Execute { msgs }, callback, timeout_seconds),
+        } => (
+            ibc::Msg::Execute { msgs },
+            callback,
+            timeout_seconds,
+            CallbackRequestType::Execute,
+        ),
         ExecuteMsg::Query {
             msgs,
             callback,
             timeout_seconds,
-        } => (ibc::Msg::Query { msgs }, Some(callback), timeout_seconds),
+        } => (
+            ibc::Msg::Query { msgs },
+            Some(callback),
+            timeout_seconds,
+            CallbackRequestType::Query,
+        ),
     };
 
-    callback::request_callback(deps.storage, deps.api, info.sender.clone(), callback)?;
+    callback::request_callback(
+        deps.storage,
+        deps.api,
+        info.sender.clone(),
+        callback,
+        request_type,
+    )?;
 
     let channel_id = CHANNEL
         .may_load(deps.storage)?
@@ -83,5 +100,9 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
                 remote_port,
             },
         )),
+        QueryMsg::RemoteAddress { local_address } => to_binary(
+            &callback::LOCAL_TO_REMOTE_ACCOUNT
+                .may_load(deps.storage, &deps.api.addr_validate(&local_address)?)?,
+        ),
     }
 }

contracts/note/src/msg.rs

@@ -44,6 +44,12 @@ pub enum QueryMsg {
     /// The contract's corresponding voice on a remote chain.
     #[returns(Option<Pair>)]
     Pair,
+    /// Returns the remote address for the provided local address. If
+    /// no account exists, returns `None`. An account can be created
+    /// by calling `ExecuteMsg::Execute` with the sender being
+    /// `local_address`.
+    #[returns(Option<String>)]
+    RemoteAddress { local_address: String },
 }
 
 /// This contract's voice. There is one voice per note, and many notes

contracts/proxy/Cargo.toml

@@ -22,6 +22,7 @@ cw-storage-plus = { workspace = true }
 cw-utils = { workspace = true }
 cw2 = { workspace = true }
 thiserror = { workspace = true }
+polytone = { workspace = true }
 
 [dev-dependencies]
 cw-multi-test = { workspace = true }

contracts/proxy/src/contract.rs

@@ -2,10 +2,10 @@
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
     to_binary, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg,
-    SubMsgResult,
+    SubMsgResponse, SubMsgResult,
 };
 use cw2::set_contract_version;
-use cw_utils::parse_execute_response_data;
+use polytone::ack::ack_execute_success;
 
 use crate::error::ContractError;
 use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg};
@@ -32,7 +32,7 @@ pub fn instantiate(
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn execute(
     deps: DepsMut,
-    _env: Env,
+    env: Env,
     info: MessageInfo,
     msg: ExecuteMsg,
 ) -> Result<Response, ContractError> {
@@ -47,7 +47,12 @@ pub fn execute(
                         msgs.into_iter()
                             .enumerate()
                             .map(|(id, msg)| SubMsg::reply_always(msg, id as u64)),
-                    ))
+                    )
+                    // handle `msgs.is_empty()` case
+                    .set_data(ack_execute_success(
+                        vec![],
+                        env.contract.address.into_string(),
+                    )))
             } else {
                 Err(ContractError::NotInstantiator)
             }
@@ -63,22 +68,29 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
 }
 
 #[cfg_attr(not(feature = "library"), entry_point)]
-pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractError> {
+pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result<Response, ContractError> {
     let mut collector = COLLECTOR.load(deps.storage)?;
 
     match msg.result {
-        SubMsgResult::Err(error) => Err(ContractError::ExecutionFailure { idx: msg.id, error }),
+        SubMsgResult::Err(error) => Err(ContractError::MsgError {
+            index: collector.len() as u64,
+            error,
+        }),
         SubMsgResult::Ok(res) => {
-            collector[msg.id as usize] = match res.data {
-                Some(data) => parse_execute_response_data(&data.0)?.data,
-                None => None,
-            };
+            collector[msg.id as usize] = Some(res);
 
             if msg.id + 1 == collector.len() as u64 {
                 COLLECTOR.remove(deps.storage);
+                let collector = collector
+                    .into_iter()
+                    .map(|res| res.unwrap())
+                    .collect::<Vec<SubMsgResponse>>();
                 Ok(Response::default()
                     .add_attribute("callbacks_processed", (msg.id + 1).to_string())
-                    .set_data(to_binary(&collector)?))
+                    .set_data(ack_execute_success(
+                        collector,
+                        env.contract.address.into_string(),
+                    )))
             } else {
                 COLLECTOR.save(deps.storage, &collector)?;
                 Ok(Response::default())

contracts/proxy/src/error.rs

@@ -2,17 +2,22 @@ use cosmwasm_std::StdError;
 use cw_utils::ParseReplyError;
 use thiserror::Error;
 
+// Take care when adding variants to this type that an attacker can't
+// create an error that will deserailize into a base64-encoded
+// `ExecutionFailure`, as the string representation of
+// `ExecutionFailure` is a base64-encoded, JSON `ExecutionFailure`.
+
 #[derive(Error, Debug)]
 pub enum ContractError {
     #[error(transparent)]
     Std(#[from] StdError),
 
     #[error(transparent)]
-    Reply(#[from] ParseReplyError),
+    Parse(#[from] ParseReplyError),
 
     #[error("caller must be the contract instantiator")]
     NotInstantiator,
 
-    #[error("host chain, msg ({idx}), error ({error})")]
-    ExecutionFailure { idx: u64, error: String },
+    #[error("executing message {index}: {error}")]
+    MsgError { index: u64, error: String },
 }

contracts/proxy/src/state.rs

@@ -1,9 +1,9 @@
-use cosmwasm_std::{Addr, Binary};
+use cosmwasm_std::{Addr, SubMsgResponse};
 use cw_storage_plus::Item;
 
 /// Stores the instantiator of the contract.
 pub const INSTANTIATOR: Item<Addr> = Item::new("owner");
 
 /// Stores a list of callback's currently being collected. Has no
 /// value if none are being collected.
-pub const COLLECTOR: Item<Vec<Option<Binary>>> = Item::new("callbacks");
+pub const COLLECTOR: Item<Vec<Option<SubMsgResponse>>> = Item::new("callbacks");

contracts/voice/src/contract.rs

@@ -2,16 +2,17 @@
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
     from_binary, instantiate2_address, to_binary, to_vec, Binary, CodeInfoResponse, ContractResult,
-    Deps, DepsMut, Env, MessageInfo, Response, StdError, StdResult, SubMsg, SystemResult, WasmMsg,
+    Deps, DepsMut, Env, MessageInfo, Response, StdResult, SubMsg, SystemResult, Uint64, WasmMsg,
 };
 use cw2::set_contract_version;
 
+use polytone::ack::{ack_query_fail, ack_query_success};
 use polytone::ibc::{Msg, Packet};
 
 use crate::error::ContractError;
 use crate::ibc::REPLY_FORWARD_DATA;
-use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg, MigrateMsg};
-use crate::state::{PROXY_CODE_ID, SENDER_TO_PROXY, BLOCK_MAX_GAS};
+use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg};
+use crate::state::{BLOCK_MAX_GAS, PROXY_CODE_ID, SENDER_TO_PROXY};
 
 const CONTRACT_NAME: &str = "crates.io:polytone-voice";
 const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");
@@ -52,25 +53,25 @@ pub fn execute(
                     Msg::Query { msgs } => {
                         let mut results = Vec::with_capacity(msgs.len());
                         for msg in msgs {
-                            let qr = deps.querier.raw_query(&to_vec(&msg)?);
-                            match qr {
-                                SystemResult::Ok(ContractResult::Err(e)) => {
-                                    return Err(StdError::generic_err(format!(
-                                        "query contract: {e}"
-                                    ))
-                                    .into())
+                            let query_result = deps.querier.raw_query(&to_vec(&msg)?);
+                            let error = match query_result {
+                                SystemResult::Ok(ContractResult::Err(error)) => error,
+                                SystemResult::Err(error) => error.to_string(),
+                                SystemResult::Ok(ContractResult::Ok(res)) => {
+                                    results.push(res);
+                                    continue;
                                 }
-                                SystemResult::Err(e) => {
-                                    return Err(
-                                        StdError::generic_err(format!("query system: {e}")).into()
-                                    )
-                                }
-                                SystemResult::Ok(ContractResult::Ok(res)) => results.push(res),
-                            }
+                            };
+                            return Ok(Response::default()
+                                .add_attribute("method", "rx_query_fail")
+                                .set_data(ack_query_fail(
+                                    Uint64::new(results.len() as u64),
+                                    error,
+                                )));
                         }
                         Ok(Response::default()
                             .add_attribute("method", "rx_query")
-                            .set_data(to_binary(&results)?))
+                            .set_data(ack_query_success(results)))
                     }
                     Msg::Execute { msgs } => {
                         let (instantiate, proxy) = if let Some(proxy) = SENDER_TO_PROXY.may_load(
@@ -112,7 +113,7 @@ pub fn execute(
                         Ok(Response::default()
                             .add_attribute("method", "rx_execute")
                             .add_messages(instantiate)
-                            .add_submessage(SubMsg::reply_on_success(
+                            .add_submessage(SubMsg::reply_always(
                                 WasmMsg::Execute {
                                     contract_addr: proxy.into_string(),
                                     msg: to_binary(&polytone_proxy::msg::ExecuteMsg::Proxy {
@@ -129,13 +130,17 @@ pub fn execute(
     }
 }
 
+/// Generates the salt used to generate an address for a user's
+/// account.
+///
 /// `local_channel` is not attacker controlled and protects from
 /// collision from an attacker generated duplicate
 /// chain. `remote_channel` ensures that two different modules on the
 /// same chain produce different addresses for the same
 /// `remote_sender`.
 fn salt(local_connection: &str, counterparty_port: &str, remote_sender: &str) -> Binary {
     use sha2::{Digest, Sha512};
+    // the salt can be a max of 64 bytes (512 bits).
     let hash = Sha512::default()
         .chain_update(local_connection.as_bytes())
         .chain_update(counterparty_port.as_bytes())
@@ -147,23 +152,22 @@ fn salt(local_connection: &str, counterparty_port: &str, remote_sender: &str) ->
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
     match msg {
-        QueryMsg::BlockMaxGas => to_binary(
-            &BLOCK_MAX_GAS.load(deps.storage)?,
-        ),
-        QueryMsg::ProxyCodeId => to_binary(
-            &PROXY_CODE_ID.load(deps.storage)?,
-        ),
+        QueryMsg::BlockMaxGas => to_binary(&BLOCK_MAX_GAS.load(deps.storage)?),
+        QueryMsg::ProxyCodeId => to_binary(&PROXY_CODE_ID.load(deps.storage)?),
     }
 }
 
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, ContractError> {
     match msg {
-        MigrateMsg::WithUpdate {proxy_code_id, block_max_gas } => {
+        MigrateMsg::WithUpdate {
+            proxy_code_id,
+            block_max_gas,
+        } => {
             // update the proxy code ID and block max gas
             PROXY_CODE_ID.save(deps.storage, &proxy_code_id.u64())?;
             BLOCK_MAX_GAS.save(deps.storage, &block_max_gas.u64())?;
-            
+
             Ok(Response::default().add_attribute("method", "migrate_with_update"))
         }
     }