Add documentation for proxies (#2344)

Author: frangio

.editorconfig

@@ -8,11 +8,14 @@ charset = utf-8
 end_of_line = lf
 indent_style = space
 insert_final_newline = true
-trim_trailing_whitespace = true
+trim_trailing_whitespace = false
 max_line_length = 120
 
 [*.sol]
 indent_size = 4
 
 [*.js]
 indent_size = 2
+
+[*.adoc]
+max_line_length = 0

contracts/proxy/Initializable.sol

@@ -4,16 +4,16 @@ pragma solidity >=0.4.24 <0.7.0;
 
 
 /**
- * @title Initializable
- *
- * @dev Helper contract to support initializer functions. To use it, replace
- * the constructor with a function that has the `initializer` modifier.
- * WARNING: Unlike constructors, initializer functions must be manually
- * invoked. This applies both to deploying an Initializable contract, as well
- * as extending an Initializable contract via inheritance.
- * WARNING: When used with inheritance, manual care must be taken to not invoke
- * a parent initializer twice, or ensure that all initializers are idempotent,
- * because this is not dealt with automatically as with constructors.
+ * @dev This is a base contract to aid in writing upgradeable contracts, or any kind of contract that will be deployed
+ * behind a proxy. Since a proxied contract can't have a constructor, it's common to move constructor logic to an
+ * external initializer function, usually called `initialize`. It then becomes necessary to protect this initializer
+ * function so it can only be called once. The {initializer} modifier provided by this contract will have this effect.
+ * 
+ * TIP: To avoid leaving the proxy in an uninitialized state, the initializer function should be called as early as
+ * possible by providing the encoded function call as the `_data` argument to {UpgradeableProxy-constructor}.
+ * 
+ * CAUTION: When used with inheritance, manual care must be taken to not invoke a parent initializer twice, or to ensure
+ * that all initializers are idempotent. This is not verified automatically as constructors are by Solidity.
  */
 contract Initializable {
 
@@ -28,7 +28,7 @@ contract Initializable {
     bool private _initializing;
 
     /**
-     * @dev Modifier to use in the initializer function of a contract.
+     * @dev Modifier to protect an initializer function from being invoked twice.
      */
     modifier initializer() {
         require(_initializing || _isConstructor() || !_initialized, "Initializable: contract is already initialized");

contracts/proxy/Proxy.sol

@@ -3,39 +3,20 @@
 pragma solidity ^0.6.0;
 
 /**
- * @title Proxy
- * @dev Implements delegation of calls to other contracts, with proper
- * forwarding of return values and bubbling of failures.
- * It defines a fallback function that delegates all calls to the address
- * returned by the abstract _implementation() internal function.
+ * @dev This abstract contract provides a fallback function that delegates all calls to another contract using the EVM
+ * instruction `delegatecall`. We refer to the second contract as the _implementation_ behind the proxy, and it has to
+ * be specified by overriding the virtual {_implementation} function.
+ * 
+ * Additionally, delegation to the implementation can be triggered manually through the {_fallback} function, or to a
+ * different contract through the {_delegate} function.
+ * 
+ * The success and return data of the delegated call will be returned back to the caller of the proxy.
  */
 abstract contract Proxy {
     /**
-     * @dev Fallback function.
-     * Implemented entirely in `_fallback`.
-     */
-    fallback () payable external {
-        _fallback();
-    }
-
-    /**
-     * @dev Receive function.
-     * Implemented entirely in `_fallback`.
-     */
-    receive () payable external {
-        _fallback();
-    }
-
-    /**
-     * @return The Address of the implementation.
-     */
-    function _implementation() internal virtual view returns (address);
-
-    /**
-     * @dev Delegates execution to an implementation contract.
-     * This is a low level function that doesn't return to its internal call site.
-     * It will return to the external caller whatever the implementation returns.
-     * @param implementation Address to delegate.
+     * @dev Delegates the current call to `implementation`.
+     * 
+     * This function does not return to its internall call site, it will return directly to the external caller.
      */
     function _delegate(address implementation) internal {
         // solhint-disable-next-line no-inline-assembly
@@ -60,19 +41,43 @@ abstract contract Proxy {
     }
 
     /**
-     * @dev Function that is run as the first thing in the fallback function.
-     * Can be redefined in derived contracts to add functionality.
-     * Redefinitions must call super._willFallback().
+     * @dev This is a virtual function that should be overriden so it returns the address to which the fallback function
+     * and {_fallback} should delegate.
      */
-    function _willFallback() internal virtual {
-    }
+    function _implementation() internal virtual view returns (address);
 
     /**
-     * @dev fallback implementation.
-     * Extracted to enable manual triggering.
+     * @dev Delegates the current call to the address returned by `_implementation()`.
+     * 
+     * This function does not return to its internall call site, it will return directly to the external caller.
      */
     function _fallback() internal {
         _willFallback();
         _delegate(_implementation());
     }
+
+    /**
+     * @dev Fallback function that delegates calls to the address returned by `_implementation()`. Will run if no other
+     * function in the contract matches the call data.
+     */
+    fallback () payable external {
+        _fallback();
+    }
+
+    /**
+     * @dev Fallback function that delegates calls to the address returned by `_implementation()`. Will run if call data
+     * is empty.
+     */
+    receive () payable external {
+        _fallback();
+    }
+
+    /**
+     * @dev Hook that is called before falling back to the implementation. Can happen as part of a manual `_fallback`
+     * call, or as part of the Solidity `fallback` or `receive` functions.
+     * 
+     * If overriden should call `super._willFallback()`.
+     */
+    function _willFallback() internal virtual {
+    }
 }

contracts/proxy/ProxyAdmin.sol

@@ -6,16 +6,17 @@ import "../access/Ownable.sol";
 import "./TransparentUpgradeableProxy.sol";
 
 /**
- * @title ProxyAdmin
- * @dev This contract is the admin of a proxy, and is in charge
- * of upgrading it as well as transferring it to another admin.
+ * @dev This is an auxiliary contract meant to be assigned as the admin of a {TransparentUpgradeableProxy}. For an
+ * explanation of why you would want to use this see the documentation for {TransparentUpgradeableProxy}.
  */
 contract ProxyAdmin is Ownable {
 
     /**
-     * @dev Returns the current implementation of a proxy.
-     * This is needed because only the proxy admin can query it.
-     * @return The address of the current implementation of the proxy.
+     * @dev Returns the current implementation of `proxy`.
+     * 
+     * Requirements:
+     * 
+     * - This contract must be the admin of `proxy`.
      */
     function getProxyImplementation(TransparentUpgradeableProxy proxy) public view returns (address) {
         // We need to manually run the static call since the getter cannot be flagged as view
@@ -26,8 +27,11 @@ contract ProxyAdmin is Ownable {
     }
 
     /**
-     * @dev Returns the admin of a proxy. Only the admin can query it.
-     * @return The address of the current admin of the proxy.
+     * @dev Returns the current admin of `proxy`.
+     * 
+     * Requirements:
+     * 
+     * - This contract must be the admin of `proxy`.
      */
     function getProxyAdmin(TransparentUpgradeableProxy proxy) public view returns (address) {
         // We need to manually run the static call since the getter cannot be flagged as view
@@ -38,31 +42,34 @@ contract ProxyAdmin is Ownable {
     }
 
     /**
-     * @dev Changes the admin of a proxy.
-     * @param proxy Proxy to change admin.
-     * @param newAdmin Address to transfer proxy administration to.
+     * @dev Changes the admin of `proxy` to `newAdmin`.
+     * 
+     * Requirements:
+     * 
+     * - This contract must be the current admin of `proxy`.
      */
     function changeProxyAdmin(TransparentUpgradeableProxy proxy, address newAdmin) public onlyOwner {
         proxy.changeAdmin(newAdmin);
     }
 
     /**
-     * @dev Upgrades a proxy to the newest implementation of a contract.
-     * @param proxy Proxy to be upgraded.
-     * @param implementation the address of the Implementation.
+     * @dev Upgrades `proxy` to `implementation`. See {TransparentUpgradeableProxy-upgradeTo}.
+     * 
+     * Requirements:
+     * 
+     * - This contract must be the admin of `proxy`.
      */
     function upgrade(TransparentUpgradeableProxy proxy, address implementation) public onlyOwner {
         proxy.upgradeTo(implementation);
     }
 
     /**
-     * @dev Upgrades a proxy to the newest implementation of a contract and forwards a function call to it.
-     * This is useful to initialize the proxied contract.
-     * @param proxy Proxy to be upgraded.
-     * @param implementation Address of the Implementation.
-     * @param data Data to send as msg.data in the low level call.
-     * It should include the signature and the parameters of the function to be called, as described in
-     * https://solidity.readthedocs.io/en/v0.4.24/abi-spec.html#function-selector-and-argument-encoding.
+     * @dev Upgrades `proxy` to `implementation` and calls a function on the new implementation. See
+     * {TransparentUpgradeableProxy-upgradeToAndCall}.
+     * 
+     * Requirements:
+     * 
+     * - This contract must be the admin of `proxy`.
      */
     function upgradeAndCall(TransparentUpgradeableProxy proxy, address implementation, bytes memory data) public payable onlyOwner {
         proxy.upgradeToAndCall{value: msg.value}(implementation, data);

contracts/proxy/README.adoc

@@ -0,0 +1,26 @@
+= Proxies
+
+[.readme-notice]
+NOTE: This document is better viewed at https://docs.openzeppelin.com/contracts/api/proxy
+
+This is a low-level set of contracts implementing the proxy pattern for upgradeability. For an in-depth overview of this pattern check out the xref:upgrades-plugins::proxies.adoc[Proxy Upgrade Pattern] page.
+
+The abstract {Proxy} contract implements the core delegation functionality. If the concrete proxies that we provide below are not suitable, we encourage building on top of this base contract since it contains an assembly block that may be hard to get right.
+
+Upgradeability is implemented in the {UpgradeableProxy} contract, although it provides only an internal upgrade interface. For an upgrade interface exposed externally to an admin, we provide {TransparentUpgradeableProxy}. Both of these contracts use the storage slots specified in https://eips.ethereum.org/EIPS/eip-1967[EIP1967] to avoid clashes with the storage of the implementation contract behind the proxy.
+
+CAUTION: Using upgradeable proxies correctly and securely is a difficult task that requires deep knowledge of the proxy pattern, Solidity, and the EVM. Unless you want a lot of low level control, we recommend using the xref:upgrades-plugins::index.adoc[OpenZeppelin Upgrades Plugins] for Truffle and Buidler.
+
+== Core
+
+{{Proxy}}
+
+{{UpgradeableProxy}}
+
+{{TransparentUpgradeableProxy}}
+
+== Utilities
+
+{{Initializable}}
+
+{{ProxyAdmin}}