Unions of PlutusData and BuiltinData

[SCMusson]

Targeting issue #367 The following would now compile

source_code = """
from dataclasses import dataclass
from typing import Dict, List, Union
from pycardano import Datum as Anything, PlutusData

@dataclass()
class A(PlutusData):
    CONSTR_ID = 0
    foo: int

def validator(x: Union[A, int, bytes, List[int], Dict[int, int]]) -> int:
    k: int = 0
    if isinstance(x, A):
        k = 5
    elif isinstance(x, bytes):
        k = 7
    elif isinstance(x, int):
        k = 6
    elif isinstance(x, List[int]):
        k = 8
    elif isinstance(x, Dict[int, int]):
        k = 9
    return k
"""

I think it can't currently distinguish between Dict[int, int] and Dict[int, bytes] for example. I'll work on that if you think this is heading in the correct direction. I also need to sit down and think of what edge cases might break this and write some more tests.

[nielstron]

I think it would be best to only allow joining Dict[Any, Any] and List[Any] in these unions, ideally making dict and list synonyms of this type.

If a user really knows what they are doing maybe an unsafe cast from dict to Dict[int, int] can then be allowed?

Also it's important that str is not allowed in these unions but this seems to be the case already.

What happens when a function has parameter type Union[A, int] and is passed an int/A (where the type of that variable was just int / A before)? Need to wrap in PlutusData in those cases

[SCMusson]

I've just realised too that isinstance(x, List[int]) and isinstance(x, Dict[int, int]) is not valid python. The following would be thrown:

TypeError: Subscripted generics cannot be used with class and instance checks

So would the following be what you are looking for:

source_code = """
from dataclasses import dataclass
from typing import Dict, List, Union
from pycardano import Datum as Anything, PlutusData

@dataclass()
class A(PlutusData):
    CONSTR_ID = 0
    foo: int

def validator(x: Union[A, int, bytes, List[Anything], Dict[Anything, Anything]]) -> int:
    k: int = 0
    if isinstance(x, A):
        k = 5
    elif isinstance(x, bytes):
        k = 7
    elif isinstance(x, int):
        k = 6
    elif isinstance(x, List):
        k = 8
    elif isinstance(x, Dict):
        k = 9
    return k
"""

Alternatively working towards using isinstance(x, list) and isinstance(x, dict) instead or as well as isinstanec(x, List) and isinstance(x, Dict).

Let me know what you think.

[nielstron]

Nice catch! If isinstance(x, Dict) is valid python that should be fine, otherwise isinstance(x, dict) would be required.

[SCMusson]

I've had some free time so I've come back to this. The following should run:

source_code = """
from dataclasses import dataclass
from typing import Dict, List, Union
from pycardano import Datum as Anything, PlutusData

@dataclass()
class A(PlutusData):
    CONSTR_ID = 0
    foo: int

def validator(x: Union[A, int, bytes, List[Anything], Dict[Anything, Anything]]) -> int:
    k: int = 0
    if isinstance(x, A):
        k = 5
    elif isinstance(x, bytes):
        k = 7
    elif isinstance(x, int):
        k = 6
    elif isinstance(x, List):
        k = 8
    elif isinstance(x, Dict):
        k = 9
    return k
"""

Only List[Anything] and Dict[Anything, Anything] are accepted in a Union now. It will raise an error if List or Dict are subscripts inside isinstance to align with actual python. Looks like python 3.11 CI errored because of timings not because of bugs from what I'm reading.

[SCMusson]

Implemented those requested changes. Also test for failure with Bool, Str, and duplicate constr_ids in Unions. I added a check for duplicate CONSTR_IDs in Unions.

[nielstron]

As we have discovered a few missing edge cases here (see #403) I will not be able to pay out the full bug bounty for this PR. If @SCMusson is willing to supply the additional fixes I would simply offer additional time to resolve this and claim the full reward, otherwise we will have to decide a reasonable split with another developer.

[SCMusson]

I plan to return to this problem however I will be short of available time for the next week after which I will be able to devote some time to fixing this.

[SCMusson]

I think this is now fixed in #403