refactor: introduce new Message type that can be deserialized safely

.github/workflows/basic.yaml

@@ -8,7 +8,7 @@ on:
       - releases/**
 
 env:
-  RUSTFLAGS: -D warnings --cfg tracing_unstable
+  RUSTFLAGS: -D warnings -A deprecated --cfg tracing_unstable
 
 name: Basic
 
@@ -67,7 +67,7 @@ jobs:
           command: wasm
           args: --locked --workspace --exclude ampd
         env:
-          RUSTFLAGS: -D warnings --cfg tracing_unstable -C link-arg=-s
+          RUSTFLAGS: -D warnings -A deprecated --cfg tracing_unstable -C link-arg=-s
 
   lints:
     name: Lints

contracts/connection-router/src/error.rs

@@ -33,8 +33,8 @@ pub enum ContractError {
     #[error("chain is frozen")]
     ChainFrozen { chain: ChainName },
 
-    #[error("address of {0} is invalid")]
-    InvalidAddress(String),
+    #[error("address is invalid")]
+    InvalidAddress,
 
     #[error("source chain does not match registered gateway")]
     WrongSourceChain,

contracts/connection-router/src/msg.rs

@@ -6,6 +6,7 @@ use crate::types::GatewayDirection;
 // Message is a type meant to be used in interfaces where the data can be provided by the user.
 // The fields have not necessarily been validated, and should be checked prior to further processing.
 #[cw_serde]
+#[deprecated(note = "use NewMessage instead")]
 pub struct Message {
     pub id: String, // should be globally unique
     pub source_address: String,

contracts/connection-router/src/state.rs

@@ -1,8 +1,15 @@
+#![allow(deprecated)]
+
 use core::panic;
+use std::ops::Deref;
+use std::str::FromStr;
 
 use cosmwasm_schema::cw_serde;
 use cosmwasm_std::{Addr, DepsMut, HexBinary, Order, StdResult};
 use cw_storage_plus::{Index, IndexList, IndexedMap, Item, MultiIndex};
+use error_stack::{Report, ResultExt};
+
+use axelar_wasm_std::nonempty;
 
 use crate::{
     msg,
@@ -77,6 +84,30 @@
 // Message represents a message for which the fields have been successfully validated.
 // This should never be supplied by the user.
 #[cw_serde]
+pub struct NewMessage {
+    pub id_on_chain: MessageID, // globally unique
+    pub destination_address: Address,
+    pub destination_chain: ChainName,
+    pub source_chain: ChainName,
+    pub source_address: Address,
+    pub payload_hash: HexBinary,
+}
+
+impl NewMessage {
+    pub fn global_id(&self) -> MessageID {
+        format!(
+            "{}{}{}",
+            &self.source_chain, ID_SEPARATOR, &self.id_on_chain
+        )
+        .try_into()
+        .expect("a valid source chain and valid id should always produce a valid global message id")
+    }
+}
+
+// Message represents a message for which the fields have been successfully validated.
+// This should never be supplied by the user.
+#[cw_serde]
+#[deprecated(note = "use NewMessage instead")]
 pub struct Message {
     pub id: MessageID, // globally unique
     pub destination_address: String,
@@ -110,11 +141,11 @@
     type Error = ContractError;
     fn try_from(value: msg::Message) -> Result<Self, Self::Error> {
         if value.destination_address.is_empty() {
-            return Err(ContractError::InvalidAddress(value.destination_address));
+            return Err(ContractError::InvalidAddress);
         }
 
         if value.source_address.is_empty() {
-            return Err(ContractError::InvalidAddress(value.source_address));
+            return Err(ContractError::InvalidAddress);
         }
 
         if !value
@@ -148,14 +179,63 @@
     }
 }
 
+#[cw_serde]
+pub struct Address(nonempty::String);
+
+impl Deref for Address {
+    type Target = String;
+
+    fn deref(&self) -> &Self::Target {
+        self.0.deref()
+    }
+}
+
+impl FromStr for Address {
+    type Err = Report<ContractError>;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Address::try_from(s.to_string())
+    }
+}
+
+impl TryFrom<String> for Address {
+    type Error = Report<ContractError>;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        Ok(Address(
+            value
+                .parse::<nonempty::String>()
+                .change_context(ContractError::InvalidAddress)?,
+        ))
+    }
+}
+
 #[cfg(test)]
 mod tests {
+    use crate::state::NewMessage;
     use cosmwasm_std::to_vec;
     use hex;
     use sha3::{Digest, Sha3_256};
 
     use super::Message;
 
+    #[test]
+    fn create_correct_global_message_id() {
+        let msg = NewMessage {
+            id_on_chain: "hash:id".to_string().parse().unwrap(),
+            source_chain: "source_chain".to_string().parse().unwrap(),
+            source_address: "source_address".parse().unwrap(),
+            destination_chain: "destination_chain".parse().unwrap(),
+            destination_address: "destination_address".parse().unwrap(),
+            payload_hash: [1; 32].into(),
+        };
+
+        assert_eq!(
+            msg.global_id().to_string(),
+            "source_chain:hash:id".to_string()
+        );
+    }
+
     #[test]
     // Any modifications to the Message struct fields or their types
     // will cause this test to fail, indicating that a migration is needed.

contracts/connection-router/src/types.rs

@@ -1,14 +1,16 @@
 use std::fmt;
+use std::ops::Deref;
 use std::str::FromStr;
 
-use axelar_wasm_std::flagset::FlagSet;
 use cosmwasm_schema::cw_serde;
 use cosmwasm_std::{Addr, StdError, StdResult};
 use cw_storage_plus::{Key, KeyDeserialize, Prefixer, PrimaryKey};
 use flagset::flags;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
+use axelar_wasm_std::flagset::FlagSet;
+
 use crate::ContractError;
 
 pub const ID_SEPARATOR: char = ':';
@@ -21,7 +23,8 @@ impl FromStr for MessageID {
     type Err = ContractError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if !s.contains(ID_SEPARATOR) || s.is_empty() {
+        // todo: should be exactly 1 when migrated to state::NewMessage
+        if s.matches(ID_SEPARATOR).count() < 1 {
             return Err(ContractError::InvalidMessageID);
         }
         Ok(MessageID(s.to_lowercase()))
@@ -41,9 +44,11 @@ impl From<MessageID> for String {
     }
 }
 
-impl<'a> MessageID {
-    pub fn as_str(&'a self) -> &'a str {
-        self.0.as_str()
+impl Deref for MessageID {
+    type Target = String;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
     }
 }
 
@@ -218,4 +223,19 @@ mod tests {
                 .to_string()
         );
     }
+
+    #[test]
+    fn message_id_must_have_at_least_one_separator() {
+        assert!(MessageID::from_str("source_chain:hash:id").is_ok());
+        assert!(serde_json::from_str::<MessageID>("\"source_chain:hash:id\"").is_ok());
+
+        assert!(MessageID::from_str("invalid_hash").is_err());
+        assert!(serde_json::from_str::<MessageID>("\"invalid_hash\"").is_err());
+    }
+
+    #[test]
+    fn message_id_is_lower_case() {
+        let msg_id = "HaSH:iD".parse::<MessageID>().unwrap();
+        assert_eq!(msg_id.to_string(), "hash:id");
+    }
 }

contracts/connection-router/tests/test.rs

@@ -288,12 +288,12 @@ fn invalid_address() {
             &[],
         )
         .unwrap_err();
+
     assert_eq!(
         res.downcast::<axelar_wasm_std::ContractError>()
             .unwrap()
             .to_string(),
-        axelar_wasm_std::ContractError::from(ContractError::InvalidAddress("".to_string()))
-            .to_string()
+        axelar_wasm_std::ContractError::from(ContractError::InvalidAddress).to_string()
     );
 
     let res = config
@@ -308,12 +308,12 @@ fn invalid_address() {
             &[],
         )
         .unwrap_err();
+
     assert_eq!(
         res.downcast::<axelar_wasm_std::ContractError>()
             .unwrap()
             .to_string(),
-        axelar_wasm_std::ContractError::from(ContractError::InvalidAddress("".to_string()))
-            .to_string()
+        axelar_wasm_std::ContractError::from(ContractError::InvalidAddress).to_string()
     );
 }
 

packages/axelar-wasm-std/src/nonempty/mod.rs

@@ -1,9 +1,11 @@
 mod error;
+mod string;
 mod timestamp;
 mod uint;
 mod vec;
 
 pub use error::Error;
+pub use string::String;
 pub use timestamp::Timestamp;
 pub use uint::{Uint256, Uint64};
 pub use vec::Vec;

packages/axelar-wasm-std/src/nonempty/string.rs

@@ -0,0 +1,64 @@
+use crate::nonempty::Error;
+use cosmwasm_schema::cw_serde;
+use std::ops::Deref;
+use std::str::FromStr;
+
+#[cw_serde]
+#[serde(try_from = "std::string::String")]
+pub struct String(std::string::String);
+
+impl TryFrom<std::string::String> for String {
+    type Error = Error;
+
+    fn try_from(value: std::string::String) -> Result<Self, Self::Error> {
+        if value.is_empty() {
+            Err(Error::InvalidValue("empty".to_string()))
+        } else {
+            Ok(String(value))
+        }
+    }
+}
+
+impl TryFrom<&str> for String {
+    type Error = Error;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        String::try_from(value.to_string())
+    }
+}
+
+impl FromStr for String {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        s.try_into()
+    }
+}
+
+impl From<String> for std::string::String {
+    fn from(d: String) -> Self {
+        d.0
+    }
+}
+
+impl Deref for String {
+    type Target = std::string::String;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::nonempty;
+
+    #[test]
+    fn cannot_convert_from_empty_string() {
+        assert!(nonempty::String::try_from("").is_err());
+        assert!(serde_json::from_str::<nonempty::String>("\"\"").is_err());
+
+        assert!(nonempty::String::try_from("some string").is_ok());
+        assert!(serde_json::from_str::<nonempty::String>("\"some string\"").is_ok());
+    }
+}