vyperlang / vyper

Pythonic Smart Contract Language for the EVM
https://vyperlang.org
Other
4.87k stars 799 forks source link

VIP: Add `DELEGATECALL` support to Vyper #1015

Closed ben-kaufman closed 5 years ago

ben-kaufman commented 6 years ago

Simple Summary

Add a delegatecall function to Vyper to allow the use of the DELEGATECALL OPCODE.

Abstract

DELEGATECALL is a relatively new OPCODE which was first added to the EVM on the Homestead hardfork. The DELEGATECALL OPCODE allows the use of code (or "logic") of a contract to be used by any other contract. In simple terms, it allows running code of another contract in the storage and context of the caller contract.

Motivation

The DELEGATECALL OPCODE is becoming widely popular lately, mainly because of 2 use cases it allows: Proxy Contracts: A "Proxy Contract" is a contract with the sole purpose of being used as an instance of another contract without redeploying the full contract code with every instance creation. This can drastically reduce the deployment costs as a Proxy Contract deployment might cost less than 200,000 gas units.

Upgradeable Proxy Contracts: The second main use-case of the DELEGATECALL OPCODE is for "Upgradeable" contracts. Since a Proxy contract merely points to another address, it is possible to add an upgrade mechanism which will change the address the contract is delegating the call to. Note that this mechanism while highly useful in some cases, might be dengerous as it violates the "immutable code" nature of smart contracts.

Please note that for a fully generic proxy implementation would require supporting EIP-211 opcodes to some extent.

Specification

I see 3 ways which we can act by:

  1. Tell everyone to use Solidity/ some other language for this OR make generic proxy conntract (not written is Vyper) which will be precompiled and included into Vyper. These 2 are basically the same by the mean that DELEGATECALL won't be added to Vyper, but we might want to make it easier to integrate with Vyper contracts. Please see EIP-1167 for a bytecode we mmay want to use for that.

  2. Add DELEGATECALL support but with the restriction of allowing only a constant ("literal") address (posiblly to EIP-211 as well). This will allow the first use case in Vyper which is less dangerous but not the latter.

  3. Add full DELEGATECALL support as well as supprt for EIP-211 OPCODES, while adding some docmentation/ compiler warning on the use of it.

Backwards Compatibility

This VIP should be backward compatible.

Copyright

Copyright and related rights waived via CC0

fubuloubu commented 6 years ago

Trail of Bits has a great article about the complexity of upgradability patterns leveraging DELEGATECALL and other such patterns: https://blog.trailofbits.com/2018/09/05/contract-upgrade-anti-patterns/

The TLDR is that it is very difficult to do this correctly and needs assurances best implemented at the compiler level to ensure safe usage of this concept/opcode. To that end, your first option is actually very interesting to me, but I think significant work on syntax would be needed to make this feature successful.

If you could work on a syntax for how the 1st option might look in practice, that would help aid in evaluation of a serious proposal for this feature IMO. Perhaps a @proxy/@upgrade decorator around the function(s) (with some standard signature) required to perform the upgrades would let us have one Vyper contract that is split into two deployed contracts. It could be mandated to live above the __init__() function of the contract for clarity that this contract is wrapped using the proxy paradigm.

fubuloubu commented 6 years ago

ethpm integration also will make managing this much easier in my belief.

jacqueswww commented 6 years ago

The first step we decided to take with this VIP is to support adding delegate call as an option for raw_call; as raw_call is a already an advanced feature, it made sense to add it here.

Combining default + raw_call I believe we should be able to do the proxy pattern.

Also delegatecall with literal sounds like a safe option, that we might add thereafter.

jacqueswww commented 6 years ago

Starting to create a simple raw_call proof of concept.

fubuloubu commented 5 years ago

@jacqueswww can we close?

jacqueswww commented 5 years ago

Yes, can always be re-opened.

mudgen commented 4 years ago

Trail of Bits has a great article about the complexity of upgradability patterns leveraging DELEGATECALL and other such patterns: https://blog.trailofbits.com/2018/09/05/contract-upgrade-anti-patterns/

The TLDR is that it is very difficult to do this correctly and needs assurances best implemented at the compiler level to ensure safe usage of this concept/opcode. To that end, your first option is actually very interesting to me, but I think significant work on syntax would be needed to make this feature successful.

It is not a great article. It's spreads FUD about a workable solution, which is proxies that use delegatecall.

Here's what @spalladino from OpenZepplin says about that article:

FWIW, I’m not a fan of the data separation pattern for upgrades. It produces very awkward code, it makes it impossible to reuse existing libraries, it’s extremely easy to screw up just by forgetting an onlyOwner modifier in any of the functions in the storage contract, and is much more expensive in terms of gas as it requires an additional CALL for every read or write operation.

Last, I’m sad to see that the Trail of Bits post is still promoting FUD on our proxies, by touting an alleged vulnerability found on our implementation. The fact is that the post picks on an unreleased implementation of the proxies which was under development in our labs repository 1 (a space dedicated to exchange ideas), and was not present in the released version. And not to mention that, if this had been an actual vulnerability, we would have appreciated a responsible disclosure privately first, instead of finding out via a public post.