refactor(minor): rename id field in CrosschainId to message_id (#538)

contracts/gateway/src/contract.rs

@@ -6,9 +6,11 @@ use cosmwasm_std::{Binary, Deps, DepsMut, Empty, Env, MessageInfo, Response};
 use gateway_api::msg::{ExecuteMsg, QueryMsg};
 use router_api::CrossChainId;
 
+use crate::contract::migrations::v0_2_3;
 use crate::msg::InstantiateMsg;
 
 mod execute;
+mod migrations;
 mod query;
 
 const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME");
@@ -20,10 +22,7 @@ pub fn migrate(
     _env: Env,
     _msg: Empty,
 ) -> Result<Response, axelar_wasm_std::error::ContractError> {
-    // any version checks should be done before here
-
-    cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
-
+    v0_2_3::migrate(deps.storage)?;
     Ok(Response::default())
 }
 
@@ -147,21 +146,3 @@ mod internal {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use cosmwasm_std::testing::{mock_dependencies, mock_env};
-
-    use super::*;
-
-    #[test]
-    fn migrate_sets_contract_version() {
-        let mut deps = mock_dependencies();
-
-        migrate(deps.as_mut(), mock_env(), Empty {}).unwrap();
-
-        let contract_version = cw2::get_contract_version(deps.as_mut().storage).unwrap();
-        assert_eq!(contract_version.contract, "gateway");
-        assert_eq!(contract_version.version, CONTRACT_VERSION);
-    }
-}

contracts/gateway/src/contract/execute.rs

@@ -37,7 +37,8 @@ pub(crate) fn route_outgoing_messages(
     let msgs = check_for_duplicates(verified)?;
 
     for msg in msgs.iter() {
-        state::save_outgoing_msg(store, &msg.cc_id, msg)
+        state::OUTGOING_MESSAGES
+            .save(store, &msg.cc_id, msg)
             .change_context(Error::InvalidStoreAccess)?;
     }
 

contracts/gateway/src/contract/migrations/mod.rs

@@ -0,0 +1 @@
+pub mod v0_2_3;

contracts/gateway/src/contract/migrations/v0_2_3.rs

@@ -0,0 +1,185 @@
+#![allow(deprecated)]
+
+use std::any::type_name;
+
+use axelar_wasm_std::error::ContractError;
+use axelar_wasm_std::nonempty;
+use cosmwasm_schema::cw_serde;
+use cosmwasm_std::{StdError, StdResult, Storage};
+use cw_storage_plus::{Key, KeyDeserialize, Map, PrimaryKey};
+use router_api::{Address, ChainName, ChainNameRaw};
+
+use crate::contract::{CONTRACT_NAME, CONTRACT_VERSION};
+
+const BASE_VERSION: &str = "0.2.3";
+
+pub fn migrate(storage: &mut dyn Storage) -> Result<(), ContractError> {
+    cw2::assert_contract_version(storage, CONTRACT_NAME, BASE_VERSION)?;
+
+    delete_outgoing_messages(storage);
+
+    cw2::set_contract_version(storage, CONTRACT_NAME, CONTRACT_VERSION)?;
+
+    Ok(())
+}
+
+fn delete_outgoing_messages(storage: &mut dyn Storage) {
+    OUTGOING_MESSAGES.clear(storage);
+}
+
+#[deprecated(since = "0.2.3", note = "only used during migration")]
+const OUTGOING_MESSAGES_NAME: &str = "outgoing_messages";
+
+#[deprecated(since = "0.2.3", note = "only used during migration")]
+const OUTGOING_MESSAGES: Map<&CrossChainId, Message> = Map::new(OUTGOING_MESSAGES_NAME);
+
+#[cw_serde]
+#[derive(Eq, Hash)]
+#[deprecated(since = "0.2.3", note = "only used during migration")]
+pub struct Message {
+    pub cc_id: CrossChainId,
+    pub source_address: Address,
+    pub destination_chain: ChainName,
+    pub destination_address: Address,
+    /// for better user experience, the payload hash gets encoded into hex at the edges (input/output),
+    /// but internally, we treat it as raw bytes to enforce its format.
+    #[serde(with = "axelar_wasm_std::hex")]
+    #[schemars(with = "String")] // necessary attribute in conjunction with #[serde(with ...)]
+    pub payload_hash: [u8; 32],
+}
+
+#[cw_serde]
+#[derive(Eq, Hash)]
+#[deprecated(since = "0.2.3", note = "only used during migration")]
+pub struct CrossChainId {
+    pub chain: ChainNameRaw,
+    pub id: nonempty::String,
+}
+
+impl PrimaryKey<'_> for CrossChainId {
+    type Prefix = ChainNameRaw;
+    type SubPrefix = ();
+    type Suffix = String;
+    type SuperSuffix = (ChainNameRaw, String);
+
+    fn key(&self) -> Vec<Key> {
+        let mut keys = self.chain.key();
+        keys.extend(self.id.key());
+        keys
+    }
+}
+
+impl KeyDeserialize for &CrossChainId {
+    type Output = CrossChainId;
+
+    fn from_vec(value: Vec<u8>) -> StdResult<Self::Output> {
+        let (chain, id) = <(ChainNameRaw, String)>::from_vec(value)?;
+        Ok(CrossChainId {
+            chain,
+            id: id
+                .try_into()
+                .map_err(|err| StdError::parse_err(type_name::<nonempty::String>(), err))?,
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info};
+    use cosmwasm_std::DepsMut;
+
+    use crate::contract::migrations::v0_2_3;
+    use crate::contract::{instantiate, CONTRACT_NAME, CONTRACT_VERSION};
+    use crate::msg::InstantiateMsg;
+    use crate::state;
+
+    #[test]
+    fn migrate_checks_contract_version() {
+        let mut deps = mock_dependencies();
+        instantiate_contract(deps.as_mut());
+
+        cw2::set_contract_version(deps.as_mut().storage, CONTRACT_NAME, "something wrong").unwrap();
+
+        assert!(v0_2_3::migrate(deps.as_mut().storage).is_err());
+
+        cw2::set_contract_version(deps.as_mut().storage, CONTRACT_NAME, v0_2_3::BASE_VERSION)
+            .unwrap();
+
+        assert!(v0_2_3::migrate(deps.as_mut().storage).is_ok());
+    }
+
+    #[test]
+    fn migrate_sets_contract_version() {
+        let mut deps = mock_dependencies();
+        instantiate_contract(deps.as_mut());
+
+        v0_2_3::migrate(deps.as_mut().storage).unwrap();
+
+        let contract_version = cw2::get_contract_version(deps.as_mut().storage).unwrap();
+        assert_eq!(contract_version.contract, CONTRACT_NAME);
+        assert_eq!(contract_version.version, CONTRACT_VERSION);
+    }
+
+    fn instantiate_contract(deps: DepsMut) {
+        instantiate(
+            deps,
+            mock_env(),
+            mock_info("admin", &[]),
+            InstantiateMsg {
+                verifier_address: "verifier".to_string(),
+                router_address: "router".to_string(),
+            },
+        )
+        .unwrap();
+    }
+
+    #[test]
+    fn migrate_outgoing_messages() {
+        let mut deps = mock_dependencies();
+
+        instantiate_contract(deps.as_mut());
+
+        let msgs = vec![
+            v0_2_3::Message {
+                cc_id: v0_2_3::CrossChainId {
+                    id: "id1".try_into().unwrap(),
+                    chain: "chain1".try_into().unwrap(),
+                },
+                source_address: "source-address".parse().unwrap(),
+                destination_chain: "destination".parse().unwrap(),
+                destination_address: "destination-address".parse().unwrap(),
+                payload_hash: [1; 32],
+            },
+            v0_2_3::Message {
+                cc_id: v0_2_3::CrossChainId {
+                    id: "id2".try_into().unwrap(),
+                    chain: "chain2".try_into().unwrap(),
+                },
+                source_address: "source-address2".parse().unwrap(),
+                destination_chain: "destination2".parse().unwrap(),
+                destination_address: "destination-address2".parse().unwrap(),
+                payload_hash: [2; 32],
+            },
+            v0_2_3::Message {
+                cc_id: v0_2_3::CrossChainId {
+                    id: "id3".try_into().unwrap(),
+                    chain: "chain3".try_into().unwrap(),
+                },
+                source_address: "source-address3".parse().unwrap(),
+                destination_chain: "destination3".parse().unwrap(),
+                destination_address: "destination-address3".parse().unwrap(),
+                payload_hash: [3; 32],
+            },
+        ];
+
+        for msg in msgs.iter() {
+            v0_2_3::OUTGOING_MESSAGES
+                .save(deps.as_mut().storage, &msg.cc_id, msg)
+                .unwrap();
+        }
+
+        assert!(v0_2_3::migrate(deps.as_mut().storage).is_ok());
+
+        assert!(state::OUTGOING_MESSAGES.is_empty(deps.as_ref().storage))
+    }
+}

contracts/gateway/src/contract/query.rs

@@ -17,7 +17,8 @@ pub fn outgoing_messages(
 }
 
 fn try_load_msg(storage: &dyn Storage, id: CrossChainId) -> Result<Message, Error> {
-    state::may_load_outgoing_msg(storage, &id)
+    state::OUTGOING_MESSAGES
+        .may_load(storage, &id)
         .change_context(Error::InvalidStoreAccess)
         .transpose()
         .unwrap_or(Err(report!(Error::MessageNotFound(id))))
@@ -51,7 +52,9 @@ mod test {
         let messages = generate_messages();
 
         for message in messages.iter() {
-            state::save_outgoing_msg(deps.as_mut().storage, &message.cc_id, message).unwrap();
+            state::OUTGOING_MESSAGES
+                .save(deps.as_mut().storage, &message.cc_id, message)
+                .unwrap();
         }
 
         let ids = messages.iter().map(|msg| msg.cc_id.clone()).collect();
@@ -79,7 +82,9 @@ mod test {
 
         let messages = generate_messages();
 
-        state::save_outgoing_msg(deps.as_mut().storage, &messages[1].cc_id, &messages[1]).unwrap();
+        state::OUTGOING_MESSAGES
+            .save(deps.as_mut().storage, &messages[1].cc_id, &messages[1])
+            .unwrap();
 
         let ids = messages.iter().map(|msg| msg.cc_id.clone()).collect();
 

contracts/gateway/src/state.rs

@@ -21,49 +21,26 @@ pub(crate) fn load_config(storage: &dyn Storage) -> Result<Config, Error> {
         .change_context(Error::LoadValue(CONFIG_NAME))
 }
 
-pub(crate) fn save_outgoing_msg(
-    storage: &mut dyn Storage,
-    key: &CrossChainId,
-    value: &Message,
-) -> Result<(), Error> {
-    OUTGOING_MESSAGES
-        .save(storage, key, value)
-        .change_context(Error::SaveValue(OUTGOING_MESSAGES_NAME))
-}
-pub(crate) fn may_load_outgoing_msg(
-    storage: &dyn Storage,
-    id: &CrossChainId,
-) -> Result<Option<Message>, Error> {
-    OUTGOING_MESSAGES
-        .may_load(storage, id)
-        .change_context(Error::Parse(OUTGOING_MESSAGES_NAME))
-        .attach_printable(id.to_string())
-}
-
 #[derive(thiserror::Error, Debug)]
 pub(crate) enum Error {
     #[error("failed to save {0}")]
     SaveValue(&'static str),
     #[error("failed to load {0}")]
     LoadValue(&'static str),
-    #[error("failed to parse key for {0}")]
-    Parse(&'static str),
 }
 
 const CONFIG_NAME: &str = "config";
 const CONFIG: Item<Config> = Item::new(CONFIG_NAME);
 const OUTGOING_MESSAGES_NAME: &str = "outgoing_messages";
-const OUTGOING_MESSAGES: Map<&CrossChainId, Message> = Map::new(OUTGOING_MESSAGES_NAME);
+pub const OUTGOING_MESSAGES: Map<&CrossChainId, Message> = Map::new(OUTGOING_MESSAGES_NAME);
 
 #[cfg(test)]
 mod test {
     use cosmwasm_std::testing::mock_dependencies;
     use cosmwasm_std::Addr;
     use router_api::{CrossChainId, Message};
 
-    use crate::state::{
-        load_config, may_load_outgoing_msg, save_config, save_outgoing_msg, Config,
-    };
+    use crate::state::{load_config, save_config, Config, OUTGOING_MESSAGES};
 
     #[test]
     fn config_storage() {
@@ -90,23 +67,31 @@ mod test {
             payload_hash: [1; 32],
         };
 
-        assert!(save_outgoing_msg(deps.as_mut().storage, &message.cc_id, &message).is_ok());
+        assert!(OUTGOING_MESSAGES
+            .save(deps.as_mut().storage, &message.cc_id, &message)
+            .is_ok());
 
         assert_eq!(
-            may_load_outgoing_msg(&deps.storage, &message.cc_id).unwrap(),
+            OUTGOING_MESSAGES
+                .may_load(&deps.storage, &message.cc_id)
+                .unwrap(),
             Some(message)
         );
 
         let unknown_chain_id = CrossChainId::new("unknown", "id").unwrap();
 
         assert_eq!(
-            may_load_outgoing_msg(&deps.storage, &unknown_chain_id).unwrap(),
+            OUTGOING_MESSAGES
+                .may_load(&deps.storage, &unknown_chain_id)
+                .unwrap(),
             None
         );
 
         let unknown_id = CrossChainId::new("chain", "unkown").unwrap();
         assert_eq!(
-            may_load_outgoing_msg(&deps.storage, &unknown_id).unwrap(),
+            OUTGOING_MESSAGES
+                .may_load(&deps.storage, &unknown_id)
+                .unwrap(),
             None
         );
     }