Hi folks. I’ve had this idea for a while and wanted to share in case it is of any use for future design considerations.
Problem
Currently, using staticcall vs call when calling a contract method is determined by the interface of the contract being called (if the function is marked as pure/view/etc). Many times these interfaces are imported from third party libraries such as solmate / OpenZeppelin / etc. But, even when using interfaces defined locally, in order to see if an external call can modify the state the reader must jump to a different file.
Most of the time we don’t even review the downloaded dependencies as we assume they use the correct visibility for their functions, but mistakes will be made (accidentally or maliciously), and something simple such as a simple call to token.balanceOf(...) could result in a nasty reentrancy or similar issues.
Potential Solution
Make ALL contract method calls staticcall by default, and force the code in the caller contract to explicitly determine which call type to use. There are many ways in which this could be expressed in the language. A simple way could look something like this:
ERC20 token = ...;
// this is fine as `balanceOf` is `view`
uint256 bal = token.balanceOf(...);
// this would raise a compiler warning as `transfer` is not `view` or `pure`
bool a = token.transfer(...);
// this uses a regular `call`
bool b = mut token.transfer(...);
// or it could also be at the contract level, similar to payable
bool c = mut(token).transfer(...);
This is not really a replacement for visibility modifiers, they just become “guides” for what the developer should use given the interface definition.
I believe something like this would make it way easier for auditors to go through code, as they don’t need to jump between files or make assumptions about imported stuff. Also if a third party dependency or a local contract function has incorrect visibility it would be caught by the compiler and emit a warning.
I feel like this was discussed before, a long time ago. I have always been a fan of everything being as restrictive as possible by default, and requiring the user to intentionally elevate. As you mentioned, this would make auditing Solidity code significantly easier, and it would also make it easier for developers to notice when they are doing something wrong.
It’s an interesting idea. I agree that being able to easily spot external calls would be good for auditing and readability.
This would be either very breaking (if it was an error) or very annoying (if a warning) though. Probably even more annoying than the explicit conversions introduced in 0.8.0. I don’t think such a breaking change is likely in the short term but long-term who knows. We do have ideas like making variables read-only by default in the backlog and if we ever do that, such a change would be on the table too.
Some things to consider:
This would of course be transitive, right? So you’d have to make an explicitly non-static call also when calling an internal function that happens to call an external one somewhere down the call chain or else it would still be too easy to miss many non-static calls. But this requires a different syntax because you may not have access to the contract object when making an internal call.
The syntax has to support external function pointers, library functions and functions bound to types with using for.
What about built-ins that wrap precompiles, which are technically external calls? E.g. sha256()?
The syntax must be extensible enough to cover also user-defined operators. In our upcoming implementation, operators do not have to be view so something like a + b could end up making an external call.
view is a bit broader than staticcall because it can also be used on internal functions (which run still in the context of your current call). The compiler won’t let you create an internal view function that reads state or emits an event, even though that function will not be executed under staticcall. Should the new mechanism enforce that too? Or only be limited to staticcall?
Overall, not a bad idea, but the syntax and the semantics have to be a bit more fleshed out.
It’s awesome you were already thinking about this! About the syntax, yeah I didn’t think that through tbh, I have no idea of the complexity of adding such a feature so I didn’t want to go with a fully developed proposal…
What you said makes sense to me, maybe it is done at the function level just to signal that the function can modify the state (if it’s internal, it can’t be view or pure, if it is external, it uses a call). So something like:
If it’s at the function level it becomes simpler to support pointers, library functions, etc… well with the exception of external library calls as they will always be delegatecalls
No idea of how it would work with operators. Maybe “mutable blocks”? mut { ... }
So overall yeah I agree it should probably be broader than just external calls.
A type modifier on the address seems good for this.
SomeContract view foo;
SomeContract mutable foo;
SomeContract auto foo;
which sets the calltype always used for functions called through that address. This may be less “breaking” than other syntax since legacy code could be defaulted to auto. Unless there’s some backwards compatibility issue with older solidity language specs, I’m not sure I see the transitivity issue since internal calls are already restricted to comport with the view/mutable declared type on the caller function, and there is no *call() involved in making an internal call; and if you’ve been runtime called using the wrong call type you’ll be reverted; and at compile time the type checker would presumably not allow you to call a mutable function via a view address.
Your idea of making all contract method calls by default and forcing the caller contract to explicitly determine the call type is an interesting approach. It could certainly help reduce potential issues related to incorrect visibility modifiers and make it easier for auditors to review code.
By making explicit the call type, developers would be more aware of the potential state changes and reentrancy issues that can occur with external calls. It would also provide a clearer indication of the intended behavior of each method call.
