From d5b9a3d4a12d17a89ae61a4aefec23dacdea72d3 Mon Sep 17 00:00:00 2001 From: Ethan Oroshiba Date: Mon, 2 Dec 2024 10:32:08 -0600 Subject: [PATCH] chore(sequencer): consolidate all action handling to one module (#1759) ## Summary Moved all action and transaction handling to single module. ## Background Previously, all implementations of `ActionHandler` were done separately in components which corresponded to their given actions (or groups of actions, such as in `bridge`). While this may make sense from an action-centered point of view, it is confusing to an outsider to have to look many different places to find where stateless/stateful checks and execution occur. This PR consolidates all of the implementations as well as their testing. ## Changes - Created new `action_handler` module and moved the corresponding `ActionHandler` trait as well as all of its `impl`s into this module. - Renamed some moved tests for clarity. - Categorized tests by action in submodule `tests`. - Categorized impls by action in submodule `impls`. ## Testing Passing all tests. ## Changelogs Changelogs updated ## Related Issues closes #1657 --- crates/astria-sequencer/CHANGELOG.md | 1 + .../astria-sequencer/src/accounts/action.rs | 99 ------ crates/astria-sequencer/src/accounts/mod.rs | 1 - .../impls/bridge_lock.rs} | 11 +- .../impls/bridge_sudo_change.rs} | 35 ++- .../impls/bridge_unlock.rs} | 25 +- .../action_handler/impls/fee_asset_change.rs | 65 ++++ .../impls/fee_change.rs} | 92 ++---- .../impls}/ibc_relayer_change.rs | 2 +- .../action_handler/impls/ibc_sudo_change.rs | 44 +++ .../impls}/ics20_withdrawal.rs | 220 +++++++------- .../impls/init_bridge_account.rs} | 9 +- .../src/action_handler/impls/mod.rs | 16 + .../impls/rollup_data_submission.rs} | 5 +- .../impls/sudo_address_change.rs | 48 +++ .../src/action_handler/impls/test_utils.rs | 5 + .../src/action_handler/impls/transaction.rs | 281 ++++++++++++++++++ .../src/action_handler/impls/transfer.rs | 46 +++ .../action_handler/impls/validator_update.rs | 67 +++++ .../src/action_handler/mod.rs | 94 ++++++ .../src/app/action_handler.rs | 26 -- crates/astria-sequencer/src/app/mod.rs | 15 +- .../src/app/tests_block_fees.rs | 0 .../src/app/tests_execute_transaction.rs | 10 +- .../astria-sequencer/src/authority/action.rs | 130 -------- crates/astria-sequencer/src/authority/mod.rs | 1 - crates/astria-sequencer/src/bridge/mod.rs | 4 - crates/astria-sequencer/src/fees/mod.rs | 1 - crates/astria-sequencer/src/fees/tests.rs | 2 +- crates/astria-sequencer/src/ibc/mod.rs | 2 - crates/astria-sequencer/src/lib.rs | 2 +- .../astria-sequencer/src/rollup_data/mod.rs | 1 - .../src/service/mempool/mod.rs | 2 +- .../astria-sequencer/src/transaction/mod.rs | 279 ----------------- 34 files changed, 861 insertions(+), 780 deletions(-) delete mode 100644 crates/astria-sequencer/src/accounts/action.rs rename crates/astria-sequencer/src/{bridge/bridge_lock_action.rs => action_handler/impls/bridge_lock.rs} (97%) rename crates/astria-sequencer/src/{bridge/bridge_sudo_change_action.rs => action_handler/impls/bridge_sudo_change.rs} (90%) rename crates/astria-sequencer/src/{bridge/bridge_unlock_action.rs => action_handler/impls/bridge_unlock.rs} (95%) create mode 100644 crates/astria-sequencer/src/action_handler/impls/fee_asset_change.rs rename crates/astria-sequencer/src/{fees/action.rs => action_handler/impls/fee_change.rs} (74%) rename crates/astria-sequencer/src/{ibc => action_handler/impls}/ibc_relayer_change.rs (98%) create mode 100644 crates/astria-sequencer/src/action_handler/impls/ibc_sudo_change.rs rename crates/astria-sequencer/src/{ibc => action_handler/impls}/ics20_withdrawal.rs (94%) rename crates/astria-sequencer/src/{bridge/init_bridge_account_action.rs => action_handler/impls/init_bridge_account.rs} (95%) create mode 100644 crates/astria-sequencer/src/action_handler/impls/mod.rs rename crates/astria-sequencer/src/{rollup_data/action.rs => action_handler/impls/rollup_data_submission.rs} (87%) create mode 100644 crates/astria-sequencer/src/action_handler/impls/sudo_address_change.rs create mode 100644 crates/astria-sequencer/src/action_handler/impls/test_utils.rs create mode 100644 crates/astria-sequencer/src/action_handler/impls/transaction.rs create mode 100644 crates/astria-sequencer/src/action_handler/impls/transfer.rs create mode 100644 crates/astria-sequencer/src/action_handler/impls/validator_update.rs create mode 100644 crates/astria-sequencer/src/action_handler/mod.rs delete mode 100644 crates/astria-sequencer/src/app/action_handler.rs delete mode 100644 crates/astria-sequencer/src/app/tests_block_fees.rs delete mode 100644 crates/astria-sequencer/src/authority/action.rs delete mode 100644 crates/astria-sequencer/src/rollup_data/mod.rs diff --git a/crates/astria-sequencer/CHANGELOG.md b/crates/astria-sequencer/CHANGELOG.md index 783b76235e..3fffea9a55 100644 --- a/crates/astria-sequencer/CHANGELOG.md +++ b/crates/astria-sequencer/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Index all event attributes [#1786](https://github.com/astriaorg/astria/pull/1786). +- Consolidate action handling to single module [#1759](https://github.com/astriaorg/astria/pull/1759). - Ensure all deposit assets are trace prefixed [#1807](https://github.com/astriaorg/astria/pull/1807). ## [1.0.0] - 2024-10-25 diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs deleted file mode 100644 index dce040abf1..0000000000 --- a/crates/astria-sequencer/src/accounts/action.rs +++ /dev/null @@ -1,99 +0,0 @@ -use astria_core::protocol::transaction::v1::action::Transfer; -use astria_eyre::eyre::{ - ensure, - Result, - WrapErr as _, -}; -use cnidarium::{ - StateRead, - StateWrite, -}; - -use super::AddressBytes; -use crate::{ - accounts::{ - StateReadExt as _, - StateWriteExt as _, - }, - address::StateReadExt as _, - app::ActionHandler, - bridge::StateReadExt as _, - transaction::StateReadExt as _, -}; - -#[async_trait::async_trait] -impl ActionHandler for Transfer { - async fn check_stateless(&self) -> Result<()> { - Ok(()) - } - - async fn check_and_execute(&self, state: S) -> Result<()> { - let from = state - .get_transaction_context() - .expect("transaction source must be present in state when executing an action") - .address_bytes(); - - ensure!( - state - .get_bridge_account_rollup_id(&from) - .await - .wrap_err("failed to get bridge account rollup id")? - .is_none(), - "cannot transfer out of bridge account; BridgeUnlock must be used", - ); - - check_transfer(self, &from, &state).await?; - execute_transfer(self, &from, state).await?; - - Ok(()) - } -} - -pub(crate) async fn execute_transfer( - action: &Transfer, - from: &TAddress, - mut state: S, -) -> Result<()> -where - S: StateWrite, - TAddress: AddressBytes, -{ - let from = from.address_bytes(); - state - .decrease_balance(from, &action.asset, action.amount) - .await - .wrap_err("failed decreasing `from` account balance")?; - state - .increase_balance(&action.to, &action.asset, action.amount) - .await - .wrap_err("failed increasing `to` account balance")?; - - Ok(()) -} - -pub(crate) async fn check_transfer( - action: &Transfer, - from: &TAddress, - state: &S, -) -> Result<()> -where - S: StateRead, - TAddress: AddressBytes, -{ - state.ensure_base_prefix(&action.to).await.wrap_err( - "failed ensuring that the destination address matches the permitted base prefix", - )?; - - let transfer_asset = &action.asset; - - let from_transfer_balance = state - .get_account_balance(from, transfer_asset) - .await - .wrap_err("failed to get account balance in transfer check")?; - ensure!( - from_transfer_balance >= action.amount, - "insufficient funds for transfer" - ); - - Ok(()) -} diff --git a/crates/astria-sequencer/src/accounts/mod.rs b/crates/astria-sequencer/src/accounts/mod.rs index 436ff211ea..6931208e9f 100644 --- a/crates/astria-sequencer/src/accounts/mod.rs +++ b/crates/astria-sequencer/src/accounts/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod action; pub(crate) mod component; pub(crate) mod query; mod state_ext; diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/action_handler/impls/bridge_lock.rs similarity index 97% rename from crates/astria-sequencer/src/bridge/bridge_lock_action.rs rename to crates/astria-sequencer/src/action_handler/impls/bridge_lock.rs index 4498a9860e..3be717fc61 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/action_handler/impls/bridge_lock.rs @@ -11,25 +11,26 @@ use astria_eyre::eyre::{ Result, WrapErr as _, }; +use async_trait::async_trait; use cnidarium::StateWrite; use crate::{ - accounts::action::{ + action_handler::{ check_transfer, execute_transfer, + ActionHandler, }, address::StateReadExt as _, - app::ActionHandler, assets::StateReadExt as _, bridge::{ StateReadExt as _, - StateWriteExt as _, + StateWriteExt, }, transaction::StateReadExt as _, utils::create_deposit_event, }; -#[async_trait::async_trait] +#[async_trait] impl ActionHandler for BridgeLock { async fn check_stateless(&self) -> Result<()> { Ok(()) @@ -122,8 +123,8 @@ mod tests { AddressBytes, StateWriteExt as _, }, + action_handler::ActionHandler as _, address::StateWriteExt as _, - app::ActionHandler as _, assets::StateWriteExt as _, benchmark_and_test_utils::{ astria_address, diff --git a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs b/crates/astria-sequencer/src/action_handler/impls/bridge_sudo_change.rs similarity index 90% rename from crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs rename to crates/astria-sequencer/src/action_handler/impls/bridge_sudo_change.rs index 10757babcd..6bdb3ee4f4 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/action_handler/impls/bridge_sudo_change.rs @@ -5,18 +5,20 @@ use astria_eyre::eyre::{ Result, WrapErr as _, }; +use async_trait::async_trait; use cnidarium::StateWrite; use crate::{ + action_handler::ActionHandler, address::StateReadExt as _, - app::ActionHandler, - bridge::state_ext::{ + bridge::{ StateReadExt as _, - StateWriteExt as _, + StateWriteExt, }, transaction::StateReadExt as _, }; -#[async_trait::async_trait] + +#[async_trait] impl ActionHandler for BridgeSudoChange { async fn check_stateless(&self) -> Result<()> { Ok(()) @@ -79,22 +81,29 @@ impl ActionHandler for BridgeSudoChange { #[cfg(test)] mod tests { use astria_core::{ - primitive::v1::{ - asset, - TransactionId, + primitive::v1::TransactionId, + protocol::{ + fees::v1::BridgeSudoChangeFeeComponents, + transaction::v1::action::BridgeSudoChange, }, - protocol::fees::v1::BridgeSudoChangeFeeComponents, }; use cnidarium::StateDelta; - use super::*; use crate::{ accounts::StateWriteExt as _, + action_handler::{ + impls::test_utils::test_asset, + ActionHandler as _, + }, address::StateWriteExt as _, benchmark_and_test_utils::{ astria_address, ASTRIA_PREFIX, }, + bridge::{ + StateReadExt as _, + StateWriteExt as _, + }, fees::StateWriteExt as _, transaction::{ StateWriteExt as _, @@ -102,12 +111,8 @@ mod tests { }, }; - fn test_asset() -> asset::Denom { - "test".parse().unwrap() - } - #[tokio::test] - async fn fails_with_unauthorized_if_signer_is_not_sudo_address() { + async fn bridge_sudo_change_fails_with_unauthorized_if_signer_is_not_sudo_address() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -146,7 +151,7 @@ mod tests { } #[tokio::test] - async fn executes() { + async fn bridge_sudo_change_executes_as_expected() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/action_handler/impls/bridge_unlock.rs similarity index 95% rename from crates/astria-sequencer/src/bridge/bridge_unlock_action.rs rename to crates/astria-sequencer/src/action_handler/impls/bridge_unlock.rs index ca906c5dca..10da98c5a4 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/action_handler/impls/bridge_unlock.rs @@ -8,23 +8,24 @@ use astria_eyre::eyre::{ Result, WrapErr as _, }; +use async_trait::async_trait; use cnidarium::StateWrite; use crate::{ - accounts::action::{ + action_handler::{ check_transfer, execute_transfer, + ActionHandler, }, address::StateReadExt as _, - app::ActionHandler, bridge::{ StateReadExt as _, - StateWriteExt as _, + StateWriteExt, }, transaction::StateReadExt as _, }; -#[async_trait::async_trait] +#[async_trait] impl ActionHandler for BridgeUnlock { // TODO(https://github.com/astriaorg/astria/issues/1430): move checks to the `BridgeUnlock` parsing. async fn check_stateless(&self) -> Result<()> { @@ -104,7 +105,6 @@ impl ActionHandler for BridgeUnlock { mod tests { use astria_core::{ primitive::v1::{ - asset, RollupId, TransactionId, }, @@ -117,8 +117,11 @@ mod tests { use crate::{ accounts::StateWriteExt as _, + action_handler::{ + impls::test_utils::test_asset, + ActionHandler as _, + }, address::StateWriteExt as _, - app::ActionHandler as _, benchmark_and_test_utils::{ assert_eyre_error, astria_address, @@ -132,12 +135,8 @@ mod tests { }, }; - fn test_asset() -> asset::Denom { - "test".parse().unwrap() - } - #[tokio::test] - async fn fails_if_bridge_account_has_no_withdrawer_address() { + async fn bridge_unlock_fails_if_bridge_account_has_no_withdrawer_address() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -176,7 +175,7 @@ mod tests { } #[tokio::test] - async fn fails_if_withdrawer_is_not_signer() { + async fn bridge_unlock_fails_if_withdrawer_is_not_signer() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -219,7 +218,7 @@ mod tests { } #[tokio::test] - async fn execute_with_duplicated_withdrawal_event_id() { + async fn bridge_unlock_executes_with_duplicated_withdrawal_event_id() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); diff --git a/crates/astria-sequencer/src/action_handler/impls/fee_asset_change.rs b/crates/astria-sequencer/src/action_handler/impls/fee_asset_change.rs new file mode 100644 index 0000000000..c92161e751 --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/fee_asset_change.rs @@ -0,0 +1,65 @@ +use astria_core::protocol::transaction::v1::action::FeeAssetChange; +use astria_eyre::eyre::{ + self, + ensure, + WrapErr as _, +}; +use async_trait::async_trait; +use cnidarium::StateWrite; +use futures::StreamExt as _; +use tokio::pin; + +use crate::{ + action_handler::ActionHandler, + authority::StateReadExt as _, + fees::{ + StateReadExt as _, + StateWriteExt as _, + }, + transaction::StateReadExt as _, +}; + +#[async_trait] +impl ActionHandler for FeeAssetChange { + async fn check_stateless(&self) -> eyre::Result<()> { + Ok(()) + } + + async fn check_and_execute(&self, mut state: S) -> eyre::Result<()> { + let from = state + .get_transaction_context() + .expect("transaction source must be present in state when executing an action") + .address_bytes(); + let authority_sudo_address = state + .get_sudo_address() + .await + .wrap_err("failed to get authority sudo address")?; + ensure!( + authority_sudo_address == from, + "unauthorized address for fee asset change" + ); + match self { + FeeAssetChange::Addition(asset) => { + state + .put_allowed_fee_asset(asset) + .context("failed to write allowed fee asset to state")?; + } + FeeAssetChange::Removal(asset) => { + state.delete_allowed_fee_asset(asset); + + pin!( + let assets = state.allowed_fee_assets(); + ); + ensure!( + assets + .filter_map(|item| std::future::ready(item.ok())) + .next() + .await + .is_some(), + "cannot remove last allowed fee asset", + ); + } + } + Ok(()) + } +} diff --git a/crates/astria-sequencer/src/fees/action.rs b/crates/astria-sequencer/src/action_handler/impls/fee_change.rs similarity index 74% rename from crates/astria-sequencer/src/fees/action.rs rename to crates/astria-sequencer/src/action_handler/impls/fee_change.rs index 89c6bb12e6..d83cf2ed61 100644 --- a/crates/astria-sequencer/src/fees/action.rs +++ b/crates/astria-sequencer/src/action_handler/impls/fee_change.rs @@ -1,27 +1,20 @@ -use astria_core::protocol::transaction::v1::action::{ - FeeAssetChange, - FeeChange, -}; +use astria_core::protocol::transaction::v1::action::FeeChange; use astria_eyre::eyre::{ self, ensure, WrapErr as _, }; +use async_trait::async_trait; use cnidarium::StateWrite; -use futures::StreamExt; -use tokio::pin; use crate::{ - app::ActionHandler, + action_handler::ActionHandler, authority::StateReadExt as _, - fees::{ - StateReadExt as _, - StateWriteExt as _, - }, + fees::StateWriteExt as _, transaction::StateReadExt as _, }; -#[async_trait::async_trait] +#[async_trait] impl ActionHandler for FeeChange { async fn check_stateless(&self) -> eyre::Result<()> { Ok(()) @@ -88,51 +81,6 @@ impl ActionHandler for FeeChange { } } -#[async_trait::async_trait] -impl ActionHandler for FeeAssetChange { - async fn check_stateless(&self) -> eyre::Result<()> { - Ok(()) - } - - async fn check_and_execute(&self, mut state: S) -> eyre::Result<()> { - let from = state - .get_transaction_context() - .expect("transaction source must be present in state when executing an action") - .address_bytes(); - let authority_sudo_address = state - .get_sudo_address() - .await - .wrap_err("failed to get authority sudo address")?; - ensure!( - authority_sudo_address == from, - "unauthorized address for fee asset change" - ); - match self { - FeeAssetChange::Addition(asset) => { - state - .put_allowed_fee_asset(asset) - .context("failed to write allowed fee asset to state")?; - } - FeeAssetChange::Removal(asset) => { - state.delete_allowed_fee_asset(asset); - - pin!( - let assets = state.allowed_fee_assets(); - ); - ensure!( - assets - .filter_map(|item| std::future::ready(item.ok())) - .next() - .await - .is_some(), - "cannot remove last allowed fee asset", - ); - } - } - Ok(()) - } -} - #[cfg(test)] mod tests { use astria_core::{ @@ -141,7 +89,7 @@ mod tests { }; use crate::{ - app::ActionHandler as _, + action_handler::ActionHandler as _, authority::StateWriteExt as _, fees::StateReadExt as _, transaction::{ @@ -216,19 +164,19 @@ mod tests { } test_fee_change_action!( - transfer => Transfer, - rollup_data_submission => RollupDataSubmission, - ics20_withdrawal => Ics20Withdrawal, - init_bridge_account => InitBridgeAccount, - bridge_lock => BridgeLock, - bridge_unlock => BridgeUnlock, - bridge_sudo_change => BridgeSudoChange, - validator_update => ValidatorUpdate, - ibc_relayer_change => IbcRelayerChange, - ibc_relay => IbcRelay, - fee_asset_change => FeeAssetChange, - fee_change => FeeChange, - sudo_address_change => SudoAddressChange, - ibc_sudo_change => IbcSudoChange, + transfer => Transfer, + rollup_data_submission => RollupDataSubmission, + ics20_withdrawal => Ics20Withdrawal, + init_bridge_account => InitBridgeAccount, + bridge_lock => BridgeLock, + bridge_unlock => BridgeUnlock, + bridge_sudo_change => BridgeSudoChange, + validator_update => ValidatorUpdate, + ibc_relayer_change => IbcRelayerChange, + ibc_relay => IbcRelay, + fee_asset_change => FeeAssetChange, + fee_change => FeeChange, + sudo_address_change => SudoAddressChange, + ibc_sudo_change => IbcSudoChange, ); } diff --git a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs b/crates/astria-sequencer/src/action_handler/impls/ibc_relayer_change.rs similarity index 98% rename from crates/astria-sequencer/src/ibc/ibc_relayer_change.rs rename to crates/astria-sequencer/src/action_handler/impls/ibc_relayer_change.rs index 0c277b412e..e5de94afbe 100644 --- a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs +++ b/crates/astria-sequencer/src/action_handler/impls/ibc_relayer_change.rs @@ -8,8 +8,8 @@ use async_trait::async_trait; use cnidarium::StateWrite; use crate::{ + action_handler::ActionHandler, address::StateReadExt as _, - app::ActionHandler, ibc::{ StateReadExt as _, StateWriteExt as _, diff --git a/crates/astria-sequencer/src/action_handler/impls/ibc_sudo_change.rs b/crates/astria-sequencer/src/action_handler/impls/ibc_sudo_change.rs new file mode 100644 index 0000000000..f82af648fc --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/ibc_sudo_change.rs @@ -0,0 +1,44 @@ +use astria_core::protocol::transaction::v1::action::IbcSudoChange; +use astria_eyre::eyre::{ + ensure, + Result, + WrapErr as _, +}; +use async_trait::async_trait; +use cnidarium::StateWrite; + +use crate::{ + action_handler::ActionHandler, + address::StateReadExt as _, + authority::StateReadExt as _, + ibc::StateWriteExt as _, + transaction::StateReadExt as _, +}; + +#[async_trait] +impl ActionHandler for IbcSudoChange { + async fn check_stateless(&self) -> Result<()> { + Ok(()) + } + + async fn check_and_execute(&self, mut state: S) -> Result<()> { + let from = state + .get_transaction_context() + .expect("transaction source must be present in state when executing an action") + .address_bytes(); + state + .ensure_base_prefix(&self.new_address) + .await + .wrap_err("desired new ibc sudo address has an unsupported prefix")?; + // ensure signer is the valid `sudo` key in state + let sudo_address = state + .get_sudo_address() + .await + .wrap_err("failed to get sudo address from state")?; + ensure!(sudo_address == from, "signer is not the sudo key"); + state + .put_ibc_sudo_address(self.new_address) + .wrap_err("failed to put ibc sudo address in state")?; + Ok(()) + } +} diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/action_handler/impls/ics20_withdrawal.rs similarity index 94% rename from crates/astria-sequencer/src/ibc/ics20_withdrawal.rs rename to crates/astria-sequencer/src/action_handler/impls/ics20_withdrawal.rs index 12975e4abf..35d1523ece 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/action_handler/impls/ics20_withdrawal.rs @@ -6,7 +6,9 @@ use astria_core::{ }, protocol::{ memos::v1::Ics20WithdrawalFromRollup, - transaction::v1::action, + transaction::v1::action::{ + self, + }, }, }; use astria_eyre::{ @@ -19,10 +21,12 @@ use astria_eyre::{ WrapErr as _, }, }; +use async_trait::async_trait; use cnidarium::{ StateRead, StateWrite, }; +use ibc_proto::ibc::apps::transfer::v2::FungibleTokenPacketData; use ibc_types::core::channel::{ ChannelId, PortId, @@ -33,21 +37,18 @@ use penumbra_ibc::component::packet::{ SendPacketWrite as _, Unchecked, }; -use penumbra_proto::core::component::ibc::v1::FungibleTokenPacketData; use crate::{ accounts::{ - AddressBytes as _, + AddressBytes, StateWriteExt as _, }, + action_handler::ActionHandler, address::StateReadExt as _, - app::{ - ActionHandler, - StateReadExt as _, - }, + app::StateReadExt as _, bridge::{ StateReadExt as _, - StateWriteExt as _, + StateWriteExt, }, ibc::{ StateReadExt as _, @@ -56,92 +57,7 @@ use crate::{ transaction::StateReadExt as _, }; -async fn create_ibc_packet_from_withdrawal( - withdrawal: &action::Ics20Withdrawal, - state: S, -) -> Result> { - let sender = if withdrawal.use_compat_address { - let ibc_compat_prefix = state.get_ibc_compat_prefix().await.context( - "need to construct bech32 compatible address for IBC communication but failed reading \ - required prefix from state", - )?; - withdrawal - .return_address() - .to_prefix(&ibc_compat_prefix) - .context("failed to convert the address to the bech32 compatible prefix")? - .to_format::() - .to_string() - } else { - withdrawal.return_address.to_string() - }; - let packet = FungibleTokenPacketData { - amount: withdrawal.amount.to_string(), - denom: withdrawal.denom.to_string(), - sender, - receiver: withdrawal.destination_chain_address.clone(), - memo: withdrawal.memo.clone(), - }; - - let serialized_packet_data = - serde_json::to_vec(&packet).context("failed to serialize fungible token packet as JSON")?; - - Ok(IBCPacket::new( - PortId::transfer(), - withdrawal.source_channel().clone(), - *withdrawal.timeout_height(), - withdrawal.timeout_time(), - serialized_packet_data, - )) -} - -/// Establishes the withdrawal target. -/// -/// The function returns the following addresses under the following conditions: -/// 1. `action.bridge_address` if `action.bridge_address` is set and `from` is its stored withdrawer -/// address. -/// 2. `from` if `action.bridge_address` is unset and `from` is *not* a bridge account. -/// -/// Errors if: -/// 1. Errors reading from DB -/// 2. `action.bridge_address` is set, but `from` is not the withdrawer address. -/// 3. `action.bridge_address` is unset, but `from` is a bridge account. -async fn establish_withdrawal_target<'a, S: StateRead>( - action: &'a action::Ics20Withdrawal, - state: &S, - from: &'a [u8; 20], -) -> Result<&'a [u8; 20]> { - // If the bridge address is set, the withdrawer on that address must match - // the from address. - if let Some(bridge_address) = &action.bridge_address { - let Some(withdrawer) = state - .get_bridge_account_withdrawer_address(bridge_address) - .await - .wrap_err("failed to get bridge withdrawer")? - else { - bail!("bridge address must have a withdrawer address set"); - }; - - ensure!( - &withdrawer == from.address_bytes(), - "sender does not match bridge withdrawer address; unauthorized" - ); - - return Ok(bridge_address.as_bytes()); - } - - // If the bridge address is not set, the sender must not be a bridge account. - if state - .is_a_bridge_account(from) - .await - .context("failed to establish whether the sender is a bridge account")? - { - bail!("sender cannot be a bridge address if bridge address is not set"); - } - - Ok(from) -} - -#[async_trait::async_trait] +#[async_trait] impl ActionHandler for action::Ics20Withdrawal { // TODO(https://github.com/astriaorg/astria/issues/1430): move checks to the `Ics20Withdrawal` parsing. async fn check_stateless(&self) -> Result<()> { @@ -257,6 +173,91 @@ impl ActionHandler for action::Ics20Withdrawal { } } +async fn create_ibc_packet_from_withdrawal( + withdrawal: &action::Ics20Withdrawal, + state: S, +) -> Result> { + let sender = if withdrawal.use_compat_address { + let ibc_compat_prefix = state.get_ibc_compat_prefix().await.context( + "need to construct bech32 compatible address for IBC communication but failed reading \ + required prefix from state", + )?; + withdrawal + .return_address() + .to_prefix(&ibc_compat_prefix) + .context("failed to convert the address to the bech32 compatible prefix")? + .to_format::() + .to_string() + } else { + withdrawal.return_address.to_string() + }; + let packet = FungibleTokenPacketData { + amount: withdrawal.amount.to_string(), + denom: withdrawal.denom.to_string(), + sender, + receiver: withdrawal.destination_chain_address.clone(), + memo: withdrawal.memo.clone(), + }; + + let serialized_packet_data = + serde_json::to_vec(&packet).context("failed to serialize fungible token packet as JSON")?; + + Ok(IBCPacket::new( + PortId::transfer(), + withdrawal.source_channel().clone(), + *withdrawal.timeout_height(), + withdrawal.timeout_time(), + serialized_packet_data, + )) +} + +/// Establishes the withdrawal target. +/// +/// The function returns the following addresses under the following conditions: +/// 1. `action.bridge_address` if `action.bridge_address` is set and `from` is its stored withdrawer +/// address. +/// 2. `from` if `action.bridge_address` is unset and `from` is *not* a bridge account. +/// +/// Errors if: +/// 1. Errors reading from DB +/// 2. `action.bridge_address` is set, but `from` is not the withdrawer address. +/// 3. `action.bridge_address` is unset, but `from` is a bridge account. +async fn establish_withdrawal_target<'a, S: StateRead>( + action: &'a action::Ics20Withdrawal, + state: &S, + from: &'a [u8; 20], +) -> Result<&'a [u8; 20]> { + // If the bridge address is set, the withdrawer on that address must match + // the from address. + if let Some(bridge_address) = &action.bridge_address { + let Some(withdrawer) = state + .get_bridge_account_withdrawer_address(bridge_address) + .await + .wrap_err("failed to get bridge withdrawer")? + else { + bail!("bridge address must have a withdrawer address set"); + }; + + ensure!( + &withdrawer == from.address_bytes(), + "sender does not match bridge withdrawer address; unauthorized" + ); + + return Ok(bridge_address.as_bytes()); + } + + // If the bridge address is not set, the sender must not be a bridge account. + if state + .is_a_bridge_account(from) + .await + .context("failed to establish whether the sender is a bridge account")? + { + bail!("sender cannot be a bridge address if bridge address is not set"); + } + + Ok(from) +} + fn is_source(source_port: &PortId, source_channel: &ChannelId, asset: &Denom) -> bool { if let Denom::TracePrefixed(trace) = asset { !trace.has_leading_port(source_port) || !trace.has_leading_channel(source_channel) @@ -267,27 +268,34 @@ fn is_source(source_port: &PortId, source_channel: &ChannelId, asset: &Denom) -> #[cfg(test)] mod tests { - use astria_core::primitive::v1::RollupId; + use astria_core::{ + primitive::v1::RollupId, + protocol::transaction::v1::action, + }; use cnidarium::StateDelta; use ibc_types::core::client::Height; - use super::*; use crate::{ + action_handler::impls::{ + ics20_withdrawal::establish_withdrawal_target, + test_utils::test_asset, + }, address::StateWriteExt as _, benchmark_and_test_utils::{ assert_eyre_error, astria_address, ASTRIA_PREFIX, }, + bridge::StateWriteExt as _, }; #[tokio::test] - async fn sender_is_withdrawal_target_if_bridge_is_not_set_and_sender_is_not_bridge() { + async fn withdrawal_target_is_sender_if_bridge_is_not_set_and_sender_is_not_bridge() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let state = StateDelta::new(snapshot); - let denom = "test".parse::().unwrap(); + let denom = test_asset(); let from = [1u8; 20]; let action = action::Ics20Withdrawal { amount: 1, @@ -312,7 +320,7 @@ mod tests { } #[tokio::test] - async fn sender_is_withdrawal_target_if_bridge_is_unset_but_sender_is_bridge() { + async fn withdrawal_target_is_sender_if_bridge_is_unset_but_sender_is_bridge() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -331,7 +339,7 @@ mod tests { .put_bridge_account_withdrawer_address(&bridge_address, bridge_address) .unwrap(); - let denom = "test".parse::().unwrap(); + let denom = test_asset(); let action = action::Ics20Withdrawal { amount: 1, denom: denom.clone(), @@ -361,21 +369,17 @@ mod tests { [1; 20] } - fn denom() -> Denom { - "test".parse().unwrap() - } - fn action() -> action::Ics20Withdrawal { action::Ics20Withdrawal { amount: 1, - denom: denom(), + denom: test_asset(), bridge_address: None, destination_chain_address: "test".to_string(), return_address: astria_address(&[1; 20]), timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset: denom(), + fee_asset: test_asset(), memo: String::new(), use_compat_address: false, } @@ -439,7 +443,7 @@ mod tests { .put_bridge_account_withdrawer_address(&bridge_address, withdrawer_address) .unwrap(); - let denom = "test".parse::().unwrap(); + let denom = test_asset(); let action = action::Ics20Withdrawal { amount: 1, denom: denom.clone(), @@ -471,7 +475,7 @@ mod tests { // sender is not the withdrawer address, so must fail let not_bridge_address = [1u8; 20]; - let denom = "test".parse::().unwrap(); + let denom = test_asset(); let action = action::Ics20Withdrawal { amount: 1, denom: denom.clone(), diff --git a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs b/crates/astria-sequencer/src/action_handler/impls/init_bridge_account.rs similarity index 95% rename from crates/astria-sequencer/src/bridge/init_bridge_account_action.rs rename to crates/astria-sequencer/src/action_handler/impls/init_bridge_account.rs index 72d62f1b76..85e05de042 100644 --- a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs +++ b/crates/astria-sequencer/src/action_handler/impls/init_bridge_account.rs @@ -7,19 +7,20 @@ use astria_eyre::eyre::{ Result, WrapErr as _, }; +use async_trait::async_trait; use cnidarium::StateWrite; use crate::{ + action_handler::ActionHandler, address::StateReadExt as _, - app::ActionHandler, - bridge::state_ext::{ + bridge::{ StateReadExt as _, - StateWriteExt as _, + StateWriteExt, }, transaction::StateReadExt as _, }; -#[async_trait::async_trait] +#[async_trait] impl ActionHandler for InitBridgeAccount { async fn check_stateless(&self) -> Result<()> { Ok(()) diff --git a/crates/astria-sequencer/src/action_handler/impls/mod.rs b/crates/astria-sequencer/src/action_handler/impls/mod.rs new file mode 100644 index 0000000000..c273cc26ff --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/mod.rs @@ -0,0 +1,16 @@ +pub(crate) mod bridge_lock; +pub(crate) mod bridge_sudo_change; +pub(crate) mod bridge_unlock; +pub(crate) mod fee_asset_change; +pub(crate) mod fee_change; +pub(crate) mod ibc_relayer_change; +pub(crate) mod ibc_sudo_change; +pub(crate) mod ics20_withdrawal; +pub(crate) mod init_bridge_account; +pub(crate) mod rollup_data_submission; +pub(crate) mod sudo_address_change; +#[cfg(test)] +pub(crate) mod test_utils; +pub(crate) mod transaction; +pub(crate) mod transfer; +pub(crate) mod validator_update; diff --git a/crates/astria-sequencer/src/rollup_data/action.rs b/crates/astria-sequencer/src/action_handler/impls/rollup_data_submission.rs similarity index 87% rename from crates/astria-sequencer/src/rollup_data/action.rs rename to crates/astria-sequencer/src/action_handler/impls/rollup_data_submission.rs index cde9f8e591..3881e221bf 100644 --- a/crates/astria-sequencer/src/rollup_data/action.rs +++ b/crates/astria-sequencer/src/action_handler/impls/rollup_data_submission.rs @@ -3,11 +3,12 @@ use astria_eyre::eyre::{ ensure, Result, }; +use async_trait::async_trait; use cnidarium::StateWrite; -use crate::app::ActionHandler; +use crate::action_handler::ActionHandler; -#[async_trait::async_trait] +#[async_trait] impl ActionHandler for RollupDataSubmission { async fn check_stateless(&self) -> Result<()> { // TODO: do we want to place a maximum on the size of the data? diff --git a/crates/astria-sequencer/src/action_handler/impls/sudo_address_change.rs b/crates/astria-sequencer/src/action_handler/impls/sudo_address_change.rs new file mode 100644 index 0000000000..7c7cb2e5e2 --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/sudo_address_change.rs @@ -0,0 +1,48 @@ +use astria_core::protocol::transaction::v1::action::SudoAddressChange; +use astria_eyre::eyre::{ + ensure, + Result, + WrapErr as _, +}; +use async_trait::async_trait; +use cnidarium::StateWrite; + +use crate::{ + action_handler::ActionHandler, + address::StateReadExt as _, + authority::{ + StateReadExt as _, + StateWriteExt as _, + }, + transaction::StateReadExt as _, +}; + +#[async_trait] +impl ActionHandler for SudoAddressChange { + async fn check_stateless(&self) -> Result<()> { + Ok(()) + } + + /// check that the signer of the transaction is the current sudo address, + /// as only that address can change the sudo address + async fn check_and_execute(&self, mut state: S) -> Result<()> { + let from = state + .get_transaction_context() + .expect("transaction source must be present in state when executing an action") + .address_bytes(); + state + .ensure_base_prefix(&self.new_address) + .await + .wrap_err("desired new sudo address has an unsupported prefix")?; + // ensure signer is the valid `sudo` key in state + let sudo_address = state + .get_sudo_address() + .await + .wrap_err("failed to get sudo address from state")?; + ensure!(sudo_address == from, "signer is not the sudo key"); + state + .put_sudo_address(self.new_address) + .wrap_err("failed to put sudo address in state")?; + Ok(()) + } +} diff --git a/crates/astria-sequencer/src/action_handler/impls/test_utils.rs b/crates/astria-sequencer/src/action_handler/impls/test_utils.rs new file mode 100644 index 0000000000..55ec91b098 --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/test_utils.rs @@ -0,0 +1,5 @@ +use astria_core::primitive::v1::asset; + +pub(super) fn test_asset() -> asset::Denom { + "test".parse().unwrap() +} diff --git a/crates/astria-sequencer/src/action_handler/impls/transaction.rs b/crates/astria-sequencer/src/action_handler/impls/transaction.rs new file mode 100644 index 0000000000..29df7cc10a --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/transaction.rs @@ -0,0 +1,281 @@ +use std::fmt; + +use astria_core::protocol::transaction::v1::{ + Action, + Transaction, +}; +use astria_eyre::{ + anyhow_to_eyre, + eyre::{ + ensure, + Context as _, + OptionExt as _, + Result, + }, +}; +use cnidarium::StateWrite; + +use crate::{ + accounts::{ + StateReadExt as _, + StateWriteExt as _, + }, + action_handler::ActionHandler, + app::StateReadExt as _, + bridge::{ + StateReadExt as _, + StateWriteExt, + }, + fees::FeeHandler, + ibc::{ + host_interface::AstriaHost, + StateReadExt as _, + }, + transaction::{ + check_balance_for_total_fees_and_transfers, + StateWriteExt as _, + }, +}; + +#[derive(Debug)] +pub(crate) struct InvalidChainId(pub(crate) String); + +impl fmt::Display for InvalidChainId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "provided chain id {} does not match expected chain id", + self.0, + ) + } +} + +impl std::error::Error for InvalidChainId {} + +#[derive(Debug)] +pub(crate) struct InvalidNonce(pub(crate) u32); + +impl fmt::Display for InvalidNonce { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "provided nonce {} does not match expected next nonce", + self.0, + ) + } +} + +impl std::error::Error for InvalidNonce {} + +#[async_trait::async_trait] +impl ActionHandler for Transaction { + async fn check_stateless(&self) -> Result<()> { + ensure!(!self.actions().is_empty(), "must have at least one action"); + + for action in self.actions() { + match action { + Action::Transfer(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for Transfer action")?, + Action::RollupDataSubmission(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for RollupDataSubmission action")?, + Action::ValidatorUpdate(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for ValidatorUpdate action")?, + Action::SudoAddressChange(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for SudoAddressChange action")?, + Action::IbcSudoChange(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for IbcSudoChange action")?, + Action::FeeChange(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for FeeChange action")?, + Action::Ibc(act) => { + let action = act + .clone() + .with_handler::(); + action + .check_stateless(()) + .await + .map_err(anyhow_to_eyre) + .wrap_err("stateless check failed for Ibc action")?; + } + Action::Ics20Withdrawal(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for Ics20Withdrawal action")?, + Action::IbcRelayerChange(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for IbcRelayerChange action")?, + Action::FeeAssetChange(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for FeeAssetChange action")?, + Action::InitBridgeAccount(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for InitBridgeAccount action")?, + Action::BridgeLock(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for BridgeLock action")?, + Action::BridgeUnlock(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for BridgeUnlock action")?, + Action::BridgeSudoChange(act) => act + .check_stateless() + .await + .wrap_err("stateless check failed for BridgeSudoChange action")?, + } + } + Ok(()) + } + + // FIXME (https://github.com/astriaorg/astria/issues/1584): because most lines come from delegating (and error wrapping) to the + // individual actions. This could be tidied up by implementing `ActionHandler for Action` + // and letting it delegate. + #[expect(clippy::too_many_lines, reason = "should be refactored")] + async fn check_and_execute(&self, mut state: S) -> Result<()> { + // Add the current signed transaction into the ephemeral state in case + // downstream actions require access to it. + // XXX: This must be deleted at the end of `check_stateful`. + let mut transaction_context = state.put_transaction_context(self); + + // Transactions must match the chain id of the node. + let chain_id = state.get_chain_id().await?; + ensure!( + self.chain_id() == chain_id.as_str(), + InvalidChainId(self.chain_id().to_string()) + ); + + // Nonce should be equal to the number of executed transactions before this tx. + // First tx has nonce 0. + let curr_nonce = state + .get_account_nonce(self.address_bytes()) + .await + .wrap_err("failed to get nonce for transaction signer")?; + ensure!(curr_nonce == self.nonce(), InvalidNonce(self.nonce())); + + // Should have enough balance to cover all actions. + check_balance_for_total_fees_and_transfers(self, &state) + .await + .wrap_err("failed to check balance for total fees and transfers")?; + + if state + .get_bridge_account_rollup_id(&self) + .await + .wrap_err("failed to check account rollup id")? + .is_some() + { + state + .put_last_transaction_id_for_bridge_account( + &self, + transaction_context.transaction_id, + ) + .wrap_err("failed to put last transaction id for bridge account")?; + } + + let from_nonce = state + .get_account_nonce(&self) + .await + .wrap_err("failed getting nonce of transaction signer")?; + let next_nonce = from_nonce + .checked_add(1) + .ok_or_eyre("overflow occurred incrementing stored nonce")?; + state + .put_account_nonce(&self, next_nonce) + .wrap_err("failed updating `from` nonce")?; + + // FIXME: this should create one span per `check_and_execute` + for (i, action) in (0..).zip(self.actions().iter()) { + transaction_context.position_in_transaction = i; + state.put_transaction_context(transaction_context); + + match action { + Action::Transfer(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("executing transfer action failed")?, + Action::RollupDataSubmission(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("executing sequence action failed")?, + Action::ValidatorUpdate(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("executing validor update")?, + Action::SudoAddressChange(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("executing sudo address change failed")?, + Action::IbcSudoChange(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("executing ibc sudo change failed")?, + Action::FeeChange(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("executing fee change failed")?, + Action::Ibc(act) => { + // FIXME: this check should be moved to check_and_execute, as it now has + // access to the the signer through state. However, what's the correct + // ibc AppHandler call to do it? Can we just update one of the trait methods + // of crate::ibc::ics20_transfer::Ics20Transfer? + ensure!( + state + .is_ibc_relayer(self) + .await + .wrap_err("failed to check if address is IBC relayer")?, + "only IBC sudo address can execute IBC actions" + ); + let action = act + .clone() + .with_handler::(); + action + .check_and_execute(&mut state) + .await + .map_err(anyhow_to_eyre) + .wrap_err("failed executing ibc action")?; + } + Action::Ics20Withdrawal(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing ics20 withdrawal")?, + Action::IbcRelayerChange(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing ibc relayer change")?, + Action::FeeAssetChange(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing fee asseet change")?, + Action::InitBridgeAccount(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing init bridge account")?, + Action::BridgeLock(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing bridge lock")?, + Action::BridgeUnlock(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing bridge unlock")?, + Action::BridgeSudoChange(act) => check_execute_and_pay_fees(act, &mut state) + .await + .wrap_err("failed executing bridge sudo change")?, + } + } + + // XXX: Delete the current transaction data from the ephemeral state. + state.delete_current_transaction_context(); + Ok(()) + } +} + +async fn check_execute_and_pay_fees( + action: &T, + mut state: S, +) -> Result<()> { + action.check_and_execute(&mut state).await?; + action.check_and_pay_fees(&mut state).await?; + Ok(()) +} diff --git a/crates/astria-sequencer/src/action_handler/impls/transfer.rs b/crates/astria-sequencer/src/action_handler/impls/transfer.rs new file mode 100644 index 0000000000..27d955bca3 --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/transfer.rs @@ -0,0 +1,46 @@ +use astria_core::protocol::transaction::v1::action::Transfer; +use astria_eyre::eyre::{ + ensure, + Result, + WrapErr as _, +}; +use async_trait::async_trait; +use cnidarium::StateWrite; + +use crate::{ + action_handler::{ + check_transfer, + execute_transfer, + ActionHandler, + }, + bridge::StateReadExt as _, + transaction::StateReadExt as _, +}; + +#[async_trait] +impl ActionHandler for Transfer { + async fn check_stateless(&self) -> Result<()> { + Ok(()) + } + + async fn check_and_execute(&self, state: S) -> Result<()> { + let from = state + .get_transaction_context() + .expect("transaction source must be present in state when executing an action") + .address_bytes(); + + ensure!( + state + .get_bridge_account_rollup_id(&from) + .await + .wrap_err("failed to get bridge account rollup id")? + .is_none(), + "cannot transfer out of bridge account; BridgeUnlock must be used", + ); + + check_transfer(self, &from, &state).await?; + execute_transfer(self, &from, state).await?; + + Ok(()) + } +} diff --git a/crates/astria-sequencer/src/action_handler/impls/validator_update.rs b/crates/astria-sequencer/src/action_handler/impls/validator_update.rs new file mode 100644 index 0000000000..4bd0d72c3c --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/impls/validator_update.rs @@ -0,0 +1,67 @@ +use astria_core::protocol::transaction::v1::action::ValidatorUpdate; +use astria_eyre::eyre::{ + bail, + ensure, + Result, + WrapErr as _, +}; +use async_trait::async_trait; +use cnidarium::StateWrite; + +use crate::{ + action_handler::ActionHandler, + authority::{ + StateReadExt as _, + StateWriteExt as _, + }, + transaction::StateReadExt as _, +}; + +#[async_trait] +impl ActionHandler for ValidatorUpdate { + async fn check_stateless(&self) -> Result<()> { + Ok(()) + } + + async fn check_and_execute(&self, mut state: S) -> Result<()> { + let from = state + .get_transaction_context() + .expect("transaction source must be present in state when executing an action") + .address_bytes(); + // ensure signer is the valid `sudo` key in state + let sudo_address = state + .get_sudo_address() + .await + .wrap_err("failed to get sudo address from state")?; + ensure!(sudo_address == from, "signer is not the sudo key"); + + // ensure that we're not removing the last validator or a validator + // that doesn't exist, these both cause issues in cometBFT + if self.power == 0 { + let validator_set = state + .get_validator_set() + .await + .wrap_err("failed to get validator set from state")?; + // check that validator exists + if validator_set + .get(self.verification_key.address_bytes()) + .is_none() + { + bail!("cannot remove a non-existing validator"); + } + // check that this is not the only validator, cannot remove the last one + ensure!(validator_set.len() != 1, "cannot remove the last validator"); + } + + // add validator update in non-consensus state to be used in end_block + let mut validator_updates = state + .get_validator_updates() + .await + .wrap_err("failed getting validator updates from state")?; + validator_updates.push_update(self.clone()); + state + .put_validator_updates(validator_updates) + .wrap_err("failed to put validator updates in state")?; + Ok(()) + } +} diff --git a/crates/astria-sequencer/src/action_handler/mod.rs b/crates/astria-sequencer/src/action_handler/mod.rs new file mode 100644 index 0000000000..09bde22b0e --- /dev/null +++ b/crates/astria-sequencer/src/action_handler/mod.rs @@ -0,0 +1,94 @@ +//! Contains the `ActionHandler` trait, which houses all stateless/stateful checks and execution, as +//! well as all of its implementations. + +use astria_core::protocol::transaction::v1::action::Transfer; +use astria_eyre::eyre::{ + ensure, + Result, + WrapErr as _, +}; +use cnidarium::{ + StateRead, + StateWrite, +}; + +use crate::{ + accounts::{ + AddressBytes, + StateReadExt as _, + StateWriteExt as _, + }, + address::StateReadExt as _, +}; + +pub(crate) mod impls; + +/// This trait is a verbatim copy of `cnidarium_component::ActionHandler`. +/// +/// It's duplicated here because all actions are foreign types, forbidding +/// the implementation of [`cnidarium_component::ActionHandler`][1] for +/// these types due to Rust orphan rules. +/// +/// [1]: https://github.com/penumbra-zone/penumbra/blob/14959350abcb8cfbf33f9aedc7463fccfd8e3f9f/crates/cnidarium-component/src/action_handler.rs#L30 +#[async_trait::async_trait] +pub(crate) trait ActionHandler { + // Commenting out for the time being as this is currently not being used. Leaving this in + // for reference as this is copied from cnidarium_component. + // ``` + // type CheckStatelessContext: Clone + Send + Sync + 'static; + // async fn check_stateless(&self, context: Self::CheckStatelessContext) -> anyhow::Result<()>; + // async fn check_historical(&self, _state: Arc) -> anyhow::Result<()> { + // Ok(()) + // } + // ``` + + async fn check_stateless(&self) -> astria_eyre::eyre::Result<()>; + + async fn check_and_execute(&self, mut state: S) + -> astria_eyre::eyre::Result<()>; +} + +async fn execute_transfer( + action: &Transfer, + from: &TAddress, + mut state: S, +) -> Result<()> +where + S: StateWrite, + TAddress: AddressBytes, +{ + let from = from.address_bytes(); + state + .decrease_balance(from, &action.asset, action.amount) + .await + .wrap_err("failed decreasing `from` account balance")?; + state + .increase_balance(&action.to, &action.asset, action.amount) + .await + .wrap_err("failed increasing `to` account balance")?; + + Ok(()) +} + +async fn check_transfer(action: &Transfer, from: &TAddress, state: &S) -> Result<()> +where + S: StateRead, + TAddress: AddressBytes, +{ + state.ensure_base_prefix(&action.to).await.wrap_err( + "failed ensuring that the destination address matches the permitted base prefix", + )?; + + let transfer_asset = &action.asset; + + let from_transfer_balance = state + .get_account_balance(from, transfer_asset) + .await + .wrap_err("failed to get account balance in transfer check")?; + ensure!( + from_transfer_balance >= action.amount, + "insufficient funds for transfer" + ); + + Ok(()) +} diff --git a/crates/astria-sequencer/src/app/action_handler.rs b/crates/astria-sequencer/src/app/action_handler.rs deleted file mode 100644 index 1ef67ea4bb..0000000000 --- a/crates/astria-sequencer/src/app/action_handler.rs +++ /dev/null @@ -1,26 +0,0 @@ -use cnidarium::StateWrite; - -/// This trait is a verbatim copy of `cnidarium_component::ActionHandler`. -/// -/// It's duplicated here because all actions are foreign types, forbidding -/// the implementation of [`cnidarium_component::ActionHandler`][1] for -/// these types due to Rust orphan rules. -/// -/// [1]: https://github.com/penumbra-zone/penumbra/blob/14959350abcb8cfbf33f9aedc7463fccfd8e3f9f/crates/cnidarium-component/src/action_handler.rs#L30 -#[async_trait::async_trait] -pub(crate) trait ActionHandler { - // Commenting out for the time being as this is currently not being used. Leaving this in - // for reference as this is copied from cnidarium_component. - // ``` - // type CheckStatelessContext: Clone + Send + Sync + 'static; - // async fn check_stateless(&self, context: Self::CheckStatelessContext) -> anyhow::Result<()>; - // async fn check_historical(&self, _state: Arc) -> anyhow::Result<()> { - // Ok(()) - // } - // ``` - - async fn check_stateless(&self) -> astria_eyre::eyre::Result<()>; - - async fn check_and_execute(&self, mut state: S) - -> astria_eyre::eyre::Result<()>; -} diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index e116664a79..55ecf9a276 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -1,4 +1,3 @@ -mod action_handler; #[cfg(any(test, feature = "benchmark"))] pub(crate) mod benchmark_and_test_utils; #[cfg(feature = "benchmark")] @@ -82,18 +81,19 @@ use tracing::{ instrument, }; -pub(crate) use self::{ - action_handler::ActionHandler, - state_ext::{ - StateReadExt, - StateWriteExt, - }, +pub(crate) use self::state_ext::{ + StateReadExt, + StateWriteExt, }; use crate::{ accounts::{ component::AccountsComponent, StateWriteExt as _, }, + action_handler::{ + impls::transaction::InvalidNonce, + ActionHandler as _, + }, address::StateWriteExt as _, assets::StateWriteExt as _, authority::{ @@ -127,7 +127,6 @@ use crate::{ GeneratedCommitments, }, }, - transaction::InvalidNonce, }; // ephemeral store key for the cache of results of executing of transactions in `prepare_proposal`. diff --git a/crates/astria-sequencer/src/app/tests_block_fees.rs b/crates/astria-sequencer/src/app/tests_block_fees.rs deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index b4785380b3..42443de0c1 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -40,6 +40,10 @@ use cnidarium::{ use super::test_utils::get_alice_signing_key; use crate::{ accounts::StateReadExt as _, + action_handler::{ + impls::transaction::InvalidChainId, + ActionHandler as _, + }, app::{ benchmark_and_test_utils::{ BOB_ADDRESS, @@ -49,7 +53,7 @@ use crate::{ get_bridge_signing_key, initialize_app, }, - ActionHandler as _, + InvalidNonce, }, authority::StateReadExt as _, benchmark_and_test_utils::{ @@ -69,10 +73,6 @@ use crate::{ }, ibc::StateReadExt as _, test_utils::calculate_rollup_data_submission_fee_from_state, - transaction::{ - InvalidChainId, - InvalidNonce, - }, utils::create_deposit_event, }; diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs deleted file mode 100644 index 78d69733eb..0000000000 --- a/crates/astria-sequencer/src/authority/action.rs +++ /dev/null @@ -1,130 +0,0 @@ -use astria_core::protocol::transaction::v1::action::{ - IbcSudoChange, - SudoAddressChange, - ValidatorUpdate, -}; -use astria_eyre::eyre::{ - bail, - ensure, - Result, - WrapErr as _, -}; -use cnidarium::StateWrite; - -use crate::{ - address::StateReadExt as _, - app::ActionHandler, - authority::{ - StateReadExt as _, - StateWriteExt as _, - }, - ibc::StateWriteExt as _, - transaction::StateReadExt as _, -}; - -#[async_trait::async_trait] -impl ActionHandler for ValidatorUpdate { - async fn check_stateless(&self) -> Result<()> { - Ok(()) - } - - async fn check_and_execute(&self, mut state: S) -> Result<()> { - let from = state - .get_transaction_context() - .expect("transaction source must be present in state when executing an action") - .address_bytes(); - // ensure signer is the valid `sudo` key in state - let sudo_address = state - .get_sudo_address() - .await - .wrap_err("failed to get sudo address from state")?; - ensure!(sudo_address == from, "signer is not the sudo key"); - - // ensure that we're not removing the last validator or a validator - // that doesn't exist, these both cause issues in cometBFT - if self.power == 0 { - let validator_set = state - .get_validator_set() - .await - .wrap_err("failed to get validator set from state")?; - // check that validator exists - if validator_set - .get(self.verification_key.address_bytes()) - .is_none() - { - bail!("cannot remove a non-existing validator"); - } - // check that this is not the only validator, cannot remove the last one - ensure!(validator_set.len() != 1, "cannot remove the last validator"); - } - - // add validator update in non-consensus state to be used in end_block - let mut validator_updates = state - .get_validator_updates() - .await - .wrap_err("failed getting validator updates from state")?; - validator_updates.push_update(self.clone()); - state - .put_validator_updates(validator_updates) - .wrap_err("failed to put validator updates in state")?; - Ok(()) - } -} - -#[async_trait::async_trait] -impl ActionHandler for SudoAddressChange { - async fn check_stateless(&self) -> Result<()> { - Ok(()) - } - - /// check that the signer of the transaction is the current sudo address, - /// as only that address can change the sudo address - async fn check_and_execute(&self, mut state: S) -> Result<()> { - let from = state - .get_transaction_context() - .expect("transaction source must be present in state when executing an action") - .address_bytes(); - state - .ensure_base_prefix(&self.new_address) - .await - .wrap_err("desired new sudo address has an unsupported prefix")?; - // ensure signer is the valid `sudo` key in state - let sudo_address = state - .get_sudo_address() - .await - .wrap_err("failed to get sudo address from state")?; - ensure!(sudo_address == from, "signer is not the sudo key"); - state - .put_sudo_address(self.new_address) - .wrap_err("failed to put sudo address in state")?; - Ok(()) - } -} - -#[async_trait::async_trait] -impl ActionHandler for IbcSudoChange { - async fn check_stateless(&self) -> Result<()> { - Ok(()) - } - - async fn check_and_execute(&self, mut state: S) -> Result<()> { - let from = state - .get_transaction_context() - .expect("transaction source must be present in state when executing an action") - .address_bytes(); - state - .ensure_base_prefix(&self.new_address) - .await - .wrap_err("desired new ibc sudo address has an unsupported prefix")?; - // ensure signer is the valid `sudo` key in state - let sudo_address = state - .get_sudo_address() - .await - .wrap_err("failed to get sudo address from state")?; - ensure!(sudo_address == from, "signer is not the sudo key"); - state - .put_ibc_sudo_address(self.new_address) - .wrap_err("failed to put ibc sudo address in state")?; - Ok(()) - } -} diff --git a/crates/astria-sequencer/src/authority/mod.rs b/crates/astria-sequencer/src/authority/mod.rs index 22d8a00784..0431f4295d 100644 --- a/crates/astria-sequencer/src/authority/mod.rs +++ b/crates/astria-sequencer/src/authority/mod.rs @@ -1,4 +1,3 @@ -mod action; pub(crate) mod component; mod state_ext; pub(crate) mod storage; diff --git a/crates/astria-sequencer/src/bridge/mod.rs b/crates/astria-sequencer/src/bridge/mod.rs index 5aec65349a..8531007ab1 100644 --- a/crates/astria-sequencer/src/bridge/mod.rs +++ b/crates/astria-sequencer/src/bridge/mod.rs @@ -1,7 +1,3 @@ -mod bridge_lock_action; -mod bridge_sudo_change_action; -mod bridge_unlock_action; -pub(crate) mod init_bridge_account_action; pub(crate) mod query; mod state_ext; pub(crate) mod storage; diff --git a/crates/astria-sequencer/src/fees/mod.rs b/crates/astria-sequencer/src/fees/mod.rs index 224073148e..195ad7be47 100644 --- a/crates/astria-sequencer/src/fees/mod.rs +++ b/crates/astria-sequencer/src/fees/mod.rs @@ -37,7 +37,6 @@ use crate::{ transaction::StateReadExt as _, }; -pub(crate) mod action; pub(crate) mod component; pub(crate) mod query; mod state_ext; diff --git a/crates/astria-sequencer/src/fees/tests.rs b/crates/astria-sequencer/src/fees/tests.rs index dd0741bbd3..41263dd024 100644 --- a/crates/astria-sequencer/src/fees/tests.rs +++ b/crates/astria-sequencer/src/fees/tests.rs @@ -37,6 +37,7 @@ use cnidarium::StateDelta; use super::base_deposit_fee; use crate::{ accounts::StateWriteExt as _, + action_handler::ActionHandler as _, address::StateWriteExt as _, app::{ benchmark_and_test_utils::{ @@ -47,7 +48,6 @@ use crate::{ get_alice_signing_key, get_bridge_signing_key, }, - ActionHandler as _, }, benchmark_and_test_utils::{ assert_eyre_error, diff --git a/crates/astria-sequencer/src/ibc/mod.rs b/crates/astria-sequencer/src/ibc/mod.rs index 5a94d8b536..b0f37b19e1 100644 --- a/crates/astria-sequencer/src/ibc/mod.rs +++ b/crates/astria-sequencer/src/ibc/mod.rs @@ -1,8 +1,6 @@ pub(crate) mod component; pub(crate) mod host_interface; -pub(crate) mod ibc_relayer_change; pub(crate) mod ics20_transfer; -pub(crate) mod ics20_withdrawal; mod state_ext; pub(crate) mod storage; diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index 8701ee6a72..76596e40eb 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -1,4 +1,5 @@ pub(crate) mod accounts; +pub(crate) mod action_handler; pub(crate) mod address; pub(crate) mod app; pub(crate) mod assets; @@ -17,7 +18,6 @@ pub(crate) mod ibc; mod mempool; pub(crate) mod metrics; pub(crate) mod proposal; -pub(crate) mod rollup_data; mod sequencer; pub(crate) mod service; pub(crate) mod storage; diff --git a/crates/astria-sequencer/src/rollup_data/mod.rs b/crates/astria-sequencer/src/rollup_data/mod.rs deleted file mode 100644 index f04dbbc6c9..0000000000 --- a/crates/astria-sequencer/src/rollup_data/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub(crate) mod action; diff --git a/crates/astria-sequencer/src/service/mempool/mod.rs b/crates/astria-sequencer/src/service/mempool/mod.rs index 88b59a5b77..ad2b76d9fc 100644 --- a/crates/astria-sequencer/src/service/mempool/mod.rs +++ b/crates/astria-sequencer/src/service/mempool/mod.rs @@ -50,8 +50,8 @@ use tracing::{ use crate::{ accounts::StateReadExt as _, + action_handler::ActionHandler as _, address::StateReadExt as _, - app::ActionHandler as _, mempool::{ get_account_balances, InsertionError, diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index 7164769b33..fbfe3e3fb6 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -1,27 +1,11 @@ mod checks; mod state_ext; -use std::fmt; - -use astria_core::protocol::transaction::v1::{ - action::Action, - Transaction, -}; -use astria_eyre::{ - anyhow_to_eyre, - eyre::{ - ensure, - OptionExt as _, - Result, - WrapErr as _, - }, -}; pub(crate) use checks::{ check_balance_for_total_fees_and_transfers, check_chain_id_mempool, get_total_transaction_cost, }; -use cnidarium::StateWrite; // Conditional to quiet warnings. This object is used throughout the codebase, // but is never explicitly named - hence Rust warns about it being unused. #[cfg(test)] @@ -30,266 +14,3 @@ pub(crate) use state_ext::{ StateReadExt, StateWriteExt, }; - -use crate::{ - accounts::{ - StateReadExt as _, - StateWriteExt as _, - }, - app::{ - ActionHandler, - StateReadExt as _, - }, - bridge::{ - StateReadExt as _, - StateWriteExt as _, - }, - fees::FeeHandler, - ibc::{ - host_interface::AstriaHost, - StateReadExt as _, - }, -}; - -#[derive(Debug)] -pub(crate) struct InvalidChainId(pub(crate) String); - -impl fmt::Display for InvalidChainId { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "provided chain id {} does not match expected chain id", - self.0, - ) - } -} - -impl std::error::Error for InvalidChainId {} - -#[derive(Debug)] -pub(crate) struct InvalidNonce(pub(crate) u32); - -impl fmt::Display for InvalidNonce { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "provided nonce {} does not match expected next nonce", - self.0, - ) - } -} - -impl std::error::Error for InvalidNonce {} - -#[async_trait::async_trait] -impl ActionHandler for Transaction { - async fn check_stateless(&self) -> Result<()> { - ensure!(!self.actions().is_empty(), "must have at least one action"); - - for action in self.actions() { - match action { - Action::Transfer(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for TransferAction")?, - Action::RollupDataSubmission(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for SequenceAction")?, - Action::ValidatorUpdate(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for ValidatorUpdateAction")?, - Action::SudoAddressChange(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for SudoAddressChangeAction")?, - Action::IbcSudoChange(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for IbcSudoChangeAction")?, - Action::FeeChange(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for FeeChangeAction")?, - Action::Ibc(act) => { - let action = act - .clone() - .with_handler::(); - action - .check_stateless(()) - .await - .map_err(anyhow_to_eyre) - .wrap_err("stateless check failed for IbcAction")?; - } - Action::Ics20Withdrawal(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for Ics20WithdrawalAction")?, - Action::IbcRelayerChange(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for IbcRelayerChangeAction")?, - Action::FeeAssetChange(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for FeeAssetChangeAction")?, - Action::InitBridgeAccount(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for InitBridgeAccountAction")?, - Action::BridgeLock(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for BridgeLockAction")?, - Action::BridgeUnlock(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for BridgeUnlockAction")?, - Action::BridgeSudoChange(act) => act - .check_stateless() - .await - .wrap_err("stateless check failed for BridgeSudoChangeAction")?, - } - } - Ok(()) - } - - // FIXME (https://github.com/astriaorg/astria/issues/1584): because most lines come from delegating (and error wrapping) to the - // individual actions. This could be tidied up by implementing `ActionHandler for Action` - // and letting it delegate. - #[expect(clippy::too_many_lines, reason = "should be refactored")] - async fn check_and_execute(&self, mut state: S) -> Result<()> { - // Add the current signed transaction into the ephemeral state in case - // downstream actions require access to it. - // XXX: This must be deleted at the end of `check_stateful`. - let mut transaction_context = state.put_transaction_context(self); - - // Transactions must match the chain id of the node. - let chain_id = state.get_chain_id().await?; - ensure!( - self.chain_id() == chain_id.as_str(), - InvalidChainId(self.chain_id().to_string()) - ); - - // Nonce should be equal to the number of executed transactions before this tx. - // First tx has nonce 0. - let curr_nonce = state - .get_account_nonce(self.address_bytes()) - .await - .wrap_err("failed to get nonce for transaction signer")?; - ensure!(curr_nonce == self.nonce(), InvalidNonce(self.nonce())); - - // Should have enough balance to cover all actions. - check_balance_for_total_fees_and_transfers(self, &state) - .await - .wrap_err("failed to check balance for total fees and transfers")?; - - if state - .get_bridge_account_rollup_id(&self) - .await - .wrap_err("failed to check account rollup id")? - .is_some() - { - state - .put_last_transaction_id_for_bridge_account( - &self, - transaction_context.transaction_id, - ) - .wrap_err("failed to put last transaction id for bridge account")?; - } - - let from_nonce = state - .get_account_nonce(&self) - .await - .wrap_err("failed getting nonce of transaction signer")?; - let next_nonce = from_nonce - .checked_add(1) - .ok_or_eyre("overflow occurred incrementing stored nonce")?; - state - .put_account_nonce(&self, next_nonce) - .wrap_err("failed updating `from` nonce")?; - - // FIXME: this should create one span per `check_and_execute` - for (i, action) in (0..).zip(self.actions().iter()) { - transaction_context.position_in_transaction = i; - state.put_transaction_context(transaction_context); - - match action { - Action::Transfer(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("executing transfer action failed")?, - Action::RollupDataSubmission(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("executing sequence action failed")?, - Action::ValidatorUpdate(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("executing validor update")?, - Action::SudoAddressChange(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("executing sudo address change failed")?, - Action::IbcSudoChange(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("executing ibc sudo change failed")?, - Action::FeeChange(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("executing fee change failed")?, - Action::Ibc(act) => { - // FIXME: this check should be moved to check_and_execute, as it now has - // access to the the signer through state. However, what's the correct - // ibc AppHandler call to do it? Can we just update one of the trait methods - // of crate::ibc::ics20_transfer::Ics20Transfer? - ensure!( - state - .is_ibc_relayer(self) - .await - .wrap_err("failed to check if address is IBC relayer")?, - "only IBC sudo address can execute IBC actions" - ); - let action = act - .clone() - .with_handler::(); - action - .check_and_execute(&mut state) - .await - .map_err(anyhow_to_eyre) - .wrap_err("failed executing ibc action")?; - } - Action::Ics20Withdrawal(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing ics20 withdrawal")?, - Action::IbcRelayerChange(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing ibc relayer change")?, - Action::FeeAssetChange(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing fee asseet change")?, - Action::InitBridgeAccount(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing init bridge account")?, - Action::BridgeLock(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing bridge lock")?, - Action::BridgeUnlock(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing bridge unlock")?, - Action::BridgeSudoChange(act) => check_execute_and_pay_fees(act, &mut state) - .await - .wrap_err("failed executing bridge sudo change")?, - } - } - - // XXX: Delete the current transaction data from the ephemeral state. - state.delete_current_transaction_context(); - Ok(()) - } -} - -async fn check_execute_and_pay_fees( - action: &T, - mut state: S, -) -> Result<()> { - action.check_and_execute(&mut state).await?; - action.check_and_pay_fees(&mut state).await?; - Ok(()) -}