diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 4e8fa5f6af..80a54d00fb 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -253,6 +253,10 @@ pub enum TxCmd { /// The selected fee tier to multiply the fee amount by. #[clap(short, long, default_value_t)] fee_tier: FeeTier, + /// Whether to use a Bech32(non-m) address for the withdrawal. + /// Required for some chains for a successful acknowledgement. + #[clap(long)] + use_compat_address: bool, }, /// Broadcast a saved transaction to the network #[clap(display_order = 1000)] @@ -974,6 +978,7 @@ impl TxCmd { channel, source, fee_tier, + use_compat_address, } => { let destination_chain_address = to; @@ -1087,6 +1092,7 @@ impl TxCmd { return_address: ephemeral_return_address, // TODO: impl From for ChannelId source_channel: ChannelId::from_str(format!("channel-{}", channel).as_ref())?, + use_compat_address: *use_compat_address, }; let plan = Planner::new(OsRng) diff --git a/crates/core/component/ibc/src/component/app_handler.rs b/crates/core/component/ibc/src/component/app_handler.rs index 40db8959b5..453ab37a72 100644 --- a/crates/core/component/ibc/src/component/app_handler.rs +++ b/crates/core/component/ibc/src/component/app_handler.rs @@ -55,7 +55,10 @@ pub trait AppHandlerExecute: Send + Sync { async fn recv_packet_execute(state: S, msg: &MsgRecvPacket) -> Result<()>; async fn timeout_packet_execute(state: S, msg: &MsgTimeout) -> Result<()>; - async fn acknowledge_packet_execute(state: S, msg: &MsgAcknowledgement); + async fn acknowledge_packet_execute( + state: S, + msg: &MsgAcknowledgement, + ) -> Result<()>; } pub trait AppHandler: AppHandlerCheck + AppHandlerExecute {} diff --git a/crates/core/component/ibc/src/component/client.rs b/crates/core/component/ibc/src/component/client.rs index 4e22db8565..b6f6ffd327 100644 --- a/crates/core/component/ibc/src/component/client.rs +++ b/crates/core/component/ibc/src/component/client.rs @@ -577,7 +577,12 @@ mod tests { async fn timeout_packet_execute(_state: S, _msg: &MsgTimeout) -> Result<()> { Ok(()) } - async fn acknowledge_packet_execute(_state: S, _msg: &MsgAcknowledgement) {} + async fn acknowledge_packet_execute( + _state: S, + _msg: &MsgAcknowledgement, + ) -> Result<()> { + Ok(()) + } } #[async_trait] diff --git a/crates/core/component/ibc/src/component/msg_handler/acknowledgement.rs b/crates/core/component/ibc/src/component/msg_handler/acknowledgement.rs index f43c25f6ec..e6298aa080 100644 --- a/crates/core/component/ibc/src/component/msg_handler/acknowledgement.rs +++ b/crates/core/component/ibc/src/component/msg_handler/acknowledgement.rs @@ -119,7 +119,7 @@ impl MsgHandler for MsgAcknowledgement { let transfer = PortId::transfer(); if self.packet.port_on_b == transfer { - AH::acknowledge_packet_execute(state, self).await; + AH::acknowledge_packet_execute(state, self).await?; } else { anyhow::bail!("invalid port id"); } diff --git a/crates/core/component/shielded-pool/Cargo.toml b/crates/core/component/shielded-pool/Cargo.toml index 79462e77f1..dfe00fea5e 100644 --- a/crates/core/component/shielded-pool/Cargo.toml +++ b/crates/core/component/shielded-pool/Cargo.toml @@ -51,7 +51,7 @@ decaf377-rdsa = {workspace = true} futures = {workspace = true} hex = {workspace = true} ibc-proto = {workspace = true, default-features = false} -ibc-types = {workspace = true, default-features = false} +ibc-types = {workspace = true, features = ["serde_with"], default-features = false} im = {workspace = true} metrics = {workspace = true} once_cell = {workspace = true} diff --git a/crates/core/component/shielded-pool/src/component/transfer.rs b/crates/core/component/shielded-pool/src/component/transfer.rs index 9b6a594532..d7f7e4c553 100644 --- a/crates/core/component/shielded-pool/src/component/transfer.rs +++ b/crates/core/component/shielded-pool/src/component/transfer.rs @@ -7,6 +7,7 @@ use crate::{ use anyhow::{Context, Result}; use async_trait::async_trait; use cnidarium::{StateRead, StateWrite}; +use ibc_types::core::channel::Packet; use ibc_types::{ core::channel::{ channel::Order as ChannelOrder, @@ -406,8 +407,8 @@ async fn recv_transfer_packet_inner( } // see: https://github.com/cosmos/ibc/blob/8326e26e7e1188b95c32481ff00348a705b23700/spec/app/ics-020-fungible-token-transfer/README.md?plain=1#L297 -async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> Result<()> { - let packet_data: FungibleTokenPacketData = serde_json::from_slice(msg.packet.data.as_slice())?; +async fn timeout_packet_inner(mut state: S, packet: &Packet) -> Result<()> { + let packet_data: FungibleTokenPacketData = serde_json::from_slice(packet.data.as_slice())?; let denom: asset::Metadata = packet_data // CRITICAL: verify that this denom is validated in upstream timeout handling .denom .as_str() @@ -430,11 +431,11 @@ async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> asset_id: denom.id(), }; - if is_source(&msg.packet.port_on_a, &msg.packet.chan_on_a, &denom, true) { + if is_source(&packet.port_on_a, &packet.chan_on_a, &denom, true) { // sender was source chain, unescrow tokens back to sender let value_balance: Amount = state .get(&state_key::ics20_value_balance::by_asset_id( - &msg.packet.chan_on_a, + &packet.chan_on_a, &denom.id(), )) .await? @@ -449,8 +450,8 @@ async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> value, &receiver, CommitmentSource::Ics20Transfer { - packet_seq: msg.packet.sequence.0, - channel_id: msg.packet.chan_on_a.0.clone(), + packet_seq: packet.sequence.0, + channel_id: packet.chan_on_a.0.clone(), sender: packet_data.sender.clone(), }, ) @@ -460,7 +461,7 @@ async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> // update the value balance let value_balance: Amount = state .get(&state_key::ics20_value_balance::by_asset_id( - &msg.packet.chan_on_a, + &packet.chan_on_a, &denom.id(), )) .await? @@ -471,13 +472,13 @@ async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> .checked_sub(&amount) .context("underflow in ics20 timeout packet value balance subtraction")?; state.put( - state_key::ics20_value_balance::by_asset_id(&msg.packet.chan_on_a, &denom.id()), + state_key::ics20_value_balance::by_asset_id(&packet.chan_on_a, &denom.id()), new_value_balance, ); } else { let value_balance: Amount = state .get(&state_key::ics20_value_balance::by_asset_id( - &msg.packet.chan_on_a, + &packet.chan_on_a, &denom.id(), )) .await? @@ -489,8 +490,8 @@ async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> &receiver, // NOTE: should this be Ics20TransferTimeout? CommitmentSource::Ics20Transfer { - packet_seq: msg.packet.sequence.0, - channel_id: msg.packet.chan_on_a.0.clone(), + packet_seq: packet.sequence.0, + channel_id: packet.chan_on_a.0.clone(), sender: packet_data.sender.clone(), }, ) @@ -499,7 +500,7 @@ async fn timeout_packet_inner(mut state: S, msg: &MsgTimeout) -> let new_value_balance = value_balance.saturating_add(&value.amount); state.put( - state_key::ics20_value_balance::by_asset_id(&msg.packet.chan_on_a, &denom.id()), + state_key::ics20_value_balance::by_asset_id(&packet.chan_on_a, &denom.id()), new_value_balance, ); } @@ -540,14 +541,30 @@ impl AppHandlerExecute for Ics20Transfer { async fn timeout_packet_execute(mut state: S, msg: &MsgTimeout) -> Result<()> { // timeouts may fail due to counterparty chains sending transfers of u128-1 - timeout_packet_inner(&mut state, msg) + timeout_packet_inner(&mut state, &msg.packet) .await .context("able to timeout packet")?; Ok(()) } - async fn acknowledge_packet_execute(_state: S, _msg: &MsgAcknowledgement) {} + async fn acknowledge_packet_execute( + mut state: S, + msg: &MsgAcknowledgement, + ) -> Result<()> { + let ack: TokenTransferAcknowledgement = + serde_json::from_slice(msg.acknowledgement.as_slice())?; + if !ack.is_successful() { + // in the case where a counterparty chain acknowledges a packet with an error, + // for example due to a middleware processing issue or other behavior, + // the funds should be unescrowed back to the packet sender. + timeout_packet_inner(&mut state, &msg.packet) + .await + .context("unable to refund packet acknowledgement")?; + } + + Ok(()) + } } impl AppHandler for Ics20Transfer {} diff --git a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs index 60256c6a74..c452d5d703 100644 --- a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs +++ b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs @@ -37,6 +37,10 @@ pub struct Ics20Withdrawal { pub timeout_time: u64, // the source channel used for the withdrawal pub source_channel: ChannelId, + + // Whether to use a "compat" (bech32, non-m) address for the return address in the withdrawal, + // for compatability with chains that expect to be able to parse the return address as bech32. + pub use_compat_address: bool, } #[cfg(feature = "component")] @@ -113,6 +117,7 @@ impl From for pb::Ics20Withdrawal { timeout_height: Some(w.timeout_height.into()), timeout_time: w.timeout_time, source_channel: w.source_channel.to_string(), + use_compat_address: w.use_compat_address, } } } @@ -142,17 +147,23 @@ impl TryFrom for Ics20Withdrawal { .try_into()?, timeout_time: s.timeout_time, source_channel: ChannelId::from_str(&s.source_channel)?, + use_compat_address: s.use_compat_address, }) } } impl From for pb::FungibleTokenPacketData { fn from(w: Ics20Withdrawal) -> Self { + let return_address = match w.use_compat_address { + true => w.return_address.compat_encoding(), + false => w.return_address.to_string(), + }; + pb::FungibleTokenPacketData { amount: w.value().amount.to_string(), denom: w.denom.to_string(), receiver: w.destination_chain_address, - sender: w.return_address.to_string(), + sender: return_address, memo: "".to_string(), } } diff --git a/crates/proto/src/gen/penumbra.core.component.ibc.v1.rs b/crates/proto/src/gen/penumbra.core.component.ibc.v1.rs index df993e3cf0..320e259a47 100644 --- a/crates/proto/src/gen/penumbra.core.component.ibc.v1.rs +++ b/crates/proto/src/gen/penumbra.core.component.ibc.v1.rs @@ -69,6 +69,10 @@ pub struct Ics20Withdrawal { /// The source channel used for the withdrawal #[prost(string, tag = "7")] pub source_channel: ::prost::alloc::string::String, + /// Whether to use a "compat" (bech32, non-m) address for the return address in the withdrawal, + /// for compatability with chains that expect to be able to parse the return address as bech32. + #[prost(bool, tag = "8")] + pub use_compat_address: bool, } impl ::prost::Name for Ics20Withdrawal { const NAME: &'static str = "Ics20Withdrawal"; diff --git a/crates/proto/src/gen/penumbra.core.component.ibc.v1.serde.rs b/crates/proto/src/gen/penumbra.core.component.ibc.v1.serde.rs index a12442b052..b4bcbf3624 100644 --- a/crates/proto/src/gen/penumbra.core.component.ibc.v1.serde.rs +++ b/crates/proto/src/gen/penumbra.core.component.ibc.v1.serde.rs @@ -1054,6 +1054,9 @@ impl serde::Serialize for Ics20Withdrawal { if !self.source_channel.is_empty() { len += 1; } + if self.use_compat_address { + len += 1; + } let mut struct_ser = serializer.serialize_struct("penumbra.core.component.ibc.v1.Ics20Withdrawal", len)?; if let Some(v) = self.amount.as_ref() { struct_ser.serialize_field("amount", v)?; @@ -1077,6 +1080,9 @@ impl serde::Serialize for Ics20Withdrawal { if !self.source_channel.is_empty() { struct_ser.serialize_field("sourceChannel", &self.source_channel)?; } + if self.use_compat_address { + struct_ser.serialize_field("useCompatAddress", &self.use_compat_address)?; + } struct_ser.end() } } @@ -1099,6 +1105,8 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { "timeoutTime", "source_channel", "sourceChannel", + "use_compat_address", + "useCompatAddress", ]; #[allow(clippy::enum_variant_names)] @@ -1110,6 +1118,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { TimeoutHeight, TimeoutTime, SourceChannel, + UseCompatAddress, __SkipField__, } impl<'de> serde::Deserialize<'de> for GeneratedField { @@ -1139,6 +1148,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { "timeoutHeight" | "timeout_height" => Ok(GeneratedField::TimeoutHeight), "timeoutTime" | "timeout_time" => Ok(GeneratedField::TimeoutTime), "sourceChannel" | "source_channel" => Ok(GeneratedField::SourceChannel), + "useCompatAddress" | "use_compat_address" => Ok(GeneratedField::UseCompatAddress), _ => Ok(GeneratedField::__SkipField__), } } @@ -1165,6 +1175,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { let mut timeout_height__ = None; let mut timeout_time__ = None; let mut source_channel__ = None; + let mut use_compat_address__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::Amount => { @@ -1211,6 +1222,12 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { } source_channel__ = Some(map_.next_value()?); } + GeneratedField::UseCompatAddress => { + if use_compat_address__.is_some() { + return Err(serde::de::Error::duplicate_field("useCompatAddress")); + } + use_compat_address__ = Some(map_.next_value()?); + } GeneratedField::__SkipField__ => { let _ = map_.next_value::()?; } @@ -1224,6 +1241,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { timeout_height: timeout_height__, timeout_time: timeout_time__.unwrap_or_default(), source_channel: source_channel__.unwrap_or_default(), + use_compat_address: use_compat_address__.unwrap_or_default(), }) } } diff --git a/crates/proto/src/gen/proto_descriptor.bin.no_lfs b/crates/proto/src/gen/proto_descriptor.bin.no_lfs index 0a63961d63..294b190c53 100644 Binary files a/crates/proto/src/gen/proto_descriptor.bin.no_lfs and b/crates/proto/src/gen/proto_descriptor.bin.no_lfs differ diff --git a/proto/penumbra/penumbra/core/component/ibc/v1/ibc.proto b/proto/penumbra/penumbra/core/component/ibc/v1/ibc.proto index 2506eba219..733ca5a3af 100644 --- a/proto/penumbra/penumbra/core/component/ibc/v1/ibc.proto +++ b/proto/penumbra/penumbra/core/component/ibc/v1/ibc.proto @@ -50,6 +50,10 @@ message Ics20Withdrawal { // The source channel used for the withdrawal string source_channel = 7; + + // Whether to use a "compat" (bech32, non-m) address for the return address in the withdrawal, + // for compatability with chains that expect to be able to parse the return address as bech32. + bool use_compat_address = 8; } message ClientData {