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

Applying type_spec_abi_is_assignable_to to ABI computed type #567

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
## Changed
* Subroutines that take ABI type of Transaction now allow any Transaction type to be passed. ([#531](https://github.com/algorand/pyteal/pull/531))
* Relaxing exact type check in `InnerTxnFieldExpr.MethodCall` by applying `abi.type_spec_is_assignable_to`. ([#561](https://github.com/algorand/pyteal/pull/561))
* Relaxing exact type check in `abi` package by applying `abi.type_spec_is_assignable_to`. ([#567](https://github.com/algorand/pyteal/pull/567))

# 0.18.1

Expand Down
37 changes: 11 additions & 26 deletions pyteal/ast/abi/address.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from enum import IntEnum
from typing import Union, Sequence, Literal, cast
from typing import Sequence, Literal, cast
from collections.abc import Sequence as CollectionSequence

from pyteal.errors import TealInputError
Expand All @@ -13,6 +13,7 @@
from pyteal.ast.abi.type import ComputedValue, BaseType
from pyteal.ast.abi.array_static import StaticArray, StaticArrayTypeSpec
from pyteal.ast.abi.uint import ByteTypeSpec, Byte
from pyteal.ast.abi.util import type_spec_is_assignable_to
from pyteal.ast.expr import Expr


Expand Down Expand Up @@ -58,18 +59,14 @@ def get(self) -> Expr:
"""
return self.stored_value.load()

def set(
def set( # type: ignore[override]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure which is preferable - What do folks think?

Options:

  • Keep the PR as is with mypy ignore.
  • Re-add removed type parameters with usage of the to-be-added cast method.

During our live discussion yesterday, I didn't realize the type error will happen. It feels to me like we should not ignore the error.

Another alternative (that's more expansive in scope) is to rework Address so that it extends TypeSpec and uses composition for value storage/retrieval.

self,
value: Union[
str,
bytes,
Expr,
Sequence[Byte],
StaticArray[Byte, Literal[AddressLength.Bytes]],
ComputedValue[StaticArray[Byte, Literal[AddressLength.Bytes]]],
"Address",
ComputedValue["Address"],
],
value: str
| bytes
| Expr
| Sequence[Byte]
| "Address"
| ComputedValue["Address"],
):
"""Set the value of this Address to the input value.

Expand All @@ -93,21 +90,9 @@ def set(

match value:
case ComputedValue():
pts = value.produced_type_spec()
if pts == AddressTypeSpec() or pts == StaticArrayTypeSpec(
ByteTypeSpec(), AddressLength.Bytes
):
return value.store_into(self)

raise TealInputError(
f"Got ComputedValue with type spec {pts}, expected AddressTypeSpec or StaticArray[Byte, Literal[AddressLength.Bytes]]"
)
return self._set_with_computed_type(value)
case BaseType():
if (
value.type_spec() == AddressTypeSpec()
or value.type_spec()
== StaticArrayTypeSpec(ByteTypeSpec(), AddressLength.Bytes)
):
if type_spec_is_assignable_to(value.type_spec(), AddressTypeSpec()):
return self.stored_value.store(value.stored_value.load())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahangsu Unrelated to the line - Based on the PR's changes, the TealInputError at the end of the method should remove StaticArray from its message.

Not requesting an immediate change because there's more pressing threads open. I leave the question to avoid forgetting the concern prior to merge.

raise TealInputError(
Expand Down
28 changes: 0 additions & 28 deletions pyteal/ast/abi/address_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,34 +112,6 @@ def test_Address_get():
assert actual == expected


def test_Address_set_StaticArray():
value_to_set = abi.StaticArray(
abi.StaticArrayTypeSpec(abi.ByteTypeSpec(), abi.AddressLength.Bytes)
)
value = abi.Address()
expr = value.set(value_to_set)
assert expr.type_of() == pt.TealType.none
assert not expr.has_return()

expected = pt.TealSimpleBlock(
[
pt.TealOp(None, pt.Op.load, value_to_set.stored_value.slot),
pt.TealOp(None, pt.Op.store, value.stored_value.slot),
]
)

actual, _ = expr.__teal__(options)
actual.addIncoming()
actual = pt.TealBlock.NormalizeBlocks(actual)

with pt.TealComponent.Context.ignoreExprEquality():
assert actual == expected

with pytest.raises(pt.TealInputError):
bogus = abi.StaticArray(abi.StaticArrayTypeSpec(abi.ByteTypeSpec(), 10))
value.set(bogus)


def test_Address_set_str():
for value_to_set in ("CEZZTYHNTVIZFZWT6X2R474Z2P3Q2DAZAKIRTPBAHL3LZ7W4O6VBROVRQA",):
value = abi.Address()
Expand Down
12 changes: 10 additions & 2 deletions pyteal/ast/abi/array_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,12 @@ def set(self, values: Sequence[T]) -> Expr:
A PyTeal expression that stores encoded sequence of ABI values in its internal
ScratchVar.
"""
from pyteal.ast.abi.util import type_spec_is_assignable_to

for index, value in enumerate(values):
if self.type_spec().value_type_spec() != value.type_spec():
if not type_spec_is_assignable_to(
value.type_spec(), self.type_spec().value_type_spec()
):
raise TealInputError(
"Cannot assign type {} at index {} to {}".format(
value.type_spec(),
Expand Down Expand Up @@ -220,7 +224,11 @@ def store_into(self, output: T) -> Expr:
Returns:
An expression that stores the byte string of the array element into value `output`.
"""
if output.type_spec() != self.produced_type_spec():
from pyteal.ast.abi.util import type_spec_is_assignable_to

if not type_spec_is_assignable_to(
self.produced_type_spec(), output.type_spec()
):
raise TealInputError("Output type does not match value type")

encodedArray = self.array.encode()
Expand Down
3 changes: 2 additions & 1 deletion pyteal/ast/abi/array_dynamic.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@ def set(
Returns:
An expression which stores the given value into this DynamicArray.
"""
from pyteal.ast.abi.util import type_spec_is_assignable_to

if isinstance(values, ComputedValue):
return self._set_with_computed_type(values)
elif isinstance(values, BaseType):
if self.type_spec() != values.type_spec():
if not type_spec_is_assignable_to(values.type_spec(), self.type_spec()):
raise TealInputError(
f"Cannot assign type {values.type_spec()} to {self.type_spec()}"
)
Expand Down
4 changes: 3 additions & 1 deletion pyteal/ast/abi/array_static.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,12 @@ def set(
Returns:
An expression which stores the given value into this StaticArray.
"""
from pyteal.ast.abi.util import type_spec_is_assignable_to

if isinstance(values, ComputedValue):
return self._set_with_computed_type(values)
elif isinstance(values, BaseType):
if self.type_spec() != values.type_spec():
if not type_spec_is_assignable_to(values.type_spec(), self.type_spec()):
raise TealInputError(
f"Cannot assign type {values.type_spec()} to {self.type_spec()}"
)
Expand Down
4 changes: 3 additions & 1 deletion pyteal/ast/abi/bool.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ def set(self, value: Union[bool, Expr, "Bool", ComputedValue["Bool"]]) -> Expr:
Returns:
An expression which stores the given value into this Bool.
"""
from pyteal.ast.abi.util import type_spec_is_assignable_to

if isinstance(value, ComputedValue):
return self._set_with_computed_type(value)

Expand All @@ -77,7 +79,7 @@ def set(self, value: Union[bool, Expr, "Bool", ComputedValue["Bool"]]) -> Expr:
checked = True

if isinstance(value, BaseType):
if value.type_spec() != self.type_spec():
if not type_spec_is_assignable_to(value.type_spec(), self.type_spec()):
raise TealInputError(
"Cannot set type bool to {}".format(value.type_spec())
)
Expand Down
6 changes: 3 additions & 3 deletions pyteal/ast/abi/string.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ def set(
An expression which stores the given value into this String.
"""

from pyteal.ast.abi.util import type_spec_is_assignable_to

match value:
case ComputedValue():
return self._set_with_computed_type(value)
case BaseType():
if value.type_spec() == StringTypeSpec() or (
value.type_spec() == DynamicArrayTypeSpec(ByteTypeSpec())
):
if type_spec_is_assignable_to(value.type_spec(), StringTypeSpec()):
return self.stored_value.store(value.stored_value.load())

raise TealInputError(
Expand Down
4 changes: 3 additions & 1 deletion pyteal/ast/abi/tuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ def _encode_tuple(values: Sequence[BaseType]) -> Expr:
def _index_tuple(
value_types: Sequence[TypeSpec], encoded: Expr, index: int, output: BaseType
) -> Expr:
from pyteal.ast.abi.util import type_spec_is_assignable_to

if not (0 <= index < len(value_types)):
raise ValueError("Index outside of range")

Expand All @@ -146,7 +148,7 @@ def _index_tuple(
offset += typeBefore.byte_length_static()

valueType = value_types[index]
if output.type_spec() != valueType:
if not type_spec_is_assignable_to(valueType, output.type_spec()):
raise TypeError("Output type does not match value type")

if type(output) is Bool:
Expand Down
14 changes: 10 additions & 4 deletions pyteal/ast/abi/type.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,12 @@ def decode(
pass

def _set_with_computed_type(self, value: "ComputedValue[BaseType]") -> Expr:
target_type_spec = value.produced_type_spec()
if self.type_spec() != target_type_spec:
from pyteal.ast.abi.util import type_spec_is_assignable_to

value_type_spec = value.produced_type_spec()
if not type_spec_is_assignable_to(value_type_spec, self.type_spec()):
raise TealInputError(
f"Cannot set {self.type_spec()} with ComputedType of {target_type_spec}"
f"Cannot set {self.type_spec()} with ComputedType of {value_type_spec}"
)
return value.store_into(self)

Expand Down Expand Up @@ -215,7 +217,11 @@ def produced_type_spec(self) -> TypeSpec:
return self.type_spec

def store_into(self, output: BaseType) -> Expr:
if output.type_spec() != self.produced_type_spec():
from pyteal.ast.abi.util import type_spec_is_assignable_to

if not type_spec_is_assignable_to(
self.produced_type_spec(), output.type_spec()
):
raise TealInputError(
f"expected type_spec {self.produced_type_spec()} but get {output.type_spec()}"
)
Expand Down
20 changes: 13 additions & 7 deletions pyteal/ast/abi/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,27 +509,31 @@ def type_spec_is_assignable_to(a: TypeSpec, b: TypeSpec) -> bool:
* value of type :code:`a` has identical encoding as value of type :code:`b`
* type :code:`b` is as general as, or more general than type :code:`a`

For `abi.NamedTuple`, we allow mutual assigning between `abi.Tuple` and `abi.NamedTuple`.
But between `abi.NamedTuple`, we only return true when the type specs are identical, or we cannot compare against generality.
On :code:`abi.Tuple`, we have following policies:

* We allow assigning from :code:`abi.NamedTuple` to :code:`abi.Tuple`, for :code:`abi.Tuple` is more general than :code:`abi.NamedTuple`.
* We disallow assigning from :code:`abi.Tuple` to :code:`abi.NamedTuple`, for :code:`abi.NamedTuple` is not as general as :code:`abi.Tuple`.
* We allow mutual assigning between two :code:`abi.Tuple` s, if they have same fields and ordering.
* We allow mutual assigning between two :code:`abi.NamedTuple` s, if they are *identical*, or we cannot compare against generality.

Some examples are illustrated as following:

=========================== =========================== ============= ====================================================================
=========================== =========================== ============= ===================================================================
Type :code:`a` Type :code:`b` Assignable? Reason
=========================== =========================== ============= ====================================================================
=========================== =========================== ============= ===================================================================
:code:`DynamicArray[Byte]` :code:`DynamicBytes` :code:`True` :code:`DynamicBytes` is as general as :code:`DynamicArray[Byte]`
:code:`DynamicBytes` :code:`DynamicArray[Byte]` :code:`True` :code:`DynamicArray[Byte]` is as general as :code:`DynamicBytes`
:code:`StaticArray[Byte,N]` :code:`StaticBytes[N]` :code:`True` :code:`StaticBytes[N]` is as general as :code:`StaticArray[Byte,N]`
:code:`StaticBytes[N]` :code:`StaticArray[Byte,N]` :code:`True` :code:`StaticArray[Byte,N]` is as general as :code:`StaticBytes[N]`
:code:`String` :code:`DynamicBytes` :code:`True` :code:`DynamicBytes` is more general than :code:`String`
:code:`DynamicBytes` :code:`String` :code:`False` :code:`String` is more specific than :code:`DynamicBytes`
:code:`Address` :code:`StaticBytes[32]` :code:`True` :code:`StaticBytes[32]` is more general than :code:`Address`
:code:`StaticBytes[32]` :code:`Address` :code:`False` :code:`Address` is more specific than :code:`StaticBytes[32]`
:code:`StaticBytes[32]` :code:`Address` :code:`False` :code:`Address` is not as general as :code:`StaticBytes[32]`
:code:`PaymentTransaction` :code:`Transaction` :code:`True` :code:`Transaction` is more general than :code:`PaymentTransaction`
:code:`Transaction` :code:`PaymentTransaction` :code:`False` :code:`PaymentTransaction` is more specific than :code`Transaction`
:code:`Uint8` :code:`Byte` :code:`True` :code:`Uint8` is as general as :code:`Byte`
:code:`Byte` :code:`Uint8` :code:`True` :code:`Byte` is as general as :code:`Uint8`
=========================== =========================== ============= ====================================================================
=========================== =========================== ============= ===================================================================

Args:
a: The abi.TypeSpec of the value on the right hand side of the assignment.
Expand All @@ -553,6 +557,8 @@ def type_spec_is_assignable_to(a: TypeSpec, b: TypeSpec) -> bool:
match a, b:
case NamedTupleTypeSpec(), NamedTupleTypeSpec():
return a == b
case TupleTypeSpec(), NamedTupleTypeSpec():
return False
case TupleTypeSpec(), TupleTypeSpec():
a, b = cast(TupleTypeSpec, a), cast(TupleTypeSpec, b)
if a.length_static() != b.length_static():
Expand All @@ -572,7 +578,7 @@ def type_spec_is_assignable_to(a: TypeSpec, b: TypeSpec) -> bool:
a, b = cast(AddressTypeSpec, a), cast(StaticArrayTypeSpec, b)
return a.length_static() == b.length_static()
case StaticArrayTypeSpec(), AddressTypeSpec():
return False
return str(a) == str(b)
case StaticArrayTypeSpec(), StaticArrayTypeSpec():
a, b = cast(StaticArrayTypeSpec, a), cast(StaticArrayTypeSpec, b)
return a.length_static() == b.length_static()
Expand Down
34 changes: 20 additions & 14 deletions pyteal/ast/abi/util_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -840,9 +840,9 @@ class SafeBidirectional(NamedTuple):
abi.Transaction,
]
),
abi.type_spec_from_annotation(NamedTDecl),
]
),
SafeBidirectional([abi.type_spec_from_annotation(NamedTDecl)]),
]
+ [
SafeBidirectional([spec])
Expand Down Expand Up @@ -889,6 +889,10 @@ class SafeAssignment(NamedTuple):
abi.StringTypeSpec(),
[abi.DynamicBytesTypeSpec(), abi.DynamicArrayTypeSpec(abi.ByteTypeSpec())],
),
SafeAssignment(
abi.AddressTypeSpec(),
[abi.StaticArrayTypeSpec(abi.ByteTypeSpec(), 32), abi.StaticBytesTypeSpec(32)],
),
SafeAssignment(
abi.type_spec_from_annotation(NamedTDecl),
[
Expand All @@ -906,22 +910,24 @@ class SafeAssignment(NamedTuple):
],
),
SafeAssignment(
abi.type_spec_from_annotation(
abi.Tuple3[
abi.Uint64,
abi.type_spec_from_annotation(NamedTDecl),
[
abi.type_spec_from_annotation(
abi.Tuple3[
abi.PaymentTransaction,
abi.Address,
abi.StaticArray[abi.Byte, Literal[16]],
],
abi.PaymentTransaction,
]
),
[abi.type_spec_from_annotation(NamedTDecl)],
abi.Uint64,
abi.Tuple3[
abi.PaymentTransaction,
abi.Address,
abi.StaticArray[abi.Byte, Literal[16]],
],
abi.Transaction,
]
),
],
),
SafeAssignment(
abi.AddressTypeSpec(),
[abi.StaticArrayTypeSpec(abi.ByteTypeSpec(), 32), abi.StaticBytesTypeSpec(32)],
abi.StaticArrayTypeSpec(abi.StringTypeSpec(), 10),
[abi.StaticArrayTypeSpec(abi.DynamicBytesTypeSpec(), 10)],
),
] + [
SafeAssignment(spec, [abi.TransactionTypeSpec()])
Expand Down