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

VIP: Support for importing interfaces from EthPM packages. #1371

Closed
pipermerriam opened this issue Mar 28, 2019 · 10 comments
Closed

VIP: Support for importing interfaces from EthPM packages. #1371

pipermerriam opened this issue Mar 28, 2019 · 10 comments
Labels
VIP: Deferred VIP is not scheduled to move forward at this time

Comments

@pipermerriam
Copy link
Contributor

Simple Summary

In Vyper the concept of a "Contract Interface" is a loose wrapper around the pre-existing concept of a contract ABI. Since EthPM exposes contract ABI as a first class attribute Vyper should support the ability to import an interface directly from an EthPM package.

Abstract

Support for importing contract interfaces from EthPM packages.

Motivation

The EthPM package format provides a robust machine readable format for describing smart contracts. Currently, if a Vyper developer wishes to interact with a contract which is being distributed by an EthPM package, they will have to either manually create an interface for the packaged contracts, or manually extract the ABI from the package. Both of these are error prone and do not lend themselves to easy upgrades if a new version of the package is released.

Supporting direct imports from EthPM packages would improve security and usability for contract developers.

Specification

Familiarity with the EthPM specification will be beneficial for understanding the following.

The EthPM spec contains a mapping of contract names to their type information under the top level key "contract_types". In the following example we assume that there is an erc20.json file located in a location that Vyper can find it.

Using this import statement as an example:

from erc20.contract_types import ERC20

When vyper encounters an import statement like the above it would do the following.

  1. Check whether there is a file in one of the defined search locations named erc20.json
  2. Check the file is a valid EthPM package manifest
    • py-ethpm contains an existing implementation of this validation.
  3. Check whether the manifest defines a contract_types key.
  4. Check whether the contract types define a contract type by the name "ERC20"
  5. Check whether the contract type named "ERC20"defines an "abi".
  6. Use the defined contract ABI to construct a contract interface

Open Questions

Runtime bytecode checking.

Since EthPM packages allow inclusion of the runtime bytecode it would be possible for vyper to include a pre-check of the EXTCODEHASH of the target contract prior to interaction at the cost of increased gas usage for external calls.

It is worth noting that this could give an illusion of security and should probably be addressed via a different mechanism.

Compatibility with future plans for import system

The semantics for how vyper negotiates imports is not well known to me (Piper). It's unclear whether there is a cohesive plan for how vyper will handle imports, nor whether this syntax could make future development in this area more difficult.

One example might be if vyper ends up supporting some semblance of Solidity's library functionality. If such functionality were introduced, then it's likely that the import semantics for that could look very similar which would require vyper to be capable of managing both concepts within the same import semantics.

Backwards Compatibility

It is unclear whether this will cause any backwards compatibility changes. Vyper's existing support for importing directly from JSON documents will have to be adjusted to do some form of content negotiation to determine which approach to use. This should be relatively straight forward and is not expected to be problematic.

Dependencies

No Dependencies

Copyright

Copyright and related rights waived via CC0

@jacqueswww jacqueswww added the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Mar 28, 2019
@fubuloubu
Copy link
Member

Love ethPM support, definitely should consider how we could build this in to make working with ethPM manifests (and the libraries and frameworks that might support them) as easy as possible.

@pipermerriam
Copy link
Contributor Author

I took a stab at writing up the concept I've had in my head for how EthPM packages would be installed to disk.

ethpm/py-ethpm#150

Not immediately relevant because importing directly from a file makes sense, but longer term if the above gets codified into a formal spec (aka ERC) then it would start to make sense for vyper to maintain some fort of $PATH concept that included whatever directory that ERC specified as well as having vyper know how to negotiate the directory structure properly.

@fubuloubu
Copy link
Member

I think another thing to keep in mind is that many tools that work this way maintain some sort of local "installed packages" directory. Truffle does this through installed_contracts/ which I like to commit to my repo so that it is clear what code and version I am using. I would expect the same from any tool making use of this feature here.

@pipermerriam
Copy link
Contributor Author

I've thought about whether this would be something that is intended to be committed to a codebase and I think the answer is that it doesn't matter (it's an implementation detail). This is under the assumption that 1) the directory structure is well spec'd out and 2) that proper implementation of tools that consume this data involves an integrity check on the data before using it via verification of all of the content hashes.

@jacqueswww
Copy link
Contributor

From meeting: whatever mechanism brings the contracts to vyper will be handled the same, and for vyper. And because we will be adding VYPER_PATHS to #1367 we should be sorted.

@pipermerriam
Copy link
Contributor Author

Sorry to miss the meesing. Child-care fell through this morning. I haven't written anything new up but I've been mulling over this concept intermittently and am making some progress on the concept. I'll try to be at the next one.

@fubuloubu
Copy link
Member

@pipermerriam do you agree with our assessment that whatever mechanism we create to handle interface imports (and other related things) can handle ethPM use cases the same way, removing the need for an ethPM-specific featureset?

@pipermerriam
Copy link
Contributor Author

@fubuloubu I think so, but I'm making some assumptions about what you mean.

Would it be correct to say that when you say ethPM-specific features you are referring to extenal API type things since internally there will still have to be some ethpm-specific handling code to read the package data. The user however writing a contract with import statements will not necessarily need to denote anything ethpm specific in their import statements.

@fubuloubu
Copy link
Member

I more mean that the higher level framework (whatever that is) will take care of ethPM-specific features (like package validation), and provide the compiler with "dumb inputs" (i.e. unaware of ethPM featuresets) such that this is not a concern at our compiler's API. I believe trying to do too much of the ethPM functionality may interrupt how it is used in different frameworks (like Truffle).

@fubuloubu
Copy link
Member

fubuloubu commented Dec 12, 2019

To refine this further, similar to how the vyper compiler cli will accept a json-encoded ABI interface from a file as an input, the vyper package itself allows passing a dict of ABI interfaces directly to it. This enables a framework to easily integrate itself with this functionality via one of the two methods.

This functionality is available in Vyper today, so this VIP can be considered obsolesced by that functionality. (Should have done this a while back)

@fubuloubu fubuloubu added VIP: Approved VIP Approved VIP: Deferred VIP is not scheduled to move forward at this time and removed VIP: Approved VIP Approved VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting labels Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Deferred VIP is not scheduled to move forward at this time
Projects
None yet
Development

No branches or pull requests

3 participants