diff --git a/charts/evm-rollup/Chart.yaml b/charts/evm-rollup/Chart.yaml index 5cc1e55191..a689d45bbd 100644 --- a/charts/evm-rollup/Chart.yaml +++ b/charts/evm-rollup/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.20.1 +version: 0.20.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to diff --git a/charts/evm-rollup/templates/configmap.yaml b/charts/evm-rollup/templates/configmap.yaml index a54405b7df..05abe412ed 100644 --- a/charts/evm-rollup/templates/configmap.yaml +++ b/charts/evm-rollup/templates/configmap.yaml @@ -68,6 +68,7 @@ data: {{- if not .Values.global.dev }} {{- else }} ASTRIA_COMPOSER_SEQUENCER_ADDRESS_PREFIX: "{{ .Values.config.sequencer.addressPrefixes.base }}" + ASTRIA_COMPOSER_FEE_ASSET: "{{ .Values.config.sequencer.nativeAssetBaseDenomination }}" {{- end }} --- {{- if .Values.config.faucet.enabled }} diff --git a/charts/evm-rollup/values.yaml b/charts/evm-rollup/values.yaml index 086752ac6a..351ea28828 100644 --- a/charts/evm-rollup/values.yaml +++ b/charts/evm-rollup/values.yaml @@ -173,6 +173,7 @@ config: sequencer: addressPrefixes: base: "astria" + nativeAssetBaseDenomination: nria chainId: "" # Block height to start syncing rollup from initialBlockHeight: "2" diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs index 87c2d67560..b3805f7db5 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/convert.rs @@ -6,7 +6,6 @@ use astria_core::{ asset::{ self, denom::TracePrefixed, - Denom, }, Address, }, @@ -55,8 +54,8 @@ pub(crate) struct EventWithMetadata { pub(crate) fn event_to_action( event_with_metadata: EventWithMetadata, - fee_asset_id: asset::Id, - rollup_asset_denom: Denom, + fee_asset: asset::Denom, + rollup_asset_denom: asset::Denom, asset_withdrawal_divisor: u128, bridge_address: Address, sequencer_address_prefix: &str, @@ -66,7 +65,7 @@ pub(crate) fn event_to_action( &event, event_with_metadata.block_number, event_with_metadata.transaction_hash, - fee_asset_id, + fee_asset, asset_withdrawal_divisor, ) .wrap_err("failed to convert sequencer withdrawal event to action")?, @@ -74,7 +73,7 @@ pub(crate) fn event_to_action( event, event_with_metadata.block_number, event_with_metadata.transaction_hash, - fee_asset_id, + fee_asset, rollup_asset_denom, asset_withdrawal_divisor, bridge_address, @@ -95,7 +94,7 @@ fn event_to_bridge_unlock( event: &SequencerWithdrawalFilter, block_number: U64, transaction_hash: TxHash, - fee_asset_id: asset::Id, + fee_asset: asset::Denom, asset_withdrawal_divisor: u128, ) -> eyre::Result { let memo = BridgeUnlockMemo { @@ -115,7 +114,7 @@ fn event_to_bridge_unlock( "failed to divide amount by asset withdrawal multiplier" ))?, memo: serde_json::to_vec(&memo).wrap_err("failed to serialize memo to json")?, - fee_asset_id, + fee_asset, bridge_address: None, }; @@ -128,8 +127,8 @@ fn event_to_ics20_withdrawal( event: Ics20WithdrawalFilter, block_number: U64, transaction_hash: TxHash, - fee_asset_id: asset::Id, - rollup_asset_denom: Denom, + fee_asset: asset::Denom, + rollup_asset_denom: asset::Denom, asset_withdrawal_divisor: u128, bridge_address: Address, sequencer_address_prefix: &str, @@ -172,7 +171,7 @@ fn event_to_ics20_withdrawal( "failed to divide amount by asset withdrawal multiplier" ))?, memo: serde_json::to_string(&memo).wrap_err("failed to serialize memo to json")?, - fee_asset_id, + fee_asset, // note: this refers to the timeout on the destination chain, which we are unaware of. // thus, we set it to the maximum possible value. timeout_height: IbcHeight::new(u64::MAX, u64::MAX) @@ -198,11 +197,13 @@ fn calculate_packet_timeout_time(timeout_delta: Duration) -> eyre::Result { #[cfg(test)] mod tests { - use asset::default_native_asset; - use super::*; use crate::bridge_withdrawer::ethereum::astria_withdrawer_interface::SequencerWithdrawalFilter; + fn default_native_asset() -> asset::Denom { + "nria".parse().unwrap() + } + #[test] fn event_to_bridge_unlock() { let denom = default_native_asset(); @@ -217,7 +218,7 @@ mod tests { }; let action = event_to_action( event_with_meta, - denom.id(), + denom.clone(), denom.clone(), 1, crate::astria_address([99u8; 20]), @@ -236,7 +237,7 @@ mod tests { transaction_hash: [2u8; 32].into(), }) .unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom, bridge_address: None, }; @@ -258,7 +259,7 @@ mod tests { let divisor = 10; let action = event_to_action( event_with_meta, - denom.id(), + denom.clone(), denom.clone(), divisor, crate::astria_address([99u8; 20]), @@ -277,7 +278,7 @@ mod tests { transaction_hash: [2u8; 32].into(), }) .unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom, bridge_address: None, }; @@ -286,7 +287,7 @@ mod tests { #[test] fn event_to_ics20_withdrawal() { - let denom = "transfer/channel-0/utia".parse::().unwrap(); + let denom = "transfer/channel-0/utia".parse::().unwrap(); let destination_chain_address = crate::astria_address([1u8; 20]).to_string(); let event_with_meta = EventWithMetadata { event: WithdrawalEvent::Ics20(Ics20WithdrawalFilter { @@ -302,7 +303,7 @@ mod tests { let bridge_address = crate::astria_address([99u8; 20]); let action = event_to_action( event_with_meta, - denom.id(), + denom.clone(), denom.clone(), 1, bridge_address, @@ -329,7 +330,7 @@ mod tests { transaction_hash: [2u8; 32], }) .unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom, timeout_height: IbcHeight::new(u64::MAX, u64::MAX).unwrap(), timeout_time: 0, // zero this for testing source_channel: "channel-0".parse().unwrap(), diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 41abd0939e..2150a0e8e8 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -121,7 +121,7 @@ pub(crate) struct Watcher { impl Watcher { pub(crate) async fn run(mut self) -> Result<()> { - let (provider, contract, fee_asset_id, asset_withdrawal_divisor, next_rollup_block_height) = + let (provider, contract, fee_asset, asset_withdrawal_divisor, next_rollup_block_height) = self.startup() .await .wrap_err("watcher failed to start up")?; @@ -144,7 +144,7 @@ impl Watcher { provider, submitter_handle, shutdown_token: shutdown_token.clone(), - fee_asset_id, + fee_asset, rollup_asset_denom, bridge_address, asset_withdrawal_divisor, @@ -197,13 +197,13 @@ impl Watcher { ) -> eyre::Result<( Arc>, IAstriaWithdrawer>, - asset::Id, + asset::Denom, u128, u64, )> { // wait for submitter to be ready let SequencerStartupInfo { - fee_asset_id, + fee_asset, next_batch_rollup_height, } = self .submitter_handle @@ -262,7 +262,7 @@ impl Watcher { Ok(( provider.clone(), contract, - fee_asset_id, + fee_asset, asset_withdrawal_divisor, next_batch_rollup_height, )) @@ -334,7 +334,7 @@ struct Batcher { provider: Arc>, submitter_handle: submitter::Handle, shutdown_token: CancellationToken, - fee_asset_id: asset::Id, + fee_asset: asset::Denom, rollup_asset_denom: Denom, bridge_address: Address, asset_withdrawal_divisor: u128, @@ -394,7 +394,7 @@ impl Batcher { }; let action = event_to_action( event_with_metadata, - self.fee_asset_id, + self.fee_asset.clone(), self.rollup_asset_denom.clone(), self.asset_withdrawal_divisor, self.bridge_address, @@ -445,9 +445,11 @@ fn address_from_string(s: &str) -> Result { #[cfg(test)] mod tests { - use asset::default_native_asset; use astria_core::{ - primitive::v1::Address, + primitive::v1::{ + asset, + Address, + }, protocol::transaction::v1alpha1::Action, }; use ethers::{ @@ -477,6 +479,10 @@ mod tests { }, }; + fn default_native_asset() -> asset::Denom { + "nria".parse().unwrap() + } + #[test] fn address_from_string_prefix() { let address = address_from_string("0x1234567890123456789012345678901234567890").unwrap(); @@ -567,7 +573,7 @@ mod tests { let denom = default_native_asset(); let expected_action = event_to_action( expected_event, - denom.id(), + denom.clone(), denom.clone(), 1, bridge_address, @@ -583,7 +589,7 @@ mod tests { let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); startup_tx .send(SequencerStartupInfo { - fee_asset_id: denom.id(), + fee_asset: denom.clone(), next_batch_rollup_height: 0, }) .unwrap(); @@ -666,7 +672,7 @@ mod tests { let denom = "transfer/channel-0/utia".parse::().unwrap(); let Action::Ics20Withdrawal(mut expected_action) = event_to_action( expected_event, - denom.id(), + denom.clone(), denom.clone(), 1, bridge_address, @@ -682,7 +688,7 @@ mod tests { let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); startup_tx .send(SequencerStartupInfo { - fee_asset_id: denom.id(), + fee_asset: denom.clone(), next_batch_rollup_height: 0, }) .unwrap(); @@ -693,7 +699,7 @@ mod tests { submitter_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), - rollup_asset_denom: denom, + rollup_asset_denom: denom.clone(), bridge_address, sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } @@ -792,7 +798,7 @@ mod tests { let bridge_address = crate::astria_address([1u8; 20]); let expected_action = event_to_action( expected_event, - denom.id(), + denom.clone(), denom.clone(), 1, bridge_address, @@ -808,7 +814,7 @@ mod tests { let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); startup_tx .send(SequencerStartupInfo { - fee_asset_id: denom.id(), + fee_asset: denom.clone(), next_batch_rollup_height: 0, }) .unwrap(); @@ -819,7 +825,7 @@ mod tests { submitter_handle, shutdown_token: CancellationToken::new(), state: Arc::new(State::new()), - rollup_asset_denom: denom, + rollup_asset_denom: denom.clone(), bridge_address, sequencer_address_prefix: crate::ASTRIA_ADDRESS_PREFIX.into(), } @@ -901,7 +907,7 @@ mod tests { let bridge_address = crate::astria_address([1u8; 20]); let Action::Ics20Withdrawal(mut expected_action) = event_to_action( expected_event, - denom.id(), + denom.clone(), denom.clone(), 1, bridge_address, @@ -917,7 +923,7 @@ mod tests { let submitter_handle = submitter::Handle::new(startup_rx, batch_tx); startup_tx .send(SequencerStartupInfo { - fee_asset_id: asset::Id::from_str_unchecked("transfer/channel-0/utia"), + fee_asset: "transfer/channel-0/utia".parse().unwrap(), next_batch_rollup_height: 0, }) .unwrap(); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index c9a0d2a3da..4265fb5eb7 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -91,7 +91,7 @@ impl BridgeWithdrawer { sequencer_key_path, sequencer_address_prefix: sequencer_address_prefix.clone(), state: state.clone(), - expected_fee_asset_id: asset::Id::from_str_unchecked(&fee_asset_denomination), + expected_fee_asset: fee_asset_denomination, min_expected_fee_asset_balance: u128::from(min_expected_fee_asset_balance), metrics, } @@ -202,7 +202,7 @@ impl BridgeWithdrawer { #[derive(Debug)] pub struct SequencerStartupInfo { - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, pub next_batch_rollup_height: u64, } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs index 8261a5f20c..5332c96129 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs @@ -62,7 +62,7 @@ pub(crate) struct Builder { pub(crate) sequencer_chain_id: String, pub(crate) sequencer_cometbft_endpoint: String, pub(crate) state: Arc, - pub(crate) expected_fee_asset_id: asset::Id, + pub(crate) expected_fee_asset: asset::Denom, pub(crate) min_expected_fee_asset_balance: u128, pub(crate) metrics: &'static Metrics, } @@ -77,7 +77,7 @@ impl Builder { sequencer_chain_id, sequencer_cometbft_endpoint, state, - expected_fee_asset_id, + expected_fee_asset, min_expected_fee_asset_balance, metrics, } = self; @@ -106,7 +106,7 @@ impl Builder { signer, sequencer_chain_id, startup_tx, - expected_fee_asset_id, + expected_fee_asset, min_expected_fee_asset_balance, metrics, }, diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs index 32206d4517..b996374be2 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs @@ -7,7 +7,7 @@ use astria_core::{ bridge::Ics20WithdrawalFromRollupMemo, primitive::v1::asset, protocol::{ - asset::v1alpha1::AllowedFeeAssetIdsResponse, + asset::v1alpha1::AllowedFeeAssetsResponse, bridge::v1alpha1::BridgeAccountLastTxHashResponse, transaction::v1alpha1::{ Action, @@ -87,7 +87,7 @@ pub(super) struct Submitter { signer: SequencerKey, sequencer_chain_id: String, startup_tx: oneshot::Sender, - expected_fee_asset_id: asset::Id, + expected_fee_asset: asset::Denom, min_expected_fee_asset_balance: u128, metrics: &'static Metrics, } @@ -184,15 +184,17 @@ impl Submitter { "sequencer_chain_id provided in config does not match chain_id returned from sequencer" ); + let expected_fee_asset_ibc = self.expected_fee_asset.to_ibc_prefixed(); // confirm that the fee asset ID is valid - let allowed_fee_asset_ids_resp = - get_allowed_fee_asset_ids(self.sequencer_cometbft_client.clone(), self.state.clone()) + let allowed_fee_assets_resp = + get_allowed_fee_assets(self.sequencer_cometbft_client.clone(), self.state.clone()) .await .wrap_err("failed to get allowed fee asset ids from sequencer")?; ensure!( - allowed_fee_asset_ids_resp - .fee_asset_ids - .contains(&self.expected_fee_asset_id), + allowed_fee_assets_resp + .fee_assets + .iter() + .any(|asset| asset.to_ibc_prefixed() == expected_fee_asset_ibc), "fee_asset_id provided in config is not a valid fee asset on the sequencer" ); @@ -207,7 +209,7 @@ impl Submitter { let fee_asset_balance = fee_asset_balances .balances .into_iter() - .find(|balance| balance.denom.id() == self.expected_fee_asset_id) + .find(|balance| balance.denom.to_ibc_prefixed() == expected_fee_asset_ibc) .ok_or_eyre("withdrawer's account does not have the minimum balance of the fee asset")? .balance; ensure!( @@ -225,7 +227,7 @@ impl Submitter { // send startup info to watcher let startup = SequencerStartupInfo { - fee_asset_id: self.expected_fee_asset_id, + fee_asset: self.expected_fee_asset.clone(), next_batch_rollup_height, }; Ok(startup) @@ -545,10 +547,10 @@ async fn get_sequencer_chain_id( } #[instrument(skip_all)] -async fn get_allowed_fee_asset_ids( +async fn get_allowed_fee_assets( client: sequencer_client::HttpClient, state: Arc, -) -> eyre::Result { +) -> eyre::Result { let retry_config = tryhard::RetryFutureConfig::new(u32::MAX) .exponential_backoff(Duration::from_millis(100)) .max_delay(Duration::from_secs(20)) @@ -572,7 +574,7 @@ async fn get_allowed_fee_asset_ids( }, ); - let res = tryhard::retry_fn(|| client.get_allowed_fee_asset_ids()) + let res = tryhard::retry_fn(|| client.get_allowed_fee_assets()) .with_config(retry_config) .await .wrap_err("failed to get allowed fee asset ids from Sequencer after a lot of attempts"); diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs index d44d777bb6..6df792ab6a 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/tests.rs @@ -10,11 +10,7 @@ use astria_core::{ bridge::Ics20WithdrawalFromRollupMemo, crypto::SigningKey, generated::protocol::account::v1alpha1::NonceResponse, - primitive::v1::asset::{ - self, - default_native_asset, - Denom, - }, + primitive::v1::asset, protocol::{ account::v1alpha1::AssetBalance, bridge::v1alpha1::BridgeAccountLastTxHashResponse, @@ -93,6 +89,10 @@ const DEFAULT_LAST_SEQUENCER_HEIGHT: u64 = 0; const DEFAULT_SEQUENCER_NONCE: u32 = 0; const DEFAULT_IBC_DENOM: &str = "transfer/channel-0/utia"; +fn default_native_asset() -> asset::Denom { + "nria".parse().unwrap() +} + static TELEMETRY: Lazy<()> = Lazy::new(|| { if std::env::var_os("TEST_LOG").is_some() { let filter_directives = std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()); @@ -153,7 +153,7 @@ impl TestSubmitter { sequencer_chain_id: SEQUENCER_CHAIN_ID.to_string(), sequencer_cometbft_endpoint, state, - expected_fee_asset_id: default_native_asset().id(), + expected_fee_asset: default_native_asset(), min_expected_fee_asset_balance: 1_000_000, metrics, } @@ -216,9 +216,9 @@ async fn register_default_chain_id_guard(cometbft_mock: &MockServer) -> MockGuar register_genesis_chain_id_response(SEQUENCER_CHAIN_ID, cometbft_mock).await } -async fn register_default_fee_asset_ids_guard(cometbft_mock: &MockServer) -> MockGuard { - let fee_asset_ids = vec![default_native_asset().id()]; - register_allowed_fee_asset_ids_response(fee_asset_ids, cometbft_mock).await +async fn register_default_fee_assets_guard(cometbft_mock: &MockServer) -> MockGuard { + let fee_assets = vec![default_native_asset()]; + register_allowed_fee_assets_response(fee_assets, cometbft_mock).await } async fn register_default_min_expected_fee_asset_balance_guard( @@ -249,8 +249,8 @@ async fn register_startup_guards(cometbft_mock: &MockServer) -> HashMap HashMap Action { - let denom = DEFAULT_IBC_DENOM.parse::().unwrap(); + let denom = DEFAULT_IBC_DENOM.parse::().unwrap(); let destination_chain_address = "address".to_string(); let inner = Ics20Withdrawal { denom: denom.clone(), @@ -287,7 +287,7 @@ fn make_ics20_withdrawal_action() -> Action { transaction_hash: [2u8; 32], }) .unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom, timeout_height: IbcHeight::new(u64::MAX, u64::MAX).unwrap(), timeout_time: 0, // zero this for testing source_channel: "channel-0".parse().unwrap(), @@ -307,7 +307,7 @@ fn make_bridge_unlock_action() -> Action { transaction_hash: [1u8; 32].into(), }) .unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom, bridge_address: None, }; Action::BridgeUnlock(inner) @@ -464,14 +464,14 @@ async fn register_genesis_chain_id_response(chain_id: &str, server: &MockServer) .await } -async fn register_allowed_fee_asset_ids_response( - fee_asset_ids: Vec, +async fn register_allowed_fee_assets_response( + fee_assets: Vec, cometbft_mock: &MockServer, ) -> MockGuard { let response = tendermint_rpc::endpoint::abci_query::Response { response: tendermint_rpc::endpoint::abci_query::AbciQuery { - value: astria_core::protocol::asset::v1alpha1::AllowedFeeAssetIdsResponse { - fee_asset_ids, + value: astria_core::protocol::asset::v1alpha1::AllowedFeeAssetsResponse { + fee_assets, height: 1, } .into_raw() @@ -481,7 +481,7 @@ async fn register_allowed_fee_asset_ids_response( }; let wrapper = response::Wrapper::new_with_id(tendermint_rpc::Id::Num(1), Some(response), None); Mock::given(body_partial_json(json!({"method": "abci_query"}))) - .and(body_string_contains("asset/allowed_fee_asset_ids")) + .and(body_string_contains("asset/allowed_fee_assets")) .respond_with( ResponseTemplate::new(200) .set_body_json(&wrapper) diff --git a/crates/astria-bridge-withdrawer/src/config.rs b/crates/astria-bridge-withdrawer/src/config.rs index abd458d5b9..ad9ed3d1ad 100644 --- a/crates/astria-bridge-withdrawer/src/config.rs +++ b/crates/astria-bridge-withdrawer/src/config.rs @@ -1,3 +1,4 @@ +use astria_core::primitive::v1::asset; use serde::{ Deserialize, Serialize, @@ -16,7 +17,7 @@ pub struct Config { // The path to the private key used to sign transactions submitted to the sequencer. pub sequencer_key_path: String, // The fee asset denomination to use for the bridge account's transactions. - pub fee_asset_denomination: String, + pub fee_asset_denomination: asset::Denom, // The minimum expected balance of the fee asset in the bridge account. pub min_expected_fee_asset_balance: u64, // The asset denomination being withdrawn from the rollup. diff --git a/crates/astria-cli/src/cli/sequencer.rs b/crates/astria-cli/src/cli/sequencer.rs index f4e2d79f99..68faa04db3 100644 --- a/crates/astria-cli/src/cli/sequencer.rs +++ b/crates/astria-cli/src/cli/sequencer.rs @@ -1,3 +1,4 @@ +use astria_core::primitive::v1::asset; use astria_sequencer_client::Address; use clap::{ Args, @@ -145,6 +146,12 @@ pub struct TransferArgs { default_value = crate::cli::DEFAULT_SEQUENCER_CHAIN_ID )] pub sequencer_chain_id: String, + /// The asset to transer. + #[arg(long, default_value = "nria")] + pub asset: asset::Denom, + /// The asset to pay the transfer fees with. + #[arg(long, default_value = "nria")] + pub fee_asset: asset::Denom, } #[derive(Args, Debug)] @@ -174,7 +181,7 @@ pub struct FeeAssetChangeArgs { pub sequencer_chain_id: String, /// Asset's denomination string #[arg(long)] - pub(crate) asset: String, + pub(crate) asset: asset::Denom, } #[derive(Args, Debug)] @@ -236,6 +243,12 @@ pub struct InitBridgeAccountArgs { /// to initialize the bridge account with. #[arg(long)] pub(crate) rollup_name: String, + /// The asset to transer. + #[arg(long, default_value = "nria")] + pub asset: asset::Denom, + /// The asset to pay the transfer fees with. + #[arg(long, default_value = "nria")] + pub fee_asset: asset::Denom, } #[derive(Args, Debug)] @@ -270,6 +283,12 @@ pub struct BridgeLockArgs { default_value = crate::cli::DEFAULT_SEQUENCER_CHAIN_ID )] pub sequencer_chain_id: String, + /// The asset to lock. + #[arg(long, default_value = "nria")] + pub asset: asset::Denom, + /// The asset to pay the transfer fees with. + #[arg(long, default_value = "nria")] + pub fee_asset: asset::Denom, } #[derive(Debug, Subcommand)] diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index c7b6523b2b..feacf4cd28 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -1,10 +1,6 @@ use astria_core::{ crypto::SigningKey, primitive::v1::{ - asset::{ - self, - default_native_asset, - }, Address, ADDRESS_LEN, }, @@ -111,7 +107,6 @@ pub(crate) async fn get_balance(args: &BasicAccountArgs) -> eyre::Result<()> { println!("Balances for address: {}", args.address); for balance in res.balances { - println!(" asset ID: {}", balance.denom.id()); println!(" {} {}", balance.balance, balance.denom); } @@ -202,8 +197,8 @@ pub(crate) async fn send_transfer(args: &TransferArgs) -> eyre::Result<()> { Action::Transfer(TransferAction { to: args.to_address, amount: args.amount, - asset_id: default_native_asset().id(), - fee_asset_id: default_native_asset().id(), + asset: args.asset.clone(), + fee_asset: args.fee_asset.clone(), }), ) .await @@ -287,8 +282,8 @@ pub(crate) async fn init_bridge_account(args: &InitBridgeAccountArgs) -> eyre::R args.private_key.as_str(), Action::InitBridgeAccount(InitBridgeAccountAction { rollup_id, - asset_id: default_native_asset().id(), - fee_asset_id: default_native_asset().id(), + asset: args.asset.clone(), + fee_asset: args.fee_asset.clone(), sudo_address: None, withdrawer_address: None, }), @@ -321,9 +316,9 @@ pub(crate) async fn bridge_lock(args: &BridgeLockArgs) -> eyre::Result<()> { args.private_key.as_str(), Action::BridgeLock(BridgeLockAction { to: args.to_address, - asset_id: default_native_asset().id(), + asset: args.asset.clone(), amount: args.amount, - fee_asset_id: default_native_asset().id(), + fee_asset: args.fee_asset.clone(), destination_chain_address: args.destination_chain_address.clone(), }), ) @@ -351,9 +346,7 @@ pub(crate) async fn fee_asset_add(args: &FeeAssetChangeArgs) -> eyre::Result<()> args.sequencer_chain_id.clone(), &args.prefix, args.private_key.as_str(), - Action::FeeAssetChange(FeeAssetChangeAction::Addition( - asset::Id::from_str_unchecked(&args.asset), - )), + Action::FeeAssetChange(FeeAssetChangeAction::Addition(args.asset.clone())), ) .await .wrap_err("failed to submit FeeAssetChangeAction::Addition transaction")?; @@ -379,9 +372,7 @@ pub(crate) async fn fee_asset_remove(args: &FeeAssetChangeArgs) -> eyre::Result< args.sequencer_chain_id.clone(), &args.prefix, args.private_key.as_str(), - Action::FeeAssetChange(FeeAssetChangeAction::Removal( - asset::Id::from_str_unchecked(&args.asset), - )), + Action::FeeAssetChange(FeeAssetChangeAction::Removal(args.asset.clone())), ) .await .wrap_err("failed to submit FeeAssetChangeAction::Removal transaction")?; diff --git a/crates/astria-composer/local.env.example b/crates/astria-composer/local.env.example index c746153556..8936089de1 100644 --- a/crates/astria-composer/local.env.example +++ b/crates/astria-composer/local.env.example @@ -69,6 +69,9 @@ ASTRIA_COMPOSER_METRICS_HTTP_LISTENER_ADDR="127.0.0.1:9000" # The address at which the gRPC collector and health services are listening. ASTRIA_COMPOSER_GRPC_ADDR="0.0.0.0:0" +# The asset to use for paying for transactions submitted to sequencer. +ASTRIA_COMPOSER_FEE_ASSET="nria" + # The OTEL specific config options follow the OpenTelemetry Protocol Exporter v1 # specification as defined here: # https://github.com/open-telemetry/opentelemetry-specification/blob/e94af89e3d0c01de30127a0f423e912f6cda7bed/specification/protocol/exporter.md diff --git a/crates/astria-composer/src/collectors/geth.rs b/crates/astria-composer/src/collectors/geth.rs index 0256822236..ae9759baa2 100644 --- a/crates/astria-composer/src/collectors/geth.rs +++ b/crates/astria-composer/src/collectors/geth.rs @@ -17,7 +17,7 @@ use std::time::Duration; use astria_core::{ primitive::v1::{ - asset::default_native_asset, + asset, RollupId, }, protocol::transaction::v1alpha1::action::SequenceAction, @@ -81,6 +81,7 @@ pub(crate) struct Geth { // Token to signal the geth collector to stop upon shutdown. shutdown_token: CancellationToken, metrics: &'static Metrics, + fee_asset: asset::Denom, } #[derive(Debug)] @@ -106,6 +107,7 @@ pub(crate) struct Builder { pub(crate) executor_handle: executor::Handle, pub(crate) shutdown_token: CancellationToken, pub(crate) metrics: &'static Metrics, + pub(crate) fee_asset: asset::Denom, } impl Builder { @@ -116,6 +118,7 @@ impl Builder { executor_handle, shutdown_token, metrics, + fee_asset, } = self; let (status, _) = watch::channel(Status::new()); let rollup_id = RollupId::from_unhashed_bytes(&chain_name); @@ -132,6 +135,7 @@ impl Builder { url, shutdown_token, metrics, + fee_asset, } } } @@ -159,6 +163,7 @@ impl Geth { shutdown_token, chain_name, metrics, + fee_asset, } = self; let txs_received_counter = metrics @@ -232,7 +237,7 @@ impl Geth { let seq_action = SequenceAction { rollup_id, data, - fee_asset_id: default_native_asset().id(), + fee_asset: fee_asset.clone(), }; txs_received_counter.increment(1); diff --git a/crates/astria-composer/src/collectors/grpc.rs b/crates/astria-composer/src/collectors/grpc.rs index 260520ab52..091cca23b2 100644 --- a/crates/astria-composer/src/collectors/grpc.rs +++ b/crates/astria-composer/src/collectors/grpc.rs @@ -9,7 +9,7 @@ use astria_core::{ SubmitRollupTransactionResponse, }, primitive::v1::{ - asset::default_native_asset, + asset, RollupId, }, protocol::transaction::v1alpha1::action::SequenceAction, @@ -33,13 +33,19 @@ use crate::{ pub(crate) struct Grpc { executor: executor::Handle, metrics: &'static Metrics, + fee_asset: asset::Denom, } impl Grpc { - pub(crate) fn new(executor: executor::Handle, metrics: &'static Metrics) -> Self { + pub(crate) fn new( + executor: executor::Handle, + metrics: &'static Metrics, + fee_asset: asset::Denom, + ) -> Self { Self { executor, metrics, + fee_asset, } } } @@ -59,7 +65,7 @@ impl GrpcCollectorService for Grpc { let sequence_action = SequenceAction { rollup_id, data: submit_rollup_tx_request.data, - fee_asset_id: default_native_asset().id(), + fee_asset: self.fee_asset.clone(), }; self.metrics.increment_grpc_txs_received(&rollup_id); diff --git a/crates/astria-composer/src/composer.rs b/crates/astria-composer/src/composer.rs index cbdedac5c3..85eb285303 100644 --- a/crates/astria-composer/src/composer.rs +++ b/crates/astria-composer/src/composer.rs @@ -5,6 +5,7 @@ use std::{ time::Duration, }; +use astria_core::primitive::v1::asset; use astria_eyre::eyre::{ self, WrapErr as _, @@ -85,6 +86,8 @@ pub struct Composer { /// Used to signal the Composer to shut down. shutdown_token: CancellationToken, metrics: &'static Metrics, + /// The asset set in config to pay for transactions and sequence actions. + fee_asset: asset::Denom, } /// Announces the current status of the Composer for other modules in the crate to use @@ -143,6 +146,7 @@ impl Composer { executor: executor_handle.clone(), shutdown_token: shutdown_token.clone(), metrics, + fee_asset: cfg.fee_asset.clone(), } .build() .await @@ -169,6 +173,7 @@ impl Composer { executor_handle: executor_handle.clone(), shutdown_token: shutdown_token.clone(), metrics, + fee_asset: cfg.fee_asset.clone(), } .build(); (rollup_name.clone(), collector) @@ -192,6 +197,7 @@ impl Composer { grpc_server, shutdown_token, metrics, + fee_asset: cfg.fee_asset.clone(), }) } @@ -230,6 +236,7 @@ impl Composer { grpc_server, shutdown_token, metrics, + fee_asset, } = self; // we need the API server to shutdown at the end, since it is used by k8s @@ -327,6 +334,7 @@ impl Composer { executor_handle: executor_handle.clone(), shutdown_token: shutdown_token.clone(), metrics, + fee_asset: fee_asset.clone(), } .build(); geth_collector_statuses.insert(rollup.clone(), collector.subscribe()); diff --git a/crates/astria-composer/src/config.rs b/crates/astria-composer/src/config.rs index 1cbf56ac6c..0816f723a1 100644 --- a/crates/astria-composer/src/config.rs +++ b/crates/astria-composer/src/config.rs @@ -65,6 +65,9 @@ pub struct Config { /// The address at which the gRPC server is listening pub grpc_addr: SocketAddr, + + /// The IBC asset to pay for transactions submiited to the sequencer. + pub fee_asset: astria_core::primitive::v1::asset::Denom, } impl Config { diff --git a/crates/astria-composer/src/executor/bundle_factory/mod.rs b/crates/astria-composer/src/executor/bundle_factory/mod.rs index 8a2644b77c..d3c3696d82 100644 --- a/crates/astria-composer/src/executor/bundle_factory/mod.rs +++ b/crates/astria-composer/src/executor/bundle_factory/mod.rs @@ -1,5 +1,5 @@ -/// ! This module is responsible for bundling sequence actions into bundles that can be -/// submitted to the sequencer. +//! This module is responsible for bundling sequence actions into bundles that can be +//! submitted to the sequencer. use std::{ collections::{ HashMap, @@ -9,11 +9,7 @@ use std::{ }; use astria_core::{ - primitive::v1::{ - RollupId, - FEE_ASSET_ID_LEN, - ROLLUP_ID_LEN, - }, + primitive::v1::RollupId, protocol::transaction::v1alpha1::{ action::SequenceAction, Action, @@ -25,6 +21,7 @@ use serde::ser::{ }; use tracing::trace; +#[cfg(test)] mod tests; #[derive(Debug, thiserror::Error)] @@ -81,7 +78,7 @@ impl SizedBundle { /// - `seq_action` is beyond the max size allowed for the entire bundle /// - `seq_action` does not fit in the remaining space in the bundle fn try_push(&mut self, seq_action: SequenceAction) -> Result<(), SizedBundleError> { - let seq_action_size = estimate_size_of_sequence_action(&seq_action); + let seq_action_size = encoded_len(&seq_action); if seq_action_size > self.max_size { return Err(SizedBundleError::SequenceActionTooLarge(seq_action)); @@ -133,17 +130,27 @@ impl SizedBundle { pub(super) enum BundleFactoryError { #[error("sequence action is larger than the max bundle size. seq_action size: {size}")] SequenceActionTooLarge { size: usize, max_size: usize }, - #[error( - "finished bundle queue is at capacity and the sequence action does not fit in the current \ - bundle. finished queue capacity: {finished_queue_capacity}, curr bundle size: \ - {curr_bundle_size}, sequence action size: {sequence_action_size}" - )] - FinishedQueueFull { - curr_bundle_size: usize, - finished_queue_capacity: usize, - sequence_action_size: usize, - seq_action: SequenceAction, - }, + #[error(transparent)] + FinishedQueueFull(Box), +} + +#[derive(Debug, thiserror::Error)] +#[error( + "finished bundle queue is at capacity and the sequence action does not fit in the current \ + bundle. finished queue capacity: {finished_queue_capacity}, curr bundle size: \ + {curr_bundle_size}, sequence action size: {sequence_action_size}" +)] +pub(super) struct FinishedQueueFull { + curr_bundle_size: usize, + finished_queue_capacity: usize, + sequence_action_size: usize, + seq_action: SequenceAction, +} + +impl From for BundleFactoryError { + fn from(value: FinishedQueueFull) -> Self { + Self::FinishedQueueFull(Box::new(value)) + } } /// Manages the bundling of sequence actions into `SizedBundle`s. A `Vec` is flushed and @@ -175,7 +182,8 @@ impl BundleFactory { &mut self, seq_action: SequenceAction, ) -> Result<(), BundleFactoryError> { - let seq_action_size = estimate_size_of_sequence_action(&seq_action); + let seq_action = with_ibc_prefixed(seq_action); + let seq_action_size = encoded_len(&seq_action); match self.curr_bundle.try_push(seq_action) { Err(SizedBundleError::SequenceActionTooLarge(_seq_action)) => { @@ -187,12 +195,13 @@ impl BundleFactory { } Err(SizedBundleError::NotEnoughSpace(seq_action)) => { if self.finished.len() >= self.finished_queue_capacity { - Err(BundleFactoryError::FinishedQueueFull { + Err(FinishedQueueFull { curr_bundle_size: self.curr_bundle.curr_size, finished_queue_capacity: self.finished_queue_capacity, sequence_action_size: seq_action_size, seq_action, - }) + } + .into()) } else { // if the bundle is full, flush it and start a new one self.finished.push_back(self.curr_bundle.flush()); @@ -262,11 +271,14 @@ impl<'a> NextFinishedBundle<'a> { } } -/// The size of the `seq_action` in bytes, including the rollup id. -fn estimate_size_of_sequence_action(seq_action: &SequenceAction) -> usize { - seq_action - .data - .len() - .saturating_add(ROLLUP_ID_LEN) - .saturating_add(FEE_ASSET_ID_LEN) +fn with_ibc_prefixed(action: SequenceAction) -> SequenceAction { + SequenceAction { + fee_asset: action.fee_asset.to_ibc_prefixed().into(), + ..action + } +} + +fn encoded_len(action: &SequenceAction) -> usize { + use prost::Message as _; + action.to_raw().encoded_len() } diff --git a/crates/astria-composer/src/executor/bundle_factory/snapshots/astria_composer__executor__bundle_factory__tests__sized_bundle_tests__snapshots.snap b/crates/astria-composer/src/executor/bundle_factory/snapshots/astria_composer__executor__bundle_factory__tests__sized_bundle_tests__snapshots.snap deleted file mode 100644 index 73fbb74b34..0000000000 --- a/crates/astria-composer/src/executor/bundle_factory/snapshots/astria_composer__executor__bundle_factory__tests__sized_bundle_tests__snapshots.snap +++ /dev/null @@ -1,8 +0,0 @@ ---- -source: crates/astria-composer/src/executor/bundle_factory/tests.rs -expression: bundle.rollup_counts ---- -{ - "AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE=": 2, - "AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgI=": 1 -} diff --git a/crates/astria-composer/src/executor/bundle_factory/tests.rs b/crates/astria-composer/src/executor/bundle_factory/tests.rs index 35d2f412af..dd8af22cf7 100644 --- a/crates/astria-composer/src/executor/bundle_factory/tests.rs +++ b/crates/astria-composer/src/executor/bundle_factory/tests.rs @@ -1,55 +1,40 @@ -#[cfg(test)] -mod sized_bundle_tests { - use astria_core::{ - primitive::v1::{ - asset::default_native_asset, - RollupId, - FEE_ASSET_ID_LEN, - ROLLUP_ID_LEN, +use astria_core::{ + primitive::v1::{ + RollupId, + ROLLUP_ID_LEN, + }, + protocol::transaction::v1alpha1::action::SequenceAction, +}; + +mod sized_bundle { + use crate::{ + executor::bundle_factory::{ + SizedBundle, + SizedBundleError, + }, + test_utils::{ + empty_sequence_action, + sequence_action_of_max_size, + sequence_action_with_n_bytes, }, - protocol::transaction::v1alpha1::action::SequenceAction, - }; - use insta::{ - assert_json_snapshot, - Settings, - }; - - use crate::executor::bundle_factory::{ - estimate_size_of_sequence_action, - SizedBundle, - SizedBundleError, }; #[test] fn push_works() { - // create a bundle with 100 bytes of max space - let mut bundle = SizedBundle::new(100); - - // push a sequence action that is 100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; - let seq_action_size = estimate_size_of_sequence_action(&seq_action); + let mut bundle = SizedBundle::new(200); - assert_eq!(seq_action_size, 100); + let seq_action = sequence_action_of_max_size(200); bundle.try_push(seq_action).unwrap(); } #[test] fn push_seq_action_too_large() { // create a bundle with 100 bytes of max space - let mut bundle = SizedBundle::new(100); + let mut bundle = SizedBundle::new(200); - // push a sequence action that is >100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN + 1], - fee_asset_id: default_native_asset().id(), - }; - - assert!(estimate_size_of_sequence_action(&seq_action) > 100); + // push an action with > 200 bytes. the proto encoding will guarantee + // to take us over 200. + let seq_action = sequence_action_with_n_bytes(200); assert!(matches!( bundle.try_push(seq_action), Err(SizedBundleError::SequenceActionTooLarge(_)) @@ -59,43 +44,26 @@ mod sized_bundle_tests { #[test] fn push_not_enough_space() { // create a bundle with 100 bytes of max space - let mut bundle = SizedBundle::new(100); + let mut bundle = SizedBundle::new(200); // push a sequence action that is 100 bytes total - let initial_seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; - bundle.try_push(initial_seq_action).unwrap(); - - // push another sequence action that won't fit as the bundle is full but is less than max - // size - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 1], - fee_asset_id: default_native_asset().id(), - }; + bundle.try_push(sequence_action_of_max_size(200)).unwrap(); - assert!(estimate_size_of_sequence_action(&seq_action) < 100); assert!(matches!( - bundle.try_push(seq_action.clone()), + bundle.try_push(empty_sequence_action()), Err(SizedBundleError::NotEnoughSpace(actual_seq_action)) - if actual_seq_action.rollup_id == seq_action.rollup_id && actual_seq_action.data == seq_action.data + if actual_seq_action.rollup_id == empty_sequence_action().rollup_id + && actual_seq_action.data == empty_sequence_action().data )); } #[test] fn flush_sanity_check() { // create a bundle with 100 bytes of max space - let mut bundle = SizedBundle::new(100); + let mut bundle = SizedBundle::new(200); // push a sequence action successfully - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![1; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle.try_push(seq_action.clone()).unwrap(); // flush the bundle @@ -111,72 +79,28 @@ mod sized_bundle_tests { assert_eq!(actual_seq_action.rollup_id, seq_action.rollup_id); assert_eq!(actual_seq_action.data, seq_action.data); } - - fn snapshot_bundle() -> SizedBundle { - let mut bundle = SizedBundle::new(264); - let seq_action1 = SequenceAction { - rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1; 50 - ROLLUP_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; - let seq_action1_2 = SequenceAction { - rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1; 50 - ROLLUP_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; - let seq_action2 = SequenceAction { - rollup_id: RollupId::new([2; ROLLUP_ID_LEN]), - data: vec![2; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; - bundle.try_push(seq_action1).unwrap(); - bundle.try_push(seq_action1_2).unwrap(); - bundle.try_push(seq_action2).unwrap(); - bundle - } - - #[test] - fn snapshots() { - let bundle = snapshot_bundle(); - - let mut settings = Settings::new(); - settings.set_sort_maps(true); - - settings.bind(|| { - assert_json_snapshot!(bundle.rollup_counts); - }); - } } #[cfg(test)] -mod bundle_factory_tests { - use astria_core::{ - primitive::v1::{ - asset::default_native_asset, - RollupId, - FEE_ASSET_ID_LEN, - ROLLUP_ID_LEN, +mod bundle_factory { + use super::*; + use crate::{ + executor::bundle_factory::{ + BundleFactory, + BundleFactoryError, + }, + test_utils::{ + sequence_action_of_max_size, + sequence_action_with_n_bytes, }, - protocol::transaction::v1alpha1::action::SequenceAction, - }; - - use crate::executor::bundle_factory::{ - estimate_size_of_sequence_action, - BundleFactory, - BundleFactoryError, }; #[test] fn try_push_works_no_flush() { // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is 100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action).unwrap(); // assert that the bundle factory has no bundles in the finished queue @@ -185,46 +109,28 @@ mod bundle_factory_tests { #[test] fn try_push_seq_action_too_large() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is >100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN + 1], - fee_asset_id: default_native_asset().id(), - }; - let actual_size = estimate_size_of_sequence_action(&seq_action); + // push an action with > 200 bytes. the proto encoding will guarantee + // to take us over 200. + let seq_action = sequence_action_with_n_bytes(200); assert!(matches!( bundle_factory.try_push(seq_action), - Err(BundleFactoryError::SequenceActionTooLarge { - size, - max_size - }) if size == actual_size && max_size == 100 + Err(BundleFactoryError::SequenceActionTooLarge { .. }) )); } #[test] fn try_push_flushes_and_pop_finished_works() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is 100 bytes total - let seq_action0 = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action0 = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action0.clone()).unwrap(); // push another sequence action that is <100 bytes total to force the current bundle to // flush - let seq_action1 = SequenceAction { - rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action1 = sequence_action_of_max_size(150); bundle_factory.try_push(seq_action1).unwrap(); // assert that the bundle factory has one bundle in the finished queue @@ -239,15 +145,10 @@ mod bundle_factory_tests { #[test] fn try_push_full_sanity_check() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 1); + let mut bundle_factory = BundleFactory::new(200, 1); // push a sequence action that is 100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action.clone()).unwrap(); // push another sequence action that is <100 bytes total to force the current bundle to @@ -257,34 +158,27 @@ mod bundle_factory_tests { // try to push a third bundle that wouldn't fit in `curr_bundle`, forcing the factory to // flush it into `finished` this shouldn't work since the `finished` queue's // capacity is 1. - let full_err = bundle_factory.try_push(seq_action.clone()); + let err = bundle_factory + .try_push(seq_action.clone()) + .expect_err("the action should be rejected"); // assert that the bundle factory has one bundle in the finished queue, that the factory is // full and that err was returned - assert!(matches!( - full_err, - Err(BundleFactoryError::FinishedQueueFull { - curr_bundle_size: _, - finished_queue_capacity: _, - sequence_action_size: _, - seq_action: _ - }) - )); + // allow: this is intended to match all possible variants + #[allow(clippy::match_wildcard_for_single_variants)] + match err { + BundleFactoryError::FinishedQueueFull(_) => {} + other => panic!("expected a FinishedQueueFull variant, but got {other:?}"), + } assert_eq!(bundle_factory.finished.len(), 1); assert!(bundle_factory.is_full()); } #[test] fn pop_finished_empty() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is 100 bytes total so it doesn't flush - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action.clone()).unwrap(); // assert that the finished queue is empty @@ -296,15 +190,9 @@ mod bundle_factory_tests { #[test] fn pop_finished_no_longer_full() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 1); + let mut bundle_factory = BundleFactory::new(200, 1); - // push a sequence action that is 100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action.clone()).unwrap(); // push another sequence action to force the current bundle to flush @@ -315,22 +203,20 @@ mod bundle_factory_tests { // capacity is 1. let seq_action1 = SequenceAction { rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), + ..sequence_action_of_max_size(200) }; - let full_err = bundle_factory.try_push(seq_action1.clone()); + let err = bundle_factory + .try_push(seq_action1.clone()) + .expect_err("the action should have been rejected"); // assert that the bundle factory has one bundle in the finished queue, that the factory is // full and that err was returned - assert!(matches!( - full_err, - Err(BundleFactoryError::FinishedQueueFull { - curr_bundle_size: _, - finished_queue_capacity: _, - sequence_action_size: _, - seq_action: _ - }) - )); + // allow: this is intended to match all possible variants + #[allow(clippy::match_wildcard_for_single_variants)] + match err { + BundleFactoryError::FinishedQueueFull(_) => {} + other => panic!("expected a FinishedQueueFull variant, but got {other:?}"), + } assert_eq!(bundle_factory.finished.len(), 1); assert!(bundle_factory.is_full()); @@ -342,15 +228,9 @@ mod bundle_factory_tests { #[test] fn pop_now_finished_empty() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is 100 bytes total so it doesn't flush - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action.clone()).unwrap(); // assert that the finished queue is empty (curr wasn't flushed) @@ -365,22 +245,16 @@ mod bundle_factory_tests { #[test] fn pop_now_finished_not_empty() { // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is 100 bytes total - let seq_action0 = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action0 = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action0.clone()).unwrap(); // push another sequence action that is <100 bytes total to force the current bundle to // flush let seq_action1 = SequenceAction { rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), + ..sequence_action_of_max_size(200) }; bundle_factory.try_push(seq_action1).unwrap(); @@ -407,23 +281,14 @@ mod bundle_factory_tests { #[test] fn pop_now_finished_then_curr_then_empty() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 10); + let mut bundle_factory = BundleFactory::new(200, 10); - // push a sequence action that is 100 bytes total - let seq_action0 = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action0 = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action0.clone()).unwrap(); - // push another sequence action that is <100 bytes total to force the current bundle to - // flush let seq_action1 = SequenceAction { rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), + ..sequence_action_of_max_size(200) }; bundle_factory.try_push(seq_action1.clone()).unwrap(); @@ -454,15 +319,10 @@ mod bundle_factory_tests { #[test] fn pop_now_full() { - // create a bundle factory with max bundle size as 100 bytes - let mut bundle_factory = BundleFactory::new(100, 1); + let mut bundle_factory = BundleFactory::new(200, 1); // push a sequence action that is 100 bytes total - let seq_action = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0; 100 - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq_action = sequence_action_of_max_size(200); bundle_factory.try_push(seq_action.clone()).unwrap(); // push another sequence action that is to force the current bundle to flush diff --git a/crates/astria-composer/src/executor/tests.rs b/crates/astria-composer/src/executor/tests.rs index 07482610c3..4dd4f15c6a 100644 --- a/crates/astria-composer/src/executor/tests.rs +++ b/crates/astria-composer/src/executor/tests.rs @@ -5,9 +5,7 @@ use std::{ use astria_core::{ primitive::v1::{ - asset::default_native_asset, RollupId, - FEE_ASSET_ID_LEN, ROLLUP_ID_LEN, }, protocol::transaction::v1alpha1::action::SequenceAction, @@ -45,6 +43,7 @@ use wiremock::{ use crate::{ executor, metrics::Metrics, + test_utils::sequence_action_of_max_size, Config, }; @@ -66,6 +65,14 @@ static TELEMETRY: Lazy<()> = Lazy::new(|| { } }); +fn sequence_action() -> SequenceAction { + SequenceAction { + rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), + data: vec![], + fee_asset: "nria".parse().unwrap(), + } +} + /// Start a mock sequencer server and mount a mock for the `accounts/nonce` query. async fn setup() -> (MockServer, MockGuard, Config, NamedTempFile) { use astria_core::generated::protocol::account::v1alpha1::NonceResponse; @@ -103,6 +110,7 @@ async fn setup() -> (MockServer, MockGuard, Config, NamedTempFile) { metrics_http_listener_addr: String::new(), pretty_print: true, grpc_addr: "127.0.0.1:0".parse().unwrap(), + fee_asset: "nria".parse().unwrap(), }; (server, startup_guard, cfg, keyfile) } @@ -236,16 +244,11 @@ async fn full_bundle() { // send two sequence actions to the executor, the first of which is large enough to fill the // bundle sending the second should cause the first to immediately be submitted in // order to make space for the second - let seq0 = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), - data: vec![0u8; cfg.max_bytes_per_bundle - ROLLUP_ID_LEN - FEE_ASSET_ID_LEN], - fee_asset_id: default_native_asset().id(), - }; + let seq0 = sequence_action_of_max_size(cfg.max_bytes_per_bundle); let seq1 = SequenceAction { rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), - data: vec![1u8; 1], - fee_asset_id: default_native_asset().id(), + ..sequence_action_of_max_size(cfg.max_bytes_per_bundle) }; // push both sequence actions to the executor in order to force the full bundle to be sent @@ -331,9 +334,8 @@ async fn bundle_triggered_by_block_timer() { // send two sequence actions to the executor, both small enough to fit in a single bundle // without filling it let seq0 = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), data: vec![0u8; cfg.max_bytes_per_bundle / 4], - fee_asset_id: default_native_asset().id(), + ..sequence_action() }; // make sure at least one block has passed so that the executor will submit the bundle @@ -418,15 +420,14 @@ async fn two_seq_actions_single_bundle() { // send two sequence actions to the executor, both small enough to fit in a single bundle // without filling it let seq0 = SequenceAction { - rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), data: vec![0u8; cfg.max_bytes_per_bundle / 4], - fee_asset_id: default_native_asset().id(), + ..sequence_action() }; let seq1 = SequenceAction { rollup_id: RollupId::new([1; ROLLUP_ID_LEN]), data: vec![1u8; cfg.max_bytes_per_bundle / 4], - fee_asset_id: default_native_asset().id(), + ..sequence_action() }; // make sure at least one block has passed so that the executor will submit the bundle diff --git a/crates/astria-composer/src/grpc.rs b/crates/astria-composer/src/grpc.rs index 478537b876..331d533d74 100644 --- a/crates/astria-composer/src/grpc.rs +++ b/crates/astria-composer/src/grpc.rs @@ -8,7 +8,10 @@ use std::net::SocketAddr; -use astria_core::generated::composer::v1alpha1::grpc_collector_service_server::GrpcCollectorServiceServer; +use astria_core::{ + generated::composer::v1alpha1::grpc_collector_service_server::GrpcCollectorServiceServer, + primitive::v1::asset, +}; use astria_eyre::{ eyre, eyre::WrapErr as _, @@ -40,6 +43,7 @@ pub(crate) struct Builder { pub(crate) executor: executor::Handle, pub(crate) shutdown_token: CancellationToken, pub(crate) metrics: &'static Metrics, + pub(crate) fee_asset: asset::Denom, } impl Builder { @@ -49,12 +53,13 @@ impl Builder { executor, shutdown_token, metrics, + fee_asset, } = self; let listener = TcpListener::bind(grpc_addr) .await .wrap_err("failed to bind socket address")?; - let grpc_collector = collectors::Grpc::new(executor.clone(), metrics); + let grpc_collector = collectors::Grpc::new(executor.clone(), metrics, fee_asset); Ok(GrpcServer { listener, diff --git a/crates/astria-composer/src/lib.rs b/crates/astria-composer/src/lib.rs index 881c2ebc96..f61f464965 100644 --- a/crates/astria-composer/src/lib.rs +++ b/crates/astria-composer/src/lib.rs @@ -47,6 +47,8 @@ mod executor; mod grpc; pub(crate) mod metrics; mod rollup; +#[cfg(test)] +pub(crate) mod test_utils; pub use build_info::BUILD_INFO; pub use composer::Composer; diff --git a/crates/astria-composer/src/test_utils.rs b/crates/astria-composer/src/test_utils.rs new file mode 100644 index 0000000000..a12f924fe2 --- /dev/null +++ b/crates/astria-composer/src/test_utils.rs @@ -0,0 +1,40 @@ +use astria_core::{ + primitive::v1::{ + asset, + RollupId, + ROLLUP_ID_LEN, + }, + protocol::transaction::v1alpha1::action::SequenceAction, +}; + +fn encoded_len(action: &SequenceAction) -> usize { + use prost::Message as _; + action.to_raw().encoded_len() +} + +pub(crate) fn sequence_action_with_n_bytes(n: usize) -> SequenceAction { + SequenceAction { + rollup_id: RollupId::new([0; ROLLUP_ID_LEN]), + data: vec![0; n], + fee_asset: "nria" + .parse::() + .unwrap() + .to_ibc_prefixed() + .into(), + } +} + +pub(crate) fn empty_sequence_action() -> SequenceAction { + sequence_action_with_n_bytes(0) +} + +pub(crate) fn sequence_action_of_max_size(max: usize) -> SequenceAction { + // an action where the data part is exactly max bytes long + let big_action = sequence_action_with_n_bytes(max); + // the number of bytes past max + let excess = encoded_len(&big_action).saturating_sub(max); + // an action with just so many bytes that the encoded len is =< max + // note that this does not guarantee == max since the len part of + // len-delimited fields is var-int encoded. + sequence_action_with_n_bytes(max.saturating_sub(excess)) +} diff --git a/crates/astria-composer/tests/blackbox/helper/mod.rs b/crates/astria-composer/tests/blackbox/helper/mod.rs index 82940aa3be..6b287a29b0 100644 --- a/crates/astria-composer/tests/blackbox/helper/mod.rs +++ b/crates/astria-composer/tests/blackbox/helper/mod.rs @@ -105,6 +105,7 @@ pub async fn spawn_composer(rollup_ids: &[&str]) -> TestComposer { metrics_http_listener_addr: String::new(), pretty_print: true, grpc_addr: "127.0.0.1:0".parse().unwrap(), + fee_asset: "nria".parse().unwrap(), }; let (composer_addr, grpc_collector_addr, composer_handle) = { let composer = Composer::from_config(&config).await.unwrap(); diff --git a/crates/astria-core/src/generated/astria.protocol.asset.v1alpha1.rs b/crates/astria-core/src/generated/astria.protocol.asset.v1alpha1.rs index a8668c79cc..6aaf4934a1 100644 --- a/crates/astria-core/src/generated/astria.protocol.asset.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.asset.v1alpha1.rs @@ -14,17 +14,17 @@ impl ::prost::Name for DenomResponse { ::prost::alloc::format!("astria.protocol.asset.v1alpha1.{}", Self::NAME) } } -/// A response containing the allowed fee asset IDs. +/// A response containing the allowed fee assets. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] -pub struct AllowedFeeAssetIdsResponse { +pub struct AllowedFeeAssetsResponse { #[prost(uint64, tag = "1")] pub height: u64, - #[prost(bytes = "bytes", repeated, tag = "2")] - pub fee_asset_ids: ::prost::alloc::vec::Vec<::prost::bytes::Bytes>, + #[prost(string, repeated, tag = "2")] + pub fee_assets: ::prost::alloc::vec::Vec<::prost::alloc::string::String>, } -impl ::prost::Name for AllowedFeeAssetIdsResponse { - const NAME: &'static str = "AllowedFeeAssetIdsResponse"; +impl ::prost::Name for AllowedFeeAssetsResponse { + const NAME: &'static str = "AllowedFeeAssetsResponse"; const PACKAGE: &'static str = "astria.protocol.asset.v1alpha1"; fn full_name() -> ::prost::alloc::string::String { ::prost::alloc::format!("astria.protocol.asset.v1alpha1.{}", Self::NAME) diff --git a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs index 08ef796d71..de203206ed 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs @@ -119,11 +119,11 @@ pub struct TransferAction { #[prost(message, optional, tag = "2")] pub amount: ::core::option::Option, /// the asset to be transferred - #[prost(bytes = "vec", tag = "3")] - pub asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "3")] + pub asset: ::prost::alloc::string::String, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "4")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "4")] + pub fee_asset: ::prost::alloc::string::String, } impl ::prost::Name for TransferAction { const NAME: &'static str = "TransferAction"; @@ -145,8 +145,8 @@ pub struct SequenceAction { #[prost(bytes = "vec", tag = "2")] pub data: ::prost::alloc::vec::Vec, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "3")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "3")] + pub fee_asset: ::prost::alloc::string::String, } impl ::prost::Name for SequenceAction { const NAME: &'static str = "SequenceAction"; @@ -202,8 +202,8 @@ pub struct Ics20Withdrawal { #[prost(string, tag = "7")] pub source_channel: ::prost::alloc::string::String, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "8")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "8")] + pub fee_asset: ::prost::alloc::string::String, /// a memo to include with the transfer #[prost(string, tag = "9")] pub memo: ::prost::alloc::string::String, @@ -272,8 +272,6 @@ impl ::prost::Name for IbcRelayerChangeAction { } /// `FeeAssetChangeAction` represents a transaction that adds /// or removes an asset for fee payments. -/// The bytes contained in each variant are the 32-byte asset ID -/// to add or remove. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct FeeAssetChangeAction { @@ -285,10 +283,10 @@ pub mod fee_asset_change_action { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum Value { - #[prost(bytes, tag = "1")] - Addition(::prost::alloc::vec::Vec), - #[prost(bytes, tag = "2")] - Removal(::prost::alloc::vec::Vec), + #[prost(string, tag = "1")] + Addition(::prost::alloc::string::String), + #[prost(string, tag = "2")] + Removal(::prost::alloc::string::String), } } impl ::prost::Name for FeeAssetChangeAction { @@ -311,11 +309,11 @@ pub struct InitBridgeAccountAction { #[prost(message, optional, tag = "1")] pub rollup_id: ::core::option::Option, /// the asset ID accepted as an incoming transfer by the bridge account - #[prost(bytes = "vec", tag = "2")] - pub asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "2")] + pub asset: ::prost::alloc::string::String, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "3")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "3")] + pub fee_asset: ::prost::alloc::string::String, /// the address corresponding to the key which has sudo capabilities; /// ie. can change the sudo and withdrawer addresses for this bridge account. /// if this is empty, the sender of the transaction is used. @@ -352,11 +350,11 @@ pub struct BridgeLockAction { #[prost(message, optional, tag = "2")] pub amount: ::core::option::Option, /// the asset to be transferred - #[prost(bytes = "vec", tag = "3")] - pub asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "3")] + pub asset: ::prost::alloc::string::String, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "4")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "4")] + pub fee_asset: ::prost::alloc::string::String, /// the address on the destination chain which /// will receive the bridged funds #[prost(string, tag = "5")] @@ -372,7 +370,7 @@ impl ::prost::Name for BridgeLockAction { /// `BridgeUnlockAction` represents a transaction that transfers /// funds from a bridge account to a sequencer account. /// -/// It's the same as a `TransferAction` but without the `asset_id` field +/// It's the same as a `TransferAction` but without the `asset` field /// and with the `memo` field. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] @@ -384,8 +382,8 @@ pub struct BridgeUnlockAction { #[prost(message, optional, tag = "2")] pub amount: ::core::option::Option, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "3")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "3")] + pub fee_asset: ::prost::alloc::string::String, /// memo for double spend prevention #[prost(bytes = "vec", tag = "4")] pub memo: ::prost::alloc::vec::Vec, @@ -423,8 +421,8 @@ pub struct BridgeSudoChangeAction { super::super::super::primitive::v1::Address, >, /// the asset used to pay the transaction fee - #[prost(bytes = "vec", tag = "4")] - pub fee_asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "4")] + pub fee_asset: ::prost::alloc::string::String, } impl ::prost::Name for BridgeSudoChangeAction { const NAME: &'static str = "BridgeSudoChangeAction"; diff --git a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs index 0c75c8bcf9..eac5b68d69 100644 --- a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.rs @@ -120,8 +120,8 @@ pub struct Deposit { pub rollup_id: ::core::option::Option, #[prost(message, optional, tag = "3")] pub amount: ::core::option::Option, - #[prost(bytes = "vec", tag = "4")] - pub asset_id: ::prost::alloc::vec::Vec, + #[prost(string, tag = "4")] + pub asset: ::prost::alloc::string::String, /// the address on the destination chain which /// will receive the bridged funds #[prost(string, tag = "5")] diff --git a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs index acbc4c2183..4a4e811243 100644 --- a/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs +++ b/crates/astria-core/src/generated/astria.sequencerblock.v1alpha1.serde.rs @@ -15,7 +15,7 @@ impl serde::Serialize for Deposit { if self.amount.is_some() { len += 1; } - if !self.asset_id.is_empty() { + if !self.asset.is_empty() { len += 1; } if !self.destination_chain_address.is_empty() { @@ -31,9 +31,8 @@ impl serde::Serialize for Deposit { if let Some(v) = self.amount.as_ref() { struct_ser.serialize_field("amount", v)?; } - if !self.asset_id.is_empty() { - #[allow(clippy::needless_borrow)] - struct_ser.serialize_field("asset_id", pbjson::private::base64::encode(&self.asset_id).as_str())?; + if !self.asset.is_empty() { + struct_ser.serialize_field("asset", &self.asset)?; } if !self.destination_chain_address.is_empty() { struct_ser.serialize_field("destination_chain_address", &self.destination_chain_address)?; @@ -53,8 +52,7 @@ impl<'de> serde::Deserialize<'de> for Deposit { "rollup_id", "rollupId", "amount", - "asset_id", - "assetId", + "asset", "destination_chain_address", "destinationChainAddress", ]; @@ -64,7 +62,7 @@ impl<'de> serde::Deserialize<'de> for Deposit { BridgeAddress, RollupId, Amount, - AssetId, + Asset, DestinationChainAddress, } impl<'de> serde::Deserialize<'de> for GeneratedField { @@ -90,7 +88,7 @@ impl<'de> serde::Deserialize<'de> for Deposit { "bridgeAddress" | "bridge_address" => Ok(GeneratedField::BridgeAddress), "rollupId" | "rollup_id" => Ok(GeneratedField::RollupId), "amount" => Ok(GeneratedField::Amount), - "assetId" | "asset_id" => Ok(GeneratedField::AssetId), + "asset" => Ok(GeneratedField::Asset), "destinationChainAddress" | "destination_chain_address" => Ok(GeneratedField::DestinationChainAddress), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } @@ -114,7 +112,7 @@ impl<'de> serde::Deserialize<'de> for Deposit { let mut bridge_address__ = None; let mut rollup_id__ = None; let mut amount__ = None; - let mut asset_id__ = None; + let mut asset__ = None; let mut destination_chain_address__ = None; while let Some(k) = map_.next_key()? { match k { @@ -136,13 +134,11 @@ impl<'de> serde::Deserialize<'de> for Deposit { } amount__ = map_.next_value()?; } - GeneratedField::AssetId => { - if asset_id__.is_some() { - return Err(serde::de::Error::duplicate_field("assetId")); + GeneratedField::Asset => { + if asset__.is_some() { + return Err(serde::de::Error::duplicate_field("asset")); } - asset_id__ = - Some(map_.next_value::<::pbjson::private::BytesDeserialize<_>>()?.0) - ; + asset__ = Some(map_.next_value()?); } GeneratedField::DestinationChainAddress => { if destination_chain_address__.is_some() { @@ -156,7 +152,7 @@ impl<'de> serde::Deserialize<'de> for Deposit { bridge_address: bridge_address__, rollup_id: rollup_id__, amount: amount__, - asset_id: asset_id__.unwrap_or_default(), + asset: asset__.unwrap_or_default(), destination_chain_address: destination_chain_address__.unwrap_or_default(), }) } diff --git a/crates/astria-core/src/primitive/v1/asset/denom.rs b/crates/astria-core/src/primitive/v1/asset/denom.rs index 9a6e34616d..2e8380ae42 100644 --- a/crates/astria-core/src/primitive/v1/asset/denom.rs +++ b/crates/astria-core/src/primitive/v1/asset/denom.rs @@ -11,7 +11,7 @@ use std::{ /// Note that the full denomination trace of the token is `prefix/base_denom`, /// in the case that a prefix is present. /// This is hashed to create the ID. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq)] pub enum Denom { TracePrefixed(TracePrefixed), IbcPrefixed(IbcPrefixed), @@ -28,14 +28,6 @@ impl Denom { self.as_trace_prefixed().is_some() } - #[must_use] - pub fn id(&self) -> super::Id { - match self { - Self::TracePrefixed(trace) => trace.id(), - Self::IbcPrefixed(ibc) => ibc.id(), - } - } - #[must_use] pub fn as_ibc_prefixed(&self) -> Option<&IbcPrefixed> { match self { @@ -52,6 +44,14 @@ impl Denom { } } + #[must_use] + pub fn to_ibc_prefixed(&self) -> IbcPrefixed { + match self { + Denom::TracePrefixed(trace) => trace.to_ibc_prefixed(), + Denom::IbcPrefixed(ibc) => *ibc, + } + } + /// Unwraps the inner ibc prefixed denom. /// /// # Panics @@ -89,6 +89,42 @@ impl From for Denom { } } +impl<'a> From<&'a IbcPrefixed> for Denom { + fn from(value: &IbcPrefixed) -> Self { + Self::IbcPrefixed(*value) + } +} + +impl<'a> From<&'a TracePrefixed> for Denom { + fn from(value: &TracePrefixed) -> Self { + Self::TracePrefixed(value.clone()) + } +} + +impl From for IbcPrefixed { + fn from(value: TracePrefixed) -> Self { + IbcPrefixed::from(&value) + } +} + +impl<'a> From<&'a TracePrefixed> for IbcPrefixed { + fn from(value: &TracePrefixed) -> Self { + value.to_ibc_prefixed() + } +} + +impl From for IbcPrefixed { + fn from(value: Denom) -> Self { + value.to_ibc_prefixed() + } +} + +impl<'a> From<&'a Denom> for IbcPrefixed { + fn from(value: &Denom) -> Self { + value.to_ibc_prefixed() + } +} + impl FromStr for Denom { type Err = ParseDenomError; @@ -142,7 +178,7 @@ impl From for ParseDenomError { } /// An ICS20 denomination of the form `[port/channel/..]base_denom`. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct TracePrefixed { trace: TraceSegments, base_denom: String, @@ -150,7 +186,7 @@ pub struct TracePrefixed { impl TracePrefixed { #[must_use] - pub fn id(&self) -> super::Id { + pub fn to_ibc_prefixed(&self) -> IbcPrefixed { use sha2::Digest as _; let mut hasher = sha2::Sha256::new(); for segment in &self.trace.inner { @@ -160,7 +196,10 @@ impl TracePrefixed { hasher.update(b"/"); } hasher.update(self.base_denom.as_bytes()); - super::Id::new(hasher.finalize().into()) + let id = hasher.finalize().into(); + IbcPrefixed { + id, + } } #[must_use] @@ -260,7 +299,7 @@ impl TracePrefixed { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq)] struct TraceSegments { inner: VecDeque, } @@ -326,7 +365,7 @@ impl FromStr for TraceSegments { Ok(parsed_segments) } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct PortAndChannel { port: String, channel: String, @@ -465,7 +504,7 @@ enum ParseIbcPrefixedErrorKind { } /// An ICS20 denomination of the form `ibc/`. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] pub struct IbcPrefixed { id: [u8; 32], } @@ -479,8 +518,8 @@ impl IbcPrefixed { } #[must_use] - pub fn id(&self) -> super::Id { - super::Id::new(self.id) + pub fn get(&self) -> [u8; 32] { + self.id } } @@ -534,8 +573,8 @@ mod serde_impl { D: Deserializer<'de>, { use serde::de::Error as _; - let s = <&str>::deserialize(deserializer)?; - s.parse().map_err(D::Error::custom) + let s = std::borrow::Cow::<'_, str>::deserialize(deserializer)?; + s.trim().parse().map_err(D::Error::custom) } } diff --git a/crates/astria-core/src/primitive/v1/asset/mod.rs b/crates/astria-core/src/primitive/v1/asset/mod.rs index e2f29887e2..1b0c4e820d 100644 --- a/crates/astria-core/src/primitive/v1/asset/mod.rs +++ b/crates/astria-core/src/primitive/v1/asset/mod.rs @@ -1,98 +1,7 @@ -use std::fmt::{ - self, - Display, - Formatter, -}; - pub mod denom; pub use denom::{ Denom, + IbcPrefixed, ParseDenomError, + TracePrefixed, }; - -/// The default sequencer asset base denomination. -pub const DEFAULT_NATIVE_ASSET_DENOM: &str = "nria"; - -/// Returns the default sequencer asset [`Denom`]. -// allow: parsing single-segment assets is unit tested -#[allow(clippy::missing_panics_doc)] -// allow: used in many places already -#[allow(clippy::module_name_repetitions)] -#[must_use] -pub fn default_native_asset() -> Denom { - DEFAULT_NATIVE_ASSET_DENOM - .parse() - .expect("parsing a single segment string must work") -} - -/// Asset ID, which is the hash of the denomination trace. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize))] -#[cfg_attr(feature = "serde", serde(transparent))] -pub struct Id( - #[cfg_attr( - feature = "serde", - serde(serialize_with = "crate::serde::base64_serialize") - )] - [u8; 32], -); - -impl Id { - #[must_use] - pub fn new(arr: [u8; 32]) -> Self { - Self(arr) - } - - /// Constructs an ID by hashing `s` without checking if `s` is a valid denom. - #[must_use] - pub fn from_str_unchecked(s: &str) -> Self { - use sha2::Digest as _; - let hash = sha2::Sha256::digest(s.as_bytes()); - Self(hash.into()) - } - - #[must_use] - pub fn get(self) -> [u8; 32] { - self.0 - } - - /// Returns an ID given a 32-byte slice. - /// - /// # Errors - /// - /// Returns an error if the slice is not 32 bytes long. - pub fn try_from_slice(slice: &[u8]) -> Result { - if slice.len() != 32 { - return Err(IncorrectAssetIdLength { - received: slice.len(), - }); - } - - let mut id = [0u8; 32]; - id.copy_from_slice(slice); - Ok(Self(id)) - } -} - -impl AsRef<[u8]> for Id { - fn as_ref(&self) -> &[u8] { - &self.0 - } -} - -impl Display for Id { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - use base64::{ - display::Base64Display, - prelude::BASE64_STANDARD, - }; - Base64Display::new(self.as_ref(), &BASE64_STANDARD).fmt(f) - } -} - -/// Indicates that the protobuf response contained an array field that was not 32 bytes long. -#[derive(Debug, thiserror::Error)] -#[error("expected 32 bytes, got {received}")] -pub struct IncorrectAssetIdLength { - received: usize, -} diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 1e9b2c43df..4c81638aab 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -20,7 +20,6 @@ use crate::{ pub const ADDRESS_LEN: usize = 20; pub const ROLLUP_ID_LEN: usize = 32; -pub const FEE_ASSET_ID_LEN: usize = 32; impl Protobuf for merkle::Proof { type Error = merkle::audit::InvalidProof; diff --git a/crates/astria-core/src/protocol/asset/v1alpha1/mod.rs b/crates/astria-core/src/protocol/asset/v1alpha1/mod.rs index 294a57b8d4..05ea2ebba0 100644 --- a/crates/astria-core/src/protocol/asset/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/asset/v1alpha1/mod.rs @@ -2,7 +2,6 @@ use super::raw; use crate::primitive::v1::asset::{ self, Denom, - IncorrectAssetIdLength, ParseDenomError, }; @@ -88,102 +87,99 @@ impl raw::DenomResponse { #[derive(Debug, thiserror::Error)] #[error(transparent)] -pub struct AllowedFeeAssetIdsResponseError(AllowedFeeAssetIdsResponseErrorKind); +pub struct AllowedFeeAssetsResponseError(AllowedFeeAssetsResponseErrorKind); #[derive(Debug, thiserror::Error)] -enum AllowedFeeAssetIdsResponseErrorKind { - #[error("failed to convert asset ID")] - IncorrectAssetIdLength(#[source] IncorrectAssetIdLength), +enum AllowedFeeAssetsResponseErrorKind { + #[error("failed to parse assets as IBC ICS20 denom")] + IncorrectAsset(#[source] ParseDenomError), } -impl AllowedFeeAssetIdsResponseError { - fn incorrect_asset_id_length(inner: IncorrectAssetIdLength) -> Self { - Self(AllowedFeeAssetIdsResponseErrorKind::IncorrectAssetIdLength( - inner, - )) +impl AllowedFeeAssetsResponseError { + fn incorrect_asset(inner: ParseDenomError) -> Self { + Self(AllowedFeeAssetsResponseErrorKind::IncorrectAsset(inner)) } } #[derive(Debug, PartialEq, Clone)] -pub struct AllowedFeeAssetIdsResponse { +pub struct AllowedFeeAssetsResponse { pub height: u64, - pub fee_asset_ids: Vec, + pub fee_assets: Vec, } -impl AllowedFeeAssetIdsResponse { - /// Converts a protobuf [`raw::AllowedFeeAssetIdsResponse`] to an astria - /// native [`AllowedFeeAssetIdsResponse`]. +impl AllowedFeeAssetsResponse { + /// Converts a protobuf [`raw::AllowedFeeAssetsResponse`] to an astria + /// native [`AllowedFeeAssetsResponse`]. /// /// # Errors - /// - If one of the serialized asset IDs cannot be converted to a [`asset::Id`]. + /// - If one of the fee asset strings cannot be parsed as an [`asset::Denom`]. pub fn try_from_raw( - proto: &raw::AllowedFeeAssetIdsResponse, - ) -> Result { - let raw::AllowedFeeAssetIdsResponse { + proto: &raw::AllowedFeeAssetsResponse, + ) -> Result { + let raw::AllowedFeeAssetsResponse { height, - fee_asset_ids, + fee_assets, } = proto; - let mut assets: Vec = Vec::new(); + let mut assets: Vec = Vec::new(); - for raw_id in fee_asset_ids { - let native_id = asset::Id::try_from_slice(raw_id) - .map_err(AllowedFeeAssetIdsResponseError::incorrect_asset_id_length)?; - assets.push(native_id); + for s in fee_assets { + let native = s + .parse() + .map_err(AllowedFeeAssetsResponseError::incorrect_asset)?; + assets.push(native); } Ok(Self { height: *height, - fee_asset_ids: assets, + fee_assets: assets, }) } - /// Converts an astria native [`AllowedFeeAssetIdsResponse`] to a - /// protobuf [`raw::AllowedFeeAssetIdsResponse`]. + /// Converts an astria native [`AllowedFeeAssetsResponse`] to a + /// protobuf [`raw::AllowedFeeAssetsResponse`]. #[must_use] - pub fn into_raw(self) -> raw::AllowedFeeAssetIdsResponse { - raw::AllowedFeeAssetIdsResponse::from_native(self) + pub fn into_raw(self) -> raw::AllowedFeeAssetsResponse { + raw::AllowedFeeAssetsResponse::from_native(self) } } -impl raw::AllowedFeeAssetIdsResponse { - /// Converts an astria native [`AllowedFeeAssetIdsResponse`] to a - /// protobuf [`raw::AllowedFeeAssetIdsResponse`]. +impl raw::AllowedFeeAssetsResponse { + /// Converts an astria native [`AllowedFeeAssetsResponse`] to a + /// protobuf [`raw::AllowedFeeAssetsResponse`]. #[must_use] - pub fn from_native(native: AllowedFeeAssetIdsResponse) -> Self { - let AllowedFeeAssetIdsResponse { + pub fn from_native(native: AllowedFeeAssetsResponse) -> Self { + let AllowedFeeAssetsResponse { height, - fee_asset_ids, + fee_assets, } = native; - let raw_assets = fee_asset_ids + let fee_assets = fee_assets .into_iter() - .map(|id| id.as_ref().to_vec().into()) + .map(|denom| denom.to_string()) .collect(); Self { height, - fee_asset_ids: raw_assets, + fee_assets, } } - /// Converts a protobuf [`raw::AllowedFeeAssetIdsResponse`] to an astria - /// native [`AllowedFeeAssetIdsResponse`]. + /// Converts a protobuf [`raw::AllowedFeeAssetsResponse`] to an astria + /// native [`AllowedFeeAssetsResponse`]. /// /// # Errors - /// - If one of the serialized asset IDs cannot be converted to a [`asset::Id`]. + /// - If one of the assets cannot be parsed as an [`asset::Denom`]. pub fn try_into_native( self, - ) -> Result { - AllowedFeeAssetIdsResponse::try_from_raw(&self) + ) -> Result { + AllowedFeeAssetsResponse::try_from_raw(&self) } - /// Converts a protobuf [`raw::AllowedFeeAssetIdsResponse`] to an astria - /// native [`AllowedFeeAssetIdsResponse`] by allocating a new - /// [`v1alpha1::AllowedFeeAssetIdsResponse`]. + /// Converts a protobuf [`raw::AllowedFeeAssetsResponse`] to an astria + /// native [`AllowedFeeAssetsResponse`] by allocating a new + /// [`v1alpha1::AllowedFeeAssetsResponse`]. /// /// # Errors - /// - If one of the serialized asset IDs cannot be converted to a [`asset::Id`]. - pub fn try_to_native( - &self, - ) -> Result { + /// - If one of the assets cannot be parsed as an [`asset::Denom`]. + pub fn try_to_native(&self) -> Result { self.clone().try_into_native() } } @@ -232,61 +228,43 @@ mod tests { } #[test] - fn allowed_fee_asset_ids_try_from_raw_is_correct() { - let raw = raw::AllowedFeeAssetIdsResponse { + fn allowed_fee_assets_try_from_raw_is_correct() { + let raw = raw::AllowedFeeAssetsResponse { height: 42, - fee_asset_ids: vec![ - asset::Id::from_str_unchecked("asset_0") - .get() - .to_vec() - .into(), - asset::Id::from_str_unchecked("asset_1") - .get() - .to_vec() - .into(), - asset::Id::from_str_unchecked("asset_2") - .get() - .to_vec() - .into(), + fee_assets: vec![ + "asset_0".to_string(), + "asset_1".to_string(), + "asset_2".to_string(), ], }; - let expected = AllowedFeeAssetIdsResponse { + let expected = AllowedFeeAssetsResponse { height: 42, - fee_asset_ids: vec![ - asset::Id::from_str_unchecked("asset_0"), - asset::Id::from_str_unchecked("asset_1"), - asset::Id::from_str_unchecked("asset_2"), + fee_assets: vec![ + "asset_0".parse().unwrap(), + "asset_1".parse().unwrap(), + "asset_2".parse().unwrap(), ], }; - let actual = AllowedFeeAssetIdsResponse::try_from_raw(&raw).unwrap(); + let actual = AllowedFeeAssetsResponse::try_from_raw(&raw).unwrap(); assert_eq!(expected, actual); } #[test] - fn allowed_fee_asset_ids_into_raw_is_correct() { - let native = AllowedFeeAssetIdsResponse { + fn allowed_fee_assets_into_raw_is_correct() { + let native = AllowedFeeAssetsResponse { height: 42, - fee_asset_ids: vec![ - asset::Id::from_str_unchecked("asset_0"), - asset::Id::from_str_unchecked("asset_1"), - asset::Id::from_str_unchecked("asset_2"), + fee_assets: vec![ + "asset_0".parse().unwrap(), + "asset_1".parse().unwrap(), + "asset_2".parse().unwrap(), ], }; - let expected = raw::AllowedFeeAssetIdsResponse { + let expected = raw::AllowedFeeAssetsResponse { height: 42, - fee_asset_ids: vec![ - asset::Id::from_str_unchecked("asset_0") - .get() - .to_vec() - .into(), - asset::Id::from_str_unchecked("asset_1") - .get() - .to_vec() - .into(), - asset::Id::from_str_unchecked("asset_2") - .get() - .to_vec() - .into(), + fee_assets: vec![ + "asset_0".to_string(), + "asset_1".to_string(), + "asset_2".to_string(), ], }; let actual = native.into_raw(); @@ -294,13 +272,13 @@ mod tests { } #[test] - fn allowed_fee_asset_ids_roundtrip_is_correct() { - let native = AllowedFeeAssetIdsResponse { + fn allowed_fee_assets_roundtrip_is_correct() { + let native = AllowedFeeAssetsResponse { height: 42, - fee_asset_ids: vec![ - asset::Id::from_str_unchecked("asset_0"), - asset::Id::from_str_unchecked("asset_1"), - asset::Id::from_str_unchecked("asset_2"), + fee_assets: vec![ + "asset_0".parse().unwrap(), + "asset_1".parse().unwrap(), + "asset_2".parse().unwrap(), ], }; let expected = native.clone(); diff --git a/crates/astria-core/src/protocol/test_utils.rs b/crates/astria-core/src/protocol/test_utils.rs index f1569e8ec4..ff7f325fd1 100644 --- a/crates/astria-core/src/protocol/test_utils.rs +++ b/crates/astria-core/src/protocol/test_utils.rs @@ -15,7 +15,6 @@ use super::{ use crate::{ crypto::SigningKey, primitive::v1::{ - asset::default_native_asset, derive_merkle_tree_from_rollup_txs, RollupId, }, @@ -96,7 +95,7 @@ impl ConfigureSequencerBlock { SequenceAction { rollup_id, data, - fee_asset_id: default_native_asset().id(), + fee_asset: "nria".parse().unwrap(), } .into() }) @@ -126,11 +125,14 @@ impl ConfigureSequencerBlock { let mut rollup_transactions = group_sequence_actions_in_signed_transaction_transactions_by_rollup_id(&txs); for (rollup_id, deposit) in deposits_map.clone() { - rollup_transactions.entry(rollup_id).or_default().extend( - deposit - .into_iter() - .map(|deposit| RollupData::Deposit(deposit).into_raw().encode_to_vec()), - ); + rollup_transactions + .entry(rollup_id) + .or_default() + .extend(deposit.into_iter().map(|deposit| { + RollupData::Deposit(Box::new(deposit)) + .into_raw() + .encode_to_vec() + })); } rollup_transactions.sort_unstable_keys(); let rollup_transactions_tree = derive_merkle_tree_from_rollup_txs(&rollup_transactions); diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index daf546915a..6a06ba6359 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -348,8 +348,8 @@ impl SequenceActionError { Self(SequenceActionErrorKind::RollupIdLength(inner)) } - fn fee_asset_id(inner: asset::IncorrectAssetIdLength) -> Self { - Self(SequenceActionErrorKind::FeeAssetId(inner)) + fn fee_asset(inner: asset::ParseDenomError) -> Self { + Self(SequenceActionErrorKind::FeeAsset(inner)) } } @@ -359,8 +359,8 @@ enum SequenceActionErrorKind { FieldNotSet(&'static str), #[error("`rollup_id` field did not contain a valid rollup ID")] RollupIdLength(IncorrectRollupIdLength), - #[error("`fee_asset_id` field did not contain a valid asset ID")] - FeeAssetId(asset::IncorrectAssetIdLength), + #[error("`fee_asset` field did not contain a valid asset ID")] + FeeAsset(#[source] asset::ParseDenomError), } #[derive(Clone, Debug)] @@ -369,7 +369,7 @@ pub struct SequenceAction { pub rollup_id: RollupId, pub data: Vec, /// asset to use for fee payment. - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, } impl SequenceAction { @@ -378,12 +378,12 @@ impl SequenceAction { let Self { rollup_id, data, - fee_asset_id, + fee_asset, } = self; raw::SequenceAction { rollup_id: Some(rollup_id.to_raw()), data, - fee_asset_id: fee_asset_id.as_ref().to_vec(), + fee_asset: fee_asset.to_string(), } } @@ -392,12 +392,12 @@ impl SequenceAction { let Self { rollup_id, data, - fee_asset_id, + fee_asset, } = self; raw::SequenceAction { rollup_id: Some(rollup_id.to_raw()), data: data.clone(), - fee_asset_id: fee_asset_id.as_ref().to_vec(), + fee_asset: fee_asset.to_string(), } } @@ -409,19 +409,18 @@ impl SequenceAction { let raw::SequenceAction { rollup_id, data, - fee_asset_id, + fee_asset, } = proto; let Some(rollup_id) = rollup_id else { return Err(SequenceActionError::field_not_set("rollup_id")); }; let rollup_id = RollupId::try_from_raw(&rollup_id).map_err(SequenceActionError::rollup_id_length)?; - let fee_asset_id = - asset::Id::try_from_slice(&fee_asset_id).map_err(SequenceActionError::fee_asset_id)?; + let fee_asset = fee_asset.parse().map_err(SequenceActionError::fee_asset)?; Ok(Self { rollup_id, data, - fee_asset_id, + fee_asset, }) } } @@ -432,9 +431,9 @@ pub struct TransferAction { pub to: Address, pub amount: u128, // asset to be transferred. - pub asset_id: asset::Id, + pub asset: asset::Denom, /// asset to use for fee payment. - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, } impl TransferAction { @@ -443,14 +442,14 @@ impl TransferAction { let Self { to, amount, - asset_id, - fee_asset_id, + asset, + fee_asset, } = self; raw::TransferAction { to: Some(to.to_raw()), amount: Some(amount.into()), - asset_id: asset_id.get().to_vec(), - fee_asset_id: fee_asset_id.as_ref().to_vec(), + asset: asset.to_string(), + fee_asset: fee_asset.to_string(), } } @@ -459,14 +458,14 @@ impl TransferAction { let Self { to, amount, - asset_id, - fee_asset_id, + asset, + fee_asset, } = self; raw::TransferAction { to: Some(to.to_raw()), amount: Some((*amount).into()), - asset_id: asset_id.get().to_vec(), - fee_asset_id: fee_asset_id.as_ref().to_vec(), + asset: asset.to_string(), + fee_asset: fee_asset.to_string(), } } @@ -480,24 +479,22 @@ impl TransferAction { let raw::TransferAction { to, amount, - asset_id, - fee_asset_id, + asset, + fee_asset, } = proto; let Some(to) = to else { return Err(TransferActionError::field_not_set("to")); }; let to = Address::try_from_raw(&to).map_err(TransferActionError::address)?; let amount = amount.map_or(0, Into::into); - let asset_id = - asset::Id::try_from_slice(&asset_id).map_err(TransferActionError::asset_id)?; - let fee_asset_id = - asset::Id::try_from_slice(&fee_asset_id).map_err(TransferActionError::fee_asset_id)?; + let asset = asset.parse().map_err(TransferActionError::asset)?; + let fee_asset = fee_asset.parse().map_err(TransferActionError::fee_asset)?; Ok(Self { to, amount, - asset_id, - fee_asset_id, + asset, + fee_asset, }) } } @@ -515,11 +512,11 @@ impl TransferActionError { Self(TransferActionErrorKind::Address(inner)) } - fn asset_id(inner: asset::IncorrectAssetIdLength) -> Self { + fn asset(inner: asset::ParseDenomError) -> Self { Self(TransferActionErrorKind::Asset(inner)) } - fn fee_asset_id(inner: asset::IncorrectAssetIdLength) -> Self { + fn fee_asset(inner: asset::ParseDenomError) -> Self { Self(TransferActionErrorKind::FeeAsset(inner)) } } @@ -530,10 +527,10 @@ enum TransferActionErrorKind { FieldNotSet(&'static str), #[error("`to` field did not contain a valid address")] Address(#[source] AddressError), - #[error("`asset_id` field did not contain a valid asset ID")] - Asset(#[source] asset::IncorrectAssetIdLength), - #[error("`fee_asset_id` field did not contain a valid asset ID")] - FeeAsset(#[source] asset::IncorrectAssetIdLength), + #[error("`asset` field did not contain a valid asset ID")] + Asset(#[source] asset::ParseDenomError), + #[error("`fee_asset` field did not contain a valid asset ID")] + FeeAsset(#[source] asset::ParseDenomError), } #[derive(Clone, Debug)] @@ -636,7 +633,7 @@ pub struct Ics20Withdrawal { // the source channel used for the withdrawal. pub source_channel: ChannelId, // the asset to use for fee payment. - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, // a memo to include with the transfer pub memo: String, // the address of the bridge account to transfer from, if this is a withdrawal @@ -688,8 +685,8 @@ impl Ics20Withdrawal { } #[must_use] - pub fn fee_asset_id(&self) -> &asset::Id { - &self.fee_asset_id + pub fn fee_asset(&self) -> &asset::Denom { + &self.fee_asset } #[must_use] @@ -718,7 +715,7 @@ impl Ics20Withdrawal { timeout_height: Some(self.timeout_height.into_raw()), timeout_time: self.timeout_time, source_channel: self.source_channel.to_string(), - fee_asset_id: self.fee_asset_id.get().to_vec(), + fee_asset: self.fee_asset.to_string(), memo: self.memo.clone(), bridge_address: self.bridge_address.as_ref().map(Address::to_raw), } @@ -734,7 +731,7 @@ impl Ics20Withdrawal { timeout_height: Some(self.timeout_height.into_raw()), timeout_time: self.timeout_time, source_channel: self.source_channel.to_string(), - fee_asset_id: self.fee_asset_id.get().to_vec(), + fee_asset: self.fee_asset.to_string(), memo: self.memo, bridge_address: self.bridge_address.map(Address::into_raw), } @@ -758,7 +755,7 @@ impl Ics20Withdrawal { timeout_height, timeout_time, source_channel, - fee_asset_id, + fee_asset, memo, bridge_address, } = proto; @@ -787,8 +784,9 @@ impl Ics20Withdrawal { source_channel: source_channel .parse() .map_err(Ics20WithdrawalError::invalid_source_channel)?, - fee_asset_id: asset::Id::try_from_slice(&fee_asset_id) - .map_err(Ics20WithdrawalError::invalid_fee_asset_id)?, + fee_asset: fee_asset + .parse() + .map_err(Ics20WithdrawalError::invalid_fee_asset)?, memo, bridge_address, }) @@ -848,8 +846,8 @@ impl Ics20WithdrawalError { } #[must_use] - fn invalid_fee_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(Ics20WithdrawalErrorKind::InvalidFeeAssetId(err)) + fn invalid_fee_asset(err: asset::ParseDenomError) -> Self { + Self(Ics20WithdrawalErrorKind::InvalidFeeAsset(err)) } #[must_use] @@ -872,8 +870,8 @@ enum Ics20WithdrawalErrorKind { ReturnAddress { source: AddressError }, #[error("`source_channel` field was invalid")] InvalidSourceChannel(#[source] IdentifierError), - #[error("`fee_asset_id` field was invalid")] - InvalidFeeAssetId(#[source] asset::IncorrectAssetIdLength), + #[error("field `fee_asset` could not be parsed")] + InvalidFeeAsset(#[source] asset::ParseDenomError), #[error("`bridge_address` field was invalid")] InvalidBridgeAddress(#[source] AddressError), #[error("`denom` field was invalid")] @@ -977,22 +975,22 @@ enum IbcRelayerChangeActionErrorKind { #[allow(clippy::module_name_repetitions)] #[derive(Debug, Clone)] pub enum FeeAssetChangeAction { - Addition(asset::Id), - Removal(asset::Id), + Addition(asset::Denom), + Removal(asset::Denom), } impl FeeAssetChangeAction { #[must_use] pub fn into_raw(self) -> raw::FeeAssetChangeAction { match self { - FeeAssetChangeAction::Addition(asset_id) => raw::FeeAssetChangeAction { + FeeAssetChangeAction::Addition(asset) => raw::FeeAssetChangeAction { value: Some(raw::fee_asset_change_action::Value::Addition( - asset_id.get().to_vec(), + asset.to_string(), )), }, - FeeAssetChangeAction::Removal(asset_id) => raw::FeeAssetChangeAction { + FeeAssetChangeAction::Removal(asset) => raw::FeeAssetChangeAction { value: Some(raw::fee_asset_change_action::Value::Removal( - asset_id.get().to_vec(), + asset.to_string(), )), }, } @@ -1001,14 +999,14 @@ impl FeeAssetChangeAction { #[must_use] pub fn to_raw(&self) -> raw::FeeAssetChangeAction { match self { - FeeAssetChangeAction::Addition(asset_id) => raw::FeeAssetChangeAction { + FeeAssetChangeAction::Addition(asset) => raw::FeeAssetChangeAction { value: Some(raw::fee_asset_change_action::Value::Addition( - asset_id.get().to_vec(), + asset.to_string(), )), }, - FeeAssetChangeAction::Removal(asset_id) => raw::FeeAssetChangeAction { + FeeAssetChangeAction::Removal(asset) => raw::FeeAssetChangeAction { value: Some(raw::fee_asset_change_action::Value::Removal( - asset_id.get().to_vec(), + asset.to_string(), )), }, } @@ -1018,26 +1016,28 @@ impl FeeAssetChangeAction { /// /// # Errors /// - /// - if the `asset_id` field is invalid + /// - if the `asset` field is invalid pub fn try_from_raw( raw: &raw::FeeAssetChangeAction, ) -> Result { match raw { raw::FeeAssetChangeAction { - value: Some(raw::fee_asset_change_action::Value::Addition(asset_id)), + value: Some(raw::fee_asset_change_action::Value::Addition(asset)), } => { - let asset_id = asset::Id::try_from_slice(asset_id) - .map_err(FeeAssetChangeActionError::invalid_asset_id)?; - Ok(FeeAssetChangeAction::Addition(asset_id)) + let asset = asset + .parse() + .map_err(FeeAssetChangeActionError::invalid_asset)?; + Ok(FeeAssetChangeAction::Addition(asset)) } raw::FeeAssetChangeAction { - value: Some(raw::fee_asset_change_action::Value::Removal(asset_id)), + value: Some(raw::fee_asset_change_action::Value::Removal(asset)), } => { - let asset_id = asset::Id::try_from_slice(asset_id) - .map_err(FeeAssetChangeActionError::invalid_asset_id)?; - Ok(FeeAssetChangeAction::Removal(asset_id)) + let asset = asset + .parse() + .map_err(FeeAssetChangeActionError::invalid_asset)?; + Ok(FeeAssetChangeAction::Removal(asset)) } - _ => Err(FeeAssetChangeActionError::missing_asset_id()), + _ => Err(FeeAssetChangeActionError::missing_asset()), } } } @@ -1048,22 +1048,22 @@ pub struct FeeAssetChangeActionError(FeeAssetChangeActionErrorKind); impl FeeAssetChangeActionError { #[must_use] - fn invalid_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(FeeAssetChangeActionErrorKind::InvalidAssetId(err)) + fn invalid_asset(err: asset::ParseDenomError) -> Self { + Self(FeeAssetChangeActionErrorKind::InvalidAsset(err)) } #[must_use] - fn missing_asset_id() -> Self { - Self(FeeAssetChangeActionErrorKind::MissingAssetId) + fn missing_asset() -> Self { + Self(FeeAssetChangeActionErrorKind::MissingAsset) } } #[derive(Debug, thiserror::Error)] enum FeeAssetChangeActionErrorKind { - #[error("the asset_id was invalid")] - InvalidAssetId(#[source] asset::IncorrectAssetIdLength), - #[error("the asset_id was missing")] - MissingAssetId, + #[error("the `asset` field was invalid")] + InvalidAsset(#[source] asset::ParseDenomError), + #[error("the `asset` field was not set")] + MissingAsset, } #[allow(clippy::module_name_repetitions)] @@ -1072,9 +1072,9 @@ pub struct InitBridgeAccountAction { // the rollup ID to register for the sender of this action pub rollup_id: RollupId, // the assets accepted by the bridge account - pub asset_id: asset::Id, + pub asset: asset::Denom, // the fee asset which to pay this action's fees with - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, // the address corresponding to the key which has sudo capabilities; // ie. can change the sudo and withdrawer addresses for this bridge account. // if unset, this is set to the sender of the transaction. @@ -1089,8 +1089,8 @@ impl InitBridgeAccountAction { pub fn into_raw(self) -> raw::InitBridgeAccountAction { raw::InitBridgeAccountAction { rollup_id: Some(self.rollup_id.to_raw()), - asset_id: self.asset_id.get().to_vec(), - fee_asset_id: self.fee_asset_id.get().to_vec(), + asset: self.asset.to_string(), + fee_asset: self.fee_asset.to_string(), sudo_address: self.sudo_address.map(Address::into_raw), withdrawer_address: self.withdrawer_address.map(Address::into_raw), } @@ -1100,8 +1100,8 @@ impl InitBridgeAccountAction { pub fn to_raw(&self) -> raw::InitBridgeAccountAction { raw::InitBridgeAccountAction { rollup_id: Some(self.rollup_id.to_raw()), - asset_id: self.asset_id.get().to_vec(), - fee_asset_id: self.fee_asset_id.get().to_vec(), + asset: self.asset.to_string(), + fee_asset: self.fee_asset.to_string(), sudo_address: self.sudo_address.as_ref().map(Address::to_raw), withdrawer_address: self.withdrawer_address.as_ref().map(Address::to_raw), } @@ -1123,10 +1123,14 @@ impl InitBridgeAccountAction { }; let rollup_id = RollupId::try_from_raw(&rollup_id) .map_err(InitBridgeAccountActionError::invalid_rollup_id)?; - let asset_id = asset::Id::try_from_slice(&proto.asset_id) - .map_err(InitBridgeAccountActionError::invalid_asset_id)?; - let fee_asset_id = asset::Id::try_from_slice(&proto.fee_asset_id) - .map_err(InitBridgeAccountActionError::invalid_fee_asset_id)?; + let asset = proto + .asset + .parse() + .map_err(InitBridgeAccountActionError::invalid_asset)?; + let fee_asset = proto + .fee_asset + .parse() + .map_err(InitBridgeAccountActionError::invalid_fee_asset)?; let sudo_address = proto .sudo_address .as_ref() @@ -1142,8 +1146,8 @@ impl InitBridgeAccountAction { Ok(Self { rollup_id, - asset_id, - fee_asset_id, + asset, + fee_asset, sudo_address, withdrawer_address, }) @@ -1166,13 +1170,13 @@ impl InitBridgeAccountActionError { } #[must_use] - fn invalid_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(InitBridgeAccountActionErrorKind::InvalidAssetId(err)) + fn invalid_asset(err: asset::ParseDenomError) -> Self { + Self(InitBridgeAccountActionErrorKind::InvalidAsset(err)) } #[must_use] - fn invalid_fee_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(InitBridgeAccountActionErrorKind::InvalidFeeAssetId(err)) + fn invalid_fee_asset(err: asset::ParseDenomError) -> Self { + Self(InitBridgeAccountActionErrorKind::InvalidFeeAsset(err)) } #[must_use] @@ -1199,9 +1203,9 @@ enum InitBridgeAccountActionErrorKind { #[error("the `rollup_id` field was invalid")] InvalidRollupId(#[source] IncorrectRollupIdLength), #[error("an asset ID was invalid")] - InvalidAssetId(#[source] asset::IncorrectAssetIdLength), - #[error("the `fee_asset_id` field was invalid")] - InvalidFeeAssetId(#[source] asset::IncorrectAssetIdLength), + InvalidAsset(#[source] asset::ParseDenomError), + #[error("the `fee_asset` field was invalid")] + InvalidFeeAsset(#[source] asset::ParseDenomError), #[error("the `sudo_address` field was invalid")] InvalidSudoAddress(#[source] AddressError), #[error("the `withdrawer_address` field was invalid")] @@ -1214,9 +1218,9 @@ pub struct BridgeLockAction { pub to: Address, pub amount: u128, // asset to be transferred. - pub asset_id: asset::Id, + pub asset: asset::Denom, // asset to use for fee payment. - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, // the address on the destination chain to send the transfer to. pub destination_chain_address: String, } @@ -1227,8 +1231,8 @@ impl BridgeLockAction { raw::BridgeLockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), - asset_id: self.asset_id.get().to_vec(), - fee_asset_id: self.fee_asset_id.as_ref().to_vec(), + asset: self.asset.to_string(), + fee_asset: self.fee_asset.to_string(), destination_chain_address: self.destination_chain_address, } } @@ -1238,8 +1242,8 @@ impl BridgeLockAction { raw::BridgeLockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), - asset_id: self.asset_id.get().to_vec(), - fee_asset_id: self.fee_asset_id.as_ref().to_vec(), + asset: self.asset.to_string(), + fee_asset: self.fee_asset.to_string(), destination_chain_address: self.destination_chain_address.clone(), } } @@ -1250,8 +1254,8 @@ impl BridgeLockAction { /// /// - if the `to` field is not set /// - if the `to` field is invalid - /// - if the `asset_id` field is invalid - /// - if the `fee_asset_id` 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 { let Some(to) = proto.to else { return Err(BridgeLockActionError::field_not_set("to")); @@ -1260,15 +1264,19 @@ impl BridgeLockAction { let amount = proto .amount .ok_or(BridgeLockActionError::missing_amount())?; - let asset_id = asset::Id::try_from_slice(&proto.asset_id) - .map_err(BridgeLockActionError::invalid_asset_id)?; - let fee_asset_id = asset::Id::try_from_slice(&proto.fee_asset_id) - .map_err(BridgeLockActionError::invalid_fee_asset_id)?; + let asset = proto + .asset + .parse() + .map_err(BridgeLockActionError::invalid_asset)?; + let fee_asset = proto + .fee_asset + .parse() + .map_err(BridgeLockActionError::invalid_fee_asset)?; Ok(Self { to, amount: amount.into(), - asset_id, - fee_asset_id, + asset, + fee_asset, destination_chain_address: proto.destination_chain_address, }) } @@ -1297,13 +1305,13 @@ impl BridgeLockActionError { } #[must_use] - fn invalid_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(BridgeLockActionErrorKind::InvalidAssetId(err)) + fn invalid_asset(err: asset::ParseDenomError) -> Self { + Self(BridgeLockActionErrorKind::InvalidAsset(err)) } #[must_use] - fn invalid_fee_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(BridgeLockActionErrorKind::InvalidFeeAssetId(err)) + fn invalid_fee_asset(err: asset::ParseDenomError) -> Self { + Self(BridgeLockActionErrorKind::InvalidFeeAsset(err)) } } @@ -1315,10 +1323,10 @@ enum BridgeLockActionErrorKind { Address { source: AddressError }, #[error("the `amount` field was not set")] MissingAmount, - #[error("the `asset_id` field was invalid")] - InvalidAssetId(#[source] asset::IncorrectAssetIdLength), - #[error("the `fee_asset_id` field was invalid")] - InvalidFeeAssetId(#[source] asset::IncorrectAssetIdLength), + #[error("the `asset` field was invalid")] + InvalidAsset(#[source] asset::ParseDenomError), + #[error("the `fee_asset` field was invalid")] + InvalidFeeAsset(#[source] asset::ParseDenomError), } #[allow(clippy::module_name_repetitions)] @@ -1327,7 +1335,7 @@ pub struct BridgeUnlockAction { pub to: Address, pub amount: u128, // asset to use for fee payment. - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, // memo for double spend protection. pub memo: Vec, // the address of the bridge account to transfer from, @@ -1342,7 +1350,7 @@ impl BridgeUnlockAction { raw::BridgeUnlockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), - fee_asset_id: self.fee_asset_id.as_ref().to_vec(), + fee_asset: self.fee_asset.to_string(), memo: self.memo, bridge_address: self.bridge_address.map(Address::into_raw), } @@ -1353,7 +1361,7 @@ impl BridgeUnlockAction { raw::BridgeUnlockAction { to: Some(self.to.to_raw()), amount: Some(self.amount.into()), - fee_asset_id: self.fee_asset_id.as_ref().to_vec(), + fee_asset: self.fee_asset.to_string(), memo: self.memo.clone(), bridge_address: self.bridge_address.as_ref().map(Address::to_raw), } @@ -1366,7 +1374,7 @@ impl BridgeUnlockAction { /// - if the `to` field is not set /// - if the `to` field is invalid /// - if the `amount` field is invalid - /// - if the `fee_asset_id` 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 { let Some(to) = proto.to else { @@ -1376,8 +1384,10 @@ impl BridgeUnlockAction { let amount = proto .amount .ok_or(BridgeUnlockActionError::missing_amount())?; - let fee_asset_id = asset::Id::try_from_slice(&proto.fee_asset_id) - .map_err(BridgeUnlockActionError::invalid_fee_asset_id)?; + let fee_asset = proto + .fee_asset + .parse() + .map_err(BridgeUnlockActionError::invalid_fee_asset)?; let bridge_address = proto .bridge_address .as_ref() @@ -1387,7 +1397,7 @@ impl BridgeUnlockAction { Ok(Self { to, amount: amount.into(), - fee_asset_id, + fee_asset, memo: proto.memo, bridge_address, }) @@ -1417,8 +1427,8 @@ impl BridgeUnlockActionError { } #[must_use] - fn invalid_fee_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(BridgeUnlockActionErrorKind::InvalidFeeAssetId(err)) + fn invalid_fee_asset(err: asset::ParseDenomError) -> Self { + Self(BridgeUnlockActionErrorKind::InvalidFeeAsset(err)) } #[must_use] @@ -1435,8 +1445,8 @@ enum BridgeUnlockActionErrorKind { Address { source: AddressError }, #[error("the `amount` field was not set")] MissingAmount, - #[error("the `fee_asset_id` field was invalid")] - InvalidFeeAssetId(#[source] asset::IncorrectAssetIdLength), + #[error("the `fee_asset` field was invalid")] + InvalidFeeAsset(#[source] asset::ParseDenomError), #[error("the `bridge_address` field was invalid")] InvalidBridgeAddress(#[source] AddressError), } @@ -1447,7 +1457,7 @@ pub struct BridgeSudoChangeAction { pub bridge_address: Address, pub new_sudo_address: Option
, pub new_withdrawer_address: Option
, - pub fee_asset_id: asset::Id, + pub fee_asset: asset::Denom, } impl BridgeSudoChangeAction { @@ -1457,7 +1467,7 @@ impl BridgeSudoChangeAction { bridge_address: Some(self.bridge_address.to_raw()), new_sudo_address: self.new_sudo_address.map(Address::into_raw), new_withdrawer_address: self.new_withdrawer_address.map(Address::into_raw), - fee_asset_id: self.fee_asset_id.get().to_vec(), + fee_asset: self.fee_asset.to_string(), } } @@ -1467,7 +1477,7 @@ impl BridgeSudoChangeAction { bridge_address: Some(self.bridge_address.to_raw()), new_sudo_address: self.new_sudo_address.as_ref().map(Address::to_raw), new_withdrawer_address: self.new_withdrawer_address.as_ref().map(Address::to_raw), - fee_asset_id: self.fee_asset_id.get().to_vec(), + fee_asset: self.fee_asset.to_string(), } } @@ -1479,7 +1489,7 @@ impl BridgeSudoChangeAction { /// - 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_id` field is invalid + /// - if the `fee_asset` field is invalid pub fn try_from_raw( proto: raw::BridgeSudoChangeAction, ) -> Result { @@ -1500,14 +1510,16 @@ impl BridgeSudoChangeAction { .map(Address::try_from_raw) .transpose() .map_err(BridgeSudoChangeActionError::invalid_new_withdrawer_address)?; - let fee_asset_id = asset::Id::try_from_slice(&proto.fee_asset_id) - .map_err(BridgeSudoChangeActionError::invalid_fee_asset_id)?; + let fee_asset = proto + .fee_asset + .parse() + .map_err(BridgeSudoChangeActionError::invalid_fee_asset)?; Ok(Self { bridge_address, new_sudo_address, new_withdrawer_address, - fee_asset_id, + fee_asset, }) } } @@ -1538,8 +1550,8 @@ impl BridgeSudoChangeActionError { } #[must_use] - fn invalid_fee_asset_id(err: asset::IncorrectAssetIdLength) -> Self { - Self(BridgeSudoChangeActionErrorKind::InvalidFeeAssetId(err)) + fn invalid_fee_asset(err: asset::ParseDenomError) -> Self { + Self(BridgeSudoChangeActionErrorKind::InvalidFeeAsset(err)) } } @@ -1553,8 +1565,8 @@ enum BridgeSudoChangeActionErrorKind { InvalidNewSudoAddress(#[source] AddressError), #[error("the `new_withdrawer_address` field was invalid")] InvalidNewWithdrawerAddress(#[source] AddressError), - #[error("the `fee_asset_id` field was invalid")] - InvalidFeeAssetId(#[source] asset::IncorrectAssetIdLength), + #[error("the `fee_asset` field was invalid")] + InvalidFeeAsset(#[source] asset::ParseDenomError), } #[derive(Debug, Clone)] diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index 4ca67df1e1..e4da3bb09e 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -475,13 +475,17 @@ mod test { use super::*; use crate::{ primitive::v1::{ - asset::default_native_asset, + asset, Address, }, protocol::transaction::v1alpha1::action::TransferAction, }; const ASTRIA_ADDRESS_PREFIX: &str = "astria"; + fn asset() -> asset::Denom { + "nria".parse().unwrap() + } + #[test] fn signed_transaction_hash() { let verification_key = VerificationKey::try_from([ @@ -503,8 +507,8 @@ mod test { .try_build() .unwrap(), amount: 0, - asset_id: default_native_asset().id(), - fee_asset_id: default_native_asset().id(), + asset: asset(), + fee_asset: asset(), }; let params = TransactionParams::from_raw(raw::TransactionParams { @@ -540,8 +544,8 @@ mod test { .try_build() .unwrap(), amount: 0, - asset_id: default_native_asset().id(), - fee_asset_id: default_native_asset().id(), + asset: asset(), + fee_asset: asset(), }; let params = TransactionParams::from_raw(raw::TransactionParams { diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap index 5a65b1ec82..2357850edd 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap @@ -3,36 +3,36 @@ source: crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs expression: tx.sha256_of_proto_encoding() --- [ - 34, - 82, - 88, - 30, - 41, - 64, - 178, - 49, - 45, - 195, - 239, - 10, - 174, - 39, - 237, - 81, - 217, - 210, - 116, - 141, - 114, - 137, - 209, - 236, - 54, - 17, - 110, - 147, - 150, - 117, - 135, - 115 + 69, + 121, + 100, + 127, + 170, + 148, + 126, + 89, + 26, + 55, + 220, + 69, + 192, + 9, + 204, + 228, + 198, + 50, + 224, + 128, + 204, + 76, + 243, + 148, + 32, + 203, + 177, + 19, + 14, + 106, + 67, + 114 ] diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs index 27c9503034..8653961f16 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs @@ -737,10 +737,11 @@ impl SequencerBlock { let signed_tx = SignedTransaction::try_from_raw(raw_tx) .map_err(SequencerBlockError::raw_signed_transaction_conversion)?; for action in signed_tx.into_unsigned().actions { + // XXX: The fee asset is dropped. We shjould explain why that's ok. if let action::Action::Sequence(action::SequenceAction { rollup_id, data, - fee_asset_id: _, + fee_asset: _, }) = action { let elem = rollup_datas.entry(rollup_id).or_insert(vec![]); @@ -750,11 +751,14 @@ impl SequencerBlock { } } for (id, deposits) in deposits { - rollup_datas.entry(id).or_default().extend( - deposits - .into_iter() - .map(|deposit| RollupData::Deposit(deposit).into_raw().encode_to_vec()), - ); + rollup_datas + .entry(id) + .or_default() + .extend(deposits.into_iter().map(|deposit| { + RollupData::Deposit(Box::new(deposit)) + .into_raw() + .encode_to_vec() + })); } // XXX: The rollup data must be sorted by its keys before constructing the merkle tree. @@ -1263,8 +1267,8 @@ pub struct Deposit { rollup_id: RollupId, // the amount that was transferred to `bridge_address` amount: u128, - // the asset ID of the asset that was transferred - asset_id: asset::Id, + // the IBC ICS20 denom of the asset that was transferred + asset: asset::Denom, // the address on the destination chain (rollup) which to send the bridged funds to destination_chain_address: String, } @@ -1281,14 +1285,14 @@ impl Deposit { bridge_address: Address, rollup_id: RollupId, amount: u128, - asset_id: asset::Id, + asset: asset::Denom, destination_chain_address: String, ) -> Self { Self { bridge_address, rollup_id, amount, - asset_id, + asset, destination_chain_address, } } @@ -1309,8 +1313,8 @@ impl Deposit { } #[must_use] - pub fn asset_id(&self) -> &asset::Id { - &self.asset_id + pub fn asset(&self) -> &asset::Denom { + &self.asset } #[must_use] @@ -1324,14 +1328,14 @@ impl Deposit { bridge_address, rollup_id, amount, - asset_id, + asset, destination_chain_address, } = self; raw::Deposit { bridge_address: Some(bridge_address.into_raw()), rollup_id: Some(rollup_id.into_raw()), amount: Some(amount.into()), - asset_id: asset_id.get().to_vec(), + asset: asset.to_string(), destination_chain_address, } } @@ -1349,7 +1353,7 @@ impl Deposit { bridge_address, rollup_id, amount, - asset_id, + asset, destination_chain_address, } = raw; let Some(bridge_address) = bridge_address else { @@ -1363,13 +1367,12 @@ impl Deposit { }; let rollup_id = RollupId::try_from_raw(&rollup_id).map_err(DepositError::incorrect_rollup_id_length)?; - let asset_id = asset::Id::try_from_slice(&asset_id) - .map_err(DepositError::incorrect_asset_id_length)?; + let asset = asset.parse().map_err(DepositError::incorrect_asset)?; Ok(Self { bridge_address, rollup_id, amount, - asset_id, + asset, destination_chain_address, }) } @@ -1394,8 +1397,8 @@ impl DepositError { Self(DepositErrorKind::IncorrectRollupIdLength(source)) } - fn incorrect_asset_id_length(source: asset::IncorrectAssetIdLength) -> Self { - Self(DepositErrorKind::IncorrectAssetIdLength(source)) + fn incorrect_asset(source: asset::ParseDenomError) -> Self { + Self(DepositErrorKind::IncorrectAsset(source)) } } @@ -1407,8 +1410,8 @@ enum DepositErrorKind { FieldNotSet(&'static str), #[error("the rollup ID length is not 32 bytes")] IncorrectRollupIdLength(#[source] IncorrectRollupIdLength), - #[error("the asset ID length is not 32 bytes")] - IncorrectAssetIdLength(#[source] asset::IncorrectAssetIdLength), + #[error("the `asset` field could not be parsed")] + IncorrectAsset(#[source] asset::ParseDenomError), } /// A piece of data that is sent to a rollup execution node. @@ -1421,7 +1424,7 @@ enum DepositErrorKind { #[derive(Debug, Clone, PartialEq)] pub enum RollupData { SequencedData(Vec), - Deposit(Deposit), + Deposit(Box), } impl RollupData { @@ -1450,6 +1453,7 @@ impl RollupData { match value { Some(raw::rollup_data::Value::SequencedData(data)) => Ok(Self::SequencedData(data)), Some(raw::rollup_data::Value::Deposit(deposit)) => Deposit::try_from_raw(deposit) + .map(Box::new) .map(Self::Deposit) .map_err(RollupDataError::deposit), None => Err(RollupDataError::field_not_set("data")), diff --git a/crates/astria-sequencer-client/src/extension_trait.rs b/crates/astria-sequencer-client/src/extension_trait.rs index f57c915e9a..d5b06bec6a 100644 --- a/crates/astria-sequencer-client/src/extension_trait.rs +++ b/crates/astria-sequencer-client/src/extension_trait.rs @@ -34,7 +34,7 @@ use std::{ }; use astria_core::protocol::{ - asset::v1alpha1::AllowedFeeAssetIdsResponse, + asset::v1alpha1::AllowedFeeAssetsResponse, bridge::v1alpha1::BridgeAccountLastTxHashResponse, }; pub use astria_core::{ @@ -472,10 +472,10 @@ pub trait SequencerClientExt: Client { /// /// - If calling tendermint `abci_query` RPC fails. /// - If the bytes contained in the abci query response cannot be deserialized as an - /// `astria.protocol.asset.v1alpha1.AllowedFeeAssetIdsResponse`. + /// `astria.protocol.asset.v1alpha1.AllowedFeeAssetsResponse`. /// - If the raw response cannot be converted to the native type. - async fn get_allowed_fee_asset_ids(&self) -> Result { - let path = "asset/allowed_fee_asset_ids".to_string(); + async fn get_allowed_fee_assets(&self) -> Result { + let path = "asset/allowed_fee_assets".to_string(); let response = self .abci_query(Some(path), vec![], Some(0u32.into()), false) @@ -483,18 +483,18 @@ pub trait SequencerClientExt: Client { .map_err(|e| Error::tendermint_rpc("abci_query", e))?; let proto_response = - astria_core::generated::protocol::asset::v1alpha1::AllowedFeeAssetIdsResponse::decode( + astria_core::generated::protocol::asset::v1alpha1::AllowedFeeAssetsResponse::decode( &*response.value, ) .map_err(|e| { Error::abci_query_deserialization( - "astria.protocol.asset.v1alpha1.AllowedFeeAssetIdsResponse", + "astria.protocol.asset.v1alpha1.AllowedFeeAssetsResponse", response, e, ) })?; - let native_response = AllowedFeeAssetIdsResponse::try_from_raw(&proto_response) - .map_err(|e| Error::native_conversion("AllowedFeeAssetIdsResponse", Arc::new(e)))?; + let native_response = AllowedFeeAssetsResponse::try_from_raw(&proto_response) + .map_err(|e| Error::native_conversion("AllowedFeeAssetsResponse", Arc::new(e)))?; Ok(native_response) } diff --git a/crates/astria-sequencer-client/src/tests/http.rs b/crates/astria-sequencer-client/src/tests/http.rs index 3a46df9ae8..2f48e22211 100644 --- a/crates/astria-sequencer-client/src/tests/http.rs +++ b/crates/astria-sequencer-client/src/tests/http.rs @@ -1,13 +1,7 @@ use astria_core::{ crypto::SigningKey, - generated::protocol::asset::v1alpha1::AllowedFeeAssetIdsResponse, - primitive::v1::{ - asset::{ - self, - default_native_asset, - }, - Address, - }, + generated::protocol::asset::v1alpha1::AllowedFeeAssetsResponse, + primitive::v1::Address, protocol::transaction::v1alpha1::{ action::TransferAction, SignedTransaction, @@ -151,8 +145,8 @@ fn create_signed_transaction() -> SignedTransaction { TransferAction { to: bob_address(), amount: 333_333, - asset_id: default_native_asset().id(), - fee_asset_id: default_native_asset().id(), + asset: "nria".parse().unwrap(), + fee_asset: "nria".parse().unwrap(), } .into(), ]; @@ -235,32 +229,23 @@ async fn get_allowed_fee_assets() { client, } = MockSequencer::start().await; - let expected_response = AllowedFeeAssetIdsResponse { + let expected_response = AllowedFeeAssetsResponse { height: 10, - fee_asset_ids: vec![ - asset::Id::from_str_unchecked("asset_0") - .get() - .to_vec() - .into(), - asset::Id::from_str_unchecked("asset_1") - .get() - .to_vec() - .into(), - asset::Id::from_str_unchecked("asset_2") - .get() - .to_vec() - .into(), + fee_assets: vec![ + "asset_0".to_string(), + "asset_1".to_string(), + "asset_2".to_string(), ], }; let _guard = register_abci_query_response( &server, - "asset/allowed_fee_asset_ids", + "asset/allowed_fee_assets", expected_response.clone(), ) .await; - let actual_response = client.get_allowed_fee_asset_ids().await; + let actual_response = client.get_allowed_fee_assets().await; let actual_response = actual_response.unwrap().into_raw(); assert_eq!(expected_response, actual_response); diff --git a/crates/astria-sequencer-utils/src/blob_parser.rs b/crates/astria-sequencer-utils/src/blob_parser.rs index 803f8f7163..a5fdb45118 100644 --- a/crates/astria-sequencer-utils/src/blob_parser.rs +++ b/crates/astria-sequencer-utils/src/blob_parser.rs @@ -527,7 +527,7 @@ struct PrintableDeposit { bridge_address: String, rollup_id: String, amount: u128, - asset_id: String, + asset: String, destination_chain_address: String, } @@ -540,7 +540,7 @@ impl TryFrom<&RawDeposit> for PrintableDeposit { bridge_address: deposit.bridge_address().to_string(), rollup_id: deposit.rollup_id().to_string(), amount: deposit.amount(), - asset_id: deposit.asset_id().to_string(), + asset: deposit.asset().to_string(), destination_chain_address: deposit.destination_chain_address().to_string(), }) } @@ -551,7 +551,7 @@ impl Display for PrintableDeposit { colored_ln(f, "bridge address", &self.bridge_address)?; colored_ln(f, "rollup id", &self.rollup_id)?; colored_ln(f, "amount", self.amount)?; - colored_ln(f, "asset id", &self.asset_id)?; + colored_ln(f, "asset id", &self.asset)?; colored( f, "destination chain address", diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index 1d030084ac..c4b7c036ca 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -29,7 +29,7 @@ pub(crate) async fn transfer_check_stateful( ) -> Result<()> { ensure!( state - .is_allowed_fee_asset(action.fee_asset_id) + .is_allowed_fee_asset(&action.fee_asset) .await .context("failed to check allowed fee assets in state")?, "invalid fee asset", @@ -39,16 +39,16 @@ pub(crate) async fn transfer_check_stateful( .get_transfer_base_fee() .await .context("failed to get transfer base fee")?; - let transfer_asset_id = action.asset_id; + let transfer_asset = action.asset.clone(); let from_fee_balance = state - .get_account_balance(from, action.fee_asset_id) + .get_account_balance(from, &action.fee_asset) .await .context("failed getting `from` account balance for fee payment")?; // if fee asset is same as transfer asset, ensure accounts has enough funds // to cover both the fee and the amount transferred - if action.fee_asset_id == transfer_asset_id { + if action.fee_asset.to_ibc_prefixed() == transfer_asset.to_ibc_prefixed() { let payment_amount = action .amount .checked_add(fee) @@ -67,7 +67,7 @@ pub(crate) async fn transfer_check_stateful( ); let from_transfer_balance = state - .get_account_balance(from, transfer_asset_id) + .get_account_balance(from, transfer_asset) .await .context("failed to get account balance in transfer check")?; ensure!( @@ -118,15 +118,13 @@ impl ActionHandler for TransferAction { .await .context("failed to get transfer base fee")?; state - .get_and_increase_block_fees(self.fee_asset_id, fee) + .get_and_increase_block_fees(&self.fee_asset, fee) .await .context("failed to add to block fees")?; - let transfer_asset_id = self.asset_id; - // if fee payment asset is same asset as transfer asset, deduct fee // from same balance as asset transferred - if transfer_asset_id == self.fee_asset_id { + if self.asset.to_ibc_prefixed() == self.fee_asset.to_ibc_prefixed() { // check_stateful should have already checked this arithmetic let payment_amount = self .amount @@ -134,28 +132,28 @@ impl ActionHandler for TransferAction { .expect("transfer amount plus fee should not overflow"); state - .decrease_balance(from, transfer_asset_id, payment_amount) + .decrease_balance(from, &self.asset, payment_amount) .await .context("failed decreasing `from` account balance")?; state - .increase_balance(self.to, transfer_asset_id, self.amount) + .increase_balance(self.to, &self.asset, self.amount) .await .context("failed increasing `to` account balance")?; } else { // otherwise, just transfer the transfer asset and deduct fee from fee asset balance // later state - .decrease_balance(from, transfer_asset_id, self.amount) + .decrease_balance(from, &self.asset, self.amount) .await .context("failed decreasing `from` account balance")?; state - .increase_balance(self.to, transfer_asset_id, self.amount) + .increase_balance(self.to, &self.asset, self.amount) .await .context("failed increasing `to` account balance")?; // deduct fee from fee asset balance state - .decrease_balance(from, self.fee_asset_id, fee) + .decrease_balance(from, &self.fee_asset, fee) .await .context("failed decreasing `from` account balance for fee payment")?; } diff --git a/crates/astria-sequencer/src/accounts/component.rs b/crates/astria-sequencer/src/accounts/component.rs index 376b9c6ceb..4c4f7bc383 100644 --- a/crates/astria-sequencer/src/accounts/component.rs +++ b/crates/astria-sequencer/src/accounts/component.rs @@ -29,7 +29,7 @@ impl Component for AccountsComponent { let native_asset = get_native_asset(); for account in &app_state.accounts { state - .put_account_balance(account.address, native_asset.id(), account.balance) + .put_account_balance(account.address, native_asset, account.balance) .context("failed writing account balance to state")?; } diff --git a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__test__storage_keys_have_not_changed.snap b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__test__storage_keys_have_not_changed.snap deleted file mode 100644 index 9bdf8ef2fe..0000000000 --- a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__test__storage_keys_have_not_changed.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -source: crates/astria-sequencer/src/accounts/state_ext.rs -expression: "balance_storage_key(address, id)" ---- -accounts/1c0c490f1b5528d8173c5de46d131160e4b2c0c3/balance/000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f diff --git a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__test__storage_keys_have_not_changed-2.snap b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed-2.snap similarity index 100% rename from crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__test__storage_keys_have_not_changed-2.snap rename to crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed-2.snap diff --git a/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap new file mode 100644 index 0000000000..2f7840b415 --- /dev/null +++ b/crates/astria-sequencer/src/accounts/snapshots/astria_sequencer__accounts__state_ext__tests__storage_keys_have_not_changed.snap @@ -0,0 +1,5 @@ +--- +source: crates/astria-sequencer/src/accounts/state_ext.rs +expression: "balance_storage_key(address, asset)" +--- +accounts/1c0c490f1b5528d8173c5de46d131160e4b2c0c3/balance/be429a02d00837245167a2616674a979a2ac6f9806468b48a975b156ad711320 diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index 1037cd6a62..bee8646eef 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -19,7 +19,6 @@ use cnidarium::{ StateWrite, }; use futures::StreamExt; -use hex::ToHex as _; use tracing::instrument; /// Newtype wrapper to read and write a u32 from rocksdb. @@ -38,7 +37,6 @@ 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> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(ACCOUNTS_PREFIX)?; @@ -49,11 +47,15 @@ impl<'a> std::fmt::Display for StorageKey<'a> { Ok(()) } } -fn balance_storage_key(address: Address, asset: asset::Id) -> String { + +fn balance_storage_key>( + address: Address, + asset: TAsset, +) -> String { format!( "{}/balance/{}", StorageKey(&address), - asset.encode_hex::() + crate::storage_keys::hunks::Asset::from(asset) ) } @@ -82,18 +84,18 @@ pub(crate) trait StateReadExt: StateRead { continue; }; - let asset_id_str = key + let asset = key .strip_prefix(&prefix) - .context("failed to strip prefix from account balance key")?; - let asset_id_bytes = hex::decode(asset_id_str).context("invalid asset id bytes")?; + .context("failed to strip prefix from account balance key")? + .parse::() + .context("failed to parse storage key suffix as address hunk")? + .get(); - let asset_id = asset::Id::try_from_slice(&asset_id_bytes) - .context("failed to parse asset id from account balance key")?; let Balance(balance) = Balance::try_from_slice(&value).context("invalid balance bytes")?; let native_asset = crate::asset::get_native_asset(); - if asset_id == native_asset.id() { + if asset == native_asset.to_ibc_prefixed() { balances.push(AssetBalance { denom: native_asset.clone(), balance, @@ -102,7 +104,7 @@ pub(crate) trait StateReadExt: StateRead { } let denom = self - .get_ibc_asset(asset_id) + .map_ibc_to_trace_prefixed_asset(asset) .await .context("failed to get ibc asset denom")? .context("asset denom not found when user has balance of it; this is a bug")? @@ -115,8 +117,11 @@ pub(crate) trait StateReadExt: StateRead { Ok(balances) } - #[instrument(skip_all, fields(address=%address, asset_id=%asset))] - async fn get_account_balance(&self, address: Address, asset: asset::Id) -> Result { + #[instrument(skip_all, fields(address=%address, %asset))] + async fn get_account_balance<'a, TAsset>(&self, address: Address, asset: TAsset) -> Result + where + TAsset: Into + std::fmt::Display + Send, + { let Some(bytes) = self .get_raw(&balance_storage_key(address, asset)) .await @@ -162,13 +167,16 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { - #[instrument(skip(self))] - fn put_account_balance( + #[instrument(skip(self, asset), fields(%asset))] + fn put_account_balance( &mut self, address: Address, - asset: asset::Id, + asset: TAsset, balance: u128, - ) -> Result<()> { + ) -> Result<()> + where + TAsset: Into + std::fmt::Display + Send, + { let bytes = borsh::to_vec(&Balance(balance)).context("failed to serialize balance")?; self.put_raw(balance_storage_key(address, asset), bytes); Ok(()) @@ -181,13 +189,17 @@ pub(crate) trait StateWriteExt: StateWrite { Ok(()) } - #[instrument(skip(self))] - async fn increase_balance( + #[instrument(skip(self, asset), fields(%asset))] + async fn increase_balance( &mut self, address: Address, - asset: asset::Id, + asset: TAsset, amount: u128, - ) -> Result<()> { + ) -> Result<()> + where + TAsset: Into + std::fmt::Display + Send, + { + let asset = asset.into(); let balance = self .get_account_balance(address, asset) .await @@ -203,13 +215,17 @@ pub(crate) trait StateWriteExt: StateWrite { Ok(()) } - #[instrument(skip(self))] - async fn decrease_balance( + #[instrument(skip(self, asset), fields(%asset))] + async fn decrease_balance( &mut self, address: Address, - asset: asset::Id, + asset: TAsset, amount: u128, - ) -> Result<()> { + ) -> Result<()> + where + TAsset: Into + std::fmt::Display + Send, + { + let asset = asset.into(); let balance = self .get_account_balance(address, asset) .await @@ -236,16 +252,9 @@ pub(crate) trait StateWriteExt: StateWrite { impl StateWriteExt for T {} #[cfg(test)] -mod test { +mod tests { use astria_core::{ - primitive::v1::{ - asset::{ - default_native_asset, - Id, - DEFAULT_NATIVE_ASSET_DENOM, - }, - Address, - }, + primitive::v1::Address, protocol::account::v1alpha1::AssetBalance, }; use cnidarium::StateDelta; @@ -263,6 +272,17 @@ mod test { asset, }; + 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() + } + #[tokio::test] async fn get_account_nonce_uninitialized_returns_zero() { let storage = cnidarium::TempStorage::new().await.unwrap(); @@ -378,7 +398,7 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount_expected = 0u128; // non-initialized accounts return zero @@ -400,17 +420,17 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let mut amount_expected = 1u128; state - .put_account_balance(address, asset, amount_expected) + .put_account_balance(address, &asset, amount_expected) .expect("putting an account balance should not fail"); // can initialize assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -421,12 +441,12 @@ mod test { amount_expected = 2u128; state - .put_account_balance(address, asset, amount_expected) + .put_account_balance(address, &asset, amount_expected) .expect("putting an asset balance for an account should not fail"); assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -442,17 +462,17 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount_expected = 1u128; state - .put_account_balance(address, asset, amount_expected) + .put_account_balance(address, &asset, amount_expected) .expect("putting an account balance should not fail"); // able to write to account's storage assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -465,11 +485,11 @@ mod test { let amount_expected_1 = 2u128; state - .put_account_balance(address_1, asset, amount_expected_1) + .put_account_balance(address_1, &asset, amount_expected_1) .expect("putting an account balance should not fail"); assert_eq!( state - .get_account_balance(address_1, asset) + .get_account_balance(address_1, &asset) .await .expect("getting an asset balance should not fail"), amount_expected_1, @@ -478,7 +498,7 @@ mod test { ); assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), amount_expected, @@ -495,22 +515,22 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset_0 = Id::from_str_unchecked("asset_0"); - let asset_1 = Id::from_str_unchecked("asset_1"); + let asset_0 = asset_0(); + let asset_1 = asset_1(); let amount_expected_0 = 1u128; let amount_expected_1 = 2u128; state - .put_account_balance(address, asset_0, amount_expected_0) + .put_account_balance(address, &asset_0, amount_expected_0) .expect("putting an account balance should not fail"); state - .put_account_balance(address, asset_1, amount_expected_1) + .put_account_balance(address, &asset_1, amount_expected_1) .expect("putting an account balance should not fail"); // wrote correct balances assert_eq!( state - .get_account_balance(address, asset_0) + .get_account_balance(address, &asset_0) .await .expect("getting an asset balance should not fail"), amount_expected_0, @@ -518,7 +538,7 @@ mod test { ); assert_eq!( state - .get_account_balance(address, asset_1) + .get_account_balance(address, &asset_1) .await .expect("getting an asset balance should not fail"), amount_expected_1, @@ -550,29 +570,26 @@ mod test { let mut state = StateDelta::new(snapshot); // need to set native asset in order to use `get_account_balances()` - crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); + crate::asset::initialize_native_asset("nria"); - let asset_0 = Id::from_str_unchecked(DEFAULT_NATIVE_ASSET_DENOM); - let asset_1 = Id::from_str_unchecked("asset_1"); - let asset_2 = Id::from_str_unchecked("asset_2"); + let asset_0 = crate::asset::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, - &default_native_asset().unwrap_trace_prefixed(), + &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, - &"asset_1".parse().unwrap(), + &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, - &"asset_2".parse().unwrap(), + &asset_2.clone().unwrap_trace_prefixed(), ) .expect("should be able to call other trait method on state object"); @@ -584,13 +601,17 @@ mod test { // add balances to the account state - .put_account_balance(address, asset_0, amount_expected_0) + .put_account_balance( + address, + asset_0.clone().unwrap_trace_prefixed(), + amount_expected_0, + ) .expect("putting an account balance should not fail"); state - .put_account_balance(address, asset_1, amount_expected_1) + .put_account_balance(address, &asset_1, amount_expected_1) .expect("putting an account balance should not fail"); state - .put_account_balance(address, asset_2, amount_expected_2) + .put_account_balance(address, &asset_2, amount_expected_2) .expect("putting an account balance should not fail"); let mut balances = state @@ -602,15 +623,15 @@ mod test { balances, vec![ AssetBalance { - denom: default_native_asset(), + denom: asset_0.clone(), balance: amount_expected_0, }, AssetBalance { - denom: "asset_1".parse().unwrap(), + denom: asset_1.clone(), balance: amount_expected_1, }, AssetBalance { - denom: "asset_2".parse().unwrap(), + denom: asset_2.clone(), balance: amount_expected_2, }, ] @@ -625,18 +646,18 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount_increase = 2u128; state - .increase_balance(address, asset, amount_increase) + .increase_balance(address, &asset, amount_increase) .await .expect("increasing account balance for uninitialized account should be ok"); // correct balance was set assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), amount_increase, @@ -644,7 +665,7 @@ mod test { ); state - .increase_balance(address, asset, amount_increase) + .increase_balance(address, &asset, amount_increase) .await .expect("increasing account balance for initialized account should be ok"); @@ -666,18 +687,18 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount_increase = 2u128; state - .increase_balance(address, asset, amount_increase) + .increase_balance(address, &asset, amount_increase) .await .expect("increasing account balance for uninitialized account should be ok"); // correct balance was set assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), amount_increase, @@ -686,13 +707,13 @@ mod test { // decrease balance state - .decrease_balance(address, asset, amount_increase) + .decrease_balance(address, &asset, amount_increase) .await .expect("decreasing account balance for initialized account should be ok"); assert_eq!( state - .get_account_balance(address, asset) + .get_account_balance(address, &asset) .await .expect("getting an asset balance should not fail"), 0, @@ -708,18 +729,18 @@ mod test { // create needed variables let address = crate::address::base_prefixed([42u8; 20]); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount_increase = 2u128; // give initial balance state - .increase_balance(address, asset, amount_increase) + .increase_balance(address, &asset, amount_increase) .await .expect("increasing account balance for uninitialized account should be ok"); // decrease balance state - .decrease_balance(address, asset, amount_increase + 1) + .decrease_balance(address, &asset, amount_increase + 1) .await .expect_err("should not be able to subtract larger balance than what existed"); } @@ -729,13 +750,14 @@ mod test { let address: Address = "astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm" .parse() .unwrap(); - let mut next = 0; - let id = astria_core::primitive::v1::asset::Id::new([0u8; 32].map(|_| { - let this = next; - next += 1; - this - })); - assert_snapshot!(balance_storage_key(address, id)); + let asset = "an/asset/with/a/prefix" + .parse::() + .unwrap(); + assert_eq!( + balance_storage_key(address, &asset), + balance_storage_key(address, asset.to_ibc_prefixed()) + ); + assert_snapshot!(balance_storage_key(address, asset)); assert_snapshot!(nonce_storage_key(address)); } } diff --git a/crates/astria-sequencer/src/api_state_ext.rs b/crates/astria-sequencer/src/api_state_ext.rs index a90332f77f..79c8c1512e 100644 --- a/crates/astria-sequencer/src/api_state_ext.rs +++ b/crates/astria-sequencer/src/api_state_ext.rs @@ -383,7 +383,6 @@ impl StateWriteExt for T {} #[cfg(test)] mod test { use astria_core::{ - primitive::v1::asset::Id, protocol::test_utils::ConfigureSequencerBlock, sequencerblock::v1alpha1::block::Deposit, }; @@ -403,13 +402,13 @@ mod test { let rollup_id = RollupId::new(rng.gen()); let bridge_address = crate::address::base_prefixed([rng.gen(); 20]); let amount = rng.gen::(); - let asset_id = Id::from_str_unchecked(&rng.gen::().to_string()); + let asset = "testasset".parse().unwrap(); let destination_chain_address = rng.gen::().to_string(); let deposit = Deposit::new( bridge_address, rollup_id, amount, - asset_id, + asset, destination_chain_address, ); deposits.push(deposit); diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index eeb53963f2..9ecdf35efc 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -226,7 +226,7 @@ impl App { state_tx.put_block_height(0); for fee_asset in &genesis_state.allowed_fee_assets { - state_tx.put_allowed_fee_asset(fee_asset.id()); + state_tx.put_allowed_fee_asset(fee_asset); } // call init_chain on all components diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap index 8a19543be3..edafad96cf 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 236, - 73, - 13, - 196, - 227, + 220, + 23, + 112, + 98, + 180, + 104, + 39, + 254, + 107, + 65, + 159, + 49, + 59, + 7, + 177, + 110, + 184, 141, - 8, - 200, + 73, + 165, 204, - 104, - 101, - 173, - 226, - 22, - 155, - 40, + 144, 102, - 150, - 205, - 99, - 31, - 146, - 109, - 130, - 38, - 208, - 88, - 107, - 26, - 152, - 22, - 74 + 28, + 247, + 177, + 145, + 116, + 114, + 230, + 27, + 186 ] diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap index 6d73e94390..10baa0c334 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 183, - 90, - 240, - 218, - 162, - 11, - 38, + 168, + 40, + 26, + 85, + 142, + 62, + 98, + 97, + 65, + 205, + 249, + 210, + 237, + 174, + 132, + 98, + 101, + 27, + 175, + 0, + 25, + 191, + 31, 107, - 135, - 22, - 241, - 45, - 229, - 255, - 59, - 198, - 247, - 33, - 39, - 108, - 41, - 22, - 106, - 29, + 175, + 139, 95, - 25, - 90, - 1, - 120, - 134, - 130, - 94 + 221, + 223, + 122, + 255, + 48 ] diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 5f972b359f..3979cbf7f2 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::{ - asset::{ - default_native_asset, - DEFAULT_NATIVE_ASSET_DENOM, - }, Address, RollupId, ADDRESS_LEN, @@ -107,9 +103,9 @@ pub(crate) fn unchecked_genesis_state() -> UncheckedGenesisState { authority_sudo_address: address_from_hex_string(JUDY_ADDRESS), ibc_sudo_address: address_from_hex_string(TED_ADDRESS), ibc_relayer_addresses: vec![], - native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), + native_asset_base_denomination: "nria".to_string(), ibc_params: IBCParameters::default(), - allowed_fee_assets: vec![default_native_asset()], + allowed_fee_assets: vec!["nria".parse().unwrap()], fees: default_fees(), } } @@ -164,7 +160,7 @@ pub(crate) fn get_mock_tx(nonce: u32) -> SignedTransaction { SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), data: vec![0x99], - fee_asset_id: astria_core::primitive::v1::asset::default_native_asset().id(), + fee_asset: "astria".parse().unwrap(), } .into(), ], diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index 5884c22654..9fdd48f880 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -1,10 +1,7 @@ use std::collections::HashMap; use astria_core::{ - primitive::v1::{ - asset::DEFAULT_NATIVE_ASSET_DENOM, - RollupId, - }, + primitive::v1::RollupId, protocol::transaction::v1alpha1::{ action::{ BridgeLockAction, @@ -90,16 +87,13 @@ async fn app_genesis_and_init_chain() { assert_eq!( balance, app.state - .get_account_balance(address, get_native_asset().id()) + .get_account_balance(address, get_native_asset()) .await .unwrap(), ); } - assert_eq!( - app.state.get_native_asset_denom().await.unwrap(), - DEFAULT_NATIVE_ASSET_DENOM - ); + assert_eq!(app.state.get_native_asset_denom().await.unwrap(), "nria"); } #[tokio::test] @@ -188,7 +182,7 @@ 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().id(); + let native_asset = get_native_asset(); for Account { address, balance, @@ -230,7 +224,7 @@ 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().id(); + let native_asset = get_native_asset().clone(); // transfer funds from Alice to Bob; use native token for fee payment let bob_address = address_from_hex_string(BOB_ADDRESS); @@ -244,8 +238,8 @@ async fn app_transfer_block_fees_to_sudo() { TransferAction { to: bob_address, amount, - asset_id: native_asset, - fee_asset_id: get_native_asset().id(), + asset: native_asset.clone(), + fee_asset: native_asset.clone(), } .into(), ], @@ -301,12 +295,12 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { let bridge_address = crate::address::base_prefixed([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + 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_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); app.apply(state_tx); app.prepare_commit(storage.clone()).await.unwrap(); @@ -316,14 +310,14 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { let lock_action = BridgeLockAction { to: bridge_address, amount, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), }; let sequence_action = SequenceAction { rollup_id, data: b"hello world".to_vec(), - fee_asset_id: asset_id, + fee_asset: asset.clone(), }; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -339,7 +333,7 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { bridge_address, rollup_id, amount, - asset_id, + asset, "nootwashere".to_string(), ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); @@ -379,7 +373,7 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { } } assert_eq!(deposits.len(), 1); - assert_eq!(deposits[0], expected_deposit); + assert_eq!(*deposits[0], expected_deposit); } // it's a test, so allow a lot of lines @@ -391,12 +385,12 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { let bridge_address = crate::address::base_prefixed([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + 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_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); app.apply(state_tx); app.prepare_commit(storage.clone()).await.unwrap(); @@ -406,14 +400,14 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { let lock_action = BridgeLockAction { to: bridge_address, amount, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), }; let sequence_action = SequenceAction { rollup_id, data: b"hello world".to_vec(), - fee_asset_id: asset_id, + fee_asset: asset.clone(), }; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -429,7 +423,7 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { bridge_address, rollup_id, amount, - asset_id, + asset, "nootwashere".to_string(), ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); @@ -548,7 +542,7 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 100_000], - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -563,7 +557,7 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 100_000], - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -621,7 +615,7 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 200_000], - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -636,7 +630,7 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { SequenceAction { rollup_id: RollupId::from([1u8; 32]), data: vec![1u8; 100_000], - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 72bbd68b5a..1924f257ec 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -15,13 +15,7 @@ use std::{ }; use astria_core::{ - primitive::v1::{ - asset::{ - default_native_asset, - DEFAULT_NATIVE_ASSET_DENOM, - }, - RollupId, - }, + primitive::v1::RollupId, protocol::transaction::v1alpha1::{ action::{ BridgeLockAction, @@ -81,9 +75,9 @@ fn unchecked_genesis_state() -> UncheckedGenesisState { authority_sudo_address: alice_address, ibc_sudo_address: alice_address, ibc_relayer_addresses: vec![], - native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), + native_asset_base_denomination: "nria".to_string(), ibc_params: IBCParameters::default(), - allowed_fee_assets: vec![default_native_asset()], + allowed_fee_assets: vec!["nria".parse().unwrap()], fees: default_fees(), } } @@ -101,12 +95,12 @@ async fn app_finalize_block_snapshot() { let bridge_address = crate::address::base_prefixed([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + 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_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); app.apply(state_tx); @@ -119,14 +113,14 @@ async fn app_finalize_block_snapshot() { let lock_action = BridgeLockAction { to: bridge_address, amount, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), }; let sequence_action = SequenceAction { rollup_id, data: b"hello world".to_vec(), - fee_asset_id: asset_id, + fee_asset: asset.clone(), }; let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -142,7 +136,7 @@ async fn app_finalize_block_snapshot() { bridge_address, rollup_id, amount, - asset_id, + asset, "nootwashere".to_string(), ); let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]); @@ -178,13 +172,10 @@ async fn app_finalize_block_snapshot() { #[allow(clippy::too_many_lines)] #[tokio::test] async fn app_execute_transaction_with_every_action_snapshot() { - use astria_core::{ - primitive::v1::asset, - protocol::transaction::v1alpha1::action::{ - FeeAssetChangeAction, - InitBridgeAccountAction, - SudoAddressChangeAction, - }, + use astria_core::protocol::transaction::v1alpha1::action::{ + FeeAssetChangeAction, + InitBridgeAccountAction, + SudoAddressChangeAction, }; use crate::genesis::Account; @@ -215,7 +206,7 @@ async fn app_execute_transaction_with_every_action_snapshot() { }; let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + let asset = get_native_asset().clone(); let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -226,14 +217,14 @@ async fn app_execute_transaction_with_every_action_snapshot() { TransferAction { to: bob_address, amount: 333_333, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), } .into(), SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"hello world".to_vec(), - fee_asset_id: asset_id, + fee_asset: asset.clone(), } .into(), Action::ValidatorUpdate(update.clone()), @@ -241,9 +232,9 @@ async fn app_execute_transaction_with_every_action_snapshot() { IbcRelayerChangeAction::Addition(carol_address).into(), IbcRelayerChangeAction::Removal(bob_address).into(), // TODO: should fee assets be stored in state? - FeeAssetChangeAction::Addition(asset::Id::from_str_unchecked("test-0")).into(), - FeeAssetChangeAction::Addition(asset::Id::from_str_unchecked("test-1")).into(), - FeeAssetChangeAction::Removal(asset::Id::from_str_unchecked("test-0")).into(), + FeeAssetChangeAction::Addition("test-0".parse().unwrap()).into(), + FeeAssetChangeAction::Addition("test-1".parse().unwrap()).into(), + FeeAssetChangeAction::Removal("test-0".parse().unwrap()).into(), SudoAddressChangeAction { new_address: bob_address, } @@ -262,8 +253,8 @@ async fn app_execute_transaction_with_every_action_snapshot() { actions: vec![ InitBridgeAccountAction { rollup_id, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), sudo_address: None, withdrawer_address: None, } @@ -282,15 +273,15 @@ async fn app_execute_transaction_with_every_action_snapshot() { BridgeLockAction { to: bridge_address, amount: 100, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), } .into(), BridgeUnlockAction { to: bob_address, amount: 10, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: vec![0u8; 32], bridge_address: None, } @@ -299,7 +290,7 @@ 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_id: asset_id, + fee_asset: asset.clone(), } .into(), ], diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 87fbcb77df..35911de417 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -3,12 +3,7 @@ use std::sync::Arc; use astria_core::{ crypto::SigningKey, primitive::v1::{ - asset::{ - self, - default_native_asset, - Denom, - DEFAULT_NATIVE_ASSET_DENOM, - }, + asset, RollupId, }, protocol::transaction::v1alpha1::{ @@ -52,6 +47,11 @@ use crate::{ }, }; +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 { @@ -75,6 +75,10 @@ fn genesis_state() -> GenesisState { unchecked_genesis_state().try_into().unwrap() } +fn test_asset() -> asset::Denom { + "test".parse().unwrap() +} + #[tokio::test] async fn app_execute_transaction_transfer() { let mut app = initialize_app(None, vec![]).await; @@ -92,8 +96,8 @@ async fn app_execute_transaction_transfer() { TransferAction { to: bob_address, amount: value, - asset_id: get_native_asset().id(), - fee_asset_id: get_native_asset().id(), + asset: get_native_asset().clone(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -102,7 +106,7 @@ async fn app_execute_transaction_transfer() { let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); app.execute_transaction(signed_tx).await.unwrap(); - let native_asset = get_native_asset().id(); + let native_asset = get_native_asset(); assert_eq!( app.state .get_account_balance(bob_address, native_asset) @@ -129,12 +133,12 @@ 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 = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let value = 333_333; let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); let mut state_tx = StateDelta::new(app.state.clone()); state_tx - .put_account_balance(alice_address, asset, value) + .put_account_balance(alice_address, &asset, value) .unwrap(); app.apply(state_tx); @@ -149,8 +153,8 @@ async fn app_execute_transaction_transfer_not_native_token() { TransferAction { to: bob_address, amount: value, - asset_id: asset, - fee_asset_id: get_native_asset().id(), + asset: asset.clone(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -159,7 +163,7 @@ async fn app_execute_transaction_transfer_not_native_token() { let signed_tx = Arc::new(tx.into_signed(&alice_signing_key)); app.execute_transaction(signed_tx).await.unwrap(); - let native_asset = get_native_asset().id(); + let native_asset = get_native_asset(); assert_eq!( app.state .get_account_balance(bob_address, native_asset) @@ -169,7 +173,7 @@ async fn app_execute_transaction_transfer_not_native_token() { ); assert_eq!( app.state - .get_account_balance(bob_address, asset) + .get_account_balance(bob_address, &asset) .await .unwrap(), value, // transferred amount @@ -185,7 +189,7 @@ async fn app_execute_transaction_transfer_not_native_token() { ); assert_eq!( app.state - .get_account_balance(alice_address, asset) + .get_account_balance(alice_address, &asset) .await .unwrap(), 0, // 0 since all funds of `asset` were transferred @@ -215,8 +219,8 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { TransferAction { to: bob, amount: 0, - asset_id: get_native_asset().id(), - fee_asset_id: get_native_asset().id(), + asset: get_native_asset().clone(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -255,7 +259,7 @@ async fn app_execute_transaction_sequence() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -267,7 +271,7 @@ async fn app_execute_transaction_sequence() { assert_eq!( app.state - .get_account_balance(alice_address, get_native_asset().id()) + .get_account_balance(alice_address, get_native_asset()) .await .unwrap(), 10u128.pow(19) - fee, @@ -281,7 +285,7 @@ async fn app_execute_transaction_invalid_fee_asset() { let (alice_signing_key, _) = get_alice_signing_key_and_address(); let data = b"hello world".to_vec(); - let fee_asset_id = asset::Id::from_str_unchecked("test"); + let fee_asset = test_asset(); let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -292,7 +296,7 @@ async fn app_execute_transaction_invalid_fee_asset() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset_id, + fee_asset, } .into(), ], @@ -470,7 +474,7 @@ async fn app_execute_transaction_fee_asset_change_addition() { let mut app = initialize_app(Some(genesis_state()), vec![]).await; - let new_asset = asset::Id::from_str_unchecked("test"); + let new_asset = test_asset(); let tx = UnsignedTransaction { params: TransactionParams::builder() @@ -478,7 +482,7 @@ async fn app_execute_transaction_fee_asset_change_addition() { .chain_id("test") .build(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Addition( - new_asset, + new_asset.clone(), ))], }; @@ -486,7 +490,7 @@ async fn app_execute_transaction_fee_asset_change_addition() { 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(&new_asset).await.unwrap()); } #[tokio::test] @@ -494,7 +498,7 @@ 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".parse::().unwrap(); + let test_asset = test_asset(); let genesis_state = UncheckedGenesisState { allowed_fee_assets: vec![default_native_asset(), test_asset.clone()], @@ -510,7 +514,7 @@ async fn app_execute_transaction_fee_asset_change_removal() { .chain_id("test") .build(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( - test_asset.id(), + test_asset.clone(), ))], }; @@ -518,12 +522,7 @@ async fn app_execute_transaction_fee_asset_change_removal() { 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.id()) - .await - .unwrap() - ); + assert!(!app.state.is_allowed_fee_asset(&test_asset).await.unwrap()); } #[tokio::test] @@ -540,7 +539,7 @@ async fn app_execute_transaction_fee_asset_change_invalid() { .chain_id("test") .build(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( - get_native_asset().id(), + get_native_asset().clone(), ))], }; @@ -566,11 +565,11 @@ async fn app_execute_transaction_init_bridge_account_ok() { app.apply(state_tx); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + let asset = get_native_asset().clone(); let action = InitBridgeAccountAction { rollup_id, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), sudo_address: None, withdrawer_address: None, }; @@ -586,7 +585,7 @@ async fn app_execute_transaction_init_bridge_account_ok() { let before_balance = app .state - .get_account_balance(alice_address, asset_id) + .get_account_balance(alice_address, &asset) .await .unwrap(); app.execute_transaction(signed_tx).await.unwrap(); @@ -601,14 +600,14 @@ async fn app_execute_transaction_init_bridge_account_ok() { ); assert_eq!( app.state - .get_bridge_account_asset_id(&alice_address) + .get_bridge_account_ibc_asset(&alice_address) .await .unwrap(), - asset_id + asset.to_ibc_prefixed(), ); assert_eq!( app.state - .get_account_balance(alice_address, asset_id) + .get_account_balance(alice_address, &asset) .await .unwrap(), before_balance - fee, @@ -623,11 +622,11 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( let mut app = initialize_app(None, vec![]).await; let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + let asset = get_native_asset(); let action = InitBridgeAccountAction { rollup_id, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), sudo_address: None, withdrawer_address: None, }; @@ -645,8 +644,8 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( let action = InitBridgeAccountAction { rollup_id, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), sudo_address: None, withdrawer_address: None, }; @@ -669,12 +668,12 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let bridge_address = crate::address::base_prefixed([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + 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_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); app.apply(state_tx); @@ -682,8 +681,8 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let action = BridgeLockAction { to: bridge_address, amount, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { @@ -698,12 +697,12 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let alice_before_balance = app .state - .get_account_balance(alice_address, asset_id) + .get_account_balance(alice_address, &asset) .await .unwrap(); let bridge_before_balance = app .state - .get_account_balance(bridge_address, asset_id) + .get_account_balance(bridge_address, &asset) .await .unwrap(); @@ -714,7 +713,7 @@ async fn app_execute_transaction_bridge_lock_action_ok() { bridge_address, rollup_id, amount, - asset_id, + asset.clone(), "nootwashere".to_string(), ); @@ -727,14 +726,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_id) + .get_account_balance(alice_address, &asset) .await .unwrap(), alice_before_balance - (amount + fee) ); assert_eq!( app.state - .get_account_balance(bridge_address, asset_id) + .get_account_balance(bridge_address, &asset) .await .unwrap(), bridge_before_balance + amount @@ -754,14 +753,14 @@ async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { // don't actually register this address as a bridge address let bridge_address = crate::address::base_prefixed([99; 20]); - let asset_id = get_native_asset().id(); + let asset = get_native_asset().clone(); let amount = 100; let action = BridgeLockAction { to: bridge_address, amount, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { @@ -793,7 +792,7 @@ async fn app_execute_transaction_invalid_nonce() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -806,7 +805,7 @@ async fn app_execute_transaction_invalid_nonce() { assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 0); assert_eq!( app.state - .get_account_balance(alice_address, get_native_asset().id()) + .get_account_balance(alice_address, get_native_asset()) .await .unwrap(), 10u128.pow(19), @@ -839,7 +838,7 @@ async fn app_execute_transaction_invalid_chain_id() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -852,7 +851,7 @@ async fn app_execute_transaction_invalid_chain_id() { assert_eq!(app.state.get_account_nonce(alice_address).await.unwrap(), 0); assert_eq!( app.state - .get_account_balance(alice_address, get_native_asset().id()) + .get_account_balance(alice_address, get_native_asset()) .await .unwrap(), 10u128.pow(19), @@ -898,8 +897,8 @@ async fn app_stateful_check_fails_insufficient_total_balance() { TransferAction { to: keypair_address, amount: fee, - asset_id: get_native_asset().id(), - fee_asset_id: get_native_asset().id(), + asset: get_native_asset().clone(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -919,13 +918,13 @@ async fn app_stateful_check_fails_insufficient_total_balance() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: data.clone(), - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: data.clone(), - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -950,7 +949,7 @@ async fn app_stateful_check_fails_insufficient_total_balance() { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data, - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -972,19 +971,19 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { let (bridge_signing_key, bridge_address) = get_bridge_signing_key_and_address(); let rollup_id: RollupId = RollupId::from_unhashed_bytes(b"testchainid"); - let asset_id = get_native_asset().id(); + 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_id, transfer_fee) + .put_account_balance(bridge_address, asset, transfer_fee) .unwrap(); // create bridge account state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, asset) .unwrap(); state_tx.put_bridge_account_withdrawer_address(&bridge_address, &bridge_address); app.apply(state_tx); @@ -993,8 +992,8 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { let action = BridgeLockAction { to: bridge_address, amount, - asset_id, - fee_asset_id: asset_id, + asset: asset.clone(), + fee_asset: asset.clone(), destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { @@ -1014,7 +1013,7 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { let action = BridgeUnlockAction { to: alice_address, amount, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: b"lilywashere".to_vec(), bridge_address: None, }; @@ -1033,7 +1032,7 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { .expect("executing bridge unlock action should succeed"); assert_eq!( app.state - .get_account_balance(bridge_address, asset_id) + .get_account_balance(bridge_address, asset) .await .expect("executing bridge unlock action should succeed"), 0, diff --git a/crates/astria-sequencer/src/asset/mod.rs b/crates/astria-sequencer/src/asset/mod.rs index f8f728522e..082844c10f 100644 --- a/crates/astria-sequencer/src/asset/mod.rs +++ b/crates/astria-sequencer/src/asset/mod.rs @@ -4,23 +4,51 @@ pub(crate) 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) static NATIVE_ASSET: OnceLock = OnceLock::new(); -pub(crate) fn initialize_native_asset(native_asset: &str) { - if NATIVE_ASSET.get().is_some() { - tracing::error!("native asset should only be set once"); - return; +#[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"); } - let denom = native_asset - .parse::() - .expect("being unable to parse the native asset breaks sequencer"); - 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") + } } -pub(crate) fn get_native_asset() -> &'static Denom { - 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/asset/query.rs b/crates/astria-sequencer/src/asset/query.rs index 51c893c028..4e16bdfa46 100644 --- a/crates/astria-sequencer/src/asset/query.rs +++ b/crates/astria-sequencer/src/asset/query.rs @@ -3,10 +3,11 @@ use astria_core::{ primitive::v1::asset, protocol::{ abci::AbciErrorCode, - asset::v1alpha1::AllowedFeeAssetIdsResponse, + asset::v1alpha1::AllowedFeeAssetsResponse, }, }; use cnidarium::Storage; +use hex::FromHex as _; use prost::Message as _; use tendermint::abci::{ request, @@ -15,7 +16,7 @@ use tendermint::abci::{ use crate::{ asset::state_ext::StateReadExt as _, - state_ext::StateReadExt, + state_ext::StateReadExt as _, }; // Retrieve the full asset denomination given the asset ID. @@ -31,8 +32,8 @@ pub(crate) async fn denom_request( // use the latest snapshot, as this is a lookup of id->denom let snapshot = storage.latest_snapshot(); - let asset_id = match preprocess_request(¶ms) { - Ok(asset_id) => asset_id, + let asset = match preprocess_request(¶ms) { + Ok(asset) => asset, Err(err_rsp) => return err_rsp, }; @@ -48,13 +49,13 @@ pub(crate) async fn denom_request( } }; - let maybe_denom = match snapshot.get_ibc_asset(asset_id).await { + let maybe_denom = match snapshot.map_ibc_to_trace_prefixed_asset(asset).await { Ok(maybe_denom) => maybe_denom, Err(err) => { return response::Query { code: AbciErrorCode::INTERNAL_ERROR.into(), info: AbciErrorCode::INTERNAL_ERROR.to_string(), - log: format!("failed to retrieve denomination `{asset_id}`: {err:#}"), + log: format!("failed to retrieve denomination `{asset}`: {err:#}"), ..response::Query::default() }; } @@ -64,7 +65,7 @@ pub(crate) async fn denom_request( return response::Query { code: AbciErrorCode::VALUE_NOT_FOUND.into(), info: AbciErrorCode::VALUE_NOT_FOUND.to_string(), - log: format!("failed to retrieve value for denomination ID`{asset_id}`"), + log: format!("failed to retrieve value for denomination ID`{asset}`"), ..response::Query::default() }; }; @@ -87,7 +88,9 @@ pub(crate) async fn denom_request( } } -fn preprocess_request(params: &[(String, String)]) -> anyhow::Result { +fn preprocess_request( + params: &[(String, String)], +) -> anyhow::Result { let Some(asset_id) = params.iter().find_map(|(k, v)| (k == "id").then_some(v)) else { return Err(response::Query { code: AbciErrorCode::INVALID_PARAMETER.into(), @@ -96,18 +99,16 @@ fn preprocess_request(params: &[(String, String)]) -> anyhow::Result::from_hex(asset_id) .context("failed decoding hex encoded bytes") - .and_then(|addr| { - asset::Id::try_from_slice(&addr).context("failed constructing asset ID from bytes") - }) + .map(asset::IbcPrefixed::new) .map_err(|err| response::Query { code: AbciErrorCode::INVALID_PARAMETER.into(), info: AbciErrorCode::INVALID_PARAMETER.to_string(), log: format!("asset ID could not be constructed from provided parameter: {err:#}"), ..response::Query::default() })?; - Ok(asset_id) + Ok(asset) } pub(crate) async fn allowed_fee_asset_ids_request( @@ -132,8 +133,8 @@ pub(crate) async fn allowed_fee_asset_ids_request( }; // get ids from snapshot at height - let fee_asset_ids = match snapshot.get_allowed_fee_assets().await { - Ok(fee_asset_ids) => fee_asset_ids, + let fee_assets = match snapshot.get_allowed_fee_assets().await { + Ok(fee_assets) => fee_assets, Err(err) => { return response::Query { code: AbciErrorCode::INTERNAL_ERROR.into(), @@ -144,9 +145,9 @@ pub(crate) async fn allowed_fee_asset_ids_request( } }; - let payload = AllowedFeeAssetIdsResponse { + let payload = AllowedFeeAssetsResponse { height, - fee_asset_ids, + fee_assets: fee_assets.into_iter().map(Into::into).collect(), } .into_raw() .encode_to_vec() 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/asset/snapshots/astria_sequencer__asset__state_ext__tests__storage_keys_are_unchanged.snap new file mode 100644 index 0000000000..67cbf77bca --- /dev/null +++ b/crates/astria-sequencer/src/asset/snapshots/astria_sequencer__asset__state_ext__tests__storage_keys_are_unchanged.snap @@ -0,0 +1,5 @@ +--- +source: crates/astria-sequencer/src/asset/state_ext.rs +expression: asset_storage_key(asset) +--- +asset/be429a02d00837245167a2616674a979a2ac6f9806468b48a975b156ad711320 diff --git a/crates/astria-sequencer/src/asset/state_ext.rs b/crates/astria-sequencer/src/asset/state_ext.rs index a239d65de5..4a40d1c290 100644 --- a/crates/astria-sequencer/src/asset/state_ext.rs +++ b/crates/astria-sequencer/src/asset/state_ext.rs @@ -15,35 +15,37 @@ use cnidarium::{ StateRead, StateWrite, }; -use hex::ToHex as _; 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: asset::Id) -> String { - format!("asset/{}", asset.encode_hex::()) +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(self))] - async fn has_ibc_asset(&self, id: asset::Id) -> Result { - match self - .get_raw(&asset_storage_key(id)) + #[instrument(skip(self, asset), fields(%asset))] + 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")? - { - Some(_) => Ok(true), - None => Ok(false), - } + .is_some()) } #[instrument(skip(self))] - async fn get_ibc_asset(&self, id: asset::Id) -> Result> { + async fn map_ibc_to_trace_prefixed_asset( + &self, + asset: asset::IbcPrefixed, + ) -> Result> { let Some(bytes) = self - .get_raw(&asset_storage_key(id)) + .get_raw(&asset_storage_key(asset)) .await .context("failed reading raw asset from state")? else { @@ -64,10 +66,10 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip(self))] - fn put_ibc_asset(&mut self, id: asset::Id, asset: &denom::TracePrefixed) -> Result<()> { + 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(id), bytes); + self.put_raw(asset_storage_key(asset), bytes); Ok(()) } } @@ -75,31 +77,38 @@ pub(crate) trait StateWriteExt: StateWrite { impl StateWriteExt for T {} #[cfg(test)] -mod test { - use astria_core::primitive::v1::asset::{ - denom::TracePrefixed, - Denom, - Id, - }; +mod tests { + use astria_core::primitive::v1::asset; use cnidarium::StateDelta; use super::{ - StateReadExt as _, + 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".parse::().unwrap().id(); + let asset = asset(); // gets for non existing assets should return none assert_eq!( state - .get_ibc_asset(asset) + .map_ibc_to_trace_prefixed_asset(asset.to_ibc_prefixed()) .await .expect("getting non existing asset should not fail"), None @@ -112,25 +121,25 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let denom = "asset".parse::().unwrap(); + let denom = asset(); // non existing calls are ok for 'has' assert!( !state - .has_ibc_asset(denom.id()) + .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.id(), &denom) + .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.id()) + .has_ibc_asset(&denom) .await .expect("'has' for existing ibc assets should be ok"), "query for existing asset should return true" @@ -144,17 +153,17 @@ mod test { let mut state = StateDelta::new(snapshot); // can write new - let denom = "asset".parse::().unwrap(); + let denom = asset(); state - .put_ibc_asset(denom.id(), &denom) + .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); assert_eq!( state - .get_ibc_asset(denom.id()) + .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, + denom.unwrap_trace_prefixed(), "stored ibc asset was not what was expected" ); } @@ -166,68 +175,54 @@ mod test { let mut state = StateDelta::new(snapshot); // can write new - let denom = "asset_0".parse::().unwrap(); + let denom = asset_0(); state - .put_ibc_asset(denom.id(), &denom) + .put_ibc_asset(&denom.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); assert_eq!( state - .get_ibc_asset(denom.id()) + .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, + denom.clone().unwrap_trace_prefixed(), "stored ibc asset was not what was expected" ); // can write another without affecting original - let denom_1 = "asset_1".parse::().unwrap(); + let denom_1 = asset_1(); state - .put_ibc_asset(denom_1.id(), &denom_1) + .put_ibc_asset(&denom_1.clone().unwrap_trace_prefixed()) .expect("putting ibc asset should not fail"); assert_eq!( state - .get_ibc_asset(denom_1.id()) + .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, + denom_1.unwrap_trace_prefixed(), "additional ibc asset was not what was expected" ); assert_eq!( state - .get_ibc_asset(denom.id()) + .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, + denom.clone().unwrap_trace_prefixed(), "original ibc asset was not what was expected" ); } - #[tokio::test] - async fn put_ibc_asset_can_write_unrelated_ids_to_denoms() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - // can write unrelated ids and denoms - let id_key = Id::from_str_unchecked("asset_0"); - let denom = "asset_1".parse::().unwrap(); - state - .put_ibc_asset(id_key, &denom) - .expect("putting ibc asset should not fail"); - - // see that id key and denom's stored id differ - assert_ne!( - state - .get_ibc_asset(id_key) - .await - .unwrap() - .expect("an ibc asset was written and must exist inside the database") - .id(), - id_key, - "stored 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/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index 65ed87ef9e..071f2f6e17 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -47,9 +47,9 @@ impl ActionHandler for BridgeLockAction { ) -> Result<()> { let transfer_action = TransferAction { to: self.to, - asset_id: self.asset_id, + asset: self.asset.clone(), amount: self.amount, - fee_asset_id: self.fee_asset_id, + fee_asset: self.fee_asset.clone(), }; // ensure the recipient is a bridge account. @@ -59,17 +59,17 @@ impl ActionHandler for BridgeLockAction { .context("failed to get bridge account rollup id")? .ok_or_else(|| anyhow::anyhow!("bridge lock must be sent to a bridge account"))?; - let allowed_asset_id = state - .get_bridge_account_asset_id(&self.to) + let allowed_asset = state + .get_bridge_account_ibc_asset(&self.to) .await .context("failed to get bridge account asset ID")?; ensure!( - allowed_asset_id == self.asset_id, + allowed_asset == self.asset.to_ibc_prefixed(), "asset ID is not authorized for transfer to bridge account", ); let from_balance = state - .get_account_balance(from, self.fee_asset_id) + .get_account_balance(from, &self.fee_asset) .await .context("failed to get sender account balance")?; let transfer_fee = state @@ -81,7 +81,7 @@ impl ActionHandler for BridgeLockAction { self.to, rollup_id, self.amount, - self.asset_id, + self.asset.clone(), self.destination_chain_address.clone(), ); @@ -102,9 +102,9 @@ impl ActionHandler for BridgeLockAction { async fn execute(&self, state: &mut S, from: Address) -> Result<()> { let transfer_action = TransferAction { to: self.to, - asset_id: self.asset_id, + asset: self.asset.clone(), amount: self.amount, - fee_asset_id: self.fee_asset_id, + fee_asset: self.fee_asset.clone(), }; transfer_action @@ -122,7 +122,7 @@ impl ActionHandler for BridgeLockAction { self.to, rollup_id, self.amount, - self.asset_id, + self.asset.clone(), self.destination_chain_address.clone(), ); @@ -135,7 +135,7 @@ impl ActionHandler for BridgeLockAction { let fee = byte_cost_multiplier.saturating_mul(get_deposit_byte_len(&deposit)); state - .decrease_balance(from, self.fee_asset_id, fee) + .decrease_balance(from, &self.fee_asset, fee) .await .context("failed to deduct fee from account balance")?; @@ -155,7 +155,7 @@ pub(crate) fn get_deposit_byte_len(deposit: &Deposit) -> u128 { } #[cfg(test)] -mod test { +mod tests { use astria_core::primitive::v1::{ asset, RollupId, @@ -163,6 +163,14 @@ mod test { use cnidarium::StateDelta; use super::*; + use crate::{ + bridge::state_ext::StateWriteExt, + state_ext::StateWriteExt as _, + }; + + fn test_asset() -> asset::Denom { + "test".parse().unwrap() + } #[tokio::test] async fn bridge_lock_check_stateful_fee_calc() { @@ -174,27 +182,27 @@ mod test { state.put_bridge_lock_byte_cost_multiplier(2); let bridge_address = crate::address::base_prefixed([1; 20]); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let bridge_lock = BridgeLockAction { to: bridge_address, - asset_id, + asset: asset.clone(), amount: 100, - fee_asset_id: asset_id, + fee_asset: asset.clone(), destination_chain_address: "someaddress".to_string(), }; let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); - state.put_allowed_fee_asset(asset_id); + state.put_allowed_fee_asset(&asset); let from_address = crate::address::base_prefixed([2; 20]); // not enough balance; should fail state - .put_account_balance(from_address, asset_id, 100) + .put_account_balance(from_address, &asset, 100) .unwrap(); assert!( bridge_lock @@ -211,11 +219,11 @@ mod test { bridge_address, rollup_id, 100, - asset_id, + asset.clone(), "someaddress".to_string(), )) * 2; state - .put_account_balance(from_address, asset_id, 100 + expected_deposit_fee) + .put_account_balance(from_address, &asset, 100 + expected_deposit_fee) .unwrap(); bridge_lock .check_stateful(&state, from_address) @@ -233,27 +241,27 @@ mod test { state.put_bridge_lock_byte_cost_multiplier(2); let bridge_address = crate::address::base_prefixed([1; 20]); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let bridge_lock = BridgeLockAction { to: bridge_address, - asset_id, + asset: asset.clone(), amount: 100, - fee_asset_id: asset_id, + fee_asset: asset.clone(), destination_chain_address: "someaddress".to_string(), }; let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); - state.put_allowed_fee_asset(asset_id); + state.put_allowed_fee_asset(&asset); let from_address = crate::address::base_prefixed([2; 20]); // not enough balance; should fail state - .put_account_balance(from_address, asset_id, 100 + transfer_fee) + .put_account_balance(from_address, &asset, 100 + transfer_fee) .unwrap(); assert!( bridge_lock @@ -270,11 +278,11 @@ mod test { bridge_address, rollup_id, 100, - asset_id, + asset.clone(), "someaddress".to_string(), )) * 2; state - .put_account_balance(from_address, asset_id, 100 + expected_deposit_fee) + .put_account_balance(from_address, &asset, 100 + expected_deposit_fee) .unwrap(); bridge_lock.execute(&mut state, from_address).await.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 177f81078a..e4945e6069 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs @@ -47,7 +47,7 @@ impl ActionHandler for BridgeSudoChangeAction { ) -> Result<()> { ensure!( state - .is_allowed_fee_asset(self.fee_asset_id) + .is_allowed_fee_asset(&self.fee_asset) .await .context("failed to check allowed fee assets in state")?, "invalid fee asset", @@ -79,7 +79,7 @@ impl ActionHandler for BridgeSudoChangeAction { .await .context("failed to get bridge sudo change fee")?; state - .decrease_balance(self.bridge_address, self.fee_asset_id, fee) + .decrease_balance(self.bridge_address, &self.fee_asset, fee) .await .context("failed to decrease balance for bridge sudo change fee")?; @@ -97,19 +97,23 @@ impl ActionHandler for BridgeSudoChangeAction { #[cfg(test)] mod tests { - use astria_core::primitive::v1::asset::Id; + use astria_core::primitive::v1::asset; use cnidarium::StateDelta; use super::*; + fn test_asset() -> asset::Denom { + "test".parse().unwrap() + } + #[tokio::test] async fn bridge_sudo_change_check_stateless_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = Id::from_str_unchecked("test"); - state.put_allowed_fee_asset(asset_id); + 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]); @@ -119,7 +123,7 @@ mod tests { bridge_address, new_sudo_address: None, new_withdrawer_address: None, - fee_asset_id: asset_id, + fee_asset: asset.clone(), }; action.check_stateful(&state, sudo_address).await.unwrap(); @@ -131,8 +135,8 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = Id::from_str_unchecked("test"); - state.put_allowed_fee_asset(asset_id); + 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]); @@ -142,7 +146,7 @@ mod tests { bridge_address, new_sudo_address: None, new_withdrawer_address: None, - fee_asset_id: asset_id, + fee_asset: asset.clone(), }; assert!( @@ -162,19 +166,19 @@ mod tests { let mut state = StateDelta::new(snapshot); state.put_bridge_sudo_change_base_fee(10); - let fee_asset_id = Id::from_str_unchecked("test"); + 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]); state - .put_account_balance(bridge_address, fee_asset_id, 10) + .put_account_balance(bridge_address, &fee_asset, 10) .unwrap(); let action = BridgeSudoChangeAction { bridge_address, new_sudo_address: Some(new_sudo_address), new_withdrawer_address: Some(new_withdrawer_address), - fee_asset_id, + fee_asset, }; action.execute(&mut state, bridge_address).await.unwrap(); diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index 563f2d37ac..16688d51d5 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -46,8 +46,8 @@ impl ActionHandler for BridgeUnlockAction { let bridge_address = self.bridge_address.unwrap_or(from); // grab the bridge account's asset - let asset_id = state - .get_bridge_account_asset_id(&bridge_address) + let asset = state + .get_bridge_account_ibc_asset(&bridge_address) .await .context("failed to get bridge's asset id, must be a bridge account")?; @@ -67,9 +67,9 @@ impl ActionHandler for BridgeUnlockAction { let transfer_action = TransferAction { to: self.to, - asset_id, + asset: asset.into(), amount: self.amount, - fee_asset_id: self.fee_asset_id, + fee_asset: self.fee_asset.clone(), }; // this performs the same checks as a normal `TransferAction` @@ -81,16 +81,16 @@ impl ActionHandler for BridgeUnlockAction { // the bridge address to withdraw funds from let bridge_address = self.bridge_address.unwrap_or(from); - let asset_id = state - .get_bridge_account_asset_id(&bridge_address) + let asset = state + .get_bridge_account_ibc_asset(&bridge_address) .await .context("failed to get bridge's asset id, must be a bridge account")?; let transfer_action = TransferAction { to: self.to, - asset_id, + asset: asset.into(), amount: self.amount, - fee_asset_id: self.fee_asset_id, + fee_asset: self.fee_asset.clone(), }; transfer_action @@ -117,13 +117,17 @@ mod test { state_ext::StateWriteExt as _, }; + fn test_asset() -> asset::Denom { + "test".parse().unwrap() + } + #[tokio::test] async fn bridge_unlock_fail_non_bridge_accounts() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_amount = 100; let address = crate::address::base_prefixed([1; 20]); @@ -132,7 +136,7 @@ mod test { let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset, memo: vec![0u8; 32], bridge_address: None, }; @@ -154,7 +158,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_amount = 100; let sender_address = crate::address::base_prefixed([1; 20]); @@ -162,14 +166,14 @@ mod test { let bridge_address = crate::address::base_prefixed([3; 20]); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); state.put_bridge_account_withdrawer_address(&bridge_address, &bridge_address); let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: vec![0u8; 32], bridge_address: Some(bridge_address), }; @@ -191,7 +195,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_amount = 100; let sender_address = crate::address::base_prefixed([1; 20]); @@ -201,13 +205,13 @@ mod test { let withdrawer_address = crate::address::base_prefixed([4; 20]); state.put_bridge_account_withdrawer_address(&bridge_address, &withdrawer_address); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset, memo: vec![0u8; 32], bridge_address: Some(bridge_address), }; @@ -229,7 +233,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_fee = 10; let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); @@ -240,22 +244,22 @@ mod test { state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); - state.put_allowed_fee_asset(asset_id); + state.put_allowed_fee_asset(&asset); state.put_bridge_account_withdrawer_address(&bridge_address, &bridge_address); let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: vec![0u8; 32], bridge_address: None, }; // not enough balance to transfer asset; should fail state - .put_account_balance(bridge_address, asset_id, transfer_amount) + .put_account_balance(bridge_address, &asset, transfer_amount) .unwrap(); assert!( bridge_unlock @@ -268,7 +272,7 @@ mod test { // enough balance; should pass state - .put_account_balance(bridge_address, asset_id, transfer_amount + transfer_fee) + .put_account_balance(bridge_address, &asset, transfer_amount + transfer_fee) .unwrap(); bridge_unlock .check_stateful(&state, bridge_address) @@ -282,7 +286,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_fee = 10; let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); @@ -293,9 +297,9 @@ mod test { state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); - state.put_allowed_fee_asset(asset_id); + state.put_allowed_fee_asset(&asset); let withdrawer_address = crate::address::base_prefixed([3; 20]); state.put_bridge_account_withdrawer_address(&bridge_address, &withdrawer_address); @@ -303,14 +307,14 @@ mod test { let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: vec![0u8; 32], bridge_address: Some(bridge_address), }; // not enough balance to transfer asset; should fail state - .put_account_balance(bridge_address, asset_id, transfer_amount) + .put_account_balance(bridge_address, &asset, transfer_amount) .unwrap(); assert!( bridge_unlock @@ -323,7 +327,7 @@ mod test { // enough balance; should pass state - .put_account_balance(bridge_address, asset_id, transfer_amount + transfer_fee) + .put_account_balance(bridge_address, &asset, transfer_amount + transfer_fee) .unwrap(); bridge_unlock .check_stateful(&state, withdrawer_address) @@ -337,7 +341,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_fee = 10; let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); @@ -348,21 +352,21 @@ mod test { state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); - state.put_allowed_fee_asset(asset_id); + state.put_allowed_fee_asset(&asset); let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: vec![0u8; 32], bridge_address: None, }; // not enough balance; should fail state - .put_account_balance(bridge_address, asset_id, transfer_amount) + .put_account_balance(bridge_address, &asset, transfer_amount) .unwrap(); assert!( bridge_unlock @@ -375,7 +379,7 @@ mod test { // enough balance; should pass state - .put_account_balance(bridge_address, asset_id, transfer_amount + transfer_fee) + .put_account_balance(bridge_address, &asset, transfer_amount + transfer_fee) .unwrap(); bridge_unlock .execute(&mut state, bridge_address) @@ -389,7 +393,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let asset_id = asset::Id::from_str_unchecked("test"); + let asset = test_asset(); let transfer_fee = 10; let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); @@ -400,21 +404,21 @@ mod test { state.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state - .put_bridge_account_asset_id(&bridge_address, &asset_id) + .put_bridge_account_ibc_asset(&bridge_address, &asset) .unwrap(); - state.put_allowed_fee_asset(asset_id); + state.put_allowed_fee_asset(&asset); let bridge_unlock = BridgeUnlockAction { to: to_address, amount: transfer_amount, - fee_asset_id: asset_id, + fee_asset: asset.clone(), memo: vec![0u8; 32], bridge_address: Some(bridge_address), }; // not enough balance; should fail state - .put_account_balance(bridge_address, asset_id, transfer_amount) + .put_account_balance(bridge_address, &asset, transfer_amount) .unwrap(); assert!( bridge_unlock @@ -427,7 +431,7 @@ mod test { // enough balance; should pass state - .put_account_balance(bridge_address, asset_id, transfer_amount + transfer_fee) + .put_account_balance(bridge_address, &asset, transfer_amount + transfer_fee) .unwrap(); bridge_unlock .execute(&mut state, bridge_address) 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 9ac76b112a..8c87241809 100644 --- a/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs +++ b/crates/astria-sequencer/src/bridge/init_bridge_account_action.rs @@ -50,7 +50,7 @@ impl ActionHandler for InitBridgeAccountAction { from: Address, ) -> Result<()> { ensure!( - state.is_allowed_fee_asset(self.fee_asset_id).await?, + state.is_allowed_fee_asset(&self.fee_asset).await?, "invalid fee asset", ); @@ -75,7 +75,7 @@ impl ActionHandler for InitBridgeAccountAction { } let balance = state - .get_account_balance(from, self.fee_asset_id) + .get_account_balance(from, &self.fee_asset) .await .context("failed getting `from` account balance for fee payment")?; @@ -96,14 +96,14 @@ impl ActionHandler for InitBridgeAccountAction { state.put_bridge_account_rollup_id(&from, &self.rollup_id); state - .put_bridge_account_asset_id(&from, &self.asset_id) + .put_bridge_account_ibc_asset(&from, &self.asset) .context("failed to put asset ID")?; state.put_bridge_account_sudo_address(&from, &self.sudo_address.unwrap_or(from)); state .put_bridge_account_withdrawer_address(&from, &self.withdrawer_address.unwrap_or(from)); state - .decrease_balance(from, self.fee_asset_id, fee) + .decrease_balance(from, &self.fee_asset, fee) .await .context("failed to deduct fee from account balance")?; Ok(()) diff --git a/crates/astria-sequencer/src/bridge/state_ext.rs b/crates/astria-sequencer/src/bridge/state_ext.rs index 64a6b35f2d..ebd9f3132d 100644 --- a/crates/astria-sequencer/src/bridge/state_ext.rs +++ b/crates/astria-sequencer/src/bridge/state_ext.rs @@ -46,12 +46,6 @@ struct Nonce(u32); #[derive(BorshSerialize, BorshDeserialize, Debug)] struct AssetId([u8; 32]); -impl From<&asset::Id> for AssetId { - fn from(id: &asset::Id) -> Self { - Self(id.get()) - } -} - /// Newtype wrapper to read and write a u128 from rocksdb. #[derive(BorshSerialize, BorshDeserialize, Debug)] struct Fee(u128); @@ -163,14 +157,15 @@ pub(crate) trait StateReadExt: StateRead { } #[instrument(skip(self))] - async fn get_bridge_account_asset_id(&self, address: &Address) -> Result { + async fn get_bridge_account_ibc_asset(&self, address: &Address) -> Result { let bytes = self .get_raw(&asset_id_storage_key(address)) .await .context("failed reading raw asset ID from state")? .ok_or_else(|| anyhow!("asset ID not found"))?; - let asset_id = asset::Id::try_from_slice(&bytes).context("invalid asset ID bytes")?; - Ok(asset_id) + let id = borsh::from_slice::(&bytes) + .context("failed to reconstruct asset ID from storage")?; + Ok(asset::IbcPrefixed::new(id.0)) } #[instrument(skip(self))] @@ -347,15 +342,19 @@ pub(crate) trait StateWriteExt: StateWrite { self.put_raw(rollup_id_storage_key(address), rollup_id.to_vec()); } - #[instrument(skip(self))] - fn put_bridge_account_asset_id( + #[instrument(skip(self, asset), fields(%asset))] + fn put_bridge_account_ibc_asset( &mut self, address: &Address, - asset_id: &asset::Id, - ) -> Result<()> { + asset: TAsset, + ) -> Result<()> + where + TAsset: Into + std::fmt::Display, + { + let ibc = asset.into(); self.put_raw( asset_id_storage_key(address), - borsh::to_vec(&AssetId::from(asset_id)).context("failed to serialize asset IDs")?, + borsh::to_vec(&AssetId(ibc.get())).context("failed to serialize asset IDs")?, ); Ok(()) } @@ -475,7 +474,7 @@ impl StateWriteExt for T {} mod test { use astria_core::{ primitive::v1::{ - asset::Id, + asset, Address, RollupId, }, @@ -493,6 +492,14 @@ mod test { StateWriteExt as _, }; + fn asset_0() -> asset::Denom { + "asset_0".parse().unwrap() + } + + fn asset_1() -> asset::Denom { + "asset_1".parse().unwrap() + } + #[tokio::test] async fn get_bridge_account_rollup_id_uninitialized_ok() { let storage = cnidarium::TempStorage::new().await.unwrap(); @@ -578,67 +585,70 @@ mod test { let address = crate::address::base_prefixed([42u8; 20]); state - .get_bridge_account_asset_id(&address) + .get_bridge_account_ibc_asset(&address) .await .expect_err("call to get bridge account asset ids should fail if no assets"); } #[tokio::test] - async fn put_bridge_account_asset_ids() { + async fn put_bridge_account_ibc_assets() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); let address = crate::address::base_prefixed([42u8; 20]); - let mut asset = Id::from_str_unchecked("asset_0"); + let mut asset = asset_0(); // can write state - .put_bridge_account_asset_id(&address, &asset) + .put_bridge_account_ibc_asset(&address, &asset) .expect("storing bridge account asset should not fail"); let mut result = state - .get_bridge_account_asset_id(&address) + .get_bridge_account_ibc_asset(&address) .await .expect("bridge asset id was written and must exist inside the database"); assert_eq!( - result, asset, + result, + asset.to_ibc_prefixed(), "returned bridge account asset id did not match expected" ); // can update - asset = Id::from_str_unchecked("asset_2"); + asset = "asset_2".parse::().unwrap(); state - .put_bridge_account_asset_id(&address, &asset) + .put_bridge_account_ibc_asset(&address, &asset) .expect("storing bridge account assets should not fail"); result = state - .get_bridge_account_asset_id(&address) + .get_bridge_account_ibc_asset(&address) .await .expect("bridge asset id was written and must exist inside the database"); assert_eq!( - result, asset, + result, + asset.to_ibc_prefixed(), "returned bridge account asset id did not match expected" ); // writing to other account also ok let address_1 = crate::address::base_prefixed([41u8; 20]); - let asset_1 = Id::from_str_unchecked("asset_0"); + let asset_1 = asset_1(); state - .put_bridge_account_asset_id(&address_1, &asset_1) + .put_bridge_account_ibc_asset(&address_1, &asset_1) .expect("storing bridge account assets should not fail"); assert_eq!( state - .get_bridge_account_asset_id(&address_1) + .get_bridge_account_ibc_asset(&address_1) .await .expect("bridge asset id was written and must exist inside the database"), - asset_1, + asset_1.into(), "second bridge account asset not what was expected" ); result = state - .get_bridge_account_asset_id(&address) + .get_bridge_account_ibc_asset(&address) .await .expect("original bridge asset id was written and must exist inside the database"); assert_eq!( - result, asset, + result, + asset.to_ibc_prefixed(), "original bridge account asset id did not match expected after new bridge account \ added" ); @@ -745,13 +755,13 @@ mod test { let rollup_id = RollupId::new([1u8; 32]); let bridge_address = crate::address::base_prefixed([42u8; 20]); let mut amount = 10u128; - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; let mut deposit = Deposit::new( bridge_address, rollup_id, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); @@ -786,7 +796,7 @@ mod test { bridge_address, rollup_id, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); deposits.append(&mut vec![deposit.clone()]); @@ -857,13 +867,13 @@ mod test { let rollup_id_0 = RollupId::new([1u8; 32]); let bridge_address = crate::address::base_prefixed([42u8; 20]); let amount = 10u128; - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; let mut deposit = Deposit::new( bridge_address, rollup_id_0, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); @@ -885,7 +895,7 @@ mod test { bridge_address, rollup_id_1, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); state @@ -928,7 +938,7 @@ mod test { let rollup_id = RollupId::new([1u8; 32]); let bridge_address = crate::address::base_prefixed([42u8; 20]); let amount = 10u128; - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; let deposit = Deposit::new( bridge_address, @@ -983,13 +993,13 @@ mod test { let rollup_id = RollupId::new([1u8; 32]); let bridge_address = crate::address::base_prefixed([42u8; 20]); let amount = 10u128; - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; let mut deposit = Deposit::new( bridge_address, rollup_id, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); @@ -1005,7 +1015,7 @@ mod test { bridge_address, rollup_id_1, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); let deposits_1 = vec![deposit.clone()]; @@ -1075,13 +1085,13 @@ mod test { let rollup_id = RollupId::new([1u8; 32]); let bridge_address = crate::address::base_prefixed([42u8; 20]); let amount = 10u128; - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let destination_chain_address = "0xdeadbeef"; let mut deposit = Deposit::new( bridge_address, rollup_id, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); @@ -1097,7 +1107,7 @@ mod test { bridge_address, rollup_id_1, amount, - asset, + asset.clone(), destination_chain_address.to_string(), ); state diff --git a/crates/astria-sequencer/src/fee_asset_change.rs b/crates/astria-sequencer/src/fee_asset_change.rs index 0abbcf0f98..9de206b8d7 100644 --- a/crates/astria-sequencer/src/fee_asset_change.rs +++ b/crates/astria-sequencer/src/fee_asset_change.rs @@ -40,11 +40,12 @@ impl ActionHandler for FeeAssetChangeAction { async fn execute(&self, state: &mut S, _from: Address) -> Result<()> { match self { FeeAssetChangeAction::Addition(asset) => { - state.put_allowed_fee_asset(*asset); + state.put_allowed_fee_asset(asset); } FeeAssetChangeAction::Removal(asset) => { - state.delete_allowed_fee_asset(*asset); + state.delete_allowed_fee_asset(asset); + // FIXME: context if state.get_allowed_fee_assets().await?.is_empty() { bail!("cannot remove last allowed fee asset"); } diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index 2182dad612..197fe214f3 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -214,7 +214,7 @@ async fn refund_tokens_check( // // check if escrow account has enough balance to refund user let balance = state - .get_ibc_channel_balance(source_channel, denom.id()) + .get_ibc_channel_balance(source_channel, denom) .await .context("failed to get channel balance in refund_tokens_check")?; @@ -342,7 +342,7 @@ async fn convert_denomination_if_ibc_prefixed( let denom = match packet_denom { Denom::TracePrefixed(trace) => trace, Denom::IbcPrefixed(ibc) => state - .get_ibc_asset(ibc.id()) + .map_ibc_to_trace_prefixed_asset(ibc) .await .context("failed to get denom trace from asset id")? .context("denom for given asset id not found in state")?, @@ -476,14 +476,14 @@ async fn execute_ics20_transfer( }; let escrow_balance = state - .get_ibc_channel_balance(escrow_channel, denom_trace.id()) + .get_ibc_channel_balance(escrow_channel, &denom_trace) .await .context("failed to get IBC channel balance in execute_ics20_transfer")?; state .put_ibc_channel_balance( escrow_channel, - denom_trace.id(), + &denom_trace, escrow_balance .checked_sub(packet_amount) .ok_or(anyhow::anyhow!( @@ -493,23 +493,23 @@ async fn execute_ics20_transfer( .context("failed to update escrow account balance in execute_ics20_transfer")?; state - .increase_balance(recipient, denom_trace.id(), packet_amount) + .increase_balance(recipient, &denom_trace, packet_amount) .await .context("failed to update user account balance in execute_ics20_transfer")?; } else { // register denomination in global ID -> denom map if it's not already there if !state - .has_ibc_asset(trace_with_dest.id()) + .has_ibc_asset(&*trace_with_dest) .await .context("failed to check if ibc asset exists in state")? { state - .put_ibc_asset(trace_with_dest.id(), &trace_with_dest) + .put_ibc_asset(&trace_with_dest) .context("failed to put IBC asset in storage")?; } state - .increase_balance(recipient, trace_with_dest.id(), packet_amount) + .increase_balance(recipient, &*trace_with_dest, packet_amount) .await .context("failed to update user account balance in execute_ics20_transfer")?; } @@ -533,7 +533,7 @@ async fn execute_rollup_withdrawal_refund( execute_deposit(state, bridge_address, denom, amount, destination_address).await?; state - .increase_balance(*bridge_address, denom.id(), amount) + .increase_balance(*bridge_address, denom, amount) .await .context( "failed to update bridge account account balance in execute_rollup_withdrawal_refund", @@ -613,12 +613,12 @@ async fn execute_deposit( bail!("bridge account rollup ID not found in state; invalid bridge address?") }; - let allowed_asset_id = state - .get_bridge_account_asset_id(bridge_address) + let allowed_asset = state + .get_bridge_account_ibc_asset(bridge_address) .await .context("failed to get bridge account asset ID")?; ensure!( - allowed_asset_id == denom.id(), + allowed_asset == denom.to_ibc_prefixed(), "asset ID is not authorized for transfer to bridge account", ); @@ -626,7 +626,7 @@ async fn execute_deposit( *bridge_address, rollup_id, amount, - denom.id(), + denom.into(), destination_address, ); state @@ -644,7 +644,11 @@ mod test { use denom::TracePrefixed; use super::*; - use crate::accounts::state_ext::StateReadExt as _; + use crate::{ + accounts::state_ext::StateReadExt as _, + bridge::state_ext::StateWriteExt, + ibc::state_ext::StateWriteExt as _, + }; #[tokio::test] async fn prefix_denomination_not_refund() { @@ -681,15 +685,11 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let denom_trace = "asset".parse::().unwrap(); - state_tx - .put_ibc_asset(denom_trace.id(), &denom_trace) - .unwrap(); + let denom_trace = "asset".parse().unwrap(); + state_tx.put_ibc_asset(&denom_trace).unwrap(); let expected = denom_trace.clone(); - let packet_denom = format!("ibc/{}", hex::encode(denom_trace.id().as_ref())) - .parse::() - .unwrap(); + let packet_denom = denom_trace.to_ibc_prefixed().into(); let denom = convert_denomination_if_ibc_prefixed(&mut state_tx, packet_denom) .await .unwrap(); @@ -742,13 +742,10 @@ mod test { .expect("valid ics20 transfer to user account; recipient, memo, and asset ID are valid"); let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); - let balance = state_tx - .get_account_balance(recipient, denom.id()) - .await - .expect( - "ics20 transfer to user account should succeed and balance should be minted to \ - this account", - ); + let balance = state_tx.get_account_balance(recipient, denom).await.expect( + "ics20 transfer to user account should succeed and balance should be minted to this \ + account", + ); assert_eq!(balance, 100); } @@ -764,7 +761,7 @@ mod test { state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_asset_id(&bridge_address, &denom.id()) + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let memo = Ics20TransferDepositMemo { @@ -794,7 +791,7 @@ mod test { let denom = "dest_port/dest_channel/nootasset".parse::().unwrap(); let balance = state_tx - .get_account_balance(bridge_address, denom.id()) + .get_account_balance(bridge_address, denom) .await .expect( "ics20 transfer from sender to bridge account should have updated funds in the \ @@ -821,7 +818,7 @@ mod test { state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_asset_id(&bridge_address, &denom.id()) + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); // use invalid memo, which should fail @@ -859,7 +856,7 @@ mod test { state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_asset_id(&bridge_address, &denom.id()) + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); // use invalid asset, which should fail @@ -897,7 +894,7 @@ mod test { state_tx .put_ibc_channel_balance( &"dest_channel".to_string().parse().unwrap(), - base_denom.id(), + &base_denom, amount, ) .unwrap(); @@ -924,15 +921,12 @@ mod test { .expect("valid ics20 transfer to user account; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(recipient_address, base_denom.id()) + .get_account_balance(recipient_address, &base_denom) .await .expect("ics20 transfer to user account should succeed"); assert_eq!(balance, amount); let balance = state_tx - .get_ibc_channel_balance( - &"dest_channel".to_string().parse().unwrap(), - base_denom.id(), - ) + .get_ibc_channel_balance(&"dest_channel".to_string().parse().unwrap(), &base_denom) .await .expect("ics20 transfer to user account from escrow account should succeed"); assert_eq!(balance, 0); @@ -950,7 +944,7 @@ mod test { state_tx .put_ibc_channel_balance( &"source_channel".to_string().parse().unwrap(), - base_denom.id(), + &base_denom, amount, ) .unwrap(); @@ -977,15 +971,12 @@ mod test { .expect("valid ics20 refund to user account; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(recipient_address, base_denom.id()) + .get_account_balance(recipient_address, &base_denom) .await .expect("ics20 refund to user account should succeed"); assert_eq!(balance, amount); let balance = state_tx - .get_ibc_channel_balance( - &"source_channel".to_string().parse().unwrap(), - base_denom.id(), - ) + .get_ibc_channel_balance(&"source_channel".to_string().parse().unwrap(), &base_denom) .await .expect("ics20 refund to user account from escrow account should succeed"); assert_eq!(balance, 0); @@ -1005,7 +996,7 @@ mod test { state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_asset_id(&bridge_address, &denom.id()) + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let amount = 100; @@ -1021,7 +1012,7 @@ mod test { .expect("valid rollup withdrawal refund"); let balance = state_tx - .get_account_balance(bridge_address, denom.id()) + .get_account_balance(bridge_address, denom) .await .expect("rollup withdrawal refund should have updated funds in the bridge address"); assert_eq!(balance, 100); @@ -1046,7 +1037,7 @@ mod test { state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id); state_tx - .put_bridge_account_asset_id(&bridge_address, &denom.id()) + .put_bridge_account_ibc_asset(&bridge_address, &denom) .unwrap(); let packet = FungibleTokenPacketData { @@ -1077,7 +1068,7 @@ mod test { .expect("valid ics20 transfer refund; recipient, memo, and asset ID are valid"); let balance = state_tx - .get_account_balance(bridge_address, denom.id()) + .get_account_balance(bridge_address, &denom) .await .expect( "ics20 transfer refunding to rollup should succeed and balance should be added to \ @@ -1096,7 +1087,7 @@ mod test { bridge_address, rollup_id, 100, - denom.id(), + denom, destination_chain_address, ); assert_eq!(deposit, &expected_deposit); diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index 4fbcfe1572..93c76ba6e1 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -135,16 +135,16 @@ impl ActionHandler for action::Ics20Withdrawal { .await .context("packet failed send check")?; - let transfer_asset_id = self.denom().id(); + let transfer_asset = self.denom(); let from_fee_balance = state - .get_account_balance(from, *self.fee_asset_id()) + .get_account_balance(from, self.fee_asset()) .await .context("failed getting `from` account balance for fee payment")?; // if fee asset is same as transfer asset, ensure accounts has enough funds // to cover both the fee and the amount transferred - if self.fee_asset_id() == &transfer_asset_id { + if self.fee_asset().to_ibc_prefixed() == transfer_asset.to_ibc_prefixed() { let payment_amount = self .amount() .checked_add(fee) @@ -163,7 +163,7 @@ impl ActionHandler for action::Ics20Withdrawal { ); let from_transfer_balance = state - .get_account_balance(from, transfer_asset_id) + .get_account_balance(from, transfer_asset) .await .context("failed to get account balance in transfer check")?; ensure!( @@ -184,12 +184,12 @@ impl ActionHandler for action::Ics20Withdrawal { let checked_packet = withdrawal_to_unchecked_ibc_packet(self).assume_checked(); state - .decrease_balance(from, self.denom().id(), self.amount()) + .decrease_balance(from, self.denom(), self.amount()) .await .context("failed to decrease sender balance")?; state - .decrease_balance(from, *self.fee_asset_id(), fee) + .decrease_balance(from, self.fee_asset(), fee) .await .context("failed to subtract fee from sender balance")?; @@ -201,14 +201,14 @@ impl ActionHandler for action::Ics20Withdrawal { self.denom(), ) { let channel_balance = state - .get_ibc_channel_balance(self.source_channel(), self.denom().id()) + .get_ibc_channel_balance(self.source_channel(), self.denom()) .await .context("failed to get channel balance")?; state .put_ibc_channel_balance( self.source_channel(), - self.denom().id(), + self.denom(), channel_balance .checked_add(self.amount()) .context("overflow when adding to channel balance")?, @@ -255,7 +255,7 @@ mod tests { timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom.clone(), memo: String::new(), }; @@ -289,7 +289,7 @@ mod tests { timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom.clone(), memo: String::new(), }; @@ -326,7 +326,7 @@ mod tests { timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom.clone(), memo: String::new(), }; @@ -364,7 +364,7 @@ mod tests { timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom.clone(), memo: String::new(), }; @@ -398,7 +398,7 @@ mod tests { timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom.clone(), memo: String::new(), }; @@ -431,7 +431,7 @@ mod tests { timeout_height: Height::new(1, 1).unwrap(), timeout_time: 1, source_channel: "channel-0".to_string().parse().unwrap(), - fee_asset_id: denom.id(), + fee_asset: denom.clone(), memo: String::new(), }; diff --git a/crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__tests__storage_keys_have_not_changed-2.snap b/crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__tests__storage_keys_have_not_changed-2.snap new file mode 100644 index 0000000000..7ba59bca3c --- /dev/null +++ b/crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__tests__storage_keys_have_not_changed-2.snap @@ -0,0 +1,5 @@ +--- +source: crates/astria-sequencer/src/ibc/state_ext.rs +expression: "channel_balance_storage_key(&channel, &asset)" +--- +ibc-data/channel-5/balance/be429a02d00837245167a2616674a979a2ac6f9806468b48a975b156ad711320 diff --git a/crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__test__storage_keys_have_not_changed.snap b/crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__tests__storage_keys_have_not_changed.snap similarity index 100% rename from crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__test__storage_keys_have_not_changed.snap rename to crates/astria-sequencer/src/ibc/snapshots/astria_sequencer__ibc__state_ext__tests__storage_keys_have_not_changed.snap diff --git a/crates/astria-sequencer/src/ibc/state_ext.rs b/crates/astria-sequencer/src/ibc/state_ext.rs index fe7fcda768..adda68485e 100644 --- a/crates/astria-sequencer/src/ibc/state_ext.rs +++ b/crates/astria-sequencer/src/ibc/state_ext.rs @@ -17,7 +17,6 @@ use cnidarium::{ StateRead, StateWrite, }; -use hex::ToHex as _; use ibc_types::core::channel::ChannelId; use tracing::{ debug, @@ -52,10 +51,13 @@ impl<'a> std::fmt::Display for IbcRelayerKey<'a> { } } -fn channel_balance_storage_key(channel: &ChannelId, asset: asset::Id) -> String { +fn channel_balance_storage_key>( + channel: &ChannelId, + asset: TAsset, +) -> String { format!( "ibc-data/{channel}/balance/{}", - asset.encode_hex::() + crate::storage_keys::hunks::Asset::from(asset), ) } @@ -65,8 +67,15 @@ fn ibc_relayer_key(address: &Address) -> String { #[async_trait] pub(crate) trait StateReadExt: StateRead { - #[instrument(skip(self))] - async fn get_ibc_channel_balance(&self, channel: &ChannelId, asset: asset::Id) -> Result { + #[instrument(skip(self, asset), fields(%asset))] + async fn get_ibc_channel_balance( + &self, + channel: &ChannelId, + asset: TAsset, + ) -> Result + where + TAsset: Into + std::fmt::Display + Send, + { let Some(bytes) = self .get_raw(&channel_balance_storage_key(channel, asset)) .await @@ -121,13 +130,16 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { - #[instrument(skip(self))] - fn put_ibc_channel_balance( + #[instrument(skip(self, asset), fields(%asset))] + fn put_ibc_channel_balance( &mut self, channel: &ChannelId, - asset: asset::Id, + asset: TAsset, balance: u128, - ) -> Result<()> { + ) -> Result<()> + where + TAsset: Into + std::fmt::Display + Send, + { let bytes = borsh::to_vec(&Balance(balance)).context("failed to serialize balance")?; self.put_raw(channel_balance_storage_key(channel, asset), bytes); Ok(()) @@ -166,9 +178,9 @@ pub(crate) trait StateWriteExt: StateWrite { impl StateWriteExt for T {} #[cfg(test)] -mod test { +mod tests { use astria_core::primitive::v1::{ - asset::Id, + asset, Address, }; use cnidarium::StateDelta; @@ -179,6 +191,14 @@ mod test { StateReadExt as _, StateWriteExt as _, }; + use crate::ibc::state_ext::channel_balance_storage_key; + + fn asset_0() -> asset::Denom { + "asset_0".parse().unwrap() + } + fn asset_1() -> asset::Denom { + "asset_1".parse().unwrap() + } #[tokio::test] async fn get_ibc_sudo_address_fails_if_not_set() { @@ -316,7 +336,7 @@ mod test { let state = StateDelta::new(snapshot); let channel = ChannelId::new(0u64); - let asset = Id::from_str_unchecked("asset"); + let asset = asset_0(); assert_eq!( state @@ -335,16 +355,16 @@ mod test { let mut state = StateDelta::new(snapshot); let channel = ChannelId::new(0u64); - let asset = Id::from_str_unchecked("asset"); + let asset = asset_0(); let mut amount = 10u128; // write initial state - .put_ibc_channel_balance(&channel, asset, amount) + .put_ibc_channel_balance(&channel, &asset, amount) .expect("should be able to set balance for channel and asset pair"); assert_eq!( state - .get_ibc_channel_balance(&channel, asset) + .get_ibc_channel_balance(&channel, &asset) .await .expect("retrieving asset balance for channel should not fail"), amount, @@ -354,11 +374,11 @@ mod test { // can update amount = 20u128; state - .put_ibc_channel_balance(&channel, asset, amount) + .put_ibc_channel_balance(&channel, &asset, amount) .expect("should be able to set balance for channel and asset pair"); assert_eq!( state - .get_ibc_channel_balance(&channel, asset) + .get_ibc_channel_balance(&channel, &asset) .await .expect("retrieving asset balance for channel should not fail"), amount, @@ -373,21 +393,21 @@ mod test { let mut state = StateDelta::new(snapshot); let channel = ChannelId::new(0u64); - let asset_0 = Id::from_str_unchecked("asset_0"); - let asset_1 = Id::from_str_unchecked("asset_1"); + let asset_0 = asset_0(); + let asset_1 = asset_1(); let amount_0 = 10u128; let amount_1 = 20u128; // write both state - .put_ibc_channel_balance(&channel, asset_0, amount_0) + .put_ibc_channel_balance(&channel, &asset_0, amount_0) .expect("should be able to set balance for channel and asset pair"); state - .put_ibc_channel_balance(&channel, asset_1, amount_1) + .put_ibc_channel_balance(&channel, &asset_1, amount_1) .expect("should be able to set balance for channel and asset pair"); assert_eq!( state - .get_ibc_channel_balance(&channel, asset_0) + .get_ibc_channel_balance(&channel, &asset_0) .await .expect("retrieving asset balance for channel should not fail"), amount_0, @@ -395,7 +415,7 @@ mod test { ); assert_eq!( state - .get_ibc_channel_balance(&channel, asset_1) + .get_ibc_channel_balance(&channel, &asset_1) .await .expect("retrieving asset balance for channel should not fail"), amount_1, @@ -411,20 +431,20 @@ mod test { let channel_0 = ChannelId::new(0u64); let channel_1 = ChannelId::new(1u64); - let asset = Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount_0 = 10u128; let amount_1 = 20u128; // write both state - .put_ibc_channel_balance(&channel_0, asset, amount_0) + .put_ibc_channel_balance(&channel_0, &asset, amount_0) .expect("should be able to set balance for channel and asset pair"); state - .put_ibc_channel_balance(&channel_1, asset, amount_1) + .put_ibc_channel_balance(&channel_1, &asset, amount_1) .expect("should be able to set balance for channel and asset pair"); assert_eq!( state - .get_ibc_channel_balance(&channel_0, asset) + .get_ibc_channel_balance(&channel_0, &asset) .await .expect("retrieving asset balance for channel should not fail"), amount_0, @@ -442,10 +462,20 @@ mod test { #[test] fn storage_keys_have_not_changed() { + let channel = ChannelId::new(5); let address: Address = "astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm" .parse() .unwrap(); assert_snapshot!(super::ibc_relayer_key(&address)); + + let asset = "an/asset/with/a/prefix" + .parse::() + .unwrap(); + assert_eq!( + channel_balance_storage_key(&channel, &asset), + channel_balance_storage_key(&channel, asset.to_ibc_prefixed()), + ); + assert_snapshot!(channel_balance_storage_key(&channel, &asset)); } } diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index c410f94725..709ee76abd 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -19,6 +19,7 @@ pub(crate) mod sequence; mod sequencer; pub(crate) mod service; pub(crate) mod state_ext; +pub(crate) mod storage_keys; pub(crate) mod transaction; mod utils; diff --git a/crates/astria-sequencer/src/proposal/commitment.rs b/crates/astria-sequencer/src/proposal/commitment.rs index e495a5147a..c0df56ee71 100644 --- a/crates/astria-sequencer/src/proposal/commitment.rs +++ b/crates/astria-sequencer/src/proposal/commitment.rs @@ -64,11 +64,14 @@ pub(crate) fn generate_rollup_datas_commitment( group_sequence_actions_in_signed_transaction_transactions_by_rollup_id(signed_txs); for (rollup_id, deposit) in deposits { - rollup_ids_to_txs.entry(rollup_id).or_default().extend( - deposit - .into_iter() - .map(|deposit| RollupData::Deposit(deposit).into_raw().encode_to_vec()), - ); + rollup_ids_to_txs + .entry(rollup_id) + .or_default() + .extend(deposit.into_iter().map(|deposit| { + RollupData::Deposit(Box::new(deposit)) + .into_raw() + .encode_to_vec() + })); } rollup_ids_to_txs.sort_unstable_keys(); @@ -89,7 +92,6 @@ pub(crate) fn generate_rollup_datas_commitment( mod test { use astria_core::{ crypto::SigningKey, - primitive::v1::asset::default_native_asset, protocol::transaction::v1alpha1::{ action::{ SequenceAction, @@ -102,25 +104,20 @@ mod test { use rand::rngs::OsRng; use super::*; - use crate::asset::{ - get_native_asset, - NATIVE_ASSET, - }; + use crate::asset::get_native_asset; #[test] fn generate_rollup_datas_commitment_should_ignore_transfers() { - let _ = NATIVE_ASSET.set(default_native_asset()); - let sequence_action = SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"helloworld".to_vec(), - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), }; let transfer_action = TransferAction { to: crate::address::base_prefixed([0u8; 20]), amount: 1, - asset_id: get_native_asset().id(), - fee_asset_id: get_native_asset().id(), + asset: get_native_asset().clone(), + fee_asset: get_native_asset().clone(), }; let signing_key = SigningKey::new(OsRng); @@ -164,18 +161,17 @@ mod test { // this tests that the commitment generated is what is expected via a test vector. // this test will only break in the case of a breaking change to the commitment scheme, // thus if this test needs to be updated, we should cut a new release. - let _ = NATIVE_ASSET.set(default_native_asset()); let sequence_action = SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"helloworld".to_vec(), - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), }; let transfer_action = TransferAction { to: crate::address::base_prefixed([0u8; 20]), amount: 1, - asset_id: get_native_asset().id(), - fee_asset_id: get_native_asset().id(), + asset: get_native_asset().clone(), + fee_asset: get_native_asset().clone(), }; let signing_key = SigningKey::new(OsRng); diff --git a/crates/astria-sequencer/src/sequence/action.rs b/crates/astria-sequencer/src/sequence/action.rs index d646c5f1bf..31347d1e00 100644 --- a/crates/astria-sequencer/src/sequence/action.rs +++ b/crates/astria-sequencer/src/sequence/action.rs @@ -30,12 +30,12 @@ impl ActionHandler for SequenceAction { from: Address, ) -> Result<()> { ensure!( - state.is_allowed_fee_asset(self.fee_asset_id).await?, + state.is_allowed_fee_asset(&self.fee_asset).await?, "invalid fee asset", ); let curr_balance = state - .get_account_balance(from, self.fee_asset_id) + .get_account_balance(from, &self.fee_asset) .await .context("failed getting `from` account balance for fee payment")?; let fee = calculate_fee_from_state(&self.data, state) @@ -66,12 +66,12 @@ impl ActionHandler for SequenceAction { .await .context("failed to calculate fee")?; state - .get_and_increase_block_fees(self.fee_asset_id, fee) + .get_and_increase_block_fees(&self.fee_asset, fee) .await .context("failed to add to block fees")?; state - .decrease_balance(from, self.fee_asset_id, fee) + .decrease_balance(from, &self.fee_asset, fee) .await .context("failed updating `from` account balance")?; Ok(()) diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index eb978c0053..051eb3fe70 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -223,13 +223,7 @@ mod test { SigningKey, VerificationKey, }, - primitive::v1::{ - asset::{ - default_native_asset, - DEFAULT_NATIVE_ASSET_DENOM, - }, - RollupId, - }, + primitive::v1::RollupId, protocol::transaction::v1alpha1::{ action::SequenceAction, TransactionParams, @@ -268,7 +262,7 @@ mod test { SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), data: b"helloworld".to_vec(), - fee_asset_id: get_native_asset().id(), + fee_asset: get_native_asset().clone(), } .into(), ], @@ -481,9 +475,9 @@ mod test { authority_sudo_address: crate::address::base_prefixed([0; 20]), ibc_sudo_address: crate::address::base_prefixed([0; 20]), ibc_relayer_addresses: vec![], - native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), + native_asset_base_denomination: "nria".to_string(), ibc_params: penumbra_ibc::params::IBCParameters::default(), - allowed_fee_assets: vec![default_native_asset()], + allowed_fee_assets: vec!["nria".parse().unwrap()], fees: default_fees(), } .try_into() diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index ac5d140c71..de0240df3c 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -161,11 +161,7 @@ impl Service for Info { #[cfg(test)] mod test { use astria_core::{ - primitive::v1::asset::{ - self, - denom::TracePrefixed, - DEFAULT_NATIVE_ASSET_DENOM, - }, + primitive::v1::asset, protocol::{ account::v1alpha1::BalanceResponse, asset::v1alpha1::DenomResponse, @@ -208,7 +204,7 @@ mod test { let mut state = StateDelta::new(storage.latest_snapshot()); state.put_storage_version_by_height(height, version); - initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); + initialize_native_asset("nria"); let address = crate::address::try_base_prefixed( &hex::decode("a034c743bed8f26cb8ee7b8db2230fd8347ae131").unwrap(), @@ -217,7 +213,7 @@ mod test { let balance = 1000; state - .put_account_balance(address, get_native_asset().id(), balance) + .put_account_balance(address, get_native_asset(), balance) .unwrap(); state.put_block_height(height); storage.commit(state).await.unwrap(); @@ -264,15 +260,14 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let mut state = StateDelta::new(storage.latest_snapshot()); - let denom = "some/ibc/asset".parse::().unwrap(); - let id = denom.id(); + let denom = "some/ibc/asset".parse().unwrap(); let height = 99; state.put_block_height(height); - state.put_ibc_asset(id, &denom).unwrap(); + state.put_ibc_asset(&denom).unwrap(); storage.commit(state).await.unwrap(); let info_request = InfoRequest::Query(request::Query { - path: format!("asset/denom/{}", hex::encode(id)), + path: format!("asset/denom/{}", hex::encode(denom.to_ibc_prefixed().get())), data: vec![].into(), height: u32::try_from(height).unwrap().into(), prove: false, @@ -306,18 +301,18 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let mut state = StateDelta::new(storage.latest_snapshot()); - let asset_ids = vec![ - asset::Id::from_str_unchecked("asset_0"), - asset::Id::from_str_unchecked("asset_1"), - asset::Id::from_str_unchecked("asset_2"), + let assets = vec![ + "asset_0".parse::().unwrap(), + "asset_1".parse::().unwrap(), + "asset_2".parse::().unwrap(), ]; let height = 99; - for &asset_id in &asset_ids { - state.put_allowed_fee_asset(asset_id); + for asset in &assets { + state.put_allowed_fee_asset(asset); assert!( state - .is_allowed_fee_asset(asset_id) + .is_allowed_fee_asset(asset) .await .expect("checking for allowed fee asset should not fail"), "fee asset was expected to be allowed" @@ -347,15 +342,17 @@ mod test { }; assert!(query_response.code.is_ok()); - let allowed_fee_assets_resp = raw::AllowedFeeAssetIdsResponse::decode(query_response.value) + let allowed_fee_assets_resp = raw::AllowedFeeAssetsResponse::decode(query_response.value) .unwrap() .try_to_native() .unwrap(); assert_eq!(allowed_fee_assets_resp.height, height); - assert_eq!(allowed_fee_assets_resp.fee_asset_ids.len(), asset_ids.len()); - for asset_id in asset_ids { + assert_eq!(allowed_fee_assets_resp.fee_assets.len(), assets.len()); + for asset in &assets { assert!( - allowed_fee_assets_resp.fee_asset_ids.contains(&asset_id), + allowed_fee_assets_resp + .fee_assets + .contains(&asset.to_ibc_prefixed().into()), "expected asset_id to be in allowed fee assets" ); } 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/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed-2.snap new file mode 100644 index 0000000000..a1d5962748 --- /dev/null +++ b/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed-2.snap @@ -0,0 +1,5 @@ +--- +source: crates/astria-sequencer/src/state_ext.rs +expression: fee_asset_key(trace_prefixed) +--- +fee_asset/ce072174ebc356c6ead681d61ab417ee72fdedd8155eb76478ece374bb04dc1d diff --git a/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap b/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap new file mode 100644 index 0000000000..1ad4249a94 --- /dev/null +++ b/crates/astria-sequencer/src/snapshots/astria_sequencer__state_ext__test__storage_keys_are_not_changed.snap @@ -0,0 +1,5 @@ +--- +source: crates/astria-sequencer/src/state_ext.rs +expression: block_fees_key(&trace_prefixed) +--- +block_fees/ce072174ebc356c6ead681d61ab417ee72fdedd8155eb76478ece374bb04dc1d diff --git a/crates/astria-sequencer/src/state_ext.rs b/crates/astria-sequencer/src/state_ext.rs index d265f9056a..f8d4d77950 100644 --- a/crates/astria-sequencer/src/state_ext.rs +++ b/crates/astria-sequencer/src/state_ext.rs @@ -22,12 +22,18 @@ fn storage_version_by_height_key(height: u64) -> Vec { format!("storage_version/{height}").into() } -fn block_fees_key(asset: asset::Id) -> Vec { - format!("{BLOCK_FEES_PREFIX}{}", crate::utils::Hex(asset.as_ref())).into() +fn block_fees_key>(asset: TAsset) -> String { + format!( + "{BLOCK_FEES_PREFIX}{}", + crate::storage_keys::hunks::Asset::from(asset) + ) } -fn fee_asset_key(asset: asset::Id) -> Vec { - format!("{FEE_ASSET_PREFIX}{}", crate::utils::Hex(asset.as_ref())).into() +fn fee_asset_key>(asset: TAsset) -> String { + format!( + "{FEE_ASSET_PREFIX}{}", + crate::storage_keys::hunks::Asset::from(asset) + ) } #[async_trait] @@ -127,56 +133,63 @@ pub(crate) trait StateReadExt: StateRead { } #[instrument(skip(self))] - async fn get_block_fees(&self) -> Result> { - let mut fees: Vec<(asset::Id, u128)> = Vec::new(); + 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 id_str = key + let suffix = key .strip_prefix(BLOCK_FEES_PREFIX.as_bytes()) .expect("prefix must always be present"); - let id = - asset::Id::try_from_slice(&hex::decode(id_str).expect("key must be hex encoded")) - .context("failed to parse asset id from hex key")?; + 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((id, u128::from_be_bytes(bytes))); + fees.push((asset, u128::from_be_bytes(bytes))); } Ok(fees) } - #[instrument(skip(self))] - async fn is_allowed_fee_asset(&self, asset: asset::Id) -> Result { + #[instrument(skip(self, asset), fields(%asset))] + 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)) + .nonverifiable_get_raw(fee_asset_key(asset).as_bytes()) .await .context("failed to read raw fee asset from state")? .is_some()) } #[instrument(skip(self))] - async fn get_allowed_fee_assets(&self) -> Result> { + 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 id_str = key + let suffix = key .strip_prefix(FEE_ASSET_PREFIX.as_bytes()) .expect("prefix must always be present"); - let id = - asset::Id::try_from_slice(&hex::decode(id_str).expect("key must be hex encoded")) - .context("failed to parse asset id from hex key")?; - - assets.push(id); + 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) @@ -226,10 +239,18 @@ pub(crate) trait StateWriteExt: StateWrite { } /// Adds `amount` to the block fees for `asset`. - #[instrument(skip(self))] - async fn get_and_increase_block_fees(&mut self, asset: asset::Id, amount: u128) -> Result<()> { + #[instrument(skip(self, asset))] + async fn get_and_increase_block_fees( + &mut self, + asset: TAsset, + amount: u128, + ) -> Result<()> + where + TAsset: Into + std::fmt::Display + Send, + { + let block_fees_key = block_fees_key(asset); let current_amount = self - .nonverifiable_get_raw(&block_fees_key(asset)) + .nonverifiable_get_raw(block_fees_key.as_bytes()) .await .context("failed to read raw block fees from state")? .map(|bytes| { @@ -246,7 +267,7 @@ pub(crate) trait StateWriteExt: StateWrite { .checked_add(amount) .context("block fees overflowed u128")?; - self.nonverifiable_put_raw(block_fees_key(asset), new_amount.to_be_bytes().to_vec()); + self.nonverifiable_put_raw(block_fees_key.into(), new_amount.to_be_bytes().to_vec()); Ok(()) } @@ -259,14 +280,20 @@ pub(crate) trait StateWriteExt: StateWrite { } } - #[instrument(skip(self))] - fn put_allowed_fee_asset(&mut self, asset: asset::Id) { - self.nonverifiable_put_raw(fee_asset_key(asset), vec![]); + #[instrument(skip(self, asset) fields(%asset))] + 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(self))] - fn delete_allowed_fee_asset(&mut self, asset: asset::Id) { - self.nonverifiable_delete(fee_asset_key(asset)); + #[instrument(skip(self, asset) fields(%asset))] + fn delete_allowed_fee_asset(&mut self, asset: TAsset) + where + TAsset: Into + std::fmt::Display, + { + self.nonverifiable_delete(fee_asset_key(asset).into()); } } @@ -291,6 +318,8 @@ fn revision_number_from_chain_id(chain_id: &str) -> u64 { #[cfg(test)] mod test { + use std::collections::HashSet; + use cnidarium::StateDelta; use tendermint::Time; @@ -299,6 +328,20 @@ 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() { @@ -578,10 +621,10 @@ mod test { assert!(fee_balances_orig.is_empty()); // can write - let asset = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_0"); + let asset = asset_0(); let amount = 100u128; state - .get_and_increase_block_fees(asset, amount) + .get_and_increase_block_fees(&asset, amount) .await .unwrap(); @@ -589,7 +632,7 @@ mod test { let fee_balances_updated = state.get_block_fees().await.unwrap(); assert_eq!( fee_balances_updated[0], - (asset, amount), + (asset.to_ibc_prefixed(), amount), "fee balances are not what they were expected to be" ); } @@ -601,25 +644,27 @@ mod test { let mut state = StateDelta::new(snapshot); // can write - let asset_first = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_0"); - let asset_second = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_1"); + 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) + .get_and_increase_block_fees(&asset_first, amount_first) .await .unwrap(); state - .get_and_increase_block_fees(asset_second, amount_second) + .get_and_increase_block_fees(&asset_second, amount_second) .await .unwrap(); // holds expected - let mut fee_balances = state.get_block_fees().await.unwrap(); - fee_balances.sort_by(|a, b| a.1.cmp(&b.1)); + let fee_balances = HashSet::<_>::from_iter(state.get_block_fees().await.unwrap()); assert_eq!( fee_balances, - vec![(asset_first, amount_first), (asset_second, amount_second)], + 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" ); @@ -640,20 +685,20 @@ mod test { let mut state = StateDelta::new(snapshot); // non-existent fees assets return false - let asset = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_0"); + let asset = asset_0(); assert!( !state - .is_allowed_fee_asset(asset) + .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); + state.put_allowed_fee_asset(&asset); assert!( state - .is_allowed_fee_asset(asset) + .is_allowed_fee_asset(&asset) .await .expect("checking for allowed fee asset should not fail"), "fee asset was expected to be allowed" @@ -667,11 +712,11 @@ mod test { let mut state = StateDelta::new(snapshot); // setup fee asset - let asset = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_0"); - state.put_allowed_fee_asset(asset); + let asset = asset_0(); + state.put_allowed_fee_asset(&asset); assert!( state - .is_allowed_fee_asset(asset) + .is_allowed_fee_asset(&asset) .await .expect("checking for allowed fee asset should not fail"), "fee asset was expected to be allowed" @@ -681,12 +726,12 @@ mod test { let assets = state.get_allowed_fee_assets().await.unwrap(); assert_eq!( assets, - vec![asset], + vec![asset.to_ibc_prefixed()], "expected returned allowed fee assets to match what was written in" ); // can delete - state.delete_allowed_fee_asset(asset); + state.delete_allowed_fee_asset(&asset); // see is deleted let assets = state.get_allowed_fee_assets().await.unwrap(); @@ -700,45 +745,64 @@ mod test { let mut state = StateDelta::new(snapshot); // setup fee assets - let asset_first = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_0"); - state.put_allowed_fee_asset(asset_first); + let asset_first = asset_0(); + state.put_allowed_fee_asset(&asset_first); assert!( state - .is_allowed_fee_asset(asset_first) + .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 = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_1"); - state.put_allowed_fee_asset(asset_second); + let asset_second = asset_1(); + state.put_allowed_fee_asset(&asset_second); assert!( state - .is_allowed_fee_asset(asset_second) + .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 = astria_core::primitive::v1::asset::Id::from_str_unchecked("asset_3"); - state.put_allowed_fee_asset(asset_third); + let asset_third = asset_2(); + state.put_allowed_fee_asset(&asset_third); assert!( state - .is_allowed_fee_asset(asset_third) + .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); + state.delete_allowed_fee_asset(&asset_second); // see is deleted - let mut assets = state.get_allowed_fee_assets().await.unwrap(); - assets.sort(); - assets.reverse(); // asset ids are hashes of the string input which the statement below is ordered by + let assets = HashSet::<_>::from_iter(state.get_allowed_fee_assets().await.unwrap()); assert_eq!( assets, - vec![asset_first, asset_third], + 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/storage_keys.rs b/crates/astria-sequencer/src/storage_keys.rs new file mode 100644 index 0000000000..6c29e1a276 --- /dev/null +++ b/crates/astria-sequencer/src/storage_keys.rs @@ -0,0 +1,60 @@ +pub(crate) mod hunks { + use std::str::FromStr; + + use astria_core::primitive::v1::asset; + + #[derive(Clone, Copy, Debug, PartialEq)] + pub(crate) struct Asset(asset::IbcPrefixed); + + impl Asset { + pub(crate) fn get(self) -> asset::IbcPrefixed { + self.0 + } + } + + impl std::fmt::Display for Asset { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for byte in self.get().get() { + f.write_fmt(format_args!("{byte:02x}"))?; + } + Ok(()) + } + } + + impl> From for Asset { + fn from(value: T) -> Self { + Self(value.into()) + } + } + + #[derive(Debug, thiserror::Error)] + #[error("failed to parse input as asset key")] + pub(crate) struct ParseAssetKeyError { + #[from] + source: hex::FromHexError, + } + + impl FromStr for Asset { + type Err = ParseAssetKeyError; + + fn from_str(s: &str) -> std::result::Result { + use hex::FromHex as _; + let bytes = <[u8; 32]>::from_hex(s)?; + Ok(Self(asset::IbcPrefixed::new(bytes))) + } + } + + #[cfg(test)] + mod tests { + use super::Asset; + #[test] + fn asset_key_to_string_parse_roundtrip() { + let asset = "an/asset/with/a/prefix" + .parse::() + .unwrap(); + let expected = Asset::from(asset); + let actual = expected.to_string().parse::().unwrap(); + assert_eq!(expected, actual); + } + } +} diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index 3e6d7068c2..01d3cbc1fc 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -93,26 +93,25 @@ pub(crate) async fn check_balance_for_total_fees( for action in &tx.actions { match action { Action::Transfer(act) => transfer_update_fees( - act.asset_id, - act.fee_asset_id, + &act.asset, + &act.fee_asset, act.amount, &mut fees_by_asset, transfer_fee, ), Action::Sequence(act) => { - sequence_update_fees(state, act.fee_asset_id, &mut fees_by_asset, &act.data) - .await?; + sequence_update_fees(state, &act.fee_asset, &mut fees_by_asset, &act.data).await?; } Action::Ics20Withdrawal(act) => ics20_withdrawal_updates_fees( - act.denom().id(), - *act.fee_asset_id(), + &act.denom, + &act.fee_asset, act.amount(), &mut fees_by_asset, ics20_withdrawal_fee, ), Action::InitBridgeAccount(act) => { fees_by_asset - .entry(act.fee_asset_id) + .entry(act.fee_asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(init_bridge_account_fee)) .or_insert(init_bridge_account_fee); } @@ -127,7 +126,7 @@ pub(crate) async fn check_balance_for_total_fees( state, act.bridge_address.unwrap_or(from), act.amount, - act.fee_asset_id, + &act.fee_asset, &mut fees_by_asset, transfer_fee, ) @@ -135,7 +134,7 @@ pub(crate) async fn check_balance_for_total_fees( } Action::BridgeSudoChange(act) => { fees_by_asset - .entry(act.fee_asset_id) + .entry(act.fee_asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(bridge_sudo_change_fee)) .or_insert(bridge_sudo_change_fee); } @@ -165,58 +164,58 @@ pub(crate) async fn check_balance_for_total_fees( } fn transfer_update_fees( - asset_id: asset::Id, - fee_asset_id: asset::Id, + asset: &asset::Denom, + fee_asset: &asset::Denom, amount: u128, - fees_by_asset: &mut HashMap, + fees_by_asset: &mut HashMap, transfer_fee: u128, ) { fees_by_asset - .entry(asset_id) + .entry(asset.to_ibc_prefixed()) .and_modify(|amt: &mut u128| *amt = amt.saturating_add(amount)) .or_insert(amount); fees_by_asset - .entry(fee_asset_id) + .entry(fee_asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(transfer_fee)) .or_insert(transfer_fee); } async fn sequence_update_fees( state: &S, - fee_asset_id: asset::Id, - fees_by_asset: &mut HashMap, + fee_asset: &asset::Denom, + fees_by_asset: &mut HashMap, data: &[u8], ) -> anyhow::Result<()> { let fee = crate::sequence::calculate_fee_from_state(data, state) .await .context("fee for sequence action overflowed; data too large")?; fees_by_asset - .entry(fee_asset_id) + .entry(fee_asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(fee)) .or_insert(fee); Ok(()) } fn ics20_withdrawal_updates_fees( - asset_id: asset::Id, - fee_asset_id: asset::Id, + asset: &asset::Denom, + fee_asset: &asset::Denom, amount: u128, - fees_by_asset: &mut HashMap, + fees_by_asset: &mut HashMap, ics20_withdrawal_fee: u128, ) { fees_by_asset - .entry(asset_id) + .entry(asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(amount)) .or_insert(amount); fees_by_asset - .entry(fee_asset_id) + .entry(fee_asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(ics20_withdrawal_fee)) .or_insert(ics20_withdrawal_fee); } fn bridge_lock_update_fees( act: &BridgeLockAction, - fees_by_asset: &mut HashMap, + fees_by_asset: &mut HashMap, transfer_fee: u128, bridge_lock_byte_cost_multiplier: u128, ) { @@ -228,18 +227,18 @@ fn bridge_lock_update_fees( // rollup ID doesn't matter here, as this is only used as a size-check RollupId::from_unhashed_bytes([0; 32]), act.amount, - act.asset_id, + act.asset.clone(), act.destination_chain_address.clone(), )) .saturating_mul(bridge_lock_byte_cost_multiplier), ); fees_by_asset - .entry(act.asset_id) + .entry(act.asset.to_ibc_prefixed()) .and_modify(|amt: &mut u128| *amt = amt.saturating_add(act.amount)) .or_insert(act.amount); fees_by_asset - .entry(act.asset_id) + .entry(act.asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(expected_deposit_fee)) .or_insert(expected_deposit_fee); } @@ -248,33 +247,30 @@ async fn bridge_unlock_update_fees( state: &S, bridge_address: Address, amount: u128, - fee_asset_id: asset::Id, - fees_by_asset: &mut HashMap, + fee_asset: &asset::Denom, + fees_by_asset: &mut HashMap, transfer_fee: u128, ) -> anyhow::Result<()> { - let asset_id = state - .get_bridge_account_asset_id(&bridge_address) + let asset = state + .get_bridge_account_ibc_asset(&bridge_address) .await .context("must be a bridge account for BridgeUnlock action")?; fees_by_asset - .entry(asset_id) + .entry(asset) .and_modify(|amt: &mut u128| *amt = amt.saturating_add(amount)) .or_insert(amount); fees_by_asset - .entry(fee_asset_id) + .entry(fee_asset.to_ibc_prefixed()) .and_modify(|amt| *amt = amt.saturating_add(transfer_fee)) .or_insert(transfer_fee); Ok(()) } #[cfg(test)] -mod test { +mod tests { use astria_core::{ primitive::v1::{ - asset::{ - Denom, - DEFAULT_NATIVE_ASSET_DENOM, - }, + asset::Denom, RollupId, ADDRESS_LEN, }, @@ -311,9 +307,8 @@ mod test { state_tx.put_bridge_lock_byte_cost_multiplier(1); state_tx.put_bridge_sudo_change_base_fee(24); - crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); - let native_asset = crate::asset::get_native_asset().id(); - let other_asset = "other".parse::().unwrap().id(); + let native_asset = crate::asset::get_native_asset(); + let other_asset = "other".parse::().unwrap(); let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); let amount = 100; @@ -331,21 +326,21 @@ mod test { .await .unwrap(); state_tx - .increase_balance(alice_address, other_asset, amount) + .increase_balance(alice_address, &other_asset, amount) .await .unwrap(); let actions = vec![ Action::Transfer(TransferAction { - asset_id: other_asset, + asset: other_asset.clone(), amount, - fee_asset_id: native_asset, + fee_asset: native_asset.clone(), to: crate::address::base_prefixed([0; ADDRESS_LEN]), }), Action::Sequence(SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), data, - fee_asset_id: native_asset, + fee_asset: native_asset.clone(), }), ]; @@ -378,9 +373,8 @@ mod test { state_tx.put_bridge_lock_byte_cost_multiplier(1); state_tx.put_bridge_sudo_change_base_fee(24); - crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); - let native_asset = crate::asset::get_native_asset().id(); - let other_asset = "other".parse::().unwrap().id(); + let native_asset = crate::asset::get_native_asset(); + let other_asset = "other".parse::().unwrap(); let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); let amount = 100; @@ -400,15 +394,15 @@ mod test { let actions = vec![ Action::Transfer(TransferAction { - asset_id: other_asset, + asset: other_asset.clone(), amount, - fee_asset_id: native_asset, + fee_asset: native_asset.clone(), to: crate::address::base_prefixed([0; ADDRESS_LEN]), }), Action::Sequence(SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), data, - fee_asset_id: native_asset, + fee_asset: native_asset.clone(), }), ]; @@ -425,6 +419,9 @@ mod test { let err = check_balance_mempool(&signed_tx, &state_tx) .await .expect_err("insufficient funds for `other` asset"); - assert!(err.to_string().contains(&other_asset.to_string())); + assert!( + err.to_string() + .contains(&other_asset.to_ibc_prefixed().to_string()) + ); } } diff --git a/proto/protocolapis/astria/protocol/asset/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/asset/v1alpha1/types.proto index d876cfe476..617d2129f1 100644 --- a/proto/protocolapis/astria/protocol/asset/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/asset/v1alpha1/types.proto @@ -8,8 +8,8 @@ message DenomResponse { string denom = 3; } -// A response containing the allowed fee asset IDs. -message AllowedFeeAssetIdsResponse { +// A response containing the allowed fee assets. +message AllowedFeeAssetsResponse { uint64 height = 1; - repeated bytes fee_asset_ids = 2; + repeated string fee_assets = 2; } diff --git a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto index 0e3c4341f8..e5f5142248 100644 --- a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto @@ -73,9 +73,9 @@ message TransferAction { astria.primitive.v1.Address to = 1; astria.primitive.v1.Uint128 amount = 2; // the asset to be transferred - bytes asset_id = 3; + string asset = 3; // the asset used to pay the transaction fee - bytes fee_asset_id = 4; + string fee_asset = 4; } // `SequenceAction` represents a transaction destined for another @@ -87,7 +87,7 @@ message SequenceAction { astria.primitive.v1.RollupId rollup_id = 1; bytes data = 2; // the asset used to pay the transaction fee - bytes fee_asset_id = 3; + string fee_asset = 3; } /// `SudoAddressChangeAction` represents a transaction that changes @@ -117,7 +117,7 @@ message Ics20Withdrawal { // the source channel used for the withdrawal. string source_channel = 7; // the asset used to pay the transaction fee - bytes fee_asset_id = 8; + string fee_asset = 8; // a memo to include with the transfer string memo = 9; // the address of the bridge account to transfer from, if this is a withdrawal @@ -149,12 +149,10 @@ message IbcRelayerChangeAction { // `FeeAssetChangeAction` represents a transaction that adds // or removes an asset for fee payments. -// The bytes contained in each variant are the 32-byte asset ID -// to add or remove. message FeeAssetChangeAction { oneof value { - bytes addition = 1; - bytes removal = 2; + string addition = 1; + string removal = 2; } } @@ -168,9 +166,9 @@ message InitBridgeAccountAction { // the rollup ID to register with the bridge account (the tx sender) astria.primitive.v1.RollupId rollup_id = 1; // the asset ID accepted as an incoming transfer by the bridge account - bytes asset_id = 2; + string asset = 2; // the asset used to pay the transaction fee - bytes fee_asset_id = 3; + string fee_asset = 3; // the address corresponding to the key which has sudo capabilities; // ie. can change the sudo and withdrawer addresses for this bridge account. // if this is empty, the sender of the transaction is used. @@ -191,9 +189,9 @@ message BridgeLockAction { // the amount to transfer astria.primitive.v1.Uint128 amount = 2; // the asset to be transferred - bytes asset_id = 3; + string asset = 3; // the asset used to pay the transaction fee - bytes fee_asset_id = 4; + string fee_asset = 4; // the address on the destination chain which // will receive the bridged funds string destination_chain_address = 5; @@ -202,7 +200,7 @@ message BridgeLockAction { // `BridgeUnlockAction` represents a transaction that transfers // funds from a bridge account to a sequencer account. // -// It's the same as a `TransferAction` but without the `asset_id` field +// It's the same as a `TransferAction` but without the `asset` field // and with the `memo` field. message BridgeUnlockAction { // the to withdraw funds to @@ -210,7 +208,7 @@ message BridgeUnlockAction { // the amount to transfer astria.primitive.v1.Uint128 amount = 2; // the asset used to pay the transaction fee - bytes fee_asset_id = 3; + string fee_asset = 3; // memo for double spend prevention bytes memo = 4; // the address of the bridge account to transfer from, @@ -227,7 +225,7 @@ message BridgeSudoChangeAction { // the new withdrawer address; unchanged if unset astria.primitive.v1.Address new_withdrawer_address = 3; // the asset used to pay the transaction fee - bytes fee_asset_id = 4; + string fee_asset = 4; } message FeeChangeAction { diff --git a/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto b/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto index 00f324e490..73cc05267b 100644 --- a/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto +++ b/proto/sequencerblockapis/astria/sequencerblock/v1alpha1/block.proto @@ -82,7 +82,7 @@ message Deposit { // the rollup_id which the funds are being deposited to astria.primitive.v1.RollupId rollup_id = 2; astria.primitive.v1.Uint128 amount = 3; - bytes asset_id = 4; + string asset = 4; // the address on the destination chain which // will receive the bridged funds string destination_chain_address = 5;