From dae075816eda7a72184234654f743454c31ab40d Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 13 May 2022 17:38:48 +0200 Subject: [PATCH] fix coding for ack and ftt --- shared/src/ledger/ibc/handler.rs | 81 ++++++++++++++++++++----------- shared/src/ledger/ibc/vp/mod.rs | 8 +-- shared/src/ledger/ibc/vp/token.rs | 19 +++----- shared/src/types/ibc/data.rs | 63 ++++++++++++++++++------ shared/src/types/token.rs | 7 +-- tests/src/vm_host_env/ibc.rs | 6 +-- 6 files changed, 117 insertions(+), 67 deletions(-) diff --git a/shared/src/ledger/ibc/handler.rs b/shared/src/ledger/ibc/handler.rs index 9e66630cce1..21f06ff68ee 100644 --- a/shared/src/ledger/ibc/handler.rs +++ b/shared/src/ledger/ibc/handler.rs @@ -605,10 +605,17 @@ pub trait IbcActions { /// Receive a packet fn receive_packet(&self, msg: &MsgRecvPacket) -> Result<()> { + // TODO for other applications // check the packet data - if let Ok(data) = serde_json::from_slice(&msg.packet.data) { - self.receive_token(&msg.packet, &data)?; - } + let packet_ack = + if let Ok(data) = serde_json::from_slice(&msg.packet.data) { + match self.receive_token(&msg.packet, &data) { + Ok(_) => PacketAck::result_success(), + Err(e) => PacketAck::result_error(e.to_string()), + } + } else { + PacketAck::result_error("unknown packet data".to_string()) + }; // store the receipt let receipt_key = storage::receipt_key( @@ -624,7 +631,7 @@ pub trait IbcActions { &msg.packet.destination_channel, msg.packet.sequence, ); - let ack = PacketAck::default().encode_to_vec(); + let ack = packet_ack.encode_to_vec(); let ack_commitment = sha2::Sha256::digest(&ack).to_vec(); self.write_ibc_data(&ack_key, ack_commitment); @@ -646,6 +653,15 @@ pub trait IbcActions { /// Receive a acknowledgement fn acknowledge_packet(&self, msg: &MsgAcknowledgement) -> Result<()> { + let ack = PacketAck::try_from(msg.acknowledgement.clone()) + .map_err(Error::IbcData)?; + if !ack.is_success() { + // TODO for other applications + if let Ok(data) = serde_json::from_slice(&msg.packet.data) { + self.refund_token(&msg.packet, &data)?; + } + } + let commitment_key = storage::commitment_key( &msg.packet.source_port, &msg.packet.source_channel, @@ -653,6 +669,14 @@ pub trait IbcActions { ); self.delete_ibc_data(&commitment_key); + // get and increment the next sequence ack + let port_channel_id = port_channel_id( + msg.packet.source_port.clone(), + msg.packet.source_channel, + ); + let seq_key = storage::next_sequence_ack_key(&port_channel_id); + self.get_and_inc_sequence(&seq_key)?; + let event = make_ack_event(msg.packet.clone()).try_into().unwrap(); self.emit_ibc_event(event); @@ -661,6 +685,7 @@ pub trait IbcActions { /// Receive a timeout fn timeout_packet(&self, msg: &MsgTimeout) -> Result<()> { + // TODO for other applications // check the packet data if let Ok(data) = serde_json::from_slice(&msg.packet.data) { self.refund_token(&msg.packet, &data)?; @@ -704,6 +729,7 @@ pub trait IbcActions { /// Receive a timeout for TimeoutOnClose fn timeout_on_close_packet(&self, msg: &MsgTimeoutOnClose) -> Result<()> { + // TODO for other applications // check the packet data if let Ok(data) = serde_json::from_slice(&msg.packet.data) { self.refund_token(&msg.packet, &data)?; @@ -824,13 +850,12 @@ pub trait IbcActions { data.sender, e )) })?; - let token_str = - data.denomination.split('/').last().ok_or_else(|| { - Error::SendingToken(format!( - "No token was specified: {}", - data.denomination - )) - })?; + let token_str = data.denom.split('/').last().ok_or_else(|| { + Error::SendingToken(format!( + "No token was specified: {}", + data.denom + )) + })?; let token = Address::decode(token_str).map_err(|e| { Error::SendingToken(format!( "Invalid token address: token {}, error {}", @@ -844,13 +869,13 @@ pub trait IbcActions { )) })?; - // check the denomination field + // check the denom field let prefix = format!( "{}/{}/", msg.source_port.clone(), msg.source_channel.clone() ); - if data.denomination.starts_with(&prefix) { + if data.denom.starts_with(&prefix) { // sink zone let burn = Address::Internal(InternalAddress::IbcBurn); self.transfer_token(&source, &burn, &token, amount); @@ -889,13 +914,12 @@ pub trait IbcActions { data.receiver, e )) })?; - let token_str = - data.denomination.split('/').last().ok_or_else(|| { - Error::ReceivingToken(format!( - "No token was specified: {}", - data.denomination - )) - })?; + let token_str = data.denom.split('/').last().ok_or_else(|| { + Error::ReceivingToken(format!( + "No token was specified: {}", + data.denom + )) + })?; let token = Address::decode(token_str).map_err(|e| { Error::ReceivingToken(format!( "Invalid token address: token {}, error {}", @@ -914,7 +938,7 @@ pub trait IbcActions { packet.source_port.clone(), packet.source_channel.clone() ); - if data.denomination.starts_with(&prefix) { + if data.denom.starts_with(&prefix) { // unescrow the token because this chain is the source let escrow = Address::Internal(InternalAddress::ibc_escrow_address( @@ -942,13 +966,12 @@ pub trait IbcActions { data.sender, e )) })?; - let token_str = - data.denomination.split('/').last().ok_or_else(|| { - Error::ReceivingToken(format!( - "No token was specified: {}", - data.denomination - )) - })?; + let token_str = data.denom.split('/').last().ok_or_else(|| { + Error::ReceivingToken(format!( + "No token was specified: {}", + data.denom + )) + })?; let token = Address::decode(token_str).map_err(|e| { Error::ReceivingToken(format!( "Invalid token address: token {}, error {}", @@ -967,7 +990,7 @@ pub trait IbcActions { packet.source_port.clone(), packet.source_channel.clone() ); - if data.denomination.starts_with(&prefix) { + if data.denom.starts_with(&prefix) { // mint the token because the sender chain is the sink zone let mint = Address::Internal(InternalAddress::IbcMint); self.transfer_token(&mint, &dest, &token, amount); diff --git a/shared/src/ledger/ibc/vp/mod.rs b/shared/src/ledger/ibc/vp/mod.rs index 7e2baf46663..bb48af3c5fb 100644 --- a/shared/src/ledger/ibc/vp/mod.rs +++ b/shared/src/ledger/ibc/vp/mod.rs @@ -1446,7 +1446,7 @@ mod tests { .write(&key, PacketReceipt::default().as_bytes().to_vec()) .expect("write failed"); let key = ack_key(&get_port_id(), &get_channel_id(), sequence); - let ack = PacketAck::default().encode_to_vec(); + let ack = PacketAck::result_success().encode_to_vec(); write_log.write(&key, ack).expect("write failed"); let tx_code = vec![]; @@ -1516,7 +1516,7 @@ mod tests { write_log.commit_block(&mut storage).expect("commit failed"); // prepare data - let ack = PacketAck::default().encode_to_vec(); + let ack = PacketAck::result_success().encode_to_vec(); let proof_packet = CommitmentProofBytes::try_from(vec![0]).unwrap(); let proofs = Proofs::new(proof_packet, None, None, None, Height::new(0, 1)) @@ -1689,7 +1689,7 @@ mod tests { &msg.packet.destination_channel, msg.packet.sequence, ); - let ack = PacketAck::default().encode_to_vec(); + let ack = PacketAck::result_success().encode_to_vec(); write_log.write(&ack_key, ack).expect("write failed"); write_log.commit_tx(); @@ -1730,7 +1730,7 @@ mod tests { .expect("write failed"); let ack_key = ack_key(&get_port_id(), &get_channel_id(), Sequence::from(1)); - let ack = PacketAck::default().encode_to_vec(); + let ack = PacketAck::result_success().encode_to_vec(); write_log.write(&ack_key, ack).expect("write failed"); write_log.commit_tx(); diff --git a/shared/src/ledger/ibc/vp/token.rs b/shared/src/ledger/ibc/vp/token.rs index f56382b3604..6de63e24eb4 100644 --- a/shared/src/ledger/ibc/vp/token.rs +++ b/shared/src/ledger/ibc/vp/token.rs @@ -121,18 +121,17 @@ where { fn validate_sending_token(&self, msg: &MsgTransfer) -> Result { let data = FungibleTokenPacketData::from(msg.clone()); - let token_str = - data.denomination.split('/').last().ok_or(Error::NoToken)?; + let token_str = data.denom.split('/').last().ok_or(Error::NoToken)?; let token = Address::decode(token_str).map_err(Error::Address)?; let amount = Amount::from_str(&data.amount).map_err(Error::Amount)?; - // check the denomination field + // check the denom field let prefix = format!( "{}/{}/", msg.source_port.clone(), msg.source_channel.clone() ); - let change = if data.denomination.starts_with(&prefix) { + let change = if data.denom.starts_with(&prefix) { // sink zone let target = Address::Internal(InternalAddress::IbcBurn); let target_key = token::balance_key(&token, &target); @@ -171,8 +170,7 @@ where let data: FungibleTokenPacketData = serde_json::from_slice(&packet.data) .map_err(Error::DecodingPacketData)?; - let token_str = - data.denomination.split('/').last().ok_or(Error::NoToken)?; + let token_str = data.denom.split('/').last().ok_or(Error::NoToken)?; let token = Address::decode(token_str).map_err(Error::Address)?; let amount = Amount::from_str(&data.amount).map_err(Error::Amount)?; @@ -181,7 +179,7 @@ where packet.source_port.clone(), packet.source_channel.clone() ); - let change = if data.denomination.starts_with(&prefix) { + let change = if data.denom.starts_with(&prefix) { // this chain is the source let source = Address::Internal(InternalAddress::ibc_escrow_address( @@ -220,18 +218,17 @@ where let data: FungibleTokenPacketData = serde_json::from_slice(&packet.data) .map_err(Error::DecodingPacketData)?; - let token_str = - data.denomination.split('/').last().ok_or(Error::NoToken)?; + let token_str = data.denom.split('/').last().ok_or(Error::NoToken)?; let token = Address::decode(token_str).map_err(Error::Address)?; let amount = Amount::from_str(&data.amount).map_err(Error::Amount)?; - // check the denomination field + // check the denom field let prefix = format!( "{}/{}/", packet.source_port.clone(), packet.source_channel.clone() ); - let change = if data.denomination.starts_with(&prefix) { + let change = if data.denom.starts_with(&prefix) { // sink zone: mint the token for the refund let source = Address::Internal(InternalAddress::IbcMint); let source_key = token::balance_key(&token, &source); diff --git a/shared/src/types/ibc/data.rs b/shared/src/types/ibc/data.rs index 4c5d5f2469c..947142ab97a 100644 --- a/shared/src/types/ibc/data.rs +++ b/shared/src/types/ibc/data.rs @@ -16,7 +16,9 @@ use crate::ibc::core::ics03_connection::msgs::conn_open_confirm::MsgConnectionOp use crate::ibc::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; use crate::ibc::core::ics03_connection::msgs::conn_open_try::MsgConnectionOpenTry; use crate::ibc::core::ics03_connection::msgs::ConnectionMsg; -use crate::ibc::core::ics04_channel::msgs::acknowledgement::MsgAcknowledgement; +use crate::ibc::core::ics04_channel::msgs::acknowledgement::{ + Acknowledgement, MsgAcknowledgement, +}; use crate::ibc::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; use crate::ibc::core::ics04_channel::msgs::chan_close_init::MsgChannelCloseInit; use crate::ibc::core::ics04_channel::msgs::chan_open_ack::MsgChannelOpenAck; @@ -32,14 +34,14 @@ use crate::ibc::core::ics26_routing::error::Error as Ics26Error; use crate::ibc::core::ics26_routing::msgs::Ics26Envelope; use crate::ibc::downcast; use crate::ibc_proto::google::protobuf::Any; -use crate::ibc_proto::ibc::core::channel::v1::acknowledgement::Response; -use crate::ibc_proto::ibc::core::channel::v1::Acknowledgement; #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { #[error("Decoding IBC data error: {0}")] DecodingData(prost::DecodeError), + #[error("Decoding Json data error: {0}")] + DecodingJsonData(serde_json::Error), #[error("Decoding message error: {0}")] DecodingMessage(Ics26Error), #[error("Downcast error: {0}")] @@ -326,39 +328,70 @@ impl Default for PacketReceipt { } /// Acknowledgement for a packet -#[derive(Clone, Debug)] -pub struct PacketAck(pub Acknowledgement); +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum PacketAck { + /// Success Acknowledgement + Result(String), + /// Error Acknowledgement + Error(String), +} + +/// Success acknowledgement +const ACK_SUCCESS_B64: &str = "AQ=="; +/// Error acknowledgement +const ACK_ERR_STR: &str = + "error handling packet on destination chain: see events for details"; // TODO temporary type. add a new type for ack to ibc-rs impl PacketAck { + /// Success acknowledgement + pub fn result_success() -> Self { + Self::Result(ACK_SUCCESS_B64.to_string()) + } + + /// Acknowledgement with an error + pub fn result_error(err: String) -> Self { + Self::Error(format!("{}: {}", ACK_ERR_STR, err)) + } + + /// Check if the ack is for success + pub fn is_success(&self) -> bool { + match self { + Self::Result(_) => true, + Self::Error(_) => false, + } + } + /// Encode the ack pub fn encode_to_vec(&self) -> Vec { - serde_json::to_vec(&self.0) + serde_json::to_vec(&self) .expect("Encoding acknowledgement shouldn't fail") } } -impl Default for PacketAck { - fn default() -> Self { - Self(Acknowledgement { - response: Some(Response::Result(vec![1_u8])), - }) +impl TryFrom for PacketAck { + type Error = Error; + + fn try_from(ack: Acknowledgement) -> Result { + serde_json::from_slice(&ack.into_bytes()) + .map_err(Error::DecodingJsonData) } } // for the string to be used by the current reader impl Display for PacketAck { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", serde_json::to_string(&self.0).unwrap()) + write!(f, "{}", serde_json::to_string(&self).unwrap()) } } -// TODO temporary type. add a new type for ack to ibc-rs +// TODO temporary type. add a new type for packet data to ibc-rs /// Data to transfer a token #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct FungibleTokenPacketData { /// the token denomination to be transferred - pub denomination: String, + pub denom: String, /// the token amount to be transferred pub amount: String, /// the sender address @@ -372,7 +405,7 @@ impl From for FungibleTokenPacketData { // TODO validation let token = msg.token.unwrap(); Self { - denomination: token.denom, + denom: token.denom, amount: token.amount, sender: msg.sender.to_string(), receiver: msg.receiver.to_string(), diff --git a/shared/src/types/token.rs b/shared/src/types/token.rs index f3e4cd4ed22..0132abea366 100644 --- a/shared/src/types/token.rs +++ b/shared/src/types/token.rs @@ -341,11 +341,8 @@ impl TryFrom for Transfer { Address::decode(&data.sender).map_err(TransferError::Address)?; let target = Address::decode(&data.receiver).map_err(TransferError::Address)?; - let token_str = data - .denomination - .split('/') - .last() - .ok_or(TransferError::NoToken)?; + let token_str = + data.denom.split('/').last().ok_or(TransferError::NoToken)?; let token = Address::decode(token_str).map_err(TransferError::Address)?; let amount = diff --git a/tests/src/vm_host_env/ibc.rs b/tests/src/vm_host_env/ibc.rs index 8988c482926..bed2d10efdf 100644 --- a/tests/src/vm_host_env/ibc.rs +++ b/tests/src/vm_host_env/ibc.rs @@ -63,7 +63,7 @@ use anoma::ledger::storage::Sha256Hasher; use anoma::proto::Tx; use anoma::tendermint_proto::Protobuf; use anoma::types::address::{self, Address, InternalAddress}; -use anoma::types::ibc::data::FungibleTokenPacketData; +use anoma::types::ibc::data::{FungibleTokenPacketData, PacketAck}; use anoma::types::ibc::IbcEvent; use anoma::types::storage::{BlockHash, BlockHeight, Key}; use anoma::types::time::Rfc3339String; @@ -610,7 +610,7 @@ pub fn msg_packet_recv(packet: Packet) -> MsgRecvPacket { pub fn msg_packet_ack(packet: Packet) -> MsgAcknowledgement { MsgAcknowledgement { packet, - acknowledgement: vec![0].into(), + acknowledgement: PacketAck::result_success().encode_to_vec().into(), proofs: dummy_proofs(), signer: Signer::new("test"), } @@ -627,7 +627,7 @@ pub fn received_packet( let timeout_timestamp = (Timestamp::now() + Duration::from_secs(100)).unwrap(); let data = FungibleTokenPacketData { - denomination: token, + denom: token, amount: 100u64.to_string(), sender: address::testing::gen_established_address().to_string(), receiver: receiver.to_string(),