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

[wip] add Vm.vy #567

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[wip] add Vm.vy #567

wants to merge 1 commit into from

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Jun 7, 2024

Adds script for generating Vm.vy file which can be used to access cheatcodes from Vyper tests.

Current version of the script produces correct interface which can be compiled and imported, however marking as wip because of the following concerns:

  • First of all, the question is how to expose this to Vyper contracts. If this would be kept in lib/forge-std then there is basically no way of importing it from user's project (It could've been something like from lib.forge-std.src import Vm, but - is disallowed in import directives). Vyper 0.4 added search_paths config option, so we should be able to support this for 0.4+.
  • enum is deprecated in favor of identical flag in Vyper 0.4. If we use flag, then Vm.vy will only compile on 0.4+, but if we use enum, a bunch of deprectation warnings will be emitted for 0.4+

One thing I've though about is that we could just keep two versions of Vm.vy and inject them under some path like forge_builtins/Vm.vy depending on the current compiler version. This gives us ability to inject different interfaces when compiling 0.3 and 0.4, and source will always be available through from forge_builtings import Vm, without the need for user to care about import resolution.

Such approach should work for Vm.sol as well, allowing us to keep entire interface in the forge binary itself and removing the need for users to wait for forge-std release to use new cheatcodes.

cc @DaniPopes @mattsse @mds1

@klkvr klkvr changed the title [wip] add Vm.vy [wip] add Vm.vy Jun 7, 2024
@mds1
Copy link
Collaborator

mds1 commented Jun 8, 2024

Nit, but small UX ask would be to have a single vm.py script that generates both interfaces—it can just run vm.solidity.py and vm_vyper.py

Since vyper support in foundry is new though, it seems ok to only support new projects using 0.4 onwards to minimize maintenance and tech debt. But otherwise I'm not familiar enough with vyper to have insight on the open questions, maybe @pcaversaccio has some thoughts?

@pcaversaccio
Copy link
Contributor

pcaversaccio commented Jun 9, 2024

Overall I think only supporting 0.4.0 onwards seems fine IMO.

First of all, the question is how to expose this to Vyper contracts. If this would be kept in lib/forge-std then there is basically no way of importing it from user's project (It could've been something like from lib.forge-std.src import Vm, but - is disallowed in import directives). Vyper 0.4 added search_paths config option, so we should be able to support this for 0.4+.

I personally would like to see a forge_std Python library that I could simply install via pip install forge_std for example. Like this I could simply import it since it's in my Python syspath or I can use pip install forge_std -t . to install it directly into my working directory.

enum is deprecated in favor of identical flag in Vyper 0.4. If we use flag, then Vm.vy will only compile on 0.4+, but if we use enum, a bunch of deprectation warnings will be emitted for 0.4+

Only 0.4.0 is completely fine.

One personal recommendation would be to put the Vm interface into a separate .vyi file.

Copy link
Contributor

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

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

As an FYI, 0.4.0 has finally landed 🫡: https://github.com/vyperlang/vyper/releases/tag/v0.4.0

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.

None yet

3 participants