Skip to content

feat(forge): add vm.lastCallGas cheatcode#7573

Merged
zerosnacks merged 35 commits into
masterfrom
zerosnacks/add-vm-lastGasUsed
Apr 8, 2024
Merged

feat(forge): add vm.lastCallGas cheatcode#7573
zerosnacks merged 35 commits into
masterfrom
zerosnacks/add-vm-lastGasUsed

Conversation

@zerosnacks

@zerosnacks zerosnacks commented Apr 5, 2024

Copy link
Copy Markdown
Contributor

Motivation

Closes #7272

Solution

  • Add the lastCallGas cheatcode, primarily to be used in isolation mode for accurate measurements.
  • Adds exclude_contracts to test filter
  • Returns a Gas struct instead of proposed uint256, with details like gas spend on memory and gas refunds

Comment thread crates/cheatcodes/src/evm.rs Outdated
Comment thread crates/cheatcodes/src/evm.rs Outdated
Comment thread crates/forge/tests/it/test_helpers.rs Outdated

@mattsse mattsse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this looks clean, only have one comment re name,

wdyt @klkvr ?

Comment thread crates/cheatcodes/spec/src/vm.rs Outdated
@mds1

mds1 commented Apr 5, 2024

Copy link
Copy Markdown
Collaborator

Before merge let's make sure this feature is actually the best solution to the problem, ref #7272 (comment)

Comment thread crates/cheatcodes/src/inspector.rs Outdated
Comment thread crates/forge/tests/it/isolate.rs Outdated
Comment thread testdata/default/isolation/RecordGas.t.sol Outdated
Comment thread crates/cheatcodes/src/evm.rs Outdated
@klkvr

klkvr commented Apr 5, 2024

Copy link
Copy Markdown
Member

Can't we make this work for non-isolated tests as well?

@zerosnacks zerosnacks changed the title feat(forge): add vm.lastGasUsed cheatcode feat(forge): add vm.lastCallGas cheatcode Apr 8, 2024

@Evalir Evalir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

excited to see this, i think it's going to be pretty useful. nits:

Comment thread crates/cheatcodes/src/evm.rs Outdated
Comment thread crates/forge/tests/it/test_helpers.rs Outdated
Comment thread crates/cheatcodes/src/evm.rs Outdated
@zerosnacks zerosnacks marked this pull request as ready for review April 8, 2024 11:27
@zerosnacks zerosnacks requested a review from DaniPopes as a code owner April 8, 2024 11:27
@zerosnacks zerosnacks requested review from Evalir, klkvr and mattsse April 8, 2024 11:28

@klkvr klkvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice work! lgtm overall, would like to improve tests for this a bit

Comment thread testdata/default/cheats/LastCallGas.t.sol Outdated
…ment subset of cheats that require to be tested in isolation mode as well
…est filter, not just individual tests or paths

@klkvr klkvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@Evalir Evalir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tests look great, lgtm

@zerosnacks zerosnacks merged commit b88d167 into master Apr 8, 2024
@zerosnacks zerosnacks deleted the zerosnacks/add-vm-lastGasUsed branch April 8, 2024 15:51
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.

Create a lastGasUsed() function in Vm.sol

5 participants