vyperlang / vyper

Pythonic Smart Contract Language for the EVM
https://vyperlang.org
Other
4.84k stars 788 forks source link

VIP: Support Non-Default `enum` Behaviours #3285

Open pcaversaccio opened 1 year ago

pcaversaccio commented 1 year ago

Original Issue Text (Before Refactoring into a VIP):

Enums are treated differently in Solidity and Vyper. uint8 (in Solidity) vs. uint256 (in Vyper) & simple increments and starting from 0 (in Solidity) vs. 2**n and starting from 1 (in Vyper). I think it would be good to have the possibility to set custom numbers to get the Solidity-style behavior and leave the default to the Vyper method. One implementation issue I currently see is the uint8 vs uint256 difference.


Simple Summary

Support non-Vyper enum behaviours. One example is Solidity, which uses normal integer-based enums.

Motivation

Enums are treated differently in Solidity and other languages, and cross-language consistency is sometimes a desirable feature to have (e.g. testing Vyper contracts with Foundry or writing a smart contract system using Solidity and Vyper, in order to leverage inline assembly for certain optimisations).

Specification

Introduce a new decorator nondefault that takes as argument solidity (and can later be extended to further other languages). This decorator will switch the default Vyper behavior in the form of 2**n, where n is the index of the member in the range 0 <= n <= 255, to normal integer-based enums supported by Solidity. Generally, this decorator can later also be used for other cross-language anomalies to ensure consistency if needed/desired.

@nondefault(solidity)
enum MyEnum:
    ADMIN # starts at 0 since we got the `@nondefault` behavior
    MINTER # increments the enum `uint8` index to 1
    MANAGER # ...
    USER #...
    ...
    # Cannot define more than 256 elements in an enum def (this should raise)

Backwards Compatibility

It was possible for Solidity versions <0.8.0 to define more than 255 (max_value(uint8)) enum members. The here-introduced feature will not support more members than 255 and therefore is not backward-compatible with older (i.e. <0.8.0) Solidity versions.

Dependencies

N/A.

References

Copyright

Copyright and related rights waived via CC0.

fubuloubu commented 1 year ago

Enums are treated differently in Solidity and Vyper. uint8 (in Solidity) vs. uint256 (in Vyper) & simple increments and starting from 0 (in Solidity) vs. 2**n and starting from 1 (in Vyper). I think it would be good to have the possibility to set custom numbers to get the Solidity-style behavior and leave the default to the Vyper method.

One way I think we can make this easy is to implement the following spec for "normal integer"-based enums:

enum MyEnum:
    VAL0 = 0  # setting *any* enum value stops the default numbering that uses 2**n order
    # NOTE: this removes certain "flag enum" methods e.g. `x is MyEnum....`
    VAL1  # it's not necessary to number all the values (useful for ease of use)
    VAL3 = 3  # setting a number that's greater than the next number in the series skips ahead
    VAL2 = 2  # Enum def must be in order, and cannot be defined as a number equal or less than the previous (this should raise)
    ...
    VAL256  # Still cannot define more than 256 elements in an enum def (this should raise)

One implementation issue I currently see is the uint8 vs uint256 difference.

This isn't really a concern since ABI encoding always encodes uint8 values the same as uint256

pcaversaccio commented 1 year ago

One way I think we can make this easy is to implement the following spec for "normal integer"-based enums:

enum MyEnum:
    VAL0 = 0  # setting *any* enum value stops the default numbering that uses 2**n order
    # NOTE: this removes certain "flag enum" methods e.g. `x is MyEnum....`
    VAL1  # it's not necessary to number all the values (useful for ease of use)
    VAL3 = 3  # setting a number that's greater than the next number in the series skips ahead
    VAL2 = 2  # Enum def must be in order, and cannot be defined as a number equal or less than the previous (this should raise)
    ...
    VAL256  # Still cannot define more than 256 elements in an enum def (this should raise)

At first sight, it looks good, but after rethinking it's probably not the best solution wrt readability since the behavior is somehow implied and not explicit, and we want to have max clarity for Vyper. So I would maybe suggest adding a decorator (with a param, see below why this could be powerful at some point) called @nondefault(solidity) or similar to increase the transparency about the behavior.

@nondefault(solidity)
enum MyEnum:
    ADMIN # starts at 0 since we got the `@nondefault` behavior
    MINTER # increments the enum `uint8` index  to 1
    MANAGER # ...
    USER #...
    ...
    # Cannot define more than 256 elements in an enum def (this should raise)

Maybe we could reuse this nondefault decorator for similar behavior we might see for Fe, Sway etc. in the future...

This isn't really a concern since ABI encoding always encodes uint8 values the same as uint256

Fair enough, but my point was more that before solc 0.8.0 it was possible to define more than 255 enum members, so it won't be backward-compatible, which is a non-issue if simply document it quickly.

fubuloubu commented 1 year ago

This isn't really a concern since ABI encoding always encodes uint8 values the same as uint256

Fair enough, but my point was more that before solc 0.8.0 it was possible to define more than 255 enum members, so it won't be backward-compatible, which is a non-issue if simply document it quickly.

I was meaning in terms of passing it through the ABI boundary, a uint8 value is functionally identical to a uint256 value, and doesn't even impact method selectors when it's passed as a return value.

pcaversaccio commented 1 year ago

Ok, I have to say (sorry kinda off-topic, but came to my mind since we're talking about the uint types lol): Thank you Vyper for not introducing uint as an alias for uint256.

fubuloubu commented 1 year ago

Can you refactor this issue into a VIP based on this discussion? Feels like a VIP

pcaversaccio commented 1 year ago

ok I refactored it - please take a look and adjust if needed.

fubuloubu commented 1 year ago

As an aside, it's definitely very useful to be able to "skip" unused items in an enum, in order to leave space for future growth or accommodate unusual behaviors.

Typically in a language that supports it, it would look like the following:

enum Stage:
    UNCLAIMED = 0
    # skip ahead, leaving space for future steps
    CLAIMED = 3

Additionally, something that's very useful for int-like enums is comparison, e.g. assert self.stage < Stage.CLAIMED, which is very clear (and not something that we support for flag-style enums since the set-style syntax makes more sense for other types of checks)

pcaversaccio commented 1 year ago

As an aside, it's definitely very useful to be able to "skip" unused items in an enum, in order to leave space for future growth or accommodate unusual behaviors.

I definitely see the benefit here, but as a caveat, I feel like this skipping behavior can lead to integration issues since the spacing/increments are unclear from the outside.

arjunaskykok commented 1 year ago

Now that we want to open a can of worms, can we support other non-defaults as well? Python has enum Flag.

https://docs.python.org/3/library/enum.html#enum.Flag https://dev.to/bjarnemagnussen/enum-vs-flag-for-bitmasks-in-python-2ig8

@nondefault(flag)
enum Color:
    RED
    GREEN
    BLUE
...
purple = Color.RED | Color.BLUE
Color.BLUE in purple => True
Color.GREEN in purple => False
pcaversaccio commented 1 year ago

what's the benefit over the existing flag attributes behavior?

enum Roles:
    MANAGER
    ADMIN
    USER

@external
def foo(a: Roles) -> bool:
    return a in (Roles.MANAGER | Roles.USER)

@external
def bar(a: Roles) -> bool:
    return a not in (Roles.MANAGER | Roles.USER)
arjunaskykok commented 1 year ago

Enum Flag in Python is much more flexible.

>>> from enum import Flag, auto
>>> class Color(Flag):
...   RED = auto()
...   GREEN = auto()
...   BLUE = auto()
... 
>>> white = Color.RED | Color.GREEN | Color.BLUE
>>> bool(white)
True
>>> Color.RED
<Color.RED: 1>
>>> black = Color(0)
>>> bool(black)
False
>>> len(Color)
3
>>> black in Color
False
>>> white
<Color.BLUE|GREEN|RED: 7>
>>> Color.BLUE in white
True

You cannot do this with the traditional enum.

arjunaskykok commented 1 year ago

I understand that enum in Vyper is modeled after enum Flag. But there are some missing behaviors like converting enum to bool which is not supported right now on Vyper.

arjunaskykok commented 1 year ago

Forget what I have said about enum Flag, I should direct my complaint to another ticket. Sorry for the inconvenience.

charles-cooper commented 1 year ago

(for readability, hiding the side discussion into comparison with enum Flags, please see https://github.com/vyperlang/vyper/issues/3289 to follow that side discussion)

z80dev commented 1 year ago

I think this would add a lot of complexity, specially when considering (as mentioned in the VIP) further expansion later to support enum formats from other languages beyond solidity.

if I was facing this problem today, I would define uint8 constants with values mirroring the enums from the solidity side. this approach would also work for other languages beyond solidity

that would let you accomplish the same thing, without adding a whole other layer of complexity to the enum implementation in the compiler.

edit: adding example of constant uint8

SolEnum_One: constant(uint8) = 0
SolEnum_Two: constant(uint8) = 1
SolEnum_Three: constant(uint8) = 2
charles-cooper commented 1 year ago

the issue with having non-default enum values is that it breaks a lot of semantics, including bitwise operations. to be safe, it would need to be a different type. we could maybe have two different kinds of enum types, enum and flag, but think that would be confusing.

pcaversaccio commented 1 year ago

it would need to be a different type

ouf, that would be really confusing since the type is called usually enum cross-language. I see your points (& the ones from @z80dev), but if you do something in a non-default way (such as unsafe operations as well) you should be aware of the implications (e.g. in this case flag enum methods would work anymore). Doing something a non-default way should issue a warning as well imho, so people are aware that there might be a footgun. There might be other languages that use even another approach, e.g. Fe, for which it would be nice to have interoperability. To be clear, it's solvable in the current state using constants, but I just don't think it's a nice solution. Just imagine you are forced to implement an enum type because of an EIP interface where you can't use uint8 constants.

arjunaskykok commented 1 year ago

One question.

Why do we use a decorator instead of a constructor?

@nondefault(solidity)
enum MyEnum:
  One
  Two

In Python, we use a constructor.

class MyEnum(IntFlag):
  One = auto()
  Two = auto()

Do we use a decorator because we want to emphasize nondefault?

pcaversaccio commented 1 year ago

Why do we use a decorator instead of a constructor?

Because of the design principles of Vyper (and the EVM if you will) - the constructor within a smart contract (called __init__ - similar to Python - in Vyper) is called during the contract creation transaction (e.g. to initialise immutable variables) and should not be confused with the constructor used in Python. The object we want to instantiate in Vyper is the runtime bytecode of the contract, and imho the type behavior should be clear at compile-time and not at construction-time. Therefore, it makes sense, at least to me, to use a decorator for such an approach.

arjunaskykok commented 1 year ago

If we're going to take the decorator path, maybe we should find another name for the decorator. nondefault indicates that this is one time case. But in the future, we might add other cases for enum types. For example:

@enumtype(soliditycompat)
enum MyEnum:
  One
  Two

MyEnum.One => 0

@enumtype(string)
enum MyEnum:
  One
  Two

MyEnum.Two => "Two"

I'm not saying enumtype is a good name though.

pcaversaccio commented 1 year ago

I think before we decide about the name of anything, we should decide if we want to go that path at all. That's step 1 imho.