Skip to content

Commit

Permalink
feat(sequencer, core): Add fee reporting (#1305)
Browse files Browse the repository at this point in the history
## Summary
Added event recording for fees on `Transfer` and `Sequence` actions.

## Background
Fees were previously not reported, and since Astria doesn't fill out gas
wanted/used on `ExecTxResult`, there was no outside way of viewing a
block or transaction's associated fees.

## Changes
- Added `get_action_type()` methods to `TransferAction` and
`SequencerAction` to facilitate passing the kind of action to
`get_and_increase_block_fees()`.
- Added construction and recording of ABCI events having kind `tx.fees`
and attributes `asset`, `feeAmount`, and `actionType`.

## Testing
Added unit test to ensure `tx.fees` event is recorded correctly in the
transaction state events.

## Related Issues
closes #1293

---------

Co-authored-by: Fraser Hutchison <[email protected]>
  • Loading branch information
ethanoroshiba and Fraser999 authored Jul 30, 2024
1 parent aed95e0 commit 7953133
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 56 deletions.
1 change: 1 addition & 0 deletions crates/astria-composer/src/executor/bundle_factory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use astria_core::{
action::SequenceAction,
Action,
},
Protobuf as _,
};
use serde::ser::{
Serialize,
Expand Down
1 change: 1 addition & 0 deletions crates/astria-composer/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use astria_core::{
ROLLUP_ID_LEN,
},
protocol::transaction::v1alpha1::action::SequenceAction,
Protobuf as _,
};

fn encoded_len(action: &SequenceAction) -> usize {
Expand Down
9 changes: 8 additions & 1 deletion crates/astria-core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use prost::Name;

#[cfg(not(target_pointer_width = "64"))]
compile_error!(
"library is only guaranteed to run on 64 bit machines due to casts from/to u64 and usize"
Expand Down Expand Up @@ -27,7 +29,7 @@ pub trait Protobuf: Sized {
/// Errors that can occur when transforming from a raw type.
type Error;
/// The raw deserialized protobuf type.
type Raw;
type Raw: prost::Name;

/// Convert from a reference to the raw protobuf type.
///
Expand Down Expand Up @@ -56,4 +58,9 @@ pub trait Protobuf: Sized {
fn into_raw(self) -> Self::Raw {
Self::to_raw(&self)
}

#[must_use]
fn full_name() -> String {
Self::Raw::full_name()
}
}
64 changes: 20 additions & 44 deletions crates/astria-core/src/protocol/transaction/v1alpha1/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,23 +391,12 @@ pub struct SequenceAction {
pub fee_asset: asset::Denom,
}

impl SequenceAction {
#[must_use]
pub fn into_raw(self) -> raw::SequenceAction {
let Self {
rollup_id,
data,
fee_asset,
} = self;
raw::SequenceAction {
rollup_id: Some(rollup_id.to_raw()),
data,
fee_asset: fee_asset.to_string(),
}
}
impl Protobuf for SequenceAction {
type Error = SequenceActionError;
type Raw = raw::SequenceAction;

#[must_use]
pub fn to_raw(&self) -> raw::SequenceAction {
fn to_raw(&self) -> raw::SequenceAction {
let Self {
rollup_id,
data,
Expand All @@ -420,22 +409,23 @@ impl SequenceAction {
}
}

/// Convert from a raw, unchecked protobuf [`raw::SequenceAction`].
/// Convert from a reference to the raw protobuf type.
///
/// # Errors
/// Returns an error if the `proto.rollup_id` field was not 32 bytes.
pub fn try_from_raw(proto: raw::SequenceAction) -> Result<Self, SequenceActionError> {
/// Returns `SequenceActionError` if the `proto.rollup_id` field was not 32 bytes.
fn try_from_raw_ref(raw: &Self::Raw) -> Result<Self, Self::Error> {
let raw::SequenceAction {
rollup_id,
data,
fee_asset,
} = proto;
} = raw;
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)?;
RollupId::try_from_raw(rollup_id).map_err(SequenceActionError::rollup_id_length)?;
let fee_asset = fee_asset.parse().map_err(SequenceActionError::fee_asset)?;
let data = data.clone();
Ok(Self {
rollup_id,
data,
Expand All @@ -449,31 +439,18 @@ impl SequenceAction {
pub struct TransferAction {
pub to: Address,
pub amount: u128,
// asset to be transferred.
/// asset to be transferred.
pub asset: asset::Denom,
/// asset to use for fee payment.
pub fee_asset: asset::Denom,
}

impl TransferAction {
#[must_use]
pub fn into_raw(self) -> raw::TransferAction {
let Self {
to,
amount,
asset,
fee_asset,
} = self;
raw::TransferAction {
to: Some(to.to_raw()),
amount: Some(amount.into()),
asset: asset.to_string(),
fee_asset: fee_asset.to_string(),
}
}
impl Protobuf for TransferAction {
type Error = TransferActionError;
type Raw = raw::TransferAction;

#[must_use]
pub fn to_raw(&self) -> raw::TransferAction {
fn to_raw(&self) -> raw::TransferAction {
let Self {
to,
amount,
Expand All @@ -488,23 +465,22 @@ impl TransferAction {
}
}

/// Convert from a raw, unchecked protobuf [`raw::TransferAction`].
/// Convert from a reference to the raw protobuf type.
///
/// # Errors
///
/// Returns an error if the raw action's `to` address did not have the expected
/// Returns `TransferActionError` if the raw action's `to` address did not have the expected
/// length.
pub fn try_from_raw(proto: raw::TransferAction) -> Result<Self, TransferActionError> {
fn try_from_raw_ref(raw: &Self::Raw) -> Result<Self, Self::Error> {
let raw::TransferAction {
to,
amount,
asset,
fee_asset,
} = proto;
} = raw;
let Some(to) = to else {
return Err(TransferActionError::field_not_set("to"));
};
let to = Address::try_from_raw(&to).map_err(TransferActionError::address)?;
let to = Address::try_from_raw(to).map_err(TransferActionError::address)?;
let amount = amount.map_or(0, Into::into);
let asset = asset.parse().map_err(TransferActionError::asset)?;
let fee_asset = fee_asset.parse().map_err(TransferActionError::fee_asset)?;
Expand Down
3 changes: 2 additions & 1 deletion crates/astria-sequencer/src/accounts/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use anyhow::{
use astria_core::{
primitive::v1::Address,
protocol::transaction::v1alpha1::action::TransferAction,
Protobuf,
};
use tracing::instrument;

Expand Down Expand Up @@ -112,7 +113,7 @@ impl ActionHandler for TransferAction {
.await
.context("failed to get transfer base fee")?;
state
.get_and_increase_block_fees(&self.fee_asset, fee)
.get_and_increase_block_fees(&self.fee_asset, fee, Self::full_name())
.await
.context("failed to add to block fees")?;

Expand Down
50 changes: 50 additions & 0 deletions crates/astria-sequencer/src/app/tests_execute_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use astria_core::{
};
use cnidarium::StateDelta;
use penumbra_ibc::params::IBCParameters;
use tendermint::abci::EventAttributeIndexExt as _;

use crate::{
accounts::state_ext::StateReadExt as _,
Expand Down Expand Up @@ -1042,3 +1043,52 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() {
"bridge should've transferred out whole balance"
);
}

#[tokio::test]
async fn transaction_execution_records_fee_event() {
let mut app = initialize_app(None, vec![]).await;

// transfer funds from Alice to Bob
let (alice_signing_key, _) = get_alice_signing_key_and_address();
let bob_address = address_from_hex_string(BOB_ADDRESS);
let value = 333_333;
let tx = UnsignedTransaction {
params: TransactionParams::builder()
.nonce(0)
.chain_id("test")
.build(),
actions: vec![
TransferAction {
to: bob_address,
amount: value,
asset: get_native_asset().clone(),
fee_asset: get_native_asset().clone(),
}
.into(),
],
};

let signed_tx = Arc::new(tx.into_signed(&alice_signing_key));

let events = app.execute_transaction(signed_tx).await.unwrap();
let transfer_fee = app.state.get_transfer_base_fee().await.unwrap();
let event = events.first().unwrap();
assert_eq!(event.kind, "tx.fees");
assert_eq!(
event.attributes[0],
("asset", get_native_asset().to_string()).index().into()
);
assert_eq!(
event.attributes[1],
("feeAmount", transfer_fee.to_string()).index().into()
);
assert_eq!(
event.attributes[2],
(
"actionType",
"astria.protocol.transactions.v1alpha1.TransferAction"
)
.index()
.into()
);
}
3 changes: 2 additions & 1 deletion crates/astria-sequencer/src/sequence/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use anyhow::{
use astria_core::{
primitive::v1::Address,
protocol::transaction::v1alpha1::action::SequenceAction,
Protobuf,
};
use tracing::instrument;

Expand Down Expand Up @@ -61,7 +62,7 @@ impl ActionHandler for SequenceAction {
.await
.context("failed to calculate fee")?;
state
.get_and_increase_block_fees(&self.fee_asset, fee)
.get_and_increase_block_fees(&self.fee_asset, fee, Self::full_name())
.await
.context("failed to add to block fees")?;

Expand Down
44 changes: 35 additions & 9 deletions crates/astria-sequencer/src/state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ use cnidarium::{
StateWrite,
};
use futures::StreamExt as _;
use tendermint::Time;
use tendermint::{
abci::{
Event,
EventAttributeIndexExt as _,
},
Time,
};
use tracing::instrument;

const NATIVE_ASSET_KEY: &[u8] = b"nativeasset";
Expand All @@ -36,6 +42,21 @@ fn fee_asset_key<TAsset: Into<asset::IbcPrefixed>>(asset: TAsset) -> String {
)
}

/// Creates `abci::Event` of kind `tx.fees` for sequencer fee reporting
fn construct_tx_fee_event<TAsset>(asset: &TAsset, fee_amount: u128, action_type: String) -> Event
where
TAsset: Into<asset::IbcPrefixed> + std::fmt::Display + Send,
{
Event::new(
"tx.fees",
[
("asset", asset.to_string()).index(),
("feeAmount", fee_amount.to_string()).index(),
("actionType", action_type).index(),
],
)
}

#[async_trait]
pub(crate) trait StateReadExt: StateRead {
#[instrument(skip_all)]
Expand Down Expand Up @@ -243,12 +264,13 @@ pub(crate) trait StateWriteExt: StateWrite {
async fn get_and_increase_block_fees<TAsset>(
&mut self,
asset: TAsset,
amount: u128,
fee_amount: u128,
action_type: String,
) -> Result<()>
where
TAsset: Into<asset::IbcPrefixed> + std::fmt::Display + Send,
TAsset: Into<asset::IbcPrefixed> + std::fmt::Display + Send + Clone,
{
let block_fees_key = block_fees_key(asset);
let block_fees_key = block_fees_key(asset.clone());
let current_amount = self
.nonverifiable_get_raw(block_fees_key.as_bytes())
.await
Expand All @@ -264,10 +286,14 @@ pub(crate) trait StateWriteExt: StateWrite {
.unwrap_or_default();

let new_amount = current_amount
.checked_add(amount)
.checked_add(fee_amount)
.context("block fees overflowed u128")?;

self.nonverifiable_put_raw(block_fees_key.into(), new_amount.to_be_bytes().to_vec());

// record the fee event to the state cache
let tx_fee_event = construct_tx_fee_event(&asset, fee_amount, action_type);
self.record(tx_fee_event);

Ok(())
}

Expand Down Expand Up @@ -624,7 +650,7 @@ mod test {
let asset = asset_0();
let amount = 100u128;
state
.get_and_increase_block_fees(&asset, amount)
.get_and_increase_block_fees(&asset, amount, "test".into())
.await
.unwrap();

Expand All @@ -650,11 +676,11 @@ mod test {
let amount_second = 200u128;

state
.get_and_increase_block_fees(&asset_first, amount_first)
.get_and_increase_block_fees(&asset_first, amount_first, "test".into())
.await
.unwrap();
state
.get_and_increase_block_fees(&asset_second, amount_second)
.get_and_increase_block_fees(&asset_second, amount_second, "test".into())
.await
.unwrap();
// holds expected
Expand Down

0 comments on commit 7953133

Please sign in to comment.