From 55f16b9627c30e69e4c4c47f5b7e3773dbaebc41 Mon Sep 17 00:00:00 2001 From: noot <36753753+noot@users.noreply.github.com> Date: Thu, 9 May 2024 18:21:26 -0400 Subject: [PATCH] feat(sequencer)!: add fees to genesis state (#1055) ## Summary instead of having hard coded default fee values, the fees now must be set in the genesis file. this makes it more explicit as to how the network is configured. ## Background related to https://github.com/astriaorg/astria/issues/1003 ## Changes - update genesis app state to contain required fees - set these fees in state in `init_chain` ## Testing unit tests ## Breaking Changelist - genesis app state must now contain fees ## Related Issues related to https://github.com/astriaorg/astria/issues/1003 --- charts/sequencer/Chart.yaml | 2 +- .../files/cometbft/config/genesis.json | 8 ++++++++ .../src/accounts/component.rs | 6 +----- crates/astria-sequencer/src/app/test_utils.rs | 13 +++++++++++++ crates/astria-sequencer/src/app/tests_app.rs | 1 + .../src/app/tests_execute_transaction.rs | 10 ++++++++++ .../astria-sequencer/src/bridge/component.rs | 13 ++----------- crates/astria-sequencer/src/genesis.rs | 19 +++++++++++++++++++ crates/astria-sequencer/src/ibc/component.rs | 4 +--- .../src/sequence/component.rs | 10 ++-------- .../astria-sequencer/src/service/consensus.rs | 2 ++ .../astria-sequencer/src/transaction/mod.rs | 8 ++------ .../test-genesis-app-state.json | 8 ++++++++ 13 files changed, 70 insertions(+), 34 deletions(-) diff --git a/charts/sequencer/Chart.yaml b/charts/sequencer/Chart.yaml index 84b88f25de..9fb38e2896 100644 --- a/charts/sequencer/Chart.yaml +++ b/charts/sequencer/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.13.5 +version: 0.13.6 # 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/sequencer/files/cometbft/config/genesis.json b/charts/sequencer/files/cometbft/config/genesis.json index a7b0575002..1e0e5566c9 100644 --- a/charts/sequencer/files/cometbft/config/genesis.json +++ b/charts/sequencer/files/cometbft/config/genesis.json @@ -12,6 +12,14 @@ ], "authority_sudo_address": "{{ .Values.config.sequencer.authoritySudoAddress }}", "native_asset_base_denomination": "{{ .Values.config.sequencer.nativeAssetBaseDenomination }}", + "fees": { + "transfer_base_fee": 12, + "sequence_base_fee": 32, + "sequence_byte_cost_multiplier": 1, + "init_bridge_account_base_fee": 48, + "bridge_lock_byte_cost_multiplier": 1, + "ics20_withdrawal_base_fee": 24 + }, "allowed_fee_assets": [ {{- range $index, $value := .Values.config.sequencer.allowedFeeAssets }} {{- if $index }},{{- end }} diff --git a/crates/astria-sequencer/src/accounts/component.rs b/crates/astria-sequencer/src/accounts/component.rs index f50896293f..376b9c6ceb 100644 --- a/crates/astria-sequencer/src/accounts/component.rs +++ b/crates/astria-sequencer/src/accounts/component.rs @@ -17,10 +17,6 @@ use crate::{ genesis::GenesisState, }; -/// Default transfer base fee. -/// TODO: put in app genesis state -pub(crate) const DEFAULT_TRANSFER_BASE_FEE: u128 = 12; - #[derive(Default)] pub(crate) struct AccountsComponent; @@ -38,7 +34,7 @@ impl Component for AccountsComponent { } state - .put_transfer_base_fee(DEFAULT_TRANSFER_BASE_FEE) + .put_transfer_base_fee(app_state.fees.transfer_base_fee) .context("failed to put transfer base fee")?; Ok(()) } diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 9f930ef0b3..347c12370e 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -19,6 +19,7 @@ use penumbra_ibc::params::IBCParameters; use crate::{ app::App, genesis::{ + self, Account, GenesisState, }, @@ -65,6 +66,17 @@ pub(crate) fn default_genesis_accounts() -> Vec { ] } +pub(crate) fn default_fees() -> genesis::Fees { + genesis::Fees { + transfer_base_fee: 12, + sequence_base_fee: 32, + sequence_byte_cost_multiplier: 1, + init_bridge_account_base_fee: 48, + bridge_lock_byte_cost_multiplier: 1, + ics20_withdrawal_base_fee: 24, + } +} + pub(crate) async fn initialize_app_with_storage( genesis_state: Option, genesis_validators: Vec, @@ -84,6 +96,7 @@ pub(crate) async fn initialize_app_with_storage( native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }); app.init_chain( diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index 8fa93de893..e23d82cfb1 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -199,6 +199,7 @@ async fn app_commit() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let (mut app, storage) = initialize_app_with_storage(Some(genesis_state), vec![]).await; diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 127880463e..0d2da2238f 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -283,6 +283,7 @@ async fn app_execute_transaction_validator_update() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -321,6 +322,7 @@ async fn app_execute_transaction_ibc_relayer_change_addition() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], ibc_params: IBCParameters::default(), + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -350,6 +352,7 @@ async fn app_execute_transaction_ibc_relayer_change_deletion() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], ibc_params: IBCParameters::default(), + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -379,6 +382,7 @@ async fn app_execute_transaction_ibc_relayer_change_invalid() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], ibc_params: IBCParameters::default(), + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -406,6 +410,7 @@ async fn app_execute_transaction_sudo_address_change() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -442,6 +447,7 @@ async fn app_execute_transaction_sudo_address_change_error() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -479,6 +485,7 @@ async fn app_execute_transaction_fee_asset_change_addition() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -519,6 +526,7 @@ async fn app_execute_transaction_fee_asset_change_removal() { DEFAULT_NATIVE_ASSET_DENOM.to_owned().into(), test_asset.clone(), ], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -558,6 +566,7 @@ async fn app_execute_transaction_fee_asset_change_invalid() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; @@ -844,6 +853,7 @@ async fn app_execute_transaction_mint() { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), }; let mut app = initialize_app(Some(genesis_state), vec![]).await; diff --git a/crates/astria-sequencer/src/bridge/component.rs b/crates/astria-sequencer/src/bridge/component.rs index 492668367f..b9e27ba32a 100644 --- a/crates/astria-sequencer/src/bridge/component.rs +++ b/crates/astria-sequencer/src/bridge/component.rs @@ -13,15 +13,6 @@ use crate::{ genesis::GenesisState, }; -/// Default base fee for a [`InitBridgeAccountAction`]. -const DEFAULT_INIT_BRIDGE_ACCOUNT_BASE_FEE: u128 = 48; - -/// Default multiplier for the cost of a byte in a [`BridgeLockAction`]. -/// -/// Note that the base fee for a [`BridgeLockAction`] is the same as the -/// base fee for a [`TransferAction`]. -const DEFAULT_BRIDGE_LOCK_BYTE_COST_MULTIPLIER: u128 = 1; - #[derive(Default)] pub(crate) struct BridgeComponent; @@ -31,8 +22,8 @@ impl Component for BridgeComponent { #[instrument(name = "BridgeComponent::init_chain", skip(state))] async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> { - state.put_init_bridge_account_base_fee(DEFAULT_INIT_BRIDGE_ACCOUNT_BASE_FEE); - state.put_bridge_lock_byte_cost_multiplier(DEFAULT_BRIDGE_LOCK_BYTE_COST_MULTIPLIER); + state.put_init_bridge_account_base_fee(app_state.fees.init_bridge_account_base_fee); + state.put_bridge_lock_byte_cost_multiplier(app_state.fees.bridge_lock_byte_cost_multiplier); Ok(()) } diff --git a/crates/astria-sequencer/src/genesis.rs b/crates/astria-sequencer/src/genesis.rs index 05555fa420..f6972f8412 100644 --- a/crates/astria-sequencer/src/genesis.rs +++ b/crates/astria-sequencer/src/genesis.rs @@ -22,6 +22,17 @@ pub(crate) struct GenesisState { pub(crate) ibc_params: IBCParameters, #[serde(deserialize_with = "deserialize_assets")] pub(crate) allowed_fee_assets: Vec, + pub(crate) fees: Fees, +} + +#[derive(Debug, Deserialize)] +pub(crate) struct Fees { + pub(crate) transfer_base_fee: u128, + pub(crate) sequence_base_fee: u128, + pub(crate) sequence_byte_cost_multiplier: u128, + pub(crate) init_bridge_account_base_fee: u128, + pub(crate) bridge_lock_byte_cost_multiplier: u128, + pub(crate) ics20_withdrawal_base_fee: u128, } #[derive(Debug, Deserialize)] @@ -101,6 +112,14 @@ mod test { "inbound_ics20_transfers_enabled": true, "outbound_ics20_transfers_enabled": true }, + "fees": { + "transfer_base_fee": 12, + "sequence_base_fee": 32, + "sequence_byte_cost_multiplier": 1, + "init_bridge_account_base_fee": 48, + "bridge_lock_byte_cost_multiplier": 1, + "ics20_withdrawal_base_fee": 24 + }, "native_asset_base_denomination": "nria", "allowed_fee_assets": ["nria"] } diff --git a/crates/astria-sequencer/src/ibc/component.rs b/crates/astria-sequencer/src/ibc/component.rs index f19cd785da..7d7a61b502 100644 --- a/crates/astria-sequencer/src/ibc/component.rs +++ b/crates/astria-sequencer/src/ibc/component.rs @@ -23,8 +23,6 @@ use crate::{ }, }; -pub(crate) const DEFAULT_ICS20_WITHDRAWAL_FEE: u128 = 24; - #[derive(Default)] pub(crate) struct IbcComponent; @@ -51,7 +49,7 @@ impl Component for IbcComponent { } state - .put_ics20_withdrawal_base_fee(DEFAULT_ICS20_WITHDRAWAL_FEE) + .put_ics20_withdrawal_base_fee(app_state.fees.ics20_withdrawal_base_fee) .context("failed to put ics20 withdrawal base fee")?; Ok(()) } diff --git a/crates/astria-sequencer/src/sequence/component.rs b/crates/astria-sequencer/src/sequence/component.rs index d76724f110..28b733f2d4 100644 --- a/crates/astria-sequencer/src/sequence/component.rs +++ b/crates/astria-sequencer/src/sequence/component.rs @@ -13,12 +13,6 @@ use crate::{ genesis::GenesisState, }; -/// Default base fee for a [`SequenceAction`]. -const DEFAULT_SEQUENCE_ACTION_BASE_FEE: u128 = 32; - -/// Default multiplier for the cost of a byte in a [`SequenceAction`]. -const DEFAULT_SEQUENCE_ACTION_BYTE_COST_MULTIPLIER: u128 = 1; - #[derive(Default)] pub(crate) struct SequenceComponent; @@ -28,9 +22,9 @@ impl Component for SequenceComponent { #[instrument(name = "SequenceComponent::init_chain", skip(state))] async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> { - state.put_sequence_action_base_fee(DEFAULT_SEQUENCE_ACTION_BASE_FEE); + state.put_sequence_action_base_fee(app_state.fees.sequence_base_fee); state - .put_sequence_action_byte_cost_multiplier(DEFAULT_SEQUENCE_ACTION_BYTE_COST_MULTIPLIER); + .put_sequence_action_byte_cost_multiplier(app_state.fees.sequence_byte_cost_multiplier); Ok(()) } diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 63661a4862..d3de2757e1 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -245,6 +245,7 @@ mod test { use super::*; use crate::{ + app::test_utils::default_fees, asset::get_native_asset, mempool::{ Mempool, @@ -470,6 +471,7 @@ mod test { native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: penumbra_ibc::params::IBCParameters::default(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], + fees: default_fees(), } } } diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index a290d3e9d1..5c2898e094 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -497,9 +497,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); - state_tx - .put_transfer_base_fee(crate::accounts::component::DEFAULT_TRANSFER_BASE_FEE) - .unwrap(); + 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(); @@ -565,9 +563,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); - state_tx - .put_transfer_base_fee(crate::accounts::component::DEFAULT_TRANSFER_BASE_FEE) - .unwrap(); + 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(); diff --git a/crates/astria-sequencer/test-genesis-app-state.json b/crates/astria-sequencer/test-genesis-app-state.json index 7cfa9b553c..5a7bc86189 100644 --- a/crates/astria-sequencer/test-genesis-app-state.json +++ b/crates/astria-sequencer/test-genesis-app-state.json @@ -21,6 +21,14 @@ "inbound_ics20_transfers_enabled": true, "outbound_ics20_transfers_enabled": true }, + "fees": { + "transfer_base_fee": 12, + "sequence_base_fee": 32, + "sequence_byte_cost_multiplier": 1, + "init_bridge_account_base_fee": 48, + "bridge_lock_byte_cost_multiplier": 1, + "ics20_withdrawal_base_fee": 24 + }, "native_asset_base_denomination": "nria", "allowed_fee_assets": ["nria"] }