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 cc833ad127..43a88f12b5 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs @@ -59,7 +59,7 @@ impl ::prost::Name for TransactionParams { pub struct Action { #[prost( oneof = "action::Value", - tags = "1, 2, 11, 12, 13, 21, 22, 50, 51, 52, 53, 54" + tags = "1, 2, 11, 12, 13, 21, 22, 50, 51, 52, 53, 54, 55" )] pub value: ::core::option::Option, } @@ -96,6 +96,8 @@ pub mod action { FeeAssetChangeAction(super::FeeAssetChangeAction), #[prost(message, tag = "54")] MintAction(super::MintAction), + #[prost(message, tag = "55")] + FeeChangeAction(super::FeeChangeAction), } } impl ::prost::Name for Action { @@ -386,3 +388,42 @@ impl ::prost::Name for BridgeUnlockAction { ::prost::alloc::format!("astria.protocol.transactions.v1alpha1.{}", Self::NAME) } } +#[allow(clippy::derive_partial_eq_without_eq)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct FeeChangeAction { + /// note that the proto number ranges are doubled from that of `Action`. + /// this to accomodate both `base_fee` and `byte_cost_multiplier` for each action. + #[prost(oneof = "fee_change_action::Value", tags = "1, 2, 3, 20, 21, 40")] + pub value: ::core::option::Option, +} +/// Nested message and enum types in `FeeChangeAction`. +pub mod fee_change_action { + /// note that the proto number ranges are doubled from that of `Action`. + /// this to accomodate both `base_fee` and `byte_cost_multiplier` for each action. + #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Oneof)] + pub enum Value { + /// core protocol fees are defined on 1-20 + #[prost(message, tag = "1")] + TransferBaseFee(super::super::super::super::primitive::v1::Uint128), + #[prost(message, tag = "2")] + SequenceBaseFee(super::super::super::super::primitive::v1::Uint128), + #[prost(message, tag = "3")] + SequenceByteCostMultiplier(super::super::super::super::primitive::v1::Uint128), + /// bridge fees are defined on 20-39 + #[prost(message, tag = "20")] + InitBridgeAccountBaseFee(super::super::super::super::primitive::v1::Uint128), + #[prost(message, tag = "21")] + BridgeLockByteCostMultiplier(super::super::super::super::primitive::v1::Uint128), + /// ibc fees are defined on 40-59 + #[prost(message, tag = "40")] + Ics20WithdrawalBaseFee(super::super::super::super::primitive::v1::Uint128), + } +} +impl ::prost::Name for FeeChangeAction { + const NAME: &'static str = "FeeChangeAction"; + const PACKAGE: &'static str = "astria.protocol.transactions.v1alpha1"; + fn full_name() -> ::prost::alloc::string::String { + ::prost::alloc::format!("astria.protocol.transactions.v1alpha1.{}", Self::NAME) + } +} diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index a9072b62fc..783f237edc 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -37,6 +37,7 @@ pub enum Action { InitBridgeAccount(InitBridgeAccountAction), BridgeLock(BridgeLockAction), BridgeUnlock(BridgeUnlockAction), + FeeChange(FeeChangeAction), } impl Action { @@ -56,6 +57,7 @@ impl Action { Action::InitBridgeAccount(act) => Value::InitBridgeAccountAction(act.into_raw()), Action::BridgeLock(act) => Value::BridgeLockAction(act.into_raw()), Action::BridgeUnlock(act) => Value::BridgeUnlockAction(act.into_raw()), + Action::FeeChange(act) => Value::FeeChangeAction(act.into_raw()), }; raw::Action { value: Some(kind), @@ -80,6 +82,7 @@ impl Action { Action::InitBridgeAccount(act) => Value::InitBridgeAccountAction(act.to_raw()), Action::BridgeLock(act) => Value::BridgeLockAction(act.to_raw()), Action::BridgeUnlock(act) => Value::BridgeUnlockAction(act.to_raw()), + Action::FeeChange(act) => Value::FeeChangeAction(act.to_raw()), }; raw::Action { value: Some(kind), @@ -140,6 +143,9 @@ impl Action { Value::BridgeUnlockAction(act) => Self::BridgeUnlock( BridgeUnlockAction::try_from_raw(act).map_err(ActionError::bridge_unlock)?, ), + Value::FeeChangeAction(act) => Self::FeeChange( + FeeChangeAction::try_from_raw(&act).map_err(ActionError::fee_change)?, + ), }; Ok(action) } @@ -227,6 +233,12 @@ impl From for Action { } } +impl From for Action { + fn from(value: FeeChangeAction) -> Self { + Self::FeeChange(value) + } +} + #[allow(clippy::module_name_repetitions)] #[derive(Debug, thiserror::Error)] #[error(transparent)] @@ -284,6 +296,10 @@ impl ActionError { fn bridge_unlock(inner: BridgeUnlockActionError) -> Self { Self(ActionErrorKind::BridgeUnlock(inner)) } + + fn fee_change(inner: FeeChangeActionError) -> Self { + Self(ActionErrorKind::FeeChange(inner)) + } } #[derive(Debug, thiserror::Error)] @@ -314,6 +330,8 @@ enum ActionErrorKind { BridgeLock(#[source] BridgeLockActionError), #[error("bridge unlock action was not valid")] BridgeUnlock(#[source] BridgeUnlockActionError), + #[error("fee change action was not valid")] + FeeChange(#[source] FeeChangeActionError), } #[derive(Debug, thiserror::Error)] @@ -1384,3 +1402,106 @@ enum BridgeUnlockActionErrorKind { #[error("the `fee_asset_id` field was invalid")] InvalidFeeAssetId(#[source] asset::IncorrectAssetIdLength), } + +#[derive(Debug, Clone)] +pub enum FeeChange { + TransferBaseFee, + SequenceBaseFee, + SequenceByteCostMultiplier, + InitBridgeAccountBaseFee, + BridgeLockByteCostMultiplier, + Ics20WithdrawalBaseFee, +} + +#[allow(clippy::module_name_repetitions)] +#[derive(Debug, Clone)] +pub struct FeeChangeAction { + pub fee_change: FeeChange, + pub new_value: u128, +} + +impl FeeChangeAction { + #[must_use] + pub fn into_raw(self) -> raw::FeeChangeAction { + self.to_raw() + } + + #[must_use] + pub fn to_raw(&self) -> raw::FeeChangeAction { + raw::FeeChangeAction { + value: Some(match self.fee_change { + FeeChange::TransferBaseFee => { + raw::fee_change_action::Value::TransferBaseFee(self.new_value.into()) + } + FeeChange::SequenceBaseFee => { + raw::fee_change_action::Value::SequenceBaseFee(self.new_value.into()) + } + FeeChange::SequenceByteCostMultiplier => { + raw::fee_change_action::Value::SequenceByteCostMultiplier(self.new_value.into()) + } + FeeChange::InitBridgeAccountBaseFee => { + raw::fee_change_action::Value::InitBridgeAccountBaseFee(self.new_value.into()) + } + FeeChange::BridgeLockByteCostMultiplier => { + raw::fee_change_action::Value::BridgeLockByteCostMultiplier( + self.new_value.into(), + ) + } + FeeChange::Ics20WithdrawalBaseFee => { + raw::fee_change_action::Value::Ics20WithdrawalBaseFee(self.new_value.into()) + } + }), + } + } + + /// Convert from a raw, unchecked protobuf [`raw::FeeChangeAction`]. + /// + /// # Errors + /// + /// - if the fee change `value` field is missing + /// - if the `new_value` field is missing + pub fn try_from_raw(proto: &raw::FeeChangeAction) -> Result { + let (fee_change, new_value) = match proto.value { + Some(raw::fee_change_action::Value::TransferBaseFee(new_value)) => { + (FeeChange::TransferBaseFee, new_value) + } + Some(raw::fee_change_action::Value::SequenceBaseFee(new_value)) => { + (FeeChange::SequenceBaseFee, new_value) + } + Some(raw::fee_change_action::Value::SequenceByteCostMultiplier(new_value)) => { + (FeeChange::SequenceByteCostMultiplier, new_value) + } + Some(raw::fee_change_action::Value::InitBridgeAccountBaseFee(new_value)) => { + (FeeChange::InitBridgeAccountBaseFee, new_value) + } + Some(raw::fee_change_action::Value::BridgeLockByteCostMultiplier(new_value)) => { + (FeeChange::BridgeLockByteCostMultiplier, new_value) + } + Some(raw::fee_change_action::Value::Ics20WithdrawalBaseFee(new_value)) => { + (FeeChange::Ics20WithdrawalBaseFee, new_value) + } + None => return Err(FeeChangeActionError::missing_value_to_change()), + }; + + Ok(Self { + fee_change, + new_value: new_value.into(), + }) + } +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct FeeChangeActionError(FeeChangeActionErrorKind); + +impl FeeChangeActionError { + fn missing_value_to_change() -> Self { + Self(FeeChangeActionErrorKind::MissingValueToChange) + } +} + +#[derive(Debug, thiserror::Error)] +enum FeeChangeActionErrorKind { + #[error("the value which to change was missing")] + MissingValueToChange, +} diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs index 6cde5a6dc5..6c5386e0b4 100644 --- a/crates/astria-sequencer/src/authority/action.rs +++ b/crates/astria-sequencer/src/authority/action.rs @@ -6,7 +6,11 @@ use anyhow::{ }; use astria_core::{ primitive::v1::Address, - protocol::transaction::v1alpha1::action::SudoAddressChangeAction, + protocol::transaction::v1alpha1::action::{ + FeeChange, + FeeChangeAction, + SudoAddressChangeAction, + }, }; use tendermint::account; use tracing::instrument; @@ -94,3 +98,183 @@ impl ActionHandler for SudoAddressChangeAction { Ok(()) } } + +#[async_trait::async_trait] +impl ActionHandler for FeeChangeAction { + /// check that the signer of the transaction is the current sudo address, + /// as only that address can change the fee + async fn check_stateful( + &self, + state: &S, + from: Address, + ) -> Result<()> { + // ensure signer is the valid `sudo` key in state + let sudo_address = state + .get_sudo_address() + .await + .context("failed to get sudo address from state")?; + ensure!(sudo_address == from, "signer is not the sudo key"); + Ok(()) + } + + #[instrument(skip_all)] + async fn execute(&self, state: &mut S, _: Address) -> Result<()> { + use crate::{ + accounts::state_ext::StateWriteExt as _, + bridge::state_ext::StateWriteExt as _, + ibc::state_ext::StateWriteExt as _, + sequence::state_ext::StateWriteExt as _, + }; + + match self.fee_change { + FeeChange::TransferBaseFee => { + state + .put_transfer_base_fee(self.new_value) + .context("failed to put transfer base fee in state")?; + } + FeeChange::SequenceBaseFee => state.put_sequence_action_base_fee(self.new_value), + FeeChange::SequenceByteCostMultiplier => { + state.put_sequence_action_byte_cost_multiplier(self.new_value); + } + FeeChange::InitBridgeAccountBaseFee => { + state.put_init_bridge_account_base_fee(self.new_value); + } + FeeChange::BridgeLockByteCostMultiplier => { + state.put_bridge_lock_byte_cost_multiplier(self.new_value); + } + FeeChange::Ics20WithdrawalBaseFee => { + state + .put_ics20_withdrawal_base_fee(self.new_value) + .context("failed to put ics20 withdrawal base fee in state")?; + } + } + + Ok(()) + } +} + +#[cfg(test)] +mod test { + use cnidarium::StateDelta; + + use super::*; + use crate::{ + accounts::state_ext::{ + StateReadExt as _, + StateWriteExt as _, + }, + bridge::state_ext::{ + StateReadExt as _, + StateWriteExt as _, + }, + ibc::state_ext::{ + StateReadExt as _, + StateWriteExt as _, + }, + sequence::state_ext::{ + StateReadExt as _, + StateWriteExt as _, + }, + }; + + #[tokio::test] + async fn fee_change_action_execute() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + let transfer_fee = 12; + state.put_transfer_base_fee(transfer_fee).unwrap(); + + let fee_change = FeeChangeAction { + fee_change: FeeChange::TransferBaseFee, + new_value: 10, + }; + + fee_change + .execute(&mut state, Address::from([1; 20])) + .await + .unwrap(); + assert_eq!(state.get_transfer_base_fee().await.unwrap(), 10); + + let sequence_base_fee = 5; + state.put_sequence_action_base_fee(sequence_base_fee); + + let fee_change = FeeChangeAction { + fee_change: FeeChange::SequenceBaseFee, + new_value: 3, + }; + + fee_change + .execute(&mut state, Address::from([1; 20])) + .await + .unwrap(); + assert_eq!(state.get_sequence_action_base_fee().await.unwrap(), 3); + + let sequence_byte_cost_multiplier = 2; + state.put_sequence_action_byte_cost_multiplier(sequence_byte_cost_multiplier); + + let fee_change = FeeChangeAction { + fee_change: FeeChange::SequenceByteCostMultiplier, + new_value: 4, + }; + + fee_change + .execute(&mut state, Address::from([1; 20])) + .await + .unwrap(); + assert_eq!( + state + .get_sequence_action_byte_cost_multiplier() + .await + .unwrap(), + 4 + ); + + let init_bridge_account_base_fee = 1; + state.put_init_bridge_account_base_fee(init_bridge_account_base_fee); + + let fee_change = FeeChangeAction { + fee_change: FeeChange::InitBridgeAccountBaseFee, + new_value: 2, + }; + + fee_change + .execute(&mut state, Address::from([1; 20])) + .await + .unwrap(); + assert_eq!(state.get_init_bridge_account_base_fee().await.unwrap(), 2); + + let bridge_lock_byte_cost_multiplier = 1; + state.put_bridge_lock_byte_cost_multiplier(bridge_lock_byte_cost_multiplier); + + let fee_change = FeeChangeAction { + fee_change: FeeChange::BridgeLockByteCostMultiplier, + new_value: 2, + }; + + fee_change + .execute(&mut state, Address::from([1; 20])) + .await + .unwrap(); + assert_eq!( + state.get_bridge_lock_byte_cost_multiplier().await.unwrap(), + 2 + ); + + let ics20_withdrawal_base_fee = 1; + state + .put_ics20_withdrawal_base_fee(ics20_withdrawal_base_fee) + .unwrap(); + + let fee_change = FeeChangeAction { + fee_change: FeeChange::Ics20WithdrawalBaseFee, + new_value: 2, + }; + + fee_change + .execute(&mut state, Address::from([1; 20])) + .await + .unwrap(); + assert_eq!(state.get_ics20_withdrawal_base_fee().await.unwrap(), 2); + } +} diff --git a/crates/astria-sequencer/src/bridge/mod.rs b/crates/astria-sequencer/src/bridge/mod.rs index b808bd84fb..6d2570f725 100644 --- a/crates/astria-sequencer/src/bridge/mod.rs +++ b/crates/astria-sequencer/src/bridge/mod.rs @@ -4,5 +4,4 @@ pub(crate) mod component; pub(crate) mod init_bridge_account_action; pub(crate) mod state_ext; -#[cfg(test)] pub(crate) use bridge_lock_action::get_deposit_byte_len; diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs new file mode 100644 index 0000000000..1d3d8a7454 --- /dev/null +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -0,0 +1,425 @@ +use std::collections::HashMap; + +use anyhow::{ + ensure, + Context as _, +}; +use astria_core::{ + primitive::v1::{ + asset, + Address, + RollupId, + }, + protocol::transaction::v1alpha1::{ + action::{ + Action, + BridgeLockAction, + }, + SignedTransaction, + UnsignedTransaction, + }, +}; + +use crate::{ + accounts::state_ext::StateReadExt, + bridge::state_ext::StateReadExt as _, + ibc::state_ext::StateReadExt as _, + state_ext::StateReadExt as _, +}; + +pub(crate) async fn check_nonce_mempool( + tx: &SignedTransaction, + state: &S, +) -> anyhow::Result<()> { + let signer_address = Address::from_verification_key(tx.verification_key()); + let curr_nonce = state + .get_account_nonce(signer_address) + .await + .context("failed to get account nonce")?; + ensure!( + tx.unsigned_transaction().params.nonce >= curr_nonce, + "nonce already used by account" + ); + Ok(()) +} + +pub(crate) async fn check_chain_id_mempool( + tx: &SignedTransaction, + state: &S, +) -> anyhow::Result<()> { + let chain_id = state + .get_chain_id() + .await + .context("failed to get chain id")?; + ensure!( + tx.unsigned_transaction().params.chain_id == chain_id.as_str(), + "chain id mismatch" + ); + Ok(()) +} + +pub(crate) async fn check_balance_mempool( + tx: &SignedTransaction, + state: &S, +) -> anyhow::Result<()> { + let signer_address = Address::from_verification_key(tx.verification_key()); + check_balance_for_total_fees(tx.unsigned_transaction(), signer_address, state).await?; + Ok(()) +} + +// Checks that the account has enough balance to cover the total fees and transferred values +// for all actions in the transaction. +pub(crate) async fn check_balance_for_total_fees( + tx: &UnsignedTransaction, + from: Address, + state: &S, +) -> anyhow::Result<()> { + let transfer_fee = state + .get_transfer_base_fee() + .await + .context("failed to get transfer base fee")?; + let ics20_withdrawal_fee = state + .get_ics20_withdrawal_base_fee() + .await + .context("failed to get ics20 withdrawal base fee")?; + let init_bridge_account_fee = state + .get_init_bridge_account_base_fee() + .await + .context("failed to get init bridge account base fee")?; + let bridge_lock_byte_cost_multiplier = state + .get_bridge_lock_byte_cost_multiplier() + .await + .context("failed to get bridge lock byte cost multiplier")?; + + let mut fees_by_asset = HashMap::new(); + for action in &tx.actions { + match action { + Action::Transfer(act) => transfer_update_fees( + act.asset_id, + act.fee_asset_id, + 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?; + } + Action::Ics20Withdrawal(act) => ics20_withdrawal_updates_fees( + act.denom().id(), + *act.fee_asset_id(), + act.amount(), + &mut fees_by_asset, + ics20_withdrawal_fee, + ), + Action::InitBridgeAccount(act) => { + fees_by_asset + .entry(act.fee_asset_id) + .and_modify(|amt| *amt = amt.saturating_add(init_bridge_account_fee)) + .or_insert(init_bridge_account_fee); + } + Action::BridgeLock(act) => bridge_lock_update_fees( + act, + &mut fees_by_asset, + transfer_fee, + bridge_lock_byte_cost_multiplier, + ), + Action::BridgeUnlock(act) => { + bridge_unlock_update_fees( + state, + from, + act.amount, + act.fee_asset_id, + &mut fees_by_asset, + transfer_fee, + ) + .await?; + } + Action::ValidatorUpdate(_) + | Action::SudoAddressChange(_) + | Action::Ibc(_) + | Action::IbcRelayerChange(_) + | Action::FeeAssetChange(_) + | Action::FeeChange(_) + | Action::Mint(_) => { + continue; + } + } + } + for (asset, total_fee) in fees_by_asset { + let balance = state + .get_account_balance(from, asset) + .await + .context("failed to get account balance")?; + ensure!( + balance >= total_fee, + "insufficient funds for asset {}", + asset + ); + } + + Ok(()) +} + +fn transfer_update_fees( + asset_id: asset::Id, + fee_asset_id: asset::Id, + amount: u128, + fees_by_asset: &mut HashMap, + transfer_fee: u128, +) { + fees_by_asset + .entry(asset_id) + .and_modify(|amt: &mut u128| *amt = amt.saturating_add(amount)) + .or_insert(amount); + fees_by_asset + .entry(fee_asset_id) + .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, + 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) + .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, + amount: u128, + fees_by_asset: &mut HashMap, + ics20_withdrawal_fee: u128, +) { + fees_by_asset + .entry(asset_id) + .and_modify(|amt| *amt = amt.saturating_add(amount)) + .or_insert(amount); + fees_by_asset + .entry(fee_asset_id) + .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, + transfer_fee: u128, + bridge_lock_byte_cost_multiplier: u128, +) { + use astria_core::sequencerblock::v1alpha1::block::Deposit; + + let expected_deposit_fee = transfer_fee.saturating_add( + crate::bridge::get_deposit_byte_len(&Deposit::new( + act.to, + // 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.destination_chain_address.clone(), + )) + .saturating_mul(bridge_lock_byte_cost_multiplier), + ); + + fees_by_asset + .entry(act.asset_id) + .and_modify(|amt: &mut u128| *amt = amt.saturating_add(act.amount)) + .or_insert(act.amount); + fees_by_asset + .entry(act.asset_id) + .and_modify(|amt| *amt = amt.saturating_add(expected_deposit_fee)) + .or_insert(expected_deposit_fee); +} + +async fn bridge_unlock_update_fees( + state: &S, + from: Address, + amount: u128, + fee_asset_id: asset::Id, + fees_by_asset: &mut HashMap, + transfer_fee: u128, +) -> anyhow::Result<()> { + let asset_id = state + .get_bridge_account_asset_id(&from) + .await + .context("must be a bridge account for BridgeUnlock action")?; + fees_by_asset + .entry(asset_id) + .and_modify(|amt: &mut u128| *amt = amt.saturating_add(amount)) + .or_insert(amount); + fees_by_asset + .entry(fee_asset_id) + .and_modify(|amt| *amt = amt.saturating_add(transfer_fee)) + .or_insert(transfer_fee); + Ok(()) +} + +#[cfg(test)] +mod test { + use astria_core::{ + primitive::v1::{ + asset::{ + Denom, + DEFAULT_NATIVE_ASSET_DENOM, + }, + RollupId, + ADDRESS_LEN, + }, + protocol::transaction::v1alpha1::{ + action::{ + SequenceAction, + TransferAction, + }, + TransactionParams, + }, + }; + use cnidarium::StateDelta; + + use super::*; + use crate::{ + accounts::state_ext::StateWriteExt as _, + app::test_utils::*, + bridge::state_ext::StateWriteExt, + ibc::state_ext::StateWriteExt as _, + sequence::state_ext::StateWriteExt as _, + }; + + #[tokio::test] + async fn check_balance_mempool_ok() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state_tx = StateDelta::new(snapshot); + + state_tx.put_transfer_base_fee(12).unwrap(); + state_tx.put_sequence_action_base_fee(0); + state_tx.put_sequence_action_byte_cost_multiplier(1); + state_tx.put_ics20_withdrawal_base_fee(1).unwrap(); + state_tx.put_init_bridge_account_base_fee(12); + state_tx.put_bridge_lock_byte_cost_multiplier(1); + + crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); + let native_asset = crate::asset::get_native_asset().id(); + let other_asset = Denom::from_base_denom("other").id(); + + let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let amount = 100; + let data = [0; 32].to_vec(); + let transfer_fee = state_tx.get_transfer_base_fee().await.unwrap(); + state_tx + .increase_balance( + alice_address, + native_asset, + transfer_fee + + crate::sequence::calculate_fee_from_state(&data, &state_tx) + .await + .unwrap(), + ) + .await + .unwrap(); + state_tx + .increase_balance(alice_address, other_asset, amount) + .await + .unwrap(); + + let actions = vec![ + Action::Transfer(TransferAction { + asset_id: other_asset, + amount, + fee_asset_id: native_asset, + to: [0; ADDRESS_LEN].into(), + }), + Action::Sequence(SequenceAction { + rollup_id: RollupId::from_unhashed_bytes([0; 32]), + data, + fee_asset_id: native_asset, + }), + ]; + + let params = TransactionParams { + nonce: 0, + chain_id: "test-chain-id".to_string(), + }; + let tx = UnsignedTransaction { + actions, + params, + }; + + let signed_tx = tx.into_signed(&alice_signing_key); + check_balance_mempool(&signed_tx, &state_tx) + .await + .expect("sufficient balance for all actions"); + } + + #[tokio::test] + async fn check_balance_mempool_insufficient_other_asset_balance() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state_tx = StateDelta::new(snapshot); + + state_tx.put_transfer_base_fee(12).unwrap(); + state_tx.put_sequence_action_base_fee(0); + state_tx.put_sequence_action_byte_cost_multiplier(1); + state_tx.put_ics20_withdrawal_base_fee(1).unwrap(); + state_tx.put_init_bridge_account_base_fee(12); + state_tx.put_bridge_lock_byte_cost_multiplier(1); + + crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); + let native_asset = crate::asset::get_native_asset().id(); + let other_asset = Denom::from_base_denom("other").id(); + + let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); + let amount = 100; + let data = [0; 32].to_vec(); + let transfer_fee = state_tx.get_transfer_base_fee().await.unwrap(); + state_tx + .increase_balance( + alice_address, + native_asset, + transfer_fee + + crate::sequence::calculate_fee_from_state(&data, &state_tx) + .await + .unwrap(), + ) + .await + .unwrap(); + + let actions = vec![ + Action::Transfer(TransferAction { + asset_id: other_asset, + amount, + fee_asset_id: native_asset, + to: [0; ADDRESS_LEN].into(), + }), + Action::Sequence(SequenceAction { + rollup_id: RollupId::from_unhashed_bytes([0; 32]), + data, + fee_asset_id: native_asset, + }), + ]; + + let params = TransactionParams { + nonce: 0, + chain_id: "test-chain-id".to_string(), + }; + let tx = UnsignedTransaction { + actions, + params, + }; + + let signed_tx = tx.into_signed(&alice_signing_key); + 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())); + } +} diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index c81034a9ac..3a03d65cba 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod action_handler; +mod checks; use std::fmt; @@ -17,6 +18,12 @@ use astria_core::{ UnsignedTransaction, }, }; +pub(crate) use checks::{ + check_balance_for_total_fees, + check_balance_mempool, + check_chain_id_mempool, + check_nonce_mempool, +}; use tracing::instrument; use crate::{ @@ -24,7 +31,6 @@ use crate::{ StateReadExt, StateWriteExt, }, - bridge::state_ext::StateReadExt as _, ibc::{ host_interface::AstriaHost, state_ext::StateReadExt as _, @@ -32,155 +38,6 @@ use crate::{ state_ext::StateReadExt as _, }; -pub(crate) async fn check_nonce_mempool( - tx: &SignedTransaction, - state: &S, -) -> anyhow::Result<()> { - let signer_address = Address::from_verification_key(tx.verification_key()); - let curr_nonce = state - .get_account_nonce(signer_address) - .await - .context("failed to get account nonce")?; - ensure!( - tx.unsigned_transaction().params.nonce >= curr_nonce, - "nonce already used by account" - ); - Ok(()) -} - -pub(crate) async fn check_chain_id_mempool( - tx: &SignedTransaction, - state: &S, -) -> anyhow::Result<()> { - let chain_id = state - .get_chain_id() - .await - .context("failed to get chain id")?; - ensure!( - tx.unsigned_transaction().params.chain_id == chain_id.as_str(), - "chain id mismatch" - ); - Ok(()) -} - -pub(crate) async fn check_balance_mempool( - tx: &SignedTransaction, - state: &S, -) -> anyhow::Result<()> { - let signer_address = Address::from_verification_key(tx.verification_key()); - check_balance_for_total_fees(tx.unsigned_transaction(), signer_address, state).await?; - Ok(()) -} - -// Checks that the account has enough balance to cover the total fees for all actions in the -// transaction. -pub(crate) async fn check_balance_for_total_fees( - tx: &UnsignedTransaction, - from: Address, - state: &S, -) -> anyhow::Result<()> { - use std::collections::HashMap; - - let transfer_fee = state - .get_transfer_base_fee() - .await - .context("failed to get transfer base fee")?; - let ics20_withdrawal_fee = state - .get_ics20_withdrawal_base_fee() - .await - .context("failed to get ics20 withdrawal base fee")?; - let init_bridge_account_fee = state - .get_init_bridge_account_base_fee() - .await - .context("failed to get init bridge account base fee")?; - - let mut fees_by_asset = HashMap::new(); - for action in &tx.actions { - match action { - Action::Transfer(act) => { - fees_by_asset - .entry(act.asset_id) - .and_modify(|amt: &mut u128| *amt = amt.saturating_add(act.amount)) - .or_insert(act.amount); - fees_by_asset - .entry(act.fee_asset_id) - .and_modify(|amt| *amt = amt.saturating_add(transfer_fee)) - .or_insert(transfer_fee); - } - Action::Sequence(act) => { - let fee = crate::sequence::calculate_fee_from_state(&act.data, state) - .await - .context("fee for sequence action overflowed; data too large")?; - fees_by_asset - .entry(act.fee_asset_id) - .and_modify(|amt| *amt = amt.saturating_add(fee)) - .or_insert(fee); - } - Action::Ics20Withdrawal(act) => { - fees_by_asset - .entry(act.denom().id()) - .and_modify(|amt| *amt = amt.saturating_add(act.amount())) - .or_insert(act.amount()); - fees_by_asset - .entry(*act.fee_asset_id()) - .and_modify(|amt| *amt = amt.saturating_add(ics20_withdrawal_fee)) - .or_insert(ics20_withdrawal_fee); - } - Action::InitBridgeAccount(act) => { - fees_by_asset - .entry(act.fee_asset_id) - .and_modify(|amt| *amt = amt.saturating_add(init_bridge_account_fee)) - .or_insert(init_bridge_account_fee); - } - Action::BridgeLock(act) => { - fees_by_asset - .entry(act.asset_id) - .and_modify(|amt| *amt = amt.saturating_add(act.amount)) - .or_insert(act.amount); - fees_by_asset - .entry(act.fee_asset_id) - .and_modify(|amt| *amt = amt.saturating_add(transfer_fee)) - .or_insert(transfer_fee); - } - Action::BridgeUnlock(act) => { - let asset_id = state - .get_bridge_account_asset_id(&from) - .await - .context("must be a bridge account for BridgeUnlock action")?; - fees_by_asset - .entry(asset_id) - .and_modify(|amt: &mut u128| *amt = amt.saturating_add(act.amount)) - .or_insert(act.amount); - fees_by_asset - .entry(act.fee_asset_id) - .and_modify(|amt| *amt = amt.saturating_add(transfer_fee)) - .or_insert(transfer_fee); - } - Action::ValidatorUpdate(_) - | Action::SudoAddressChange(_) - | Action::Ibc(_) - | Action::IbcRelayerChange(_) - | Action::FeeAssetChange(_) - | Action::Mint(_) => { - continue; - } - } - } - for (asset, total_fee) in fees_by_asset { - let balance = state - .get_account_balance(from, asset) - .await - .context("failed to get account balance")?; - ensure!( - balance >= total_fee, - "insufficient funds for asset {}", - asset - ); - } - - Ok(()) -} - pub(crate) async fn check_stateless(tx: &SignedTransaction) -> anyhow::Result<()> { tx.unsigned_transaction() .check_stateless() @@ -290,6 +147,10 @@ impl ActionHandler for UnsignedTransaction { .check_stateless() .await .context("stateless check failed for BridgeLockAction")?, + Action::FeeChange(act) => act + .check_stateless() + .await + .context("stateless check failed for FeeChangeAction")?, Action::BridgeUnlock(act) => act .check_stateless() .await @@ -376,6 +237,10 @@ impl ActionHandler for UnsignedTransaction { .check_stateful(state, from) .await .context("stateful check failed for BridgeLockAction")?, + Action::FeeChange(act) => act + .check_stateful(state, from) + .await + .context("stateful check failed for FeeChangeAction")?, Action::BridgeUnlock(act) => act .check_stateful(state, from) .await @@ -468,6 +333,11 @@ impl ActionHandler for UnsignedTransaction { .await .context("execution failed for BridgeLockAction")?; } + Action::FeeChange(act) => { + act.execute(state, from) + .await + .context("execution failed for FeeChangeAction")?; + } Action::BridgeUnlock(act) => { act.execute(state, from) .await @@ -487,163 +357,3 @@ impl ActionHandler for UnsignedTransaction { Ok(()) } } - -#[cfg(test)] -mod test { - use astria_core::{ - primitive::v1::{ - asset::{ - Denom, - DEFAULT_NATIVE_ASSET_DENOM, - }, - RollupId, - ADDRESS_LEN, - }, - protocol::transaction::v1alpha1::{ - action::{ - SequenceAction, - TransferAction, - }, - TransactionParams, - }, - }; - use cnidarium::StateDelta; - - use super::*; - use crate::{ - accounts::state_ext::StateWriteExt as _, - app::test_utils::*, - bridge::state_ext::StateWriteExt, - ibc::state_ext::StateWriteExt as _, - sequence::state_ext::StateWriteExt as _, - }; - - #[tokio::test] - async fn check_balance_mempool_ok() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state_tx = StateDelta::new(snapshot); - - state_tx.put_transfer_base_fee(12).unwrap(); - state_tx.put_sequence_action_base_fee(0); - state_tx.put_sequence_action_byte_cost_multiplier(1); - state_tx.put_ics20_withdrawal_base_fee(1).unwrap(); - state_tx.put_init_bridge_account_base_fee(12); - state_tx.put_bridge_lock_byte_cost_multiplier(1); - - crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); - let native_asset = crate::asset::get_native_asset().id(); - let other_asset = Denom::from_base_denom("other").id(); - - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); - let amount = 100; - let data = [0; 32].to_vec(); - let transfer_fee = state_tx.get_transfer_base_fee().await.unwrap(); - state_tx - .increase_balance( - alice_address, - native_asset, - transfer_fee - + crate::sequence::calculate_fee_from_state(&data, &state_tx) - .await - .unwrap(), - ) - .await - .unwrap(); - state_tx - .increase_balance(alice_address, other_asset, amount) - .await - .unwrap(); - - let actions = vec![ - Action::Transfer(TransferAction { - asset_id: other_asset, - amount, - fee_asset_id: native_asset, - to: [0; ADDRESS_LEN].into(), - }), - Action::Sequence(SequenceAction { - rollup_id: RollupId::from_unhashed_bytes([0; 32]), - data, - fee_asset_id: native_asset, - }), - ]; - - let params = TransactionParams { - nonce: 0, - chain_id: "test-chain-id".to_string(), - }; - let tx = UnsignedTransaction { - actions, - params, - }; - - let signed_tx = tx.into_signed(&alice_signing_key); - check_balance_mempool(&signed_tx, &state_tx) - .await - .expect("sufficient balance for all actions"); - } - - #[tokio::test] - async fn check_balance_mempool_insufficient_other_asset_balance() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state_tx = StateDelta::new(snapshot); - - state_tx.put_transfer_base_fee(12).unwrap(); - state_tx.put_sequence_action_base_fee(0); - state_tx.put_sequence_action_byte_cost_multiplier(1); - state_tx.put_ics20_withdrawal_base_fee(1).unwrap(); - state_tx.put_init_bridge_account_base_fee(12); - state_tx.put_bridge_lock_byte_cost_multiplier(1); - - crate::asset::initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); - let native_asset = crate::asset::get_native_asset().id(); - let other_asset = Denom::from_base_denom("other").id(); - - let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); - let amount = 100; - let data = [0; 32].to_vec(); - let transfer_fee = state_tx.get_transfer_base_fee().await.unwrap(); - state_tx - .increase_balance( - alice_address, - native_asset, - transfer_fee - + crate::sequence::calculate_fee_from_state(&data, &state_tx) - .await - .unwrap(), - ) - .await - .unwrap(); - - let actions = vec![ - Action::Transfer(TransferAction { - asset_id: other_asset, - amount, - fee_asset_id: native_asset, - to: [0; ADDRESS_LEN].into(), - }), - Action::Sequence(SequenceAction { - rollup_id: RollupId::from_unhashed_bytes([0; 32]), - data, - fee_asset_id: native_asset, - }), - ]; - - let params = TransactionParams { - nonce: 0, - chain_id: "test-chain-id".to_string(), - }; - let tx = UnsignedTransaction { - actions, - params, - }; - - let signed_tx = tx.into_signed(&alice_signing_key); - 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())); - } -} diff --git a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto index 39b19d60e4..5be997d2ac 100644 --- a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto @@ -53,11 +53,12 @@ message Action { IbcRelayerChangeAction ibc_relayer_change_action = 52; FeeAssetChangeAction fee_asset_change_action = 53; MintAction mint_action = 54; + FeeChangeAction fee_change_action = 55; } reserved 3 to 10; reserved 14 to 20; reserved 23 to 30; - reserved 55 to 60; + reserved 56 to 60; } // `TransferAction` represents a value transfer transaction. @@ -201,3 +202,21 @@ message BridgeUnlockAction { // memo for double spend prevention bytes memo = 4; } + +message FeeChangeAction { + // note that the proto number ranges are doubled from that of `Action`. + // this to accomodate both `base_fee` and `byte_cost_multiplier` for each action. + oneof value { + // core protocol fees are defined on 1-20 + astria.primitive.v1.Uint128 transfer_base_fee = 1; + astria.primitive.v1.Uint128 sequence_base_fee = 2; + astria.primitive.v1.Uint128 sequence_byte_cost_multiplier = 3; + + // bridge fees are defined on 20-39 + astria.primitive.v1.Uint128 init_bridge_account_base_fee = 20; + astria.primitive.v1.Uint128 bridge_lock_byte_cost_multiplier = 21; + + // ibc fees are defined on 40-59 + astria.primitive.v1.Uint128 ics20_withdrawal_base_fee = 40; + } +}