OpShin / opshin

A simple pythonic programming language for Smart Contracts on Cardano
https://book.opshin.dev/
MIT License
140 stars 27 forks source link

BuiltinData is Union type but fails isinstance #70

Open juliusfrost opened 1 year ago

juliusfrost commented 1 year ago

Describe the bug

Anything, PlutusData, BuiltinData, Redeemer, Datum are all equivalent to pycardano.Datum

Datum = Union[PlutusData, dict, IndefiniteList, int, bytes, RawCBOR, RawPlutusData]

However, eopsin does not allow compilation with isinstance.

To Reproduce Use eopsin compile on the following script:

from eopsin.prelude import *

def validator(datum: BuiltinData, redeemer: BuiltinData, context: BuiltinData) -> None:
    if isinstance(redeemer, int):
        assert redeemer == 42

Error message:

    if isinstance(redeemer, int):
        result = redeemer == 42
    ^
AssertionError: Can only cast instances of Union types
Note that eopsin errors may be overly restrictive as they aim to prevent code with unintended consequences.

Expected behavior Script compiles and above types function as Union type.

Additional context Relevant code snippet: https://github.com/ImperatorLang/eopsin/blob/cee2aa9bb2910732f283ff9ac69e870a94024770/eopsin/type_inference.py#L222-L262

nielstron commented 1 year ago

Hi @juliusfrost, Thanks for reporting! The error message should really be changed to "can only cast instances of Unions of PlutusData"

You can not check what instance something that's not a PlutusData object is right now... though I will have to think about changing that to allow BuiltinData.

Suggestions to fix the problem at hand:

nielstron commented 1 year ago

I have updated the error message in bda917157b644e24b25644ed14e6ee7b2b05086d

Currently it is also not allowed to introduce user-defined Union types with PlutusData and int/bytes/list/dict. Since this is technically feasible with UPLC however, I think I will change a few things in the framework in a unified way

I have a bit of a philosophical issue with isinstance checks on Anything/BuiltinData, because they give the wrong impression of there actually being a type check performed, which is not what happens. Eopsin will just try to squeeze whatever it got into the the type you asked for. To maybe illustrate the problem

from eopsin.prelude import *

@dataclass()
class A(PlutusData):
   x: int

@dataclass()
class B(PlutusData):
   y: int

def validator(z: BuiltinData) -> None:
   if isinstance(z, A):
      print("Is A with " + str(z.x))
   if isinstance(z, B):
      print("Is B with " + str(z.y))

If I give it A(3) as input the logs would be

$ eopsin eval sample.py "{\"constructor\":0, \"fields\":[{\"int\":3}]}"
Is A with 3
Is B with 3

isinstance can only reasonably check the constructor id as the actual class name is not encoded in the object itself. This is not what the user would sensibly expect. Maybe I need to think a bit more about how to make this safe.

To contrast this, the following will just not compile, resulting in a safer program

from eopsin.prelude import *

@dataclass
class A(PlutusData):
   x: int

@dataclass
class B(PlutusData):
  y: int

def validator(z: Union[A, B]) -> None:
   if isinstance(z, A):
      print("Is A with " + str(z.x))
   if isinstance(z, B):
      print("Is B with " + str(z.y))
$ eopsin compile sample.py
Traceback (most recent call last):
  File "/home/niels/git/eopsin-lang/venv/bin/eopsin", line 33, in <module>
    sys.exit(load_entry_point('eopsin-lang', 'console_scripts', 'eopsin')())
  File "/home/niels/git/eopsin-lang/eopsin/__main__.py", line 134, in main
    raise SyntaxError(
  File "sample.py", line 11
    def validator(z: Union[A, B]) -> None:
                 ^
AssertionError: Union must combine PlutusData classes with unique constructors
Note that eopsin errors may be overly restrictive as they aim to prevent code with unintended consequences.

The current way of writing it with BuiltinData is this, which makes more clear what is actually happening

from eopsin.prelude import *

@dataclass()
class A(PlutusData):
    x: int

@dataclass()
class B(PlutusData):
    y: int

def validator(z: BuiltinData) -> None:
   z_a: A = z
   print("Is A with " + str(z_a.x))
   z_b: B = z
   print("Is B with " + str(z_b.y))