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

Add vm.unixTime to VmSafe interface #465

Merged
merged 4 commits into from
Oct 2, 2023

Conversation

Tudmotu
Copy link
Contributor

@Tudmotu Tudmotu commented Oct 2, 2023

Following foundry-rs/foundry#5952.
Adding vm.unixTime() cheatcode to VmSafe interface.
I tested this change manually on my machine.

@mds1
Copy link
Collaborator

mds1 commented Oct 2, 2023

Thanks! Sorry for the conflicts, but I just merged #464 which has two main changes:

  1. It refactors the Vm file quite a bit to be more organized
  2. It adds a test to verify we didn't accidentally change the Vm interfaces

Could you fix the conflicts on this branch as a result? We can put the new cheat in VmSafe under the "Filesystem" comment block. (I think we should rename that section to "OS and Filesystem", then move the two ffi cheats from the "External Interactions" into that section too, and delete "External Interactions". If you want to do that, that'd be awesome! No worries if not though)

Then you'll also need to update the failing test based on whatever the new interfaceId is

@Tudmotu
Copy link
Contributor Author

Tudmotu commented Oct 2, 2023

Hi @mds1 thanks for the heads up.
I updated my branch with your asks. I was not sure what should I name the new "subsections".
I named them "Running executables" and "System information". Please let me know what you think:
image

I can change these if you'd like me to name them something else.

Edit: I now realize maybe I should have just used the "External Interactions" for the subsection title and group ffi & unixTime together? Let me know what you think.

Copy link
Collaborator

@mds1 mds1 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!

@mds1 mds1 merged commit dcb0d52 into foundry-rs:master Oct 2, 2023
3 checks passed
@Tudmotu
Copy link
Contributor Author

Tudmotu commented Oct 3, 2023

TIL what ffi means haha
Thanks for merging @mds1 🙏

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