fix penumbra-specific timestamp reads in ibc module (penumbra-zone#4822)

## Describe your changes fix penumbra-specific timestamp reads in ibc module by using `HostInterface` instead. - `Ics20WithdrawalWithHandler` was created analogously to `IbcRelayWithHandler`, which attaches a `HostInterface` to be used in `check_and_execute` to get the current block timestamp - the rpc method `client_state` was also updated to use `HostInterface` ## Issue ticket number and link closes penumbra-zone#4812 ## Checklist before requesting a review - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > the timestamp reads are unchanged for penumbra, as the `PenumbraHost` is used, which calls the same methods as before --------- Co-authored-by: turbocrime <[email protected]>
astriaorg · Aug 21, 2024 · 87adc8d · 87adc8d
1 parent 0c5afd9
commit 87adc8d
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 36 deletions.
diff --git a/crates/core/app/src/action_handler/actions.rs b/crates/core/app/src/action_handler/actions.rs
@@ -43,7 +43,13 @@ impl AppActionHandler for Action {
                     .check_stateless(())
                     .await
             }
-            Action::Ics20Withdrawal(action) => action.check_stateless(()).await,
+            Action::Ics20Withdrawal(action) => {
+                action
+                    .clone()
+                    .with_handler::<PenumbraHost>()
+                    .check_stateless(())
+                    .await
+            }
             Action::CommunityPoolSpend(action) => action.check_stateless(()).await,
             Action::CommunityPoolOutput(action) => action.check_stateless(()).await,
             Action::CommunityPoolDeposit(action) => action.check_stateless(()).await,
@@ -78,7 +84,13 @@ impl AppActionHandler for Action {
                     .check_historical(state)
                     .await
             }
-            Action::Ics20Withdrawal(action) => action.check_historical(state).await,
+            Action::Ics20Withdrawal(action) => {
+                action
+                    .clone()
+                    .with_handler::<PenumbraHost>()
+                    .check_historical(state)
+                    .await
+            }
             Action::CommunityPoolSpend(action) => action.check_historical(state).await,
             Action::CommunityPoolOutput(action) => action.check_historical(state).await,
             Action::CommunityPoolDeposit(action) => action.check_historical(state).await,
@@ -113,7 +125,13 @@ impl AppActionHandler for Action {
                     .check_and_execute(state)
                     .await
             }
-            Action::Ics20Withdrawal(action) => action.check_and_execute(state).await,
+            Action::Ics20Withdrawal(action) => {
+                action
+                    .clone()
+                    .with_handler::<PenumbraHost>()
+                    .check_and_execute(state)
+                    .await
+            }
             Action::CommunityPoolSpend(action) => action.check_and_execute(state).await,
             Action::CommunityPoolOutput(action) => action.check_and_execute(state).await,
             Action::CommunityPoolDeposit(action) => action.check_and_execute(state).await,

diff --git a/crates/core/app/src/penumbra_host_chain.rs b/crates/core/app/src/penumbra_host_chain.rs
@@ -4,7 +4,7 @@ use penumbra_sct::component::clock::EpochRead;
 
 use crate::app::StateReadExt;
 
-/// The implementation of [`penumbr_ibc::component::HostInterface`] for Penumbra.
+/// The implementation of [`penumbra_ibc::component::HostInterface`] for Penumbra.
 /// It encapsulates all of the chain-specific data that the ibc implementation needs.
 #[derive(Clone)]
 pub struct PenumbraHost {}

diff --git a/crates/core/component/ibc/src/component/client.rs b/crates/core/component/ibc/src/component/client.rs
@@ -240,7 +240,7 @@ pub trait StateWriteExt: StateWrite + StateReadExt {
 impl<T: StateWrite + ?Sized> StateWriteExt for T {}
 
 #[async_trait]
-pub trait StateReadExt: StateRead + penumbra_sct::component::clock::EpochRead {
+pub trait StateReadExt: StateRead {
     async fn client_counter(&self) -> Result<ClientCounter> {
         self.get("ibc_client_counter")
             .await
@@ -266,7 +266,11 @@ pub trait StateReadExt: StateRead + penumbra_sct::component::clock::EpochRead {
         client_state.context(format!("could not find client state for {client_id}"))
     }
 
-    async fn get_client_status(&self, client_id: &ClientId) -> ClientStatus {
+    async fn get_client_status(
+        &self,
+        client_id: &ClientId,
+        current_block_time: tendermint::Time,
+    ) -> ClientStatus {
         let client_type = self.get_client_type(client_id).await;
 
         if client_type.is_err() {
@@ -303,13 +307,6 @@ pub trait StateReadExt: StateRead + penumbra_sct::component::clock::EpochRead {
 
         let latest_consensus_state = latest_consensus_state.expect("latest consensus state is Ok");
 
-        let current_block_time = self.get_current_block_timestamp().await;
-
-        if current_block_time.is_err() {
-            return ClientStatus::Unknown;
-        }
-
-        let current_block_time = current_block_time.expect("current block time is Ok");
         let time_elapsed = current_block_time.duration_since(latest_consensus_state.timestamp);
         if time_elapsed.is_err() {
             return ClientStatus::Unknown;

diff --git a/crates/core/component/ibc/src/component/packet.rs b/crates/core/component/ibc/src/component/packet.rs
@@ -5,7 +5,7 @@ use ibc_types::core::{
     channel::{channel::State as ChannelState, events, ChannelId, Packet, PortId},
     client::Height,
 };
-use penumbra_sct::component::clock::EpochRead;
+use tendermint::Time;
 
 use crate::component::{
     channel::{StateReadExt as _, StateWriteExt as _},
@@ -96,7 +96,11 @@ impl<S: CheckStatus> IBCPacket<S> {
 #[async_trait]
 pub trait SendPacketRead: StateRead {
     /// send_packet_check verifies that a packet can be sent using the provided parameters.
-    async fn send_packet_check(&self, packet: IBCPacket<Unchecked>) -> Result<IBCPacket<Checked>> {
+    async fn send_packet_check(
+        &self,
+        packet: IBCPacket<Unchecked>,
+        current_block_time: Time,
+    ) -> Result<IBCPacket<Checked>> {
         let channel = self
             .get_channel(&packet.source_channel, &packet.source_port)
             .await?
@@ -134,7 +138,6 @@ pub trait SendPacketRead: StateRead {
             .get_verified_consensus_state(&client_state.latest_height(), &connection.client_id)
             .await?;
 
-        let current_block_time = self.get_current_block_timestamp().await?;
         let time_elapsed = current_block_time.duration_since(latest_consensus_state.timestamp)?;
 
         if client_state.expired(time_elapsed) {

diff --git a/crates/core/component/ibc/src/component/rpc/client_query.rs b/crates/core/component/ibc/src/component/rpc/client_query.rs
@@ -228,8 +228,10 @@ impl<HI: HostInterface + Send + Sync + 'static> ClientQuery for IbcQuery<HI> {
         let snapshot = self.storage.latest_snapshot();
         let client_id = ClientId::from_str(&request.get_ref().client_id)
             .map_err(|e| tonic::Status::invalid_argument(format!("invalid client id: {e}")))?;
-
-        let client_status = snapshot.get_client_status(&client_id).await;
+        let timestamp = HI::get_block_timestamp(snapshot.clone())
+            .await
+            .map_err(|e| tonic::Status::aborted(format!("couldn't get block timestamp: {e}")))?;
+        let client_status = snapshot.get_client_status(&client_id, timestamp).await;
         let resp = QueryClientStatusResponse {
             status: client_status.to_string(),
         };

diff --git a/crates/core/component/shielded-pool/src/component.rs b/crates/core/component/shielded-pool/src/component.rs
@@ -3,6 +3,7 @@
 mod action_handler;
 mod assets;
 mod fmd;
+mod ics20_withdrawal_with_handler;
 mod metrics;
 mod note_manager;
 mod shielded_pool;
@@ -11,6 +12,7 @@ mod transfer;
 pub use self::metrics::register_metrics;
 pub use assets::{AssetRegistry, AssetRegistryRead};
 pub use fmd::ClueManager;
+pub use ics20_withdrawal_with_handler::Ics20WithdrawalWithHandler;
 pub use note_manager::NoteManager;
 pub use shielded_pool::{ShieldedPool, StateReadExt, StateWriteExt};
 pub use transfer::Ics20Transfer;

diff --git a/crates/core/component/shielded-pool/src/component/action_handler/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/component/action_handler/ics20_withdrawal.rs
@@ -1,24 +1,18 @@
 use std::sync::Arc;
 
 use anyhow::{ensure, Result};
-use async_trait::async_trait;
 use cnidarium::{StateRead, StateWrite};
-use cnidarium_component::ActionHandler;
-use penumbra_ibc::StateReadExt as _;
+use penumbra_ibc::{component::HostInterface, StateReadExt as _};
 
-use crate::{
-    component::transfer::{Ics20TransferReadExt as _, Ics20TransferWriteExt as _},
-    Ics20Withdrawal,
-};
+use crate::component::transfer::{Ics20TransferReadExt as _, Ics20TransferWriteExt as _};
+use crate::component::Ics20WithdrawalWithHandler;
 
-#[async_trait]
-impl ActionHandler for Ics20Withdrawal {
-    type CheckStatelessContext = ();
-    async fn check_stateless(&self, _context: ()) -> Result<()> {
-        self.validate()
+impl<HI: HostInterface> Ics20WithdrawalWithHandler<HI> {
+    pub async fn check_stateless(&self, _context: ()) -> Result<()> {
+        self.action().validate()
     }
 
-    async fn check_historical<S: StateRead + 'static>(&self, state: Arc<S>) -> Result<()> {
+    pub async fn check_historical<S: StateRead + 'static>(&self, state: Arc<S>) -> Result<()> {
         ensure!(
             state
                 .get_ibc_params()
@@ -29,8 +23,11 @@ impl ActionHandler for Ics20Withdrawal {
         Ok(())
     }
 
-    async fn check_and_execute<S: StateWrite>(&self, mut state: S) -> Result<()> {
-        state.withdrawal_check(self).await?;
-        state.withdrawal_execute(self).await
+    pub async fn check_and_execute<S: StateWrite>(&self, mut state: S) -> Result<()> {
+        let current_block_time = HI::get_block_timestamp(&state).await?;
+        state
+            .withdrawal_check(self.action(), current_block_time)
+            .await?;
+        state.withdrawal_execute(self.action()).await
     }
 }
diff --git a/crates/core/component/shielded-pool/src/component/ics20_withdrawal_with_handler.rs b/crates/core/component/shielded-pool/src/component/ics20_withdrawal_with_handler.rs
@@ -0,0 +1,31 @@
+use crate::Ics20Withdrawal;
+use penumbra_ibc::component::HostInterface;
+use std::marker::PhantomData;
+
+pub struct Ics20WithdrawalWithHandler<HI>(Ics20Withdrawal, PhantomData<HI>);
+
+impl<HI> Ics20WithdrawalWithHandler<HI> {
+    pub fn new(action: Ics20Withdrawal) -> Self {
+        Self(action, PhantomData)
+    }
+
+    pub fn action(&self) -> &Ics20Withdrawal {
+        &self.0
+    }
+
+    pub fn into_inner(self) -> Ics20Withdrawal {
+        self.0
+    }
+}
+
+impl<HI> From<Ics20WithdrawalWithHandler<HI>> for Ics20Withdrawal {
+    fn from(value: Ics20WithdrawalWithHandler<HI>) -> Self {
+        value.0
+    }
+}
+
+impl Ics20Withdrawal {
+    pub fn with_handler<HI: HostInterface>(self) -> Ics20WithdrawalWithHandler<HI> {
+        Ics20WithdrawalWithHandler::new(self)
+    }
+}
diff --git a/crates/core/component/shielded-pool/src/component/transfer.rs b/crates/core/component/shielded-pool/src/component/transfer.rs
@@ -35,6 +35,7 @@ use penumbra_ibc::component::{
     },
     state_key,
 };
+use tendermint::Time;
 
 // returns a bool indicating if the provided denom was issued locally or if it was bridged in.
 // this logic is a bit tricky, and adapted from https://github.com/cosmos/ibc/tree/main/spec/app/ics-020-fungible-token-transfer (sendFungibleTokens).
@@ -68,12 +69,16 @@ pub struct Ics20Transfer {}
 
 #[async_trait]
 pub trait Ics20TransferReadExt: StateRead {
-    async fn withdrawal_check(&self, withdrawal: &Ics20Withdrawal) -> Result<()> {
+    async fn withdrawal_check(
+        &self,
+        withdrawal: &Ics20Withdrawal,
+        current_block_time: Time,
+    ) -> Result<()> {
         // create packet
         let packet: IBCPacket<Unchecked> = withdrawal.clone().into();
 
         // send packet
-        self.send_packet_check(packet).await?;
+        self.send_packet_check(packet, current_block_time).await?;
 
         Ok(())
     }