From f53727dc68c369da468004ad1fc9925aa3b93230 Mon Sep 17 00:00:00 2001 From: Hussein Ait Lahcen Date: Tue, 14 May 2024 14:45:59 +0200 Subject: [PATCH] fix(ucs01): better event and generalize u128 over u256 --- cosmwasm/ucs01-relay-api/src/protocol.rs | 67 +++++++++++++++++------ cosmwasm/ucs01-relay-api/src/types.rs | 25 ++++----- cosmwasm/ucs01-relay/src/protocol.rs | 40 ++++++-------- evm/contracts/apps/ucs/01-relay/Relay.sol | 4 +- 4 files changed, 82 insertions(+), 54 deletions(-) diff --git a/cosmwasm/ucs01-relay-api/src/protocol.rs b/cosmwasm/ucs01-relay-api/src/protocol.rs index 4d3fead7f5..4ebc6ca22c 100644 --- a/cosmwasm/ucs01-relay-api/src/protocol.rs +++ b/cosmwasm/ucs01-relay-api/src/protocol.rs @@ -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; @@ -37,12 +37,20 @@ pub struct TransferInput { pub tokens: Vec, } +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 @@ -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), ])) } @@ -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)) @@ -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)) } @@ -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)) }; @@ -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::(attr.value).unwrap(); + assert_eq!(attr.key, "denom"); + assert_eq!(coin.denom, "factory/1/2/3"); + assert_eq!(coin.amount, Uint128::from(0xDEAD_u64)); + } +} diff --git a/cosmwasm/ucs01-relay-api/src/types.rs b/cosmwasm/ucs01-relay-api/src/types.rs index f6333c1bee..e634fea9af 100644 --- a/cosmwasm/ucs01-relay-api/src/types.rs +++ b/cosmwasm/ucs01-relay-api/src/types.rs @@ -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; @@ -23,14 +23,14 @@ pub struct TransferPacketCommon { #[derive(Clone, PartialEq, Eq, Debug)] pub struct TransferToken { pub denom: String, - pub amount: Uint256, + pub amount: Uint128, } impl From for TransferToken { fn from(value: Coin) -> Self { Self { denom: value.denom, - amount: value.amount.into(), + amount: value.amount, } } } @@ -81,7 +81,7 @@ impl TryFrom 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(), @@ -101,7 +101,7 @@ impl TryFrom for Ucs01TransferPacket { ParamType::Bytes, ParamType::Array(Box::new(ParamType::Tuple(vec![ ParamType::String, - ParamType::Uint(256), + ParamType::Uint(128), ]))), ], &value, @@ -121,8 +121,7 @@ impl TryFrom 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!(), } @@ -143,7 +142,7 @@ impl TryFrom 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")] @@ -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}; @@ -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), }, ], }; @@ -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(), diff --git a/cosmwasm/ucs01-relay/src/protocol.rs b/cosmwasm/ucs01-relay/src/protocol.rs index 5abf26d737..fd1ff01b05 100644 --- a/cosmwasm/ucs01-relay/src/protocol.rs +++ b/cosmwasm/ucs01-relay/src/protocol.rs @@ -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)?; @@ -279,9 +276,6 @@ trait ForTokens { ) -> Result>, 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 } => { @@ -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; @@ -703,7 +697,7 @@ mod tests { "receiver", vec![TransferToken { denom: "from-counterparty".into(), - amount: Uint256::from(100u128) + amount: Uint128::from(100u128) },], ), Ok(vec![ @@ -760,7 +754,7 @@ mod tests { "receiver", vec![TransferToken { denom: "from-counterparty".into(), - amount: Uint256::from(100u128), + amount: Uint128::from(100u128), }], ) .unwrap()[0] @@ -777,7 +771,7 @@ mod tests { "receiver", vec![TransferToken { denom: "from-counterparty".into(), - amount: Uint256::from(100u128), + amount: Uint128::from(100u128), }], ) .unwrap()[0] @@ -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 { @@ -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 { @@ -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) }, ], ), @@ -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) } ], ), @@ -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!( @@ -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 }) ); } @@ -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 }) ); } diff --git a/evm/contracts/apps/ucs/01-relay/Relay.sol b/evm/contracts/apps/ucs/01-relay/Relay.sol index 6bb219857d..3b22903469 100644 --- a/evm/contracts/apps/ucs/01-relay/Relay.sol +++ b/evm/contracts/apps/ucs/01-relay/Relay.sol @@ -27,7 +27,7 @@ struct LocalToken { struct Token { string denom; - uint256 amount; + uint128 amount; } struct RelayPacket { @@ -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({