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 - define top level namespace and relegate everything else to "standard library" #1347

Closed
pipermerriam opened this issue Mar 11, 2019 · 11 comments
Labels
VIP: Approved VIP Approved

Comments

@pipermerriam
Copy link
Contributor

pipermerriam commented Mar 11, 2019

Simple Summary

As vyper adds new built-in functions and reserved words there will be an increased likelihood of name collision in user code. Support for namespacing new built-ins will remedy this issue by reducing the number of names in the top level namespace.

Abstract

Vyper needs some version of what python calls, the "standard library". A set of modules that are available by default for any Vyper contract to import and use, but which are not available in the global namespace by default.

Motivation

The motivation for this comes from seeing multiple VIPs that introduce new top-level namespace variables.

All of these are names that a user could have already created in their contracts. By adding these to the top level namespace, we will 1) reduce the number of available names a user may use in their contract code and 2) cause breakage in existing code that already uses one of these names.

Specification

I propose introducing an importable standard library to vyper.

from math import sqrt
from accounting import funds, credit, debit
from bigmath import bigadd, bigmul, bigdiv, bigmod

Similar to python, I propose we define a builtins module that contains all of the functions which are available by default in the global namespace. The vyper compiler would treat every vyper file as-if it had imported all of the top-level names from the builtins module.

Any functions that are currently in the top level namespace which are deemed not to belong in the builtins should 1) be moved to an importable module and 2) be replaced by a version that behaves the same but raises deprecations warnings for some period before being removed from the global namespace.

Bonus points if we could combine this with EthPM packages in some way as it would lay the foundation for installing 3rd party modules which provide fancy functions and functionality not available in the standard library.

Extra bonus points if this API allows us to leverage DELEGATECALL style libraries to reduce deployment costs when appropriate.

Backwards Compatibility

This would be backwards incompatible with respect to any function that currently exists in the top level namespace that is deemed to not belong there, but there is a clear and simple deprecation path for these.

Dependencies

None that I know of.

Copyright

Copyright and related rights waived via CC0

@fubuloubu
Copy link
Member

I like this proposal, would need a deeper investigation of what keywords we would cover under this. math is probably clear one

@pipermerriam
Copy link
Contributor Author

In case it isn't clear to anyone, the names I used in my example math/accounting/bignum are meant to be illustrative. Defining the actual namespaces/module is something that can/should be done at a later stage once vyper can handle the concepts of importing these.

@jacqueswww
Copy link
Contributor

jacqueswww commented Mar 12, 2019

Would it be more explicit to force namespace importing?

i.e.

from vyper.functions import math

def some_func() -> decimal:
       math.sqrt(1, 2)

I think this might be even better.
Also note that this breaks current interface imports - so we would have to figure another way to do interface imports (builtin vyper.interfaces are simple, we could just say that stdlib function import from vyper.* at all times)?

@jacqueswww jacqueswww added the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Mar 14, 2019
@pipermerriam pipermerriam changed the title [VIP] define top level namespace and relegate everything else to "standard library" VIP - define top level namespace and relegate everything else to "standard library" Mar 15, 2019
@jacqueswww
Copy link
Contributor

jacqueswww commented Mar 25, 2019

I see I missed this VIP for this weeks' call, added to agenda for the next one ;)

@pipermerriam
Copy link
Contributor Author

I'll try to make the meeting on the 8th so I can explain some of my thinking on this topic.

@jacqueswww
Copy link
Contributor

Approved, but decisions have to be made still once implemented:

  • Conflicting interface syntax
  • Which functions go where

@jacqueswww jacqueswww added the VIP: Approved VIP Approved label Apr 8, 2019
@fubuloubu fubuloubu removed the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Aug 8, 2019
@fubuloubu fubuloubu added this to the v0.2 Release milestone Jun 21, 2020
@fubuloubu
Copy link
Member

fubuloubu commented Jun 30, 2020

Suggested namespacing (note: still illegal names for variables/functions):

# Environment variables
from ethereum.environment import block, chain, msg, tx 

# Ethereum utility functions
from ethereum.utils import (
    as_wei_value,
    blockhash,
    create_forwarder_to,
    method_id,
    raw_call,
    raw_log,
    selfdestruct,
)

# Built-in Constants
from vyper.constants import ZERO_ADDRESS, EMPTY_BYTES32, ...  # And all the rest

# Cryptography functions
from vyper.cryptography import keccak256, sha256, ecrecover, ecadd, ecmul

# Math functions
from vyper.math import ceil, floor, sqrt, uint256_addmod, uint256_mulmod

Note that the following are not namespaced:

assert
concat
convert
empty
extract32
raise
send
slice

@iamdefinitelyahuman
Copy link
Contributor

  1. I'm not a fan of requiring that the environment variables be imported.
  • I don't think I've ever written a contract where I didn't access msg.sender or msg.value at some point.
  • The value for these variables is constant, but often constant in relation to the active transaction. And depending on the function mutability, I can only access some of these values sometimes. So we have an import of a thing that has a different value depending on where I access it, and that I can't always access. It feels weird,
  • Given all environment variables are struct-like, it leads to questions such as: is from ethereum.environment.msg import value legal?
  1. I'd also prefer to keep everything in the same root package: ethereum.utils should be vyper.utils.
  • Every name we use is one less name available to the user for their own project structure.
  • If I know that everything is in vyper it's much easier to know that anything that isn't vyper is definitely importing an interface. As soon as there are two options.. what if a new patch release added a third option? It makes it harder to grok what an import statement is doing.

Generally the layout you propose for utils, constants, cryptography, math makes sense to me, but I need to stare at it some more.

@fubuloubu
Copy link
Member

fubuloubu commented Jun 30, 2020

I put them underethereum like that because they are specific to an Ethereum target architecture. I was thinking about if we ever supported non-Ethereum targets, how difficult it would be to ween people off of using these (or having to swap the functionality of these items when another target is chosen). Perhaps it is too onerous and weird to do that for environment variables (we could simple raise if another target didn't supply them like we do with chain.id), but I'd like to keep the functions the way they are

Everything under the vyper namespace is specific to Vyper and not dependent on it's target architecture

@fubuloubu fubuloubu removed this from the v0.2.0 Release milestone Jun 30, 2020
@fubuloubu
Copy link
Member

Punting on v0.2.0 milestone because the bikeshed should be purple

@charles-cooper
Copy link
Member

as of 0.4.0 on a technical level we can do this fairly easily (see for instance that c6f457a moves builtin interfaces to pure vyper .vyi files). i'm closing this issue however because it is too broad, we can revisit each class of builtins piecemeal.

@charles-cooper charles-cooper closed this as not planned Won't fix, can't repro, duplicate, stale Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Approved VIP Approved
Projects
None yet
Development

No branches or pull requests

5 participants