feat(forge): add vm.stopRecord by tushar994 · Pull Request #10370 · foundry-rs/foundry (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits11 Checks22 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Motivation

solves #10240

PR Checklist

• Added Tests
• Added Documentation
• Breaking changes

tushar994 changed the titleadd vm.stopAndReturnAccesses() cheatcode add vm.stopRecordAndReturnAccesses() cheatcode

Apr 25, 2025

Choose a reason for hiding this comment

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

Thanks, this looks good! However, I realized that this might not cover all use cases: calling stopRecordAndReturnAccesses resets the accesses and it returns them for one address; if I want to stop and return for multiple addresses, this is not possible as accesses calls after will always return empty arrays;

I think we need a stopRecord that stops recording without clearing, and then clear on the next record call instead.

@grandizzy @mattsse thoughts?

tushar994 changed the titleadd vm.stopRecordAndReturnAccesses() cheatcode WIP: add vm.stopRecordAndReturnAccesses() cheatcode

Apr 26, 2025

@grandizzy sorry for noob question, but how do I run the integration tests in testdata/default/cheats locally?

I think we need a stopRecord that stops recording without clearing, and then clear on the next record call instead.

@grandizzy @mattsse thoughts?

Thanks, this looks good! However, I realized that this might not cover all use cases: calling stopRecordAndReturnAccesses resets the accesses and it returns them for one address; if I want to stop and return for multiple addresses, this is not possible as accesses calls after will always return empty arrays;

I think we need a stopRecord that stops recording without clearing, and then clear on the next record call instead.

@grandizzy @mattsse thoughts?

Yeah, that makes sense. So stopRecord should not set state.accesses to None but rather set a flag to mark it as stopped/no new records allowed - that is to still be able to retrieve records after (would here make sense to have a resumeRecord that will reallow records in case one wants to skip some actions?). Then a new record call should set the flag to true and reset all recorded accesses. @DaniPopes @tushar994 wdyt?

I think we need a stopRecord that stops recording without clearing, and then clear on the next record call instead.
@grandizzy @mattsse thoughts?

Thanks, this looks good! However, I realized that this might not cover all use cases: calling stopRecordAndReturnAccesses resets the accesses and it returns them for one address; if I want to stop and return for multiple addresses, this is not possible as accesses calls after will always return empty arrays;
I think we need a stopRecord that stops recording without clearing, and then clear on the next record call instead.
@grandizzy @mattsse thoughts?

Yeah, that makes sense. So stopRecord should not set state.accesses to None but rather set a flag to mark it as stopped/no new records allowed - that is to still be able to retrieve records after (would here make sense to have a resumeRecord that will reallow records in case one wants to skip some actions?). Then a new record call should set the flag to true and reset all recorded accesses. @DaniPopes @tushar994 wdyt?

I had thought the same, but didnt implement it because I thought i'd just stick to what was proposed in the issue. Will implement that! thank you!

sorry closed it by accident

tushar994 changed the titleWIP: add vm.stopRecordAndReturnAccesses() cheatcode add stopRecord and resetRecord cheatcodes

Apr 29, 2025

Have added two cheatcodes

• resetRecord. This clears all records calls by accesses.
• stopRecord. This pauses recording of calls.
  @grandizzy

please review. thank you!!

Choose a reason for hiding this comment

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

Thanks! I fixed some nits. I also removed resetRecord since we reset on the next record call anyway, and so resetRecord can be "implemented" using vm.record(); vm.stopRecord(); if really necessary.

DaniPopes changed the titleadd stopRecord and resetRecord cheatcodes feat(vm): add stopRecord

Apr 29, 2025

DaniPopes changed the titlefeat(vm): add stopRecord feat(cheats): add stopRecord

Apr 29, 2025

DaniPopes changed the titlefeat(cheats): add stopRecord feat(forge): add vm.stopRecord

Apr 29, 2025

Thanks! I fixed some nits. I also removed resetRecord since we reset on the next record call anyway, and so resetRecord can be "implemented" using vm.record(); vm.stopRecord(); if really necessary.

got it! makes sense. I thought keeping different commands for each keeps it more general, so it will accomodate more use cases. with the current implementation, theres no way to pause recording and then unpause with previous state intact. however, that also might not be a use case that anyone wants. thank you!!

grandizzy added a commit to foundry-rs/forge-std that referenced this pull request

May 16, 2025

Reviewers

grandizzy grandizzy approved these changes

DaniPopes DaniPopes left review comments

klkvr Awaiting requested review from klkvr klkvr is a code owner

mattsse Awaiting requested review from mattsse mattsse is a code owner

yash-atreya Awaiting requested review from yash-atreya yash-atreya is a code owner

zerosnacks Awaiting requested review from zerosnacks zerosnacks is a code owner