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

VIP: syntactic distinction for external vs internal calls #2856

Closed
Tracked by #2846
charles-cooper opened this issue May 14, 2022 · 11 comments
Closed
Tracked by #2846

VIP: syntactic distinction for external vs internal calls #2856

charles-cooper opened this issue May 14, 2022 · 11 comments
Labels
VIP: Approved VIP Approved
Milestone

Comments

@charles-cooper
Copy link
Member

charles-cooper commented May 14, 2022

Simple Summary

Add a syntactic distinction between internal and external calls. This could be in the form of a keyword, e.g. await SomeInterface(msg.sender).foo(). The keyword could also be call.

Motivation

Internal and external calls have very different semantics. External calls invoke the CALL opcode and pass execution context to another (probably untrusted) contract, while internal calls are "safe" in that they just JUMP around the local contract. This is already reflected in the semantics of internal vs external calls, for instance external calls support the keywords gas=, value=, skip_contract_check=, default_return_value=, which are not supported for internal calls.

Right now, it is relatively easy to tell visually whether a function call is an internal or external call, as internal calls will all use the self namespace (e.g. self.foo(), as opposed to self.bar.foo()). However, as we move towards a more complex module system in vyper (cf. #1954, #2431), it will become more difficult to tell at a glance whether any given call is internal or external, and will require referencing the definition of a function to determine if it is internal or external. This seems to go against vyper's goal of readability/auditability. It would also help the author of a contract, as it forces them to consider the consequences every time they call to an external contract. External calls are expensive to reason about, and the syntax should reflect that! Lastly, this VIP makes it easier to find all external calls made by a contract (which might be done during code review, audit or vulnerability scanning), as it can be done with simple text search.

This VIP proposes the use of the await keyword to signal that a call is external. This keyword is already familiar to Python programmers, and fits well with await's cooperative multitasking semantics - "this will pass execution control to something else, and we may get control back after it returns".

Potential Drawbacks

A drawback of this VIP is that it introduces a code reusability concern. For instance, if HelperContract is initially implemented as an internal module, called as follows

self.helper_contract.foo()

but then later due to code size issues, needs to be factored out into a separately deployed contract, any usages like the above would need to be changed to

await self.helper_contract.foo()

This might be annoying to do. But, maybe this is a feature, not a bug(!), in that it forces the programmer to be explicit about the scope and execution context of the helper contract.

Specification

Calls to external contracts are required to be prefixed with the await keyword. If it is not (or, conversely, if a call to an internal module is prefixed with await), a semantic or typechecker error should be thrown.

Backwards Compatibility

Users will need to syntactically update every call to an external contract.

Dependencies

References

#1954, #2431

Copyright

Copyright and related rights waived via CC0

@fubuloubu fubuloubu added the VIP: Approved VIP Approved label May 16, 2022
@charles-cooper
Copy link
Member Author

meeting notes: approved, use await keyword, wait until next "minor" release. (may reconsider the keyword in the future if actual async calls are a thing, e.g. sharded execution)

@charles-cooper
Copy link
Member Author

charles-cooper commented Jul 18, 2022

another even more exotic thought -- we could have all external fns be declared with external def, (and internal is the default visibility). this maps directly onto the python AsyncFunctionDef AST type (https://docs.python.org/3/library/ast.html#ast.AsyncFunctionDef, e.g. async def foo(): ...)

this starts to break down a bit though because in python, await is only allowed within async functions whereas in vyper, await would be allowed in any function.

@scherrey
Copy link

scherrey commented May 2, 2023

Definitely don't care for 'await' being used in this regard because it strongly infers asynchronous semantics which are not present in the EVM. 'call' or 'send' (as in sending a msg to another actor/contract entity) seem closer semantically to me. I would also suggest that some variant of this could be used to represent delegate-style calls possibly but I obviously haven't thought that one through all the way.

@AlbertoCentonze
Copy link
Contributor

Hello, maybe I'm a bit late to the party but I think it would be worth considering other naming possibilities rather than the await keyword. I get why this was proposed but I think it might be better to use something like ext or the previously proposed call. While I understand the need for an additional keyword I feel like await might create serious confusion.

@charles-cooper
Copy link
Member Author

another idea for the keyword (from kotlin) - suspend

@charles-cooper
Copy link
Member Author

How about messagecall?

@cyberthirst
Copy link
Collaborator

I also dislike await for similar reasons.

I think we both want to mark the call and also make the distinction between jmp and call explicit - as such I prefer keyword variants with call within them.

I like either call or extcall. But messagecall is also fine by me.

@AlbertoCentonze
Copy link
Contributor

What about just external? The fact that is a function call should be implicit and we're still making the developer aware of the difference. Otherwise ext or extcall seem reasonable as well to me.

@charles-cooper
Copy link
Member Author

charles-cooper commented Feb 21, 2024

just tried out a few different options, and msgcall, await, extcall and external decent on the screen - i think those options are a good number of characters

@charles-cooper
Copy link
Member Author

charles-cooper commented Feb 29, 2024

after trying out rewriting some test contracts, it seems that a further differentiation between extcall and staticcall is useful. from a UX perspective, extcall yells at the programmer "this needs its own line!", whereas staticcall looks idempotent (can call it multiple times without changing the result).

@charles-cooper
Copy link
Member Author

this was implemented in #2938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Approved VIP Approved
Projects
None yet
Development

No branches or pull requests

5 participants