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

Pre-forking state refactor #329

Merged
merged 72 commits into from
Mar 6, 2024
Merged

Pre-forking state refactor #329

merged 72 commits into from
Mar 6, 2024

Conversation

FabijanC
Copy link
Contributor

@FabijanC FabijanC commented Jan 31, 2024

Usage related changes

Development related changes

  • Ignore tests that rely on full state archive (9 tests)
  • Unify declaring and storing of class in StarknetState
  • Unify state diff calculation and state clearing
    • Dirty vs clean state terminology replaced by relying on CachedState commitment diff
    • No more explicit applying of state diff
    • Class diff done via new utility struct CommittedClassStorage (property of StarknetState)
  • Introduce DictState
    • Copy of blockifier's DictStateReader testing utility with added State (writing) functionality
    • Used for predeclaration and predeployment
      • Prevents invalid state diff (since state diff is now calculated using CachedState diff
    • Currently CachedState always takes DictState as its underlying state, but in a future PR this will be made generic to support a ForkedState that will be the same as DictState, but will also an origin
  • Setting initial balance is now a part of account predeployment
  • Remove StateChanger: where applicable, replaced with blockifier::State
  • Remove StateExtractor: where applicable, replaced with blockifier::StateReader
  • Introduce CustomState and CustomStateReader
    • Seems redundant as no function parameters use impl CustomStateReader, the methods of these traits could be implemented directly under StarknetState
  • Since we are more relying on CachedState now and many of its methods require &mut state
    • This mutability requirement propagates to other parts of our code
    • Lock acquiring changed in some places from state.read() to state.write() accordingly
  • Proposals regarding things not changed by this PR (perhaps these need separate GitHub issues):
    • Rename trait Deployed to Predeployed and its methods accordingly (predeploy, predeclare)
    • Block context needs to be stored together with state for correct old state calls
    • Do something about StateDiff:
      • In production it is only used to be converted to ThinStateDiff
      • Other use is only in testing

Checklist:

  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/clippy_check.sh
  • No unused dependencies - ./scripts/check_unused_deps.sh
  • Performed code self-review
  • Rebased to the last commit of the target branch (or merged it into my branch)
  • Documented the changes
  • Linked the issues which this PR resolves
  • Checked the TODO section in README.md if this PR resolves it
  • Updated the tests
  • All tests are passing - cargo test

FabijanC and others added 30 commits January 18, 2024 15:08
@FabijanC
Copy link
Contributor Author

FabijanC commented Feb 26, 2024

While working on #360, I spotted a bug in state diff generation. Converting to draft until fixed.
EDIT: merging #353 will have the bug fixed
EDIT: merged

@FabijanC FabijanC marked this pull request as draft February 26, 2024 13:13
@FabijanC FabijanC mentioned this pull request Feb 26, 2024
10 tasks
@FabijanC FabijanC marked this pull request as ready for review March 4, 2024 08:31
* [skip ci] - because #353 not yet merged
@FabijanC FabijanC mentioned this pull request Mar 5, 2024
10 tasks
@FabijanC FabijanC merged commit cec6ce2 into main Mar 6, 2024
1 check passed
@FabijanC FabijanC deleted the state-refactor branch March 6, 2024 16:40
@FabijanC FabijanC linked an issue Apr 24, 2024 that may be closed by this pull request
@FabijanC FabijanC mentioned this pull request Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants