Support injecting code before dispatcher execution in a contract

[spalladino]

This is a feature request, and I wanted to know what your thoughts about it may be. Apologies in advance if this has already been discussed (couldn't find anything related here).

It'd be useful to be able to declare a sort-of contract-level modifier, that allows to inject code that executes on every call received by the contract, and executes based on raw msg data, signature, etc. This allows to provide a global behaviour to the contract, without needing to manually add a modifier to every external function, which is error-prone if one is forgotten.

We have identified two main use cases so far where this feature would be helpful:

• Global pausable contract: we have a Pausable behaviour in OpenZeppelin, which requires the user to add whenNotPaused modifiers to every public function that must be blocked. It would be much easier if the user of the contract could just add a global behaviour that blocks every external call to the contract if it's paused, except for specific unpause calls that could be detected manually via msg.sig in the global modifier.

• Block calls to proxy backing implementations: we are working heavily on proxies at the moment, which depend on a backing implementation where all calls are delegated. However, it would be interesting to be able to lock the implementations, so any direct call made to them (without going via a proxy) is automatically reverted, in order to prevent confusions from users that inadvertently call those contracts directly. Doing this via a contract-level modifier that checks that this != implementationAddress on every call is trivial, while adding this check manually to every public function is cumbersome.

Please let me know what you think about the feature, and whether you would consider adding it to the roadmap, or accept a pull request with its implementation.

[axic]

Wouldn't modifier areas (#623) be a good solution for this?

[spalladino]

@axic I wasn't familiar with modifier areas, looks interesting! However, there is one part in which modifier areas fall short, which is inheritance.

If a user extends a contract with a modifier area covering all of its public functions (the equivalent of a contract-level modifier), they must remember to also include the modifier area in all the new functions they declare. A contract-level modifier, on the other hand, would affect all functions in it, and all functions in all contracts that extend from it.

[axic]

Inheritance with modifiers is a good point, we just ran into it on a different issue.

[chriseth]

@spalladino modifier areas were an idea to solve exactly the issue you have and we already realized that inheritance would be a problem there. Do you have any suggestions for the syntax?

[spalladino]

@chriseth nothing in particular. I thought about something very similar to the existing syntax for defining a modifier, using underscore to denote the fall-through to the dispatcher (or to the next contract-level-modifier in the inheritance chain), with the option to explicitly call an existing function.

contract GlobalPausable {
  bool paused;
  undecidedKeywordForThisBlock() {
    require(!paused || msg.sig == GlobalPausable.unpause.sig);
    _;
  }
}

This would probably require introducing a new reserved keyword to identify these blocks (filter? wrap? around? onCall? decorate? tbh I don't like any of these). Alternatively, a combination of existing keywords could be used (like contract modifier); however, this could lead to confusion as there is a fundamental difference to function-level modifiers: modifier defines a block to be used later, whereas contract modifier would introduce a block that executes where it is defined.

Anyway, these are just a few ideas, and I have no strong preference towards any syntax in particular.

[Agusx1211]

Adding to the use cases, this can be used to fix the compatibility issue between the reference implementation of ERC165 and Solidity 5 ethereum/EIPs#1640

It also looks like a useful tool to fix similar bugs on new versions of Solidity.

[chriseth]

We still need a good proposal for the syntax. Also, we need to discuss how this interacts with the call protection for libraries.

[fulldecent]

I don't see how this issue is related to 165.

[Agusx1211]

@fulldecent To create a legacy implementation of ERC165, compatible with a contract having the big described in there/EIPs#1640 is necessary to disable the msg.data size checks imposed in Solidity 5, this could be performed by returning to the call before the dispatcher

[spalladino]

We still need a good proposal for the syntax.

handleCall, borrowing naming from OTP?

Also, we need to discuss how this interacts with the call protection for libraries.

I'd expect this to kick in after the library's call protection. Also, I'd expect to be able to implement a custom call protection on any contract as well.

[fulldecent]

Sorry, I am still not 100% getting this. Are you able to please provide a failing test case?

[nanexcool]

Bumpity bump!

Any way to move this forward? How can the community help? Proposing a syntax?

[github-actions]

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.