Skip to content

Commit

Permalink
refactor(sequencer): move asset state methods to asset module (#1313)
Browse files Browse the repository at this point in the history
## 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 `<component>::StateReadExt` and
`<component>::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 <component>::StateReadExt` (and similar for write)
instead of `use <component>::state_ext::StateReadExt`.

## Testing
No tests were changed (minus import paths). Asset-related tests were
moved (including snapshot tests)

## Related Issues
Closes #1312
  • Loading branch information
SuperFluffy authored Aug 1, 2024
1 parent 7953133 commit 8171eed
Show file tree
Hide file tree
Showing 42 changed files with 960 additions and 937 deletions.
27 changes: 14 additions & 13 deletions crates/astria-sequencer/src/accounts/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S: StateReadExt + 'static>(
pub(crate) async fn transfer_check_stateful<S>(
action: &TransferAction,
state: &S,
from: Address,
) -> Result<()> {
) -> Result<()>
where
S: accounts::StateReadExt + assets::StateReadExt + 'static,
{
ensure!(
state
.is_allowed_fee_asset(&action.fee_asset)
Expand Down Expand Up @@ -87,7 +85,7 @@ impl ActionHandler for TransferAction {
Ok(())
}

async fn check_stateful<S: StateReadExt + 'static>(
async fn check_stateful<S: accounts::StateReadExt + 'static>(
&self,
state: &S,
from: Address,
Expand All @@ -107,7 +105,10 @@ impl ActionHandler for TransferAction {
}

#[instrument(skip_all)]
async fn execute<S: StateWriteExt>(&self, state: &mut S, from: Address) -> Result<()> {
async fn execute<S>(&self, state: &mut S, from: Address) -> Result<()>
where
S: accounts::StateWriteExt + assets::StateWriteExt,
{
let fee = state
.get_transfer_base_fee()
.await
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-sequencer/src/accounts/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tracing::instrument;

use super::state_ext::StateWriteExt;
use crate::{
asset::get_native_asset,
assets::get_native_asset,
component::Component,
};

Expand Down
7 changes: 6 additions & 1 deletion crates/astria-sequencer/src/accounts/mod.rs
Original file line number Diff line number Diff line change
@@ -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,
};
42 changes: 17 additions & 25 deletions crates/astria-sequencer/src/accounts/state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<AssetBalance>> {
use crate::asset::state_ext::StateReadExt as _;
use crate::assets::StateReadExt as _;

let prefix = format!("{}/balance/", StorageKey(&address));
let mut balances: Vec<AssetBalance> = Vec::new();
Expand All @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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]);
Expand Down
46 changes: 22 additions & 24 deletions crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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/<ID> 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/<ID>`")
};
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);
Expand Down Expand Up @@ -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<S: StateReadExt>(
async fn update_mempool_after_finalization<S: accounts::StateReadExt>(
mempool: &mut Mempool,
state: S,
) -> anyhow::Result<()> {
Expand Down
20 changes: 13 additions & 7 deletions crates/astria-sequencer/src/app/tests_app.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::collections::HashMap;

use astria_core::{
primitive::v1::RollupId,
primitive::v1::{
asset::TracePrefixed,
RollupId,
},
protocol::transaction::v1alpha1::{
action::{
BridgeLockAction,
Expand Down Expand Up @@ -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 _,
Expand Down Expand Up @@ -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::<TracePrefixed>().unwrap()
);
}

#[tokio::test]
Expand Down
4 changes: 2 additions & 2 deletions crates/astria-sequencer/src/app/tests_breaking_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
22 changes: 12 additions & 10 deletions crates/astria-sequencer/src/app/tests_execute_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 8171eed

Please sign in to comment.