Skip to content

Commit

Permalink
refactor: introduce new Message type that can be deserialized safely (#…
Browse files Browse the repository at this point in the history
cgorenflo authored Sep 19, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 8d62f88 commit 273ae75
Showing 8 changed files with 182 additions and 15 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/basic.yaml
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions contracts/connection-router/src/error.rs
Original file line number Diff line number Diff line change
@@ -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,
1 change: 1 addition & 0 deletions contracts/connection-router/src/msg.rs
Original file line number Diff line number Diff line change
@@ -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,
84 changes: 82 additions & 2 deletions contracts/connection-router/src/state.rs
Original file line number Diff line number Diff line change
@@ -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 @@ impl<'a> IndexList<ChainEndpoint> for ChainEndpointIndexes<'a> {
// 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 @@ impl TryFrom<msg::Message> for Message {
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 @@ impl From<Message> for msg::Message {
}
}

#[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.
30 changes: 25 additions & 5 deletions contracts/connection-router/src/types.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
8 changes: 4 additions & 4 deletions contracts/connection-router/tests/test.rs
Original file line number Diff line number Diff line change
@@ -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()
);
}

2 changes: 2 additions & 0 deletions packages/axelar-wasm-std/src/nonempty/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
64 changes: 64 additions & 0 deletions packages/axelar-wasm-std/src/nonempty/string.rs
Original file line number Diff line number Diff line change
@@ -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());
}
}

0 comments on commit 273ae75

Please sign in to comment.