From ee1749a409a32abf2c8ca59bfd116555d12fc3c9 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Tue, 22 Oct 2024 14:44:25 -0500 Subject: [PATCH 1/3] move fee event recording to tx level --- crates/astria-sequencer/src/app/mod.rs | 3 --- .../src/app/tests_execute_transaction.rs | 6 +---- crates/astria-sequencer/src/fees/mod.rs | 18 --------------- crates/astria-sequencer/src/fees/state_ext.rs | 23 +++++++++++++++++++ 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index 42ca12d328..18ce6c48d5 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -108,7 +108,6 @@ use crate::{ component::Component as _, fees::{ component::FeesComponent, - construct_tx_fee_event, StateReadExt as _, StateWriteExt as _, }, @@ -1232,8 +1231,6 @@ impl App { .increase_balance(fee_recipient, fee.asset(), fee.amount()) .await .wrap_err("failed to increase fee recipient balance")?; - let fee_event = construct_tx_fee_event(&fee); - state_tx.record(fee_event); } let events = self.apply(state_tx); diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index b223040a72..9121e7d6b5 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -1266,12 +1266,8 @@ async fn transaction_execution_records_fee_event() { .try_build() .unwrap(); let signed_tx = Arc::new(tx.sign(&alice)); - app.execute_transaction(signed_tx).await.unwrap(); - - let sudo_address = app.state.get_sudo_address().await.unwrap(); - let end_block = app.end_block(1, &sudo_address).await.unwrap(); + let events = app.execute_transaction(signed_tx).await.unwrap(); - let events = end_block.events; let event = events.first().unwrap(); assert_eq!(event.kind, "tx.fees"); assert_eq!(event.attributes[0].key, "actionName"); diff --git a/crates/astria-sequencer/src/fees/mod.rs b/crates/astria-sequencer/src/fees/mod.rs index b931208d7a..9baa245c9b 100644 --- a/crates/astria-sequencer/src/fees/mod.rs +++ b/crates/astria-sequencer/src/fees/mod.rs @@ -30,10 +30,6 @@ use astria_eyre::eyre::{ }; use cnidarium::StateWrite; use penumbra_ibc::IbcRelay; -use tendermint::abci::{ - Event, - EventAttributeIndexExt as _, -}; use tracing::{ instrument, Level, @@ -379,17 +375,3 @@ fn base_deposit_fee(asset: &asset::Denom, destination_chain_address: &str) -> u1 .expect("converting a usize to a u128 should work on any currently existing machine") .saturating_add(DEPOSIT_BASE_FEE) } - -/// Creates `abci::Event` of kind `tx.fees` for sequencer fee reporting -pub(crate) fn construct_tx_fee_event(fee: &Fee) -> Event { - Event::new( - "tx.fees", - [ - ("actionName", fee.action_name.to_string()).index(), - ("asset", fee.asset.to_string()).index(), - ("feeAmount", fee.amount.to_string()).index(), - ("sourceTransactionId", fee.source_transaction_id.to_string()).index(), - ("sourceActionIndex", fee.source_action_index.to_string()).index(), - ], - ) -} diff --git a/crates/astria-sequencer/src/fees/state_ext.rs b/crates/astria-sequencer/src/fees/state_ext.rs index 9c9dbe0ef1..cd2de5d6e1 100644 --- a/crates/astria-sequencer/src/fees/state_ext.rs +++ b/crates/astria-sequencer/src/fees/state_ext.rs @@ -36,6 +36,10 @@ use cnidarium::{ StateWrite, }; use futures::StreamExt as _; +use tendermint::abci::{ + Event, + EventAttributeIndexExt as _, +}; use tracing::instrument; use super::{ @@ -367,6 +371,11 @@ pub(crate) trait StateWriteExt: StateWrite { source_transaction_id, source_action_index, }; + + // Fee ABCI event recorded for reporting + let fee_event = construct_tx_fee_event(&fee); + self.record(fee_event); + let new_fees = if let Some(mut fees) = current_fees { fees.push(fee); fees @@ -532,6 +541,20 @@ pub(crate) trait StateWriteExt: StateWrite { impl StateWriteExt for T {} +/// Creates `abci::Event` of kind `tx.fees` for sequencer fee reporting +fn construct_tx_fee_event(fee: &Fee) -> Event { + Event::new( + "tx.fees", + [ + ("actionName", fee.action_name.to_string()).index(), + ("asset", fee.asset.to_string()).index(), + ("feeAmount", fee.amount.to_string()).index(), + ("sourceTransactionId", fee.source_transaction_id.to_string()).index(), + ("sourceActionIndex", fee.source_action_index.to_string()).index(), + ], + ) +} + #[cfg(test)] mod tests { use std::collections::HashSet; From a72a7c99b8773a1474973d88316412b1f4d947c6 Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Wed, 23 Oct 2024 15:45:35 -0500 Subject: [PATCH 2/3] remove transaction id from fee event --- .../src/app/tests_execute_transaction.rs | 3 +-- crates/astria-sequencer/src/fees/mod.rs | 9 ++----- crates/astria-sequencer/src/fees/state_ext.rs | 27 +++---------------- 3 files changed, 7 insertions(+), 32 deletions(-) diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 9121e7d6b5..ef3af94ce3 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -1273,6 +1273,5 @@ async fn transaction_execution_records_fee_event() { assert_eq!(event.attributes[0].key, "actionName"); assert_eq!(event.attributes[1].key, "asset"); assert_eq!(event.attributes[2].key, "feeAmount"); - assert_eq!(event.attributes[3].key, "sourceTransactionId"); - assert_eq!(event.attributes[4].key, "sourceActionIndex"); + assert_eq!(event.attributes[3].key, "sourceActionIndex"); } diff --git a/crates/astria-sequencer/src/fees/mod.rs b/crates/astria-sequencer/src/fees/mod.rs index 9baa245c9b..2e3547e7fb 100644 --- a/crates/astria-sequencer/src/fees/mod.rs +++ b/crates/astria-sequencer/src/fees/mod.rs @@ -1,8 +1,5 @@ use astria_core::{ - primitive::v1::{ - asset, - TransactionId, - }, + primitive::v1::asset, protocol::transaction::{ self, v1::action::{ @@ -70,7 +67,6 @@ pub(crate) struct Fee { action_name: String, asset: asset::Denom, amount: u128, - source_transaction_id: TransactionId, source_action_index: u64, } @@ -343,7 +339,6 @@ async fn check_and_pay_fees( .get_transaction_context() .expect("transaction source must be present in state when executing an action"); let from = transaction_context.address_bytes(); - let transaction_id = transaction_context.transaction_id; let source_action_index = transaction_context.source_action_index; ensure!( @@ -354,7 +349,7 @@ async fn check_and_pay_fees( "invalid fee asset", ); state - .add_fee_to_block_fees::<_, T>(fee_asset, total_fees, transaction_id, source_action_index) + .add_fee_to_block_fees::<_, T>(fee_asset, total_fees, source_action_index) .wrap_err("failed to add to block fees")?; state .decrease_balance(&from, fee_asset, total_fees) diff --git a/crates/astria-sequencer/src/fees/state_ext.rs b/crates/astria-sequencer/src/fees/state_ext.rs index cd2de5d6e1..a632055fe8 100644 --- a/crates/astria-sequencer/src/fees/state_ext.rs +++ b/crates/astria-sequencer/src/fees/state_ext.rs @@ -1,10 +1,7 @@ use std::borrow::Cow; use astria_core::{ - primitive::v1::{ - asset, - TransactionId, - }, + primitive::v1::asset, protocol::fees::v1::{ BridgeLockFeeComponents, BridgeSudoChangeFeeComponents, @@ -355,7 +352,6 @@ pub(crate) trait StateWriteExt: StateWrite { &mut self, asset: &'a TAsset, amount: u128, - source_transaction_id: TransactionId, source_action_index: u64, ) -> Result<()> where @@ -368,7 +364,6 @@ pub(crate) trait StateWriteExt: StateWrite { action_name: T::full_name(), asset: asset::IbcPrefixed::from(asset).into(), amount, - source_transaction_id, source_action_index, }; @@ -549,7 +544,6 @@ fn construct_tx_fee_event(fee: &Fee) -> Event { ("actionName", fee.action_name.to_string()).index(), ("asset", fee.asset.to_string()).index(), ("feeAmount", fee.amount.to_string()).index(), - ("sourceTransactionId", fee.source_transaction_id.to_string()).index(), ("sourceActionIndex", fee.source_action_index.to_string()).index(), ], ) @@ -591,7 +585,7 @@ mod tests { let asset = asset_0(); let amount = 100u128; state - .add_fee_to_block_fees::<_, Transfer>(&asset, amount, TransactionId::new([0; 32]), 0) + .add_fee_to_block_fees::<_, Transfer>(&asset, amount, 0) .unwrap(); // holds expected @@ -602,7 +596,6 @@ mod tests { action_name: "astria.protocol.transaction.v1.Transfer".to_string(), asset: asset.to_ibc_prefixed().into(), amount, - source_transaction_id: TransactionId::new([0; 32]), source_action_index: 0 }, "fee balances are not what they were expected to be" @@ -622,20 +615,10 @@ mod tests { let amount_second = 200u128; state - .add_fee_to_block_fees::<_, Transfer>( - &asset_first, - amount_first, - TransactionId::new([0; 32]), - 0, - ) + .add_fee_to_block_fees::<_, Transfer>(&asset_first, amount_first, 0) .unwrap(); state - .add_fee_to_block_fees::<_, Transfer>( - &asset_second, - amount_second, - TransactionId::new([0; 32]), - 1, - ) + .add_fee_to_block_fees::<_, Transfer>(&asset_second, amount_second, 1) .unwrap(); // holds expected let fee_balances = HashSet::<_>::from_iter(state.get_block_fees()); @@ -646,14 +629,12 @@ mod tests { action_name: "astria.protocol.transaction.v1.Transfer".to_string(), asset: asset_first.to_ibc_prefixed().into(), amount: amount_first, - source_transaction_id: TransactionId::new([0; 32]), source_action_index: 0 }, Fee { action_name: "astria.protocol.transaction.v1.Transfer".to_string(), asset: asset_second.to_ibc_prefixed().into(), amount: amount_second, - source_transaction_id: TransactionId::new([0; 32]), source_action_index: 1 }, ]), From 6d6b982054432b1101f9a99ddbd36957ff5b89ec Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Thu, 24 Oct 2024 10:57:20 -0500 Subject: [PATCH 3/3] rename "sourceActionIndex" to "positionInTransaction" --- crates/astria-sequencer/src/app/tests_execute_transaction.rs | 2 +- crates/astria-sequencer/src/fees/state_ext.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index ef3af94ce3..8b4f7409c1 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -1273,5 +1273,5 @@ async fn transaction_execution_records_fee_event() { assert_eq!(event.attributes[0].key, "actionName"); assert_eq!(event.attributes[1].key, "asset"); assert_eq!(event.attributes[2].key, "feeAmount"); - assert_eq!(event.attributes[3].key, "sourceActionIndex"); + assert_eq!(event.attributes[3].key, "positionInTransaction"); } diff --git a/crates/astria-sequencer/src/fees/state_ext.rs b/crates/astria-sequencer/src/fees/state_ext.rs index b6d0698b3e..dd66e87bb6 100644 --- a/crates/astria-sequencer/src/fees/state_ext.rs +++ b/crates/astria-sequencer/src/fees/state_ext.rs @@ -577,7 +577,7 @@ fn construct_tx_fee_event(fee: &Fee) -> Event { ("actionName", fee.action_name.to_string()).index(), ("asset", fee.asset.to_string()).index(), ("feeAmount", fee.amount.to_string()).index(), - ("sourceActionIndex", fee.source_action_index.to_string()).index(), + ("positionInTransaction", fee.source_action_index.to_string()).index(), ], ) }