Skip to content

Commit

Permalink
fix(ucs01): better event and generalize u128 over u256
Browse files Browse the repository at this point in the history
  • Loading branch information
hussein-aitlahcen committed May 14, 2024
1 parent 83eae9d commit f53727d
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 54 deletions.
67 changes: 51 additions & 16 deletions cosmwasm/ucs01-relay-api/src/protocol.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::fmt::Debug;

use cosmwasm_std::{
Addr, Binary, CosmosMsg, Event, IbcBasicResponse, IbcEndpoint, IbcMsg, IbcOrder,
IbcReceiveResponse, Response, SubMsg, Timestamp,
Addr, Attribute, Binary, Coin, CosmosMsg, Event, IbcBasicResponse, IbcEndpoint, IbcMsg,
IbcOrder, IbcReceiveResponse, Response, SubMsg, Timestamp,
};
use thiserror::Error;

Expand Down Expand Up @@ -37,12 +37,20 @@ pub struct TransferInput {
pub tokens: Vec<TransferToken>,
}

pub fn token_to_attr(TransferToken { denom, amount }: &TransferToken) -> Attribute {
(
"denom",
cosmwasm_std::to_json_string(&Coin::new(*amount, denom)).expect("impossible"),
)
.into()
}

// We follow the following module implementation, events and attributes are
// almost 1:1 with the traditional go implementation. As we generalized the base
// implementation for multi-tokens transfer, the events are not containing a
// single ('denom', 'value') and ('amount', 'value') attributes but rather a set
// of ('denom:x', 'amount_value') attributes for each denom `x` that is
// transferred. i.e. [('denom:muno', '10'), ('denom:port/channel/weth', '150'), ..]
// of ('denom', json_string(('x', '10'))) attributes for each denom `x` with
// amount 10 that is transferred.
// https://github.com/cosmos/ibc-go/blob/7be17857b10457c67cbf66a49e13a9751eb10e8e/modules/apps/transfer/ibc_module.go
pub trait TransferProtocol {
/// Must be unique per Protocol
Expand Down Expand Up @@ -144,9 +152,7 @@ pub trait TransferProtocol {
("sender", input.sender.as_str()),
("receiver", input.receiver.as_str()),
])
.add_attributes(input.tokens.into_iter().map(
|TransferToken { denom, amount }| (format!("denom:{}", denom), amount),
)),
.add_attributes(input.tokens.into_iter().map(|token| token_to_attr(&token))),
Event::new("message").add_attribute("module", MODULE_NAME),
]))
}
Expand Down Expand Up @@ -190,9 +196,12 @@ pub trait TransferProtocol {
("receiver", packet.receiver().to_string().as_str()),
("acknowledgement", &raw_ack.into().to_string()),
])
.add_attributes(packet.tokens().into_iter().map(
|TransferToken { denom, amount }| (format!("denom:{}", denom), amount),
)),
.add_attributes(
packet
.tokens()
.into_iter()
.map(|token| token_to_attr(&token)),
),
)
.add_event(Event::new(PACKET_EVENT).add_attributes(ack_attr))
.add_messages(ack_msgs))
Expand Down Expand Up @@ -221,9 +230,12 @@ pub trait TransferProtocol {
("module", MODULE_NAME),
("refund_receiver", packet.sender().to_string().as_str()),
])
.add_attributes(packet.tokens().into_iter().map(
|TransferToken { denom, amount }| (format!("denom:{}", denom), amount),
)),
.add_attributes(
packet
.tokens()
.into_iter()
.map(|token| token_to_attr(&token)),
),
)
.add_messages(refund_msgs))
}
Expand Down Expand Up @@ -267,9 +279,12 @@ pub trait TransferProtocol {
("receiver", packet.receiver().to_string().as_str()),
("success", "true"),
])
.add_attributes(packet.tokens().into_iter().map(
|TransferToken { denom, amount }| (format!("denom:{}", denom), amount),
)),
.add_attributes(
packet
.tokens()
.into_iter()
.map(|token| token_to_attr(&token)),
),
)
.add_submessages(transfer_msgs))
};
Expand All @@ -295,3 +310,23 @@ pub trait TransferProtocol {
]))
}
}

#[cfg(test)]
mod tests {
use cosmwasm_std::{Coin, Uint128};

use crate::{protocol::token_to_attr, types::TransferToken};

#[test]
fn test_token_attr() {
let token = TransferToken {
denom: "factory/1/2/3".into(),
amount: 0xDEAD_u64.into(),
};
let attr = token_to_attr(&token);
let coin = cosmwasm_std::from_json::<Coin>(attr.value).unwrap();
assert_eq!(attr.key, "denom");
assert_eq!(coin.denom, "factory/1/2/3");
assert_eq!(coin.amount, Uint128::from(0xDEAD_u64));
}
}
25 changes: 12 additions & 13 deletions cosmwasm/ucs01-relay-api/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cosmwasm_schema::cw_serde;
use cosmwasm_std::{Binary, Coin, HexBinary, IbcEndpoint, Uint256};
use cosmwasm_std::{Binary, Coin, HexBinary, IbcEndpoint, Uint128, Uint256};
use ethabi::{ParamType, Token};

pub type GenericAck = Result<Binary, String>;
Expand All @@ -23,14 +23,14 @@ pub struct TransferPacketCommon<T> {
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct TransferToken {
pub denom: String,
pub amount: Uint256,
pub amount: Uint128,
}

impl From<Coin> for TransferToken {
fn from(value: Coin) -> Self {
Self {
denom: value.denom,
amount: value.amount.into(),
amount: value.amount,
}
}
}
Expand Down Expand Up @@ -81,7 +81,7 @@ impl TryFrom<Ucs01TransferPacket> for Binary {
.map(|TransferToken { denom, amount }| {
Token::Tuple(vec![
Token::String(denom),
Token::Uint(amount.to_be_bytes().into()),
Token::Uint(Uint256::from(amount).to_be_bytes().into()),
])
})
.collect(),
Expand All @@ -101,7 +101,7 @@ impl TryFrom<Binary> for Ucs01TransferPacket {
ParamType::Bytes,
ParamType::Array(Box::new(ParamType::Tuple(vec![
ParamType::String,
ParamType::Uint(256),
ParamType::Uint(128),
]))),
],
&value,
Expand All @@ -121,8 +121,7 @@ impl TryFrom<Binary> for Ucs01TransferPacket {
match &encoded_token_inner[..] {
[Token::String(denom), Token::Uint(amount)] => TransferToken {
denom: denom.clone(),
// NOTE: both structures uses big endian by default
amount: Uint256::new((*amount).into()),
amount: Uint128::new(amount.as_u128()),
},
_ => unreachable!(),
}
Expand All @@ -143,7 +142,7 @@ impl TryFrom<Binary> for Ucs01TransferPacket {
#[cw_serde]
pub struct Ics20Packet {
pub denom: String,
pub amount: Uint256,
pub amount: Uint128,
pub sender: String,
pub receiver: String,
#[serde(default, skip_serializing_if = "String::is_empty")]
Expand Down Expand Up @@ -362,7 +361,7 @@ impl<'a> From<(&'a str, &IbcEndpoint)> for DenomOrigin<'a> {

#[cfg(test)]
mod tests {
use cosmwasm_std::{Binary, IbcEndpoint, Uint256};
use cosmwasm_std::{Binary, IbcEndpoint, Uint128};

use super::{Ics20Packet, TransferToken, Ucs01Ack, Ucs01TransferPacket};
use crate::types::{DenomOrigin, Ics20Ack};
Expand All @@ -375,15 +374,15 @@ mod tests {
tokens: vec![
TransferToken {
denom: "denom-0".into(),
amount: Uint256::from(1u32),
amount: Uint128::from(1u32),
},
TransferToken {
denom: "denom-1".into(),
amount: Uint256::MAX,
amount: Uint128::MAX,
},
TransferToken {
denom: "denom-2".into(),
amount: Uint256::from(1337u32),
amount: Uint128::from(1337u32),
},
],
};
Expand Down Expand Up @@ -412,7 +411,7 @@ mod tests {
fn ics20_packet_encode_decode_iso() {
let packet = Ics20Packet {
denom: "a".into(),
amount: Uint256::from(1337u32),
amount: Uint128::from(1337u32),
sender: "c".into(),
receiver: "d".into(),
memo: "bla".into(),
Expand Down
40 changes: 17 additions & 23 deletions cosmwasm/ucs01-relay/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,6 @@ trait OnReceive {
tokens
.into_iter()
.map(|TransferToken { denom, amount }| {
let amount = amount
.try_into()
.expect("CosmWasm require transferred amount to be Uint128...");
match DenomOrigin::from((denom.as_str(), counterparty_endpoint)) {
DenomOrigin::Local { denom } => {
self.local_unescrow(&endpoint.channel_id, denom, amount)?;
Expand Down Expand Up @@ -279,9 +276,6 @@ trait ForTokens {
) -> Result<Vec<CosmosMsg<TokenFactoryMsg>>, ContractError> {
let mut messages = Vec::with_capacity(tokens.len());
for TransferToken { denom, amount } in tokens {
let amount = amount
.try_into()
.expect("CosmWasm require transferred amount to be Uint128...");
// This is the origin from the counterparty POV
match DenomOrigin::from((denom.as_str(), endpoint)) {
DenomOrigin::Local { denom } => {
Expand Down Expand Up @@ -645,7 +639,7 @@ impl<'a> TransferProtocol for Ucs01Protocol<'a> {
mod tests {
use cosmwasm_std::{
testing::mock_dependencies, wasm_execute, Addr, BankMsg, Coin, CosmosMsg, IbcEndpoint,
Uint128, Uint256,
Uint128,
};
use token_factory_api::TokenFactoryMsg;
use ucs01_relay_api::types::TransferToken;
Expand Down Expand Up @@ -703,7 +697,7 @@ mod tests {
"receiver",
vec![TransferToken {
denom: "from-counterparty".into(),
amount: Uint256::from(100u128)
amount: Uint128::from(100u128)
},],
),
Ok(vec![
Expand Down Expand Up @@ -760,7 +754,7 @@ mod tests {
"receiver",
vec![TransferToken {
denom: "from-counterparty".into(),
amount: Uint256::from(100u128),
amount: Uint128::from(100u128),
}],
)
.unwrap()[0]
Expand All @@ -777,7 +771,7 @@ mod tests {
"receiver",
vec![TransferToken {
denom: "from-counterparty".into(),
amount: Uint256::from(100u128),
amount: Uint128::from(100u128),
}],
)
.unwrap()[0]
Expand All @@ -804,7 +798,7 @@ mod tests {
"receiver",
vec![TransferToken {
denom: "from-counterparty".into(),
amount: Uint256::from(100u128)
amount: Uint128::from(100u128)
},],
),
Ok(vec![TokenFactoryMsg::MintTokens {
Expand Down Expand Up @@ -835,7 +829,7 @@ mod tests {
"receiver",
vec![TransferToken {
denom: "transfer/channel-34/local-denom".into(),
amount: Uint256::from(119u128)
amount: Uint128::from(119u128)
}],
),
Ok(vec![BankMsg::Send {
Expand Down Expand Up @@ -893,15 +887,15 @@ mod tests {
vec![
TransferToken {
denom: "transfer-source/blabla/remote-denom".into(),
amount: Uint256::from(119u128)
amount: Uint128::from(119u128)
},
TransferToken {
denom: "transfer-source/blabla-2/remote-denom".into(),
amount: Uint256::from(10u128)
amount: Uint128::from(10u128)
},
TransferToken {
denom: "transfer-source/blabla/remote-denom2".into(),
amount: Uint256::from(129u128)
amount: Uint128::from(129u128)
},
],
),
Expand Down Expand Up @@ -970,11 +964,11 @@ mod tests {
vec![
TransferToken {
denom: "transfer/channel-2/remote-denom".into(),
amount: Uint256::from(119u128)
amount: Uint128::from(119u128)
},
TransferToken {
denom: "transfer/channel-2/remote-denom2".into(),
amount: Uint256::from(129u128)
amount: Uint128::from(129u128)
}
],
),
Expand All @@ -995,12 +989,12 @@ mod tests {
},
TransferToken {
denom: "factory/0xDEADC0DE/0xaf30fd00576e1d27471a4d2b0c0487dc6876e0589e".into(),
amount: Uint256::MAX
amount: Uint128::MAX
}
),
Ok(TransferToken {
denom: "factory/0xDEADC0DE/0xaf30fd00576e1d27471a4d2b0c0487dc6876e0589e".into(),
amount: Uint256::MAX
amount: Uint128::MAX
})
);
assert_eq!(
Expand All @@ -1013,12 +1007,12 @@ mod tests {
},
TransferToken {
denom: "factory/0xDEADC0DE/0xaf30fd00576e1d27471a4d2b0c0487dc6876e0589e".into(),
amount: Uint256::MAX
amount: Uint128::MAX
}
),
Ok(TransferToken {
denom: "factory/0xDEADC0DE/0xaf30fd00576e1d27471a4d2b0c0487dc6876e0589e".into(),
amount: Uint256::MAX
amount: Uint128::MAX
})
);
}
Expand All @@ -1035,12 +1029,12 @@ mod tests {
},
TransferToken {
denom: "factory/0xDEADC0DE/0xaf30fd00576e1d27471a4d2b0c0487dc6876e0589e".into(),
amount: Uint256::MAX
amount: Uint128::MAX
}
),
Ok(TransferToken {
denom: "transfer/channel-332/blabla-1".into(),
amount: Uint256::MAX
amount: Uint128::MAX
})
);
}
Expand Down
4 changes: 2 additions & 2 deletions evm/contracts/apps/ucs/01-relay/Relay.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct LocalToken {

struct Token {
string denom;
uint256 amount;
uint128 amount;
}

struct RelayPacket {
Expand Down Expand Up @@ -284,7 +284,7 @@ contract UCS01Relay is
for (uint256 i = 0; i < tokensLength; i++) {
LocalToken calldata localToken = tokens[i];
normalizedTokens[i].denom = sendToken(sourceChannel, localToken);
normalizedTokens[i].amount = uint256(localToken.amount);
normalizedTokens[i].amount = localToken.amount;
}
string memory sender = msg.sender.toHexString();
RelayPacket memory packet = RelayPacket({
Expand Down

0 comments on commit f53727d

Please sign in to comment.