opshin icon indicating copy to clipboard operation
opshin copied to clipboard

BuiltinData is Union type but fails isinstance

Open juliusfrost opened this issue 1 year ago • 2 comments

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

juliusfrost avatar Mar 08 '23 05:03 juliusfrost

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:

  • downcast the datum to int via redeemer_cast: int = redeemer
  • just change the type hint for redeemer to int

nielstron avatar Mar 08 '23 06:03 nielstron

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

  • Union should be allowed to union bytes, int, List, Dict and PlutusData with unique constructor ids
  • it should be possible to isinstance check on these union types to differentiate between them

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))

nielstron avatar Mar 08 '23 08:03 nielstron