ape-vyper icon indicating copy to clipboard operation
ape-vyper copied to clipboard

feat: vyper 0.4 support

Open antazoey opened this issue 10 months ago • 19 comments

What I did

Make it so can compile on 0.4.0

How I did it

Had to use absolute paths.

How to verify it

Checklist

  • [ ] Passes all linting checks (pre-commit and CI jobs)
  • [ ] New test cases have been added and are passing
  • [ ] Documentation has been updated
  • [ ] PR title follows Conventional Commit standard (will be automatically included in the changelog)

antazoey avatar Apr 11 '24 14:04 antazoey

Is this checked against 0.3.10 and 0.4.0?

Sorry meant to make this a draft.. The tests are all pre-0.4.0, but I need to add more tests for 0.4.0 range.

antazoey avatar Apr 11 '24 15:04 antazoey

i think rather than using absolute paths, you can add the "./" to the search path

charles-cooper avatar Apr 11 '24 15:04 charles-cooper

i think rather than using absolute paths, you can add the "./" to the search path

I tried this but was running into other issues, I can keep trying though but I was in a tangled web of confusion.

antazoey avatar Apr 11 '24 15:04 antazoey

Looks like vyper in the 0.3.0 range does not like the absolute paths

antazoey avatar Apr 11 '24 15:04 antazoey

currently it fails to process .vyi files image

pcaversaccio avatar Apr 11 '24 15:04 pcaversaccio

Looks like imports don't resolve yet:

image

pcaversaccio avatar Apr 11 '24 15:04 pcaversaccio

WIP - still in development, please be patient.

antazoey avatar Apr 11 '24 16:04 antazoey

FYI @fubuloubu, it doesn't work yet on snekmate: https://github.com/pcaversaccio/snekmate/actions/runs/8651807519/job/23771914522#step:8:1 Screenshot_20240413_081527_GitHub.jpg

pcaversaccio avatar Apr 13 '24 06:04 pcaversaccio

i think constructing the input dict from just a directory is going to be pretty complicated, because it's hard to predict how the compiler will interact with search path (cf. for instance: vyperlang/vyper@b0ea5b6f1c8cd8d09db6f37e9857f9b3837fb386, vyperlang/vyper@d3723783caf3edfd8905fea0b221f99f18eeb27b, vyperlang/vyper@c6f457a73db40e4b113497883bd330e0dcec28d1, vyperlang/vyper@a3bc3eb50ea10788a688ea79d74d294cd9a418d6, vyperlang/vyper@5d10ea0d2a26ab0c58beab4b0b9a4a3d90c9c439).

i'm not sure what direction ape wants to take here, but maybe the best thing might be to call the compiler and recursively get its ast output (per vyperlang/vyper@9428196a0c48ec7cc02938ee4ae1a0e2fee133c8), or just use the cli and get -f combined_json or something. we could do something else too like adding -f import_graph as a compiler output option or something, i'm open to suggestions.

charles-cooper avatar Apr 14 '24 13:04 charles-cooper

we could do something else too like adding -f import_graph as a compiler output option or something, i'm open to suggestions.

Having the import graph would be extremely useful, in both directions (know which code modules import which other code modules so that a downstream module change will trigger us to recompile one that hasn't because of the dependency)

fubuloubu avatar Apr 14 '24 16:04 fubuloubu

@fubuloubu also check -f annotated_ast output, it contains information about the imported modules including the sha256sum of the content. you can use this to compute a reproducible import graph. https://github.com/vyperlang/vyper/pull/3891 goes further and includes an integrity hash output, which is the recursive hash of the file and all its dependencies. in vyperlang/vyper#3891 we could maybe add a json output, i'm not sure if that's very helpful here (since in any case we have to run in "regular" non-json mode first to grab the import graph).

maybe the best thing is to ditch standard-json input here? because it's not trivial to construct. from CLI, everything just works since the compiler will resolve things in the filesystem automatically.

if you want to keep using standard-json and one-shot the compilation (i.e., not have to query the compiler multiple times about a file), the other way would be to bundle everything ape knows is in the environment (hypothetical example: shove contracts/, libraries/, modules/) all into json input. that provides the best chance of successful compilation, although you still might end up getting divergences between what the compiler thinks is in the environment and what ape thinks is in the environment.

charles-cooper avatar Apr 14 '24 16:04 charles-cooper

@pcaversaccio I promise I will make sure snekmate compiles before we merge this.

antazoey avatar Apr 15 '24 13:04 antazoey

@fubuloubu also check -f annotated_ast output, it contains information about the imported modules including the sha256sum of the content. you can use this to compute a reproducible import graph. vyperlang/vyper#3891 goes further and includes an integrity hash output, which is the recursive hash of the file and all its dependencies. in vyperlang/vyper#3891 we could maybe add a json output, i'm not sure if that's very helpful here (since in any case we have to run in "regular" non-json mode first to grab the import graph).

maybe the best thing is to ditch standard-json input here? because it's not trivial to construct. from CLI, everything just works since the compiler will resolve things in the filesystem automatically.

if you want to keep using standard-json and one-shot the compilation (i.e., not have to query the compiler multiple times about a file), the other way would be to bundle everything ape knows is in the environment (hypothetical example: shove contracts/, libraries/, modules/) all into json input. that provides the best chance of successful compilation, although you still might end up getting divergences between what the compiler thinks is in the environment and what ape thinks is in the environment.

We basically want a way to know that we don't need to even call vyper in the first place because none of the files (or their dependencies) have made an update which should change their output.

Since subprocessing out to vyper is more expensive than not, having the checksum information in Ape's project metadata helps us avoid unnecessary extra compiler invocations

It is also still valuable that vyper does this check to, but would be great if that information were made accessible back to how Ape stores it's own project cache (which may include multiple compilers to check for)

fubuloubu avatar Apr 15 '24 21:04 fubuloubu

Let's see if this is even a problem first as well. This is still in development, but getting closer each day. If we can handle Solidity, I think we can handle Vyper.

antazoey avatar Apr 15 '24 21:04 antazoey

Let's see if this is even a problem first as well. This is still in development, but getting closer each day. If we can handle Solidity, I think we can handle Vyper.

0.4.0 made a big shift to absolute file locations from std json input, this is very different than how solidity or vyper (in the past) has handled file inputs

but yeah, calling annotated_ast (if available) sounds like it will give us the checksum information we can store in our own metadata cache. I missed the integrity hash concept, but since we only add back references for dependencies to be able to determine this, it could also help. That was non-standard in our EthPM v3 implementation

fubuloubu avatar Apr 15 '24 22:04 fubuloubu

We basically want a way to know that we don't need to even call vyper in the first place because none of the files (or their dependencies) have made an update which should change their output.

Since subprocessing out to vyper is more expensive than not, having the checksum information in Ape's project metadata helps us avoid unnecessary extra compiler invocations

since dependencies can change even if source code (of the compilation target) has not changed, there are two surefire ways to do this. one is going to be to call the vyper compiler and get the integrity hash (this only runs the frontend analysis passes, which based on latest benchmarks can process roughly 1000 lines of source code per second). one thing i can do (and i was thinking about doing this anyway) is to move the compiler's import analysis into an even earlier phase, so basically the only work the compiler does to get the integrity hash is resolving imports + hashing. but it would be a bit nontrivial to implement.

the other way is to package all the project files into a single package (like standard json input) and check that nothing in the package has changed.

charles-cooper avatar Apr 15 '24 22:04 charles-cooper

one thing i can do (and i was thinking about doing this anyway) is to move the compiler's import analysis into an even earlier phase, so basically the only work the compiler does to get the integrity hash is resolving imports + hashing. but it would be a bit nontrivial to implement.

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

fubuloubu avatar Apr 16 '24 15:04 fubuloubu

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

charles-cooper avatar Apr 16 '24 17:04 charles-cooper

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

how do we know what the targets are?

fubuloubu avatar Apr 17 '24 01:04 fubuloubu

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

how do we know what the targets are?

oh i missed this -- targets are whatever the user asks for. (anything in contracts/?)

charles-cooper avatar Jun 10 '24 18:06 charles-cooper

we essentially do import analysis to build the graph of the project, so this would be really nice to expose. we may in the future execute multiple compiler processes in parallel if we determine (based on the graph) that there are disjoint sets within the project (allowing us to parallelize compiler invocations). an obvious first version of this would be to just parallelize invocations for vyper and solidity files within the same project

there isn't really work-sharing between different compilation targets. like if X imports A, and Y imports A, each one will re-compile A from scratch. so you can just always parallelize compilation targets.

how do we know what the targets are?

oh i missed this -- targets are whatever the user asks for. (anything in contracts/?)

we don't require the user to manually specify their targets, typically we try to identify the most efficient way to compile everything so that when the user goes and asks for a specific target (e.g. project.MyContractTarget) that target is already compiled and available for immediate use.

requiring the user to specify all of their targets strikes me as poor developer experience.

fubuloubu avatar Jun 10 '24 21:06 fubuloubu

So I tested the latest commit and snekmate contracts compile: https://github.com/pcaversaccio/snekmate/pull/244.

Two notes:

  1. When installing Ape, Ape pulls in the old Vyper 0.3.x series (example):
Collecting vyper~=0.3.7 (from ape-vyper==0.1.0b3.dev69+g6154453)
  Downloading vyper-0.3.10-py3-none-any.whl.metadata (6.7 kB)

This is unfortunate since I need to uninstall Vyper and reinstall the new Vyper later again (otherwise I have Vyper 0.3.x installed locally or in the CI). It would be great to have an option to disable this installation step (@antazoey explained to me why this is currently needed for the flattener).

  1. @charles-cooper: in the ape-config.yaml of my PR you can see how I set contracts_folder: src/snekmate and exclude: r"(?!.*_mock\.vy$)", which can be used to isolate what is being compiled by Ape. Also, since 0.8.0, the default base path for compiling projects is now the project root instead of the contracts/ folder.

pcaversaccio avatar Jun 11 '24 11:06 pcaversaccio

  1. When installing Ape, Ape pulls in the old Vyper 0.3.x series (example):
Collecting vyper~=0.3.7 (from ape-vyper==0.1.0b3.dev69+g6154453)
  Downloading vyper-0.3.10-py3-none-any.whl.metadata (6.7 kB)

This is unfortunate since I need to uninstall Vyper and reinstall the new Vyper later again (otherwise I have Vyper 0.3.x installed locally or in the CI). It would be great to have an option to disable this installation step (@antazoey explained to me why this is currently needed for the flattener).

We should probably only have a lower pin for the flattener CLI, or as low a pin as we can manage

Wonder if it makes sense as an extra (and don't add flattener CLI if vyper not installed?) Spitballing here

fubuloubu avatar Jun 11 '24 20:06 fubuloubu

@fubuloubu

Wonder if it makes sense as an extra (and don't add flattener CLI if vyper not installed?) Spitballing here

This was my thought as well.

Another option I was going to look into was check what was being imported from vyper and see if i can copy it in manually (provided it is the same across all versions). It would save a lot of headache and the code won't change much...

Either way, can we do this in another PR?

antazoey avatar Jun 11 '24 21:06 antazoey

Either way, can we do this in another PR?

sounds good, there is a workaround for now (just re-install vyper afterwards)

Another option I was going to look into was check what was being imported from vyper and see if i can copy it in manually (provided it is the same across all versions). It would save a lot of headache and the code won't change much...

can you mark this in an issue?

fubuloubu avatar Jun 12 '24 00:06 fubuloubu

can you mark this in an issue?

https://github.com/ApeWorX/ape-vyper/issues/114

antazoey avatar Jun 12 '24 01:06 antazoey