🐛 ModuleNotFound:interfaces.IERC20Permit via ape

[tloriato]

Describe the issue:

I'm terribly sorry to be opening an issue, but I have exhausted Google and hit a wall 2 hours ago.

I have a boilerplate Ape framework created with ape init and the necessary plugins to run Vyper. I can successfully compile examples of Vyper copied-and-paste from the official documentation. My ape-config.yaml is extremely simple with just the solidity: version: 0.8.1

I installed snekmate with the suggested pip method via pip install snekmate.

I'm trying to create a test ERC20 to validate my development setup. The ERC20.vy inside the /contracts folder is:

# pragma version ~=0.4.0
from snekmate.auth import ownable
from snekmate.tokens import erc20

initializes: ownable
initializes: erc20[ownable := ownable]

@deploy
def __init__(_name: String[25]):
    ownable.__init__()
    erc20.__init__(_name, "ERC20", 18, "name", "name2")

When running ape compile I ge thte following:

ERROR: 'interfaces' may not be installed. Could not find it in Ape dependencies or Python's site-packages. Error: Package 'interfaces' not found in site-packages.
ERROR: 'interfaces' may not be installed. Could not find it in Ape dependencies or Python's site-packages. Error: Package 'interfaces' not found in site-packages.
INFO: Compiling using Vyper compiler '0.4.0'.
Input:
        contracts/ERC20.vy
        [...stack-traces...]
ERROR: (CompilerError) contracts/ERC20.vy
ModuleNotFound:interfaces.IERC20Permit

  contract ".venv/lib/python3.10/site-packages/snekmate/tokens/erc20.vy:69", line 69:0 
       68 # syntax.
  ---> 69 import interfaces.IERC20Permit as IERC20Permit
  --------^
       70 implements: IERC20Permit

I'm not sure what I'm doing wrong. I'm uploading this "boilerplate" struct to a personal repository for easiness.

Code example to reproduce the issue:

(not using python virtual environments)

git clone https://github.com/tloriato/ape-boilerplate
cd ape-boilerplate

pip install requirements.txt
ape compile

Version:

0.3.10+commit.9136169

Relevant log output:

No response

[pcaversaccio]

I think this an ape issue not a snekmate issue. Just to confirm, you use Vyper 0.4.0 (because you write you use 0.3.10)? Also, can you please use a pyhon virtual env.

[tloriato]

@pcaversaccio

• Sorry, I can confirm that I'm using Vyper 0.4.0 with Ape, you are right.
• I'm using a virtual env, I just tried to strip everything from the other repo, sorry.

I can see it might be a ape issue, you are right. Seems weird that no one has bumped into it yet, I tried to google without any luck, especially considering it seems the "default stack".

Should I cross-post to them?

[pcaversaccio]

@pcaversaccio

• Sorry, I can confirm that I'm using Vyper 0.4.0 with Ape, you are right.
• I'm using a virtual env, I just tried to strip everything from the other repo, sorry.

I can see it might be a ape issue, you are right. Seems weird that no one has bumped into it yet, I tried to google without any luck, especially considering it seems the "default stack".

Should I cross-post to them?

Yes please close this issue for now and create a new one in ape referring to this issue.

[tloriato]

@pcaversaccio As a curiosity, is there something you might recommend instead of Ape?

[pcaversaccio]

Try titanoboa as well.

[fubuloubu]

Tracking this down, its because snekmate is ambigously referencing an interfaces module that exists relative to the local file in some sources, which relies on a new feature of vyper leveraging a user-configurable syspath.

My suggestion would be that snekmate use a relative path here (e.g. from .interfaces) instead of absolute, just in case the implementation details of the vyper syspath change in a future version, or there is a user configuration issue leading to non-deterministic builds for end users of snekmate.

We are going to fix this in ape regardless, since we do some build pre-processing to determine build artifact caches and do publishing tasks (especially on explorers/chains that dont support the new syspath-based archive format vyper has), but thank you for reporting this issue!

[pcaversaccio]

I disagree - it's not ambiguous. Both statements are technically equivalent:

• import interfaces.IERC20Permit as IERC20Permit
• from .interfaces import IERC20Permit

I have been using this approach even before Vyper 0.4.0, see e.g. here (release 0.5.0 using Vyper 0.3.10). Also, Vyper's current syspath approach is powered by Python and as long as this is strategy is kept, it will work.

[fubuloubu]

I disagree - it's not ambiguous.

PEP standards suggest that a relative import should be referenced by a relative path, to avoid ambiguities when core modules are added that may conflict (as they supersede even relative paths), and a number of other scenarios that can potentially be triggered when a user manages their own syspath (which is possible/encouraged with Vyper)
https://peps.python.org/pep-0328/

While it certainly works, it is bad practice and should be discouraged in publicly released libraries that may find use downstream in many other user's projects, included those that need to be maintained for quite a long time (where things are likely to change, new modules added to stdlib, etc.). End users are still likely do it, which is why we are going to build in handling for this anyways, but it sets a poor example for others to follow for snekmate to publish code using a known bad practice.

I have been using this approach even before Vyper 0.4.0

Yes, prior to v0.4, Vyper was much more constrained on how you reference paths. No longer a constraint though since you are not maintaining Vyper <v0.4 compatibility. Also, Vyper's module search functionality is dramatically expanded with v0.4.

Also, Vyper's current syspath approach is powered by Python and as long as this is strategy is kept, it will work.

Python can make changes to how syspath is processed in future versions, there are no guarantees for how paths are managed, only suggestions on how developers should specify their own paths in order to minimize potential conflicts. Vyper can also make changes to how this is done as well, including adding more stdlib modules (actually not that long ago interfaces was indeed a stdlib "module" under the vyper namespace)

Lastly, why deal with the footgun if from .interfaces import IERC20Permit is actually shorter, easier to type, and less ambigious than import interfaces.IERC20Permit as IERC20Permit? Seems like a strange convention to set across your library.

[pcaversaccio]

Let me explain why everything I do is a very sane approach and not "bad practice" or "strange" how you call it.

First, according to PEP 8, absolute imports are recommended:

Absolute imports are recommended, as they are usually more readable and tend to be better behaved (or at least give better error messages) if the import system is incorrectly configured (such as when a directory inside a package ends up on sys.path) [...].

Standard library code should avoid complex package layouts and always use absolute imports.

Second, I use the following reasoning:

• Use import ... as ... if the interfaces folder is at the same directory level as the importing contract.
• Use from ... import ... if the interfaces folder is somewhere else in the directory tree.

You might wonder why this is a sane to do now: well the answer is simply the Vyper import rules:

When looking for a file to import, Vyper will first search relative to the same folder as the contract being compiled. It then checks for the file in the provided search paths, in the precedence provided. Vyper checks for the file name with a .vy suffix first, then .vyi, then .json.

These rules differ from the normal Python rules which will not look in the current working directory unless it is explicitly added to sys.path. Thus, import interfaces.IERC20Permit as IERC20Permit will resolve faster under Vyper import rules.

Finally, this is not a hard argument but I very much dislike the dot notation for imports and only use it if really required.

[fubuloubu]

First, according to PEP 8, absolute imports are recommended:

Absolute imports are recommended, as they are usually more readable and tend to be better behaved (or at least give better error messages) if the import system is incorrectly configured (such as when a directory inside a package ends up on sys.path) [...].

Standard library code should avoid complex package layouts and always use absolute imports.

Absolute imports from the root package are the recommended best practice. So, you would have to qualify everything from snekmate as the root package to actually be following this suggestion (because snekmate is not ambigious given the user already configures their path to use it)

They are definitely not suggesting to use an absolute path to a relative module, especially when that module is a common name both across your library and in general (many projects end up adding an interfaces submodule as a relative import).

Second, I use the following reasoning:

• Use import ... as ... if the interfaces folder is at the same directory level as the importing contract.
• Use from ... import ... if the interfaces folder is somewhere else in the directory tree.

Up to you on what style you want to maintain, but seems like you could reduce this to always prefer the second so its easier for contributors to understand.

Thus, import interfaces.IERC20Permit as IERC20Permit will resolve faster under Vyper import rules.

Does this actually have any palable impact on compiling speed? Legitimately curious

[bitwise-constructs]

I think the confusion here is that what the Vyper docs refer to as an "absolute import" is not the same as Python's "absolute import", where the path is wholly unambiguous because the first level is always a package. Instead Vyper's absolute import is more like Python's "implicit relative import" which has been removed from Python 3 because it was so ambiguous. So if we're invoking PEP 8 it would prefer absolute(Python) > relative > absolute(Vyper).

The Ape compiler will need to match Vyper's import rule priority when searching for an unknown absolute(Vyper) import but I do not understand why we believe that this would resolve faster than a relative import, which has no searching to do.

[pcaversaccio]

Does this actually have any palable impact on compiling speed? Legitimately curious

No palpable impact at all, but when I started building the current existing imports ~2 years ago I was reading the Vyper docs and it stated (as I already referenced above): "When looking for a file to import, Vyper will first search relative to the same folder as the contract being compiled." and thus I thought if you use an absolute path approach like import interfaces.IERC20Permit as IERC20Permit for contracts which have an interfaces directory at the same level as to-be-compiled contract, it will resolve in the most efficient way, provided how Vyper (and underlying Python & OS) resolves paths.

I think the confusion here is that what the Vyper docs refer to as an "absolute import" is not the same as Python's "absolute import", where the path is wholly unambiguous because the first level is always a package. Instead Vyper's absolute import is more like Python's "implicit relative import" which has been removed from Python 3 because it was so ambiguous. So if we're invoking PEP 8 it would prefer absolute(Python) > relative > absolute(Vyper).

The Ape compiler will need to match Vyper's import rule priority when searching for an unknown absolute(Vyper) import but I do not understand why we believe that this would resolve faster than a relative import, which has no searching to do.

In Python, resolving an absolute path is generally faster than resolving a relative path. The reason being that when you use an absolute path the OS can directly locate the file without needing any further resolution steps (the full path from the root directory (in our case, the directory where the contract which will be compiled sits) to the target file is specified). When you use a relative path, Python (and the underlying OS) must first determine the current working directory and then resolve the relative path from there. This adds an additional step to the path resolution process.

Let me ping @charles-cooper as obviously there is some confusion around imports here. Who knows, maybe we're all wrong...

[fubuloubu]

I think the confusion here is that what the Vyper docs refer to as an "absolute import" is not the same as Python's "absolute import", where the path is wholly unambiguous because the first level is always a package. Instead Vyper's absolute import is more like Python's "implicit relative import" which has been removed from Python 3 because it was so ambiguous. So if we're invoking PEP 8 it would prefer absolute(Python) > relative > absolute(Vyper).

The Ape compiler will need to match Vyper's import rule priority when searching for an unknown absolute(Vyper) import but I do not understand why we believe that this would resolve faster than a relative import, which has no searching to do.

In Python, resolving an absolute path is generally faster than resolving a relative path. The reason being that when you use an absolute path the OS can directly locate the file without needing any further resolution steps (the full path from the root directory (in our case, the directory where the contract which will be compiled sits) to the target file is specified). When you use a relative path, Python (and the underlying OS) must first determine the current working directory and then resolve the relative path from there. This adds an additional step to the path resolution process.

The absolute path relies on python syspath convention that the local directory of a script is added to the path in the first position when it is parsed, so if there is a conflict (due to ambigiuity) whatever takes precedence in the syspath is chosen. Since syspath is configurable, that means different orderings can be applied (in fact, Vyper uses this internally by modifying the python syspath as it compiles)

Using relative import is unambigious because you can't name a file the same as another file, and there's other rules such as a module file and a module folder cannot co-exist.

Sure, maybe its slightly "slower" to have to look up a module relatively in this way, but I don't think of that as a perceivable difference to the end user. The confusion when a completely different interface resolves due to some path misconfiguration or change in rules when upgrading the compiler will be very noticable however.

Let me ping @charles-cooper as obviously there is some confusion around imports here. Who knows, maybe we're all wrong...

Please do, I think in general we shouldn't use "what vyper did in the past" as any sort of argument of what to do here, because the syspath rules makes it much more complicated due to the resolution order being configurable by a user.

PEP8 specifically calls out the legacy practice of absolute relative imports as dangerous due to its ambigiuty, I don't remember if this triggers a flake8 error specifically or if its just a suggested best practice, but as a maintainer of over 60 python packages this ticks a lot of alarm bells for me.

Since snekmate is an important living example of vyper code, what is "standard practice" here becomes common elsewhere, so I think we should highlight the best way to do things in the least ambigious way possible. That's my reason why I am not letting this go.

[charles-cooper]

Yea hm when I wrote the vyper 0.4.0 import resolver I adopted the rule that the directory of the current module is added to the syspath, including in the recursion. This breaks from the pythonic way which only performs the syspath change at the top level. This could be arguably a bug in the vyper implementation as I did not intend to diverge from python.

I think my position is we could "fix" the resolution rules in the recursion in vyper 0.4.1 or 0.4.2. I don't feel super strongly about this though.

[pcaversaccio]

After thinking about it carefully for another day, my personal goal is that the 🐍 snekmate contracts aim to minimise friction across all Vyper development frameworks, so adopting relative imports is probably the most future-proof strategy. Addressing this via #263.