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

chore(sequencer): add fee docs #1666

Closed
wants to merge 5 commits into from
Closed

chore(sequencer): add fee docs #1666

wants to merge 5 commits into from

Conversation

ethanoroshiba
Copy link
Contributor

Summary

Added more comments/docs to the fees module.

Background

Fees module was lacking in comments because it was done so quickly. This is meant to make the process of implementing new fees and the FeeHandler trait clearer to anyone who is adding a new action.

Changes

  • Added comments/docs.

Testing

None needed - no code in doc comments.

Related Issues

closes #1663

@ethanoroshiba ethanoroshiba added the documentation Improvements or additions to documentation label Oct 16, 2024
@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Oct 16, 2024
@ethanoroshiba ethanoroshiba marked this pull request as ready for review October 16, 2024 16:45
@ethanoroshiba ethanoroshiba requested a review from a team as a code owner October 16, 2024 16:45
Comment on lines +64 to +70
// This module contains the fee handling logic for all actions. All actions must implement the
// [`FeeHandler`] trait. In addition to this, new actions should provide new, unique read/write
// methods in [`fees::StateReadExt`] and [`fees::StateWriteExt`] for the fees associated with the
// action, which take/return a domain type [`<action_name>FeeComponents`]. If there are no fees
// stored to the state for the given action, it is assumed to be disabled, and the transaction
// execution should fail. If the action is free, the fees should be explicitly set to zero in the
// state. See action implementations for examples of how to implement this trait for new actions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is intended to be a doc comment, it should be at the top of this file, and start with //!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this should be a doc comment? I had intentionally left it more as a means of guiding people who are adding actions/fees in the future, since it deals more with the implementation details and figured that probably wouldn't be best for public-facing docs. Happy to make it a doc comment if you think it warrants it, though

@ethanoroshiba
Copy link
Contributor Author

Closing as these comments will no longer be accurate as of #1811.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sequencer: add more docs/comments to the fees component
2 participants