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: explicit behavior for nonce update persistence #729

Conversation

nbaztec
Copy link
Collaborator

@nbaztec nbaztec commented Nov 16, 2024

What 💻

  • Defines nonce update behavior for tx caller, explicitly.

@nbaztec nbaztec requested a review from a team as a code owner November 16, 2024 07:17
Copy link
Contributor

@elfedy elfedy left a comment

Choose a reason for hiding this comment

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

LGTM, though @Jrigada should probably review as well

Copy link
Contributor

@Karrq Karrq left a comment

Choose a reason for hiding this comment

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

Agreed on waiting for @Jrigada

}

/// Retrieve if a nonce update must be persisted, or not. Resets the state to default.
pub fn get(&mut self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd actually rename this a bit to be slightly more explicit. I mean the &mut still indicates that we will mutate it but I believe a more descriptive name would be better...
Maybe read_and_reset or get_and_clear

crates/cheatcodes/src/inspector.rs Show resolved Hide resolved
@nbaztec
Copy link
Collaborator Author

nbaztec commented Nov 19, 2024

@Jrigada can you review this please?

Copy link
Contributor

@Jrigada Jrigada left a comment

Choose a reason for hiding this comment

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

Thank you Nish

@nbaztec nbaztec merged commit 57db920 into jrigada-fix-nonce-mismatch-scripts Nov 19, 2024
4 checks passed
@nbaztec nbaztec deleted the nish-jrigada-fix-nonce-mismatch-scripts branch November 19, 2024 16:13
Jrigada added a commit that referenced this pull request Nov 21, 2024
* Nonce mismatch on setup with script

* Add toggle to update nonce only on first operation to avoid nonce mismatch with the network

* Remove prints and format test

* Update crates/cheatcodes/src/inspector.rs

Co-authored-by: Federico Rodríguez <[email protected]>

* Update crates/cheatcodes/src/inspector.rs

Co-authored-by: Federico Rodríguez <[email protected]>

* Update crates/forge/tests/fixtures/zk/ScriptSetup.s.sol

Co-authored-by: Federico Rodríguez <[email protected]>

* Update crates/cheatcodes/src/inspector.rs

Co-authored-by: Federico Rodríguez <[email protected]>

* change variable and test name

* Change test name of noncemismatch

* Add description, filter before iteration for loop and take() in cheatcode context creation

* Remove taking in inspector of zk should update nonce

* chore: explicit behavior for nonce update persistence (#729)

* explicit behavior for nonce update persistence

* prevent short-circuit bug

* enable persistance only once per test contract deployment

* Format and change test

* Forge fmt

* improve comments

* Add comment to test

---------

Co-authored-by: Federico Rodríguez <[email protected]>
Co-authored-by: Nisheeth Barthwal <[email protected]>
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.

4 participants