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): move asset state methods to asset module #1313

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

SuperFluffy
Copy link
Member

@SuperFluffy SuperFluffy commented Jul 30, 2024

Summary

Moved methods for manipulating state assets to the state asset module.

Background

Sequencer uses the convention of having state setters and writers as methods in a trait <component>::StateReadExt and <component>::StateWriteExt. For example, a sudo address (which lives in the authority module) can be read with authority::StateReadExt::get_sudo_address. The exception to this are the methods for manipulating assets, which were under astria_sequencer::state_ext::StateExt. This is because the asset module was just recently introduced.

Changes

No business logic was changed in this patch!

  • Rename crate::asset -> crate::assets
  • Move all methods for reading and writing assets from astria_sequencer::state_ext::State{Read,Write}Ext to astria_sequencer::assets::State{Read,Write}Ext.
  • Reexport State{Read,Write}Ext from all parent modules so that one can write use <component>::StateReadExt (and similar for write) instead of use <component>::state_ext::StateReadExt.

Testing

No tests were changed (minus import paths). Asset-related tests were moved (including snapshot tests)

Related Issues

Closes #1312

@SuperFluffy SuperFluffy requested a review from Fraser999 July 30, 2024 12:44
@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Jul 30, 2024
@SuperFluffy SuperFluffy marked this pull request as ready for review July 30, 2024 12:52
@SuperFluffy SuperFluffy requested review from a team, joroshiba and noot as code owners July 30, 2024 12:52
Copy link
Contributor

@Fraser999 Fraser999 left a comment

Choose a reason for hiding this comment

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

LGTM, but a couple of non-blocking nitpicks.

@SuperFluffy SuperFluffy enabled auto-merge August 1, 2024 15:57
@SuperFluffy SuperFluffy added this pull request to the merge queue Aug 1, 2024
Copy link
Member

@joroshiba joroshiba left a comment

Choose a reason for hiding this comment

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

approving the snapshot relocate updates

Merged via the queue into main with commit 8171eed Aug 1, 2024
42 checks passed
@SuperFluffy SuperFluffy deleted the ENG-659 branch August 1, 2024 16:41
steezeburger added a commit that referenced this pull request Aug 2, 2024
* main:
  chore(core): Implement Protobuf trait for tx actions (#1320)
  refactor(sequencer): remove global state (#1317)
  refactor(sequencer): move asset state methods to asset module (#1313)
  feat(sequencer, core): Add fee reporting (#1305)
  chore(bridge-withdrawer): cleanup nonce handling (#1292)
  fix(charts, bridge): fix ci test (#1310)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move methods for manipulating asset state to asset module
3 participants