Skip to content

Conversation

Mouzayan
Copy link
Contributor

@Mouzayan Mouzayan commented Apr 23, 2025

Summary

This draft PR closes #10027 by reviewing and updating the mutability modifiers (view / pure) on cheatcodes that were previously declared external without an explicit mutability tag.

Motivation

This was a chore tracked in the repo.

Solution

Followed the plan below:
• Identified cheatcodes currently marked external with no view or pure modifier.
• Evaluated each case to determine:
• If the cheatcode mutates observable state (EVM, filesystem, logs, interpreter) → left as-is.
• If the cheatcode reads state but does not mutate → marked as view.
• If the cheatcode has no dependencies or side effects → marked as pure.
• Updated ABI declarations and signatures accordingly.

Note

• Ran cargo check, cargo clippy, and cargo +nightly fmt --check, all passed.
• Ran cargo test --all --all-features multiple times. Observed inconsistent failures across unrelated tests.
• To verify test isolation, ran: cargo test --all --all-features -- --test-threads=1.
With this, previously failing tests (like decode_traces_with_project_artifacts) passed.

PR Checklist

  • No new tests required
  • No documentation changes needed
  • No breaking changes introduced

@Mouzayan Mouzayan force-pushed the chore-10027-cheatcode-mutability-tags branch from 5f5ec27 to 5d97d85 Compare May 6, 2025 02:06
@Mouzayan Mouzayan marked this pull request as ready for review May 6, 2025 02:21
@Mouzayan Mouzayan force-pushed the chore-10027-cheatcode-mutability-tags branch from 26238e8 to 8ec5aaa Compare May 7, 2025 03:30
@onbjerg onbjerg changed the title chore(10027-cheatcode-mutability-tags): Fix Mutability Tags for Cheatcodes that are Missing Them chore: Fix Mutability Tags for Cheatcodes that are Missing Them Aug 25, 2025
onbjerg
onbjerg previously approved these changes Aug 25, 2025
Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@onbjerg onbjerg self-assigned this Aug 25, 2025
@onbjerg onbjerg added A-cheatcodes Area: cheatcodes T-debt Type: code debt labels Aug 25, 2025
DaniPopes
DaniPopes previously approved these changes Aug 25, 2025
@onbjerg onbjerg enabled auto-merge (squash) August 25, 2025 20:18
@@ -594,7 +595,7 @@ interface Vm {

/// Utility cheatcode to remove any EIP-2930 access list set by `accessList` cheatcode.
#[cheatcode(group = Evm, safety = Unsafe)]
function noAccessList() external;
function noAccessList() external view;
Copy link
Member

Choose a reason for hiding this comment

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

This is stateful, imo this should not be view

Copy link
Member

Choose a reason for hiding this comment

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

resolved in ba90336

@zerosnacks zerosnacks disabled auto-merge August 25, 2025 20:27
@zerosnacks zerosnacks dismissed stale reviews from DaniPopes and onbjerg via ba90336 August 26, 2025 07:48
@zerosnacks zerosnacks enabled auto-merge (squash) August 26, 2025 07:50
@zerosnacks zerosnacks self-assigned this Aug 26, 2025
@zerosnacks zerosnacks moved this to Ready For Review in Foundry Aug 26, 2025
DaniPopes
DaniPopes previously approved these changes Aug 26, 2025
@zerosnacks zerosnacks merged commit f267f28 into foundry-rs:master Aug 26, 2025
39 of 44 checks passed
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Aug 26, 2025
@zerosnacks
Copy link
Member

Thanks @Mouzayan for your PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes T-debt Type: code debt
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

chore: review external / pure / view rule for cheatcodes that only operate on cheatcode state
6 participants