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

feat(StateBuilder): switch builder option from without_bundle to with_bundle #688

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

rakita
Copy link
Member

@rakita rakita commented Sep 5, 2023

In general output state and creating of revert is better to be something to be enabled, and not disabled.

@rakita rakita changed the title feat(StateBuilder): make builder option become feat(StateBuilder): switch builder option from without_bundle to with_bundle Sep 5, 2023
Comment on lines 18 to 25
/// This will initialize cache to this state.
pub with_cache_prestate: Option<CacheState>,
/// Do we want to create reverts and update bundle state.
/// Default is true.
pub without_bundle_update: bool,
/// Default is false.
pub with_bundle_update: bool,
/// Do we want to merge transitions in background.
/// This will allows evm to continue executing.
/// Default is false.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should all of these be private(pub crate) instead?
I don't think they should be mut accessible from the outside

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense

pub fn without_bundle_update(self) -> Self {
/// This is needed option if we want to create reverts
/// and getting output of changed states.
pub fn witho_bundle_update(self) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub fn witho_bundle_update(self) -> Self {
pub fn with_bundle_update(self) -> Self {

@rakita rakita merged commit ee13aac into main Sep 6, 2023
7 checks passed
@rakita rakita deleted the with_bundle branch September 6, 2023 11:56
DaniPopes pushed a commit to DaniPopes/revm that referenced this pull request Sep 6, 2023
…_bundle (bluealloy#688)

* feat(StateBuilder): make builder option  become

* make private field, nit rename

* fix tests
mikelodder7 pushed a commit to LIT-Protocol/revm that referenced this pull request Sep 12, 2023
…_bundle (bluealloy#688)

* feat(StateBuilder): make builder option  become

* make private field, nit rename

* fix tests
Evalir pushed a commit to Evalir/revm that referenced this pull request Sep 14, 2023
…_bundle (bluealloy#688)

* feat(StateBuilder): make builder option  become

* make private field, nit rename

* fix tests
@github-actions github-actions bot mentioned this pull request Jan 12, 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
Development

Successfully merging this pull request may close these issues.

2 participants