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

refactor(sequencer): refactor app #1793

Closed
wants to merge 4 commits into from
Closed

Conversation

ethanoroshiba
Copy link
Contributor

Summary

Large refactoring of the sequencer app.

Background

The sequencer app had a lot of duplicated code due to methods which did similar things slightly differently. In addition to this, naming of these different methods was confusing, with some using the outdated begin_block and end_block ABCI methods, and others just being helper methods. This refactor is meant to make the sequencer app clearer in its design and eliminate duplicated code.

Changes

  • Created AppAbci trait for housing all of the app's methods which correspond to ABCI methods.
  • Combined app::begin_block and app::end_block with their corresponding calling methods.
  • Removed begin_block and end_block from Component, since they were largely unimplemented. Moved all implementations of these to prepare_state_for_execution (formerly pre_execute_transactions) and update_state_and_end_block (formerly post_execute_transasctions).
  • Renamed Component to Genesis to reflect its only remaining method, init_chain.
  • Refactored execute_transactions_prepare_proposal and execute_transactions_process_proposal to utilize shared code, and renamed them prepare_proposal_tx_execution and process_proposal_tx_execution for clarity as to how they fit in with the ABCI methods.

Testing

Fixed all tests which depended upon this code, now all passing.

Changelogs

Ensure all relevant changelog files are updated as necessary. See
keepachangelog for change
categories. Replace this text with e.g. "Changelogs updated." or "No updates
required." to acknowledge changelogs have been considered.

Related Issues

closes #1785

@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Nov 7, 2024
@ethanoroshiba ethanoroshiba added the refactor code refactoring or maintainence label Nov 7, 2024
@ethanoroshiba
Copy link
Contributor Author

After offline discussion, I am going to break this up into many smaller PRs.

@Fraser999 Fraser999 deleted the ENG-995/refactor_app branch November 19, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor code refactoring or maintainence sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sequencer: refactor app
1 participant