app: fold ibc stateful/historical check into their AHs

crates/core/app/src/action_handler/actions.rs

@@ -3,7 +3,6 @@ use std::sync::Arc;
 use anyhow::Result;
 use async_trait::async_trait;
 use cnidarium::{StateRead, StateWrite};
-use penumbra_ibc::component::StateReadExt as _;
 use penumbra_shielded_pool::component::Ics20Transfer;
 use penumbra_transaction::Action;
 use penumbra_txhash::TransactionContext;
@@ -73,25 +72,13 @@ impl AppActionHandler for Action {
             Action::Spend(action) => action.check_historical(state).await,
             Action::Output(action) => action.check_historical(state).await,
             Action::IbcRelay(action) => {
-                if !state.get_ibc_params().await?.ibc_enabled {
-                    anyhow::bail!("transaction contains IBC actions, but IBC is not enabled");
-                }
                 action
                     .clone()
                     .with_handler::<Ics20Transfer, PenumbraHost>()
-                    .check_stateful(state)
+                    .check_historical(state)
                     .await
             }
-            Action::Ics20Withdrawal(action) => {
-                if !state
-                    .get_ibc_params()
-                    .await?
-                    .outbound_ics20_transfers_enabled
-                {
-                    anyhow::bail!("transaction an ICS20 withdrawal, but outbound ICS20 withdrawals are not enabled");
-                }
-                action.check_historical(state).await
-            }
+            Action::Ics20Withdrawal(action) => action.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,
@@ -123,7 +110,7 @@ impl AppActionHandler for Action {
                 action
                     .clone()
                     .with_handler::<Ics20Transfer, PenumbraHost>()
-                    .execute(state)
+                    .check_and_execute(state)
                     .await
             }
             Action::Ics20Withdrawal(action) => action.check_and_execute(state).await,

crates/core/component/ibc/src/component/action_handler/ibc_action.rs

@@ -1,11 +1,11 @@
 use std::sync::Arc;
 
-use anyhow::{Context, Result};
+use anyhow::{ensure, Context, Result};
 use cnidarium::{StateRead, StateWrite};
 
 use crate::{
     component::{app_handler::AppHandler, HostInterface, MsgHandler as _},
-    IbcRelay, IbcRelayWithHandlers,
+    IbcRelay, IbcRelayWithHandlers, StateReadExt as _,
 };
 
 impl<AH: AppHandler, HI: HostInterface> IbcRelayWithHandlers<AH, HI> {
@@ -37,12 +37,17 @@ impl<AH: AppHandler, HI: HostInterface> IbcRelayWithHandlers<AH, HI> {
         Ok(())
     }
 
-    pub async fn check_stateful<S: StateRead + 'static>(&self, _state: Arc<S>) -> Result<()> {
-        // No-op: IBC actions merge check_stateful and execute.
+    pub async fn check_historical<S: StateRead + 'static>(&self, state: Arc<S>) -> Result<()> {
+        // SAFETY: this is safe to check here because ibc component parameters cannot change
+        // during transaction processing.
+        ensure!(
+            state.get_ibc_params().await?.ibc_enabled,
+            "transaction contains IBC actions, but IBC is not enabled"
+        );
         Ok(())
     }
 
-    pub async fn execute<S: StateWrite>(&self, state: S) -> Result<()> {
+    pub async fn check_and_execute<S: StateWrite>(&self, state: S) -> Result<()> {
         let action = self.action();
         match action {
             IbcRelay::CreateClient(msg) => msg

crates/core/component/ibc/src/component/client.rs

@@ -376,7 +376,7 @@ mod tests {
 
     use crate::component::ibc_action_with_handler::IbcRelayWithHandlers;
     use crate::component::ClientStateReadExt;
-    use crate::IbcRelay;
+    use crate::{IbcRelay, StateWriteExt};
 
     use crate::component::app_handler::{AppHandler, AppHandlerCheck, AppHandlerExecute};
     use ibc_types::core::channel::msgs::{
@@ -522,6 +522,11 @@ mod tests {
         let mut state_tx = state.try_begin_transaction().unwrap();
         state_tx.put_block_timestamp(timestamp);
         state_tx.put_block_height(1);
+        state_tx.put_ibc_params(crate::params::IBCParameters {
+            ibc_enabled: true,
+            inbound_ics20_transfers_enabled: true,
+            outbound_ics20_transfers_enabled: true,
+        });
         state_tx.put_epoch_by_height(
             1,
             Epoch {
@@ -558,19 +563,23 @@ mod tests {
         );
 
         create_client_action.check_stateless(()).await?;
-        create_client_action.check_stateful(state.clone()).await?;
+        create_client_action.check_historical(state.clone()).await?;
         let mut state_tx = state.try_begin_transaction().unwrap();
-        create_client_action.execute(&mut state_tx).await?;
+        create_client_action
+            .check_and_execute(&mut state_tx)
+            .await?;
         state_tx.apply();
 
         // Check that state reflects +1 client apps registered.
         assert_eq!(state.client_counter().await.unwrap().0, 1);
 
         // Now we update the client and confirm that the update landed in state.
         update_client_action.check_stateless(()).await?;
-        update_client_action.check_stateful(state.clone()).await?;
+        update_client_action.check_historical(state.clone()).await?;
         let mut state_tx = state.try_begin_transaction().unwrap();
-        update_client_action.execute(&mut state_tx).await?;
+        update_client_action
+            .check_and_execute(&mut state_tx)
+            .await?;
         state_tx.apply();
 
         // We've had one client update, yes. What about second client update?
@@ -587,12 +596,45 @@ mod tests {
 
         second_update_client_action.check_stateless(()).await?;
         second_update_client_action
-            .check_stateful(state.clone())
+            .check_historical(state.clone())
             .await?;
         let mut state_tx = state.try_begin_transaction().unwrap();
-        second_update_client_action.execute(&mut state_tx).await?;
+        second_update_client_action
+            .check_and_execute(&mut state_tx)
+            .await?;
         state_tx.apply();
 
         Ok(())
     }
+
+    #[tokio::test]
+    /// Check that we're not able to create a client if the IBC component is disabled.
+    async fn test_disabled_ibc_component() -> anyhow::Result<()> {
+        let mut state = Arc::new(StateDelta::new(()));
+        let mut state_tx = state.try_begin_transaction().unwrap();
+        state_tx.put_ibc_params(crate::params::IBCParameters {
+            ibc_enabled: false,
+            inbound_ics20_transfers_enabled: true,
+            outbound_ics20_transfers_enabled: true,
+        });
+
+        let msg_create_client_stargaze_raw = BASE64_STANDARD
+            .decode(include_str!("./test/create_client.msg").replace('\n', ""))
+            .unwrap();
+        let msg_create_stargaze_client =
+            MsgCreateClient::decode(msg_create_client_stargaze_raw.as_slice()).unwrap();
+
+        let create_client_action = IbcRelayWithHandlers::<MockAppHandler, MockHost>::new(
+            IbcRelay::CreateClient(msg_create_stargaze_client),
+        );
+        state_tx.apply();
+
+        create_client_action.check_stateless(()).await?;
+        create_client_action
+            .check_historical(state.clone())
+            .await
+            .expect_err("should not be able to create a client");
+
+        Ok(())
+    }
 }

crates/core/component/shielded-pool/src/component/action_handler/ics20_withdrawal.rs

@@ -1,7 +1,10 @@
-use anyhow::Result;
+use std::sync::Arc;
+
+use anyhow::{ensure, Result};
 use async_trait::async_trait;
-use cnidarium::StateWrite;
+use cnidarium::{StateRead, StateWrite};
 use cnidarium_component::ActionHandler;
+use penumbra_ibc::StateReadExt as _;
 
 use crate::{
     component::transfer::{Ics20TransferReadExt as _, Ics20TransferWriteExt as _},
@@ -15,6 +18,17 @@ impl ActionHandler for Ics20Withdrawal {
         self.validate()
     }
 
+    async fn check_historical<S: StateRead + 'static>(&self, state: Arc<S>) -> Result<()> {
+        ensure!(
+            state
+                .get_ibc_params()
+                .await?
+                .outbound_ics20_transfers_enabled,
+            "transaction an ICS20 withdrawal, but outbound ICS20 withdrawals are not enabled"
+        );
+        Ok(())
+    }
+
     async fn check_and_execute<S: StateWrite>(&self, mut state: S) -> Result<()> {
         state.withdrawal_check(self).await?;
         state.withdrawal_execute(self).await