Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework fee reporting #1369

Closed
ethanoroshiba opened this issue Aug 15, 2024 · 0 comments · Fixed by #1647
Closed

Rework fee reporting #1369

ethanoroshiba opened this issue Aug 15, 2024 · 0 comments · Fixed by #1647
Assignees
Labels
code-quality sequencer pertaining to the astria-sequencer crate

Comments

@ethanoroshiba
Copy link
Contributor

ethanoroshiba commented Aug 15, 2024

Issue

#1305 added fee reporting by way of get_and_increase_block_fees(), which was fixed in #1343 by adding a call to this function in all actions which require fees. This setup requires passing ABCI events all the way through to finalize_block(), where the events are retroactively placed into ExecTxResults. We should rework this such that the result Txs are constructed at a lower level and passed back through the execution calls to finalize_block().

Note: this should only be done after #1332 is merged

Relevant Code

Fee reporting in get_and_increase_block_fees

let tx_fee_event = construct_tx_fee_event(&asset, amount, action_type);

self.record(tx_fee_event);

Construction of fee event
fn construct_tx_fee_event<T: std::fmt::Display>(
asset: &T,
fee_amount: u128,
action_type: String,
) -> Event {
Event::new(
"tx.fees",
[
("asset", asset.to_string()).index(),
("feeAmount", fee_amount.to_string()).index(),
("actionType", action_type).index(),
],
)
}

Construction of ExecTxResult from ABCI event
match self.execute_transaction(Arc::new(signed_tx)).await {
Ok(events) => tx_results.push(ExecTxResult {
events,
..Default::default()

┆Issue Number: ENG-718

@ethanoroshiba ethanoroshiba added sequencer pertaining to the astria-sequencer crate code-quality labels Aug 15, 2024
@ethanoroshiba ethanoroshiba self-assigned this Aug 15, 2024
@ethanoroshiba ethanoroshiba linked a pull request Aug 29, 2024 that will close this issue
github-merge-queue bot pushed a commit that referenced this issue Oct 15, 2024
## Summary
Gutted all current fee handling and replaced all with single fee
calculation method in new trait.

## Background
This implementation will simplify not only our fee calculation, but the
process for adding new actions in the future.

## Changes
- Created `FeeComponents` types for all fee-bearing transactions which
all have a base fee component and a computed cost multiplier component.
- Moved all fee checks and payment to one single function within the new
sequencer fees component. Fee calculation is now always the following
formula: `base_fee + computed_cost_base * computed_cost_multiplier`
- Moved all state reads/writes for fees to the new fees component.
- Initialized all fees in the fee component's `init_chain()`.
- Changed `FeeChange` to be an enum which takes any fee-bearing action's
fee components.
- Moved `FeeChange` and `FeeAssetChange`'s `ActionHandler` impls to the
fees component.
- Allowed fee assets now stored in verifiable storage instead of
non-verifiable.

## Testing
All previous tests passing, added new tests for all state fee
reads/writes.

## Breaking Changelist
- Changed shape of `FeeChange` action.
- Added storage of all the action fees, breaking the app hash.
- Changed app genesis.
- Removed a bunch of storage keys, breaking these snapshot tests
- Allowed fee assets moved from non-verifiable to verifiable storage. 

## Related Issues
closes #1369
closes #1145

---------

Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io>
Co-authored-by: Fraser Hutchison <fraser@astria.org>
Co-authored-by: Jordan Oroshiba <jordan@astria.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality sequencer pertaining to the astria-sequencer crate
Projects
None yet
1 participant