Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ) | ||
|
|
||
| def compare_type(self, other): | ||
| from vyper.semantics.types.module import InterfaceT |
There was a problem hiding this comment.
Will be solved once we refactor compare_type away
Gas ChangesNo changes detected. Summary
|
charles-cooper
changed the title
charles-cooper
changed the title
charles-cooper
changed the title
address
charles-cooper
left a comment
charles-cooper
left a comment
There was a problem hiding this comment.
lgtm pending clanker review
📊 Bytecode Size Changes (venom)No changes detected. Full bytecode sizes
|
charles-cooper
changed the title
addressaddress
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fc664a5a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from vyper.semantics.types.module import InterfaceT | ||
|
|
||
| # interfaces can be widened to addresses | ||
| return isinstance(other, InterfaceT) or super().compare_type(other) |
There was a problem hiding this comment.
This subtype hook makes an individual interface acceptable where address is expected, but expressions that first compute a common type still fail for two different interface types because get_common_types() only selects one of the operands' possible types and never adds their AddressT supertype. For example, in a function returning address, return a if cond else b still raises TypeMismatch when a: IFoo and b: IBar, and the same applies to list literals such as [a, b] assigned to DynArray[address, ...], even though each operand is now meant to widen to an address. Please either add AddressT as the common candidate for interface/address operands or contextually type these expression forms against the expected address.
Useful? React with 👍 / 👎.
| @property | ||
| def abi_type(self) -> ABIType: | ||
| return ABI_Address() |
There was a problem hiding this comment.
With the getter override removed here, foo: public(IERC20) now has a getter return type of IERC20, so external_interface/interface output prints def foo() -> IERC20 without also emitting the import or interface definition needed to resolve IERC20. That regresses the common use case where generated interface output is consumed as a standalone .vyi; before this change the same getter printed -> address and remained self-contained. Please keep generated interface text self-contained, e.g. by rendering public interface getter returns as their ABI address type or by emitting the required interface definition/import.
Useful? React with 👍 / 👎.
| from vyper.semantics.types.module import InterfaceT | ||
|
|
||
| # interfaces can be widened to addresses | ||
| return isinstance(other, InterfaceT) or super().compare_type(other) |
There was a problem hiding this comment.
This only teaches direct expected-type checks that an interface is assignable to address; membership validation still uses _is_type_in_list()/is_equivalent_to(), so a in xs is rejected when a: IFoo and xs: DynArray[address, 3] with Cannot perform membership comparison between dislike types. Since the array element type is an address position and the generated comparison would be a word equality, this leaves another address-expected context that the new widening rule does not cover.
Useful? React with 👍 / 👎.
3 participants
What I did
Allow widening interfaces to addresses:
An interface value can be used anywhere an address is expected, for example:
a: addr = token(wheretoken: IERC20)addrto implement a method returning an addressPreviously the latter was allowed by forcing the getter to return an address, this caused #4721.
The new logic only widens when needed, both cases are allowed:
asset: public(IERC20)can implement bothdef asset() -> IERC20anddef asset() -> addressFix #3954
Fix #4721
Implement #3701 (comment) (note: not the issue as a whole, only this comment)
How I did it
Make interfaces subtypes of address: for any
SomeInterfaceanInterfaceT:SomeInterface <: AddressTRemove the special case that public interface members return an address
How to verify it
pytestSee added tests
Commit message
Description for the changelog
Allow assigning interfaces where addresses are expected
Fix code like
asset: public(IERC20)not being allowed to implementdef asset() -> IERC20Cute Animal Picture