Преглед на файлове

Improve Hooks documentation (#2199)

* Improve Hooks docs

* Improve Utils docs

* Apply suggestions from code review

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Add enumerable code samples

* Remove import statement from example

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
Nicolás Venturo преди 5 години
родител
ревизия
5bb8d0245b

+ 1 - 1
contracts/token/ERC20/ERC20.sol

@@ -299,7 +299,7 @@ contract ERC20 is Context, IERC20 {
      * - when `to` is zero, `amount` of ``from``'s tokens will be burned.
      * - `from` and `to` are never both zero.
      *
-     * To learn more about hooks, head to xref:ROOT:using-hooks.adoc[Using Hooks].
+     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
      */
     function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual { }
 }

+ 1 - 1
contracts/token/ERC721/ERC721.sol

@@ -514,7 +514,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
      * - when `to` is zero, ``from``'s `tokenId` will be burned.
      * - `from` and `to` are never both zero.
      *
-     * To learn more about hooks, head to xref:ROOT:using-hooks.adoc[Using Hooks].
+     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
      */
     function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual { }
 }

+ 1 - 1
contracts/token/ERC777/ERC777.sol

@@ -492,7 +492,7 @@ contract ERC777 is Context, IERC777, IERC20 {
      * - when `to` is zero, ``from``'s `tokenId` will be burned.
      * - `from` and `to` are never both zero.
      *
-     * To learn more about hooks, head to xref:ROOT:using-hooks.adoc[Using Hooks].
+     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
      */
     function _beforeTokenTransfer(address operator, address from, address to, uint256 tokenId) internal virtual { }
 }

+ 24 - 0
contracts/utils/EnumerableMap.sol

@@ -1,5 +1,29 @@
 pragma solidity ^0.6.0;
 
+/**
+ * @dev Library for managing an enumerable variant of Solidity's
+ * https://solidity.readthedocs.io/en/latest/types.html#mapping-types[`mapping`]
+ * type.
+ *
+ * Maps have the following properties:
+ *
+ * - Entries are added, removed, and checked for existence in constant time
+ * (O(1)).
+ * - Entries are enumerated in O(n). No guarantees are made on the ordering.
+ *
+ * ```
+ * contract Example {
+ *     // Add the library methods
+ *     using EnumerableMap for EnumerableMap.UintToAddressMap;
+ *
+ *     // Declare a set state variable
+ *     EnumerableMap.UintToAddressMap private myMap;
+ * }
+ * ```
+ *
+ * As of v3.0.0, only maps of type `uint256 -> address` (`UintToAddressMap`) are
+ * supported.
+ */
 library EnumerableMap {
     // To implement this library for multiple types with as little code
     // repetition as possible, we write it in terms of a generic Map type with

+ 10 - 3
contracts/utils/EnumerableSet.sol

@@ -11,11 +11,18 @@ pragma solidity ^0.6.0;
  * (O(1)).
  * - Elements are enumerated in O(n). No guarantees are made on the ordering.
  *
- * As of v2.5.0, only `address` sets are supported.
+ * ```
+ * contract Example {
+ *     // Add the library methods
+ *     using EnumerableSet for EnumerableSet.AddressSet;
  *
- * Include with `using EnumerableSet for EnumerableSet.AddressSet;`.
+ *     // Declare a set state variable
+ *     EnumerableSet.AddressSet private mySet;
+ * }
+ * ```
  *
- * @author Alberto Cuesta Cañada
+ * As of v3.0.0, only sets of type `address` (`AddressSet`) and `uint256`
+ * (`UintSet`) are supported.
  */
 library EnumerableSet {
     // To implement this library for multiple types with as little code

+ 33 - 9
contracts/utils/README.adoc

@@ -1,25 +1,49 @@
 = Utilities
 
-Miscellaneous contracts containing utility functions, often related to working with different data types.
+Miscellaneous contracts and libraries containing utility functions you can use to improve security, work with new data types, or safely use low-level primitives.
+
+Security tools include:
+
+ * {Pausable}: provides a simple way to halt activity in your contracts (often in reponse to an external threat).
+ * {ReentrancyGuard}: protects you from https://blog.openzeppelin.com/reentrancy-after-istanbul/[reentrant calls].
+
+The {Address}, {Arrays} and {Strings} libraries provide more operations related to these native data types, while {SafeCast} adds ways to safely convert between the different signed and unsigned numeric types.
+
+For new data types:
+
+ * {Counters}: a simple way to get a counter that can only be incremented or decremented. Very useful for ID generation, counting contract activity, among others.
+ * {EnumerableMap}: like Solidity's https://solidity.readthedocs.io/en/latest/types.html#mapping-types[`mapping`] type, but with key-value _enumeration_: this will let you know how many entries a mapping has, and iterate over them (which is not possible with `mapping`).
+ * {EnumerableSet}: like {EnumerableMap}, but for https://en.wikipedia.org/wiki/Set_(abstract_data_type)[sets]. Can be used to store privileged accounts, issued IDs, etc.
+
+[NOTE]
+====
+Because Solidity does not support generic types, {EnumerableMap} and {EnumerableSet} are specialized to a limited number of key-value types.
+
+As of v3.0, {EnumerableMap} supports `uint256 -> address` (`UintToAddressMap`), and {EnumerableSet} supports `address` and `uint256` (`AddressSet` and `UintSet`).
+====
+
+Finally, {Create2} contains all necessary utilities to safely use the https://blog.openzeppelin.com/getting-the-most-out-of-create2/[`CREATE2` EVM opcode], without having to deal with low-level assembly.
 
 == Contracts
 
-{{Address}}
+{{Pausable}}
 
-{{SafeCast}}
+{{ReentrancyGuard}}
+
+== Libraries
+
+{{Address}}
 
 {{Arrays}}
 
 {{Counters}}
 
-{{Strings}}
-
-{{EnumerableSet}}
+{{Create2}}
 
 {{EnumerableMap}}
 
-{{Create2}}
+{{EnumerableSet}}
 
-{{ReentrancyGuard}}
+{{SafeCast}}
 
-{{Pausable}}
+{{Strings}}

+ 2 - 3
docs/modules/ROOT/nav.adoc

@@ -8,12 +8,11 @@
 ** xref:erc721.adoc[ERC721]
 ** xref:erc777.adoc[ERC777]
 
-* xref:using-hooks.adoc[Using Hooks]
-
 * xref:gsn.adoc[Gas Station Network]
 ** xref:gsn-strategies.adoc[Strategies]
 
-* xref:utilities.adoc[Utilities]
+* xref:extending-contracts.adoc[Extending Contracts]
 
+* xref:utilities.adoc[Utilities]
 
 * xref:releases-stability.adoc[Releases & Stability]

+ 123 - 0
docs/modules/ROOT/pages/extending-contracts.adoc

@@ -0,0 +1,123 @@
+= Extending Contracts
+
+Most of the OpenZeppelin Contracts are expected to be used via https://solidity.readthedocs.io/en/latest/contracts.html#inheritance[inheritance]: you will _inherit_ from them when writing your own contracts.
+
+This is the commonly found `is` syntax, like in `contract MyToken is ERC20`.
+
+[NOTE]
+====
+Unlike ``contract``s, Solidity ``library``s are not inherited from and instead rely on the https://solidity.readthedocs.io/en/latest/contracts.html#using-for[`using for`] syntax.
+
+OpenZeppelin Contracts has some ``library``s: most are in the xref:api:utils.adoc[Utils] directory.
+====
+
+== Overriding
+
+Inheritance is often used to add the parent contract's functionality to your own contract, but that's not all it can do. You can also _change_ how some parts of the parent behave using _overrides_.
+
+For example, imagine you want to change xref:api:access.adoc#AccessControl[`AccessControl`] so that xref:api:access.adoc#AccessControl-revokeRole-bytes32-address-[`revokeRole`] can no longer be called. This can be achieved using overrides:
+
+```solidity
+pragma solidity ^0.6.0;
+
+import "@openzeppelin/contracts/access/AccessControl.sol";
+
+contract ModifiedAccessControl is AccessControl {
+    // Override the revokeRole function
+    function revokeRole(bytes32, address) public override {
+        revert("ModifiedAccessControl: cannot revoke roles");
+    }
+}
+```
+
+The old `revokeRole` is then replaced by our override, and any calls to it will immediately revert. We cannot _remove_ the function from the contract, but reverting on all calls is good enough.
+
+=== Calling `super`
+
+Sometimes you want to _extend_ a parent's behavior, instead of outright changing it to something else. This is where `super` comes in.
+
+The `super` keyword will let you call functions defined in a parent contract, even if they are overridden. This mechanism can be used to add additional checks to a function, emit events, or otherwise add functionality as you see fit.
+
+TIP: For more information on how overrides work, head over to the https://solidity.readthedocs.io/en/latest/contracts.html#index-17[official Solidity documentation].
+
+Here is a modified version of xref:api:access.adoc#AccessControl[`AccessControl`] where xref:api:access.adoc#AccessControl-revokeRole-bytes32-address-[`revokeRole`] cannot be used to revoke the `DEFAULT_ADMIN_ROLE`:
+
+
+```solidity
+pragma solidity ^0.6.0;
+
+import "@openzeppelin/contracts/access/AccessControl.sol";
+
+contract ModifiedAccessControl is AccessControl {
+    function revokeRole(bytes32 role, address account) public override {
+        require(
+            role != DEFAULT_ADMIN_ROLE,
+            "ModifiedAccessControl: cannot revoke default admin role"
+        );
+
+        super.revokeRole(role, account);
+    }
+}
+```
+
+The `super.revokeRole` statement at the end will invoke ``AccessControl``'s original version of `revokeRole`, the same code that would've run if there were no overrides in place.
+
+NOTE: As of v3.0.0, `view` functions are not `virtual` in OpenZeppelin, and therefore cannot be overriden. We're considering https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2154[lifting this restriction] in an upcoming release. Let us know if this is something you care about!
+
+[[using-hooks]]
+== Using Hooks
+
+Sometimes, in order to extend a parent contract you will need to override multiple related functions, which leads to code duplication and increased likelihood of bugs.
+
+For example, consider implementing safe xref:api:token/ERC20.adoc#ERC20[`ERC20`] transfers in the style of xref:api:token/ERC721.adoc#IERC721Receiver[`IERC721Receiver`]. You may think overriding xref:api:token/ERC20.adoc#ERC20-transfer-address-uint256-[`transfer`] and xref:api:token/ERC20.adoc#ERC20-transferFrom-address-address-uint256-[`transferFrom`] would be enough, but what about  xref:api:token/ERC20.adoc#ERC20-_transfer-address-address-uint256-[`_transfer`] and xref:api:token/ERC20.adoc#ERC20-_mint-address-uint256-[`_mint`]? To prevent you from having to deal with these details, we introduced **hooks**.
+
+Hooks are simply functions that are called before or after some action takes place. They provide a centralized point to _hook into_ and extend the original behavior.
+
+Here's how you would implement the `IERC721Receiver` pattern in `ERC20`, using the xref:api:token/ERC20.adoc#ERC20-_beforeTokenTransfer-address-address-uint256-[`_beforeTokenTransfer`] hook:
+
+```solidity
+pragma solidity ^0.6.0;
+
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+
+contract ERC20WithSafeTransfer is ERC20 {
+    function _beforeTokenTransfer(address from, address to, uint256 amount)
+        internal virtual override
+    {
+        super._beforeTokenTransfer(from, to, amount);
+
+        require(_validRecipient(to), "ERC20WithSafeTransfer: invalid recipient");
+    }
+
+    function _validRecipient(address to) private view returns (bool) {
+        ...
+    }
+```
+
+Using hooks this way leads to cleaner and safer code, without having to rely on a deep understanding of the parent's internals.
+
+[NOTE]
+====
+Hooks are a new feature of OpenZeppelin Contracts v3.0.0, and we're eager to learn how you plan to use them!
+
+So far, the only available hook is `_beforeTransferHook`, in all of xref:api:token/ERC20.adoc#ERC20-_beforeTokenTransfer-address-address-uint256-[`ERC20`], xref:api:token/ERC721.adoc#ERC721-_beforeTokenTransfer-address-address-uint256-[ERC721] and xref:api:token/ERC777.adoc#ERC777-_beforeTokenTransfer-address-address-address-uint256-[ERC777]. If you have ideas for new hooks, let us know!
+====
+
+=== Rules of Hooks
+
+There's a few guidelines you should follow when writing code that uses hooks in order to prevent issues. They are very simple, but do make sure you follow them:
+
+1. Whenever you override a parent's hook, re-apply the `virtual` attribute to the hook. That will allow child contracts to add more functionality to the hook.
+2. **Always** call the parent's hook in your override using `super`. This will make sure all hooks in the inheritance tree are called: contracts like xref:api:token/ERC20.adoc#ERC20Pausable[`ERC20Pausable`] rely on this behavior.
+
+```solidity
+contract MyToken is ERC20 {
+    function _beforeTokenTransfer(address from, address to, uint256 amount)
+        internal virtual override // Add virtual here!
+    {
+        super._beforeTokenTransfer(from, to, amount); // Call parent hook
+        ...
+    }
+}
+```
+That's it! Enjoy simpler code using hooks!

+ 0 - 7
docs/modules/ROOT/pages/using-hooks.adoc

@@ -1,7 +0,0 @@
-# Using Hooks
-
-is a good idea.
-
-using hooks:
-  super.hook() must _always_ be called
-  if you want to cancel the process, revert. works both in before and after