Draft and lifecycles directories cleanup (#2122)

* Move Pausable into utils * Move Strings into utils * Move Counters into utils * Move SignedSafeMath into math * Remove ERC1046 * Make ERC20Snapshot.snapshot internal * Move ERC20Snapshot into ERC20 * Add drafts deprecation notice * Remove drafts directory * Add changelog entry * Apply suggestions from code review Co-Authored-By: Francisco Giordano <frangio.1@gmail.com> Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
OpenZeppelin · Mar 16, 2020 · c963052 · c963052
1 parent 8176a90
commit c963052
Show file tree

Hide file tree

Showing 30 changed files with 79 additions and 299 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@
 
 ### Breaking changes
  * `ECDSA`: when receiving an invalid signature, `recover` now reverts instead of returning the zero address. ([#2114](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2114))
+ * `Pausable`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `Strings`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `Counters`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `SignedSafeMath`: moved to the `math` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `ERC20Snapshot`: moved to the `token/ERC20` directory. `snapshot` was changed into an `internal` function. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
  * `Ownable`: moved to the `access` directory. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))
  * `Ownable`: removed `isOwner`. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))
  * `Secondary`: removed from the library, use `Ownable` instead. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))

diff --git a/contracts/drafts/ERC1046/ERC20Metadata.sol b/contracts/drafts/ERC1046/ERC20Metadata.sol
diff --git a/contracts/drafts/README.adoc b/contracts/drafts/README.adoc
diff --git a/contracts/lifecycle/README.adoc b/contracts/lifecycle/README.adoc
diff --git a/contracts/math/README.adoc b/contracts/math/README.adoc
@@ -6,4 +6,6 @@ These are math-related utilities.
 
 {{SafeMath}}
 
+{{SignedSafeMath}}
+
 {{Math}}
diff --git a/contracts/drafts/SignedSafeMath.sol → contracts/math/SignedSafeMath.sol b/contracts/drafts/SignedSafeMath.sol → contracts/math/SignedSafeMath.sol
diff --git a/contracts/mocks/CountersImpl.sol b/contracts/mocks/CountersImpl.sol
@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/Counters.sol";
+import "../utils/Counters.sol";
 
 contract CountersImpl {
     using Counters for Counters.Counter;

diff --git a/contracts/mocks/ERC20MetadataMock.sol b/contracts/mocks/ERC20MetadataMock.sol
diff --git a/contracts/mocks/ERC20SnapshotMock.sol b/contracts/mocks/ERC20SnapshotMock.sol
@@ -1,13 +1,17 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/ERC20Snapshot.sol";
+import "../token/ERC20/ERC20Snapshot.sol";
 
 
 contract ERC20SnapshotMock is ERC20Snapshot {
     constructor(address initialAccount, uint256 initialBalance) public {
         _mint(initialAccount, initialBalance);
     }
 
+    function snapshot() public {
+        _snapshot();
+    }
+
     function mint(address account, uint256 amount) public {
         _mint(account, amount);
     }

diff --git a/contracts/mocks/PausableMock.sol b/contracts/mocks/PausableMock.sol
@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../lifecycle/Pausable.sol";
+import "../utils/Pausable.sol";
 
 contract PausableMock is Pausable {
     bool public drasticMeasureTaken;

diff --git a/contracts/mocks/SignedSafeMathMock.sol b/contracts/mocks/SignedSafeMathMock.sol
@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/SignedSafeMath.sol";
+import "../math/SignedSafeMath.sol";
 
 contract SignedSafeMathMock {
     function mul(int256 a, int256 b) public pure returns (int256) {

diff --git a/contracts/mocks/StringsMock.sol b/contracts/mocks/StringsMock.sol
@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/Strings.sol";
+import "../utils/Strings.sol";
 
 contract StringsMock {
     function fromUint256(uint256 value) public pure returns (string memory) {

diff --git a/contracts/token/ERC20/ERC20Pausable.sol b/contracts/token/ERC20/ERC20Pausable.sol
@@ -1,7 +1,7 @@
 pragma solidity ^0.6.0;
 
 import "./ERC20.sol";
-import "../../lifecycle/Pausable.sol";
+import "../../utils/Pausable.sol";
 
 /**
  * @title Pausable token

diff --git a/contracts/drafts/ERC20Snapshot.sol → contracts/token/ERC20/ERC20Snapshot.sol b/contracts/drafts/ERC20Snapshot.sol → contracts/token/ERC20/ERC20Snapshot.sol
@@ -1,15 +1,12 @@
 pragma solidity ^0.6.0;
 
-import "../math/SafeMath.sol";
-import "../utils/Arrays.sol";
-import "../drafts/Counters.sol";
-import "../token/ERC20/ERC20.sol";
+import "../../math/SafeMath.sol";
+import "../../utils/Arrays.sol";
+import "../../utils/Counters.sol";
+import "./ERC20.sol";
 
 /**
- * @title ERC20 token with snapshots.
- * @dev Inspired by Jordi Baylina's
- * https://github.com/Giveth/minimd/blob/ea04d950eea153a04c51fa510b068b9dded390cb/contracts/MiniMeToken.sol[MiniMeToken]
- * to record historical balances.
+ * @dev ERC20 token with snapshots.
  *
  * When a snapshot is made, the balances and total supply at the time of the snapshot are recorded for later
  * access.
@@ -21,6 +18,9 @@ import "../token/ERC20/ERC20.sol";
  * @author Validity Labs AG <info@validitylabs.org>
  */
 contract ERC20Snapshot is ERC20 {
+    // Inspired by Jordi Baylina's MiniMeToken to record historical balances:
+    // https://github.com/Giveth/minimd/blob/ea04d950eea153a04c51fa510b068b9dded390cb/contracts/MiniMeToken.sol
+
     using SafeMath for uint256;
     using Arrays for uint256[];
     using Counters for Counters.Counter;
@@ -40,10 +40,12 @@ contract ERC20Snapshot is ERC20 {
 
     event Snapshot(uint256 id);
 
-    // Creates a new snapshot id. Balances are only stored in snapshots on demand: unless a snapshot was taken, a
-    // balance change will not be recorded. This means the extra added cost of storing snapshotted balances is only paid
-    // when required, but is also flexible enough that it allows for e.g. daily snapshots.
-    function snapshot() public virtual returns (uint256) {
+    /**
+     * @dev Creates a new snapshot id. Balances are only stored in snapshots on demand: unless a snapshot was taken, a
+     * balance change will not be recorded. This means the extra added cost of storing snapshotted balances is only paid
+     * when required, but is also flexible enough that it allows for e.g. daily snapshots.
+     */
+    function _snapshot() internal virtual returns (uint256) {
         _currentSnapshotId.increment();
 
         uint256 currentId = _currentSnapshotId.current();
@@ -87,26 +89,27 @@ contract ERC20Snapshot is ERC20 {
         super._burn(account, value);
     }
 
-    // When a valid snapshot is queried, there are three possibilities:
-    //  a) The queried value was not modified after the snapshot was taken. Therefore, a snapshot entry was never
-    //  created for this id, and all stored snapshot ids are smaller than the requested one. The value that corresponds
-    //  to this id is the current one.
-    //  b) The queried value was modified after the snapshot was taken. Therefore, there will be an entry with the
-    //  requested id, and its value is the one to return.
-    //  c) More snapshots were created after the requested one, and the queried value was later modified. There will be
-    //  no entry for the requested id: the value that corresponds to it is that of the smallest snapshot id that is
-    //  larger than the requested one.
-    //
-    // In summary, we need to find an element in an array, returning the index of the smallest value that is larger if
-    // it is not found, unless said value doesn't exist (e.g. when all values are smaller). Arrays.findUpperBound does
-    // exactly this.
     function _valueAt(uint256 snapshotId, Snapshots storage snapshots)
         private view returns (bool, uint256)
     {
         require(snapshotId > 0, "ERC20Snapshot: id is 0");
         // solhint-disable-next-line max-line-length
         require(snapshotId <= _currentSnapshotId.current(), "ERC20Snapshot: nonexistent id");
 
+        // When a valid snapshot is queried, there are three possibilities:
+        //  a) The queried value was not modified after the snapshot was taken. Therefore, a snapshot entry was never
+        //  created for this id, and all stored snapshot ids are smaller than the requested one. The value that corresponds
+        //  to this id is the current one.
+        //  b) The queried value was modified after the snapshot was taken. Therefore, there will be an entry with the
+        //  requested id, and its value is the one to return.
+        //  c) More snapshots were created after the requested one, and the queried value was later modified. There will be
+        //  no entry for the requested id: the value that corresponds to it is that of the smallest snapshot id that is
+        //  larger than the requested one.
+        //
+        // In summary, we need to find an element in an array, returning the index of the smallest value that is larger if
+        // it is not found, unless said value doesn't exist (e.g. when all values are smaller). Arrays.findUpperBound does
+        // exactly this.
+
         uint256 index = snapshots.ids.findUpperBound(snapshotId);
 
         if (index == snapshots.ids.length) {

diff --git a/contracts/token/ERC20/README.adoc b/contracts/token/ERC20/README.adoc
@@ -43,6 +43,8 @@ NOTE: This page is incomplete. We're working to improve it for the next release.
 
 {{ERC20Capped}}
 
+{{ERC20Snapshot}}
+
 == Utilities
 
 {{SafeERC20}}

diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol
@@ -5,7 +5,7 @@ import "./IERC721.sol";
 import "./IERC721Receiver.sol";
 import "../../math/SafeMath.sol";
 import "../../utils/Address.sol";
-import "../../drafts/Counters.sol";
+import "../../utils/Counters.sol";
 import "../../introspection/ERC165.sol";
 
 /**

diff --git a/contracts/token/ERC721/ERC721Pausable.sol b/contracts/token/ERC721/ERC721Pausable.sol
@@ -1,7 +1,7 @@
 pragma solidity ^0.6.0;
 
 import "./ERC721.sol";
-import "../../lifecycle/Pausable.sol";
+import "../../utils/Pausable.sol";
 
 /**
  * @title ERC721 Non-Fungible Pausable token

diff --git a/contracts/drafts/Counters.sol → contracts/utils/Counters.sol b/contracts/drafts/Counters.sol → contracts/utils/Counters.sol
diff --git a/contracts/lifecycle/Pausable.sol → contracts/utils/Pausable.sol b/contracts/lifecycle/Pausable.sol → contracts/utils/Pausable.sol
diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc
@@ -10,8 +10,14 @@ Miscellaneous contracts containing utility functions, often related to working w
 
 {{Arrays}}
 
+{{Counters}}
+
+{{Strings}}
+
 {{EnumerableSet}}
 
 {{Create2}}
 
 {{ReentrancyGuard}}
+
+{{Pausable}}
diff --git a/contracts/drafts/Strings.sol → contracts/utils/Strings.sol b/contracts/drafts/Strings.sol → contracts/utils/Strings.sol
@@ -6,11 +6,12 @@ pragma solidity ^0.6.0;
  */
 library Strings {
     /**
-     * @dev Converts a `uint256` to a `string`.
-     * via OraclizeAPI - MIT licence
-     * https://github.com/oraclize/ethereum-api/blob/b42146b063c7d6ee1358846c198246239e9360e8/oraclizeAPI_0.4.25.sol
+     * @dev Converts a `uint256` to its ASCII `string` representation.
      */
     function fromUint256(uint256 value) internal pure returns (string memory) {
+        // Inspired by OraclizeAPI's implementation - MIT licence
+        // https://github.com/oraclize/ethereum-api/blob/b42146b063c7d6ee1358846c198246239e9360e8/oraclizeAPI_0.4.25.sol
+
         if (value == 0) {
             return "0";
         }

diff --git a/docs/modules/ROOT/pages/drafts.adoc b/docs/modules/ROOT/pages/drafts.adoc
@@ -0,0 +1,19 @@
+= Drafts
+
+All draft contracts were either moved into a different directory or removed from the OpenZeppelin Contracts library on the https://forum.openzeppelin.com/t/openzeppelin-contracts-v3-0-beta-release/2256[v3.0.0 release].
+
+* `ERC20Migrator`: removed.
+* xref:api:token/ERC20.adoc#ERC20Snapshot[`ERC20Snapshot`]: moved to `token/ERC20`.
+* `ERC20Detailed` and `ERC1046`: removed.
+* `TokenVesting`: removed. Pending a replacement that is being discussed in https://github.com/OpenZeppelin/openzeppelin-contracts/issues/1214[`#1214`].
+* xref:api:utils.adoc#Counters[`Counters`]: moved to xref:api:utils.adoc[`utils`].
+* xref:api:utils.adoc#Strings[`Strings`]: moved to xref:api:utils.adoc[`utils`].
+* xref:api:utils.adoc#SignedSafeMath[`SignedSafeMath`]: moved to xref:api:utils.adoc[`utils`].
+
+Removed contracts are still available on the v2.5 release of OpenZeppelin Contracts, which you can install by running:
+
+```console
+$ npm install @openzeppelin/contracts@v2.5
+```
+
+Refer to the xref:2.x@contracts:api:utils.adoc[v2.x documentation] when working with them.
diff --git a/docs/modules/ROOT/pages/erc721.adoc b/docs/modules/ROOT/pages/erc721.adoc
@@ -15,7 +15,7 @@ Here's what a contract for tokenized items might look like:
 pragma solidity ^0.5.0;
 
 import "@openzeppelin/contracts/token/ERC721/ERC721Full.sol";
-import "@openzeppelin/contracts/drafts/Counters.sol";
+import "@openzeppelin/contracts/utils/Counters.sol";
 
 contract GameItem is ERC721Full {
     using Counters for Counters.Counter;

diff --git a/test/drafts/ERC1046/ERC20Metadata.test.js b/test/drafts/ERC1046/ERC20Metadata.test.js