diff --git a/crates/core/app/src/action_handler/actions.rs b/crates/core/app/src/action_handler/actions.rs index 4d74d185db..27773aeaac 100644 --- a/crates/core/app/src/action_handler/actions.rs +++ b/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::() - .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::() - .execute(state) + .check_and_execute(state) .await } Action::Ics20Withdrawal(action) => action.check_and_execute(state).await, diff --git a/crates/core/component/ibc/src/component/action_handler/ibc_action.rs b/crates/core/component/ibc/src/component/action_handler/ibc_action.rs index 2cf5839908..45e079bf13 100644 --- a/crates/core/component/ibc/src/component/action_handler/ibc_action.rs +++ b/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 IbcRelayWithHandlers { @@ -37,12 +37,17 @@ impl IbcRelayWithHandlers { Ok(()) } - pub async fn check_stateful(&self, _state: Arc) -> Result<()> { - // No-op: IBC actions merge check_stateful and execute. + pub async fn check_historical(&self, state: Arc) -> 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(&self, state: S) -> Result<()> { + pub async fn check_and_execute(&self, state: S) -> Result<()> { let action = self.action(); match action { IbcRelay::CreateClient(msg) => msg diff --git a/crates/core/component/ibc/src/component/client.rs b/crates/core/component/ibc/src/component/client.rs index e6b4be0dd5..cc7e132804 100644 --- a/crates/core/component/ibc/src/component/client.rs +++ b/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,9 +563,11 @@ 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. @@ -568,9 +575,11 @@ mod tests { // 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::::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(()) + } } 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 index 3b83c05084..77657dd099 100644 --- 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,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(&self, state: Arc) -> 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(&self, mut state: S) -> Result<()> { state.withdrawal_check(self).await?; state.withdrawal_execute(self).await