From 8171eed2934bec693ed462b3be0f52f7df2f45ef Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 1 Aug 2024 18:32:42 +0200 Subject: [PATCH 1/3] refactor(sequencer): move asset state methods to asset module (#1313) ## Summary Moved methods for manipulating state assets to the state asset module. ## Background Sequencer uses the convention of having state setters and writers as methods in a trait `::StateReadExt` and `::StateWriteExt`. For example, a sudo address (which lives in the authority module) can be read with `authority::StateReadExt::get_sudo_address`. The exception to this are the methods for manipulating assets, which were under `astria_sequencer::state_ext::StateExt`. This is because the asset module was just recently introduced. ## Changes No business logic was changed in this patch! - Rename `crate::asset -> crate::assets` - Move all methods for reading and writing assets from `astria_sequencer::state_ext::State{Read,Write}Ext` to `astria_sequencer::assets::State{Read,Write}Ext`. - Reexport `State{Read,Write}Ext` from all parent modules so that one can write `use ::StateReadExt` (and similar for write) instead of `use ::state_ext::StateReadExt`. ## Testing No tests were changed (minus import paths). Asset-related tests were moved (including snapshot tests) ## Related Issues Closes #1312 --- .../astria-sequencer/src/accounts/action.rs | 27 +- .../src/accounts/component.rs | 2 +- crates/astria-sequencer/src/accounts/mod.rs | 7 +- .../src/accounts/state_ext.rs | 42 +- crates/astria-sequencer/src/app/mod.rs | 46 +- crates/astria-sequencer/src/app/tests_app.rs | 20 +- .../src/app/tests_breaking_changes.rs | 4 +- .../src/app/tests_execute_transaction.rs | 22 +- .../astria-sequencer/src/asset/state_ext.rs | 228 ------ .../src/{asset => assets}/mod.rs | 6 +- .../src/{asset => assets}/query.rs | 2 +- ..._tests__storage_keys_are_unchanged-2.snap} | 2 +- ..._tests__storage_keys_are_unchanged-3.snap} | 2 +- ...t__tests__storage_keys_are_unchanged.snap} | 2 +- .../astria-sequencer/src/assets/state_ext.rs | 649 ++++++++++++++++++ .../astria-sequencer/src/authority/action.rs | 18 +- .../src/authority/component.rs | 2 +- crates/astria-sequencer/src/authority/mod.rs | 90 ++- .../src/authority/state_ext.rs | 91 +-- .../src/bridge/bridge_lock_action.rs | 13 +- .../src/bridge/bridge_sudo_change_action.rs | 4 +- .../src/bridge/bridge_unlock_action.rs | 8 +- .../src/bridge/init_bridge_account_action.rs | 3 +- crates/astria-sequencer/src/bridge/mod.rs | 6 +- crates/astria-sequencer/src/bridge/query.rs | 8 +- .../astria-sequencer/src/fee_asset_change.rs | 28 +- crates/astria-sequencer/src/grpc/sequencer.rs | 4 +- .../src/ibc/ics20_transfer.rs | 27 +- .../src/ibc/ics20_withdrawal.rs | 8 +- crates/astria-sequencer/src/ibc/mod.rs | 7 +- crates/astria-sequencer/src/lib.rs | 2 +- .../src/proposal/commitment.rs | 2 +- .../astria-sequencer/src/sequence/action.rs | 24 +- crates/astria-sequencer/src/sequence/mod.rs | 6 +- crates/astria-sequencer/src/sequencer.rs | 6 +- .../astria-sequencer/src/service/consensus.rs | 2 +- .../astria-sequencer/src/service/info/mod.rs | 14 +- .../astria-sequencer/src/service/mempool.rs | 2 +- crates/astria-sequencer/src/state_ext.rs | 435 +----------- .../src/transaction/checks.rs | 18 +- .../astria-sequencer/src/transaction/mod.rs | 6 +- .../astria-sequencer/src/transaction/query.rs | 2 +- 42 files changed, 960 insertions(+), 937 deletions(-) delete mode 100644 crates/astria-sequencer/src/asset/state_ext.rs rename crates/astria-sequencer/src/{asset => assets}/mod.rs (94%) rename crates/astria-sequencer/src/{asset => assets}/query.rs (99%) rename crates/astria-sequencer/src/{snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap => assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-2.snap} (69%) rename crates/astria-sequencer/src/{snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed-2.snap => assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-3.snap} (69%) rename crates/astria-sequencer/src/{asset/snapshots/astria_sequencer__asset__state_ext__tests__storage_keys_are_unchanged.snap => assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged.snap} (67%) create mode 100644 crates/astria-sequencer/src/assets/state_ext.rs diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index 4948df966b..051258cf84 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -11,23 +11,21 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts::state_ext::{ - StateReadExt, - StateWriteExt, - }, - bridge::state_ext::StateReadExt as _, - state_ext::{ - StateReadExt as _, - StateWriteExt as _, - }, + accounts, + accounts::StateReadExt as _, + assets, + bridge::StateReadExt as _, transaction::action_handler::ActionHandler, }; -pub(crate) async fn transfer_check_stateful( +pub(crate) async fn transfer_check_stateful( action: &TransferAction, state: &S, from: Address, -) -> Result<()> { +) -> Result<()> +where + S: accounts::StateReadExt + assets::StateReadExt + 'static, +{ ensure!( state .is_allowed_fee_asset(&action.fee_asset) @@ -87,7 +85,7 @@ impl ActionHandler for TransferAction { Ok(()) } - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, @@ -107,7 +105,10 @@ impl ActionHandler for TransferAction { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, from: Address) -> Result<()> { + async fn execute(&self, state: &mut S, from: Address) -> Result<()> + where + S: accounts::StateWriteExt + assets::StateWriteExt, + { let fee = state .get_transfer_base_fee() .await diff --git a/crates/astria-sequencer/src/accounts/component.rs b/crates/astria-sequencer/src/accounts/component.rs index 870a5a381c..2e4addba3f 100644 --- a/crates/astria-sequencer/src/accounts/component.rs +++ b/crates/astria-sequencer/src/accounts/component.rs @@ -12,7 +12,7 @@ use tracing::instrument; use super::state_ext::StateWriteExt; use crate::{ - asset::get_native_asset, + assets::get_native_asset, component::Component, }; diff --git a/crates/astria-sequencer/src/accounts/mod.rs b/crates/astria-sequencer/src/accounts/mod.rs index 3a22a65dd2..ae0b420808 100644 --- a/crates/astria-sequencer/src/accounts/mod.rs +++ b/crates/astria-sequencer/src/accounts/mod.rs @@ -1,4 +1,9 @@ pub(crate) mod action; pub(crate) mod component; pub(crate) mod query; -pub(crate) mod state_ext; +mod state_ext; + +pub(crate) use state_ext::{ + StateReadExt, + StateWriteExt, +}; diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index 34fc7edad6..a3d70710d5 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -67,7 +67,7 @@ fn nonce_storage_key(address: Address) -> String { pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] async fn get_account_balances(&self, address: Address) -> Result> { - use crate::asset::state_ext::StateReadExt as _; + use crate::assets::StateReadExt as _; let prefix = format!("{}/balance/", StorageKey(&address)); let mut balances: Vec = Vec::new(); @@ -94,7 +94,7 @@ pub(crate) trait StateReadExt: StateRead { let Balance(balance) = Balance::try_from_slice(&value).context("invalid balance bytes")?; - let native_asset = crate::asset::get_native_asset(); + let native_asset = crate::assets::get_native_asset(); if asset == native_asset.to_ibc_prefixed() { balances.push(AssetBalance { denom: native_asset.clone(), @@ -264,12 +264,9 @@ mod tests { StateReadExt as _, StateWriteExt as _, }; - use crate::{ - accounts::state_ext::{ - balance_storage_key, - nonce_storage_key, - }, - asset, + use crate::accounts::state_ext::{ + balance_storage_key, + nonce_storage_key, }; fn asset_0() -> astria_core::primitive::v1::asset::Denom { @@ -565,33 +562,28 @@ mod tests { #[tokio::test] async fn get_account_balances() { + use crate::assets::StateWriteExt as _; let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); // need to set native asset in order to use `get_account_balances()` - crate::asset::initialize_native_asset("nria"); + crate::assets::initialize_native_asset("nria"); - let asset_0 = crate::asset::get_native_asset(); + let asset_0 = crate::assets::get_native_asset(); let asset_1 = asset_1(); let asset_2 = asset_2(); // also need to add assets to the ibc state - asset::state_ext::StateWriteExt::put_ibc_asset( - &mut state, - &asset_0.clone().unwrap_trace_prefixed(), - ) - .expect("should be able to call other trait method on state object"); - asset::state_ext::StateWriteExt::put_ibc_asset( - &mut state, - &asset_1.clone().unwrap_trace_prefixed(), - ) - .expect("should be able to call other trait method on state object"); - asset::state_ext::StateWriteExt::put_ibc_asset( - &mut state, - &asset_2.clone().unwrap_trace_prefixed(), - ) - .expect("should be able to call other trait method on state object"); + state + .put_ibc_asset(&asset_0.clone().unwrap_trace_prefixed()) + .expect("should be able to call other trait method on state object"); + state + .put_ibc_asset(&asset_1.clone().unwrap_trace_prefixed()) + .expect("should be able to call other trait method on state object"); + state + .put_ibc_asset(&asset_2.clone().unwrap_trace_prefixed()) + .expect("should be able to call other trait method on state object"); // create needed variables let address = crate::address::base_prefixed([42u8; 20]); diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index 3946032dfa..6e052d0c1d 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -63,32 +63,29 @@ use tracing::{ }; use crate::{ + accounts, accounts::{ component::AccountsComponent, - state_ext::{ - StateReadExt, - StateWriteExt as _, - }, + StateWriteExt as _, }, address::StateWriteExt as _, api_state_ext::StateWriteExt as _, - asset::state_ext::StateWriteExt as _, + assets::{ + StateReadExt as _, + StateWriteExt as _, + }, authority::{ component::{ AuthorityComponent, AuthorityComponentAppState, }, - state_ext::{ - StateReadExt as _, - StateWriteExt as _, - }, + StateReadExt as _, + StateWriteExt as _, }, bridge::{ component::BridgeComponent, - state_ext::{ - StateReadExt as _, - StateWriteExt, - }, + StateReadExt as _, + StateWriteExt as _, }, component::Component as _, ibc::component::IbcComponent, @@ -223,17 +220,18 @@ impl App { .context("failed setting global base prefix")?; state_tx.put_base_prefix(&genesis_state.address_prefixes().base); - crate::asset::initialize_native_asset(genesis_state.native_asset_base_denomination()); - let native_asset = crate::asset::get_native_asset(); - if let Some(trace_native_asset) = native_asset.as_trace_prefixed() { - state_tx - .put_ibc_asset(trace_native_asset) - .context("failed to put native asset")?; - } else { - bail!("native asset must not be in ibc/ form") - } + crate::assets::initialize_native_asset(genesis_state.native_asset_base_denomination()); + let Some(native_asset) = crate::assets::get_native_asset() + .as_trace_prefixed() + .cloned() + else { + bail!("native asset must be trace-prefixed, not of form `ibc/`") + }; + state_tx + .put_ibc_asset(&native_asset) + .context("failed to put native asset")?; - state_tx.put_native_asset_denom(genesis_state.native_asset_base_denomination()); + state_tx.put_native_asset(&native_asset); state_tx.put_chain_id_and_revision_number(chain_id.try_into().context("invalid chain ID")?); state_tx.put_block_height(0); @@ -1147,7 +1145,7 @@ impl App { // NOTE: this function locks the mempool until every tx has been checked. // this could potentially stall consensus from moving to the next round if // the mempool is large. -async fn update_mempool_after_finalization( +async fn update_mempool_after_finalization( mempool: &mut Mempool, state: S, ) -> anyhow::Result<()> { diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index 9b89547c05..c34047f79f 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -1,7 +1,10 @@ use std::collections::HashMap; use astria_core::{ - primitive::v1::RollupId, + primitive::v1::{ + asset::TracePrefixed, + RollupId, + }, protocol::transaction::v1alpha1::{ action::{ BridgeLockAction, @@ -36,17 +39,17 @@ use tendermint::{ use super::*; use crate::{ - accounts::state_ext::StateReadExt as _, + accounts::StateReadExt as _, app::test_utils::*, - asset::get_native_asset, - authority::state_ext::{ + assets::get_native_asset, + authority::{ StateReadExt as _, StateWriteExt as _, ValidatorSet, }, - bridge::state_ext::{ + bridge::{ StateReadExt as _, - StateWriteExt, + StateWriteExt as _, }, proposal::commitment::generate_rollup_datas_commitment, state_ext::StateReadExt as _, @@ -94,7 +97,10 @@ async fn app_genesis_and_init_chain() { ); } - assert_eq!(app.state.get_native_asset_denom().await.unwrap(), "nria"); + assert_eq!( + app.state.get_native_asset().await.unwrap(), + "nria".parse::().unwrap() + ); } #[tokio::test] diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 17c00e785a..209a6317e0 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -60,8 +60,8 @@ use crate::{ BOB_ADDRESS, CAROL_ADDRESS, }, - asset::get_native_asset, - bridge::state_ext::StateWriteExt as _, + assets::get_native_asset, + bridge::StateWriteExt as _, proposal::commitment::generate_rollup_datas_commitment, }; diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index fc65c2ce6e..a596f6d6e5 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -32,17 +32,19 @@ use penumbra_ibc::params::IBCParameters; use tendermint::abci::EventAttributeIndexExt as _; use crate::{ - accounts::state_ext::StateReadExt as _, + accounts::StateReadExt as _, app::test_utils::*, - asset::get_native_asset, - authority::state_ext::StateReadExt as _, - bridge::state_ext::{ + assets::{ + get_native_asset, StateReadExt as _, - StateWriteExt, }, - ibc::state_ext::StateReadExt as _, + authority::StateReadExt as _, + bridge::{ + StateReadExt as _, + StateWriteExt as _, + }, + ibc::StateReadExt as _, sequence::calculate_fee_from_state, - state_ext::StateReadExt as _, transaction::{ InvalidChainId, InvalidNonce, @@ -130,7 +132,7 @@ async fn app_execute_transaction_transfer() { #[tokio::test] async fn app_execute_transaction_transfer_not_native_token() { - use crate::accounts::state_ext::StateWriteExt as _; + use crate::accounts::StateWriteExt as _; let mut app = initialize_app(None, vec![]).await; @@ -240,7 +242,7 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { #[tokio::test] async fn app_execute_transaction_sequence() { - use crate::sequence::state_ext::StateWriteExt as _; + use crate::sequence::StateWriteExt as _; let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); @@ -967,7 +969,7 @@ async fn app_stateful_check_fails_insufficient_total_balance() { #[tokio::test] async fn app_execute_transaction_bridge_lock_unlock_action_ok() { - use crate::accounts::state_ext::StateWriteExt as _; + use crate::accounts::StateWriteExt as _; let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); let mut app = initialize_app(None, vec![]).await; diff --git a/crates/astria-sequencer/src/asset/state_ext.rs b/crates/astria-sequencer/src/asset/state_ext.rs deleted file mode 100644 index 84a0e37601..0000000000 --- a/crates/astria-sequencer/src/asset/state_ext.rs +++ /dev/null @@ -1,228 +0,0 @@ -use anyhow::{ - Context as _, - Result, -}; -use astria_core::primitive::v1::{ - asset, - asset::denom, -}; -use async_trait::async_trait; -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; -use cnidarium::{ - StateRead, - StateWrite, -}; -use tracing::instrument; - -/// Newtype wrapper to read and write a denomination trace from rocksdb. -#[derive(BorshSerialize, BorshDeserialize, Debug)] -struct DenominationTrace(String); - -fn asset_storage_key>(asset: TAsset) -> String { - format!("asset/{}", crate::storage_keys::hunks::Asset::from(asset)) -} - -#[async_trait] -pub(crate) trait StateReadExt: StateRead { - #[instrument(skip_all)] - async fn has_ibc_asset(&self, asset: TAsset) -> Result - where - TAsset: Into + std::fmt::Display + Send, - { - Ok(self - .get_raw(&asset_storage_key(asset)) - .await - .context("failed reading raw asset from state")? - .is_some()) - } - - #[instrument(skip_all)] - async fn map_ibc_to_trace_prefixed_asset( - &self, - asset: asset::IbcPrefixed, - ) -> Result> { - let Some(bytes) = self - .get_raw(&asset_storage_key(asset)) - .await - .context("failed reading raw asset from state")? - else { - return Ok(None); - }; - - let DenominationTrace(denom_str) = - DenominationTrace::try_from_slice(&bytes).context("invalid asset bytes")?; - let denom = denom_str - .parse() - .context("failed to parse retrieved denom string as a Denom")?; - Ok(Some(denom)) - } -} - -impl StateReadExt for T {} - -#[async_trait] -pub(crate) trait StateWriteExt: StateWrite { - #[instrument(skip_all)] - fn put_ibc_asset(&mut self, asset: &denom::TracePrefixed) -> Result<()> { - let bytes = borsh::to_vec(&DenominationTrace(asset.to_string())) - .context("failed to serialize asset")?; - self.put_raw(asset_storage_key(asset), bytes); - Ok(()) - } -} - -impl StateWriteExt for T {} - -#[cfg(test)] -mod tests { - use astria_core::primitive::v1::asset; - use cnidarium::StateDelta; - - use super::{ - asset_storage_key, - StateReadExt, - StateWriteExt as _, - }; - - fn asset() -> asset::Denom { - "asset".parse().unwrap() - } - fn asset_0() -> asset::Denom { - "asset_0".parse().unwrap() - } - fn asset_1() -> asset::Denom { - "asset_1".parse().unwrap() - } - - #[tokio::test] - async fn get_ibc_asset_non_existent() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let state = StateDelta::new(snapshot); - - let asset = asset(); - - // gets for non existing assets should return none - assert_eq!( - state - .map_ibc_to_trace_prefixed_asset(asset.to_ibc_prefixed()) - .await - .expect("getting non existing asset should not fail"), - None - ); - } - - #[tokio::test] - async fn has_ibc_asset() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - let denom = asset(); - - // non existing calls are ok for 'has' - assert!( - !state - .has_ibc_asset(&denom) - .await - .expect("'has' for non existing ibc assets should be ok"), - "query for non existing asset should return false" - ); - - state - .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) - .expect("putting ibc asset should not fail"); - - // existing calls are ok for 'has' - assert!( - state - .has_ibc_asset(&denom) - .await - .expect("'has' for existing ibc assets should be ok"), - "query for existing asset should return true" - ); - } - - #[tokio::test] - async fn put_ibc_asset_simple() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // can write new - let denom = asset(); - state - .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) - .expect("putting ibc asset should not fail"); - assert_eq!( - state - .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) - .await - .unwrap() - .expect("an ibc asset was written and must exist inside the database"), - denom.unwrap_trace_prefixed(), - "stored ibc asset was not what was expected" - ); - } - - #[tokio::test] - async fn put_ibc_asset_complex() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // can write new - let denom = asset_0(); - state - .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) - .expect("putting ibc asset should not fail"); - assert_eq!( - state - .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) - .await - .unwrap() - .expect("an ibc asset was written and must exist inside the database"), - denom.clone().unwrap_trace_prefixed(), - "stored ibc asset was not what was expected" - ); - - // can write another without affecting original - let denom_1 = asset_1(); - state - .put_ibc_asset(&denom_1.clone().unwrap_trace_prefixed()) - .expect("putting ibc asset should not fail"); - assert_eq!( - state - .map_ibc_to_trace_prefixed_asset(denom_1.to_ibc_prefixed()) - .await - .unwrap() - .expect("an additional ibc asset was written and must exist inside the database"), - denom_1.unwrap_trace_prefixed(), - "additional ibc asset was not what was expected" - ); - assert_eq!( - state - .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) - .await - .unwrap() - .expect("an ibc asset was written and must exist inside the database"), - denom.clone().unwrap_trace_prefixed(), - "original ibc asset was not what was expected" - ); - } - - #[test] - fn storage_keys_are_unchanged() { - let asset = "an/asset/with/a/prefix" - .parse::() - .unwrap(); - assert_eq!( - asset_storage_key(&asset), - asset_storage_key(asset.to_ibc_prefixed()), - ); - insta::assert_snapshot!(asset_storage_key(asset)); - } -} diff --git a/crates/astria-sequencer/src/asset/mod.rs b/crates/astria-sequencer/src/assets/mod.rs similarity index 94% rename from crates/astria-sequencer/src/asset/mod.rs rename to crates/astria-sequencer/src/assets/mod.rs index 082844c10f..12468148fc 100644 --- a/crates/astria-sequencer/src/asset/mod.rs +++ b/crates/astria-sequencer/src/assets/mod.rs @@ -1,5 +1,5 @@ pub(crate) mod query; -pub(crate) mod state_ext; +mod state_ext; use std::sync::OnceLock; @@ -8,6 +8,10 @@ use astria_core::primitive::v1::asset::Denom; pub(crate) use intests::*; #[cfg(not(test))] pub(crate) use regular::*; +pub(crate) use state_ext::{ + StateReadExt, + StateWriteExt, +}; pub(crate) static NATIVE_ASSET: OnceLock = OnceLock::new(); diff --git a/crates/astria-sequencer/src/asset/query.rs b/crates/astria-sequencer/src/assets/query.rs similarity index 99% rename from crates/astria-sequencer/src/asset/query.rs rename to crates/astria-sequencer/src/assets/query.rs index 3d0dc65c9a..7628bab106 100644 --- a/crates/astria-sequencer/src/asset/query.rs +++ b/crates/astria-sequencer/src/assets/query.rs @@ -15,7 +15,7 @@ use tendermint::abci::{ }; use crate::{ - asset::state_ext::StateReadExt as _, + assets::StateReadExt as _, state_ext::StateReadExt as _, }; diff --git a/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap b/crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-2.snap similarity index 69% rename from crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap rename to crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-2.snap index 1ad4249a94..8c31993c79 100644 --- a/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap +++ b/crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-2.snap @@ -1,5 +1,5 @@ --- -source: crates/astria-sequencer/src/state_ext.rs +source: crates/astria-sequencer/src/assets/state_ext.rs expression: block_fees_key(&trace_prefixed) --- block_fees/ce072174ebc356c6ead681d61ab417ee72fdedd8155eb76478ece374bb04dc1d diff --git a/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed-2.snap b/crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-3.snap similarity index 69% rename from crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed-2.snap rename to crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-3.snap index a1d5962748..a3e273ee76 100644 --- a/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed-2.snap +++ b/crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged-3.snap @@ -1,5 +1,5 @@ --- -source: crates/astria-sequencer/src/state_ext.rs +source: crates/astria-sequencer/src/assets/state_ext.rs expression: fee_asset_key(trace_prefixed) --- fee_asset/ce072174ebc356c6ead681d61ab417ee72fdedd8155eb76478ece374bb04dc1d diff --git a/crates/astria-sequencer/src/asset/snapshots/astria_sequencer__asset__state_ext__tests__storage_keys_are_unchanged.snap b/crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged.snap similarity index 67% rename from crates/astria-sequencer/src/asset/snapshots/astria_sequencer__asset__state_ext__tests__storage_keys_are_unchanged.snap rename to crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged.snap index 67cbf77bca..3ad210d6a2 100644 --- a/crates/astria-sequencer/src/asset/snapshots/astria_sequencer__asset__state_ext__tests__storage_keys_are_unchanged.snap +++ b/crates/astria-sequencer/src/assets/snapshots/astria_sequencer__assets__state_ext__tests__storage_keys_are_unchanged.snap @@ -1,5 +1,5 @@ --- -source: crates/astria-sequencer/src/asset/state_ext.rs +source: crates/astria-sequencer/src/assets/state_ext.rs expression: asset_storage_key(asset) --- asset/be429a02d00837245167a2616674a979a2ac6f9806468b48a975b156ad711320 diff --git a/crates/astria-sequencer/src/assets/state_ext.rs b/crates/astria-sequencer/src/assets/state_ext.rs new file mode 100644 index 0000000000..48deeaf6a0 --- /dev/null +++ b/crates/astria-sequencer/src/assets/state_ext.rs @@ -0,0 +1,649 @@ +use anyhow::{ + bail, + Context as _, + Result, +}; +use astria_core::primitive::v1::asset; +use async_trait::async_trait; +use borsh::{ + BorshDeserialize, + BorshSerialize, +}; +use cnidarium::{ + StateRead, + StateWrite, +}; +use futures::StreamExt as _; +use tendermint::abci::{ + Event, + EventAttributeIndexExt as _, +}; +use tracing::instrument; + +/// Newtype wrapper to read and write a denomination trace from rocksdb. +#[derive(BorshSerialize, BorshDeserialize, Debug)] +struct DenominationTrace(String); + +const BLOCK_FEES_PREFIX: &str = "block_fees/"; +const FEE_ASSET_PREFIX: &str = "fee_asset/"; +const NATIVE_ASSET_KEY: &[u8] = b"nativeasset"; + +fn asset_storage_key>(asset: TAsset) -> String { + format!("asset/{}", crate::storage_keys::hunks::Asset::from(asset)) +} + +fn block_fees_key>(asset: TAsset) -> String { + format!( + "{BLOCK_FEES_PREFIX}{}", + crate::storage_keys::hunks::Asset::from(asset) + ) +} + +fn fee_asset_key>(asset: TAsset) -> String { + format!( + "{FEE_ASSET_PREFIX}{}", + crate::storage_keys::hunks::Asset::from(asset) + ) +} + +/// Creates `abci::Event` of kind `tx.fees` for sequencer fee reporting +fn construct_tx_fee_event( + asset: &T, + fee_amount: u128, + action_type: String, +) -> Event { + Event::new( + "tx.fees", + [ + ("asset", asset.to_string()).index(), + ("feeAmount", fee_amount.to_string()).index(), + ("actionType", action_type).index(), + ], + ) +} + +#[async_trait] +pub(crate) trait StateReadExt: StateRead { + #[instrument(skip_all)] + async fn get_native_asset(&self) -> Result { + let Some(bytes) = self + .nonverifiable_get_raw(NATIVE_ASSET_KEY) + .await + .context("failed to read raw native asset from state")? + else { + bail!("native asset denom not found in state"); + }; + + let asset = std::str::from_utf8(&bytes) + .context("bytes stored in state not utf8 encoded")? + .parse::() + .context("failed to parse bytes retrieved from state as trace prefixed IBC asset")?; + Ok(asset) + } + + #[instrument(skip_all)] + async fn has_ibc_asset(&self, asset: TAsset) -> Result + where + TAsset: Into + std::fmt::Display + Send, + { + Ok(self + .get_raw(&asset_storage_key(asset)) + .await + .context("failed reading raw asset from state")? + .is_some()) + } + + #[instrument(skip_all)] + async fn map_ibc_to_trace_prefixed_asset( + &self, + asset: asset::IbcPrefixed, + ) -> Result> { + let Some(bytes) = self + .get_raw(&asset_storage_key(asset)) + .await + .context("failed reading raw asset from state")? + else { + return Ok(None); + }; + + let DenominationTrace(denom_str) = + DenominationTrace::try_from_slice(&bytes).context("invalid asset bytes")?; + let denom = denom_str + .parse() + .context("failed to parse retrieved denom string as a Denom")?; + Ok(Some(denom)) + } + + #[instrument(skip_all)] + async fn get_block_fees(&self) -> Result> { + let mut fees = Vec::new(); + + let mut stream = + std::pin::pin!(self.nonverifiable_prefix_raw(BLOCK_FEES_PREFIX.as_bytes())); + while let Some(Ok((key, value))) = stream.next().await { + // if the key isn't of the form `block_fees/{asset_id}`, then we have a bug + // in `put_block_fees` + let suffix = key + .strip_prefix(BLOCK_FEES_PREFIX.as_bytes()) + .expect("prefix must always be present"); + let asset = std::str::from_utf8(suffix) + .context("key suffix was not utf8 encoded; this should not happen")? + .parse::() + .context("failed to parse storage key suffix as address hunk")? + .get(); + + let Ok(bytes): Result<[u8; 16], _> = value.try_into() else { + bail!("failed turning raw block fees bytes into u128; not 16 bytes?"); + }; + + fees.push((asset, u128::from_be_bytes(bytes))); + } + + Ok(fees) + } + + #[instrument(skip_all)] + async fn is_allowed_fee_asset(&self, asset: TAsset) -> Result + where + TAsset: Into + std::fmt::Display + Send, + { + Ok(self + .nonverifiable_get_raw(fee_asset_key(asset).as_bytes()) + .await + .context("failed to read raw fee asset from state")? + .is_some()) + } + + #[instrument(skip_all)] + async fn get_allowed_fee_assets(&self) -> Result> { + let mut assets = Vec::new(); + + let mut stream = std::pin::pin!(self.nonverifiable_prefix_raw(FEE_ASSET_PREFIX.as_bytes())); + while let Some(Ok((key, _))) = stream.next().await { + // if the key isn't of the form `fee_asset/{asset_id}`, then we have a bug + // in `put_allowed_fee_asset` + let suffix = key + .strip_prefix(FEE_ASSET_PREFIX.as_bytes()) + .expect("prefix must always be present"); + let asset = std::str::from_utf8(suffix) + .context("key suffix was not utf8 encoded; this should not happen")? + .parse::() + .context("failed to parse storage key suffix as address hunk")? + .get(); + assets.push(asset); + } + + Ok(assets) + } +} + +impl StateReadExt for T {} + +#[async_trait] +pub(crate) trait StateWriteExt: StateWrite { + #[instrument(skip_all)] + fn put_native_asset(&mut self, asset: &asset::TracePrefixed) { + self.nonverifiable_put_raw(NATIVE_ASSET_KEY.to_vec(), asset.to_string().into_bytes()); + } + + #[instrument(skip_all)] + fn put_ibc_asset(&mut self, asset: &asset::TracePrefixed) -> Result<()> { + let bytes = borsh::to_vec(&DenominationTrace(asset.to_string())) + .context("failed to serialize asset")?; + self.put_raw(asset_storage_key(asset), bytes); + Ok(()) + } + + /// Adds `amount` to the block fees for `asset`. + #[instrument(skip_all)] + async fn get_and_increase_block_fees( + &mut self, + asset: TAsset, + amount: u128, + action_type: String, + ) -> Result<()> + where + TAsset: Into + std::fmt::Display + Send, + { + let tx_fee_event = construct_tx_fee_event(&asset, amount, action_type); + let block_fees_key = block_fees_key(asset); + + let current_amount = self + .nonverifiable_get_raw(block_fees_key.as_bytes()) + .await + .context("failed to read raw block fees from state")? + .map(|bytes| { + let Ok(bytes): Result<[u8; 16], _> = bytes.try_into() else { + // this shouldn't happen + bail!("failed turning raw block fees bytes into u128; not 16 bytes?"); + }; + Ok(u128::from_be_bytes(bytes)) + }) + .transpose()? + .unwrap_or_default(); + + let new_amount = current_amount + .checked_add(amount) + .context("block fees overflowed u128")?; + + self.nonverifiable_put_raw(block_fees_key.into(), new_amount.to_be_bytes().to_vec()); + + self.record(tx_fee_event); + + Ok(()) + } + + #[instrument(skip_all)] + async fn clear_block_fees(&mut self) { + let mut stream = + std::pin::pin!(self.nonverifiable_prefix_raw(BLOCK_FEES_PREFIX.as_bytes())); + while let Some(Ok((key, _))) = stream.next().await { + self.nonverifiable_delete(key); + } + } + + #[instrument(skip_all)] + fn delete_allowed_fee_asset(&mut self, asset: TAsset) + where + TAsset: Into + std::fmt::Display, + { + self.nonverifiable_delete(fee_asset_key(asset).into()); + } + + #[instrument(skip_all)] + fn put_allowed_fee_asset(&mut self, asset: TAsset) + where + TAsset: Into + std::fmt::Display + Send, + { + self.nonverifiable_put_raw(fee_asset_key(asset).into(), vec![]); + } +} + +impl StateWriteExt for T {} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use astria_core::primitive::v1::asset; + use cnidarium::StateDelta; + + use super::{ + asset_storage_key, + block_fees_key, + fee_asset_key, + StateReadExt as _, + StateWriteExt as _, + }; + + fn asset() -> asset::Denom { + "asset".parse().unwrap() + } + + fn asset_0() -> asset::Denom { + "asset_0".parse().unwrap() + } + fn asset_1() -> asset::Denom { + "asset_1".parse().unwrap() + } + fn asset_2() -> asset::Denom { + "asset_2".parse().unwrap() + } + + #[tokio::test] + async fn native_asset() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // doesn't exist at first + state + .get_native_asset() + .await + .expect_err("no native asset denom should exist at first"); + + // can write + let denom_orig = "denom_orig".parse().unwrap(); + state.put_native_asset(&denom_orig); + assert_eq!( + state.get_native_asset().await.expect( + "a native asset denomination was written and must exist inside the database" + ), + denom_orig, + "stored native asset denomination was not what was expected" + ); + + // can write new value + let denom_update = "denom_update".parse().unwrap(); + state.put_native_asset(&denom_update); + assert_eq!( + state.get_native_asset().await.expect( + "a native asset denomination update was written and must exist inside the database" + ), + denom_update, + "updated native asset denomination was not what was expected" + ); + } + + #[tokio::test] + async fn block_fee_read_and_increase() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // doesn't exist at first + let fee_balances_orig = state.get_block_fees().await.unwrap(); + assert!(fee_balances_orig.is_empty()); + + // can write + let asset = asset_0(); + let amount = 100u128; + state + .get_and_increase_block_fees(&asset, amount, "test".into()) + .await + .unwrap(); + + // holds expected + let fee_balances_updated = state.get_block_fees().await.unwrap(); + assert_eq!( + fee_balances_updated[0], + (asset.to_ibc_prefixed(), amount), + "fee balances are not what they were expected to be" + ); + } + + #[tokio::test] + async fn block_fee_read_and_increase_can_delete() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // can write + let asset_first = asset_0(); + let asset_second = asset_1(); + let amount_first = 100u128; + let amount_second = 200u128; + + state + .get_and_increase_block_fees(&asset_first, amount_first, "test".into()) + .await + .unwrap(); + state + .get_and_increase_block_fees(&asset_second, amount_second, "test".into()) + .await + .unwrap(); + // holds expected + let fee_balances = HashSet::<_>::from_iter(state.get_block_fees().await.unwrap()); + assert_eq!( + fee_balances, + HashSet::from_iter(vec![ + (asset_first.to_ibc_prefixed(), amount_first), + (asset_second.to_ibc_prefixed(), amount_second) + ]), + "returned fee balance vector not what was expected" + ); + + // can delete + state.clear_block_fees().await; + + let fee_balances_updated = state.get_block_fees().await.unwrap(); + assert!( + fee_balances_updated.is_empty(), + "fee balances were expected to be deleted but were not" + ); + } + + #[tokio::test] + async fn get_ibc_asset_non_existent() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let state = StateDelta::new(snapshot); + + let asset = asset(); + + // gets for non existing assets should return none + assert_eq!( + state + .map_ibc_to_trace_prefixed_asset(asset.to_ibc_prefixed()) + .await + .expect("getting non existing asset should not fail"), + None + ); + } + + #[tokio::test] + async fn has_ibc_asset() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + let denom = asset(); + + // non existing calls are ok for 'has' + assert!( + !state + .has_ibc_asset(&denom) + .await + .expect("'has' for non existing ibc assets should be ok"), + "query for non existing asset should return false" + ); + + state + .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) + .expect("putting ibc asset should not fail"); + + // existing calls are ok for 'has' + assert!( + state + .has_ibc_asset(&denom) + .await + .expect("'has' for existing ibc assets should be ok"), + "query for existing asset should return true" + ); + } + + #[tokio::test] + async fn put_ibc_asset_simple() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // can write new + let denom = asset(); + state + .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) + .expect("putting ibc asset should not fail"); + assert_eq!( + state + .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) + .await + .unwrap() + .expect("an ibc asset was written and must exist inside the database"), + denom.unwrap_trace_prefixed(), + "stored ibc asset was not what was expected" + ); + } + + #[tokio::test] + async fn put_ibc_asset_complex() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // can write new + let denom = asset_0(); + state + .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) + .expect("putting ibc asset should not fail"); + assert_eq!( + state + .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) + .await + .unwrap() + .expect("an ibc asset was written and must exist inside the database"), + denom.clone().unwrap_trace_prefixed(), + "stored ibc asset was not what was expected" + ); + + // can write another without affecting original + let denom_1 = asset_1(); + state + .put_ibc_asset(&denom_1.clone().unwrap_trace_prefixed()) + .expect("putting ibc asset should not fail"); + assert_eq!( + state + .map_ibc_to_trace_prefixed_asset(denom_1.to_ibc_prefixed()) + .await + .unwrap() + .expect("an additional ibc asset was written and must exist inside the database"), + denom_1.unwrap_trace_prefixed(), + "additional ibc asset was not what was expected" + ); + assert_eq!( + state + .map_ibc_to_trace_prefixed_asset(denom.to_ibc_prefixed()) + .await + .unwrap() + .expect("an ibc asset was written and must exist inside the database"), + denom.clone().unwrap_trace_prefixed(), + "original ibc asset was not what was expected" + ); + } + + #[tokio::test] + async fn is_allowed_fee_asset() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // non-existent fees assets return false + let asset = asset_0(); + assert!( + !state + .is_allowed_fee_asset(&asset) + .await + .expect("checking for allowed fee asset should not fail"), + "fee asset was expected to return false" + ); + + // existent fee assets return true + state.put_allowed_fee_asset(&asset); + assert!( + state + .is_allowed_fee_asset(&asset) + .await + .expect("checking for allowed fee asset should not fail"), + "fee asset was expected to be allowed" + ); + } + + #[tokio::test] + async fn can_delete_allowed_fee_assets_simple() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // setup fee asset + let asset = asset_0(); + state.put_allowed_fee_asset(&asset); + assert!( + state + .is_allowed_fee_asset(&asset) + .await + .expect("checking for allowed fee asset should not fail"), + "fee asset was expected to be allowed" + ); + + // see can get fee asset + let assets = state.get_allowed_fee_assets().await.unwrap(); + assert_eq!( + assets, + vec![asset.to_ibc_prefixed()], + "expected returned allowed fee assets to match what was written in" + ); + + // can delete + state.delete_allowed_fee_asset(&asset); + + // see is deleted + let assets = state.get_allowed_fee_assets().await.unwrap(); + assert!(assets.is_empty(), "fee assets should be empty post delete"); + } + + #[tokio::test] + async fn can_delete_allowed_fee_assets_complex() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + // setup fee assets + let asset_first = asset_0(); + state.put_allowed_fee_asset(&asset_first); + assert!( + state + .is_allowed_fee_asset(&asset_first) + .await + .expect("checking for allowed fee asset should not fail"), + "fee asset was expected to be allowed" + ); + let asset_second = asset_1(); + state.put_allowed_fee_asset(&asset_second); + assert!( + state + .is_allowed_fee_asset(&asset_second) + .await + .expect("checking for allowed fee asset should not fail"), + "fee asset was expected to be allowed" + ); + let asset_third = asset_2(); + state.put_allowed_fee_asset(&asset_third); + assert!( + state + .is_allowed_fee_asset(&asset_third) + .await + .expect("checking for allowed fee asset should not fail"), + "fee asset was expected to be allowed" + ); + + // can delete + state.delete_allowed_fee_asset(&asset_second); + + // see is deleted + let assets = HashSet::<_>::from_iter(state.get_allowed_fee_assets().await.unwrap()); + assert_eq!( + assets, + HashSet::from_iter(vec![ + asset_first.to_ibc_prefixed(), + asset_third.to_ibc_prefixed() + ]), + "delete for allowed fee asset did not behave as expected" + ); + } + + #[test] + fn storage_keys_are_unchanged() { + let asset = "an/asset/with/a/prefix" + .parse::() + .unwrap(); + assert_eq!( + asset_storage_key(&asset), + asset_storage_key(asset.to_ibc_prefixed()), + ); + insta::assert_snapshot!(asset_storage_key(asset)); + + let trace_prefixed = "a/denom/with/a/prefix" + .parse::() + .unwrap(); + assert_eq!( + block_fees_key(&trace_prefixed), + block_fees_key(trace_prefixed.to_ibc_prefixed()), + ); + insta::assert_snapshot!(block_fees_key(&trace_prefixed)); + + assert_eq!( + fee_asset_key(&trace_prefixed), + fee_asset_key(trace_prefixed.to_ibc_prefixed()), + ); + insta::assert_snapshot!(fee_asset_key(trace_prefixed)); + } +} diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs index 399b9e4801..60611fff73 100644 --- a/crates/astria-sequencer/src/authority/action.rs +++ b/crates/astria-sequencer/src/authority/action.rs @@ -16,7 +16,7 @@ use astria_core::{ use tracing::instrument; use crate::{ - authority::state_ext::{ + authority::{ StateReadExt, StateWriteExt, }, @@ -126,10 +126,10 @@ impl ActionHandler for FeeChangeAction { #[instrument(skip_all)] async fn execute(&self, state: &mut S, _: Address) -> Result<()> { use crate::{ - accounts::state_ext::StateWriteExt as _, - bridge::state_ext::StateWriteExt as _, - ibc::state_ext::StateWriteExt as _, - sequence::state_ext::StateWriteExt as _, + accounts::StateWriteExt as _, + bridge::StateWriteExt as _, + ibc::StateWriteExt as _, + sequence::StateWriteExt as _, }; match self.fee_change { @@ -168,19 +168,19 @@ mod test { use super::*; use crate::{ - accounts::state_ext::{ + accounts::{ StateReadExt as _, StateWriteExt as _, }, - bridge::state_ext::{ + bridge::{ StateReadExt as _, StateWriteExt as _, }, - ibc::state_ext::{ + ibc::{ StateReadExt as _, StateWriteExt as _, }, - sequence::state_ext::{ + sequence::{ StateReadExt as _, StateWriteExt as _, }, diff --git a/crates/astria-sequencer/src/authority/component.rs b/crates/astria-sequencer/src/authority/component.rs index 0d1aa6eb09..91d8d71c7b 100644 --- a/crates/astria-sequencer/src/authority/component.rs +++ b/crates/astria-sequencer/src/authority/component.rs @@ -14,7 +14,7 @@ use tendermint::abci::request::{ }; use tracing::instrument; -use super::state_ext::{ +use super::{ StateReadExt, StateWriteExt, ValidatorSet, diff --git a/crates/astria-sequencer/src/authority/mod.rs b/crates/astria-sequencer/src/authority/mod.rs index e6337af965..0eb1de4a6b 100644 --- a/crates/astria-sequencer/src/authority/mod.rs +++ b/crates/astria-sequencer/src/authority/mod.rs @@ -1,3 +1,89 @@ -pub(crate) mod action; +mod action; pub(crate) mod component; -pub(crate) mod state_ext; +mod state_ext; + +use std::collections::BTreeMap; + +use anyhow::Context as _; +use astria_core::{ + crypto::VerificationKey, + primitive::v1::ADDRESS_LEN, + protocol::transaction::v1alpha1::action::ValidatorUpdate, +}; +use serde::{ + Deserialize, + Serialize, +}; +pub(crate) use state_ext::{ + StateReadExt, + StateWriteExt, +}; + +#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Ord, PartialOrd)] +pub(crate) struct ValidatorSetKey(#[serde(with = "::hex::serde")] [u8; ADDRESS_LEN]); + +impl From<[u8; ADDRESS_LEN]> for ValidatorSetKey { + fn from(value: [u8; ADDRESS_LEN]) -> Self { + Self(value) + } +} + +impl From for ValidatorSetKey { + fn from(value: VerificationKey) -> Self { + Self(value.address_bytes()) + } +} + +/// Newtype wrapper to read and write a validator set or set of updates from rocksdb. +/// +/// Contains a map of hex-encoded public keys to validator updates. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub(crate) struct ValidatorSet(BTreeMap); + +impl ValidatorSet { + pub(crate) fn new_from_updates(updates: Vec) -> Self { + Self( + updates + .into_iter() + .map(|update| (update.verification_key.into(), update)) + .collect::>(), + ) + } + + pub(crate) fn len(&self) -> usize { + self.0.len() + } + + pub(crate) fn get>(&self, address: T) -> Option<&ValidatorUpdate> { + self.0.get(&address.into()) + } + + pub(super) fn push_update(&mut self, update: ValidatorUpdate) { + self.0.insert(update.verification_key.into(), update); + } + + pub(super) fn remove>(&mut self, address: T) { + self.0.remove(&address.into()); + } + + /// Apply updates to the validator set. + /// + /// If the power of a validator is set to 0, remove it from the set. + /// Otherwise, update the validator's power. + pub(super) fn apply_updates(&mut self, validator_updates: ValidatorSet) { + for (address, update) in validator_updates.0 { + match update.power { + 0 => self.0.remove(&address), + _ => self.0.insert(address, update), + }; + } + } + + pub(crate) fn try_into_cometbft(self) -> anyhow::Result> { + self.0 + .into_values() + .map(crate::utils::sequencer_to_cometbft_validator) + .collect::, _>>() + .context("failed to map one or more astria validators to cometbft validators") + } +} diff --git a/crates/astria-sequencer/src/authority/state_ext.rs b/crates/astria-sequencer/src/authority/state_ext.rs index 34adde2737..20601c4435 100644 --- a/crates/astria-sequencer/src/authority/state_ext.rs +++ b/crates/astria-sequencer/src/authority/state_ext.rs @@ -5,13 +5,9 @@ use anyhow::{ Context, Result, }; -use astria_core::{ - crypto::VerificationKey, - primitive::v1::{ - Address, - ADDRESS_LEN, - }, - protocol::transaction::v1alpha1::action::ValidatorUpdate, +use astria_core::primitive::v1::{ + Address, + ADDRESS_LEN, }; use async_trait::async_trait; use borsh::{ @@ -22,85 +18,14 @@ use cnidarium::{ StateRead, StateWrite, }; -use serde::{ - Deserialize, - Serialize, -}; use tracing::instrument; +use super::ValidatorSet; + /// Newtype wrapper to read and write an address from rocksdb. #[derive(BorshSerialize, BorshDeserialize, Debug)] struct SudoAddress([u8; ADDRESS_LEN]); -/// Newtype wrapper to read and write a validator set or set of updates from rocksdb. -/// -/// Contains a map of hex-encoded public keys to validator updates. -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] -pub(crate) struct ValidatorSet(BTreeMap); - -#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Ord, PartialOrd)] -pub(crate) struct ValidatorSetKey(#[serde(with = "::hex::serde")] [u8; ADDRESS_LEN]); - -impl From<[u8; ADDRESS_LEN]> for ValidatorSetKey { - fn from(value: [u8; ADDRESS_LEN]) -> Self { - Self(value) - } -} - -impl From for ValidatorSetKey { - fn from(value: VerificationKey) -> Self { - Self(value.address_bytes()) - } -} - -impl ValidatorSet { - pub(crate) fn new_from_updates(updates: Vec) -> Self { - Self( - updates - .into_iter() - .map(|update| (update.verification_key.into(), update)) - .collect::>(), - ) - } - - pub(crate) fn len(&self) -> usize { - self.0.len() - } - - pub(crate) fn get>(&self, address: T) -> Option<&ValidatorUpdate> { - self.0.get(&address.into()) - } - - pub(crate) fn push_update(&mut self, update: ValidatorUpdate) { - self.0.insert(update.verification_key.into(), update); - } - - pub(crate) fn remove>(&mut self, address: T) { - self.0.remove(&address.into()); - } - - /// Apply updates to the validator set. - /// - /// If the power of a validator is set to 0, remove it from the set. - /// Otherwise, update the validator's power. - pub(crate) fn apply_updates(&mut self, validator_updates: ValidatorSet) { - for (address, update) in validator_updates.0 { - match update.power { - 0 => self.0.remove(&address), - _ => self.0.insert(address, update), - }; - } - } - - pub(crate) fn try_into_cometbft(self) -> anyhow::Result> { - self.0 - .into_values() - .map(crate::utils::sequencer_to_cometbft_validator) - .collect::, _>>() - .context("failed to map one or more astria validators to cometbft validators") - } -} - const SUDO_STORAGE_KEY: &str = "sudo"; const VALIDATOR_SET_STORAGE_KEY: &str = "valset"; const VALIDATOR_UPDATES_KEY: &[u8] = b"valupdates"; @@ -204,9 +129,11 @@ mod test { use super::{ StateReadExt as _, StateWriteExt as _, - ValidatorSet, }; - use crate::test_utils::verification_key; + use crate::{ + authority::ValidatorSet, + test_utils::verification_key, + }; fn empty_validator_set() -> ValidatorSet { ValidatorSet::new_from_updates(vec![]) diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index 071f2f6e17..4daa373f31 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -16,12 +16,10 @@ use tracing::instrument; use crate::{ accounts::{ action::transfer_check_stateful, - state_ext::{ - StateReadExt as _, - StateWriteExt as _, - }, + StateReadExt as _, + StateWriteExt as _, }, - bridge::state_ext::{ + bridge::{ StateReadExt as _, StateWriteExt as _, }, @@ -163,10 +161,7 @@ mod tests { use cnidarium::StateDelta; use super::*; - use crate::{ - bridge::state_ext::StateWriteExt, - state_ext::StateWriteExt as _, - }; + use crate::assets::StateWriteExt as _; fn test_asset() -> asset::Denom { "test".parse().unwrap() diff --git a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs index e4945e6069..13e21012ca 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs @@ -10,7 +10,8 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts::state_ext::StateWriteExt as _, + accounts::StateWriteExt as _, + assets::StateReadExt as _, bridge::state_ext::{ StateReadExt as _, StateWriteExt as _, @@ -101,6 +102,7 @@ mod tests { use cnidarium::StateDelta; use super::*; + use crate::assets::StateWriteExt as _; fn test_asset() -> asset::Denom { "test".parse().unwrap() diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index d29dbbff54..f6f45937c6 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -15,7 +15,7 @@ use tracing::instrument; use crate::{ accounts::action::transfer_check_stateful, - bridge::state_ext::StateReadExt as _, + bridge::StateReadExt as _, state_ext::{ StateReadExt, StateWriteExt, @@ -112,9 +112,9 @@ mod test { use super::*; use crate::{ - accounts::state_ext::StateWriteExt as _, - bridge::state_ext::StateWriteExt, - state_ext::StateWriteExt as _, + accounts::StateWriteExt as _, + assets::StateWriteExt as _, + bridge::StateWriteExt, }; fn test_asset() -> asset::Denom { diff --git a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs index 8c87241809..a116771e14 100644 --- a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs +++ b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs @@ -11,10 +11,11 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts::state_ext::{ + accounts::{ StateReadExt as _, StateWriteExt as _, }, + assets::StateReadExt as _, bridge::state_ext::{ StateReadExt as _, StateWriteExt as _, diff --git a/crates/astria-sequencer/src/bridge/mod.rs b/crates/astria-sequencer/src/bridge/mod.rs index 9123572fcf..f322762bc4 100644 --- a/crates/astria-sequencer/src/bridge/mod.rs +++ b/crates/astria-sequencer/src/bridge/mod.rs @@ -4,6 +4,10 @@ mod bridge_unlock_action; pub(crate) mod component; pub(crate) mod init_bridge_account_action; pub(crate) mod query; -pub(crate) mod state_ext; +mod state_ext; pub(crate) use bridge_lock_action::get_deposit_byte_len; +pub(crate) use state_ext::{ + StateReadExt, + StateWriteExt, +}; diff --git a/crates/astria-sequencer/src/bridge/query.rs b/crates/astria-sequencer/src/bridge/query.rs index e7a0a2c3c8..2a64baf39d 100644 --- a/crates/astria-sequencer/src/bridge/query.rs +++ b/crates/astria-sequencer/src/bridge/query.rs @@ -14,8 +14,8 @@ use tendermint::abci::{ }; use crate::{ - asset::state_ext::StateReadExt as _, - bridge::state_ext::StateReadExt as _, + assets::StateReadExt as _, + bridge::StateReadExt as _, state_ext::StateReadExt as _, }; @@ -269,8 +269,8 @@ mod test { use super::*; use crate::{ - asset::state_ext::StateWriteExt, - bridge::state_ext::StateWriteExt as _, + assets::StateWriteExt, + bridge::StateWriteExt as _, state_ext::StateWriteExt as _, }; diff --git a/crates/astria-sequencer/src/fee_asset_change.rs b/crates/astria-sequencer/src/fee_asset_change.rs index 9de206b8d7..7ccdfdf804 100644 --- a/crates/astria-sequencer/src/fee_asset_change.rs +++ b/crates/astria-sequencer/src/fee_asset_change.rs @@ -9,23 +9,21 @@ use astria_core::{ protocol::transaction::v1alpha1::action::FeeAssetChangeAction, }; use async_trait::async_trait; -use cnidarium::{ - StateRead, - StateWrite, -}; use crate::{ - authority::state_ext::StateReadExt as _, - state_ext::{ - StateReadExt as _, - StateWriteExt as _, - }, + assets, + assets::StateReadExt as _, + authority, transaction::action_handler::ActionHandler, }; #[async_trait] impl ActionHandler for FeeAssetChangeAction { - async fn check_stateful(&self, state: &S, from: Address) -> Result<()> { + async fn check_stateful( + &self, + state: &S, + from: Address, + ) -> Result<()> { let authority_sudo_address = state .get_sudo_address() .await @@ -37,7 +35,7 @@ impl ActionHandler for FeeAssetChangeAction { Ok(()) } - async fn execute(&self, state: &mut S, _from: Address) -> Result<()> { + async fn execute(&self, state: &mut S, _from: Address) -> Result<()> { match self { FeeAssetChangeAction::Addition(asset) => { state.put_allowed_fee_asset(asset); @@ -45,8 +43,12 @@ impl ActionHandler for FeeAssetChangeAction { FeeAssetChangeAction::Removal(asset) => { state.delete_allowed_fee_asset(asset); - // FIXME: context - if state.get_allowed_fee_assets().await?.is_empty() { + if state + .get_allowed_fee_assets() + .await + .context("failed to retrieve allowed fee assets")? + .is_empty() + { bail!("cannot remove last allowed fee asset"); } } diff --git a/crates/astria-sequencer/src/grpc/sequencer.rs b/crates/astria-sequencer/src/grpc/sequencer.rs index 24fc096800..89609b809d 100644 --- a/crates/astria-sequencer/src/grpc/sequencer.rs +++ b/crates/astria-sequencer/src/grpc/sequencer.rs @@ -169,7 +169,7 @@ impl SequencerService for SequencerServer { ) -> Result, Status> { use astria_core::primitive::v1::Address; - use crate::accounts::state_ext::StateReadExt as _; + use crate::accounts::StateReadExt as _; let request = request.into_inner(); let Some(address) = request.address else { @@ -277,7 +277,7 @@ mod test { #[tokio::test] async fn get_pending_nonce_in_storage() { - use crate::accounts::state_ext::StateWriteExt as _; + use crate::accounts::StateWriteExt as _; let storage = cnidarium::TempStorage::new().await.unwrap(); let mempool = Mempool::new(); diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index bd1d8a44a0..917c0f9f51 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -59,19 +59,17 @@ use penumbra_ibc::component::app_handler::{ use penumbra_proto::penumbra::core::component::ibc::v1::FungibleTokenPacketData; use crate::{ - accounts::state_ext::StateWriteExt as _, - asset::state_ext::{ + accounts::StateWriteExt as _, + assets::{ StateReadExt as _, StateWriteExt as _, }, - bridge::state_ext::{ + bridge::{ StateReadExt as _, StateWriteExt as _, }, - ibc::state_ext::{ - StateReadExt, - StateWriteExt, - }, + ibc, + ibc::StateReadExt as _, }; /// The maximum length of the encoded Ics20 `FungibleTokenPacketData` in bytes. @@ -329,7 +327,7 @@ impl AppHandlerExecute for Ics20Transfer { #[async_trait::async_trait] impl AppHandler for Ics20Transfer {} -async fn convert_denomination_if_ibc_prefixed( +async fn convert_denomination_if_ibc_prefixed( state: &mut S, packet_denom: Denom, ) -> Result { @@ -370,7 +368,7 @@ fn prepend_denom_if_not_refund<'a>( // FIXME: temporarily allowed, but this must be fixed #[allow(clippy::too_many_lines)] -async fn execute_ics20_transfer( +async fn execute_ics20_transfer( state: &mut S, data: &[u8], source_port: &PortId, @@ -524,7 +522,7 @@ async fn execute_ics20_transfer( /// /// this functions sends the tokens back to the rollup via a `Deposit` event, /// and locks the tokens back in the specified bridge account. -async fn execute_rollup_withdrawal_refund( +async fn execute_rollup_withdrawal_refund( state: &mut S, bridge_address: Address, denom: &denom::TracePrefixed, @@ -547,7 +545,7 @@ async fn execute_rollup_withdrawal_refund( /// /// if the recipient is not a bridge account, or the incoming packet is a refund, /// this function is a no-op. -async fn execute_ics20_transfer_bridge_lock( +async fn execute_ics20_transfer_bridge_lock( state: &mut S, recipient: Address, denom: &denom::TracePrefixed, @@ -603,7 +601,7 @@ async fn execute_ics20_transfer_bridge_lock( .await } -async fn execute_deposit( +async fn execute_deposit( state: &mut S, bridge_address: Address, denom: &denom::TracePrefixed, @@ -653,9 +651,8 @@ mod test { use super::*; use crate::{ - accounts::state_ext::StateReadExt as _, - bridge::state_ext::StateWriteExt, - ibc::state_ext::StateWriteExt as _, + accounts::StateReadExt as _, + ibc::StateWriteExt as _, }; #[tokio::test] diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index e2677cdff0..d2a218179c 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -25,12 +25,12 @@ use penumbra_ibc::component::packet::{ use tracing::instrument; use crate::{ - accounts::state_ext::{ + accounts::{ StateReadExt, StateWriteExt, }, - bridge::state_ext::StateReadExt as _, - ibc::state_ext::{ + bridge::StateReadExt as _, + ibc::{ StateReadExt as _, StateWriteExt as _, }, @@ -236,7 +236,7 @@ mod tests { use ibc_types::core::client::Height; use super::*; - use crate::bridge::state_ext::StateWriteExt as _; + use crate::bridge::StateWriteExt as _; #[tokio::test] async fn ics20_withdrawal_check_stateful_bridge_account_not_bridge() { diff --git a/crates/astria-sequencer/src/ibc/mod.rs b/crates/astria-sequencer/src/ibc/mod.rs index f8cba5ac53..3236da2942 100644 --- a/crates/astria-sequencer/src/ibc/mod.rs +++ b/crates/astria-sequencer/src/ibc/mod.rs @@ -3,4 +3,9 @@ pub(crate) mod host_interface; pub(crate) mod ibc_relayer_change; pub(crate) mod ics20_transfer; pub(crate) mod ics20_withdrawal; -pub(crate) mod state_ext; +mod state_ext; + +pub(crate) use state_ext::{ + StateReadExt, + StateWriteExt, +}; diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index 9e5ccdae22..dc84d73303 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -2,7 +2,7 @@ pub(crate) mod accounts; pub(crate) mod address; mod api_state_ext; pub(crate) mod app; -pub(crate) mod asset; +pub(crate) mod assets; pub(crate) mod authority; pub(crate) mod bridge; mod build_info; diff --git a/crates/astria-sequencer/src/proposal/commitment.rs b/crates/astria-sequencer/src/proposal/commitment.rs index c0df56ee71..c50b49e4a5 100644 --- a/crates/astria-sequencer/src/proposal/commitment.rs +++ b/crates/astria-sequencer/src/proposal/commitment.rs @@ -104,7 +104,7 @@ mod test { use rand::rngs::OsRng; use super::*; - use crate::asset::get_native_asset; + use crate::assets::get_native_asset; #[test] fn generate_rollup_datas_commitment_should_ignore_transfers() { diff --git a/crates/astria-sequencer/src/sequence/action.rs b/crates/astria-sequencer/src/sequence/action.rs index 602f90a16b..5b52bfe760 100644 --- a/crates/astria-sequencer/src/sequence/action.rs +++ b/crates/astria-sequencer/src/sequence/action.rs @@ -11,15 +11,13 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts::state_ext::{ + accounts, + accounts::{ StateReadExt, StateWriteExt, }, - sequence::state_ext::StateReadExt as SequenceStateReadExt, - state_ext::{ - StateReadExt as _, - StateWriteExt as _, - }, + assets, + sequence, transaction::action_handler::ActionHandler, }; @@ -29,7 +27,10 @@ impl ActionHandler for SequenceAction { &self, state: &S, from: Address, - ) -> Result<()> { + ) -> Result<()> + where + S: accounts::StateReadExt + assets::StateReadExt + 'static, + { ensure!( state.is_allowed_fee_asset(&self.fee_asset).await?, "invalid fee asset", @@ -57,15 +58,18 @@ impl ActionHandler for SequenceAction { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, from: Address) -> Result<()> { + async fn execute(&self, state: &mut S, from: Address) -> Result<()> + where + S: assets::StateWriteExt, + { let fee = calculate_fee_from_state(&self.data, state) .await .context("failed to calculate fee")?; + state .get_and_increase_block_fees(&self.fee_asset, fee, Self::full_name()) .await .context("failed to add to block fees")?; - state .decrease_balance(from, &self.fee_asset, fee) .await @@ -75,7 +79,7 @@ impl ActionHandler for SequenceAction { } /// Calculates the fee for a sequence `Action` based on the length of the `data`. -pub(crate) async fn calculate_fee_from_state( +pub(crate) async fn calculate_fee_from_state( data: &[u8], state: &S, ) -> Result { diff --git a/crates/astria-sequencer/src/sequence/mod.rs b/crates/astria-sequencer/src/sequence/mod.rs index 4295cd7cf9..8131baaea1 100644 --- a/crates/astria-sequencer/src/sequence/mod.rs +++ b/crates/astria-sequencer/src/sequence/mod.rs @@ -1,5 +1,9 @@ pub(crate) mod action; pub(crate) mod component; -pub(crate) mod state_ext; +mod state_ext; pub(crate) use action::calculate_fee_from_state; +pub(crate) use state_ext::{ + StateReadExt, + StateWriteExt, +}; diff --git a/crates/astria-sequencer/src/sequencer.rs b/crates/astria-sequencer/src/sequencer.rs index cfcfb39123..7e89363900 100644 --- a/crates/astria-sequencer/src/sequencer.rs +++ b/crates/astria-sequencer/src/sequencer.rs @@ -33,13 +33,13 @@ use tracing::{ use crate::{ address::StateReadExt as _, app::App, + assets::StateReadExt as _, config::Config, grpc::sequencer::SequencerServer, ibc::host_interface::AstriaHost, mempool::Mempool, metrics::Metrics, service, - state_ext::StateReadExt as _, }; pub struct Sequencer; @@ -91,10 +91,10 @@ impl Sequencer { if storage.latest_version() != u64::MAX { // native asset should be stored, fetch it let native_asset = snapshot - .get_native_asset_denom() + .get_native_asset() .await .context("failed to get native asset from storage")?; - crate::asset::initialize_native_asset(&native_asset); + crate::assets::initialize_native_asset(&native_asset.to_string()); let base_prefix = snapshot .get_base_prefix() .await diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 7e8a6e8244..77155d4ac3 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -233,7 +233,7 @@ mod test { use super::*; use crate::{ app::test_utils::default_fees, - asset::get_native_asset, + assets::get_native_asset, mempool::Mempool, metrics::Metrics, proposal::commitment::generate_rollup_datas_commitment, diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 121e85f4bc..400b930b5b 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -56,12 +56,12 @@ impl Info { ) .context("invalid path: `accounts/nonce/:account`")?; query_router - .insert("asset/denom/:id", crate::asset::query::denom_request) + .insert("asset/denom/:id", crate::assets::query::denom_request) .context("invalid path: `asset/denom/:id`")?; query_router .insert( "asset/allowed_fee_assets", - crate::asset::query::allowed_fee_assets_request, + crate::assets::query::allowed_fee_assets_request, ) .context("invalid path: `asset/allowed_fee_asset_ids`")?; query_router @@ -189,16 +189,14 @@ mod test { use super::Info; use crate::{ - accounts::state_ext::StateWriteExt as _, - asset::{ + accounts::StateWriteExt as _, + assets::{ get_native_asset, initialize_native_asset, - state_ext::StateWriteExt, - }, - state_ext::{ - StateReadExt, + StateReadExt as _, StateWriteExt as _, }, + state_ext::StateWriteExt as _, }; #[tokio::test] diff --git a/crates/astria-sequencer/src/service/mempool.rs b/crates/astria-sequencer/src/service/mempool.rs index 7f48d20587..3257a6ac44 100644 --- a/crates/astria-sequencer/src/service/mempool.rs +++ b/crates/astria-sequencer/src/service/mempool.rs @@ -34,7 +34,7 @@ use tracing::{ }; use crate::{ - accounts::state_ext::StateReadExt, + accounts::StateReadExt, mempool::{ Mempool as AppMempool, RemovalReason, diff --git a/crates/astria-sequencer/src/state_ext.rs b/crates/astria-sequencer/src/state_ext.rs index 73758a8348..78eb4c32cd 100644 --- a/crates/astria-sequencer/src/state_ext.rs +++ b/crates/astria-sequencer/src/state_ext.rs @@ -3,60 +3,20 @@ use anyhow::{ Context as _, Result, }; -use astria_core::primitive::v1::asset; use async_trait::async_trait; use cnidarium::{ StateRead, StateWrite, }; -use futures::StreamExt as _; -use tendermint::{ - abci::{ - Event, - EventAttributeIndexExt as _, - }, - Time, -}; +use tendermint::Time; use tracing::instrument; -const NATIVE_ASSET_KEY: &[u8] = b"nativeasset"; const REVISION_NUMBER_KEY: &str = "revision_number"; -const BLOCK_FEES_PREFIX: &str = "block_fees/"; -const FEE_ASSET_PREFIX: &str = "fee_asset/"; fn storage_version_by_height_key(height: u64) -> Vec { format!("storage_version/{height}").into() } -fn block_fees_key>(asset: TAsset) -> String { - format!( - "{BLOCK_FEES_PREFIX}{}", - crate::storage_keys::hunks::Asset::from(asset) - ) -} - -fn fee_asset_key>(asset: TAsset) -> String { - format!( - "{FEE_ASSET_PREFIX}{}", - crate::storage_keys::hunks::Asset::from(asset) - ) -} - -/// Creates `abci::Event` of kind `tx.fees` for sequencer fee reporting -fn construct_tx_fee_event(asset: &TAsset, fee_amount: u128, action_type: String) -> Event -where - TAsset: Into + std::fmt::Display + Send, -{ - Event::new( - "tx.fees", - [ - ("asset", asset.to_string()).index(), - ("feeAmount", fee_amount.to_string()).index(), - ("actionType", action_type).index(), - ], - ) -} - #[async_trait] pub(crate) trait StateReadExt: StateRead { #[instrument(skip_all)] @@ -139,82 +99,6 @@ pub(crate) trait StateReadExt: StateRead { }; Ok(u64::from_be_bytes(bytes)) } - - #[instrument(skip_all)] - async fn get_native_asset_denom(&self) -> Result { - let Some(bytes) = self - .nonverifiable_get_raw(NATIVE_ASSET_KEY) - .await - .context("failed to read raw native_asset_denom from state")? - else { - bail!("native asset denom not found"); - }; - - String::from_utf8(bytes).context("failed to parse native asset denom from raw bytes") - } - - #[instrument(skip_all)] - async fn get_block_fees(&self) -> Result> { - // let mut fees: Vec<(asset::Id, u128)> = Vec::new(); - let mut fees = Vec::new(); - - let mut stream = - std::pin::pin!(self.nonverifiable_prefix_raw(BLOCK_FEES_PREFIX.as_bytes())); - while let Some(Ok((key, value))) = stream.next().await { - // if the key isn't of the form `block_fees/{asset_id}`, then we have a bug - // in `put_block_fees` - let suffix = key - .strip_prefix(BLOCK_FEES_PREFIX.as_bytes()) - .expect("prefix must always be present"); - let asset = std::str::from_utf8(suffix) - .context("key suffix was not utf8 encoded; this should not happen")? - .parse::() - .context("failed to parse storage key suffix as address hunk")? - .get(); - - let Ok(bytes): Result<[u8; 16], _> = value.try_into() else { - bail!("failed turning raw block fees bytes into u128; not 16 bytes?"); - }; - - fees.push((asset, u128::from_be_bytes(bytes))); - } - - Ok(fees) - } - - #[instrument(skip_all)] - async fn is_allowed_fee_asset(&self, asset: TAsset) -> Result - where - TAsset: Into + std::fmt::Display + Send, - { - Ok(self - .nonverifiable_get_raw(fee_asset_key(asset).as_bytes()) - .await - .context("failed to read raw fee asset from state")? - .is_some()) - } - - #[instrument(skip_all)] - async fn get_allowed_fee_assets(&self) -> Result> { - let mut assets = Vec::new(); - - let mut stream = std::pin::pin!(self.nonverifiable_prefix_raw(FEE_ASSET_PREFIX.as_bytes())); - while let Some(Ok((key, _))) = stream.next().await { - // if the key isn't of the form `fee_asset/{asset_id}`, then we have a bug - // in `put_allowed_fee_asset` - let suffix = key - .strip_prefix(FEE_ASSET_PREFIX.as_bytes()) - .expect("prefix must always be present"); - let asset = std::str::from_utf8(suffix) - .context("key suffix was not utf8 encoded; this should not happen")? - .parse::() - .context("failed to parse storage key suffix as address hunk")? - .get(); - assets.push(asset); - } - - Ok(assets) - } } impl StateReadExt for T {} @@ -253,74 +137,6 @@ pub(crate) trait StateWriteExt: StateWrite { version.to_be_bytes().to_vec(), ); } - - #[instrument(skip_all)] - fn put_native_asset_denom(&mut self, denom: &str) { - self.nonverifiable_put_raw(NATIVE_ASSET_KEY.to_vec(), denom.as_bytes().to_vec()); - } - - /// Adds `amount` to the block fees for `asset`. - #[instrument(skip_all)] - async fn get_and_increase_block_fees( - &mut self, - asset: TAsset, - fee_amount: u128, - action_type: String, - ) -> Result<()> - where - TAsset: Into + std::fmt::Display + Send + Clone, - { - let block_fees_key = block_fees_key(asset.clone()); - let current_amount = self - .nonverifiable_get_raw(block_fees_key.as_bytes()) - .await - .context("failed to read raw block fees from state")? - .map(|bytes| { - let Ok(bytes): Result<[u8; 16], _> = bytes.try_into() else { - // this shouldn't happen - bail!("failed turning raw block fees bytes into u128; not 16 bytes?"); - }; - Ok(u128::from_be_bytes(bytes)) - }) - .transpose()? - .unwrap_or_default(); - - let new_amount = current_amount - .checked_add(fee_amount) - .context("block fees overflowed u128")?; - self.nonverifiable_put_raw(block_fees_key.into(), new_amount.to_be_bytes().to_vec()); - - // record the fee event to the state cache - let tx_fee_event = construct_tx_fee_event(&asset, fee_amount, action_type); - self.record(tx_fee_event); - - Ok(()) - } - - #[instrument(skip_all)] - async fn clear_block_fees(&mut self) { - let mut stream = - std::pin::pin!(self.nonverifiable_prefix_raw(BLOCK_FEES_PREFIX.as_bytes())); - while let Some(Ok((key, _))) = stream.next().await { - self.nonverifiable_delete(key); - } - } - - #[instrument(skip_all)] - fn put_allowed_fee_asset(&mut self, asset: TAsset) - where - TAsset: Into + std::fmt::Display + Send, - { - self.nonverifiable_put_raw(fee_asset_key(asset).into(), vec![]); - } - - #[instrument(skip_all)] - fn delete_allowed_fee_asset(&mut self, asset: TAsset) - where - TAsset: Into + std::fmt::Display, - { - self.nonverifiable_delete(fee_asset_key(asset).into()); - } } impl StateWriteExt for T {} @@ -343,9 +159,7 @@ fn revision_number_from_chain_id(chain_id: &str) -> u64 { } #[cfg(test)] -mod test { - use std::collections::HashSet; - +mod tests { use cnidarium::StateDelta; use tendermint::Time; @@ -354,20 +168,6 @@ mod test { StateReadExt as _, StateWriteExt as _, }; - use crate::state_ext::{ - block_fees_key, - fee_asset_key, - }; - - fn asset_0() -> astria_core::primitive::v1::asset::Denom { - "asset_0".parse().unwrap() - } - fn asset_1() -> astria_core::primitive::v1::asset::Denom { - "asset_1".parse().unwrap() - } - fn asset_2() -> astria_core::primitive::v1::asset::Denom { - "asset_2".parse().unwrap() - } #[test] fn revision_number_from_chain_id_regex() { @@ -600,235 +400,4 @@ mod test { "original but updated storage version was not what was expected" ); } - - #[tokio::test] - async fn native_asset_denom() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // doesn't exist at first - state - .get_native_asset_denom() - .await - .expect_err("no native asset denom should exist at first"); - - // can write - let denom_orig = "denom_orig"; - state.put_native_asset_denom(denom_orig); - assert_eq!( - state.get_native_asset_denom().await.expect( - "a native asset denomination was written and must exist inside the database" - ), - denom_orig, - "stored native asset denomination was not what was expected" - ); - - // can write new value - let denom_update = "denom_update"; - state.put_native_asset_denom(denom_update); - assert_eq!( - state.get_native_asset_denom().await.expect( - "a native asset denomination update was written and must exist inside the database" - ), - denom_update, - "updated native asset denomination was not what was expected" - ); - } - - #[tokio::test] - async fn block_fee_read_and_increase() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // doesn't exist at first - let fee_balances_orig = state.get_block_fees().await.unwrap(); - assert!(fee_balances_orig.is_empty()); - - // can write - let asset = asset_0(); - let amount = 100u128; - state - .get_and_increase_block_fees(&asset, amount, "test".into()) - .await - .unwrap(); - - // holds expected - let fee_balances_updated = state.get_block_fees().await.unwrap(); - assert_eq!( - fee_balances_updated[0], - (asset.to_ibc_prefixed(), amount), - "fee balances are not what they were expected to be" - ); - } - - #[tokio::test] - async fn block_fee_read_and_increase_can_delete() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // can write - let asset_first = asset_0(); - let asset_second = asset_1(); - let amount_first = 100u128; - let amount_second = 200u128; - - state - .get_and_increase_block_fees(&asset_first, amount_first, "test".into()) - .await - .unwrap(); - state - .get_and_increase_block_fees(&asset_second, amount_second, "test".into()) - .await - .unwrap(); - // holds expected - let fee_balances = HashSet::<_>::from_iter(state.get_block_fees().await.unwrap()); - assert_eq!( - fee_balances, - HashSet::from_iter(vec![ - (asset_first.to_ibc_prefixed(), amount_first), - (asset_second.to_ibc_prefixed(), amount_second) - ]), - "returned fee balance vector not what was expected" - ); - - // can delete - state.clear_block_fees().await; - - let fee_balances_updated = state.get_block_fees().await.unwrap(); - assert!( - fee_balances_updated.is_empty(), - "fee balances were expected to be deleted but were not" - ); - } - - #[tokio::test] - async fn is_allowed_fee_asset() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // non-existent fees assets return false - let asset = asset_0(); - assert!( - !state - .is_allowed_fee_asset(&asset) - .await - .expect("checking for allowed fee asset should not fail"), - "fee asset was expected to return false" - ); - - // existent fee assets return true - state.put_allowed_fee_asset(&asset); - assert!( - state - .is_allowed_fee_asset(&asset) - .await - .expect("checking for allowed fee asset should not fail"), - "fee asset was expected to be allowed" - ); - } - - #[tokio::test] - async fn can_delete_allowed_fee_assets_simple() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // setup fee asset - let asset = asset_0(); - state.put_allowed_fee_asset(&asset); - assert!( - state - .is_allowed_fee_asset(&asset) - .await - .expect("checking for allowed fee asset should not fail"), - "fee asset was expected to be allowed" - ); - - // see can get fee asset - let assets = state.get_allowed_fee_assets().await.unwrap(); - assert_eq!( - assets, - vec![asset.to_ibc_prefixed()], - "expected returned allowed fee assets to match what was written in" - ); - - // can delete - state.delete_allowed_fee_asset(&asset); - - // see is deleted - let assets = state.get_allowed_fee_assets().await.unwrap(); - assert!(assets.is_empty(), "fee assets should be empty post delete"); - } - - #[tokio::test] - async fn can_delete_allowed_fee_assets_complex() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // setup fee assets - let asset_first = asset_0(); - state.put_allowed_fee_asset(&asset_first); - assert!( - state - .is_allowed_fee_asset(&asset_first) - .await - .expect("checking for allowed fee asset should not fail"), - "fee asset was expected to be allowed" - ); - let asset_second = asset_1(); - state.put_allowed_fee_asset(&asset_second); - assert!( - state - .is_allowed_fee_asset(&asset_second) - .await - .expect("checking for allowed fee asset should not fail"), - "fee asset was expected to be allowed" - ); - let asset_third = asset_2(); - state.put_allowed_fee_asset(&asset_third); - assert!( - state - .is_allowed_fee_asset(&asset_third) - .await - .expect("checking for allowed fee asset should not fail"), - "fee asset was expected to be allowed" - ); - - // can delete - state.delete_allowed_fee_asset(&asset_second); - - // see is deleted - let assets = HashSet::<_>::from_iter(state.get_allowed_fee_assets().await.unwrap()); - assert_eq!( - assets, - HashSet::from_iter(vec![ - asset_first.to_ibc_prefixed(), - asset_third.to_ibc_prefixed() - ]), - "delete for allowed fee asset did not behave as expected" - ); - } - - #[test] - fn storage_keys_are_not_changed() { - let trace_prefixed = "a/denom/with/a/prefix" - .parse::() - .unwrap(); - assert_eq!( - block_fees_key(&trace_prefixed), - block_fees_key(trace_prefixed.to_ibc_prefixed()), - ); - insta::assert_snapshot!(block_fees_key(&trace_prefixed)); - - assert_eq!( - fee_asset_key(&trace_prefixed), - fee_asset_key(trace_prefixed.to_ibc_prefixed()), - ); - insta::assert_snapshot!(fee_asset_key(trace_prefixed)); - } } diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index 53a3025b03..e36ad95587 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -22,9 +22,9 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts::state_ext::StateReadExt, - bridge::state_ext::StateReadExt as _, - ibc::state_ext::StateReadExt as _, + accounts::StateReadExt, + bridge::StateReadExt as _, + ibc::StateReadExt as _, state_ext::StateReadExt as _, }; @@ -308,11 +308,11 @@ mod tests { use super::*; use crate::{ - accounts::state_ext::StateWriteExt as _, + accounts::StateWriteExt as _, app::test_utils::*, - bridge::state_ext::StateWriteExt, - ibc::state_ext::StateWriteExt as _, - sequence::state_ext::StateWriteExt as _, + bridge::StateWriteExt, + ibc::StateWriteExt as _, + sequence::StateWriteExt as _, }; #[tokio::test] @@ -329,7 +329,7 @@ mod tests { state_tx.put_bridge_lock_byte_cost_multiplier(1); state_tx.put_bridge_sudo_change_base_fee(24); - let native_asset = crate::asset::get_native_asset(); + let native_asset = crate::assets::get_native_asset(); let other_asset = "other".parse::().unwrap(); let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); @@ -395,7 +395,7 @@ mod tests { state_tx.put_bridge_lock_byte_cost_multiplier(1); state_tx.put_bridge_sudo_change_base_fee(24); - let native_asset = crate::asset::get_native_asset(); + let native_asset = crate::assets::get_native_asset(); let other_asset = "other".parse::().unwrap(); let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index f683518ec7..5348b0a42e 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -26,13 +26,13 @@ pub(crate) use checks::{ use tracing::instrument; use crate::{ - accounts::state_ext::{ + accounts::{ StateReadExt, StateWriteExt, }, ibc::{ host_interface::AstriaHost, - state_ext::StateReadExt as _, + StateReadExt as _, }, state_ext::StateReadExt as _, }; @@ -60,7 +60,7 @@ pub(crate) async fn execute( tx: &SignedTransaction, state: &mut S, ) -> anyhow::Result<()> { - use crate::bridge::state_ext::{ + use crate::bridge::{ StateReadExt as _, StateWriteExt as _, }; diff --git a/crates/astria-sequencer/src/transaction/query.rs b/crates/astria-sequencer/src/transaction/query.rs index 0bbc1233d2..1bda4fe45d 100644 --- a/crates/astria-sequencer/src/transaction/query.rs +++ b/crates/astria-sequencer/src/transaction/query.rs @@ -13,7 +13,7 @@ use tendermint::abci::{ }; use crate::{ - asset::state_ext::StateReadExt as _, + assets::StateReadExt as _, state_ext::StateReadExt as _, transaction::checks::get_fees_for_transaction, }; From c765408c97442549db04f7a8c46dd486e4f2be21 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 1 Aug 2024 18:49:48 +0200 Subject: [PATCH 2/3] refactor(sequencer): remove global state (#1317) ## Summary Removed all global from sequencer. ## Background Upon init, sequencer stored its native asset and address base prefix at a fixed global memory locations. This did not yet lead to issues during runtime (because we don't mutate global after startup), it made testing unnecessarily difficult: sequencer relies heavily on unit tests, yet the tests were impure because they relied on the same global state. The changes to the `mempool` module shows another issue: while the mempool could be wholly agnostic to the specific global state, heavy use of the global base prefix meant was less decoupled than it could be. Because sequencer explicitly passes around state (in the form of state deltas, the data inside which are made accessible via various `StateReadExt` and `StateWriteExt` traits), we can read the configured base prefix and native assets from there. The downside is that this patch introduces more database reads. However, given the amounts of data sequencer is dealing with this might not matter and can be alleviated by using a rocksdb Cache (or other mechanism) instead of relying on globals. ## Changes - Remove global setters and writers in `crate::address` and `crate::assets` - Update all code to read the base address prefix and native asset through `crate::address::StateReadExt` and `crate::address::assets::StateReadExt` instead. - Update the mempool to be agnostic of address prefixes and use address bytes insteda: this change is in line with sequencer design considerations in that the address prefix only matters at the boundary while state reads are done via the address bytes underlying the bech32m addresses. - Require that the sequencer genesis contains a `TracePrefixed` asset. ## Testing Updated all tests to either use hard coded prefixes, native assets (if not performing reads or writes on state deltas), or write a base prefix and native asset to state before performing tests (if accessing state). ## Breaking Changelist While the change to `astria-core` is technically breaking (because a field that was of type `asset::Denom` is now `asset::TracePrefixed`), in practice this change is not breaking because Sequencer required native assets to be non-ibc prefixed. ## Related Issues Closes #1208 --- crates/astria-core/src/sequencer.rs | 13 +- .../src/genesis_example.rs | 2 +- .../astria-sequencer/src/accounts/action.rs | 20 +- .../src/accounts/component.rs | 20 +- .../src/accounts/state_ext.rs | 122 +++++--- crates/astria-sequencer/src/address/mod.rs | 106 ------- .../astria-sequencer/src/address/state_ext.rs | 46 ++- crates/astria-sequencer/src/api_state_ext.rs | 3 +- crates/astria-sequencer/src/app/mod.rs | 23 +- crates/astria-sequencer/src/app/test_utils.rs | 46 +-- crates/astria-sequencer/src/app/tests_app.rs | 87 +++--- .../src/app/tests_breaking_changes.rs | 72 +++-- .../src/app/tests_execute_transaction.rs | 295 +++++++++--------- crates/astria-sequencer/src/assets/mod.rs | 51 --- .../astria-sequencer/src/authority/action.rs | 41 +-- .../src/authority/state_ext.rs | 22 +- .../src/bridge/bridge_lock_action.rs | 29 +- .../src/bridge/bridge_sudo_change_action.rs | 67 ++-- .../src/bridge/bridge_unlock_action.rs | 90 +++--- .../src/bridge/init_bridge_account_action.rs | 28 +- crates/astria-sequencer/src/bridge/query.rs | 15 +- .../astria-sequencer/src/bridge/state_ext.rs | 42 ++- crates/astria-sequencer/src/grpc/sequencer.rs | 16 +- .../src/ibc/ibc_relayer_change.rs | 29 +- .../src/ibc/ics20_transfer.rs | 23 +- .../src/ibc/ics20_withdrawal.rs | 85 ++--- crates/astria-sequencer/src/ibc/state_ext.rs | 40 ++- .../src/mempool/benchmarks.rs | 18 +- crates/astria-sequencer/src/mempool/mod.rs | 121 ++++--- .../src/proposal/commitment.rs | 17 +- crates/astria-sequencer/src/sequencer.rs | 12 +- .../astria-sequencer/src/service/consensus.rs | 13 +- .../astria-sequencer/src/service/info/mod.rs | 21 +- .../astria-sequencer/src/service/mempool.rs | 27 +- crates/astria-sequencer/src/test_utils.rs | 35 ++- .../src/transaction/checks.rs | 118 ++++--- .../astria-sequencer/src/transaction/mod.rs | 62 ++-- 37 files changed, 1010 insertions(+), 867 deletions(-) diff --git a/crates/astria-core/src/sequencer.rs b/crates/astria-core/src/sequencer.rs index 940b67c99c..4138e96b6e 100644 --- a/crates/astria-core/src/sequencer.rs +++ b/crates/astria-core/src/sequencer.rs @@ -2,7 +2,10 @@ pub use penumbra_ibc::params::IBCParameters; use crate::primitive::v1::{ - asset, + asset::{ + self, + TracePrefixed, + }, Address, }; @@ -26,7 +29,7 @@ pub struct GenesisState { authority_sudo_address: Address, ibc_sudo_address: Address, ibc_relayer_addresses: Vec
, - native_asset_base_denomination: String, + native_asset_base_denomination: TracePrefixed, ibc_params: IBCParameters, allowed_fee_assets: Vec, fees: Fees, @@ -59,7 +62,7 @@ impl GenesisState { } #[must_use] - pub fn native_asset_base_denomination(&self) -> &str { + pub fn native_asset_base_denomination(&self) -> &TracePrefixed { &self.native_asset_base_denomination } @@ -140,7 +143,7 @@ pub struct UncheckedGenesisState { pub authority_sudo_address: Address, pub ibc_sudo_address: Address, pub ibc_relayer_addresses: Vec
, - pub native_asset_base_denomination: String, + pub native_asset_base_denomination: TracePrefixed, pub ibc_params: IBCParameters, pub allowed_fee_assets: Vec, pub fees: Fees, @@ -295,7 +298,7 @@ mod tests { authority_sudo_address: alice(), ibc_sudo_address: alice(), ibc_relayer_addresses: vec![alice(), bob()], - native_asset_base_denomination: "nria".to_string(), + native_asset_base_denomination: "nria".parse().unwrap(), ibc_params: IBCParameters { ibc_enabled: true, inbound_ics20_transfers_enabled: true, diff --git a/crates/astria-sequencer-utils/src/genesis_example.rs b/crates/astria-sequencer-utils/src/genesis_example.rs index f089dd7464..05d66baa07 100644 --- a/crates/astria-sequencer-utils/src/genesis_example.rs +++ b/crates/astria-sequencer-utils/src/genesis_example.rs @@ -68,7 +68,7 @@ fn genesis_state() -> GenesisState { authority_sudo_address: alice(), ibc_sudo_address: alice(), ibc_relayer_addresses: vec![alice(), bob()], - native_asset_base_denomination: "nria".to_string(), + native_asset_base_denomination: "nria".parse().unwrap(), ibc_params: IBCParameters { ibc_enabled: true, inbound_ics20_transfers_enabled: true, diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index 051258cf84..8d4853ea68 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -11,8 +11,11 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts, - accounts::StateReadExt as _, + accounts::{ + self, + StateReadExt as _, + }, + address, assets, bridge::StateReadExt as _, transaction::action_handler::ActionHandler, @@ -81,15 +84,16 @@ where #[async_trait::async_trait] impl ActionHandler for TransferAction { async fn check_stateless(&self) -> Result<()> { - crate::address::ensure_base_prefix(&self.to).context("destination address is invalid")?; Ok(()) } - async fn check_stateful( - &self, - state: &S, - from: Address, - ) -> Result<()> { + async fn check_stateful(&self, state: &S, from: Address) -> Result<()> + where + S: accounts::StateReadExt + address::StateReadExt + 'static, + { + state.ensure_base_prefix(&self.to).await.context( + "failed ensuring that the destination address matches the permitted base prefix", + )?; ensure!( state .get_bridge_account_rollup_id(&from) diff --git a/crates/astria-sequencer/src/accounts/component.rs b/crates/astria-sequencer/src/accounts/component.rs index 2e4addba3f..615cb24bbd 100644 --- a/crates/astria-sequencer/src/accounts/component.rs +++ b/crates/astria-sequencer/src/accounts/component.rs @@ -10,9 +10,9 @@ use tendermint::abci::request::{ }; use tracing::instrument; -use super::state_ext::StateWriteExt; use crate::{ - assets::get_native_asset, + accounts, + assets, component::Component, }; @@ -24,11 +24,17 @@ impl Component for AccountsComponent { type AppState = astria_core::sequencer::GenesisState; #[instrument(name = "AccountsComponent::init_chain", skip_all)] - async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> { - let native_asset = get_native_asset(); + async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> + where + S: accounts::StateWriteExt + assets::StateReadExt, + { + let native_asset = state + .get_native_asset() + .await + .context("failed to read native asset from state")?; for account in app_state.accounts() { state - .put_account_balance(account.address, native_asset, account.balance) + .put_account_balance(account.address, &native_asset, account.balance) .context("failed writing account balance to state")?; } @@ -39,7 +45,7 @@ impl Component for AccountsComponent { } #[instrument(name = "AccountsComponent::begin_block", skip_all)] - async fn begin_block( + async fn begin_block( _state: &mut Arc, _begin_block: &BeginBlock, ) -> Result<()> { @@ -47,7 +53,7 @@ impl Component for AccountsComponent { } #[instrument(name = "AccountsComponent::end_block", skip_all)] - async fn end_block( + async fn end_block( _state: &mut Arc, _end_block: &EndBlock, ) -> Result<()> { diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index a3d70710d5..fa0fe00a6c 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -3,9 +3,14 @@ use anyhow::{ Result, }; use astria_core::{ + crypto::{ + SigningKey, + VerificationKey, + }, primitive::v1::{ asset, Address, + ADDRESS_LEN, }, protocol::account::v1alpha1::AssetBalance, }; @@ -36,12 +41,49 @@ struct Fee(u128); const ACCOUNTS_PREFIX: &str = "accounts"; const TRANSFER_BASE_FEE_STORAGE_KEY: &str = "transferfee"; -struct StorageKey<'a>(&'a Address); -impl<'a> std::fmt::Display for StorageKey<'a> { +trait GetAddressBytes: Send + Sync { + fn get_address_bytes(&self) -> [u8; ADDRESS_LEN]; +} + +impl GetAddressBytes for Address { + fn get_address_bytes(&self) -> [u8; ADDRESS_LEN] { + self.bytes() + } +} + +impl GetAddressBytes for [u8; ADDRESS_LEN] { + fn get_address_bytes(&self) -> [u8; ADDRESS_LEN] { + *self + } +} + +impl GetAddressBytes for SigningKey { + fn get_address_bytes(&self) -> [u8; ADDRESS_LEN] { + self.verification_key().get_address_bytes() + } +} + +impl GetAddressBytes for VerificationKey { + fn get_address_bytes(&self) -> [u8; ADDRESS_LEN] { + self.address_bytes() + } +} + +impl<'a, T> GetAddressBytes for &'a T +where + T: GetAddressBytes, +{ + fn get_address_bytes(&self) -> [u8; ADDRESS_LEN] { + (*self).get_address_bytes() + } +} + +struct StorageKey<'a, T>(&'a T); +impl<'a, T: GetAddressBytes> std::fmt::Display for StorageKey<'a, T> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(ACCOUNTS_PREFIX)?; f.write_str("/")?; - for byte in self.0.bytes() { + for byte in self.0.get_address_bytes() { f.write_fmt(format_args!("{byte:02x}"))?; } Ok(()) @@ -59,16 +101,14 @@ fn balance_storage_key>( ) } -fn nonce_storage_key(address: Address) -> String { +fn nonce_storage_key(address: T) -> String { format!("{}/nonce", StorageKey(&address)) } #[async_trait] -pub(crate) trait StateReadExt: StateRead { +pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { #[instrument(skip_all)] async fn get_account_balances(&self, address: Address) -> Result> { - use crate::assets::StateReadExt as _; - let prefix = format!("{}/balance/", StorageKey(&address)); let mut balances: Vec = Vec::new(); @@ -94,10 +134,13 @@ pub(crate) trait StateReadExt: StateRead { let Balance(balance) = Balance::try_from_slice(&value).context("invalid balance bytes")?; - let native_asset = crate::assets::get_native_asset(); + let native_asset = self + .get_native_asset() + .await + .context("failed to read native asset from state")?; if asset == native_asset.to_ibc_prefixed() { balances.push(AssetBalance { - denom: native_asset.clone(), + denom: native_asset.into(), balance, }); continue; @@ -134,7 +177,7 @@ pub(crate) trait StateReadExt: StateRead { } #[instrument(skip_all)] - async fn get_account_nonce(&self, address: Address) -> Result { + async fn get_account_nonce(&self, address: T) -> Result { let bytes = self .get_raw(&nonce_storage_key(address)) .await @@ -264,9 +307,19 @@ mod tests { StateReadExt as _, StateWriteExt as _, }; - use crate::accounts::state_ext::{ - balance_storage_key, - nonce_storage_key, + use crate::{ + accounts::state_ext::{ + balance_storage_key, + nonce_storage_key, + }, + assets::{ + StateReadExt as _, + StateWriteExt as _, + }, + test_utils::{ + astria_address, + nria, + }, }; fn asset_0() -> astria_core::primitive::v1::asset::Denom { @@ -287,7 +340,7 @@ mod tests { let state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let nonce_expected = 0u32; // uninitialized accounts return zero @@ -308,7 +361,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let nonce_expected = 0u32; // can write new @@ -346,7 +399,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let nonce_expected = 2u32; // can write new @@ -363,7 +416,7 @@ mod tests { ); // writing additional account preserves first account's values - let address_1 = crate::address::base_prefixed([41u8; 20]); + let address_1 = astria_address(&[41u8; 20]); let nonce_expected_1 = 3u32; state @@ -394,7 +447,7 @@ mod tests { let state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset = asset_0(); let amount_expected = 0u128; @@ -416,7 +469,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset = asset_0(); let mut amount_expected = 1u128; @@ -458,7 +511,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset = asset_0(); let amount_expected = 1u128; @@ -478,7 +531,7 @@ mod tests { // writing to other accounts does not affect original account // create needed variables - let address_1 = crate::address::base_prefixed([41u8; 20]); + let address_1 = astria_address(&[41u8; 20]); let amount_expected_1 = 2u128; state @@ -511,7 +564,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset_0 = asset_0(); let asset_1 = asset_1(); let amount_expected_0 = 1u128; @@ -550,7 +603,7 @@ mod tests { let state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); // see that call was ok let balances = state @@ -562,21 +615,20 @@ mod tests { #[tokio::test] async fn get_account_balances() { - use crate::assets::StateWriteExt as _; let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); // need to set native asset in order to use `get_account_balances()` - crate::assets::initialize_native_asset("nria"); + state.put_native_asset(&nria()); - let asset_0 = crate::assets::get_native_asset(); + let asset_0 = state.get_native_asset().await.unwrap(); let asset_1 = asset_1(); let asset_2 = asset_2(); // also need to add assets to the ibc state state - .put_ibc_asset(&asset_0.clone().unwrap_trace_prefixed()) + .put_ibc_asset(&asset_0.clone()) .expect("should be able to call other trait method on state object"); state .put_ibc_asset(&asset_1.clone().unwrap_trace_prefixed()) @@ -586,18 +638,14 @@ mod tests { .expect("should be able to call other trait method on state object"); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let amount_expected_0 = 1u128; let amount_expected_1 = 2u128; let amount_expected_2 = 3u128; // add balances to the account state - .put_account_balance( - address, - asset_0.clone().unwrap_trace_prefixed(), - amount_expected_0, - ) + .put_account_balance(address, asset_0.clone(), amount_expected_0) .expect("putting an account balance should not fail"); state .put_account_balance(address, &asset_1, amount_expected_1) @@ -615,7 +663,7 @@ mod tests { balances, vec![ AssetBalance { - denom: asset_0.clone(), + denom: asset_0.into(), balance: amount_expected_0, }, AssetBalance { @@ -637,7 +685,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset = asset_0(); let amount_increase = 2u128; @@ -678,7 +726,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset = asset_0(); let amount_increase = 2u128; @@ -720,7 +768,7 @@ mod tests { let mut state = StateDelta::new(snapshot); // create needed variables - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let asset = asset_0(); let amount_increase = 2u128; diff --git a/crates/astria-sequencer/src/address/mod.rs b/crates/astria-sequencer/src/address/mod.rs index 65a9b00009..35bea31ada 100644 --- a/crates/astria-sequencer/src/address/mod.rs +++ b/crates/astria-sequencer/src/address/mod.rs @@ -1,111 +1,5 @@ -use anyhow::ensure; -use astria_core::primitive::v1::{ - Address, - AddressError, - ADDRESS_LEN, -}; - mod state_ext; -#[cfg(not(test))] -pub(crate) use regular::*; pub(crate) use state_ext::{ StateReadExt, StateWriteExt, }; -#[cfg(test)] -pub(crate) use testonly::*; - -pub(crate) fn base_prefixed(arr: [u8; ADDRESS_LEN]) -> Address { - Address::builder() - .array(arr) - .prefix(get_base_prefix()) - .try_build() - .expect("the prefix must have been set as a valid bech32 prefix, so this should never fail") -} - -pub(crate) fn try_base_prefixed(slice: &[u8]) -> Result { - Address::builder() - .slice(slice) - .prefix(get_base_prefix()) - .try_build() -} - -pub(crate) fn ensure_base_prefix(address: &Address) -> anyhow::Result<()> { - ensure!( - get_base_prefix() == address.prefix(), - "address has prefix `{}` but only `{}` is permitted", - address.prefix(), - crate::address::get_base_prefix(), - ); - Ok(()) -} - -#[cfg(not(test))] -mod regular { - //! Logic to be used for a normal debug or release build of sequencer. - - use std::sync::OnceLock; - - use anyhow::Context as _; - - static BASE_PREFIX: OnceLock = OnceLock::new(); - - pub(crate) fn initialize_base_prefix(base_prefix: &str) -> anyhow::Result<()> { - // construct a dummy address to see if we can construct it; fail otherwise. - try_construct_dummy_address_from_prefix(base_prefix) - .context("failed constructing a dummy address from the provided prefix")?; - - BASE_PREFIX.set(base_prefix.to_string()).expect( - "THIS IS A BUG: attempted to set the base prefix more than once; it should only be set - once when serving the `InitChain` consensus request, or immediately after Sequencer is - restarted. It cannot be initialized twice or concurrently from more than one task or \ - thread.", - ); - - Ok(()) - } - - pub(crate) fn get_base_prefix() -> &'static str { - BASE_PREFIX - .get() - .expect( - "the base prefix must have been set while serving the `InitChain` consensus \ - request or upon Sequencer restart; if not set, the chain was initialized \ - incorrectly, or the base prefix not read from storage", - ) - .as_str() - } - - fn try_construct_dummy_address_from_prefix( - s: &str, - ) -> Result<(), astria_core::primitive::v1::AddressError> { - use astria_core::primitive::v1::{ - Address, - ADDRESS_LEN, - }; - // construct a dummy address to see if we can construct it; fail otherwise. - Address::builder() - .array([0u8; ADDRESS_LEN]) - .prefix(s) - .try_build() - .map(|_| ()) - } -} - -#[cfg(test)] -mod testonly { - // allow: this has to match the definition of the non-test function - #[allow(clippy::unnecessary_wraps)] - pub(crate) fn initialize_base_prefix(base_prefix: &str) -> anyhow::Result<()> { - assert_eq!( - base_prefix, - get_base_prefix(), - "all tests should be initialized with a \"astria\" as the base prefix" - ); - Ok(()) - } - - pub(crate) fn get_base_prefix() -> &'static str { - "astria" - } -} diff --git a/crates/astria-sequencer/src/address/state_ext.rs b/crates/astria-sequencer/src/address/state_ext.rs index 917e6ce422..924479107a 100644 --- a/crates/astria-sequencer/src/address/state_ext.rs +++ b/crates/astria-sequencer/src/address/state_ext.rs @@ -1,8 +1,10 @@ use anyhow::{ bail, + ensure, Context as _, Result, }; +use astria_core::primitive::v1::Address; use async_trait::async_trait; use cnidarium::{ StateRead, @@ -16,6 +18,31 @@ fn base_prefix_key() -> &'static str { #[async_trait] pub(crate) trait StateReadExt: StateRead { + async fn ensure_base_prefix(&self, address: &Address) -> anyhow::Result<()> { + let prefix = self + .get_base_prefix() + .await + .context("failed to read base prefix from state")?; + ensure!( + prefix == address.prefix(), + "address has prefix `{}` but only `{prefix}` is permitted", + address.prefix(), + ); + Ok(()) + } + + async fn try_base_prefixed(&self, slice: &[u8]) -> anyhow::Result
{ + let prefix = self + .get_base_prefix() + .await + .context("failed to read base prefix from state")?; + Address::builder() + .slice(slice) + .prefix(prefix) + .try_build() + .context("failed to construct address from byte slice and state-provided base prefix") + } + #[instrument(skip_all)] async fn get_base_prefix(&self) -> Result { let Some(bytes) = self @@ -34,13 +61,28 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_base_prefix(&mut self, prefix: &str) { + fn put_base_prefix(&mut self, prefix: &str) -> anyhow::Result<()> { + try_construct_dummy_address_from_prefix(prefix) + .context("failed constructing a dummy address from the provided prefix")?; self.put_raw(base_prefix_key().into(), prefix.into()); + Ok(()) } } impl StateWriteExt for T {} +fn try_construct_dummy_address_from_prefix( + s: &str, +) -> Result<(), astria_core::primitive::v1::AddressError> { + use astria_core::primitive::v1::ADDRESS_LEN; + // construct a dummy address to see if we can construct it; fail otherwise. + Address::builder() + .array([0u8; ADDRESS_LEN]) + .prefix(s) + .try_build() + .map(|_| ()) +} + #[cfg(test)] mod test { use cnidarium::StateDelta; @@ -56,7 +98,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix("astria"); + state.put_base_prefix("astria").unwrap(); assert_eq!("astria", &state.get_base_prefix().await.unwrap()); } } diff --git a/crates/astria-sequencer/src/api_state_ext.rs b/crates/astria-sequencer/src/api_state_ext.rs index 79c8c1512e..ec3fe43a40 100644 --- a/crates/astria-sequencer/src/api_state_ext.rs +++ b/crates/astria-sequencer/src/api_state_ext.rs @@ -390,6 +390,7 @@ mod test { use rand::Rng; use super::*; + use crate::test_utils::astria_address; // creates new sequencer block, optionally shifting all values except the height by 1 fn make_test_sequencer_block(height: u32) -> SequencerBlock { @@ -400,7 +401,7 @@ mod test { let mut deposits = vec![]; for _ in 0..2 { let rollup_id = RollupId::new(rng.gen()); - let bridge_address = crate::address::base_prefixed([rng.gen(); 20]); + let bridge_address = astria_address(&[rng.gen(); 20]); let amount = rng.gen::(); let asset = "testasset".parse().unwrap(); let destination_chain_address = rng.gen::().to_string(); diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index 6e052d0c1d..cc59071f13 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -14,7 +14,6 @@ use std::{ use anyhow::{ anyhow, - bail, ensure, Context, }; @@ -216,22 +215,16 @@ impl App { .try_begin_transaction() .expect("state Arc should not be referenced elsewhere"); - crate::address::initialize_base_prefix(&genesis_state.address_prefixes().base) - .context("failed setting global base prefix")?; - state_tx.put_base_prefix(&genesis_state.address_prefixes().base); + state_tx + .put_base_prefix(&genesis_state.address_prefixes().base) + .context("failed to write base prefix to state")?; - crate::assets::initialize_native_asset(genesis_state.native_asset_base_denomination()); - let Some(native_asset) = crate::assets::get_native_asset() - .as_trace_prefixed() - .cloned() - else { - bail!("native asset must be trace-prefixed, not of form `ibc/`") - }; + let native_asset = genesis_state.native_asset_base_denomination(); + state_tx.put_native_asset(native_asset); state_tx - .put_ibc_asset(&native_asset) - .context("failed to put native asset")?; + .put_ibc_asset(native_asset) + .context("failed to commit native asset as ibc asset to state")?; - state_tx.put_native_asset(&native_asset); state_tx.put_chain_id_and_revision_number(chain_id.try_into().context("invalid chain ID")?); state_tx.put_block_height(0); @@ -1149,7 +1142,7 @@ async fn update_mempool_after_finalization( mempool: &mut Mempool, state: S, ) -> anyhow::Result<()> { - let current_account_nonce_getter = |address: Address| state.get_account_nonce(address); + let current_account_nonce_getter = |address: [u8; 20]| state.get_account_nonce(address); mempool.run_maintenance(current_account_nonce_getter).await } diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index d942e99ec5..6ef60fac9c 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -1,10 +1,6 @@ use astria_core::{ crypto::SigningKey, - primitive::v1::{ - Address, - RollupId, - ADDRESS_LEN, - }, + primitive::v1::RollupId, protocol::transaction::v1alpha1::{ action::{ SequenceAction, @@ -29,57 +25,46 @@ use crate::{ app::App, mempool::Mempool, metrics::Metrics, + test_utils::astria_address_from_hex_string, }; -// attempts to decode the given hex string into an address. -pub(crate) fn address_from_hex_string(s: &str) -> Address { - let bytes = hex::decode(s).unwrap(); - let arr: [u8; ADDRESS_LEN] = bytes.try_into().unwrap(); - crate::address::base_prefixed(arr) -} - pub(crate) const ALICE_ADDRESS: &str = "1c0c490f1b5528d8173c5de46d131160e4b2c0c3"; pub(crate) const BOB_ADDRESS: &str = "34fec43c7fcab9aef3b3cf8aba855e41ee69ca3a"; pub(crate) const CAROL_ADDRESS: &str = "60709e2d391864b732b4f0f51e387abb76743871"; pub(crate) const JUDY_ADDRESS: &str = "bc5b91da07778eeaf622d0dcf4d7b4233525998d"; pub(crate) const TED_ADDRESS: &str = "4c4f91d8a918357ab5f6f19c1e179968fc39bb44"; -pub(crate) fn get_alice_signing_key_and_address() -> (SigningKey, Address) { +pub(crate) fn get_alice_signing_key() -> SigningKey { // this secret key corresponds to ALICE_ADDRESS let alice_secret_bytes: [u8; 32] = hex::decode("2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90") .unwrap() .try_into() .unwrap(); - let alice_signing_key = SigningKey::from(alice_secret_bytes); - let alice = crate::address::base_prefixed(alice_signing_key.verification_key().address_bytes()); - (alice_signing_key, alice) + SigningKey::from(alice_secret_bytes) } -pub(crate) fn get_bridge_signing_key_and_address() -> (SigningKey, Address) { +pub(crate) fn get_bridge_signing_key() -> SigningKey { let bridge_secret_bytes: [u8; 32] = hex::decode("db4982e01f3eba9e74ac35422fcd49aa2b47c3c535345c7e7da5220fe3a0ce79") .unwrap() .try_into() .unwrap(); - let bridge_signing_key = SigningKey::from(bridge_secret_bytes); - let bridge = - crate::address::base_prefixed(bridge_signing_key.verification_key().address_bytes()); - (bridge_signing_key, bridge) + SigningKey::from(bridge_secret_bytes) } pub(crate) fn default_genesis_accounts() -> Vec { vec![ Account { - address: address_from_hex_string(ALICE_ADDRESS), + address: astria_address_from_hex_string(ALICE_ADDRESS), balance: 10u128.pow(19), }, Account { - address: address_from_hex_string(BOB_ADDRESS), + address: astria_address_from_hex_string(BOB_ADDRESS), balance: 10u128.pow(19), }, Account { - address: address_from_hex_string(CAROL_ADDRESS), + address: astria_address_from_hex_string(CAROL_ADDRESS), balance: 10u128.pow(19), }, ] @@ -101,14 +86,14 @@ pub(crate) fn unchecked_genesis_state() -> UncheckedGenesisState { UncheckedGenesisState { accounts: default_genesis_accounts(), address_prefixes: AddressPrefixes { - base: crate::address::get_base_prefix().to_string(), + base: crate::test_utils::ASTRIA_PREFIX.into(), }, - authority_sudo_address: address_from_hex_string(JUDY_ADDRESS), - ibc_sudo_address: address_from_hex_string(TED_ADDRESS), + authority_sudo_address: astria_address_from_hex_string(JUDY_ADDRESS), + ibc_sudo_address: astria_address_from_hex_string(TED_ADDRESS), ibc_relayer_addresses: vec![], - native_asset_base_denomination: "nria".to_string(), + native_asset_base_denomination: crate::test_utils::nria(), ibc_params: IBCParameters::default(), - allowed_fee_assets: vec!["nria".parse().unwrap()], + allowed_fee_assets: vec![crate::test_utils::nria().into()], fees: default_fees(), } } @@ -153,7 +138,6 @@ pub(crate) async fn initialize_app( } pub(crate) fn get_mock_tx(nonce: u32) -> SignedTransaction { - let (alice_signing_key, _) = get_alice_signing_key_and_address(); let tx = UnsignedTransaction { params: TransactionParams::builder() .nonce(nonce) @@ -169,5 +153,5 @@ pub(crate) fn get_mock_tx(nonce: u32) -> SignedTransaction { ], }; - tx.into_signed(&alice_signing_key) + tx.into_signed(&get_alice_signing_key()) } diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index c34047f79f..762a27ba1c 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -41,7 +41,7 @@ use super::*; use crate::{ accounts::StateReadExt as _, app::test_utils::*, - assets::get_native_asset, + assets::StateReadExt as _, authority::{ StateReadExt as _, StateWriteExt as _, @@ -53,7 +53,12 @@ use crate::{ }, proposal::commitment::generate_rollup_datas_commitment, state_ext::StateReadExt as _, - test_utils::verification_key, + test_utils::{ + astria_address, + astria_address_from_hex_string, + nria, + verification_key, + }, }; fn default_tendermint_header() -> Header { @@ -91,7 +96,7 @@ async fn app_genesis_and_init_chain() { assert_eq!( balance, app.state - .get_account_balance(address, get_native_asset()) + .get_account_balance(address, nria()) .await .unwrap(), ); @@ -177,7 +182,6 @@ async fn app_commit() { let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; assert_eq!(app.state.get_block_height().await.unwrap(), 0); - let native_asset = get_native_asset(); for Account { address, balance, @@ -186,7 +190,7 @@ async fn app_commit() { assert_eq!( balance, app.state - .get_account_balance(address, native_asset) + .get_account_balance(address, nria()) .await .unwrap() ); @@ -205,10 +209,7 @@ async fn app_commit() { } in default_genesis_accounts() { assert_eq!( - snapshot - .get_account_balance(address, native_asset) - .await - .unwrap(), + snapshot.get_account_balance(address, nria()).await.unwrap(), balance ); } @@ -218,11 +219,10 @@ async fn app_commit() { async fn app_transfer_block_fees_to_sudo() { let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); - let native_asset = get_native_asset().clone(); + let alice = get_alice_signing_key(); // transfer funds from Alice to Bob; use native token for fee payment - let bob_address = address_from_hex_string(BOB_ADDRESS); + let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let amount = 333_333; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -233,14 +233,14 @@ async fn app_transfer_block_fees_to_sudo() { TransferAction { to: bob_address, amount, - asset: native_asset.clone(), - fee_asset: native_asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = tx.into_signed(&alice_signing_key); + let signed_tx = tx.into_signed(&alice); let proposer_address: tendermint::account::Id = [99u8; 20].to_vec().try_into().unwrap(); @@ -268,7 +268,7 @@ async fn app_transfer_block_fees_to_sudo() { let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); assert_eq!( app.state - .get_account_balance(address_from_hex_string(JUDY_ADDRESS), native_asset) + .get_account_balance(astria_address_from_hex_string(JUDY_ADDRESS), nria()) .await .unwrap(), transfer_fee, @@ -285,17 +285,16 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { use crate::api_state_ext::StateReadExt as _; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset().clone(); let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(&bridge_address, &asset) + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); app.prepare_commit(storage.clone()).await.unwrap(); @@ -305,14 +304,14 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { let lock_action = BridgeLockAction { to: bridge_address, amount, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), }; let sequence_action = SequenceAction { rollup_id, data: b"hello world".to_vec(), - fee_asset: asset.clone(), + fee_asset: nria().into(), }; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -322,13 +321,13 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { actions: vec![lock_action.into(), sequence_action.into()], }; - let signed_tx = tx.into_signed(&alice_signing_key); + let signed_tx = tx.into_signed(&alice); let expected_deposit = Deposit::new( bridge_address, rollup_id, amount, - asset, + nria().into(), "nootwashere".to_string(), ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); @@ -375,12 +374,12 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { #[tokio::test] #[allow(clippy::too_many_lines)] async fn app_execution_results_match_proposal_vs_after_proposal() { - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset().clone(); + let asset = nria().clone(); let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); @@ -395,14 +394,14 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { let lock_action = BridgeLockAction { to: bridge_address, amount, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), }; let sequence_action = SequenceAction { rollup_id, data: b"hello world".to_vec(), - fee_asset: asset.clone(), + fee_asset: nria().into(), }; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -412,13 +411,13 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { actions: vec![lock_action.into(), sequence_action.into()], }; - let signed_tx = tx.into_signed(&alice_signing_key); + let signed_tx = tx.into_signed(&alice); let expected_deposit = Deposit::new( bridge_address, rollup_id, amount, - asset, + nria().into(), "nootwashere".to_string(), ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); @@ -527,7 +526,7 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { app.commit(storage.clone()).await; // create txs which will cause cometBFT overflow - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let tx_pass = UnsignedTransaction { params: TransactionParams::builder() .nonce(0) @@ -537,12 +536,12 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 100_000], - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], } - .into_signed(&alice_signing_key); + .into_signed(&alice); let tx_overflow = UnsignedTransaction { params: TransactionParams::builder() .nonce(1) @@ -552,12 +551,12 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 100_000], - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], } - .into_signed(&alice_signing_key); + .into_signed(&alice); app.mempool.insert(tx_pass, 0).await.unwrap(); app.mempool.insert(tx_overflow, 0).await.unwrap(); @@ -600,7 +599,7 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { app.commit(storage.clone()).await; // create txs which will cause sequencer overflow (max is currently 256_000 bytes) - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let tx_pass = UnsignedTransaction { params: TransactionParams::builder() .nonce(0) @@ -610,12 +609,12 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 200_000], - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], } - .into_signed(&alice_signing_key); + .into_signed(&alice); let tx_overflow = UnsignedTransaction { params: TransactionParams::builder() .nonce(1) @@ -625,12 +624,12 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 100_000], - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], } - .into_signed(&alice_signing_key); + .into_signed(&alice); app.mempool.insert(tx_pass, 0).await.unwrap(); app.mempool.insert(tx_overflow, 0).await.unwrap(); @@ -680,7 +679,7 @@ async fn app_end_block_validator_updates() { ]; let mut app = initialize_app(None, initial_validator_set).await; - let proposer_address = crate::address::base_prefixed([0u8; 20]); + let proposer_address = astria_address(&[0u8; 20]); let validator_updates = vec![ ValidatorUpdate { diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 209a6317e0..b7c11a8bf1 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -50,36 +50,41 @@ use tendermint::{ use crate::{ app::test_utils::{ - address_from_hex_string, default_fees, default_genesis_accounts, - get_alice_signing_key_and_address, - get_bridge_signing_key_and_address, + get_alice_signing_key, + get_bridge_signing_key, initialize_app, initialize_app_with_storage, BOB_ADDRESS, CAROL_ADDRESS, }, - assets::get_native_asset, bridge::StateWriteExt as _, proposal::commitment::generate_rollup_datas_commitment, + test_utils::{ + astria_address, + astria_address_from_hex_string, + nria, + ASTRIA_PREFIX, + }, }; /// XXX: This should be expressed in terms of `crate::app::test_utils::unchecked_genesis_state` to /// be consistent everywhere. `get_alice_signing_key` already is, why not this? fn unchecked_genesis_state() -> UncheckedGenesisState { - let (_, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); UncheckedGenesisState { accounts: vec![], address_prefixes: AddressPrefixes { - base: crate::address::get_base_prefix().to_string(), + base: ASTRIA_PREFIX.into(), }, authority_sudo_address: alice_address, ibc_sudo_address: alice_address, ibc_relayer_addresses: vec![], - native_asset_base_denomination: "nria".to_string(), + native_asset_base_denomination: nria(), ibc_params: IBCParameters::default(), - allowed_fee_assets: vec!["nria".parse().unwrap()], + allowed_fee_assets: vec![nria().into()], fees: default_fees(), } } @@ -92,17 +97,16 @@ async fn app_genesis_snapshot() { #[tokio::test] async fn app_finalize_block_snapshot() { - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset().clone(); let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(&bridge_address, &asset) + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); @@ -115,14 +119,14 @@ async fn app_finalize_block_snapshot() { let lock_action = BridgeLockAction { to: bridge_address, amount, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), }; let sequence_action = SequenceAction { rollup_id, data: b"hello world".to_vec(), - fee_asset: asset.clone(), + fee_asset: nria().into(), }; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -132,13 +136,13 @@ async fn app_finalize_block_snapshot() { actions: vec![lock_action.into(), sequence_action.into()], }; - let signed_tx = tx.into_signed(&alice_signing_key); + let signed_tx = tx.into_signed(&alice); let expected_deposit = Deposit::new( bridge_address, rollup_id, amount, - asset, + nria().into(), "nootwashere".to_string(), ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); @@ -180,10 +184,11 @@ async fn app_execute_transaction_with_every_action_snapshot() { SudoAddressChangeAction, }; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); - let (bridge_signing_key, bridge_address) = get_bridge_signing_key_and_address(); - let bob_address = address_from_hex_string(BOB_ADDRESS); - let carol_address = address_from_hex_string(CAROL_ADDRESS); + let alice = get_alice_signing_key(); + let bridge = get_bridge_signing_key(); + let bridge_address = astria_address(&bridge.address_bytes()); + let bob_address = astria_address_from_hex_string(BOB_ADDRESS); + let carol_address = astria_address_from_hex_string(CAROL_ADDRESS); let mut accounts = default_genesis_accounts(); accounts.push(Account { address: bridge_address, @@ -205,7 +210,6 @@ async fn app_execute_transaction_with_every_action_snapshot() { }; let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset().clone(); let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -216,14 +220,14 @@ async fn app_execute_transaction_with_every_action_snapshot() { TransferAction { to: bob_address, amount: 333_333, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), } .into(), SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"hello world".to_vec(), - fee_asset: asset.clone(), + fee_asset: nria().into(), } .into(), Action::ValidatorUpdate(update.clone()), @@ -241,7 +245,7 @@ async fn app_execute_transaction_with_every_action_snapshot() { ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); let tx = UnsignedTransaction { @@ -252,15 +256,15 @@ async fn app_execute_transaction_with_every_action_snapshot() { actions: vec![ InitBridgeAccountAction { rollup_id, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), sudo_address: None, withdrawer_address: None, } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&bridge_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&bridge)); app.execute_transaction(signed_tx).await.unwrap(); let tx = UnsignedTransaction { @@ -272,15 +276,15 @@ async fn app_execute_transaction_with_every_action_snapshot() { BridgeLockAction { to: bridge_address, amount: 100, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), } .into(), BridgeUnlockAction { to: bob_address, amount: 10, - fee_asset: asset.clone(), + fee_asset: nria().into(), memo: "{}".into(), bridge_address: None, } @@ -289,13 +293,13 @@ async fn app_execute_transaction_with_every_action_snapshot() { bridge_address, new_sudo_address: Some(bob_address), new_withdrawer_address: Some(bob_address), - fee_asset: asset.clone(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&bridge_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&bridge)); app.execute_transaction(signed_tx).await.unwrap(); app.prepare_commit(storage.clone()).await.unwrap(); diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index a596f6d6e5..f0c83ddfa6 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -34,10 +34,7 @@ use tendermint::abci::EventAttributeIndexExt as _; use crate::{ accounts::StateReadExt as _, app::test_utils::*, - assets::{ - get_native_asset, - StateReadExt as _, - }, + assets::StateReadExt as _, authority::StateReadExt as _, bridge::{ StateReadExt as _, @@ -45,32 +42,32 @@ use crate::{ }, ibc::StateReadExt as _, sequence::calculate_fee_from_state, + test_utils::{ + astria_address, + astria_address_from_hex_string, + nria, + }, transaction::{ InvalidChainId, InvalidNonce, }, }; -const DEFAULT_NATIVE_ASSET_DENOM: &str = "nria"; -fn default_native_asset() -> asset::Denom { - DEFAULT_NATIVE_ASSET_DENOM.parse().unwrap() -} - /// XXX: This should be expressed in terms of `crate::app::test_utils::unchecked_genesis_state` to /// be consistent everywhere. `get_alice_sining_key` already is, why not this?? fn unchecked_genesis_state() -> UncheckedGenesisState { - let (_, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); UncheckedGenesisState { accounts: default_genesis_accounts(), address_prefixes: AddressPrefixes { - base: crate::address::get_base_prefix().to_string(), + base: crate::test_utils::ASTRIA_PREFIX.into(), }, - authority_sudo_address: alice_address, - ibc_sudo_address: alice_address, + authority_sudo_address: crate::test_utils::astria_address(&alice.address_bytes()), + ibc_sudo_address: crate::test_utils::astria_address(&alice.address_bytes()), ibc_relayer_addresses: vec![], - native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), + native_asset_base_denomination: crate::test_utils::nria(), ibc_params: IBCParameters::default(), - allowed_fee_assets: vec![default_native_asset()], + allowed_fee_assets: vec![crate::test_utils::nria().into()], fees: default_fees(), } } @@ -88,8 +85,9 @@ async fn app_execute_transaction_transfer() { let mut app = initialize_app(None, vec![]).await; // transfer funds from Alice to Bob - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); - let bob_address = address_from_hex_string(BOB_ADDRESS); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let value = 333_333; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -100,20 +98,19 @@ async fn app_execute_transaction_transfer() { TransferAction { to: bob_address, amount: value, - asset: get_native_asset().clone(), - fee_asset: get_native_asset().clone(), + asset: crate::test_utils::nria().into(), + fee_asset: crate::test_utils::nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - let native_asset = get_native_asset(); assert_eq!( app.state - .get_account_balance(bob_address, native_asset) + .get_account_balance(bob_address, nria()) .await .unwrap(), value + 10u128.pow(19) @@ -121,7 +118,7 @@ async fn app_execute_transaction_transfer() { let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); assert_eq!( app.state - .get_account_balance(alice_address, native_asset) + .get_account_balance(alice_address, nria()) .await .unwrap(), 10u128.pow(19) - (value + transfer_fee), @@ -137,17 +134,18 @@ async fn app_execute_transaction_transfer_not_native_token() { let mut app = initialize_app(None, vec![]).await; // create some asset to be transferred and update Alice's balance of it - let asset = test_asset(); let value = 333_333; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + let mut state_tx = StateDelta::new(app.state.clone()); state_tx - .put_account_balance(alice_address, &asset, value) + .put_account_balance(alice_address, &test_asset(), value) .unwrap(); app.apply(state_tx); // transfer funds from Alice to Bob; use native token for fee payment - let bob_address = address_from_hex_string(BOB_ADDRESS); + let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let tx = UnsignedTransaction { params: TransactionParams::builder() .nonce(0) @@ -157,27 +155,26 @@ async fn app_execute_transaction_transfer_not_native_token() { TransferAction { to: bob_address, amount: value, - asset: asset.clone(), - fee_asset: get_native_asset().clone(), + asset: test_asset(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); - let native_asset = get_native_asset(); assert_eq!( app.state - .get_account_balance(bob_address, native_asset) + .get_account_balance(bob_address, nria()) .await .unwrap(), 10u128.pow(19), // genesis balance ); assert_eq!( app.state - .get_account_balance(bob_address, &asset) + .get_account_balance(bob_address, test_asset()) .await .unwrap(), value, // transferred amount @@ -186,14 +183,14 @@ async fn app_execute_transaction_transfer_not_native_token() { let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); assert_eq!( app.state - .get_account_balance(alice_address, native_asset) + .get_account_balance(alice_address, nria()) .await .unwrap(), 10u128.pow(19) - transfer_fee, // genesis balance - fee ); assert_eq!( app.state - .get_account_balance(alice_address, &asset) + .get_account_balance(alice_address, test_asset()) .await .unwrap(), 0, // 0 since all funds of `asset` were transferred @@ -211,7 +208,7 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { // create a new key; will have 0 balance let keypair = SigningKey::new(OsRng); - let bob = address_from_hex_string(BOB_ADDRESS); + let bob = astria_address_from_hex_string(BOB_ADDRESS); // 0-value transfer; only fee is deducted from sender let tx = UnsignedTransaction { @@ -223,8 +220,8 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { TransferAction { to: bob, amount: 0, - asset: get_native_asset().clone(), - fee_asset: get_native_asset().clone(), + asset: nria().into(), + fee_asset: nria().into(), } .into(), ], @@ -250,7 +247,8 @@ async fn app_execute_transaction_sequence() { state_tx.put_sequence_action_byte_cost_multiplier(1); app.apply(state_tx); - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let data = b"hello world".to_vec(); let fee = calculate_fee_from_state(&data, &app.state).await.unwrap(); @@ -263,19 +261,19 @@ async fn app_execute_transaction_sequence() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); assert_eq!( app.state - .get_account_balance(alice_address, get_native_asset()) + .get_account_balance(alice_address, nria()) .await .unwrap(), 10u128.pow(19) - fee, @@ -286,11 +284,9 @@ async fn app_execute_transaction_sequence() { async fn app_execute_transaction_invalid_fee_asset() { let mut app = initialize_app(None, vec![]).await; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let data = b"hello world".to_vec(); - let fee_asset = test_asset(); - let tx = UnsignedTransaction { params: TransactionParams::builder() .nonce(0) @@ -300,19 +296,20 @@ async fn app_execute_transaction_invalid_fee_asset() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset, + fee_asset: test_asset(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); } #[tokio::test] async fn app_execute_transaction_validator_update() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let mut app = initialize_app(Some(genesis_state()), vec![]).await; @@ -329,7 +326,7 @@ async fn app_execute_transaction_validator_update() { actions: vec![Action::ValidatorUpdate(update.clone())], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); @@ -343,7 +340,8 @@ async fn app_execute_transaction_validator_update() { #[tokio::test] async fn app_execute_transaction_ibc_relayer_change_addition() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let mut app = initialize_app(Some(genesis_state()), vec![]).await; @@ -355,7 +353,7 @@ async fn app_execute_transaction_ibc_relayer_change_addition() { actions: vec![IbcRelayerChangeAction::Addition(alice_address).into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); assert!(app.state.is_ibc_relayer(&alice_address).await.unwrap()); @@ -363,7 +361,8 @@ async fn app_execute_transaction_ibc_relayer_change_addition() { #[tokio::test] async fn app_execute_transaction_ibc_relayer_change_deletion() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let genesis_state = UncheckedGenesisState { ibc_relayer_addresses: vec![alice_address], @@ -381,7 +380,7 @@ async fn app_execute_transaction_ibc_relayer_change_deletion() { actions: vec![IbcRelayerChangeAction::Removal(alice_address).into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); assert!(!app.state.is_ibc_relayer(&alice_address).await.unwrap()); @@ -389,10 +388,10 @@ async fn app_execute_transaction_ibc_relayer_change_deletion() { #[tokio::test] async fn app_execute_transaction_ibc_relayer_change_invalid() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); - + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let genesis_state = UncheckedGenesisState { - ibc_sudo_address: crate::address::base_prefixed([0; 20]), + ibc_sudo_address: astria_address(&[0; 20]), ibc_relayer_addresses: vec![alice_address], ..unchecked_genesis_state() } @@ -408,17 +407,18 @@ async fn app_execute_transaction_ibc_relayer_change_invalid() { actions: vec![IbcRelayerChangeAction::Removal(alice_address).into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); } #[tokio::test] async fn app_execute_transaction_sudo_address_change() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let mut app = initialize_app(Some(genesis_state()), vec![]).await; - let new_address = address_from_hex_string(BOB_ADDRESS); + let new_address = astria_address_from_hex_string(BOB_ADDRESS); let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -430,7 +430,7 @@ async fn app_execute_transaction_sudo_address_change() { })], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); @@ -440,12 +440,13 @@ async fn app_execute_transaction_sudo_address_change() { #[tokio::test] async fn app_execute_transaction_sudo_address_change_error() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); - let authority_sudo_address = address_from_hex_string(CAROL_ADDRESS); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + let authority_sudo_address = astria_address_from_hex_string(CAROL_ADDRESS); let genesis_state = UncheckedGenesisState { authority_sudo_address, - ibc_sudo_address: crate::address::base_prefixed([0u8; 20]), + ibc_sudo_address: astria_address(&[0u8; 20]), ..unchecked_genesis_state() } .try_into() @@ -462,7 +463,7 @@ async fn app_execute_transaction_sudo_address_change_error() { })], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let res = app .execute_transaction(signed_tx) .await @@ -476,38 +477,37 @@ async fn app_execute_transaction_sudo_address_change_error() { async fn app_execute_transaction_fee_asset_change_addition() { use astria_core::protocol::transaction::v1alpha1::action::FeeAssetChangeAction; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let mut app = initialize_app(Some(genesis_state()), vec![]).await; - let new_asset = test_asset(); - let tx = UnsignedTransaction { params: TransactionParams::builder() .nonce(0) .chain_id("test") .build(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Addition( - new_asset.clone(), + test_asset(), ))], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); - assert!(app.state.is_allowed_fee_asset(&new_asset).await.unwrap()); + assert!(app.state.is_allowed_fee_asset(&test_asset()).await.unwrap()); } #[tokio::test] async fn app_execute_transaction_fee_asset_change_removal() { use astria_core::protocol::transaction::v1alpha1::action::FeeAssetChangeAction; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); - let test_asset = test_asset(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let genesis_state = UncheckedGenesisState { - allowed_fee_assets: vec![default_native_asset(), test_asset.clone()], + allowed_fee_assets: vec![nria().into(), test_asset()], ..unchecked_genesis_state() } .try_into() @@ -520,22 +520,22 @@ async fn app_execute_transaction_fee_asset_change_removal() { .chain_id("test") .build(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( - test_asset.clone(), + test_asset(), ))], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); - assert!(!app.state.is_allowed_fee_asset(&test_asset).await.unwrap()); + assert!(!app.state.is_allowed_fee_asset(test_asset()).await.unwrap()); } #[tokio::test] async fn app_execute_transaction_fee_asset_change_invalid() { use astria_core::protocol::transaction::v1alpha1::action::FeeAssetChangeAction; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let mut app = initialize_app(Some(genesis_state()), vec![]).await; @@ -545,11 +545,11 @@ async fn app_execute_transaction_fee_asset_change_invalid() { .chain_id("test") .build(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( - get_native_asset().clone(), + nria().into(), ))], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let res = app .execute_transaction(signed_tx) .await @@ -563,7 +563,9 @@ async fn app_execute_transaction_fee_asset_change_invalid() { async fn app_execute_transaction_init_bridge_account_ok() { use astria_core::protocol::transaction::v1alpha1::action::InitBridgeAccountAction; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); let fee = 12; // arbitrary @@ -571,11 +573,10 @@ async fn app_execute_transaction_init_bridge_account_ok() { app.apply(state_tx); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset().clone(); let action = InitBridgeAccountAction { rollup_id, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), sudo_address: None, withdrawer_address: None, }; @@ -587,11 +588,11 @@ async fn app_execute_transaction_init_bridge_account_ok() { actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let before_balance = app .state - .get_account_balance(alice_address, &asset) + .get_account_balance(alice_address, nria()) .await .unwrap(); app.execute_transaction(signed_tx).await.unwrap(); @@ -609,11 +610,11 @@ async fn app_execute_transaction_init_bridge_account_ok() { .get_bridge_account_ibc_asset(&alice_address) .await .unwrap(), - asset.to_ibc_prefixed(), + nria().to_ibc_prefixed(), ); assert_eq!( app.state - .get_account_balance(alice_address, &asset) + .get_account_balance(alice_address, &nria()) .await .unwrap(), before_balance - fee, @@ -624,15 +625,14 @@ async fn app_execute_transaction_init_bridge_account_ok() { async fn app_execute_transaction_init_bridge_account_account_already_registered() { use astria_core::protocol::transaction::v1alpha1::action::InitBridgeAccountAction; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let mut app = initialize_app(None, vec![]).await; let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset(); let action = InitBridgeAccountAction { rollup_id, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), sudo_address: None, withdrawer_address: None, }; @@ -645,13 +645,13 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); let action = InitBridgeAccountAction { rollup_id, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), sudo_address: None, withdrawer_address: None, }; @@ -663,23 +663,23 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); } #[tokio::test] async fn app_execute_transaction_bridge_lock_action_ok() { - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let mut app = initialize_app(None, vec![]).await; - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset().clone(); let mut state_tx = StateDelta::new(app.state.clone()); state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(&bridge_address, &asset) + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); app.apply(state_tx); @@ -687,8 +687,8 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let action = BridgeLockAction { to: bridge_address, amount, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { @@ -699,16 +699,16 @@ async fn app_execute_transaction_bridge_lock_action_ok() { actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let alice_before_balance = app .state - .get_account_balance(alice_address, &asset) + .get_account_balance(alice_address, nria()) .await .unwrap(); let bridge_before_balance = app .state - .get_account_balance(bridge_address, &asset) + .get_account_balance(bridge_address, nria()) .await .unwrap(); @@ -719,7 +719,7 @@ async fn app_execute_transaction_bridge_lock_action_ok() { bridge_address, rollup_id, amount, - asset.clone(), + nria().into(), "nootwashere".to_string(), ); @@ -732,14 +732,14 @@ async fn app_execute_transaction_bridge_lock_action_ok() { * crate::bridge::get_deposit_byte_len(&expected_deposit); assert_eq!( app.state - .get_account_balance(alice_address, &asset) + .get_account_balance(alice_address, nria()) .await .unwrap(), alice_before_balance - (amount + fee) ); assert_eq!( app.state - .get_account_balance(bridge_address, &asset) + .get_account_balance(bridge_address, nria()) .await .unwrap(), bridge_before_balance + amount @@ -754,19 +754,18 @@ async fn app_execute_transaction_bridge_lock_action_ok() { async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { use astria_core::protocol::transaction::v1alpha1::action::BridgeLockAction; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let mut app = initialize_app(None, vec![]).await; // don't actually register this address as a bridge address - let bridge_address = crate::address::base_prefixed([99; 20]); - let asset = get_native_asset().clone(); + let bridge_address = astria_address(&[99; 20]); let amount = 100; let action = BridgeLockAction { to: bridge_address, amount, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { @@ -777,7 +776,7 @@ async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); assert!(app.execute_transaction(signed_tx).await.is_err()); } @@ -785,7 +784,8 @@ async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { async fn app_execute_transaction_invalid_nonce() { let mut app = initialize_app(None, vec![]).await; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); // create tx with invalid nonce 1 let data = b"hello world".to_vec(); @@ -798,20 +798,20 @@ async fn app_execute_transaction_invalid_nonce() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let response = app.execute_transaction(signed_tx).await; // check that tx was not executed by checking nonce and balance are unchanged assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 0); assert_eq!( app.state - .get_account_balance(alice_address, get_native_asset()) + .get_account_balance(alice_address, nria()) .await .unwrap(), 10u128.pow(19), @@ -831,7 +831,8 @@ async fn app_execute_transaction_invalid_nonce() { async fn app_execute_transaction_invalid_chain_id() { let mut app = initialize_app(None, vec![]).await; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); // create tx with invalid nonce 1 let data = b"hello world".to_vec(); @@ -844,20 +845,20 @@ async fn app_execute_transaction_invalid_chain_id() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let response = app.execute_transaction(signed_tx).await; // check that tx was not executed by checking nonce and balance are unchanged assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 0); assert_eq!( app.state - .get_account_balance(alice_address, get_native_asset()) + .get_account_balance(alice_address, nria()) .await .unwrap(), 10u128.pow(19), @@ -881,11 +882,11 @@ async fn app_stateful_check_fails_insufficient_total_balance() { let mut app = initialize_app(None, vec![]).await; - let (alice_signing_key, _) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); // create a new key; will have 0 balance let keypair = SigningKey::new(OsRng); - let keypair_address = crate::address::base_prefixed(keypair.verification_key().address_bytes()); + let keypair_address = astria_address(&keypair.verification_key().address_bytes()); // figure out needed fee for a single transfer let data = b"hello world".to_vec(); @@ -903,13 +904,13 @@ async fn app_stateful_check_fails_insufficient_total_balance() { TransferAction { to: keypair_address, amount: fee, - asset: get_native_asset().clone(), - fee_asset: get_native_asset().clone(), + asset: nria().into(), + fee_asset: nria().into(), } .into(), ], } - .into_signed(&alice_signing_key); + .into_signed(&alice); // make transfer app.execute_transaction(Arc::new(signed_tx)).await.unwrap(); @@ -924,13 +925,13 @@ async fn app_stateful_check_fails_insufficient_total_balance() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: data.clone(), - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: data.clone(), - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], @@ -955,7 +956,7 @@ async fn app_stateful_check_fails_insufficient_total_balance() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset: get_native_asset().clone(), + fee_asset: nria().into(), } .into(), ], @@ -971,25 +972,27 @@ async fn app_stateful_check_fails_insufficient_total_balance() { async fn app_execute_transaction_bridge_lock_unlock_action_ok() { use crate::accounts::StateWriteExt as _; - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + let mut app = initialize_app(None, vec![]).await; let mut state_tx = StateDelta::new(app.state.clone()); - let (bridge_signing_key, bridge_address) = get_bridge_signing_key_and_address(); + let bridge = get_bridge_signing_key(); + let bridge_address = astria_address(&bridge.address_bytes()); let rollup_id: RollupId = RollupId::from_unhashed_bytes(b"testchainid"); - let asset = get_native_asset(); // give bridge eoa funds so it can pay for the // unlock transfer action let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); state_tx - .put_account_balance(bridge_address, asset, transfer_fee) + .put_account_balance(bridge_address, nria(), transfer_fee) .unwrap(); // create bridge account state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_ibc_asset(&bridge_address, asset) + .put_bridge_account_ibc_asset(&bridge_address, nria()) .unwrap(); state_tx.put_bridge_account_withdrawer_address(&bridge_address, &bridge_address); app.apply(state_tx); @@ -998,8 +1001,8 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { let action = BridgeLockAction { to: bridge_address, amount, - asset: asset.clone(), - fee_asset: asset.clone(), + asset: nria().into(), + fee_asset: nria().into(), destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { @@ -1010,7 +1013,7 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); app.execute_transaction(signed_tx).await.unwrap(); assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 1); @@ -1019,7 +1022,7 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { let action = BridgeUnlockAction { to: alice_address, amount, - fee_asset: asset.clone(), + fee_asset: nria().into(), memo: "{ \"msg\": \"lilywashere\" }".into(), bridge_address: None, }; @@ -1032,13 +1035,13 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { actions: vec![action.into()], }; - let signed_tx = Arc::new(tx.into_signed(&bridge_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&bridge)); app.execute_transaction(signed_tx) .await .expect("executing bridge unlock action should succeed"); assert_eq!( app.state - .get_account_balance(bridge_address, asset) + .get_account_balance(bridge_address, nria()) .await .expect("executing bridge unlock action should succeed"), 0, @@ -1051,8 +1054,8 @@ async fn transaction_execution_records_fee_event() { let mut app = initialize_app(None, vec![]).await; // transfer funds from Alice to Bob - let (alice_signing_key, _) = get_alice_signing_key_and_address(); - let bob_address = address_from_hex_string(BOB_ADDRESS); + let alice = get_alice_signing_key(); + let bob_address = astria_address_from_hex_string(BOB_ADDRESS); let value = 333_333; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -1063,14 +1066,14 @@ async fn transaction_execution_records_fee_event() { TransferAction { to: bob_address, amount: value, - asset: get_native_asset().clone(), - fee_asset: get_native_asset().clone(), + asset: nria().into(), + fee_asset: nria().into(), } .into(), ], }; - let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); + let signed_tx = Arc::new(tx.into_signed(&alice)); let events = app.execute_transaction(signed_tx).await.unwrap(); let transfer_fee = app.state.get_transfer_base_fee().await.unwrap(); @@ -1078,7 +1081,7 @@ async fn transaction_execution_records_fee_event() { assert_eq!(event.kind, "tx.fees"); assert_eq!( event.attributes[0], - ("asset", get_native_asset().to_string()).index().into() + ("asset", nria().to_string()).index().into() ); assert_eq!( event.attributes[1], diff --git a/crates/astria-sequencer/src/assets/mod.rs b/crates/astria-sequencer/src/assets/mod.rs index 12468148fc..cd334f0dee 100644 --- a/crates/astria-sequencer/src/assets/mod.rs +++ b/crates/astria-sequencer/src/assets/mod.rs @@ -1,58 +1,7 @@ pub(crate) mod query; mod state_ext; -use std::sync::OnceLock; - -use astria_core::primitive::v1::asset::Denom; -#[cfg(test)] -pub(crate) use intests::*; -#[cfg(not(test))] -pub(crate) use regular::*; pub(crate) use state_ext::{ StateReadExt, StateWriteExt, }; - -pub(crate) static NATIVE_ASSET: OnceLock = OnceLock::new(); - -#[cfg(not(test))] -mod regular { - - pub(crate) fn initialize_native_asset(native_asset: &str) { - if super::NATIVE_ASSET.get().is_some() { - tracing::error!("native asset should only be set once"); - return; - } - - let denom = native_asset - .parse() - .expect("being unable to parse the native asset breaks sequencer"); - super::NATIVE_ASSET - .set(denom) - .expect("native asset should only be set once"); - } - - pub(crate) fn get_native_asset() -> &'static super::Denom { - super::NATIVE_ASSET - .get() - .expect("native asset should be set") - } -} - -#[cfg(test)] -mod intests { - pub(crate) fn initialize_native_asset(native_asset: &str) { - assert_eq!( - "nria", native_asset, - "all tests should be initialized with \"nria\" as the native asset" - ); - } - - pub(crate) fn get_native_asset() -> &'static super::Denom { - super::NATIVE_ASSET.get_or_init(|| { - "nria" - .parse() - .expect("being unable to parse the native asset breaks sequencer") - }) - } -} diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs index 60611fff73..baf6220db9 100644 --- a/crates/astria-sequencer/src/authority/action.rs +++ b/crates/astria-sequencer/src/authority/action.rs @@ -16,16 +16,14 @@ use astria_core::{ use tracing::instrument; use crate::{ - authority::{ - StateReadExt, - StateWriteExt, - }, + address, + authority, transaction::action_handler::ActionHandler, }; #[async_trait::async_trait] impl ActionHandler for ValidatorUpdate { - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, @@ -58,7 +56,11 @@ impl ActionHandler for ValidatorUpdate { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, _: Address) -> Result<()> { + async fn execute( + &self, + state: &mut S, + _: Address, + ) -> Result<()> { // add validator update in non-consensus state to be used in end_block let mut validator_updates = state .get_validator_updates() @@ -75,18 +77,20 @@ impl ActionHandler for ValidatorUpdate { #[async_trait::async_trait] impl ActionHandler for SudoAddressChangeAction { async fn check_stateless(&self) -> Result<()> { - crate::address::ensure_base_prefix(&self.new_address) - .context("desired new sudo address has an unsupported prefix")?; 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_stateful( + async fn check_stateful( &self, state: &S, from: Address, ) -> Result<()> { + state + .ensure_base_prefix(&self.new_address) + .await + .context("desired new sudo address has an unsupported prefix")?; // ensure signer is the valid `sudo` key in state let sudo_address = state .get_sudo_address() @@ -97,7 +101,7 @@ impl ActionHandler for SudoAddressChangeAction { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, _: Address) -> Result<()> { + async fn execute(&self, state: &mut S, _: Address) -> Result<()> { state .put_sudo_address(self.new_address) .context("failed to put sudo address in state")?; @@ -109,7 +113,7 @@ impl ActionHandler for SudoAddressChangeAction { impl ActionHandler for FeeChangeAction { /// check that the signer of the transaction is the current sudo address, /// as only that address can change the fee - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, @@ -124,7 +128,7 @@ impl ActionHandler for FeeChangeAction { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, _: Address) -> Result<()> { + async fn execute(&self, state: &mut S, _: Address) -> Result<()> { use crate::{ accounts::StateWriteExt as _, bridge::StateWriteExt as _, @@ -184,6 +188,7 @@ mod test { StateReadExt as _, StateWriteExt as _, }, + test_utils::astria_address, }; #[tokio::test] @@ -200,7 +205,7 @@ mod test { }; fee_change - .execute(&mut state, crate::address::base_prefixed([1; 20])) + .execute(&mut state, astria_address(&[1; 20])) .await .unwrap(); assert_eq!(state.get_transfer_base_fee().await.unwrap(), 10); @@ -214,7 +219,7 @@ mod test { }; fee_change - .execute(&mut state, crate::address::base_prefixed([1; 20])) + .execute(&mut state, astria_address(&[1; 20])) .await .unwrap(); assert_eq!(state.get_sequence_action_base_fee().await.unwrap(), 3); @@ -228,7 +233,7 @@ mod test { }; fee_change - .execute(&mut state, crate::address::base_prefixed([1; 20])) + .execute(&mut state, astria_address(&[1; 20])) .await .unwrap(); assert_eq!( @@ -248,7 +253,7 @@ mod test { }; fee_change - .execute(&mut state, crate::address::base_prefixed([1; 20])) + .execute(&mut state, astria_address(&[1; 20])) .await .unwrap(); assert_eq!(state.get_init_bridge_account_base_fee().await.unwrap(), 2); @@ -262,7 +267,7 @@ mod test { }; fee_change - .execute(&mut state, crate::address::base_prefixed([1; 20])) + .execute(&mut state, astria_address(&[1; 20])) .await .unwrap(); assert_eq!( @@ -281,7 +286,7 @@ mod test { }; fee_change - .execute(&mut state, crate::address::base_prefixed([1; 20])) + .execute(&mut state, astria_address(&[1; 20])) .await .unwrap(); assert_eq!(state.get_ics20_withdrawal_base_fee().await.unwrap(), 2); diff --git a/crates/astria-sequencer/src/authority/state_ext.rs b/crates/astria-sequencer/src/authority/state_ext.rs index 20601c4435..afdb4856a8 100644 --- a/crates/astria-sequencer/src/authority/state_ext.rs +++ b/crates/astria-sequencer/src/authority/state_ext.rs @@ -21,6 +21,7 @@ use cnidarium::{ use tracing::instrument; use super::ValidatorSet; +use crate::address; /// Newtype wrapper to read and write an address from rocksdb. #[derive(BorshSerialize, BorshDeserialize, Debug)] @@ -31,7 +32,7 @@ const VALIDATOR_SET_STORAGE_KEY: &str = "valset"; const VALIDATOR_UPDATES_KEY: &[u8] = b"valupdates"; #[async_trait] -pub(crate) trait StateReadExt: StateRead { +pub(crate) trait StateReadExt: StateRead + address::StateReadExt { #[instrument(skip_all)] async fn get_sudo_address(&self) -> Result
{ let Some(bytes) = self @@ -44,7 +45,9 @@ pub(crate) trait StateReadExt: StateRead { }; let SudoAddress(address_bytes) = SudoAddress::try_from_slice(&bytes).context("invalid sudo key bytes")?; - Ok(crate::address::base_prefixed(address_bytes)) + self.try_base_prefixed(&address_bytes) + .await + .context("failed constructing address from prefixed stored in state") } #[instrument(skip_all)] @@ -122,7 +125,7 @@ pub(crate) trait StateWriteExt: StateWrite { impl StateWriteExt for T {} #[cfg(test)] -mod test { +mod tests { use astria_core::protocol::transaction::v1alpha1::action::ValidatorUpdate; use cnidarium::StateDelta; @@ -131,8 +134,13 @@ mod test { StateWriteExt as _, }; use crate::{ + address::StateWriteExt as _, authority::ValidatorSet, - test_utils::verification_key, + test_utils::{ + astria_address, + verification_key, + ASTRIA_PREFIX, + }, }; fn empty_validator_set() -> ValidatorSet { @@ -145,6 +153,8 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // doesn't exist at first state .get_sudo_address() @@ -152,7 +162,7 @@ mod test { .expect_err("no sudo address should exist at first"); // can write new - let mut address_expected = crate::address::base_prefixed([42u8; 20]); + let mut address_expected = astria_address(&[42u8; 20]); state .put_sudo_address(address_expected) .expect("writing sudo address should not fail"); @@ -166,7 +176,7 @@ mod test { ); // can rewrite with new value - address_expected = crate::address::base_prefixed([41u8; 20]); + address_expected = astria_address(&[41u8; 20]); state .put_sudo_address(address_expected) .expect("writing sudo address should not fail"); diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index 4daa373f31..2326593150 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -19,6 +19,7 @@ use crate::{ StateReadExt as _, StateWriteExt as _, }, + address, bridge::{ StateReadExt as _, StateWriteExt as _, @@ -33,16 +34,18 @@ use crate::{ #[async_trait::async_trait] impl ActionHandler for BridgeLockAction { async fn check_stateless(&self) -> Result<()> { - crate::address::ensure_base_prefix(&self.to) - .context("destination address has an unsupported prefix")?; Ok(()) } - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, ) -> Result<()> { + state + .ensure_base_prefix(&self.to) + .await + .context("failed check for base prefix of destination address")?; let transfer_action = TransferAction { to: self.to, asset: self.asset.clone(), @@ -154,6 +157,7 @@ pub(crate) fn get_deposit_byte_len(deposit: &Deposit) -> u128 { #[cfg(test)] mod tests { + use address::StateWriteExt; use astria_core::primitive::v1::{ asset, RollupId, @@ -161,7 +165,13 @@ mod tests { use cnidarium::StateDelta; use super::*; - use crate::assets::StateWriteExt as _; + use crate::{ + assets::StateWriteExt as _, + test_utils::{ + astria_address, + ASTRIA_PREFIX, + }, + }; fn test_asset() -> asset::Denom { "test".parse().unwrap() @@ -173,10 +183,12 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); let transfer_fee = 12; + + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); state.put_transfer_base_fee(transfer_fee).unwrap(); state.put_bridge_lock_byte_cost_multiplier(2); - let bridge_address = crate::address::base_prefixed([1; 20]); + let bridge_address = astria_address(&[1; 20]); let asset = test_asset(); let bridge_lock = BridgeLockAction { to: bridge_address, @@ -193,12 +205,13 @@ mod tests { .unwrap(); state.put_allowed_fee_asset(&asset); - let from_address = crate::address::base_prefixed([2; 20]); + let from_address = astria_address(&[2; 20]); // not enough balance; should fail state .put_account_balance(from_address, &asset, 100) .unwrap(); + assert!( bridge_lock .check_stateful(&state, from_address) @@ -235,7 +248,7 @@ mod tests { state.put_transfer_base_fee(transfer_fee).unwrap(); state.put_bridge_lock_byte_cost_multiplier(2); - let bridge_address = crate::address::base_prefixed([1; 20]); + let bridge_address = astria_address(&[1; 20]); let asset = test_asset(); let bridge_lock = BridgeLockAction { to: bridge_address, @@ -252,7 +265,7 @@ mod tests { .unwrap(); state.put_allowed_fee_asset(&asset); - let from_address = crate::address::base_prefixed([2; 20]); + let from_address = astria_address(&[2; 20]); // not enough balance; should fail state diff --git a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs index 13e21012ca..518a9417e9 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs @@ -11,6 +11,7 @@ use tracing::instrument; use crate::{ accounts::StateWriteExt as _, + address, assets::StateReadExt as _, bridge::state_ext::{ StateReadExt as _, @@ -26,26 +27,31 @@ use crate::{ #[async_trait::async_trait] impl ActionHandler for BridgeSudoChangeAction { async fn check_stateless(&self) -> Result<()> { - crate::address::ensure_base_prefix(&self.bridge_address) - .context("bridge address has an unsupported prefix")?; - self.new_sudo_address - .as_ref() - .map(crate::address::ensure_base_prefix) - .transpose() - .context("new sudo address has an unsupported prefix")?; - self.new_withdrawer_address - .as_ref() - .map(crate::address::ensure_base_prefix) - .transpose() - .context("new withdrawer address has an unsupported prefix")?; Ok(()) } - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, ) -> Result<()> { + state + .ensure_base_prefix(&self.bridge_address) + .await + .context("failed check for base prefix of bridge address")?; + if let Some(new_sudo_address) = &self.new_sudo_address { + state + .ensure_base_prefix(new_sudo_address) + .await + .context("failed check for base prefix of new sudo address")?; + } + if let Some(new_withdrawer_address) = &self.new_withdrawer_address { + state + .ensure_base_prefix(new_withdrawer_address) + .await + .context("failed check for base prefix of new withdrawer address")?; + } + ensure!( state .is_allowed_fee_asset(&self.fee_asset) @@ -98,27 +104,36 @@ impl ActionHandler for BridgeSudoChangeAction { #[cfg(test)] mod tests { + use address::StateWriteExt; use astria_core::primitive::v1::asset; use cnidarium::StateDelta; use super::*; - use crate::assets::StateWriteExt as _; + use crate::{ + assets::StateWriteExt as _, + test_utils::{ + astria_address, + ASTRIA_PREFIX, + }, + }; fn test_asset() -> asset::Denom { "test".parse().unwrap() } #[tokio::test] - async fn bridge_sudo_change_check_stateless_ok() { + async fn check_stateless_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset = test_asset(); state.put_allowed_fee_asset(&asset); - let bridge_address = crate::address::base_prefixed([99; 20]); - let sudo_address = crate::address::base_prefixed([98; 20]); + let bridge_address = astria_address(&[99; 20]); + let sudo_address = astria_address(&[98; 20]); state.put_bridge_account_sudo_address(&bridge_address, &sudo_address); let action = BridgeSudoChangeAction { @@ -132,16 +147,18 @@ mod tests { } #[tokio::test] - async fn bridge_sudo_change_check_stateless_unauthorized() { + async fn check_stateless_unauthorized() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset = test_asset(); state.put_allowed_fee_asset(&asset); - let bridge_address = crate::address::base_prefixed([99; 20]); - let sudo_address = crate::address::base_prefixed([98; 20]); + let bridge_address = astria_address(&[99; 20]); + let sudo_address = astria_address(&[98; 20]); state.put_bridge_account_sudo_address(&bridge_address, &sudo_address); let action = BridgeSudoChangeAction { @@ -162,16 +179,18 @@ mod tests { } #[tokio::test] - async fn bridge_sudo_change_execute_ok() { + async fn execute_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); state.put_bridge_sudo_change_base_fee(10); let fee_asset = test_asset(); - let bridge_address = crate::address::base_prefixed([99; 20]); - let new_sudo_address = crate::address::base_prefixed([98; 20]); - let new_withdrawer_address = crate::address::base_prefixed([97; 20]); + let bridge_address = astria_address(&[99; 20]); + let new_sudo_address = astria_address(&[98; 20]); + let new_withdrawer_address = astria_address(&[97; 20]); state .put_account_balance(bridge_address, &fee_asset, 10) .unwrap(); diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index f6f45937c6..550cf5dce4 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -15,6 +15,7 @@ use tracing::instrument; use crate::{ accounts::action::transfer_check_stateful, + address, bridge::StateReadExt as _, state_ext::{ StateReadExt, @@ -26,21 +27,25 @@ use crate::{ #[async_trait::async_trait] impl ActionHandler for BridgeUnlockAction { async fn check_stateless(&self) -> Result<()> { - crate::address::ensure_base_prefix(&self.to) - .context("destination address has an unsupported prefix")?; - self.bridge_address - .as_ref() - .map(crate::address::ensure_base_prefix) - .transpose() - .context("bridge address has an unsupported prefix")?; Ok(()) } - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, ) -> Result<()> { + state + .ensure_base_prefix(&self.to) + .await + .context("failed check for base prefix of destination address")?; + if let Some(bridge_address) = &self.bridge_address { + state + .ensure_base_prefix(bridge_address) + .await + .context("failed check for base prefix of bridge address")?; + } + // the bridge address to withdraw funds from // if unset, use the tx sender's address let bridge_address = self.bridge_address.unwrap_or(from); @@ -113,8 +118,13 @@ mod test { use super::*; use crate::{ accounts::StateWriteExt as _, + address::StateWriteExt as _, assets::StateWriteExt as _, - bridge::StateWriteExt, + bridge::StateWriteExt as _, + test_utils::{ + astria_address, + ASTRIA_PREFIX, + }, }; fn test_asset() -> asset::Denom { @@ -122,16 +132,18 @@ mod test { } #[tokio::test] - async fn bridge_unlock_fail_non_bridge_accounts() { + async fn fail_non_bridge_accounts() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); - let state = StateDelta::new(snapshot); + let mut state = StateDelta::new(snapshot); + + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); let asset = test_asset(); let transfer_amount = 100; - let address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); let bridge_unlock = BridgeUnlockAction { to: to_address, @@ -153,18 +165,20 @@ mod test { } #[tokio::test] - async fn bridge_unlock_fail_withdrawer_unset_invalid_withdrawer() { + async fn fail_withdrawer_unset_invalid_withdrawer() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset = test_asset(); let transfer_amount = 100; - let sender_address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let sender_address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); - let bridge_address = crate::address::base_prefixed([3; 20]); + let bridge_address = astria_address(&[3; 20]); state .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); @@ -190,19 +204,21 @@ mod test { } #[tokio::test] - async fn bridge_unlock_fail_withdrawer_set_invalid_withdrawer() { + async fn fail_withdrawer_set_invalid_withdrawer() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset = test_asset(); let transfer_amount = 100; - let sender_address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let sender_address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); - let bridge_address = crate::address::base_prefixed([3; 20]); - let withdrawer_address = crate::address::base_prefixed([4; 20]); + let bridge_address = astria_address(&[3; 20]); + let withdrawer_address = astria_address(&[4; 20]); state.put_bridge_account_withdrawer_address(&bridge_address, &withdrawer_address); state .put_bridge_account_ibc_asset(&bridge_address, &asset) @@ -228,18 +244,20 @@ mod test { } #[tokio::test] - async fn bridge_unlock_fee_check_stateful_from_none() { + async fn fee_check_stateful_from_none() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset = test_asset(); let transfer_fee = 10; let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); - let bridge_address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let bridge_address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); @@ -281,18 +299,20 @@ mod test { } #[tokio::test] - async fn bridge_unlock_fee_check_stateful_from_some() { + async fn fee_check_stateful_from_some() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset = test_asset(); let transfer_fee = 10; let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); - let bridge_address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let bridge_address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); @@ -301,7 +321,7 @@ mod test { .unwrap(); state.put_allowed_fee_asset(&asset); - let withdrawer_address = crate::address::base_prefixed([3; 20]); + let withdrawer_address = astria_address(&[3; 20]); state.put_bridge_account_withdrawer_address(&bridge_address, &withdrawer_address); let bridge_unlock = BridgeUnlockAction { @@ -336,7 +356,7 @@ mod test { } #[tokio::test] - async fn bridge_unlock_execute_from_none() { + async fn execute_from_none() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -346,8 +366,8 @@ mod test { let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); - let bridge_address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let bridge_address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); @@ -388,7 +408,7 @@ mod test { } #[tokio::test] - async fn bridge_unlock_execute_from_some() { + async fn execute_from_some() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -398,8 +418,8 @@ mod test { let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); - let bridge_address = crate::address::base_prefixed([1; 20]); - let to_address = crate::address::base_prefixed([2; 20]); + let bridge_address = astria_address(&[1; 20]); + let to_address = astria_address(&[2; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); diff --git a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs index a116771e14..986a314bfe 100644 --- a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs +++ b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs @@ -15,6 +15,7 @@ use crate::{ StateReadExt as _, StateWriteExt as _, }, + address, assets::StateReadExt as _, bridge::state_ext::{ StateReadExt as _, @@ -30,26 +31,27 @@ use crate::{ #[async_trait::async_trait] impl ActionHandler for InitBridgeAccountAction { async fn check_stateless(&self) -> Result<()> { - self.withdrawer_address - .as_ref() - .map(crate::address::ensure_base_prefix) - .transpose() - .context("the withdrawer address has an unsupported prefix")?; - - self.sudo_address - .as_ref() - .map(crate::address::ensure_base_prefix) - .transpose() - .context("the sudo address has an unsupported")?; - Ok(()) } - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, ) -> Result<()> { + if let Some(withdrawer_address) = &self.withdrawer_address { + state + .ensure_base_prefix(withdrawer_address) + .await + .context("failed check for base prefix of withdrawer address")?; + } + if let Some(sudo_address) = &self.sudo_address { + state + .ensure_base_prefix(sudo_address) + .await + .context("failed check for base prefix of sudo address")?; + } + ensure!( state.is_allowed_fee_asset(&self.fee_asset).await?, "invalid fee asset", diff --git a/crates/astria-sequencer/src/bridge/query.rs b/crates/astria-sequencer/src/bridge/query.rs index 2a64baf39d..3ea29d867d 100644 --- a/crates/astria-sequencer/src/bridge/query.rs +++ b/crates/astria-sequencer/src/bridge/query.rs @@ -269,9 +269,14 @@ mod test { use super::*; use crate::{ - assets::StateWriteExt, + address::StateWriteExt as _, + assets::StateWriteExt as _, bridge::StateWriteExt as _, state_ext::StateWriteExt as _, + test_utils::{ + astria_address, + ASTRIA_PREFIX, + }, }; #[tokio::test] @@ -280,11 +285,13 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + let asset: astria_core::primitive::v1::asset::Denom = "test".parse().unwrap(); let rollup_id = RollupId::from_unhashed_bytes("test"); - let bridge_address = crate::address::base_prefixed([0u8; 20]); - let sudo_address = crate::address::base_prefixed([1u8; 20]); - let withdrawer_address = crate::address::base_prefixed([2u8; 20]); + let bridge_address = astria_address(&[0u8; 20]); + let sudo_address = astria_address(&[1u8; 20]); + let withdrawer_address = astria_address(&[2u8; 20]); state.put_block_height(1); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state diff --git a/crates/astria-sequencer/src/bridge/state_ext.rs b/crates/astria-sequencer/src/bridge/state_ext.rs index a4ad85d2c3..3b8ba0b5f6 100644 --- a/crates/astria-sequencer/src/bridge/state_ext.rs +++ b/crates/astria-sequencer/src/bridge/state_ext.rs @@ -34,6 +34,8 @@ use tracing::{ instrument, }; +use crate::address; + /// Newtype wrapper to read and write a u128 from rocksdb. #[derive(BorshSerialize, BorshDeserialize, Debug)] struct Balance(u128); @@ -139,7 +141,7 @@ fn last_transaction_hash_for_bridge_account_storage_key(address: &Address) -> Ve } #[async_trait] -pub(crate) trait StateReadExt: StateRead { +pub(crate) trait StateReadExt: StateRead + address::StateReadExt { #[instrument(skip_all)] async fn get_bridge_account_rollup_id(&self, address: &Address) -> Result> { let Some(rollup_id_bytes) = self @@ -182,8 +184,10 @@ pub(crate) trait StateReadExt: StateRead { return Ok(None); }; - let sudo_address = crate::address::try_base_prefixed(&sudo_address_bytes) - .context("invalid sudo address bytes")?; + let sudo_address = self.try_base_prefixed(&sudo_address_bytes).await.context( + "failed check for constructing sudo address from address bytes and prefix stored \ + retrieved from state", + )?; Ok(Some(sudo_address)) } @@ -203,8 +207,13 @@ pub(crate) trait StateReadExt: StateRead { return Ok(None); }; - let withdrawer_address = crate::address::try_base_prefixed(&withdrawer_address_bytes) - .context("invalid withdrawer address bytes")?; + let withdrawer_address = self + .try_base_prefixed(&withdrawer_address_bytes) + .await + .context( + "failed check for constructing withdrawer address from address bytes and prefix \ + stored retrieved from state", + )?; Ok(Some(withdrawer_address)) } @@ -491,6 +500,7 @@ mod test { StateReadExt as _, StateWriteExt as _, }; + use crate::test_utils::astria_address; fn asset_0() -> asset::Denom { "asset_0".parse().unwrap() @@ -506,7 +516,7 @@ mod test { let snapshot = storage.latest_snapshot(); let state = StateDelta::new(snapshot); - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); // uninitialized ok assert_eq!( @@ -525,7 +535,7 @@ mod test { let mut state = StateDelta::new(snapshot); let mut rollup_id = RollupId::new([1u8; 32]); - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); // can write new state.put_bridge_account_rollup_id(&address, &rollup_id); @@ -554,7 +564,7 @@ mod test { // can write additional account and both valid let rollup_id_1 = RollupId::new([2u8; 32]); - let address_1 = crate::address::base_prefixed([41u8; 20]); + let address_1 = astria_address(&[41u8; 20]); state.put_bridge_account_rollup_id(&address_1, &rollup_id_1); assert_eq!( state @@ -583,7 +593,7 @@ mod test { let snapshot = storage.latest_snapshot(); let state = StateDelta::new(snapshot); - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); state .get_bridge_account_ibc_asset(&address) .await @@ -596,7 +606,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); let mut asset = asset_0(); // can write @@ -629,7 +639,7 @@ mod test { ); // writing to other account also ok - let address_1 = crate::address::base_prefixed([41u8; 20]); + let address_1 = astria_address(&[41u8; 20]); let asset_1 = asset_1(); state .put_bridge_account_ibc_asset(&address_1, &asset_1) @@ -753,7 +763,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = crate::address::base_prefixed([42u8; 20]); + let bridge_address = astria_address(&[42u8; 20]); let mut amount = 10u128; let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; @@ -865,7 +875,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id_0 = RollupId::new([1u8; 32]); - let bridge_address = crate::address::base_prefixed([42u8; 20]); + let bridge_address = astria_address(&[42u8; 20]); let amount = 10u128; let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; @@ -936,7 +946,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = crate::address::base_prefixed([42u8; 20]); + let bridge_address = astria_address(&[42u8; 20]); let amount = 10u128; let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; @@ -991,7 +1001,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = crate::address::base_prefixed([42u8; 20]); + let bridge_address = astria_address(&[42u8; 20]); let amount = 10u128; let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; @@ -1083,7 +1093,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = crate::address::base_prefixed([42u8; 20]); + let bridge_address = astria_address(&[42u8; 20]); let amount = 10u128; let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; diff --git a/crates/astria-sequencer/src/grpc/sequencer.rs b/crates/astria-sequencer/src/grpc/sequencer.rs index 89609b809d..c17f899587 100644 --- a/crates/astria-sequencer/src/grpc/sequencer.rs +++ b/crates/astria-sequencer/src/grpc/sequencer.rs @@ -186,7 +186,7 @@ impl SequencerService for SequencerServer { ); Status::invalid_argument(format!("invalid address: {e}")) })?; - let nonce = self.mempool.pending_nonce(&address).await; + let nonce = self.mempool.pending_nonce(address.bytes()).await; if let Some(nonce) = nonce { return Ok(Response::new(GetPendingNonceResponse { @@ -221,7 +221,9 @@ mod test { use super::*; use crate::{ api_state_ext::StateWriteExt as _, + app::test_utils::get_alice_signing_key, state_ext::StateWriteExt, + test_utils::astria_address, }; fn make_test_sequencer_block(height: u32) -> SequencerBlock { @@ -256,7 +258,8 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let mempool = Mempool::new(); - let (_, address) = crate::app::test_utils::get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); let nonce = 99; let tx = crate::app::test_utils::get_mock_tx(nonce); mempool.insert(tx, 0).await.unwrap(); @@ -268,7 +271,7 @@ mod test { let server = Arc::new(SequencerServer::new(storage.clone(), mempool)); let request = GetPendingNonceRequest { - address: Some(address.into_raw()), + address: Some(alice_address.into_raw()), }; let request = Request::new(request); let response = server.get_pending_nonce(request).await.unwrap(); @@ -282,13 +285,14 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let mempool = Mempool::new(); let mut state_tx = StateDelta::new(storage.latest_snapshot()); - let (_, address) = crate::app::test_utils::get_alice_signing_key_and_address(); - state_tx.put_account_nonce(address, 99).unwrap(); + let alice = get_alice_signing_key(); + let alice_address = astria_address(&alice.address_bytes()); + state_tx.put_account_nonce(alice_address, 99).unwrap(); storage.commit(state_tx).await.unwrap(); let server = Arc::new(SequencerServer::new(storage.clone(), mempool)); let request = GetPendingNonceRequest { - address: Some(address.into_raw()), + address: Some(alice_address.into_raw()), }; let request = Request::new(request); let response = server.get_pending_nonce(request).await.unwrap(); diff --git a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs b/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs index f60f8726bd..cb37a1ac30 100644 --- a/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs +++ b/crates/astria-sequencer/src/ibc/ibc_relayer_change.rs @@ -8,33 +8,32 @@ use astria_core::{ protocol::transaction::v1alpha1::action::IbcRelayerChangeAction, }; use async_trait::async_trait; -use cnidarium::{ - StateRead, - StateWrite, -}; use crate::{ - ibc::state_ext::{ - StateReadExt, - StateWriteExt, - }, + address, + ibc, transaction::action_handler::ActionHandler, }; #[async_trait] impl ActionHandler for IbcRelayerChangeAction { async fn check_stateless(&self) -> Result<()> { + Ok(()) + } + + async fn check_stateful( + &self, + state: &S, + from: Address, + ) -> Result<()> { match self { IbcRelayerChangeAction::Addition(addr) | IbcRelayerChangeAction::Removal(addr) => { - crate::address::ensure_base_prefix(addr) - .context("provided address to be added or removed has an unsupported prefix")?; + state.ensure_base_prefix(addr).await.context( + "failed check for base prefix of provided address to be added/removed", + )?; } } - Ok(()) - } - - async fn check_stateful(&self, state: &S, from: Address) -> Result<()> { let ibc_sudo_address = state .get_ibc_sudo_address() .await @@ -46,7 +45,7 @@ impl ActionHandler for IbcRelayerChangeAction { Ok(()) } - async fn execute(&self, state: &mut S, _from: Address) -> Result<()> { + async fn execute(&self, state: &mut S, _from: Address) -> Result<()> { match self { IbcRelayerChangeAction::Addition(address) => { state.put_ibc_relayer_address(address); diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index 917c0f9f51..1b2dac46f8 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -653,6 +653,10 @@ mod test { use crate::{ accounts::StateReadExt as _, ibc::StateWriteExt as _, + test_utils::{ + astria_address, + astria_address_from_hex_string, + }, }; #[tokio::test] @@ -721,10 +725,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let recipient = crate::address::try_base_prefixed( - &hex::decode("1c0c490f1b5528d8173c5de46d131160e4b2c0c3").unwrap(), - ) - .unwrap(); + let recipient = astria_address_from_hex_string("1c0c490f1b5528d8173c5de46d131160e4b2c0c3"); let packet = FungibleTokenPacketData { denom: "nootasset".to_string(), sender: String::new(), @@ -760,7 +761,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); @@ -817,7 +818,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); @@ -855,7 +856,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = crate::address::base_prefixed([99; 20]); + let bridge_address = astria_address(&[99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); @@ -893,7 +894,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let recipient_address = crate::address::base_prefixed([1; 20]); + let recipient_address = astria_address(&[1; 20]); let amount = 100; let base_denom = "nootasset".parse::().unwrap(); state_tx @@ -943,7 +944,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let recipient_address = crate::address::base_prefixed([1; 20]); + let recipient_address = astria_address(&[1; 20]); let amount = 100; let base_denom = "nootasset".parse::().unwrap(); state_tx @@ -993,7 +994,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = crate::address::base_prefixed([99u8; 20]); + let bridge_address = astria_address(&[99u8; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset" .parse::() @@ -1035,7 +1036,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = crate::address::base_prefixed([99u8; 20]); + let bridge_address = astria_address(&[99u8; 20]); let destination_chain_address = bridge_address.to_string(); let denom = "nootasset".parse::().unwrap(); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index d2a218179c..859ca1d920 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -25,10 +25,8 @@ use penumbra_ibc::component::packet::{ use tracing::instrument; use crate::{ - accounts::{ - StateReadExt, - StateWriteExt, - }, + accounts, + address, bridge::StateReadExt as _, ibc::{ StateReadExt as _, @@ -53,7 +51,7 @@ fn withdrawal_to_unchecked_ibc_packet( ) } -async fn ics20_withdrawal_check_stateful_bridge_account( +async fn ics20_withdrawal_check_stateful_bridge_account( action: &action::Ics20Withdrawal, state: &S, from: Address, @@ -102,14 +100,6 @@ impl ActionHandler for action::Ics20Withdrawal { async fn check_stateless(&self) -> Result<()> { ensure!(self.timeout_time() != 0, "timeout time must be non-zero",); - crate::address::ensure_base_prefix(&self.return_address) - .context("return address has an unsupported prefix")?; - self.bridge_address - .as_ref() - .map(crate::address::ensure_base_prefix) - .transpose() - .context("bridge address has an unsupported prefix")?; - // NOTE (from penumbra): we could validate the destination chain address as bech32 to // prevent mistyped addresses, but this would preclude sending to chains that don't // use bech32 addresses. @@ -117,11 +107,22 @@ impl ActionHandler for action::Ics20Withdrawal { } #[instrument(skip_all)] - async fn check_stateful( + async fn check_stateful( &self, state: &S, from: Address, ) -> Result<()> { + state + .ensure_base_prefix(&self.return_address) + .await + .context("failed to verify that return address address has permitted base prefix")?; + + if let Some(bridge_address) = &self.bridge_address { + state.ensure_base_prefix(bridge_address).await.context( + "failed to verify that bridge address address has permitted base prefix", + )?; + } + ics20_withdrawal_check_stateful_bridge_account(self, state, from).await?; let fee = state @@ -176,7 +177,11 @@ impl ActionHandler for action::Ics20Withdrawal { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, from: Address) -> Result<()> { + async fn execute( + &self, + state: &mut S, + from: Address, + ) -> Result<()> { let fee = state .get_ics20_withdrawal_base_fee() .await @@ -231,21 +236,28 @@ fn is_source(source_port: &PortId, source_channel: &ChannelId, asset: &Denom) -> #[cfg(test)] mod tests { + use address::StateWriteExt; use astria_core::primitive::v1::RollupId; use cnidarium::StateDelta; use ibc_types::core::client::Height; use super::*; - use crate::bridge::StateWriteExt as _; + use crate::{ + bridge::StateWriteExt as _, + test_utils::{ + astria_address, + ASTRIA_PREFIX, + }, + }; #[tokio::test] - async fn ics20_withdrawal_check_stateful_bridge_account_not_bridge() { + async fn check_stateful_bridge_account_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 from = crate::address::base_prefixed([1u8; 20]); + let from = astria_address(&[1u8; 20]); let action = action::Ics20Withdrawal { amount: 1, denom: denom.clone(), @@ -265,14 +277,15 @@ mod tests { } #[tokio::test] - async fn ics20_withdrawal_check_stateful_bridge_account_sender_is_bridge_bridge_address_none_ok() - { + async fn check_stateful_bridge_account_sender_is_bridge_bridge_address_none_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // sender is a bridge address, which is also the withdrawer, so it's ok - let bridge_address = crate::address::base_prefixed([1u8; 20]); + let bridge_address = astria_address(&[1u8; 20]); state.put_bridge_account_rollup_id( &bridge_address, &RollupId::from_unhashed_bytes("testrollupid"), @@ -299,22 +312,20 @@ mod tests { } #[tokio::test] - async fn ics20_withdrawal_check_stateful_bridge_account_sender_is_bridge_bridge_address_none_invalid() - { + async fn check_stateful_bridge_account_sender_is_bridge_bridge_address_none_invalid() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // withdraw is *not* the bridge address, Ics20Withdrawal must be sent by the withdrawer - let bridge_address = crate::address::base_prefixed([1u8; 20]); + let bridge_address = astria_address(&[1u8; 20]); state.put_bridge_account_rollup_id( &bridge_address, &RollupId::from_unhashed_bytes("testrollupid"), ); - state.put_bridge_account_withdrawer_address( - &bridge_address, - &crate::address::base_prefixed([2u8; 20]), - ); + state.put_bridge_account_withdrawer_address(&bridge_address, &astria_address(&[2u8; 20])); let denom = "test".parse::().unwrap(); let action = action::Ics20Withdrawal { @@ -340,14 +351,16 @@ mod tests { } #[tokio::test] - async fn ics20_withdrawal_check_stateful_bridge_account_bridge_address_some_ok() { + async fn check_stateful_bridge_account_bridge_address_some_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // sender the withdrawer address, so it's ok - let bridge_address = crate::address::base_prefixed([1u8; 20]); - let withdrawer_address = crate::address::base_prefixed([2u8; 20]); + let bridge_address = astria_address(&[1u8; 20]); + let withdrawer_address = astria_address(&[2u8; 20]); state.put_bridge_account_rollup_id( &bridge_address, &RollupId::from_unhashed_bytes("testrollupid"), @@ -374,14 +387,16 @@ mod tests { } #[tokio::test] - async fn ics20_withdrawal_check_stateful_bridge_account_bridge_address_some_invalid_sender() { + async fn check_stateful_bridge_account_bridge_address_some_invalid_sender() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // sender is not the withdrawer address, so must fail - let bridge_address = crate::address::base_prefixed([1u8; 20]); - let withdrawer_address = crate::address::base_prefixed([2u8; 20]); + let bridge_address = astria_address(&[1u8; 20]); + let withdrawer_address = astria_address(&[2u8; 20]); state.put_bridge_account_rollup_id( &bridge_address, &RollupId::from_unhashed_bytes("testrollupid"), @@ -419,7 +434,7 @@ mod tests { let state = StateDelta::new(snapshot); // sender is not the withdrawer address, so must fail - let not_bridge_address = crate::address::base_prefixed([1u8; 20]); + let not_bridge_address = astria_address(&[1u8; 20]); let denom = "test".parse::().unwrap(); let action = action::Ics20Withdrawal { diff --git a/crates/astria-sequencer/src/ibc/state_ext.rs b/crates/astria-sequencer/src/ibc/state_ext.rs index 3e979972c7..3abde83481 100644 --- a/crates/astria-sequencer/src/ibc/state_ext.rs +++ b/crates/astria-sequencer/src/ibc/state_ext.rs @@ -23,6 +23,8 @@ use tracing::{ instrument, }; +use crate::address; + /// Newtype wrapper to read and write a u128 from rocksdb. #[derive(BorshSerialize, BorshDeserialize, Debug)] struct Balance(u128); @@ -66,7 +68,7 @@ fn ibc_relayer_key(address: &Address) -> String { } #[async_trait] -pub(crate) trait StateReadExt: StateRead { +pub(crate) trait StateReadExt: StateRead + address::StateReadExt { #[instrument(skip_all)] async fn get_ibc_channel_balance( &self, @@ -100,7 +102,10 @@ pub(crate) trait StateReadExt: StateRead { }; let SudoAddress(address_bytes) = SudoAddress::try_from_slice(&bytes).context("invalid ibc sudo key bytes")?; - Ok(crate::address::base_prefixed(address_bytes)) + Ok(self.try_base_prefixed(&address_bytes).await.context( + "failed constructing ibc sudo address from address bytes and address prefix stored in \ + state", + )?) } #[instrument(skip_all)] @@ -191,7 +196,14 @@ mod tests { StateReadExt as _, StateWriteExt as _, }; - use crate::ibc::state_ext::channel_balance_storage_key; + use crate::{ + address::StateWriteExt, + ibc::state_ext::channel_balance_storage_key, + test_utils::{ + astria_address, + ASTRIA_PREFIX, + }, + }; fn asset_0() -> asset::Denom { "asset_0".parse().unwrap() @@ -219,8 +231,10 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // can write new - let mut address = crate::address::base_prefixed([42u8; 20]); + let mut address = astria_address(&[42u8; 20]); state .put_ibc_sudo_address(address) .expect("writing sudo address should not fail"); @@ -234,7 +248,7 @@ mod tests { ); // can rewrite with new value - address = crate::address::base_prefixed([41u8; 20]); + address = astria_address(&[41u8; 20]); state .put_ibc_sudo_address(address) .expect("writing sudo address should not fail"); @@ -252,10 +266,12 @@ mod tests { async fn is_ibc_relayer_ok_if_not_set() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); - let state = StateDelta::new(snapshot); + let mut state = StateDelta::new(snapshot); + + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); // unset address returns false - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); assert!( !state .is_ibc_relayer(&address) @@ -271,8 +287,10 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // can write - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); state.put_ibc_relayer_address(&address); assert!( state @@ -299,8 +317,10 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); + state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + // can write - let address = crate::address::base_prefixed([42u8; 20]); + let address = astria_address(&[42u8; 20]); state.put_ibc_relayer_address(&address); assert!( state @@ -311,7 +331,7 @@ mod tests { ); // can write multiple - let address_1 = crate::address::base_prefixed([41u8; 20]); + let address_1 = astria_address(&[41u8; 20]); state.put_ibc_relayer_address(&address_1); assert!( state diff --git a/crates/astria-sequencer/src/mempool/benchmarks.rs b/crates/astria-sequencer/src/mempool/benchmarks.rs index dae8b13143..7c97d278c1 100644 --- a/crates/astria-sequencer/src/mempool/benchmarks.rs +++ b/crates/astria-sequencer/src/mempool/benchmarks.rs @@ -13,7 +13,6 @@ use astria_core::{ Denom, IbcPrefixed, }, - Address, RollupId, }, protocol::transaction::v1alpha1::{ @@ -60,7 +59,6 @@ fn signing_keys() -> impl Iterator { fn transactions() -> &'static Vec { static TXS: OnceLock> = OnceLock::new(); TXS.get_or_init(|| { - crate::address::initialize_base_prefix("benchmarks").unwrap(); let mut nonces_and_chain_ids = HashMap::new(); signing_keys() .map(move |signing_key| { @@ -321,7 +319,7 @@ fn run_maintenance(bencher: divan::Bencher) { // Although in production this getter will be hitting the state store and will be slower than // this test one, it's probably insignificant as the getter is only called once per address, // and we don't expect a high number of discrete addresses in the mempool entries. - let current_account_nonce_getter = |_: Address| async { Ok(new_nonce) }; + let current_account_nonce_getter = |_: [u8; 20]| async { Ok(new_nonce) }; bencher .with_inputs(|| init_mempool::()) .bench_values(move |mempool| { @@ -351,18 +349,16 @@ fn pending_nonce(bencher: divan::Bencher) { .unwrap(); bencher .with_inputs(|| { - let address = crate::address::base_prefixed( - transactions() - .first() - .unwrap() - .verification_key() - .address_bytes(), - ); + let address = transactions() + .first() + .unwrap() + .verification_key() + .address_bytes(); (init_mempool::(), address) }) .bench_values(move |(mempool, address)| { runtime.block_on(async { - mempool.pending_nonce(&address).await.unwrap(); + mempool.pending_nonce(address).await.unwrap(); }); }); } diff --git a/crates/astria-sequencer/src/mempool/mod.rs b/crates/astria-sequencer/src/mempool/mod.rs index ad76dd25d1..6ccf5f9e22 100644 --- a/crates/astria-sequencer/src/mempool/mod.rs +++ b/crates/astria-sequencer/src/mempool/mod.rs @@ -20,7 +20,7 @@ use std::{ use anyhow::Context; use astria_core::{ crypto::SigningKey, - primitive::v1::Address, + primitive::v1::ADDRESS_LEN, protocol::transaction::v1alpha1::{ SignedTransaction, TransactionParams, @@ -78,16 +78,16 @@ impl PartialOrd for TransactionPriority { pub(crate) struct EnqueuedTransaction { tx_hash: [u8; 32], signed_tx: Arc, - address: Address, + address_bytes: [u8; ADDRESS_LEN], } impl EnqueuedTransaction { fn new(signed_tx: SignedTransaction) -> Self { - let address = crate::address::base_prefixed(signed_tx.verification_key().address_bytes()); + let address_bytes = signed_tx.verification_key().address_bytes(); Self { tx_hash: signed_tx.sha256_of_proto_encoding(), signed_tx: Arc::new(signed_tx), - address, + address_bytes, } } @@ -117,8 +117,8 @@ impl EnqueuedTransaction { self.signed_tx.clone() } - pub(crate) fn address(&self) -> &Address { - &self.address + pub(crate) fn address_bytes(&self) -> [u8; 20] { + self.address_bytes } } @@ -301,11 +301,11 @@ impl Mempool { /// removes a transaction from the mempool #[instrument(skip_all)] pub(crate) async fn remove(&self, tx_hash: [u8; 32]) { - let (signed_tx, address) = dummy_signed_tx(); + let (signed_tx, address_bytes) = dummy_signed_tx(); let enqueued_tx = EnqueuedTransaction { tx_hash, signed_tx, - address, + address_bytes, }; self.queue.write().await.remove(&enqueued_tx); } @@ -337,7 +337,7 @@ impl Mempool { current_account_nonce_getter: F, ) -> anyhow::Result<()> where - F: Fn(Address) -> O, + F: Fn([u8; ADDRESS_LEN]) -> O, O: Future>, { let mut txs_to_remove = Vec::new(); @@ -346,7 +346,7 @@ impl Mempool { let mut queue = self.queue.write().await; let mut removal_cache = self.comet_bft_removal_cache.write().await; for (enqueued_tx, priority) in queue.iter_mut() { - let address = enqueued_tx.address(); + let address_bytes = enqueued_tx.address_bytes(); // check if the transactions has expired if priority.time_first_seen.elapsed() > self.tx_ttl { @@ -357,14 +357,16 @@ impl Mempool { } // Try to get the current account nonce from the ones already retrieved. - let current_account_nonce = if let Some(nonce) = current_account_nonces.get(&address) { + let current_account_nonce = if let Some(nonce) = + current_account_nonces.get(&address_bytes) + { *nonce } else { // Fall back to getting via the getter and adding it to the local temp collection. - let nonce = current_account_nonce_getter(*enqueued_tx.address()) + let nonce = current_account_nonce_getter(enqueued_tx.address_bytes()) .await .context("failed to fetch account nonce")?; - current_account_nonces.insert(address, nonce); + current_account_nonces.insert(address_bytes, nonce); nonce }; match enqueued_tx.priority(current_account_nonce, Some(priority.time_first_seen)) { @@ -390,11 +392,11 @@ impl Mempool { /// returns the pending nonce for the given address, /// if it exists in the mempool. #[instrument(skip_all)] - pub(crate) async fn pending_nonce(&self, address: &Address) -> Option { + pub(crate) async fn pending_nonce(&self, address: [u8; ADDRESS_LEN]) -> Option { let inner = self.queue.read().await; let mut nonce = None; for (tx, _priority) in inner.iter() { - if tx.address() == address { + if tx.address_bytes() == address { nonce = Some(cmp::max(nonce.unwrap_or_default(), tx.signed_tx.nonce())); } } @@ -409,23 +411,26 @@ impl Mempool { /// this `signed_tx` field is ignored in the `PartialEq` and `Hash` impls of `EnqueuedTransaction` - /// only the tx hash is considered. So we create an `EnqueuedTransaction` on the fly with the /// correct tx hash and this dummy signed tx when removing from the queue. -fn dummy_signed_tx() -> (Arc, Address) { - static TX: OnceLock<(Arc, Address)> = OnceLock::new(); - let (signed_tx, address) = TX.get_or_init(|| { +fn dummy_signed_tx() -> (Arc, [u8; ADDRESS_LEN]) { + static TX: OnceLock<(Arc, [u8; ADDRESS_LEN])> = OnceLock::new(); + let (signed_tx, address_bytes) = TX.get_or_init(|| { let actions = vec![]; let params = TransactionParams::builder() .nonce(0) .chain_id("dummy") .build(); let signing_key = SigningKey::from([0; 32]); - let address = crate::address::base_prefixed(signing_key.verification_key().address_bytes()); + let address_bytes = signing_key.verification_key().address_bytes(); let unsigned_tx = UnsignedTransaction { actions, params, }; - (Arc::new(unsigned_tx.into_signed(&signing_key)), address) + ( + Arc::new(unsigned_tx.into_signed(&signing_key)), + address_bytes, + ) }); - (signed_tx.clone(), *address) + (signed_tx.clone(), *address_bytes) } #[cfg(test)] @@ -509,23 +514,17 @@ mod test { let tx0 = EnqueuedTransaction { tx_hash: [0; 32], signed_tx: Arc::new(get_mock_tx(0)), - address: crate::address::base_prefixed( - get_mock_tx(0).verification_key().address_bytes(), - ), + address_bytes: get_mock_tx(0).address_bytes(), }; let other_tx0 = EnqueuedTransaction { tx_hash: [0; 32], signed_tx: Arc::new(get_mock_tx(1)), - address: crate::address::base_prefixed( - get_mock_tx(1).verification_key().address_bytes(), - ), + address_bytes: get_mock_tx(1).address_bytes(), }; let tx1 = EnqueuedTransaction { tx_hash: [1; 32], signed_tx: Arc::new(get_mock_tx(0)), - address: crate::address::base_prefixed( - get_mock_tx(0).verification_key().address_bytes(), - ), + address_bytes: get_mock_tx(0).address_bytes(), }; assert!(tx0 == other_tx0); assert!(tx0 != tx1); @@ -620,7 +619,7 @@ mod test { mempool.insert(get_mock_tx(1), 0).await.unwrap(); // Insert txs from a different signer with nonces 100 and 102. - let other_signing_key = SigningKey::from([1; 32]); + let other = SigningKey::from([1; 32]); let other_mock_tx = |nonce: u32| -> SignedTransaction { let actions = get_mock_tx(0).actions().to_vec(); UnsignedTransaction { @@ -630,28 +629,29 @@ mod test { .build(), actions, } - .into_signed(&other_signing_key) + .into_signed(&other) }; mempool.insert(other_mock_tx(100), 0).await.unwrap(); mempool.insert(other_mock_tx(102), 0).await.unwrap(); assert_eq!(mempool.len().await, 4); - let (alice_signing_key, alice_address) = - crate::app::test_utils::get_alice_signing_key_and_address(); - let other_address = - crate::address::base_prefixed(other_signing_key.verification_key().address_bytes()); + let alice = crate::app::test_utils::get_alice_signing_key(); // Create a getter fn which will returns 1 for alice's current account nonce, and 101 for // the other signer's. - let current_account_nonce_getter = |address: Address| async move { - if address == alice_address { - return Ok(1); - } - if address == other_address { - return Ok(101); + let current_account_nonce_getter = |address: [u8; ADDRESS_LEN]| { + let alice = alice.clone(); + let other = other.clone(); + async move { + if address == alice.address_bytes() { + return Ok(1); + } + if address == other.address_bytes() { + return Ok(101); + } + Err(anyhow::anyhow!("invalid address")) } - Err(anyhow::anyhow!("invalid address")) }; // Update the priorities. Alice's first tx (with nonce 0) and other's first (with nonce @@ -666,19 +666,13 @@ mod test { // Alice's remaining tx should be the highest priority (nonce diff of 1 - 1 == 0). let (tx, priority) = mempool.pop().await.unwrap(); assert_eq!(tx.signed_tx.nonce(), 1); - assert_eq!( - *tx.signed_tx.verification_key(), - alice_signing_key.verification_key() - ); + assert_eq!(*tx.signed_tx.verification_key(), alice.verification_key()); assert_eq!(priority.nonce_diff, 0); // Other's remaining tx should be the highest priority (nonce diff of 102 - 101 == 1). let (tx, priority) = mempool.pop().await.unwrap(); assert_eq!(tx.signed_tx.nonce(), 102); - assert_eq!( - *tx.signed_tx.verification_key(), - other_signing_key.verification_key() - ); + assert_eq!(*tx.signed_tx.verification_key(), other.verification_key()); assert_eq!(priority.nonce_diff, 1); } @@ -758,7 +752,7 @@ mod test { mempool.insert(get_mock_tx(1), 0).await.unwrap(); // Insert txs from a different signer with nonces 100 and 101. - let other_signing_key = SigningKey::from([1; 32]); + let other = SigningKey::from([1; 32]); let other_mock_tx = |nonce: u32| -> SignedTransaction { let actions = get_mock_tx(0).actions().to_vec(); UnsignedTransaction { @@ -768,7 +762,7 @@ mod test { .build(), actions, } - .into_signed(&other_signing_key) + .into_signed(&other) }; mempool.insert(other_mock_tx(100), 0).await.unwrap(); mempool.insert(other_mock_tx(101), 0).await.unwrap(); @@ -776,19 +770,18 @@ mod test { assert_eq!(mempool.len().await, 4); // Check the pending nonce for alice is 1 and for the other signer is 101. - let alice_address = crate::app::test_utils::get_alice_signing_key_and_address().1; - assert_eq!(mempool.pending_nonce(&alice_address).await.unwrap(), 1); - let other_address = - crate::address::base_prefixed(other_signing_key.verification_key().address_bytes()); - assert_eq!(mempool.pending_nonce(&other_address).await.unwrap(), 101); + let alice = crate::app::test_utils::get_alice_signing_key(); + assert_eq!( + mempool.pending_nonce(alice.address_bytes()).await.unwrap(), + 1 + ); + assert_eq!( + mempool.pending_nonce(other.address_bytes()).await.unwrap(), + 101 + ); // Check the pending nonce for an address with no enqueued txs is `None`. - assert!( - mempool - .pending_nonce(&crate::address::base_prefixed([1; 20])) - .await - .is_none() - ); + assert!(mempool.pending_nonce([1; 20]).await.is_none()); } #[tokio::test] diff --git a/crates/astria-sequencer/src/proposal/commitment.rs b/crates/astria-sequencer/src/proposal/commitment.rs index c50b49e4a5..ed41a2aa9c 100644 --- a/crates/astria-sequencer/src/proposal/commitment.rs +++ b/crates/astria-sequencer/src/proposal/commitment.rs @@ -104,20 +104,19 @@ mod test { use rand::rngs::OsRng; use super::*; - use crate::assets::get_native_asset; #[test] fn generate_rollup_datas_commitment_should_ignore_transfers() { let sequence_action = SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"helloworld".to_vec(), - fee_asset: get_native_asset().clone(), + fee_asset: crate::test_utils::nria().into(), }; let transfer_action = TransferAction { - to: crate::address::base_prefixed([0u8; 20]), + to: crate::test_utils::astria_address(&[0u8; 20]), amount: 1, - asset: get_native_asset().clone(), - fee_asset: get_native_asset().clone(), + asset: crate::test_utils::nria().into(), + fee_asset: crate::test_utils::nria().into(), }; let signing_key = SigningKey::new(OsRng); @@ -165,13 +164,13 @@ mod test { let sequence_action = SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"helloworld".to_vec(), - fee_asset: get_native_asset().clone(), + fee_asset: crate::test_utils::nria().into(), }; let transfer_action = TransferAction { - to: crate::address::base_prefixed([0u8; 20]), + to: crate::test_utils::astria_address(&[0u8; 20]), amount: 1, - asset: get_native_asset().clone(), - fee_asset: get_native_asset().clone(), + asset: crate::test_utils::nria().into(), + fee_asset: crate::test_utils::nria().into(), }; let signing_key = SigningKey::new(OsRng); diff --git a/crates/astria-sequencer/src/sequencer.rs b/crates/astria-sequencer/src/sequencer.rs index 7e89363900..1e7ef52bc4 100644 --- a/crates/astria-sequencer/src/sequencer.rs +++ b/crates/astria-sequencer/src/sequencer.rs @@ -89,18 +89,14 @@ impl Sequencer { // denomination, and it is set in storage during init_chain. // on subsequent startups, we load the native asset from storage. if storage.latest_version() != u64::MAX { - // native asset should be stored, fetch it - let native_asset = snapshot + let _ = snapshot .get_native_asset() .await - .context("failed to get native asset from storage")?; - crate::assets::initialize_native_asset(&native_asset.to_string()); - let base_prefix = snapshot + .context("failed to query state for native asset")?; + let _ = snapshot .get_base_prefix() .await - .context("failed to get address base prefix from storage")?; - crate::address::initialize_base_prefix(&base_prefix) - .context("failed to initialize global address base prefix")?; + .context("failed to query state for base prefix")?; } let mempool = Mempool::new(); diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 77155d4ac3..b9da05020e 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -233,7 +233,6 @@ mod test { use super::*; use crate::{ app::test_utils::default_fees, - assets::get_native_asset, mempool::Mempool, metrics::Metrics, proposal::commitment::generate_rollup_datas_commitment, @@ -249,7 +248,7 @@ mod test { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"helloworld".to_vec(), - fee_asset: get_native_asset().clone(), + fee_asset: crate::test_utils::nria().into(), } .into(), ], @@ -448,7 +447,7 @@ mod test { async fn new_consensus_service(funded_key: Option) -> (Consensus, Mempool) { let accounts = if funded_key.is_some() { vec![Account { - address: crate::address::base_prefixed(funded_key.unwrap().address_bytes()), + address: crate::test_utils::astria_address(&funded_key.unwrap().address_bytes()), balance: 10u128.pow(19), }] } else { @@ -457,12 +456,12 @@ mod test { let genesis_state = UncheckedGenesisState { accounts, address_prefixes: AddressPrefixes { - base: crate::address::get_base_prefix().to_string(), + base: crate::test_utils::ASTRIA_PREFIX.into(), }, - authority_sudo_address: crate::address::base_prefixed([0; 20]), - ibc_sudo_address: crate::address::base_prefixed([0; 20]), + authority_sudo_address: crate::test_utils::astria_address(&[0; 20]), + ibc_sudo_address: crate::test_utils::astria_address(&[0; 20]), ibc_relayer_addresses: vec![], - native_asset_base_denomination: "nria".to_string(), + native_asset_base_denomination: crate::test_utils::nria(), ibc_params: penumbra_ibc::params::IBCParameters::default(), allowed_fee_assets: vec!["nria".parse().unwrap()], fees: default_fees(), diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 400b930b5b..f887be2a7d 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -190,9 +190,11 @@ mod test { use super::Info; use crate::{ accounts::StateWriteExt as _, + address::{ + StateReadExt as _, + StateWriteExt as _, + }, assets::{ - get_native_asset, - initialize_native_asset, StateReadExt as _, StateWriteExt as _, }, @@ -214,16 +216,17 @@ mod test { let mut state = StateDelta::new(storage.latest_snapshot()); state.put_storage_version_by_height(height, version); - initialize_native_asset("nria"); + state.put_base_prefix("astria").unwrap(); + state.put_native_asset(&crate::test_utils::nria()); - let address = crate::address::try_base_prefixed( - &hex::decode("a034c743bed8f26cb8ee7b8db2230fd8347ae131").unwrap(), - ) - .unwrap(); + let address = state + .try_base_prefixed(&hex::decode("a034c743bed8f26cb8ee7b8db2230fd8347ae131").unwrap()) + .await + .unwrap(); let balance = 1000; state - .put_account_balance(address, get_native_asset(), balance) + .put_account_balance(address, crate::test_utils::nria(), balance) .unwrap(); state.put_block_height(height); storage.commit(state).await.unwrap(); @@ -250,7 +253,7 @@ mod test { assert!(query_response.code.is_ok()); let expected_balance = AssetBalance { - denom: get_native_asset().clone(), + denom: crate::test_utils::nria().into(), balance, }; diff --git a/crates/astria-sequencer/src/service/mempool.rs b/crates/astria-sequencer/src/service/mempool.rs index 3257a6ac44..8ce3d114d3 100644 --- a/crates/astria-sequencer/src/service/mempool.rs +++ b/crates/astria-sequencer/src/service/mempool.rs @@ -7,6 +7,7 @@ use std::{ time::Instant, }; +use anyhow::Context as _; use astria_core::{ generated::protocol::transaction::v1alpha1 as raw, protocol::{ @@ -18,6 +19,7 @@ use cnidarium::Storage; use futures::{ Future, FutureExt, + TryFutureExt as _, }; use prost::Message as _; use tendermint::v0_38::abci::{ @@ -34,7 +36,8 @@ use tracing::{ }; use crate::{ - accounts::StateReadExt, + accounts, + address, mempool::{ Mempool as AppMempool, RemovalReason, @@ -102,7 +105,7 @@ impl Service for Mempool { /// If the tx passes all checks, status code 0 is returned. #[allow(clippy::too_many_lines)] #[instrument(skip_all)] -async fn handle_check_tx( +async fn handle_check_tx( req: request::CheckTx, state: S, mempool: &mut AppMempool, @@ -259,12 +262,22 @@ async fn handle_check_tx( ); // tx is valid, push to mempool - let current_account_nonce = state - .get_account_nonce(crate::address::base_prefixed( - signed_tx.verification_key().address_bytes(), - )) + let current_account_nonce = match state + .try_base_prefixed(&signed_tx.verification_key().address_bytes()) + .and_then(|address| state.get_account_nonce(address)) .await - .expect("can fetch account nonce"); + .context("failed fetching nonce for account") + { + Err(err) => { + return response::CheckTx { + code: AbciErrorCode::INTERNAL_ERROR.into(), + info: AbciErrorCode::INTERNAL_ERROR.to_string(), + log: format!("transaction failed execution because: {err:#?}"), + ..response::CheckTx::default() + }; + } + Ok(nonce) => nonce, + }; let actions_count = signed_tx.actions().len(); diff --git a/crates/astria-sequencer/src/test_utils.rs b/crates/astria-sequencer/src/test_utils.rs index 15d34d4e56..24a078e30f 100644 --- a/crates/astria-sequencer/src/test_utils.rs +++ b/crates/astria-sequencer/src/test_utils.rs @@ -1,8 +1,37 @@ -use astria_core::crypto::{ - SigningKey, - VerificationKey, +use astria_core::{ + crypto::{ + SigningKey, + VerificationKey, + }, + primitive::v1::{ + asset::TracePrefixed, + Address, + }, }; +pub(crate) const ASTRIA_PREFIX: &str = "astria"; + +pub(crate) fn astria_address(bytes: &[u8]) -> Address { + Address::builder() + .prefix(ASTRIA_PREFIX) + .slice(bytes) + .try_build() + .unwrap() +} + +pub(crate) fn astria_address_from_hex_string(s: &str) -> Address { + let bytes = hex::decode(s).unwrap(); + Address::builder() + .prefix(ASTRIA_PREFIX) + .slice(bytes) + .try_build() + .unwrap() +} + +pub(crate) fn nria() -> TracePrefixed { + "nria".parse().unwrap() +} + pub(crate) fn verification_key(seed: u64) -> VerificationKey { use rand::SeedableRng as _; let rng = rand_chacha::ChaChaRng::seed_from_u64(seed); diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index e36ad95587..f96f26ebd3 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -22,18 +22,25 @@ use astria_core::{ use tracing::instrument; use crate::{ - accounts::StateReadExt, + accounts, + address, bridge::StateReadExt as _, ibc::StateReadExt as _, state_ext::StateReadExt as _, }; #[instrument(skip_all)] -pub(crate) async fn check_nonce_mempool( - tx: &SignedTransaction, - state: &S, -) -> anyhow::Result<()> { - let signer_address = crate::address::base_prefixed(tx.verification_key().address_bytes()); +pub(crate) async fn check_nonce_mempool(tx: &SignedTransaction, state: &S) -> anyhow::Result<()> +where + S: accounts::StateReadExt + address::StateReadExt + 'static, +{ + let signer_address = state + .try_base_prefixed(&tx.verification_key().address_bytes()) + .await + .context( + "failed constructing the signer address from signed transaction verification and \ + prefix provided by app state", + )?; let curr_nonce = state .get_account_nonce(signer_address) .await @@ -43,10 +50,13 @@ pub(crate) async fn check_nonce_mempool( } #[instrument(skip_all)] -pub(crate) async fn check_chain_id_mempool( +pub(crate) async fn check_chain_id_mempool( tx: &SignedTransaction, state: &S, -) -> anyhow::Result<()> { +) -> anyhow::Result<()> +where + S: accounts::StateReadExt + 'static, +{ let chain_id = state .get_chain_id() .await @@ -56,11 +66,20 @@ pub(crate) async fn check_chain_id_mempool( } #[instrument(skip_all)] -pub(crate) async fn check_balance_mempool( +pub(crate) async fn check_balance_mempool( tx: &SignedTransaction, state: &S, -) -> anyhow::Result<()> { - let signer_address = crate::address::base_prefixed(tx.verification_key().address_bytes()); +) -> anyhow::Result<()> +where + S: accounts::StateReadExt + address::StateReadExt + 'static, +{ + let signer_address = state + .try_base_prefixed(&tx.verification_key().address_bytes()) + .await + .context( + "failed constructing the signer address from signed transaction verification and \ + prefix provided by app state", + )?; check_balance_for_total_fees_and_transfers(tx.unsigned_transaction(), signer_address, state) .await .context("failed to check balance for total fees and transfers")?; @@ -68,7 +87,7 @@ pub(crate) async fn check_balance_mempool( } #[instrument(skip_all)] -pub(crate) async fn get_fees_for_transaction( +pub(crate) async fn get_fees_for_transaction( tx: &UnsignedTransaction, state: &S, ) -> anyhow::Result> { @@ -144,11 +163,14 @@ pub(crate) async fn get_fees_for_transaction( // Checks that the account has enough balance to cover the total fees and transferred values // for all actions in the transaction. #[instrument(skip_all)] -pub(crate) async fn check_balance_for_total_fees_and_transfers( +pub(crate) async fn check_balance_for_total_fees_and_transfers( tx: &UnsignedTransaction, from: Address, state: &S, -) -> anyhow::Result<()> { +) -> anyhow::Result<()> +where + S: accounts::StateReadExt + 'static, +{ let mut cost_by_asset = get_fees_for_transaction(tx, state) .await .context("failed to get fees for transaction")?; @@ -224,12 +246,15 @@ fn transfer_update_fees( .or_insert(transfer_fee); } -async fn sequence_update_fees( +async fn sequence_update_fees( state: &S, fee_asset: &asset::Denom, fees_by_asset: &mut HashMap, data: &[u8], -) -> anyhow::Result<()> { +) -> anyhow::Result<()> +where + S: accounts::StateReadExt, +{ let fee = crate::sequence::calculate_fee_from_state(data, state) .await .context("fee for sequence action overflowed; data too large")?; @@ -290,6 +315,10 @@ fn bridge_unlock_update_fees( #[cfg(test)] mod tests { + use address::{ + StateReadExt, + StateWriteExt, + }; use astria_core::{ primitive::v1::{ asset::Denom, @@ -308,9 +337,13 @@ mod tests { use super::*; use crate::{ - accounts::StateWriteExt as _, + accounts::{ + StateReadExt as _, + StateWriteExt as _, + }, app::test_utils::*, - bridge::StateWriteExt, + assets::StateWriteExt as _, + bridge::StateWriteExt as _, ibc::StateWriteExt as _, sequence::StateWriteExt as _, }; @@ -321,6 +354,8 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); + state_tx.put_base_prefix("astria").unwrap(); + state_tx.put_native_asset(&crate::test_utils::nria()); state_tx.put_transfer_base_fee(12).unwrap(); state_tx.put_sequence_action_base_fee(0); state_tx.put_sequence_action_byte_cost_multiplier(1); @@ -329,17 +364,19 @@ mod tests { state_tx.put_bridge_lock_byte_cost_multiplier(1); state_tx.put_bridge_sudo_change_base_fee(24); - let native_asset = crate::assets::get_native_asset(); let other_asset = "other".parse::().unwrap(); - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let amount = 100; let data = [0; 32].to_vec(); let transfer_fee = state_tx.get_transfer_base_fee().await.unwrap(); state_tx .increase_balance( - alice_address, - native_asset, + state_tx + .try_base_prefixed(&alice.address_bytes()) + .await + .unwrap(), + crate::test_utils::nria(), transfer_fee + crate::sequence::calculate_fee_from_state(&data, &state_tx) .await @@ -348,7 +385,14 @@ mod tests { .await .unwrap(); state_tx - .increase_balance(alice_address, &other_asset, amount) + .increase_balance( + state_tx + .try_base_prefixed(&alice.address_bytes()) + .await + .unwrap(), + &other_asset, + amount, + ) .await .unwrap(); @@ -356,13 +400,13 @@ mod tests { Action::Transfer(TransferAction { asset: other_asset.clone(), amount, - fee_asset: native_asset.clone(), - to: crate::address::base_prefixed([0; ADDRESS_LEN]), + fee_asset: crate::test_utils::nria().into(), + to: state_tx.try_base_prefixed(&[0; ADDRESS_LEN]).await.unwrap(), }), Action::Sequence(SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), data, - fee_asset: native_asset.clone(), + fee_asset: crate::test_utils::nria().into(), }), ]; @@ -375,7 +419,7 @@ mod tests { params, }; - let signed_tx = tx.into_signed(&alice_signing_key); + let signed_tx = tx.into_signed(&alice); check_balance_mempool(&signed_tx, &state_tx) .await .expect("sufficient balance for all actions"); @@ -387,6 +431,8 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); + state_tx.put_base_prefix("nria").unwrap(); + state_tx.put_native_asset(&crate::test_utils::nria()); state_tx.put_transfer_base_fee(12).unwrap(); state_tx.put_sequence_action_base_fee(0); state_tx.put_sequence_action_byte_cost_multiplier(1); @@ -395,17 +441,19 @@ mod tests { state_tx.put_bridge_lock_byte_cost_multiplier(1); state_tx.put_bridge_sudo_change_base_fee(24); - let native_asset = crate::assets::get_native_asset(); let other_asset = "other".parse::().unwrap(); - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let alice = get_alice_signing_key(); let amount = 100; let data = [0; 32].to_vec(); let transfer_fee = state_tx.get_transfer_base_fee().await.unwrap(); state_tx .increase_balance( - alice_address, - native_asset, + state_tx + .try_base_prefixed(&alice.address_bytes()) + .await + .unwrap(), + crate::test_utils::nria(), transfer_fee + crate::sequence::calculate_fee_from_state(&data, &state_tx) .await @@ -418,13 +466,13 @@ mod tests { Action::Transfer(TransferAction { asset: other_asset.clone(), amount, - fee_asset: native_asset.clone(), - to: crate::address::base_prefixed([0; ADDRESS_LEN]), + fee_asset: crate::test_utils::nria().into(), + to: state_tx.try_base_prefixed(&[0; ADDRESS_LEN]).await.unwrap(), }), Action::Sequence(SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), data, - fee_asset: native_asset.clone(), + fee_asset: crate::test_utils::nria().into(), }), ]; @@ -437,7 +485,7 @@ mod tests { params, }; - let signed_tx = tx.into_signed(&alice_signing_key); + let signed_tx = tx.into_signed(&alice); let err = check_balance_mempool(&signed_tx, &state_tx) .await .err() diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index 5348b0a42e..1426a91804 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -26,10 +26,9 @@ pub(crate) use checks::{ use tracing::instrument; use crate::{ - accounts::{ - StateReadExt, - StateWriteExt, - }, + accounts, + address, + bridge, ibc::{ host_interface::AstriaHost, StateReadExt as _, @@ -46,26 +45,37 @@ pub(crate) async fn check_stateless(tx: &SignedTransaction) -> anyhow::Result<() } #[instrument(skip_all)] -pub(crate) async fn check_stateful( - tx: &SignedTransaction, - state: &S, -) -> anyhow::Result<()> { - let signer_address = crate::address::base_prefixed(tx.verification_key().address_bytes()); +pub(crate) async fn check_stateful(tx: &SignedTransaction, state: &S) -> anyhow::Result<()> +where + S: accounts::StateReadExt + address::StateReadExt + 'static, +{ + let signer_address = state + .try_base_prefixed(&tx.verification_key().address_bytes()) + .await + .context( + "failed constructing signed address from state and verification key contained in \ + signed transaction", + )?; tx.unsigned_transaction() .check_stateful(state, signer_address) .await } -pub(crate) async fn execute( - tx: &SignedTransaction, - state: &mut S, -) -> anyhow::Result<()> { - use crate::bridge::{ - StateReadExt as _, - StateWriteExt as _, - }; - - let signer_address = crate::address::base_prefixed(tx.verification_key().address_bytes()); +pub(crate) async fn execute(tx: &SignedTransaction, state: &mut S) -> anyhow::Result<()> +where + S: accounts::StateReadExt + + accounts::StateWriteExt + + address::StateReadExt + + bridge::StateReadExt + + bridge::StateWriteExt, +{ + let signer_address = state + .try_base_prefixed(&tx.verification_key().address_bytes()) + .await + .context( + "failed constructing signed address from state and verification key contained in \ + signed transaction", + )?; if state .get_bridge_account_rollup_id(&signer_address) @@ -183,11 +193,10 @@ impl ActionHandler for UnsignedTransaction { Ok(()) } - async fn check_stateful( - &self, - state: &S, - from: Address, - ) -> anyhow::Result<()> { + async fn check_stateful(&self, state: &S, from: Address) -> anyhow::Result<()> + where + S: accounts::StateReadExt + 'static, + { // Transactions must match the chain id of the node. let chain_id = state.get_chain_id().await?; ensure!( @@ -271,7 +280,10 @@ impl ActionHandler for UnsignedTransaction { } #[instrument(skip_all)] - async fn execute(&self, state: &mut S, from: Address) -> anyhow::Result<()> { + async fn execute(&self, state: &mut S, from: Address) -> anyhow::Result<()> + where + S: accounts::StateReadExt + accounts::StateWriteExt, + { let from_nonce = state .get_account_nonce(from) .await From be139d558f1f57df2f99a2606fc688e889bf4ed9 Mon Sep 17 00:00:00 2001 From: Ethan Oroshiba Date: Thu, 1 Aug 2024 14:11:07 -0500 Subject: [PATCH 3/3] chore(core): Implement Protobuf trait for tx actions (#1320) ## Summary Implemented the `Protobuf` trait for all TX `Action`s and variants. ## Background `Action` and variants are refined types from generated Protobuf code, but did not implement the `Protobuf` trait. This change is to standardize the types `Error` and `Raw` across `Action`, as well as the methods `to_raw()`, `into_raw()`, `try_from_raw()`, and `try_from_raw_ref()`. ## Changes - Implemented `Protobuf` trait, types, and necessary functions for `Action` and all of its variants. - Deleted redundant functions which took ownership, but kept those which prioritized cost over their `ref` counterparts. ## Testing Passing all tests. ## Related Issues closes #1307 --------- Co-authored-by: Fraser Hutchison <190532+Fraser999@users.noreply.github.com> --- .../protocol/transaction/v1alpha1/action.rs | 316 ++++++++++++------ .../src/protocol/transaction/v1alpha1/mod.rs | 1 + crates/astria-sequencer/src/utils.rs | 1 + 3 files changed, 211 insertions(+), 107 deletions(-) diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index 81256f14e3..da22b1039e 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -45,32 +45,12 @@ pub enum Action { FeeChange(FeeChangeAction), } -impl Action { - #[must_use] - pub fn into_raw(self) -> raw::Action { - use raw::action::Value; - let kind = match self { - Action::Sequence(act) => Value::SequenceAction(act.into_raw()), - Action::Transfer(act) => Value::TransferAction(act.into_raw()), - Action::ValidatorUpdate(act) => Value::ValidatorUpdateAction(act.into_raw()), - Action::SudoAddressChange(act) => Value::SudoAddressChangeAction(act.into_raw()), - Action::Ibc(act) => Value::IbcAction(act.into()), - Action::Ics20Withdrawal(act) => Value::Ics20Withdrawal(act.into_raw()), - Action::IbcRelayerChange(act) => Value::IbcRelayerChangeAction(act.into_raw()), - Action::FeeAssetChange(act) => Value::FeeAssetChangeAction(act.into_raw()), - Action::InitBridgeAccount(act) => Value::InitBridgeAccountAction(act.into_raw()), - Action::BridgeLock(act) => Value::BridgeLockAction(act.into_raw()), - Action::BridgeUnlock(act) => Value::BridgeUnlockAction(act.into_raw()), - Action::BridgeSudoChange(act) => Value::BridgeSudoChangeAction(act.into_raw()), - Action::FeeChange(act) => Value::FeeChangeAction(act.into_raw()), - }; - raw::Action { - value: Some(kind), - } - } +impl Protobuf for Action { + type Error = ActionError; + type Raw = raw::Action; #[must_use] - pub fn to_raw(&self) -> raw::Action { + fn to_raw(&self) -> Self::Raw { use raw::action::Value; let kind = match self { Action::Sequence(act) => Value::SequenceAction(act.to_raw()), @@ -94,13 +74,23 @@ impl Action { } } + /// Attempt to convert from a reference to raw, unchecked protobuf [`raw::Action`]. + /// + /// # Errors + /// + /// Returns an error if conversion of one of the inner raw action variants + /// to a native action ([`SequenceAction`] or [`TransferAction`]) fails. + fn try_from_raw_ref(raw: &Self::Raw) -> Result { + Self::try_from_raw(raw.clone()) + } + /// Attempt to convert from a raw, unchecked protobuf [`raw::Action`]. /// /// # Errors /// /// Returns an error if conversion of one of the inner raw action variants /// to a native action ([`SequenceAction`] or [`TransferAction`]) fails. - pub fn try_from_raw(proto: raw::Action) -> Result { + fn try_from_raw(proto: raw::Action) -> Result { use raw::action::Value; let raw::Action { value, @@ -129,11 +119,12 @@ impl Action { Ics20Withdrawal::try_from_raw(act).map_err(ActionError::ics20_withdrawal)?, ), Value::IbcRelayerChangeAction(act) => Self::IbcRelayerChange( - IbcRelayerChangeAction::try_from_raw(&act) + IbcRelayerChangeAction::try_from_raw_ref(&act) .map_err(ActionError::ibc_relayer_change)?, ), Value::FeeAssetChangeAction(act) => Self::FeeAssetChange( - FeeAssetChangeAction::try_from_raw(&act).map_err(ActionError::fee_asset_change)?, + FeeAssetChangeAction::try_from_raw_ref(&act) + .map_err(ActionError::fee_asset_change)?, ), Value::InitBridgeAccountAction(act) => Self::InitBridgeAccount( InitBridgeAccountAction::try_from_raw(act) @@ -150,12 +141,14 @@ impl Action { .map_err(ActionError::bridge_sudo_change)?, ), Value::FeeChangeAction(act) => Self::FeeChange( - FeeChangeAction::try_from_raw(&act).map_err(ActionError::fee_change)?, + FeeChangeAction::try_from_raw_ref(&act).map_err(ActionError::fee_change)?, ), }; Ok(action) } +} +impl Action { #[must_use] pub fn as_sequence(&self) -> Option<&SequenceAction> { let Self::Sequence(sequence_action) = self else { @@ -580,7 +573,10 @@ pub struct ValidatorUpdate { pub verification_key: crate::crypto::VerificationKey, } -impl ValidatorUpdate { +impl Protobuf for ValidatorUpdate { + type Error = ValidatorUpdateError; + type Raw = crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate; + /// Create a validator update by verifying a raw protobuf-decoded /// [`crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate`]. /// @@ -589,7 +585,7 @@ impl ValidatorUpdate { /// is not set, or if `.pub_key` contains a non-ed25519 variant, or /// if the ed25519 has invalid bytes (that is, bytes from which an /// ed25519 public key cannot be constructed). - pub fn try_from_raw( + fn try_from_raw( value: crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate, ) -> Result { use crate::generated::astria_vendored::tendermint::crypto::{ @@ -623,13 +619,20 @@ impl ValidatorUpdate { }) } - #[must_use] - pub fn into_raw(self) -> crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate { - self.to_raw() + /// Create a validator update by verifying a reference to raw protobuf-decoded + /// [`crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate`]. + /// + /// # Errors + /// Returns an error if the `.power` field is negative, if `.pub_key` + /// is not set, or if `.pub_key` contains a non-ed25519 variant, or + /// if the ed25519 has invalid bytes (that is, bytes from which an + /// ed25519 public key cannot be constructed). + fn try_from_raw_ref(raw: &Self::Raw) -> Result { + Self::try_from_raw(raw.clone()) } #[must_use] - pub fn to_raw(&self) -> crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate { + fn to_raw(&self) -> crate::generated::astria_vendored::tendermint::abci::ValidatorUpdate { use crate::generated::astria_vendored::tendermint::crypto::{ public_key, PublicKey, @@ -676,9 +679,11 @@ pub struct SudoAddressChangeAction { pub new_address: Address, } -impl SudoAddressChangeAction { - #[must_use] - pub fn into_raw(self) -> raw::SudoAddressChangeAction { +impl Protobuf for SudoAddressChangeAction { + type Error = SudoAddressChangeActionError; + type Raw = raw::SudoAddressChangeAction; + + fn into_raw(self) -> raw::SudoAddressChangeAction { let Self { new_address, } = self; @@ -688,7 +693,7 @@ impl SudoAddressChangeAction { } #[must_use] - pub fn to_raw(&self) -> raw::SudoAddressChangeAction { + fn to_raw(&self) -> raw::SudoAddressChangeAction { let Self { new_address, } = self; @@ -697,15 +702,13 @@ impl SudoAddressChangeAction { } } - /// Convert from a raw, unchecked protobuf [`raw::SudoAddressChangeAction`]. + /// Convert from a reference to a raw, unchecked protobuf [`raw::SudoAddressChangeAction`]. /// /// # Errors /// /// Returns an error if the raw action's `new_address` did not have the expected /// length. - pub fn try_from_raw( - proto: raw::SudoAddressChangeAction, - ) -> Result { + fn try_from_raw_ref(proto: &Self::Raw) -> Result { let raw::SudoAddressChangeAction { new_address, } = proto; @@ -713,7 +716,7 @@ impl SudoAddressChangeAction { return Err(SudoAddressChangeActionError::field_not_set("new_address")); }; let new_address = - Address::try_from_raw(&new_address).map_err(SudoAddressChangeActionError::address)?; + Address::try_from_raw(new_address).map_err(SudoAddressChangeActionError::address)?; Ok(Self { new_address, }) @@ -841,9 +844,14 @@ impl Ics20Withdrawal { memo: self.memo.clone(), } } +} + +impl Protobuf for Ics20Withdrawal { + type Error = Ics20WithdrawalError; + type Raw = raw::Ics20Withdrawal; #[must_use] - pub fn to_raw(&self) -> raw::Ics20Withdrawal { + fn to_raw(&self) -> raw::Ics20Withdrawal { raw::Ics20Withdrawal { amount: Some(self.amount.into()), denom: self.denom.to_string(), @@ -859,7 +867,7 @@ impl Ics20Withdrawal { } #[must_use] - pub fn into_raw(self) -> raw::Ics20Withdrawal { + fn into_raw(self) -> raw::Ics20Withdrawal { raw::Ics20Withdrawal { amount: Some(self.amount.into()), denom: self.denom.to_string(), @@ -883,7 +891,7 @@ impl Ics20Withdrawal { /// - if the `return_address` field is invalid or missing /// - if the `timeout_height` field is missing /// - if the `source_channel` field is invalid - pub fn try_from_raw(proto: raw::Ics20Withdrawal) -> Result { + fn try_from_raw(proto: raw::Ics20Withdrawal) -> Result { let raw::Ics20Withdrawal { amount, denom, @@ -928,6 +936,64 @@ impl Ics20Withdrawal { bridge_address, }) } + + /// Convert from a reference to raw, unchecked protobuf [`raw::Ics20Withdrawal`]. + /// + /// # Errors + /// + /// - if the `amount` field is missing + /// - if the `denom` field is invalid + /// - if the `return_address` field is invalid or missing + /// - if the `timeout_height` field is missing + /// - if the `source_channel` field is invalid + fn try_from_raw_ref(proto: &raw::Ics20Withdrawal) -> Result { + let raw::Ics20Withdrawal { + amount, + denom, + destination_chain_address, + return_address, + timeout_height, + timeout_time, + source_channel, + fee_asset, + memo, + bridge_address, + } = proto; + let amount = amount.ok_or(Ics20WithdrawalError::field_not_set("amount"))?; + let return_address = Address::try_from_raw( + return_address + .as_ref() + .ok_or(Ics20WithdrawalError::field_not_set("return_address"))?, + ) + .map_err(Ics20WithdrawalError::return_address)?; + + let timeout_height = timeout_height + .clone() + .ok_or(Ics20WithdrawalError::field_not_set("timeout_height"))? + .into(); + let bridge_address = bridge_address + .as_ref() + .map(Address::try_from_raw) + .transpose() + .map_err(Ics20WithdrawalError::invalid_bridge_address)?; + + Ok(Self { + amount: amount.into(), + denom: denom.parse().map_err(Ics20WithdrawalError::invalid_denom)?, + destination_chain_address: destination_chain_address.clone(), + return_address, + timeout_height, + timeout_time: *timeout_time, + source_channel: source_channel + .parse() + .map_err(Ics20WithdrawalError::invalid_source_channel)?, + fee_asset: fee_asset + .parse() + .map_err(Ics20WithdrawalError::invalid_fee_asset)?, + memo: memo.clone(), + bridge_address, + }) + } } impl From for IbcHeight { @@ -1022,25 +1088,12 @@ pub enum IbcRelayerChangeAction { Removal(Address), } -impl IbcRelayerChangeAction { - #[must_use] - pub fn into_raw(self) -> raw::IbcRelayerChangeAction { - match self { - IbcRelayerChangeAction::Addition(address) => raw::IbcRelayerChangeAction { - value: Some(raw::ibc_relayer_change_action::Value::Addition( - address.to_raw(), - )), - }, - IbcRelayerChangeAction::Removal(address) => raw::IbcRelayerChangeAction { - value: Some(raw::ibc_relayer_change_action::Value::Removal( - address.to_raw(), - )), - }, - } - } +impl Protobuf for IbcRelayerChangeAction { + type Error = IbcRelayerChangeActionError; + type Raw = raw::IbcRelayerChangeAction; #[must_use] - pub fn to_raw(&self) -> raw::IbcRelayerChangeAction { + fn to_raw(&self) -> raw::IbcRelayerChangeAction { match self { IbcRelayerChangeAction::Addition(address) => raw::IbcRelayerChangeAction { value: Some(raw::ibc_relayer_change_action::Value::Addition( @@ -1060,7 +1113,7 @@ impl IbcRelayerChangeAction { /// # Errors /// /// - if the `address` field is invalid - pub fn try_from_raw( + fn try_from_raw_ref( raw: &raw::IbcRelayerChangeAction, ) -> Result { match raw { @@ -1116,25 +1169,12 @@ pub enum FeeAssetChangeAction { Removal(asset::Denom), } -impl FeeAssetChangeAction { - #[must_use] - pub fn into_raw(self) -> raw::FeeAssetChangeAction { - match self { - FeeAssetChangeAction::Addition(asset) => raw::FeeAssetChangeAction { - value: Some(raw::fee_asset_change_action::Value::Addition( - asset.to_string(), - )), - }, - FeeAssetChangeAction::Removal(asset) => raw::FeeAssetChangeAction { - value: Some(raw::fee_asset_change_action::Value::Removal( - asset.to_string(), - )), - }, - } - } +impl Protobuf for FeeAssetChangeAction { + type Error = FeeAssetChangeActionError; + type Raw = raw::FeeAssetChangeAction; #[must_use] - pub fn to_raw(&self) -> raw::FeeAssetChangeAction { + fn to_raw(&self) -> raw::FeeAssetChangeAction { match self { FeeAssetChangeAction::Addition(asset) => raw::FeeAssetChangeAction { value: Some(raw::fee_asset_change_action::Value::Addition( @@ -1149,12 +1189,12 @@ impl FeeAssetChangeAction { } } - /// Convert from a raw, unchecked protobuf [`raw::FeeAssetChangeAction`]. + /// Convert from a reference to a raw, unchecked protobuf [`raw::FeeAssetChangeAction`]. /// /// # Errors /// /// - if the `asset` field is invalid - pub fn try_from_raw( + fn try_from_raw_ref( raw: &raw::FeeAssetChangeAction, ) -> Result { match raw { @@ -1221,9 +1261,12 @@ pub struct InitBridgeAccountAction { pub withdrawer_address: Option
, } -impl InitBridgeAccountAction { +impl Protobuf for InitBridgeAccountAction { + type Error = InitBridgeAccountActionError; + type Raw = raw::InitBridgeAccountAction; + #[must_use] - pub fn into_raw(self) -> raw::InitBridgeAccountAction { + fn into_raw(self) -> raw::InitBridgeAccountAction { raw::InitBridgeAccountAction { rollup_id: Some(self.rollup_id.to_raw()), asset: self.asset.to_string(), @@ -1234,7 +1277,7 @@ impl InitBridgeAccountAction { } #[must_use] - pub fn to_raw(&self) -> raw::InitBridgeAccountAction { + fn to_raw(&self) -> raw::InitBridgeAccountAction { raw::InitBridgeAccountAction { rollup_id: Some(self.rollup_id.to_raw()), asset: self.asset.to_string(), @@ -1252,7 +1295,7 @@ impl InitBridgeAccountAction { /// - if the `rollup_id` field is invalid /// - if the `sudo_address` field is invalid /// - if the `withdrawer_address` field is invalid - pub fn try_from_raw( + fn try_from_raw( proto: raw::InitBridgeAccountAction, ) -> Result { let Some(rollup_id) = proto.rollup_id else { @@ -1289,6 +1332,18 @@ impl InitBridgeAccountAction { withdrawer_address, }) } + + /// Convert from a reference to a raw, unchecked protobuf [`raw::InitBridgeAccountAction`]. + /// + /// # Errors + /// + /// - if the `rollup_id` field is not set + /// - if the `rollup_id` field is invalid + /// - if the `sudo_address` field is invalid + /// - if the `withdrawer_address` field is invalid + fn try_from_raw_ref(proto: &Self::Raw) -> Result { + Self::try_from_raw(proto.clone()) + } } #[derive(Debug, thiserror::Error)] @@ -1362,9 +1417,12 @@ pub struct BridgeLockAction { pub destination_chain_address: String, } -impl BridgeLockAction { +impl Protobuf for BridgeLockAction { + type Error = BridgeLockActionError; + type Raw = raw::BridgeLockAction; + #[must_use] - pub fn into_raw(self) -> raw::BridgeLockAction { + fn into_raw(self) -> raw::BridgeLockAction { raw::BridgeLockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), @@ -1375,7 +1433,7 @@ impl BridgeLockAction { } #[must_use] - pub fn to_raw(&self) -> raw::BridgeLockAction { + fn to_raw(&self) -> raw::BridgeLockAction { raw::BridgeLockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), @@ -1393,7 +1451,7 @@ impl BridgeLockAction { /// - if the `to` field is invalid /// - if the `asset` field is invalid /// - if the `fee_asset` field is invalid - pub fn try_from_raw(proto: raw::BridgeLockAction) -> Result { + fn try_from_raw(proto: raw::BridgeLockAction) -> Result { let Some(to) = proto.to else { return Err(BridgeLockActionError::field_not_set("to")); }; @@ -1417,6 +1475,18 @@ impl BridgeLockAction { destination_chain_address: proto.destination_chain_address, }) } + + /// Convert from a reference to a raw, unchecked protobuf [`raw::BridgeLockAction`]. + /// + /// # Errors + /// + /// - if the `to` field is not set + /// - if the `to` field is invalid + /// - if the `asset` field is invalid + /// - if the `fee_asset` field is invalid + fn try_from_raw_ref(proto: &raw::BridgeLockAction) -> Result { + Self::try_from_raw(proto.clone()) + } } #[derive(Debug, thiserror::Error)] @@ -1481,9 +1551,12 @@ pub struct BridgeUnlockAction { pub bridge_address: Option
, } -impl BridgeUnlockAction { +impl Protobuf for BridgeUnlockAction { + type Error = BridgeUnlockActionError; + type Raw = raw::BridgeUnlockAction; + #[must_use] - pub fn into_raw(self) -> raw::BridgeUnlockAction { + fn into_raw(self) -> raw::BridgeUnlockAction { raw::BridgeUnlockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), @@ -1494,7 +1567,7 @@ impl BridgeUnlockAction { } #[must_use] - pub fn to_raw(&self) -> raw::BridgeUnlockAction { + fn to_raw(&self) -> raw::BridgeUnlockAction { raw::BridgeUnlockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), @@ -1513,7 +1586,7 @@ impl BridgeUnlockAction { /// - if the `amount` field is invalid /// - if the `fee_asset` field is invalid /// - if the `from` field is invalid - pub fn try_from_raw(proto: raw::BridgeUnlockAction) -> Result { + fn try_from_raw(proto: raw::BridgeUnlockAction) -> Result { let Some(to) = proto.to else { return Err(BridgeUnlockActionError::field_not_set("to")); }; @@ -1539,6 +1612,19 @@ impl BridgeUnlockAction { bridge_address, }) } + + /// Convert from a reference to a raw, unchecked protobuf [`raw::BridgeUnlockAction`]. + /// + /// # Errors + /// + /// - if the `to` field is not set + /// - if the `to` field is invalid + /// - if the `amount` field is invalid + /// - if the `fee_asset` field is invalid + /// - if the `from` field is invalid + fn try_from_raw_ref(proto: &raw::BridgeUnlockAction) -> Result { + Self::try_from_raw(proto.clone()) + } } #[derive(Debug, thiserror::Error)] @@ -1597,9 +1683,12 @@ pub struct BridgeSudoChangeAction { pub fee_asset: asset::Denom, } -impl BridgeSudoChangeAction { +impl Protobuf for BridgeSudoChangeAction { + type Error = BridgeSudoChangeActionError; + type Raw = raw::BridgeSudoChangeAction; + #[must_use] - pub fn into_raw(self) -> raw::BridgeSudoChangeAction { + fn into_raw(self) -> raw::BridgeSudoChangeAction { raw::BridgeSudoChangeAction { bridge_address: Some(self.bridge_address.to_raw()), new_sudo_address: self.new_sudo_address.map(Address::into_raw), @@ -1609,7 +1698,7 @@ impl BridgeSudoChangeAction { } #[must_use] - pub fn to_raw(&self) -> raw::BridgeSudoChangeAction { + fn to_raw(&self) -> raw::BridgeSudoChangeAction { raw::BridgeSudoChangeAction { bridge_address: Some(self.bridge_address.to_raw()), new_sudo_address: self.new_sudo_address.as_ref().map(Address::to_raw), @@ -1627,7 +1716,7 @@ impl BridgeSudoChangeAction { /// - if the `new_sudo_address` field is invalid /// - if the `new_withdrawer_address` field is invalid /// - if the `fee_asset` field is invalid - pub fn try_from_raw( + fn try_from_raw( proto: raw::BridgeSudoChangeAction, ) -> Result { let Some(bridge_address) = proto.bridge_address else { @@ -1659,6 +1748,21 @@ impl BridgeSudoChangeAction { fee_asset, }) } + + /// Convert from a reference to a raw, unchecked protobuf [`raw::BridgeSudoChangeAction`]. + /// + /// # Errors + /// + /// - if the `bridge_address` field is not set + /// - if the `bridge_address` field is invalid + /// - if the `new_sudo_address` field is invalid + /// - if the `new_withdrawer_address` field is invalid + /// - if the `fee_asset` field is invalid + fn try_from_raw_ref( + proto: &raw::BridgeSudoChangeAction, + ) -> Result { + Self::try_from_raw(proto.clone()) + } } #[derive(Debug, thiserror::Error)] @@ -1724,14 +1828,12 @@ pub struct FeeChangeAction { pub new_value: u128, } -impl FeeChangeAction { - #[must_use] - pub fn into_raw(self) -> raw::FeeChangeAction { - self.to_raw() - } +impl Protobuf for FeeChangeAction { + type Error = FeeChangeActionError; + type Raw = raw::FeeChangeAction; #[must_use] - pub fn to_raw(&self) -> raw::FeeChangeAction { + fn to_raw(&self) -> raw::FeeChangeAction { raw::FeeChangeAction { value: Some(match self.fee_change { FeeChange::TransferBaseFee => { @@ -1761,13 +1863,13 @@ impl FeeChangeAction { } } - /// Convert from a raw, unchecked protobuf [`raw::FeeChangeAction`]. + /// Convert from a reference to a raw, unchecked protobuf [`raw::FeeChangeAction`]. /// /// # Errors /// /// - if the fee change `value` field is missing /// - if the `new_value` field is missing - pub fn try_from_raw(proto: &raw::FeeChangeAction) -> Result { + fn try_from_raw_ref(proto: &raw::FeeChangeAction) -> Result { let (fee_change, new_value) = match proto.value { Some(raw::fee_change_action::Value::TransferBaseFee(new_value)) => { (FeeChange::TransferBaseFee, new_value) diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index 77032792cd..d5e8543001 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -15,6 +15,7 @@ use crate::{ asset, ADDRESS_LEN, }, + Protobuf as _, }; pub mod action; diff --git a/crates/astria-sequencer/src/utils.rs b/crates/astria-sequencer/src/utils.rs index d2d2d3bf27..0ee4ffffa6 100644 --- a/crates/astria-sequencer/src/utils.rs +++ b/crates/astria-sequencer/src/utils.rs @@ -2,6 +2,7 @@ use anyhow::Context as _; use astria_core::{ generated::astria_vendored::tendermint::abci as raw, protocol::transaction::v1alpha1::action::ValidatorUpdate, + Protobuf as _, }; pub(crate) struct Hex<'a>(pub(crate) &'a [u8]);